Register both splice connection sockets with epoll using empty events (events=0) in tcp_splice_connect(), before initiating the connection. This allows tcp_splice_epoll_ctl() to always use EPOLL_CTL_MOD, removing the need to check whether fds are already registered. As a result, the conditional ADD/MOD logic is no longer needed, simplifying the function. If the second flow_epoll_set() fails after the first succeeds, we don't need explicit rollback: tcp_splice_conn_from_sock() sets the CLOSING flag on error, and conn_flag() handles it by calling epoll_del() for both sockets. Signed-off-by: Laurent Vivier --- tcp_splice.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tcp_splice.c b/tcp_splice.c index a7c04ca8652a..cb81e012ee4b 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -142,20 +142,12 @@ static uint32_t tcp_splice_conn_epoll_events(uint16_t events, unsigned sidei) static int tcp_splice_epoll_ctl(struct tcp_splice_conn *conn) { uint32_t events[2]; - int m; - - if (flow_in_epoll(&conn->f)) { - m = EPOLL_CTL_MOD; - } else { - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); - m = EPOLL_CTL_ADD; - } events[0] = tcp_splice_conn_epoll_events(conn->events, 0); events[1] = tcp_splice_conn_epoll_events(conn->events, 1); - if (flow_epoll_set(&conn->f, m, events[0], conn->s[0], 0) || - flow_epoll_set(&conn->f, m, events[1], conn->s[1], 1)) { + if (flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events[0], conn->s[0], 0) || + flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events[1], conn->s[1], 1)) { int ret = -errno; flow_perror(conn, "ERROR on epoll_ctl()"); return ret; @@ -368,6 +360,14 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn) pif_sockaddr(c, &sa, tgtpif, &tgt->eaddr, tgt->eport); + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); + if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->s[0], 0) || + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->s[1], 1)) { + int ret = -errno; + flow_perror(conn, "Cannot register to epollfd"); + return ret; + } + conn_event(conn, SPLICE_CONNECT); if (connect(conn->s[1], &sa.sa, socklen_inany(&sa))) { -- 2.52.0

On Mon, Jan 19, 2026 at 05:19:14PM +0100, Laurent Vivier wrote:

Register connection sockets with epoll using empty events (events=0) in tcp_conn_from_tap(), tcp_tap_conn_from_sock() and tcp_flow_repair_socket().

This allows tcp_epoll_ctl() to always use EPOLL_CTL_MOD, removing the need to check whether fds are already registered. As a result, the conditional ADD/MOD logic is no longer needed, simplifying the function.

Signed-off-by: Laurent Vivier

Reviewed-by: David Gibson

--- flow.c | 1 + tcp.c | 46 ++++++++++++++++++++++++---------------------- 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/flow.c b/flow.c index cefe6c8b5b24..532339ce7fe1 100644 --- a/flow.c +++ b/flow.c @@ -357,6 +357,7 @@ static void flow_set_state(struct flow_common *f, enum flow_state state) * * Return: true if flow is registered with epoll, false otherwise */ +/* cppcheck-suppress unusedFunction */ bool flow_in_epoll(const struct flow_common *f) { return f->epollid != EPOLLFD_ID_INVALID; diff --git a/tcp.c b/tcp.c index 1db861705ddb..29d69354bd94 100644 --- a/tcp.c +++ b/tcp.c @@ -528,37 +528,22 @@ static uint32_t tcp_conn_epoll_events(uint8_t events, uint8_t conn_flags) static int tcp_epoll_ctl(struct tcp_tap_conn *conn) { uint32_t events; - int m;

if (conn->events == CLOSED) { - if (flow_in_epoll(&conn->f)) { - int epollfd = flow_epollfd(&conn->f); + int epollfd = flow_epollfd(&conn->f);

- epoll_del(epollfd, conn->sock); - if (conn->timer != -1) - epoll_del(epollfd, conn->timer); - } + epoll_del(epollfd, conn->sock); + if (conn->timer != -1) + epoll_del(epollfd, conn->timer);

return 0; }

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

- if (flow_in_epoll(&conn->f)) { - m = EPOLL_CTL_MOD; - } else { - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); - m = EPOLL_CTL_ADD; - } - - if (flow_epoll_set(&conn->f, m, events, conn->sock, - !TAPSIDE(conn)) < 0) { - int ret = -errno; - - if (m == EPOLL_CTL_ADD) - flow_epollid_clear(&conn->f); - return ret; - } + if (flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events, conn->sock, + !TAPSIDE(conn)) < 0) + return -errno;

return 0; } @@ -1710,6 +1695,11 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af, conn->sock = s; conn->timer = -1; conn->listening_sock = -1; + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); + if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, TGTSIDE) < 0) { + flow_perror(flow, "Can't register with epoll"); + goto cancel; + } conn_event(c, conn, TAP_SYN_RCVD);

conn->wnd_to_tap = WINDOW_DEFAULT; @@ -2433,6 +2423,15 @@ static void tcp_tap_conn_from_sock(const struct ctx *c, union flow *flow, conn->sock = s; conn->timer = -1; conn->ws_to_tap = conn->ws_from_tap = 0; + + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); + if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, INISIDE) < 0) { + flow_perror(flow, "Can't register with epoll"); + conn_flag(c, conn, CLOSING); + FLOW_ACTIVATE(conn); + return; + } + conn_event(c, conn, SOCK_ACCEPTED);

hash = flow_hash_insert(c, TAP_SIDX(conn)); @@ -3825,6 +3824,9 @@ int tcp_flow_migrate_target(struct ctx *c, int fd) return 0; }

+ flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->sock, !TAPSIDE(conn)); + flow_hash_insert(c, TAP_SIDX(conn)); FLOW_ACTIVATE(conn);

-- 2.52.0

-- 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 Mon, Jan 19, 2026 at 05:19:15PM +0100, Laurent Vivier wrote:

As all flows are now registered with an epollid at creation, we no longer need to test if a flow is in epoll. Remove all related code including flow_in_epoll() and flow_epollid_clear().

Signed-off-by: Laurent Vivier

Reviewed-by: David Gibson

--- flow.c | 35 +++-------------------------------- flow.h | 6 +----- icmp.c | 1 - tcp.c | 1 - udp_flow.c | 1 - 5 files changed, 4 insertions(+), 40 deletions(-)

diff --git a/flow.c b/flow.c index 532339ce7fe1..1b8643358575 100644 --- a/flow.c +++ b/flow.c @@ -127,7 +127,7 @@ static_assert(ARRAY_SIZE(flow_epoll) == FLOW_NUM_TYPES, unsigned flow_first_free; union flow flowtab[FLOW_MAX]; static const union flow *flow_new_entry; /* = NULL */ -static int epoll_id_to_fd[EPOLLFD_ID_MAX]; +static int epoll_id_to_fd[EPOLLFD_ID_SIZE];

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

-/** - * flow_in_epoll() - Check if flow is registered with an epoll instance - * @f: Flow to check - * - * Return: true if flow is registered with epoll, false otherwise - */ -/* cppcheck-suppress unusedFunction */ -bool flow_in_epoll(const struct flow_common *f) -{ - return f->epollid != EPOLLFD_ID_INVALID; -} - /** * flow_epollfd() - Get the epoll file descriptor for a flow * @f: Flow to query @@ -371,13 +359,6 @@ bool flow_in_epoll(const struct flow_common *f) */ int flow_epollfd(const struct flow_common *f) { - if (f->epollid >= EPOLLFD_ID_MAX) { - flow_log_(f, true, LOG_WARNING, - "Invalid epollid %i for flow, assuming default", - f->epollid); - return epoll_id_to_fd[EPOLLFD_ID_DEFAULT]; - } - return epoll_id_to_fd[f->epollid]; }

@@ -388,20 +369,11 @@ int flow_epollfd(const struct flow_common *f) */ void flow_epollid_set(struct flow_common *f, int epollid) { - ASSERT(epollid < EPOLLFD_ID_MAX); + ASSERT(epollid < EPOLLFD_ID_SIZE);

f->epollid = epollid; }

-/** - * flow_epollid_clear() - Clear the flow epoll id - * @f: Flow to update - */ -void flow_epollid_clear(struct flow_common *f) -{ - f->epollid = EPOLLFD_ID_INVALID; -} - /** * flow_epoll_set() - Add or modify epoll registration for a flow socket * @f: Flow to register socket for @@ -435,7 +407,7 @@ int flow_epoll_set(const struct flow_common *f, int command, uint32_t events, */ void flow_epollid_register(int epollid, int epollfd) { - ASSERT(epollid < EPOLLFD_ID_MAX); + ASSERT(epollid < EPOLLFD_ID_SIZE);

epoll_id_to_fd[epollid] = epollfd; } @@ -643,7 +615,6 @@ union flow *flow_alloc(void)

flow_new_entry = flow; memset(flow, 0, sizeof(*flow)); - flow_epollid_clear(&flow->f); flow_set_state(&flow->f, FLOW_STATE_NEW);

return flow; diff --git a/flow.h b/flow.h index a74e13507957..d636358df422 100644 --- a/flow.h +++ b/flow.h @@ -178,7 +178,7 @@ int flowside_connect(const struct ctx *c, int s, * @pif[]: Interface for each side of the flow * @side[]: Information for each side of the flow * @tap_omac: MAC address of remote endpoint as seen from the guest - * @epollid: epollfd identifier, or EPOLLFD_ID_INVALID + * @epollid: epollfd identifier */ struct flow_common { #ifdef __GNUC__ @@ -203,8 +203,6 @@ struct flow_common {

#define EPOLLFD_ID_DEFAULT 0 #define EPOLLFD_ID_SIZE (1 << EPOLLFD_ID_BITS) -#define EPOLLFD_ID_MAX (EPOLLFD_ID_SIZE - 1) -#define EPOLLFD_ID_INVALID EPOLLFD_ID_MAX

#define FLOW_INDEX_BITS 17 /* 128k - 1 */ #define FLOW_MAX MAX_FROM_BITS(FLOW_INDEX_BITS) @@ -261,10 +259,8 @@ flow_sidx_t flow_lookup_sa(const struct ctx *c, uint8_t proto, uint8_t pif, union flow;

void flow_init(void); -bool flow_in_epoll(const struct flow_common *f); int flow_epollfd(const struct flow_common *f); void flow_epollid_set(struct flow_common *f, int epollid); -void flow_epollid_clear(struct flow_common *f); int flow_epoll_set(const struct flow_common *f, int command, uint32_t events, int fd, unsigned int sidei); void flow_epollid_register(int epollid, int epollfd); diff --git a/icmp.c b/icmp.c index eb7f11be5dad..9da5a787d181 100644 --- a/icmp.c +++ b/icmp.c @@ -213,7 +213,6 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c, if (flow_epoll_set(&pingf->f, EPOLL_CTL_ADD, EPOLLIN, pingf->sock, TGTSIDE) < 0) { close(pingf->sock); - flow_epollid_clear(&pingf->f); goto cancel; }

diff --git a/tcp.c b/tcp.c index 29d69354bd94..31d4e41ecb1a 100644 --- a/tcp.c +++ b/tcp.c @@ -3759,7 +3759,6 @@ static int tcp_flow_repair_connect(const struct ctx *c, return rc; }

- flow_epollid_clear(&conn->f); conn->timer = -1; conn->listening_sock = -1;

diff --git a/udp_flow.c b/udp_flow.c index 80b15433f0ac..d824e2c55b2c 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -86,7 +86,6 @@ static int udp_flow_sock(const struct ctx *c, flow_epollid_set(&uflow->f, EPOLLFD_ID_DEFAULT); if (flow_epoll_set(&uflow->f, EPOLL_CTL_ADD, EPOLLIN, s, sidei) < 0) { rc = -errno; - flow_epollid_clear(&uflow->f); close(s); return rc; } -- 2.52.0

-- 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 Mon, 19 Jan 2026 17:19:14 +0100 Laurent Vivier wrote:

Register connection sockets with epoll using empty events (events=0) in tcp_conn_from_tap(), tcp_tap_conn_from_sock() and tcp_flow_repair_socket().

This allows tcp_epoll_ctl() to always use EPOLL_CTL_MOD, removing the need to check whether fds are already registered. As a result, the conditional ADD/MOD logic is no longer needed, simplifying the function.

Signed-off-by: Laurent Vivier --- flow.c | 1 + tcp.c | 46 ++++++++++++++++++++++++---------------------- 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/flow.c b/flow.c index cefe6c8b5b24..532339ce7fe1 100644 --- a/flow.c +++ b/flow.c @@ -357,6 +357,7 @@ static void flow_set_state(struct flow_common *f, enum flow_state state) * * Return: true if flow is registered with epoll, false otherwise */ +/* cppcheck-suppress unusedFunction */ bool flow_in_epoll(const struct flow_common *f) { return f->epollid != EPOLLFD_ID_INVALID; diff --git a/tcp.c b/tcp.c index 1db861705ddb..29d69354bd94 100644 --- a/tcp.c +++ b/tcp.c @@ -528,37 +528,22 @@ static uint32_t tcp_conn_epoll_events(uint8_t events, uint8_t conn_flags) static int tcp_epoll_ctl(struct tcp_tap_conn *conn) { uint32_t events; - int m;

if (conn->events == CLOSED) { - if (flow_in_epoll(&conn->f)) { - int epollfd = flow_epollfd(&conn->f); + int epollfd = flow_epollfd(&conn->f);

- epoll_del(epollfd, conn->sock); - if (conn->timer != -1) - epoll_del(epollfd, conn->timer); - } + epoll_del(epollfd, conn->sock); + if (conn->timer != -1) + epoll_del(epollfd, conn->timer);

return 0; }

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

- if (flow_in_epoll(&conn->f)) { - m = EPOLL_CTL_MOD; - } else { - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); - m = EPOLL_CTL_ADD; - } - - if (flow_epoll_set(&conn->f, m, events, conn->sock, - !TAPSIDE(conn)) < 0) { - int ret = -errno; - - if (m == EPOLL_CTL_ADD) - flow_epollid_clear(&conn->f); - return ret; - } + if (flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events, conn->sock, + !TAPSIDE(conn)) < 0) + return -errno;

return 0; } @@ -1710,6 +1695,11 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af, conn->sock = s; conn->timer = -1; conn->listening_sock = -1; + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); + if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, TGTSIDE) < 0) { + flow_perror(flow, "Can't register with epoll"); + goto cancel; + } conn_event(c, conn, TAP_SYN_RCVD);

conn->wnd_to_tap = WINDOW_DEFAULT; @@ -2433,6 +2423,15 @@ static void tcp_tap_conn_from_sock(const struct ctx *c, union flow *flow, conn->sock = s; conn->timer = -1; conn->ws_to_tap = conn->ws_from_tap = 0; + + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); + if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, INISIDE) < 0) { + flow_perror(flow, "Can't register with epoll"); + conn_flag(c, conn, CLOSING); + FLOW_ACTIVATE(conn); + return; + } + conn_event(c, conn, SOCK_ACCEPTED);

hash = flow_hash_insert(c, TAP_SIDX(conn)); @@ -3825,6 +3824,9 @@ int tcp_flow_migrate_target(struct ctx *c, int fd) return 0; }

+ flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->sock, !TAPSIDE(conn));

Oops, I overlooked this, because I missed re-checking on v2 after David pointed it out: this causes Coverity Scan to (reasonably, I think) complain that: /home/sbrivio/passt/tcp.c:3693:2: Type: Unchecked return value (CHECKED_RETURN) /home/sbrivio/passt/tcp.c:3648:2: Unchecked call to function 1. path: Condition "!(flow = flow_alloc())", taking false branch. /home/sbrivio/passt/tcp.c:3653:2: 2. path: Condition "read_all_buf(fd, &t, 112UL /* sizeof (t) */)", taking false branch. /home/sbrivio/passt/tcp.c:3685:2: 3. path: Condition "rc = tcp_flow_repair_socket(c, conn)", taking false branch. /home/sbrivio/passt/tcp.c:3693:2: 4. path: Condition "!(conn->f.pif[1] == PIF_TAP)", taking true branch. /home/sbrivio/passt/tcp.c:3693:2: 5. check_return: Calling "flow_epoll_set" without checking return value (as is done elsewhere 6 out of 7 times). /home/sbrivio/passt/icmp.c:213:2: Examples where return value from this function is checked 6. example_checked: Example 1: "flow_epoll_set(&pingf->f, 1, EPOLLIN, pingf->sock, 1U)" has its value checked in "flow_epoll_set(&pingf->f, 1, EPOLLIN, pingf->sock, 1U) < 0". /home/sbrivio/passt/tcp.c:1695:2: Examples where return value from this function is checked 7. example_checked: Example 2: "flow_epoll_set(&conn->f, 1, 0U, s, 1U)" has its value checked in "flow_epoll_set(&conn->f, 1, 0U, s, 1U) < 0". /home/sbrivio/passt/tcp_splice.c:364:2: Examples where return value from this function is checked 8. example_checked: Example 3: "flow_epoll_set(&conn->f, 1, 0U, conn->s[1], 1U)" has its value checked in "flow_epoll_set(&conn->f, 1, 0U, conn->s[1], 1U)". /home/sbrivio/passt/tcp_splice.c:364:2: Examples where return value from this function is checked 9. example_checked: Example 4: "flow_epoll_set(&conn->f, 1, 0U, conn->s[0], 0U)" has its value checked in "flow_epoll_set(&conn->f, 1, 0U, conn->s[0], 0U)". /home/sbrivio/passt/udp_flow.c:87:2: Examples where return value from this function is checked 10. example_checked: Example 5: "flow_epoll_set(&uflow->f, 1, EPOLLIN, s, sidei)" has its value checked in "flow_epoll_set(&uflow->f, 1, EPOLLIN, s, sidei) < 0". I don't think it's overly problematic but epoll_ctl() can, in theory, fail regardless of the arguments.

+ flow_hash_insert(c, TAP_SIDX(conn)); FLOW_ACTIVATE(conn);

-- Stefano