Skip to content

Commit

Permalink
Forward correct evset in vsock muxer
Browse files Browse the repository at this point in the history
When forwarding an epoll event from the unix muxer to the
targeted connection event handler, the eventset the connection
registered is forwarded instead of the actual epoll
operation (IN/OUT).

For example, if the connection was registered for EPOLLIN,
and receives an EPOLLOUT, the connection will actually handle
an EPOLLOUT.

This is the root cause of previous bug, which caused the
introduction of some workarounds (i.e: handling ewouldblock
when reading after receiving EPOLLIN, which should never happen).

When matching the connection, we retrieve and use the evset of
the connection instead of the one passed as a parameter.
The compiler does not complain for an unused variable because
it was first logged in a debug! statement.

This is an unfortunate naming mistake that caused a lot of problems.

Signed-off-by: Eduard Kyvenko <eduard.kyvenko@gmail.com>
  • Loading branch information
halfzebra authored and alindima committed Jun 29, 2021
1 parent 5898fe1 commit 38bc78b
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions src/devices/src/virtio/vsock/unix/muxer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ impl VsockEpollListener for VsockMuxer {
/// Get the epoll events to be polled upstream.
///
/// Since the polled FD is a nested epoll FD, we're only interested in EPOLLIN events (i.e.
/// some event occured on one of the FDs registered under our epoll FD).
/// some event occurred on one of the FDs registered under our epoll FD).
fn get_polled_evset(&self) -> EventSet {
EventSet::IN
}
Expand Down Expand Up @@ -327,30 +327,29 @@ impl VsockMuxer {
local_port_set: HashSet::with_capacity(defs::MAX_CONNECTIONS),
};

// Listen on the host initiated socket, for incomming connections.
// Listen on the host initiated socket, for incoming connections.
muxer.add_listener(muxer.host_sock.as_raw_fd(), EpollListener::HostSock)?;
Ok(muxer)
}

/// Handle/dispatch an epoll event to its listener.
fn handle_event(&mut self, fd: RawFd, evset: EventSet) {
fn handle_event(&mut self, fd: RawFd, event_set: EventSet) {
debug!(
"vsock: muxer processing event: fd={}, evset={:?}",
fd, evset
fd, event_set
);

match self.listener_map.get_mut(&fd) {
// This event needs to be forwarded to a `MuxerConnection` that is listening for
// it.
Some(EpollListener::Connection { key, evset }) => {
Some(EpollListener::Connection { key, evset: _ }) => {
let key_copy = *key;
let evset_copy = *evset;
// The handling of this event will most probably mutate the state of the
// receiving conection. We'll need to check for new pending RX, event set
// receiving connection. We'll need to check for new pending RX, event set
// mutation, and all that, so we're wrapping the event delivery inside those
// checks.
self.apply_conn_mutation(key_copy, |conn| {
conn.notify(evset_copy);
conn.notify(event_set);
});
}

Expand Down Expand Up @@ -412,7 +411,10 @@ impl VsockMuxer {
}

_ => {
info!("vsock: unexpected event: fd={:?}, evset={:?}", fd, evset);
info!(
"vsock: unexpected event: fd={:?}, evset={:?}",
fd, event_set
);
METRICS.vsock.muxer_event_fails.inc();
}
}
Expand Down

0 comments on commit 38bc78b

Please sign in to comment.