Re: [PATCH v5 4/7] tcp, flow: Replace per-connection in_epoll flag with an epollid in flow_common

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

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

On Tue, Oct 21, 2025 at 11:01:13PM +0200, Laurent Vivier wrote:
> The in_epoll boolean flag in tcp_tap_conn and tcp_splice_conn only tracked
> whether a connection was registered with epoll, not which epoll instance.
> This limited flexibility for future multi-epoll support.
> 
> Replace the boolean with an epollid field in flow_common that identifies
> which epoll instance the flow is registered with.
> Use FLOW_EPOLLID_INVALID to indicate when a flow is not registered with
> any epoll instance. An epoll_id_to_fd[] mapping table translates
> epoll ids to their corresponding epoll file descriptors.
> 
> Add helper functions:
> - flow_in_epoll() to check if a flow is registered with epoll
> - flow_epollfd() to retrieve the epoll fd for a flow's thread
> - flow_epollid_register() to register an epoll fd with an epollid
> - flow_epollid_set() to set the epollid of a flow
> - flow_epollid_clear() to reset the epoll id of a flow
> 
> This change also simplifies tcp_timer_ctl() and conn_flag_do() by removing
> the need to pass the context 'c', since the epoll fd is now directly
> accessible from the flow structure via flow_epollfd().
> 
> Add a defensive check at the beginning of tcp_flow_repair_queue() to
> avoid a false positive with "make clang-tidy":
>   error: The 1st argument to 'send' is < 0 but should be >= 0
>    3230 |                 ssize_t rc = send(conn->sock, p, MIN(len, chunk), 0);
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

[snip]
> @@ -570,7 +573,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
>  		}
>  		conn->timer = fd;
>  
> -		if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, conn->timer, &ev)) {
> +		if (epoll_ctl(epollfd, EPOLL_CTL_ADD, conn->timer, &ev)) {

I still think it's a bug that the timer's ref.fd field isn't the timer
fd, but I recognize that fixing that is not in scope for this series.

-- 
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 --]