On Fri, Jan 16, 2026 at 04:52:22PM +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
Couple of queries, but the concept looks good.
--- flow.c | 1 + tcp.c | 36 ++++++++++++++---------------------- 2 files changed, 15 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..d9bca041dea8 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,8 @@ 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); + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, !TAPSIDE(conn));
Do we need to handle errors here? Because this is conn_from_tap(), we know that !TAPSIDE() will always be TGTSIDE in this case.
conn_event(c, conn, TAP_SYN_RCVD);
conn->wnd_to_tap = WINDOW_DEFAULT; @@ -2433,6 +2420,8 @@ 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); + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, !TAPSIDE(conn));
Same comments as above, except here we know it's INISIDE.
conn_event(c, conn, SOCK_ACCEPTED);
hash = flow_hash_insert(c, TAP_SIDX(conn)); @@ -3825,6 +3814,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