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