[PATCH 0/3] Register TCP flows with epoll at creation time
This series simplifies epoll registration by moving it to flow creation time rather than deferring it to the first event update. By registering sockets with epoll immediately (using events=0), the epoll control functions can always use EPOLL_CTL_MOD, eliminating the need to track whether a flow is already registered. Patch 1 registers both splice sockets with epoll in tcp_splice_connect() before initiating the connection, allowing tcp_splice_epoll_ctl() to always use EPOLL_CTL_MOD. Patch 2 does the same for regular TCP connections in tcp_conn_from_tap(), tcp_tap_conn_from_sock(), and tcp_flow_migrate_target(), simplifying tcp_epoll_ctl() by removing the ADD/MOD conditional logic. Patch 3 removes EPOLLFD_ID_INVALID and related infrastructure now that all flows are registered at creation: flow_in_epoll(), flow_epollid_clear(), and the defensive checks in flow_epollfd(). Thanks, Laurent Laurent Vivier (3): tcp_splice: Register fds with epoll at flow creation tcp: Register fds with epoll at flow creation flow: Remove EPOLLFD_ID_INVALID flow.c | 34 ++-------------------------------- flow.h | 6 +----- icmp.c | 1 - tcp.c | 37 ++++++++++++++----------------------- tcp_splice.c | 20 ++++++++++---------- udp_flow.c | 1 - 6 files changed, 27 insertions(+), 72 deletions(-) -- 2.52.0
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
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.
Signed-off-by: Laurent Vivier
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
On Fri, Jan 16, 2026 at 04:52:23PM +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
LGTM, except for one possible nit.
--- flow.c | 35 ++--------------------------------- flow.h | 6 +----- icmp.c | 1 - tcp.c | 1 - udp_flow.c | 1 - 5 files changed, 3 insertions(+), 41 deletions(-)
diff --git a/flow.c b/flow.c index 532339ce7fe1..0c5503164714 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,8 +407,6 @@ 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); -
As with the other ASSERT()s this one could be updated to EPOLLFD_ID_SIZE, rather than removed.
epoll_id_to_fd[epollid] = epollfd; }
@@ -643,7 +613,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 d9bca041dea8..a23e4b067b36 100644 --- a/tcp.c +++ b/tcp.c @@ -3749,7 +3749,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 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
On Fri, Jan 16, 2026 at 04:52:21PM +0100, Laurent Vivier wrote:
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.
Signed-off-by: Laurent Vivier
Nice! One query below.
--- 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;
Do we need to worry about rollback here, if the first one succeeds, but the second one fails?
+ } + conn_event(conn, SPLICE_CONNECT);
if (connect(conn->s[1], &sa.sa, socklen_inany(&sa))) { -- 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 1/19/26 05:45, David Gibson wrote:
On Fri, Jan 16, 2026 at 04:52:21PM +0100, Laurent Vivier wrote:
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.
Signed-off-by: Laurent Vivier
Nice! One query below.
--- 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;
Do we need to worry about rollback here, if the first one succeeds, but the second one fails?
If we return an error here, tcp_splice_conn_from_sock() sets the CLOSING flag on the connection and conn_flag() handles the closing flag by calling epoll_del() for both sockets. So the cleanup path handles this case already. None of the error cases in tcp_splice_connect() worries about rollback, so it's simpler to do the same. Thanks, Laurent
+ } + conn_event(conn, SPLICE_CONNECT);
if (connect(conn->s[1], &sa.sa, socklen_inany(&sa))) { -- 2.52.0
On Mon, Jan 19, 2026 at 09:10:42AM +0100, Laurent Vivier wrote:
On 1/19/26 05:45, David Gibson wrote:
On Fri, Jan 16, 2026 at 04:52:21PM +0100, Laurent Vivier wrote:
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.
Signed-off-by: Laurent Vivier
Nice! One query below.
--- 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;
Do we need to worry about rollback here, if the first one succeeds, but the second one fails?
If we return an error here, tcp_splice_conn_from_sock() sets the CLOSING flag on the connection and conn_flag() handles the closing flag by calling epoll_del() for both sockets. So the cleanup path handles this case already.
None of the error cases in tcp_splice_connect() worries about rollback, so it's simpler to do the same.
Ok, that makes sense. Maybe add that explanation to the commit message? -- 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 1/19/26 05:48, David Gibson wrote:
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?
It's a good question. If EPOLL_CTL_ADD fails, the subsequent EPOLL_CTL_MOD called from tcp_epoll_ctl() will fail, so we can rely on that to handle the error... but in conn_event(), tcp_epoll_ctl() error is not handler. So I think we need at least to handle the error 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.
Same answer.
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
participants (2)
-
David Gibson
-
Laurent Vivier