Skip to content

Commit

Permalink
Fix performance collapse in network.
Browse files Browse the repository at this point in the history
The code instructing the polling thread to poll had a race condition
that meant that sometimes the polling thread would poll only for reads
or writes (or neither, just for subsequent wake-ups).

With this, the iperf test in #777 runs to completion with an average of
around 3Gb/s.

Fixes #681

Related to #499
  • Loading branch information
davidchisnall authored and Sean T. Allen committed Aug 14, 2020
1 parent 9ecec0a commit 9131c84
Showing 1 changed file with 5 additions and 21 deletions.
26 changes: 5 additions & 21 deletions src/host_interface/virtio_netdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,6 @@ struct netdev_fd
{
/* file-descriptor based device */
int fd;
/*
* Controlls the poll mask for fd. Can be acccessed concurrently from
* poll, tx, or rx routines but there is no need for syncronization
* because:
*
* (a) TX and RX routines set different variables so even if they update
* at the same time there is no race condition
*
* (b) Even if poll and TX / RX update at the same time poll cannot
* stall: when poll resets the poll variable we know that TX / RX will
* run which means that eventually the poll variable will be set.
*/
int poll_tx, poll_rx;
/* control pipe */
int pipe[2];
};
Expand Down Expand Up @@ -161,17 +148,14 @@ static int virtio_net_fd_net_poll(uint8_t netdev_id)
struct pollfd pfds[2] = {
{
.fd = nd_fd->fd,
.events = POLLIN | POLLOUT | POLLPRI,
},
{
.fd = nd_fd->pipe[0],
.events = POLLIN,
},
};

if (nd_fd->poll_rx)
pfds[0].events |= POLLIN | POLLPRI;
if (nd_fd->poll_tx)
pfds[0].events |= POLLOUT;

do
{
Expand Down Expand Up @@ -206,13 +190,11 @@ static int virtio_net_fd_net_poll(uint8_t netdev_id)
ret = 0;
if (pfds[0].revents & (POLLIN | POLLPRI))
{
nd_fd->poll_rx = 0;
ret |= DEV_NET_POLL_RX;
}

if (pfds[0].revents & POLLOUT)
{
nd_fd->poll_tx = 0;
ret |= DEV_NET_POLL_TX;
}
return ret;
Expand Down Expand Up @@ -256,7 +238,7 @@ static int virtio_net_fd_net_tx(uint8_t netdev_id, struct iovec* iov, int cnt)
switch (errno)
{
case EAGAIN:
nd_fd->poll_tx = 1;
{
int pipe_ret = write(nd_fd->pipe[1], &tmp, 1);

// Check if there was an error but the fd has not been closed
Expand All @@ -269,6 +251,7 @@ static int virtio_net_fd_net_tx(uint8_t netdev_id, struct iovec* iov, int cnt)
errno,
strerror(errno));
break;
}

// Check if the fd has been closed and return error
case EBADF:
Expand Down Expand Up @@ -307,7 +290,7 @@ static int virtio_net_fd_net_rx(uint8_t netdev_id, struct iovec* iov, int cnt)
switch (errno)
{
case EAGAIN:
nd_fd->poll_rx = 1;
{
int pipe_ret = write(nd_fd->pipe[1], &tmp, 1);

// Check if there was an error but the fd has not been closed
Expand All @@ -321,6 +304,7 @@ static int virtio_net_fd_net_rx(uint8_t netdev_id, struct iovec* iov, int cnt)
errno,
strerror(errno));
break;
}

// Check if the fd has been closed and return error
case EBADF:
Expand Down

0 comments on commit 9131c84

Please sign in to comment.