Re: [PATCH 4/7] flow: Delegate epoll file descriptor management to threading subsystem

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4686 bytes --]

On Fri, Jul 31, 2026 at 06:46:25PM +0200, Laurent Vivier wrote:
> Remove the flow-local epoll_id_to_fd mapping array and instead
> rely on the threading subsystem to provide the epoll file descriptor
> for a given thread number.
> 
> Update all protocol handlers (ICMP, TCP, TCP splice, UDP).
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  flow.c  | 17 +++++++++--------
>  flow.h  |  4 +---
>  passt.c |  2 +-
>  tcp.c   |  3 ++-
>  4 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/flow.c b/flow.c
> index 3012a07ed680..7821b3f91285 100644
> --- a/flow.c
> +++ b/flow.c
> @@ -24,6 +24,7 @@
>  #include "repair.h"
>  #include "epoll_ctl.h"
>  #include "serialise.h"
> +#include "threading.h"
>  
>  const char *flow_state_str[] = {
>  	[FLOW_STATE_FREE]	= "FREE",
> @@ -221,7 +222,6 @@ static_assert(ARRAY_SIZE(flow_epoll) == FLOW_NUM_TYPES,
>  unsigned flow_first_free;
>  union flow flowtab[FLOW_MAX];
>  static _Thread_local const union flow *flow_new_entry; /* = NULL */
> -int qpair_to_fd[FLOW_QPAIR_SIZE];
>  
>  /* Hash table to index it */
>  #define FLOW_HASH_LOAD		70		/* % */
> @@ -457,7 +457,12 @@ static void flow_set_state(struct flow_common *f, enum flow_state state)
>   */
>  int flow_epollfd(const struct flow_common *f)
>  {
> -	return qpair_to_fd[f->qpair];
> +	/* mapping 1:1 between qpair and threadid
> +	 * return threading_epollfd(f->qpair);
> +	 * but for the moment we have only one thread
> +	 */
> +	(void)f;
> +	return threading_epollfd(THREADING_ID_DEFAULT);

This seems like it's going backwards, always using thread 0's epollfd,
rather than choosing the right thread based on the qpair.

>  }
>  
>  /**
> @@ -525,7 +530,7 @@ bool flow_migrate_epollfd(struct flow_common *f, unsigned int qpair,
>  	flow_trace((union flow *)f,
>  		   "migrating from qpair %d to %d", qpair, target);
>  
> -	epoll_del(qpair_to_fd[qpair], ref.fd);
> +	epoll_del(threading_epollfd(qpair), ref.fd);
>  	flow_setqp(f, target);
>  	flow_epoll_set(f, EPOLL_CTL_ADD, events, ref.fd, ref.flowside.sidei);
>  	ret = true;
> @@ -1444,9 +1449,8 @@ int flow_migrate_target(struct ctx *c, const struct migrate_stage *stage,
>  
>  /**
>   * flow_init() - Initialise flow related data structures
> - * @c:		Execution context
>   */
> -void flow_init(const struct ctx *c)
> +void flow_init(void)
>  {
>  	unsigned b;
>  
> @@ -1456,7 +1460,4 @@ void flow_init(const struct ctx *c)
>  
>  	for (b = 0; b < FLOW_HASH_SIZE; b++)
>  		flow_hashtab[b] = FLOW_SIDX_NONE;
> -
> -	for (b = 0; b < FLOW_QPAIR_SIZE; b++)
> -		qpair_to_fd[b] = c->epollfd;
>  }
> diff --git a/flow.h b/flow.h
> index c31a51a9cc96..e2693efd0f70 100644
> --- a/flow.h
> +++ b/flow.h
> @@ -157,8 +157,6 @@ struct flowside {
>  	in_port_t		eport;
>  };
>  
> -extern int qpair_to_fd[];
> -
>  /**
>   * flowside_eq() - Check if two flowsides are equal
>   * @left, @right:	Flowsides to compare
> @@ -266,7 +264,7 @@ flow_sidx_t flow_lookup_sa(const struct ctx *c, uint8_t proto, uint8_t pif,
>  
>  union flow;
>  
> -void flow_init(const struct ctx *c);
> +void flow_init(void);
>  int flow_epollfd(const struct flow_common *f);
>  int flow_epoll_set(const struct flow_common *f, int command, uint32_t events,
>  		   int fd, unsigned int sidei);
> diff --git a/passt.c b/passt.c
> index 2cedb7ba0756..57290a5a7c6e 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -399,7 +399,7 @@ int main(int argc, char **argv)
>  	if (clock_gettime(CLOCK_MONOTONIC, &now))
>  		die_perror("Failed to get CLOCK_MONOTONIC time");
>  
> -	flow_init(c);
> +	flow_init();
>  	fwd_scan_ports_init(c);
>  
>  	if ((!c->no_udp && udp_init(c)) || (!c->no_tcp && tcp_init(c)))
> diff --git a/tcp.c b/tcp.c
> index 2767a8494107..e8da65410bcf 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -317,6 +317,7 @@
>  #include "tcp_buf.h"
>  #include "tcp_vu.h"
>  #include "epoll_ctl.h"
> +#include "threading.h"
>  
>  /*
>   * The size of TCP header (including options) is given by doff (Data Offset)
> @@ -2721,7 +2722,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref,
>  	assert(conn->f.type == FLOW_TCP);
>  
>  	if (conn->f.qpair != qpair) {
> -		int old_epollfd = qpair_to_fd[qpair];
> +		int old_epollfd = threading_epollfd(qpair);
>  
>  		epoll_del(old_epollfd, conn->timer);
>  		if (tcp_timer_epoll_add(conn, conn->timer, now) < 0) {
> -- 
> 2.54.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]