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 --- 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 diff --git a/Makefile b/Makefile index 3328f8324140..91e037b8fd3c 100644 --- a/Makefile +++ b/Makefile @@ -37,23 +37,23 @@ FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE) FLAGS += -DVERSION=\"$(VERSION)\" FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) -PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c fwd.c \ - icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c \ - ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c \ - repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c \ - udp_vu.c util.c vhost_user.c virtio.c vu_common.c +PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c \ + flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c \ + log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c \ + pif.c repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c \ + udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c QRAP_SRCS = qrap.c PASST_REPAIR_SRCS = passt-repair.c SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1 -PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h flow.h fwd.h \ - flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h isolation.h \ - lineread.h log.h migrate.h ndp.h netlink.h packet.h passt.h pasta.h \ - pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h \ - tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h udp_internal.h \ - udp_vu.h util.h vhost_user.h virtio.h vu_common.h +PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h epoll_ctl.h \ + flow.h fwd.h flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h \ + isolation.h lineread.h log.h migrate.h ndp.h netlink.h packet.h \ + passt.h pasta.h pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h \ + tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h \ + udp_internal.h udp_vu.h util.h vhost_user.h virtio.h vu_common.h HEADERS = $(PASST_HEADERS) seccomp.h C := \#include \nint main(){int a=getrandom(0, 0, 0);} diff --git a/epoll_ctl.c b/epoll_ctl.c new file mode 100644 index 000000000000..0a06350f87a5 --- /dev/null +++ b/epoll_ctl.c @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* epoll_ctl.c - epoll manipulation helpers + * + * Copyright Red Hat + * Author: Laurent Vivier + */ + +#include + +#include "epoll_ctl.h" + +/** + * epoll_add() - Add a file descriptor to an epollfd + * @epollfd: epoll file descriptor to add to + * @events: epoll events + * @ref: epoll reference for the file descriptor (includes fd and metadata) + * + * Return: 0 on success, negative errno on failure + */ +int epoll_add(int epollfd, uint32_t events, union epoll_ref *ref) +{ + struct epoll_event ev; + int ret; + + ev.events = events; + ev.data.u64 = ref->u64; + + ret = epoll_ctl(epollfd, EPOLL_CTL_ADD, ref->fd, &ev); + if (ret == -1) { + ret = -errno; + warn("Failed to add fd to epoll: %s", strerror_(-ret)); + } + + return ret; +} + +/** + * epoll_del() - Remove a file descriptor from an epollfd + * @epollfd: epoll file descriptor to remove from + * @fd: File descriptor to remove + */ +void epoll_del(int epollfd, int fd) +{ + epoll_ctl(epollfd, EPOLL_CTL_DEL, fd, NULL); +} diff --git a/epoll_ctl.h b/epoll_ctl.h new file mode 100644 index 000000000000..cf92b0f63f26 --- /dev/null +++ b/epoll_ctl.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright Red Hat + * Author: Laurent Vivier + */ + +#ifndef EPOLL_CTL_H +#define EPOLL_CTL_H + +#include + +#include "util.h" +#include "passt.h" +#include "epoll_type.h" +#include "flow.h" +#include "tcp.h" +#include "udp.h" + +/** + * union epoll_ref - Breakdown of reference for epoll fd bookkeeping + * @type: Type of fd (tells us what to do with events) + * @fd: File descriptor number (implies < 2^24 total descriptors) + * @flow: Index of the flow this fd is linked to + * @tcp_listen: TCP-specific reference part for listening sockets + * @udp: UDP-specific reference part + * @data: Data handled by protocol handlers + * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone + * @queue: vhost-user queue index for this fd + * @u64: Opaque reference for epoll_ctl() and epoll_wait() + */ +union epoll_ref { + struct { + enum epoll_type type:8; + int32_t fd:FD_REF_BITS; + union { + uint32_t flow; + flow_sidx_t flowside; + union tcp_listen_epoll_ref tcp_listen; + union udp_listen_epoll_ref udp; + uint32_t data; + int nsdir_fd; + int queue; + }; + }; + uint64_t u64; +}; +static_assert(sizeof(union epoll_ref) <= sizeof(union epoll_data), + "epoll_ref must have same size as epoll_data"); + +int epoll_add(int epollfd, uint32_t events, union epoll_ref *ref); +void epoll_del(int epollfd, int fd); +#endif /* EPOLL_CTL_H */ diff --git a/icmp.c b/icmp.c index bd3108a21675..c26561da80bf 100644 --- a/icmp.c +++ b/icmp.c @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -23,10 +22,8 @@ #include #include #include -#include #include #include -#include #include #include @@ -41,6 +38,7 @@ #include "inany.h" #include "icmp.h" #include "flow_table.h" +#include "epoll_ctl.h" #define ICMP_ECHO_TIMEOUT 60 /* s, timeout for ICMP socket activity */ #define ICMP_NUM_IDS (1U << 16) diff --git a/passt.c b/passt.c index 31fbb75b1b12..e595f13a56f7 100644 --- a/passt.c +++ b/passt.c @@ -19,7 +19,6 @@ * created in a separate network namespace). */ -#include #include #include #include @@ -53,6 +52,7 @@ #include "vu_common.h" #include "migrate.h" #include "repair.h" +#include "epoll_ctl.h" #define EPOLL_EVENTS 8 diff --git a/passt.h b/passt.h index 0075eb4b3b16..befe56bb167b 100644 --- a/passt.h +++ b/passt.h @@ -35,40 +35,6 @@ union epoll_ref; #define MAC_OUR_LAA \ ((uint8_t [ETH_ALEN]){0x9a, 0x55, 0x9a, 0x55, 0x9a, 0x55}) -/** - * union epoll_ref - Breakdown of reference for epoll fd bookkeeping - * @type: Type of fd (tells us what to do with events) - * @fd: File descriptor number (implies < 2^24 total descriptors) - * @flow: Index of the flow this fd is linked to - * @tcp_listen: TCP-specific reference part for listening sockets - * @udp: UDP-specific reference part - * @icmp: ICMP-specific reference part - * @data: Data handled by protocol handlers - * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone - * @queue: vhost-user queue index for this fd - * @u64: Opaque reference for epoll_ctl() and epoll_wait() - */ -union epoll_ref { - struct { - enum epoll_type type:8; -#define FD_REF_BITS 24 -#define FD_REF_MAX ((int)MAX_FROM_BITS(FD_REF_BITS)) - int32_t fd:FD_REF_BITS; - union { - uint32_t flow; - flow_sidx_t flowside; - union tcp_listen_epoll_ref tcp_listen; - union udp_listen_epoll_ref udp; - uint32_t data; - int nsdir_fd; - int queue; - }; - }; - uint64_t u64; -}; -static_assert(sizeof(union epoll_ref) <= sizeof(union epoll_data), - "epoll_ref must have same size as epoll_data"); - /* Large enough for ~128 maximum size frames */ #define PKT_BUF_BYTES (8UL << 20) diff --git a/pasta.c b/pasta.c index 687406b6e736..e905f6d33b95 100644 --- a/pasta.c +++ b/pasta.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -49,6 +48,7 @@ #include "isolation.h" #include "netlink.h" #include "log.h" +#include "epoll_ctl.h" #define HOSTNAME_PREFIX "pasta-" @@ -444,7 +444,6 @@ static int pasta_netns_quit_timer(void) */ void pasta_netns_quit_init(const struct ctx *c) { - struct epoll_event ev = { .events = EPOLLIN }; int flags = O_NONBLOCK | O_CLOEXEC; struct statfs s = { 0 }; bool try_inotify = true; @@ -487,8 +486,8 @@ void pasta_netns_quit_init(const struct ctx *c) die("netns monitor file number %i too big, exiting", fd); ref.fd = fd; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev); + + epoll_add(c->epollfd, EPOLLIN, &ref); } /** diff --git a/repair.c b/repair.c index f6b1bf36479c..c8f4737fa62a 100644 --- a/repair.c +++ b/repair.c @@ -22,6 +22,7 @@ #include "inany.h" #include "flow.h" #include "flow_table.h" +#include "epoll_ctl.h" #include "repair.h" @@ -47,7 +48,6 @@ static int repair_nfds; void repair_sock_init(const struct ctx *c) { union epoll_ref ref = { .type = EPOLL_TYPE_REPAIR_LISTEN }; - struct epoll_event ev = { 0 }; if (c->fd_repair_listen == -1) return; @@ -58,9 +58,7 @@ void repair_sock_init(const struct ctx *c) } ref.fd = c->fd_repair_listen; - ev.events = EPOLLIN | EPOLLHUP | EPOLLET; - ev.data.u64 = ref.u64; - if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_repair_listen, &ev)) + if (epoll_add(c->epollfd, EPOLLIN | EPOLLHUP | EPOLLET, &ref)) err_perror("repair helper socket epoll_ctl(), won't migrate"); } @@ -74,7 +72,6 @@ void repair_sock_init(const struct ctx *c) int repair_listen_handler(struct ctx *c, uint32_t events) { union epoll_ref ref = { .type = EPOLL_TYPE_REPAIR }; - struct epoll_event ev = { 0 }; struct ucred ucred; socklen_t len; int rc; @@ -112,10 +109,9 @@ int repair_listen_handler(struct ctx *c, uint32_t events) info("Accepted TCP_REPAIR helper, PID %i", ucred.pid); ref.fd = c->fd_repair; - ev.events = EPOLLHUP | EPOLLET; - ev.data.u64 = ref.u64; - if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_repair, &ev)) { - rc = errno; + + rc = epoll_add(c->epollfd, EPOLLHUP | EPOLLET, &ref); + if (rc < 0) { debug_perror("epoll_ctl() on TCP_REPAIR helper socket"); close(c->fd_repair); c->fd_repair = -1; diff --git a/tap.c b/tap.c index 134c37a72979..873874796f79 100644 --- a/tap.c +++ b/tap.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -61,6 +60,7 @@ #include "log.h" #include "vhost_user.h" #include "vu_common.h" +#include "epoll_ctl.h" /* Maximum allowed frame lengths (including L2 header) */ @@ -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); } /** @@ -1343,7 +1341,6 @@ static void tap_sock_unix_init(const struct ctx *c) */ static void tap_start_connection(const struct ctx *c) { - struct epoll_event ev = { 0 }; union epoll_ref ref = { 0 }; ref.fd = c->fd_tap; @@ -1359,9 +1356,7 @@ static void tap_start_connection(const struct ctx *c) break; } - ev.events = EPOLLIN | EPOLLRDHUP; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); + epoll_add(c->epollfd, EPOLLIN | EPOLLRDHUP, &ref); if (c->ifi4) arp_send_init_req(c); diff --git a/tcp.c b/tcp.c index 9100c67e57e0..c2d842bbdf4f 100644 --- a/tcp.c +++ b/tcp.c @@ -279,7 +279,6 @@ #include #include #include -#include #include #include #include @@ -309,6 +308,7 @@ #include "tcp_internal.h" #include "tcp_buf.h" #include "tcp_vu.h" +#include "epoll_ctl.h" /* * The size of TCP header (including options) is given by doff (Data Offset) diff --git a/tcp_splice.c b/tcp_splice.c index 666ee62b738f..6f21184bdc55 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -44,7 +44,6 @@ #include #include #include -#include #include #include @@ -56,6 +55,7 @@ #include "siphash.h" #include "inany.h" #include "flow.h" +#include "epoll_ctl.h" #include "flow_table.h" diff --git a/udp.c b/udp.c index 86585b7e0942..3812d5c2336f 100644 --- a/udp.c +++ b/udp.c @@ -94,7 +94,6 @@ #include #include #include -#include #include #include #include @@ -115,6 +114,7 @@ #include "flow_table.h" #include "udp_internal.h" #include "udp_vu.h" +#include "epoll_ctl.h" #define UDP_MAX_FRAMES 32 /* max # of frames to receive at once */ diff --git a/udp_flow.c b/udp_flow.c index 84973f807167..d9c75f1bb1d8 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -15,6 +15,7 @@ #include "passt.h" #include "flow_table.h" #include "udp_internal.h" +#include "epoll_ctl.h" #define UDP_CONN_TIMEOUT 180 /* s, timeout for ephemeral or local bind */ diff --git a/util.c b/util.c index 88a91b1100f5..b2490123590a 100644 --- a/util.c +++ b/util.c @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -35,6 +34,7 @@ #include "packet.h" #include "log.h" #include "pcap.h" +#include "epoll_ctl.h" #ifdef HAS_GETRANDOM #include #endif @@ -58,7 +58,6 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, sa_family_t af = ((const struct sockaddr *)sa)->sa_family; union epoll_ref ref = { .type = type, .data = data }; bool freebind = false; - struct epoll_event ev; int fd, y = 1, ret; uint8_t proto; int socktype; @@ -172,13 +171,9 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, return ret; } - ev.events = EPOLLIN; - ev.data.u64 = ref.u64; - if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) { - ret = -errno; - warn("L4 epoll_ctl: %s", strerror_(-ret)); + ret = epoll_add(c->epollfd, EPOLLIN, &ref); + if (ret < 0) return ret; - } return fd; } @@ -994,17 +989,6 @@ void raw_random(void *buf, size_t buflen) die("Unexpected EOF on random data source"); } -/** - * epoll_del() - Remove a file descriptor from our passt epoll - * @epollfd: epoll file descriptor to add to - * @fd: File descriptor to remove - */ -void epoll_del(int epollfd, int fd) -{ - epoll_ctl(epollfd, EPOLL_CTL_DEL, fd, NULL); - -} - /** * encode_domain_name() - Encode domain name according to RFC 1035, section 3.1 * @buf: Buffer to fill in with encoded domain name diff --git a/util.h b/util.h index c61cbef357aa..8e4b4c5c6032 100644 --- a/util.h +++ b/util.h @@ -193,6 +193,9 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, #define SNDBUF_BIG (4ULL * 1024 * 1024) #define SNDBUF_SMALL (128ULL * 1024) +#define FD_REF_BITS 24 +#define FD_REF_MAX ((int)MAX_FROM_BITS(FD_REF_BITS)) + #include #include #include @@ -300,7 +303,6 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m) #define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__) void raw_random(void *buf, size_t buflen); -void epoll_del(int epollfd, int fd); /* * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, diff --git a/vhost_user.c b/vhost_user.c index f8324c59cc6c..aea1e2cbcea5 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -32,8 +32,6 @@ #include #include #include -#include -#include #include #include #include @@ -45,6 +43,7 @@ #include "vhost_user.h" #include "pcap.h" #include "migrate.h" +#include "epoll_ctl.h" /* vhost-user version we are compatible with */ #define VHOST_USER_VERSION 1 @@ -753,11 +752,8 @@ static void vu_set_watch(const struct vu_dev *vdev, int idx) .fd = vdev->vq[idx].kick_fd, .queue = idx }; - struct epoll_event ev = { 0 }; - ev.data.u64 = ref.u64; - ev.events = EPOLLIN; - epoll_ctl(vdev->context->epollfd, EPOLL_CTL_ADD, ref.fd, &ev); + epoll_add(vdev->context->epollfd, EPOLLIN, &ref); } /** diff --git a/vu_common.c b/vu_common.c index b716070ea3c3..b13b7c308fd8 100644 --- a/vu_common.c +++ b/vu_common.c @@ -6,7 +6,6 @@ */ #include -#include #include #include #include @@ -19,6 +18,7 @@ #include "pcap.h" #include "vu_common.h" #include "migrate.h" +#include "epoll_ctl.h" #define VU_MAX_TX_BUFFER_NB 2 -- 2.50.1

Change epoll_del() to accept the epoll file descriptor directly instead of the full context structure. This simplifies the interface and aligns with the threading refactoring by reducing dependency on the context structure for basic epoll operations as we will manage an epollfd per thread. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- icmp.c | 2 +- tap.c | 2 +- tcp.c | 6 +++--- tcp_splice.c | 4 ++-- udp_flow.c | 4 ++-- util.c | 6 +++--- util.h | 2 +- vhost_user.c | 6 +++--- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/icmp.c b/icmp.c index 6dffafb0bf54..bd3108a21675 100644 --- a/icmp.c +++ b/icmp.c @@ -151,7 +151,7 @@ unexpected: static void icmp_ping_close(const struct ctx *c, const struct icmp_ping_flow *pingf) { - epoll_del(c, pingf->sock); + epoll_del(c->epollfd, pingf->sock); close(pingf->sock); flow_hash_remove(c, FLOW_SIDX(pingf, INISIDE)); } diff --git a/tap.c b/tap.c index 95d309bd1938..134c37a72979 100644 --- a/tap.c +++ b/tap.c @@ -1142,7 +1142,7 @@ void tap_sock_reset(struct ctx *c) } /* Close the connected socket, wait for a new connection */ - epoll_del(c, c->fd_tap); + epoll_del(c->epollfd, c->fd_tap); close(c->fd_tap); c->fd_tap = -1; if (c->mode == MODE_VU) diff --git a/tcp.c b/tcp.c index 91aa33016f36..9100c67e57e0 100644 --- a/tcp.c +++ b/tcp.c @@ -511,9 +511,9 @@ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn) if (conn->events == CLOSED) { if (conn->in_epoll) - epoll_del(c, conn->sock); + epoll_del(c->epollfd, conn->sock); if (conn->timer != -1) - epoll_del(c, conn->timer); + epoll_del(c->epollfd, conn->timer); return 0; } @@ -3456,7 +3456,7 @@ int tcp_flow_migrate_source_ext(const struct ctx *c, if (c->migrate_no_linger) close(s); else - epoll_del(c, s); + epoll_del(c->epollfd, s); /* Adjustments unrelated to FIN segments: sequence numbers we dumped are * based on the end of the queues. diff --git a/tcp_splice.c b/tcp_splice.c index 26cb63064583..666ee62b738f 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -204,8 +204,8 @@ static void conn_flag_do(const struct ctx *c, struct tcp_splice_conn *conn, } if (flag == CLOSING) { - epoll_del(c, conn->s[0]); - epoll_del(c, conn->s[1]); + epoll_del(c->epollfd, conn->s[0]); + epoll_del(c->epollfd, conn->s[1]); } } diff --git a/udp_flow.c b/udp_flow.c index cef3fb588bbe..84973f807167 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -51,7 +51,7 @@ void udp_flow_close(const struct ctx *c, struct udp_flow *uflow) flow_foreach_sidei(sidei) { flow_hash_remove(c, FLOW_SIDX(uflow, sidei)); if (uflow->s[sidei] >= 0) { - epoll_del(c, uflow->s[sidei]); + epoll_del(c->epollfd, uflow->s[sidei]); close(uflow->s[sidei]); uflow->s[sidei] = -1; } @@ -88,7 +88,7 @@ static int udp_flow_sock(const struct ctx *c, if (flowside_connect(c, s, pif, side) < 0) { int rc = -errno; - epoll_del(c, s); + epoll_del(c->epollfd, s); close(s); flow_dbg_perror(uflow, "Couldn't connect flow socket"); diff --git a/util.c b/util.c index c492f904b3fc..88a91b1100f5 100644 --- a/util.c +++ b/util.c @@ -996,12 +996,12 @@ void raw_random(void *buf, size_t buflen) /** * epoll_del() - Remove a file descriptor from our passt epoll - * @c: Execution context + * @epollfd: epoll file descriptor to add to * @fd: File descriptor to remove */ -void epoll_del(const struct ctx *c, int fd) +void epoll_del(int epollfd, int fd) { - epoll_ctl(c->epollfd, EPOLL_CTL_DEL, fd, NULL); + epoll_ctl(epollfd, EPOLL_CTL_DEL, fd, NULL); } diff --git a/util.h b/util.h index 22eaac56e719..c61cbef357aa 100644 --- a/util.h +++ b/util.h @@ -300,7 +300,7 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m) #define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__) void raw_random(void *buf, size_t buflen); -void epoll_del(const struct ctx *c, int fd); +void epoll_del(int epollfd, int fd); /* * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, diff --git a/vhost_user.c b/vhost_user.c index 223332d5018e..f8324c59cc6c 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -733,7 +733,7 @@ static bool vu_get_vring_base_exec(struct vu_dev *vdev, vdev->vq[idx].call_fd = -1; } if (vdev->vq[idx].kick_fd != -1) { - epoll_del(vdev->context, vdev->vq[idx].kick_fd); + epoll_del(vdev->context->epollfd, vdev->vq[idx].kick_fd); close(vdev->vq[idx].kick_fd); vdev->vq[idx].kick_fd = -1; } @@ -801,7 +801,7 @@ static bool vu_set_vring_kick_exec(struct vu_dev *vdev, vu_check_queue_msg_file(vmsg); if (vdev->vq[idx].kick_fd != -1) { - epoll_del(vdev->context, vdev->vq[idx].kick_fd); + epoll_del(vdev->context->epollfd, vdev->vq[idx].kick_fd); close(vdev->vq[idx].kick_fd); vdev->vq[idx].kick_fd = -1; } @@ -1093,7 +1093,7 @@ void vu_cleanup(struct vu_dev *vdev) vq->err_fd = -1; } if (vq->kick_fd != -1) { - epoll_del(vdev->context, vq->kick_fd); + epoll_del(vdev->context->epollfd, vq->kick_fd); close(vq->kick_fd); vq->kick_fd = -1; } -- 2.50.1

Store the thread number in the flow_common structure for UDP flows using flow_epollfd_set() and retrieve the corresponding epoll file descriptor with flow_epollfd_get() instead of passing c->epollfd directly. This makes UDP consistent with the recent TCP and ICMP changes. Signed-off-by: Laurent Vivier --- udp_flow.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/udp_flow.c b/udp_flow.c index caaf3dababb1..7f6d88d37259 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -52,7 +52,7 @@ void udp_flow_close(const struct ctx *c, struct udp_flow *uflow) flow_foreach_sidei(sidei) { flow_hash_remove(c, FLOW_SIDX(uflow, sidei)); if (uflow->s[sidei] >= 0) { - epoll_del(c->epollfd, uflow->s[sidei]); + epoll_del(flow_epollfd_get(&uflow->f), uflow->s[sidei]); close(uflow->s[sidei]); uflow->s[sidei] = -1; } @@ -92,7 +92,9 @@ static int udp_flow_sock(const struct ctx *c, ref.data = fref.data; ref.fd = s; - rc = epoll_add(c->epollfd, EPOLLIN, &ref); + flow_epollfd_set(&uflow->f, 0, c->epollfd); + + rc = epoll_add(flow_epollfd_get(&uflow->f), EPOLLIN, &ref); if (rc < 0) { close(s); return rc; @@ -101,7 +103,7 @@ static int udp_flow_sock(const struct ctx *c, if (flowside_connect(c, s, pif, side) < 0) { rc = -errno; - epoll_del(c->epollfd, s); + epoll_del(flow_epollfd_get(&uflow->f), s); close(s); flow_dbg_perror(uflow, "Couldn't connect flow socket"); -- 2.50.1

On Thu, Oct 09, 2025 at 03:04:05PM +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

Nice cleanup. One nit noted below.

--- 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

diff --git a/Makefile b/Makefile index 3328f8324140..91e037b8fd3c 100644 --- a/Makefile +++ b/Makefile @@ -37,23 +37,23 @@ FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE) FLAGS += -DVERSION=\"$(VERSION)\" FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)

-PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c fwd.c \ - icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c \ - ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c \ - repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c \ - udp_vu.c util.c vhost_user.c virtio.c vu_common.c +PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c \ + flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c \ + log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c \ + pif.c repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c \ + udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c QRAP_SRCS = qrap.c PASST_REPAIR_SRCS = passt-repair.c SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS)

MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1

-PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h flow.h fwd.h \ - flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h isolation.h \ - lineread.h log.h migrate.h ndp.h netlink.h packet.h passt.h pasta.h \ - pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h \ - tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h udp_internal.h \ - udp_vu.h util.h vhost_user.h virtio.h vu_common.h +PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h epoll_ctl.h \ + flow.h fwd.h flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h \ + isolation.h lineread.h log.h migrate.h ndp.h netlink.h packet.h \ + passt.h pasta.h pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h \ + tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h \ + udp_internal.h udp_vu.h util.h vhost_user.h virtio.h vu_common.h HEADERS = $(PASST_HEADERS) seccomp.h

C := \#include \nint main(){int a=getrandom(0, 0, 0);} diff --git a/epoll_ctl.c b/epoll_ctl.c new file mode 100644 index 000000000000..0a06350f87a5 --- /dev/null +++ b/epoll_ctl.c @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* epoll_ctl.c - epoll manipulation helpers + * + * Copyright Red Hat + * Author: Laurent Vivier + */ + +#include + +#include "epoll_ctl.h" + +/** + * epoll_add() - Add a file descriptor to an epollfd + * @epollfd: epoll file descriptor to add to + * @events: epoll events + * @ref: epoll reference for the file descriptor (includes fd and metadata) + * + * Return: 0 on success, negative errno on failure + */ +int epoll_add(int epollfd, uint32_t events, union epoll_ref *ref) +{ + struct epoll_event ev; + int ret; + + ev.events = events; + ev.data.u64 = ref->u64; + + ret = epoll_ctl(epollfd, EPOLL_CTL_ADD, ref->fd, &ev); + if (ret == -1) { + ret = -errno; + warn("Failed to add fd to epoll: %s", strerror_(-ret));

I think this should be err() not warn(). If we're unable to add an fd to epoll things are almost certainly going to go badly wrong.

+ } + + return ret; +} + +/** + * epoll_del() - Remove a file descriptor from an epollfd + * @epollfd: epoll file descriptor to remove from + * @fd: File descriptor to remove + */ +void epoll_del(int epollfd, int fd) +{ + epoll_ctl(epollfd, EPOLL_CTL_DEL, fd, NULL); +} diff --git a/epoll_ctl.h b/epoll_ctl.h new file mode 100644 index 000000000000..cf92b0f63f26 --- /dev/null +++ b/epoll_ctl.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright Red Hat + * Author: Laurent Vivier + */ + +#ifndef EPOLL_CTL_H +#define EPOLL_CTL_H + +#include + +#include "util.h" +#include "passt.h" +#include "epoll_type.h" +#include "flow.h" +#include "tcp.h" +#include "udp.h" + +/** + * union epoll_ref - Breakdown of reference for epoll fd bookkeeping + * @type: Type of fd (tells us what to do with events) + * @fd: File descriptor number (implies < 2^24 total descriptors) + * @flow: Index of the flow this fd is linked to + * @tcp_listen: TCP-specific reference part for listening sockets + * @udp: UDP-specific reference part + * @data: Data handled by protocol handlers + * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone + * @queue: vhost-user queue index for this fd + * @u64: Opaque reference for epoll_ctl() and epoll_wait() + */ +union epoll_ref { + struct { + enum epoll_type type:8; + int32_t fd:FD_REF_BITS; + union { + uint32_t flow; + flow_sidx_t flowside; + union tcp_listen_epoll_ref tcp_listen; + union udp_listen_epoll_ref udp; + uint32_t data; + int nsdir_fd; + int queue; + }; + }; + uint64_t u64; +}; +static_assert(sizeof(union epoll_ref) <= sizeof(union epoll_data), + "epoll_ref must have same size as epoll_data"); + +int epoll_add(int epollfd, uint32_t events, union epoll_ref *ref); +void epoll_del(int epollfd, int fd); +#endif /* EPOLL_CTL_H */ diff --git a/icmp.c b/icmp.c index bd3108a21675..c26561da80bf 100644 --- a/icmp.c +++ b/icmp.c @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -23,10 +22,8 @@ #include #include #include -#include #include #include -#include #include

#include @@ -41,6 +38,7 @@ #include "inany.h" #include "icmp.h" #include "flow_table.h" +#include "epoll_ctl.h"

#define ICMP_ECHO_TIMEOUT 60 /* s, timeout for ICMP socket activity */ #define ICMP_NUM_IDS (1U << 16) diff --git a/passt.c b/passt.c index 31fbb75b1b12..e595f13a56f7 100644 --- a/passt.c +++ b/passt.c @@ -19,7 +19,6 @@ * created in a separate network namespace). */

-#include #include #include #include @@ -53,6 +52,7 @@ #include "vu_common.h" #include "migrate.h" #include "repair.h" +#include "epoll_ctl.h"

#define EPOLL_EVENTS 8

diff --git a/passt.h b/passt.h index 0075eb4b3b16..befe56bb167b 100644 --- a/passt.h +++ b/passt.h @@ -35,40 +35,6 @@ union epoll_ref; #define MAC_OUR_LAA \ ((uint8_t [ETH_ALEN]){0x9a, 0x55, 0x9a, 0x55, 0x9a, 0x55})

-/** - * union epoll_ref - Breakdown of reference for epoll fd bookkeeping - * @type: Type of fd (tells us what to do with events) - * @fd: File descriptor number (implies < 2^24 total descriptors) - * @flow: Index of the flow this fd is linked to - * @tcp_listen: TCP-specific reference part for listening sockets - * @udp: UDP-specific reference part - * @icmp: ICMP-specific reference part - * @data: Data handled by protocol handlers - * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone - * @queue: vhost-user queue index for this fd - * @u64: Opaque reference for epoll_ctl() and epoll_wait() - */ -union epoll_ref { - struct { - enum epoll_type type:8; -#define FD_REF_BITS 24 -#define FD_REF_MAX ((int)MAX_FROM_BITS(FD_REF_BITS)) - int32_t fd:FD_REF_BITS; - union { - uint32_t flow; - flow_sidx_t flowside; - union tcp_listen_epoll_ref tcp_listen; - union udp_listen_epoll_ref udp; - uint32_t data; - int nsdir_fd; - int queue; - }; - }; - uint64_t u64; -}; -static_assert(sizeof(union epoll_ref) <= sizeof(union epoll_data), - "epoll_ref must have same size as epoll_data"); - /* Large enough for ~128 maximum size frames */ #define PKT_BUF_BYTES (8UL << 20)

diff --git a/pasta.c b/pasta.c index 687406b6e736..e905f6d33b95 100644 --- a/pasta.c +++ b/pasta.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -49,6 +48,7 @@ #include "isolation.h" #include "netlink.h" #include "log.h" +#include "epoll_ctl.h"

#define HOSTNAME_PREFIX "pasta-"

@@ -444,7 +444,6 @@ static int pasta_netns_quit_timer(void) */ void pasta_netns_quit_init(const struct ctx *c) { - struct epoll_event ev = { .events = EPOLLIN }; int flags = O_NONBLOCK | O_CLOEXEC; struct statfs s = { 0 }; bool try_inotify = true; @@ -487,8 +486,8 @@ void pasta_netns_quit_init(const struct ctx *c) die("netns monitor file number %i too big, exiting", fd);

ref.fd = fd; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev); + + epoll_add(c->epollfd, EPOLLIN, &ref); }

/** diff --git a/repair.c b/repair.c index f6b1bf36479c..c8f4737fa62a 100644 --- a/repair.c +++ b/repair.c @@ -22,6 +22,7 @@ #include "inany.h" #include "flow.h" #include "flow_table.h" +#include "epoll_ctl.h"

#include "repair.h"

@@ -47,7 +48,6 @@ static int repair_nfds; void repair_sock_init(const struct ctx *c) { union epoll_ref ref = { .type = EPOLL_TYPE_REPAIR_LISTEN }; - struct epoll_event ev = { 0 };

if (c->fd_repair_listen == -1) return; @@ -58,9 +58,7 @@ void repair_sock_init(const struct ctx *c) }

ref.fd = c->fd_repair_listen; - ev.events = EPOLLIN | EPOLLHUP | EPOLLET; - ev.data.u64 = ref.u64; - if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_repair_listen, &ev)) + if (epoll_add(c->epollfd, EPOLLIN | EPOLLHUP | EPOLLET, &ref)) err_perror("repair helper socket epoll_ctl(), won't migrate"); }

@@ -74,7 +72,6 @@ void repair_sock_init(const struct ctx *c) int repair_listen_handler(struct ctx *c, uint32_t events) { union epoll_ref ref = { .type = EPOLL_TYPE_REPAIR }; - struct epoll_event ev = { 0 }; struct ucred ucred; socklen_t len; int rc; @@ -112,10 +109,9 @@ int repair_listen_handler(struct ctx *c, uint32_t events) info("Accepted TCP_REPAIR helper, PID %i", ucred.pid);

ref.fd = c->fd_repair; - ev.events = EPOLLHUP | EPOLLET; - ev.data.u64 = ref.u64; - if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_repair, &ev)) { - rc = errno; + + rc = epoll_add(c->epollfd, EPOLLHUP | EPOLLET, &ref); + if (rc < 0) { debug_perror("epoll_ctl() on TCP_REPAIR helper socket"); close(c->fd_repair); c->fd_repair = -1; diff --git a/tap.c b/tap.c index 134c37a72979..873874796f79 100644 --- a/tap.c +++ b/tap.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -61,6 +60,7 @@ #include "log.h" #include "vhost_user.h" #include "vu_common.h" +#include "epoll_ctl.h"

/* Maximum allowed frame lengths (including L2 header) */

@@ -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); }

/** @@ -1343,7 +1341,6 @@ static void tap_sock_unix_init(const struct ctx *c) */ static void tap_start_connection(const struct ctx *c) { - struct epoll_event ev = { 0 }; union epoll_ref ref = { 0 };

ref.fd = c->fd_tap; @@ -1359,9 +1356,7 @@ static void tap_start_connection(const struct ctx *c) break; }

- ev.events = EPOLLIN | EPOLLRDHUP; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); + epoll_add(c->epollfd, EPOLLIN | EPOLLRDHUP, &ref);

if (c->ifi4) arp_send_init_req(c); diff --git a/tcp.c b/tcp.c index 9100c67e57e0..c2d842bbdf4f 100644 --- a/tcp.c +++ b/tcp.c @@ -279,7 +279,6 @@ #include #include #include -#include #include #include #include @@ -309,6 +308,7 @@ #include "tcp_internal.h" #include "tcp_buf.h" #include "tcp_vu.h" +#include "epoll_ctl.h"

/* * The size of TCP header (including options) is given by doff (Data Offset) diff --git a/tcp_splice.c b/tcp_splice.c index 666ee62b738f..6f21184bdc55 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -44,7 +44,6 @@ #include #include #include -#include #include #include

@@ -56,6 +55,7 @@ #include "siphash.h" #include "inany.h" #include "flow.h" +#include "epoll_ctl.h"

#include "flow_table.h"

diff --git a/udp.c b/udp.c index 86585b7e0942..3812d5c2336f 100644 --- a/udp.c +++ b/udp.c @@ -94,7 +94,6 @@ #include #include #include -#include #include #include #include @@ -115,6 +114,7 @@ #include "flow_table.h" #include "udp_internal.h" #include "udp_vu.h" +#include "epoll_ctl.h"

#define UDP_MAX_FRAMES 32 /* max # of frames to receive at once */

diff --git a/udp_flow.c b/udp_flow.c index 84973f807167..d9c75f1bb1d8 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -15,6 +15,7 @@ #include "passt.h" #include "flow_table.h" #include "udp_internal.h" +#include "epoll_ctl.h"

#define UDP_CONN_TIMEOUT 180 /* s, timeout for ephemeral or local bind */

diff --git a/util.c b/util.c index 88a91b1100f5..b2490123590a 100644 --- a/util.c +++ b/util.c @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -35,6 +34,7 @@ #include "packet.h" #include "log.h" #include "pcap.h" +#include "epoll_ctl.h" #ifdef HAS_GETRANDOM #include #endif @@ -58,7 +58,6 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, sa_family_t af = ((const struct sockaddr *)sa)->sa_family; union epoll_ref ref = { .type = type, .data = data }; bool freebind = false; - struct epoll_event ev; int fd, y = 1, ret; uint8_t proto; int socktype; @@ -172,13 +171,9 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, return ret; }

- ev.events = EPOLLIN; - ev.data.u64 = ref.u64; - if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) { - ret = -errno; - warn("L4 epoll_ctl: %s", strerror_(-ret)); + ret = epoll_add(c->epollfd, EPOLLIN, &ref); + if (ret < 0) return ret; - }

return fd; } @@ -994,17 +989,6 @@ void raw_random(void *buf, size_t buflen) die("Unexpected EOF on random data source"); }

-/** - * epoll_del() - Remove a file descriptor from our passt epoll - * @epollfd: epoll file descriptor to add to - * @fd: File descriptor to remove - */ -void epoll_del(int epollfd, int fd) -{ - epoll_ctl(epollfd, EPOLL_CTL_DEL, fd, NULL); - -} - /** * encode_domain_name() - Encode domain name according to RFC 1035, section 3.1 * @buf: Buffer to fill in with encoded domain name diff --git a/util.h b/util.h index c61cbef357aa..8e4b4c5c6032 100644 --- a/util.h +++ b/util.h @@ -193,6 +193,9 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, #define SNDBUF_BIG (4ULL * 1024 * 1024) #define SNDBUF_SMALL (128ULL * 1024)

+#define FD_REF_BITS 24 +#define FD_REF_MAX ((int)MAX_FROM_BITS(FD_REF_BITS)) + #include #include #include @@ -300,7 +303,6 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m) #define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__)

void raw_random(void *buf, size_t buflen); -void epoll_del(int epollfd, int fd);

/* * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, diff --git a/vhost_user.c b/vhost_user.c index f8324c59cc6c..aea1e2cbcea5 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -32,8 +32,6 @@ #include #include #include -#include -#include #include #include #include @@ -45,6 +43,7 @@ #include "vhost_user.h" #include "pcap.h" #include "migrate.h" +#include "epoll_ctl.h"

/* vhost-user version we are compatible with */ #define VHOST_USER_VERSION 1 @@ -753,11 +752,8 @@ static void vu_set_watch(const struct vu_dev *vdev, int idx) .fd = vdev->vq[idx].kick_fd, .queue = idx }; - struct epoll_event ev = { 0 };

- ev.data.u64 = ref.u64; - ev.events = EPOLLIN; - epoll_ctl(vdev->context->epollfd, EPOLL_CTL_ADD, ref.fd, &ev); + epoll_add(vdev->context->epollfd, EPOLLIN, &ref); }

/** diff --git a/vu_common.c b/vu_common.c index b716070ea3c3..b13b7c308fd8 100644 --- a/vu_common.c +++ b/vu_common.c @@ -6,7 +6,6 @@ */

#include -#include #include #include #include @@ -19,6 +18,7 @@ #include "pcap.h" #include "vu_common.h" #include "migrate.h" +#include "epoll_ctl.h"

#define VU_MAX_TX_BUFFER_NB 2

-- 2.50.1

-- 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

On Thu, Oct 09, 2025 at 03:04:07PM +0200, Laurent Vivier wrote:

The in_epoll boolean flag in tcp_tap_conn and tcp_splice_conn only tracked whether a connection was registered with epoll, not which epoll instance. This limited flexibility for future multi-epoll support.

Replace the boolean with a threadnb field in flow_common that identifies which thread (and thus which epoll instance) the flow is registered with. Use FLOW_THREADNB_INVALID to indicate when a flow is not registered with any epoll instance. A threadnb_to_epollfd[] mapping table translates thread numbers to their corresponding epoll file descriptors.

Add helper functions: - flow_epollfd_valid() to check if a flow is registered with epoll - flow_epollfd_get() to retrieve the epoll fd for a flow's thread - flow_epollfd_set() to register a flow with a thread's epoll instance

This change also simplifies tcp_timer_ctl() and conn_flag_do() by removing the need to pass the context 'c', since the epoll fd is now directly accessible from the flow structure via flow_epollfd_get().

Signed-off-by: Laurent Vivier

I have mixed feelings on the naming. "threadnb" suggests the intended use for this for multithreading. However, this mechanism doesn't inherently have to do with threading. Not sure which consideration is more important here.

--- flow.c | 43 ++++++++++++++++++++++++++++++++++++++++++- flow.h | 11 +++++++++++ tcp.c | 39 +++++++++++++++++++++------------------ tcp_conn.h | 8 +------- tcp_splice.c | 24 ++++++++++++------------ 5 files changed, 87 insertions(+), 38 deletions(-)

diff --git a/flow.c b/flow.c index b14e9d8b63ff..6298bbafcd51 100644 --- a/flow.c +++ b/flow.c @@ -116,6 +116,7 @@ static_assert(ARRAY_SIZE(flow_proto) == FLOW_NUM_TYPES, unsigned flow_first_free; union flow flowtab[FLOW_MAX]; static const union flow *flow_new_entry; /* = NULL */ +static int threadnb_to_epollfd[FLOW_THREADNB_SIZE];

/* Hash table to index it */ #define FLOW_HASH_LOAD 70 /* % */ @@ -347,6 +348,45 @@ static void flow_set_state(struct flow_common *f, enum flow_state state) flow_log_details_(f, LOG_DEBUG, MAX(state, oldstate)); }

+/** + * flow_epollfd_valid() - Check if flow is registered with an epoll instance

I don't think "valid" really conveys what this does. Maybe flow_in_epoll(), since it reproduces the old in_epoll flag?

+ * @f: Flow to check + * + * Return: true if flow is registered with epoll, false otherwise + */ +bool flow_epollfd_valid(const struct flow_common *f) +{ + return f->threadnb != FLOW_THREADNB_INVALID; +} + +/** + * flow_epollfd_get() - Get the epoll file descriptor for a flow

flow_epollfd(). The "get" doesn't really add anything.

+ * @f: Flow to query + * + * Return: epoll file descriptor associated with the flow's thread + */ +int flow_epollfd_get(const struct flow_common *f) +{ + ASSERT(f->threadnb < FLOW_THREADNB_MAX); + + return threadnb_to_epollfd[f->threadnb]; +} + +/** + * flow_epollfd_set() - Set the epoll instance for a flow + * @f: Flow to update + * @threadnb: Thread number to associate with this flow + * @epollfd: epoll file descriptor for the thread + */ +void flow_epollfd_set(struct flow_common *f, int threadnb, int epollfd) +{ + ASSERT(threadnb < FLOW_THREADNB_MAX); + ASSERT(epollfd >= 0); + + threadnb_to_epollfd[threadnb] = epollfd; + f->threadnb = threadnb;

This feels like it's mixing concerns: the threadb_to_epollfd[] table should only be populated once, but each thread will need to have its threadnb initialised. It works as long as its always called with matching threadnb and epollfd, but it seems like a footgun.

+} + /** * flow_initiate_() - Move flow to INI, setting pif[INISIDE] * @flow: Flow to change state @@ -548,6 +588,7 @@ union flow *flow_alloc(void)

flow_new_entry = flow; memset(flow, 0, sizeof(*flow)); + flow->f.threadnb = FLOW_THREADNB_INVALID; flow_set_state(&flow->f, FLOW_STATE_NEW);

return flow; @@ -827,7 +868,7 @@ void flow_defer_handler(const struct ctx *c, const struct timespec *now) case FLOW_TCP_SPLICE: closed = tcp_splice_flow_defer(&flow->tcp_splice); if (!closed && timer) - tcp_splice_timer(c, &flow->tcp_splice); + tcp_splice_timer(&flow->tcp_splice); break; case FLOW_PING4: case FLOW_PING6: diff --git a/flow.h b/flow.h index ef138b83add8..18ae45d67720 100644 --- a/flow.h +++ b/flow.h @@ -177,6 +177,8 @@ int flowside_connect(const struct ctx *c, int s, * @type: Type of packet flow * @pif[]: Interface for each side of the flow * @side[]: Information for each side of the flow + * @threadnb: Thread number flow is registered with + * (FLOW_THREADNB_INVALID if not) */ struct flow_common { #ifdef __GNUC__ @@ -192,8 +194,14 @@ struct flow_common { #endif uint8_t pif[SIDES]; struct flowside side[SIDES]; +#define FLOW_THREADNB_BITS 8 + unsigned int threadnb:FLOW_THREADNB_BITS; };

+#define FLOW_THREADNB_SIZE (1 << FLOW_THREADNB_BITS) +#define FLOW_THREADNB_MAX (FLOW_THREADNB_SIZE - 1) +#define FLOW_THREADNB_INVALID FLOW_THREADNB_MAX + #define FLOW_INDEX_BITS 17 /* 128k - 1 */ #define FLOW_MAX MAX_FROM_BITS(FLOW_INDEX_BITS)

@@ -249,6 +257,9 @@ flow_sidx_t flow_lookup_sa(const struct ctx *c, uint8_t proto, uint8_t pif, union flow;

void flow_init(void); +bool flow_epollfd_valid(const struct flow_common *f); +int flow_epollfd_get(const struct flow_common *f); +void flow_epollfd_set(struct flow_common *f, int threadnb, int epollfd); void flow_defer_handler(const struct ctx *c, const struct timespec *now); int flow_migrate_source_early(struct ctx *c, const struct migrate_stage *stage, int fd); diff --git a/tcp.c b/tcp.c index c2d842bbdf4f..f9c67e2b006d 100644 --- a/tcp.c +++ b/tcp.c @@ -504,25 +504,27 @@ static uint32_t tcp_conn_epoll_events(uint8_t events, uint8_t conn_flags) */ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn) { - int m = conn->in_epoll ? EPOLL_CTL_MOD : EPOLL_CTL_ADD; + int m = flow_epollfd_valid(&conn->f) ? EPOLL_CTL_MOD : EPOLL_CTL_ADD; union epoll_ref ref = { .type = EPOLL_TYPE_TCP, .fd = conn->sock, .flowside = FLOW_SIDX(conn, !TAPSIDE(conn)), }; struct epoll_event ev = { .data.u64 = ref.u64 }; + int epollfd = flow_epollfd_valid(&conn->f) ? flow_epollfd_get(&conn->f) + : c->epollfd;

if (conn->events == CLOSED) { - if (conn->in_epoll) - epoll_del(c->epollfd, conn->sock); + if (flow_epollfd_valid(&conn->f)) + epoll_del(epollfd, conn->sock); if (conn->timer != -1) - epoll_del(c->epollfd, conn->timer); + epoll_del(epollfd, conn->timer); return 0; }

ev.events = tcp_conn_epoll_events(conn->events, conn->flags);

- if (epoll_ctl(c->epollfd, m, conn->sock, &ev)) + if (epoll_ctl(epollfd, m, conn->sock, &ev)) return -errno;

- conn->in_epoll = true; + flow_epollfd_set(&conn->f, 0, epollfd);

if (conn->timer != -1) { union epoll_ref ref_t = { .type = EPOLL_TYPE_TCP_TIMER, @@ -531,7 +533,8 @@ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn) struct epoll_event ev_t = { .data.u64 = ref_t.u64, .events = EPOLLIN | EPOLLET };

- if (epoll_ctl(c->epollfd, EPOLL_CTL_MOD, conn->timer, &ev_t)) + if (epoll_ctl(flow_epollfd_get(&conn->f), EPOLL_CTL_MOD, + conn->timer, &ev_t)) return -errno; }

@@ -540,12 +543,11 @@ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn)

/** * tcp_timer_ctl() - Set timerfd based on flags/events, create timerfd if needed - * @c: Execution context * @conn: Connection pointer * * #syscalls timerfd_create timerfd_settime */ -static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) +static void tcp_timer_ctl(struct tcp_tap_conn *conn) { struct itimerspec it = { { 0 }, { 0 } };

@@ -570,7 +572,8 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) } conn->timer = fd;

- if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, conn->timer, &ev)) { + if (epoll_ctl(flow_epollfd_get(&conn->f), EPOLL_CTL_ADD, + conn->timer, &ev)) { flow_dbg_perror(conn, "failed to add timer"); close(conn->timer); conn->timer = -1; @@ -628,7 +631,7 @@ void conn_flag_do(const struct ctx *c, struct tcp_tap_conn *conn, * flags and factor this into the logic below. */ if (flag == ACK_FROM_TAP_DUE) - tcp_timer_ctl(c, conn); + tcp_timer_ctl(conn);

return; } @@ -644,7 +647,7 @@ void conn_flag_do(const struct ctx *c, struct tcp_tap_conn *conn, if (flag == ACK_FROM_TAP_DUE || flag == ACK_TO_TAP_DUE || (flag == ~ACK_FROM_TAP_DUE && (conn->flags & ACK_TO_TAP_DUE)) || (flag == ~ACK_TO_TAP_DUE && (conn->flags & ACK_FROM_TAP_DUE))) - tcp_timer_ctl(c, conn); + tcp_timer_ctl(conn); }

/** @@ -699,7 +702,7 @@ void conn_event_do(const struct ctx *c, struct tcp_tap_conn *conn, tcp_epoll_ctl(c, conn);

if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED)) - tcp_timer_ctl(c, conn); + tcp_timer_ctl(conn); }

/** @@ -1737,7 +1740,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, seq, conn->seq_from_tap);

tcp_send_flag(c, conn, ACK); - tcp_timer_ctl(c, conn); + tcp_timer_ctl(conn);

if (p->count == 1) { tcp_tap_window_update(c, conn, @@ -2386,7 +2389,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)

if (conn->flags & ACK_TO_TAP_DUE) { tcp_send_flag(c, conn, ACK_IF_NEEDED); - tcp_timer_ctl(c, conn); + tcp_timer_ctl(conn); } else if (conn->flags & ACK_FROM_TAP_DUE) { if (!(conn->events & ESTABLISHED)) { flow_dbg(conn, "handshake timeout"); @@ -2408,7 +2411,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) return;

tcp_data_from_sock(c, conn); - tcp_timer_ctl(c, conn); + tcp_timer_ctl(conn); } } else { struct itimerspec new = { { 0 }, { ACT_TIMEOUT, 0 } }; @@ -3456,7 +3459,7 @@ int tcp_flow_migrate_source_ext(const struct ctx *c, if (c->migrate_no_linger) close(s); else - epoll_del(c->epollfd, s); + epoll_del(flow_epollfd_get(&conn->f), s);

/* Adjustments unrelated to FIN segments: sequence numbers we dumped are * based on the end of the queues. @@ -3605,7 +3608,7 @@ static int tcp_flow_repair_connect(const struct ctx *c, return rc; }

- conn->in_epoll = 0; + conn->f.threadnb = FLOW_THREADNB_INVALID; conn->timer = -1; conn->listening_sock = -1;

diff --git a/tcp_conn.h b/tcp_conn.h index 38b5c541f003..81333122d531 100644 --- a/tcp_conn.h +++ b/tcp_conn.h @@ -12,7 +12,6 @@ /** * struct tcp_tap_conn - Descriptor for a TCP connection (not spliced) * @f: Generic flow information - * @in_epoll: Is the connection in the epoll set? * @retrans: Number of retransmissions occurred due to ACK_TIMEOUT * @ws_from_tap: Window scaling factor advertised from tap/guest * @ws_to_tap: Window scaling factor advertised to tap/guest @@ -36,8 +35,6 @@ struct tcp_tap_conn { /* Must be first element */ struct flow_common f;

- bool in_epoll :1; -

One wrinkle with moving this to flow_common is that we maybe need to be a bit clearer about what the semantics of "in_epoll" vs. not "in_epoll" are - however it's encoded. After all some flow types have multiple fds, which could independently be in or out of the epoll. I think I considered moving in_epoll to flow_common before, but decided against it, because I punted on pinning down those semantics: as a per flow type flag, it has a per flow type meaning.

#define TCP_RETRANS_BITS 3 unsigned int retrans :TCP_RETRANS_BITS; #define TCP_MAX_RETRANS MAX_FROM_BITS(TCP_RETRANS_BITS) @@ -196,7 +193,6 @@ struct tcp_tap_transfer_ext { * @written: Bytes written (not fully written from one other side read) * @events: Events observed/actions performed on connection * @flags: Connection flags (attributes, not events) - * @in_epoll: Is the connection in the epoll set? */ struct tcp_splice_conn { /* Must be first element */ @@ -220,8 +216,6 @@ struct tcp_splice_conn { #define RCVLOWAT_SET(sidei_) ((sidei_) ? BIT(1) : BIT(0)) #define RCVLOWAT_ACT(sidei_) ((sidei_) ? BIT(3) : BIT(2)) #define CLOSING BIT(4) - - bool in_epoll :1; };

/* Socket pools */ @@ -245,7 +239,7 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd bool tcp_flow_is_established(const struct tcp_tap_conn *conn);

bool tcp_splice_flow_defer(struct tcp_splice_conn *conn); -void tcp_splice_timer(const struct ctx *c, struct tcp_splice_conn *conn); +void tcp_splice_timer(struct tcp_splice_conn *conn); int tcp_conn_pool_sock(int pool[]); int tcp_conn_sock(sa_family_t af); int tcp_sock_refill_pool(int pool[], sa_family_t af); diff --git a/tcp_splice.c b/tcp_splice.c index 6f21184bdc55..8842feef0684 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -149,7 +149,9 @@ static void tcp_splice_conn_epoll_events(uint16_t events, static int tcp_splice_epoll_ctl(const struct ctx *c, struct tcp_splice_conn *conn) { - int m = conn->in_epoll ? EPOLL_CTL_MOD : EPOLL_CTL_ADD; + int epollfd = flow_epollfd_valid(&conn->f) ? flow_epollfd_get(&conn->f) + : c->epollfd; + int m = flow_epollfd_valid(&conn->f) ? EPOLL_CTL_MOD : EPOLL_CTL_ADD; const union epoll_ref ref[SIDES] = { { .type = EPOLL_TYPE_TCP_SPLICE, .fd = conn->s[0], .flowside = FLOW_SIDX(conn, 0) }, @@ -161,25 +163,24 @@ static int tcp_splice_epoll_ctl(const struct ctx *c,

tcp_splice_conn_epoll_events(conn->events, ev);

- if (epoll_ctl(c->epollfd, m, conn->s[0], &ev[0]) || - epoll_ctl(c->epollfd, m, conn->s[1], &ev[1])) { + + if (epoll_ctl(epollfd, m, conn->s[0], &ev[0]) || + epoll_ctl(epollfd, m, conn->s[1], &ev[1])) { int ret = -errno; flow_perror(conn, "ERROR on epoll_ctl()"); return ret; } - - conn->in_epoll = true; + flow_epollfd_set(&conn->f, 0, epollfd);

return 0; }

/** * conn_flag_do() - Set/unset given flag, log, update epoll on CLOSING flag - * @c: Execution context * @conn: Connection pointer * @flag: Flag to set, or ~flag to unset */ -static void conn_flag_do(const struct ctx *c, struct tcp_splice_conn *conn, +static void conn_flag_do(struct tcp_splice_conn *conn, unsigned long flag) { if (flag & (flag - 1)) { @@ -204,15 +205,15 @@ static void conn_flag_do(const struct ctx *c, struct tcp_splice_conn *conn, }

if (flag == CLOSING) { - epoll_del(c->epollfd, conn->s[0]); - epoll_del(c->epollfd, conn->s[1]); + epoll_del(flow_epollfd_get(&conn->f), conn->s[0]); + epoll_del(flow_epollfd_get(&conn->f), conn->s[1]); } }

#define conn_flag(c, conn, flag) \ do { \ flow_trace(conn, "flag at %s:%i", __func__, __LINE__); \ - conn_flag_do(c, conn, flag); \ + conn_flag_do(conn, flag); \ } while (0)

/** @@ -751,10 +752,9 @@ void tcp_splice_init(struct ctx *c)

/** * tcp_splice_timer() - Timer for spliced connections - * @c: Execution context * @conn: Connection to handle */ -void tcp_splice_timer(const struct ctx *c, struct tcp_splice_conn *conn) +void tcp_splice_timer(struct tcp_splice_conn *conn) { unsigned sidei;

-- 2.50.1

-- 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

On Thu, 9 Oct 2025 15:04:05 +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 --- 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

diff --git a/Makefile b/Makefile index 3328f8324140..91e037b8fd3c 100644 --- a/Makefile +++ b/Makefile @@ -37,23 +37,23 @@ FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE) FLAGS += -DVERSION=\"$(VERSION)\" FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)

-PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c fwd.c \ - icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c \ - ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c \ - repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c \ - udp_vu.c util.c vhost_user.c virtio.c vu_common.c +PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c \ + flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c \ + log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c \ + pif.c repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c \ + udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c QRAP_SRCS = qrap.c PASST_REPAIR_SRCS = passt-repair.c SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS)

MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1

-PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h flow.h fwd.h \ - flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h isolation.h \ - lineread.h log.h migrate.h ndp.h netlink.h packet.h passt.h pasta.h \ - pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h \ - tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h udp_internal.h \ - udp_vu.h util.h vhost_user.h virtio.h vu_common.h +PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h epoll_ctl.h \ + flow.h fwd.h flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h \ + isolation.h lineread.h log.h migrate.h ndp.h netlink.h packet.h \ + passt.h pasta.h pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h \ + tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h \ + udp_internal.h udp_vu.h util.h vhost_user.h virtio.h vu_common.h HEADERS = $(PASST_HEADERS) seccomp.h

C := \#include \nint main(){int a=getrandom(0, 0, 0);} diff --git a/epoll_ctl.c b/epoll_ctl.c new file mode 100644 index 000000000000..0a06350f87a5 --- /dev/null +++ b/epoll_ctl.c @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* epoll_ctl.c - epoll manipulation helpers + * + * Copyright Red Hat + * Author: Laurent Vivier + */ + +#include + +#include "epoll_ctl.h" + +/** + * epoll_add() - Add a file descriptor to an epollfd + * @epollfd: epoll file descriptor to add to + * @events: epoll events + * @ref: epoll reference for the file descriptor (includes fd and metadata) + * + * Return: 0 on success, negative errno on failure + */ +int epoll_add(int epollfd, uint32_t events, union epoll_ref *ref) +{ + struct epoll_event ev; + int ret; + + ev.events = events; + ev.data.u64 = ref->u64; + + ret = epoll_ctl(epollfd, EPOLL_CTL_ADD, ref->fd, &ev); + if (ret == -1) { + ret = -errno; + warn("Failed to add fd to epoll: %s", strerror_(-ret)); + } + + return ret; +} + +/** + * epoll_del() - Remove a file descriptor from an epollfd + * @epollfd: epoll file descriptor to remove from + * @fd: File descriptor to remove + */ +void epoll_del(int epollfd, int fd) +{ + epoll_ctl(epollfd, EPOLL_CTL_DEL, fd, NULL); +} diff --git a/epoll_ctl.h b/epoll_ctl.h new file mode 100644 index 000000000000..cf92b0f63f26 --- /dev/null +++ b/epoll_ctl.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright Red Hat + * Author: Laurent Vivier + */ + +#ifndef EPOLL_CTL_H +#define EPOLL_CTL_H + +#include + +#include "util.h" +#include "passt.h" +#include "epoll_type.h" +#include "flow.h" +#include "tcp.h" +#include "udp.h" + +/** + * union epoll_ref - Breakdown of reference for epoll fd bookkeeping + * @type: Type of fd (tells us what to do with events) + * @fd: File descriptor number (implies < 2^24 total descriptors) + * @flow: Index of the flow this fd is linked to + * @tcp_listen: TCP-specific reference part for listening sockets + * @udp: UDP-specific reference part + * @data: Data handled by protocol handlers + * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone + * @queue: vhost-user queue index for this fd + * @u64: Opaque reference for epoll_ctl() and epoll_wait() + */ +union epoll_ref { + struct { + enum epoll_type type:8; + int32_t fd:FD_REF_BITS;

Do the definitions of FD_REF_BITS and FD_REF_MAX really belong to util.h? It would be clearer to have them here. I didn't check all the possible places where you would need to include this header though.

+ union { + uint32_t flow; + flow_sidx_t flowside; + union tcp_listen_epoll_ref tcp_listen; + union udp_listen_epoll_ref udp; + uint32_t data; + int nsdir_fd; + int queue; + }; + }; + uint64_t u64; +}; +static_assert(sizeof(union epoll_ref) <= sizeof(union epoll_data), + "epoll_ref must have same size as epoll_data");

Either the comment is misleading, or it should be sizeof(...) != ... The rest of the patch looks good to me, I didn't finish reviewing the series though. -- Stefano