On Tue, Oct 21, 2025 at 01:52:25PM +0200, Laurent Vivier wrote:
On 20/10/2025 03:20, David Gibson wrote:
On Fri, Oct 17, 2025 at 12:31:24PM +0200, Laurent Vivier wrote:
Centralize epoll_add() and epoll_del() helper functions into new epoll_ctl.c/h files.
This also moves the union epoll_ref definition from passt.h to epoll_ctl.h where it's more logically placed.
The new epoll_add() helper simplifies adding file descriptors to epoll by taking an epoll_ref and events, handling error reporting consistently across all call sites.
Signed-off-by: Laurent Vivier
Concept looks good, some minor details in execution.
--- Makefile | 22 +++++++++++----------- epoll_ctl.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ epoll_ctl.h | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ icmp.c | 4 +--- passt.c | 2 +- passt.h | 34 ---------------------------------- pasta.c | 7 +++---- repair.c | 14 +++++--------- tap.c | 13 ++++--------- tcp.c | 2 +- tcp_splice.c | 2 +- udp.c | 2 +- udp_flow.c | 1 + util.c | 22 +++------------------- util.h | 4 +++- vhost_user.c | 8 ++------ vu_common.c | 2 +- 17 files changed, 134 insertions(+), 101 deletions(-) create mode 100644 epoll_ctl.c create mode 100644 epoll_ctl.h ... @@ -1327,14 +1327,12 @@ static void tap_backend_show_hints(struct ctx *c) static void tap_sock_unix_init(const struct ctx *c) { union epoll_ref ref = { .type = EPOLL_TYPE_TAP_LISTEN }; - struct epoll_event ev = { 0 }; listen(c->fd_tap_listen, 0); ref.fd = c->fd_tap_listen; - ev.events = EPOLLIN | EPOLLET; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap_listen, &ev); + + epoll_add(c->epollfd, EPOLLIN | EPOLLET, &ref);
Preexisting, but we should probably check for errors here.
To do what? die() ? err() to report the place where the error happens?
The case I'm concerned about is this fails, but we carry on. Then we lose 1/Nth of our packets because we're not getting events for one queue, and weird failures follow. It takes us ages to debug, because we don't consider that the fd might have silently failed to make it into the epoll set. err() would be probably be enough to make this debuggable. But honestly, if we can't put our tap fds into the epoll, we're in a sufficiently bad state that die() makes sense. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson