tap: Don't use EPOLLET on Qemu sockets
Currently we set EPOLLET (edge trigger) on the epoll flags for the connected Qemu Unix socket. It's not clear that there's a reason for doing this: for TCP sockets we need to use EPOLLET, because we leave data in the socket buffers for our flow control handling. That consideration doesn't apply to the way we handle the qemu socket however. Furthermore, using EPOLLET causes additional complications: 1) We don't set EPOLLET when opening /dev/net/tun for pasta mode, however we *do* set it when using pasta mode with --fd. This inconsistency doesn't seem to have broken anything, but it's odd. 2) EPOLLET requires that tap_handler_passt() loop until all data available is read (otherwise we may have data in the buffer but never get an event causing us to read it). We do that with a rather ugly goto. Worse, our condition for that goto appears to be incorrect. We'll only loop if rem is non-zero, which will only happen if we perform a blocking recv() for a partially received frame. We'll only perform that second recv() if the original recv() resulted in a partially read frame. As far as I can tell the original recv() could end on a frame boundary (never triggering the second recv()) even if there is additional data in the socket buffer. In that circumstance we wouldn't goto redo and could leave unprocessed frames in the qemu socket buffer indefinitely. This doesn't seem to have caused any problems in practice, but since there's no obvious reason to use EPOLLET here anyway, we might as well get rid of it. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
parent
9e3f2355c4
commit
4684f60344
1 changed files with 4 additions and 10 deletions
14
tap.c
14
tap.c
|
@ -989,7 +989,7 @@ static void tap_sock_reset(struct ctx *c)
|
|||
void tap_handler_passt(struct ctx *c, uint32_t events,
|
||||
const struct timespec *now)
|
||||
{
|
||||
ssize_t n, rem;
|
||||
ssize_t n;
|
||||
char *p;
|
||||
|
||||
if (events & (EPOLLRDHUP | EPOLLHUP | EPOLLERR)) {
|
||||
|
@ -997,9 +997,7 @@ void tap_handler_passt(struct ctx *c, uint32_t events,
|
|||
return;
|
||||
}
|
||||
|
||||
redo:
|
||||
p = pkt_buf;
|
||||
rem = 0;
|
||||
|
||||
tap_flush_pools();
|
||||
|
||||
|
@ -1028,7 +1026,7 @@ redo:
|
|||
* needs to be blocking.
|
||||
*/
|
||||
if (l2len > n) {
|
||||
rem = recv(c->fd_tap, p + n, l2len - n, 0);
|
||||
ssize_t rem = recv(c->fd_tap, p + n, l2len - n, 0);
|
||||
if ((n += rem) != l2len)
|
||||
return;
|
||||
}
|
||||
|
@ -1040,10 +1038,6 @@ redo:
|
|||
}
|
||||
|
||||
tap_handler(c, now);
|
||||
|
||||
/* We can't use EPOLLET otherwise. */
|
||||
if (rem)
|
||||
goto redo;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -1228,7 +1222,7 @@ void tap_listen_handler(struct ctx *c, uint32_t events)
|
|||
trace("tap: failed to set SO_SNDBUF to %i", v);
|
||||
|
||||
ref.fd = c->fd_tap;
|
||||
ev.events = EPOLLIN | EPOLLET | EPOLLRDHUP;
|
||||
ev.events = EPOLLIN | EPOLLRDHUP;
|
||||
ev.data.u64 = ref.u64;
|
||||
epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev);
|
||||
}
|
||||
|
@ -1317,7 +1311,7 @@ void tap_sock_init(struct ctx *c)
|
|||
else
|
||||
ref.type = EPOLL_TYPE_TAP_PASTA;
|
||||
|
||||
ev.events = EPOLLIN | EPOLLET | EPOLLRDHUP;
|
||||
ev.events = EPOLLIN | EPOLLRDHUP;
|
||||
ev.data.u64 = ref.u64;
|
||||
epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev);
|
||||
return;
|
||||
|
|
Loading…
Reference in a new issue