Re: [PATCH] tcp: remove timer update in tcp_epoll_ctl()

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

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

On Thu, Jan 08, 2026 at 01:22:41PM +0100, Laurent Vivier wrote:
> Timer update in tcp_epoll_ctl() has been added by
>   bb708111833e ("treewide: Packet abstraction with mandatory boundary checks")
> because epoll_ref stores "conn - tc" that can change in tcp_table_compact().
> 
> But since
>   e2e8219f13b8 ("flow, tcp: Consolidate flow pointer<->index helpers")
> we use FLOW_IDX() and since
>   8981a720aac4 ("flow: Avoid moving flow entries to compact table")
> flow table doesn't use a compaction procedure so FLOW_IDX() never changes.
> 
> Updating the timer event is now a no-op, remove it from tcp_epoll_ctl().
> 
> Fixes: 8981a720aac4 ("flow: Avoid moving flow entries to compact table")
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Nice catch.

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

> ---
>  tcp.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index e7fa85f346b2..5141cdc7e839 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -552,18 +552,6 @@ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
>  
>  	flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT);
>  
> -	if (conn->timer != -1) {
> -		union epoll_ref ref_t = { .type = EPOLL_TYPE_TCP_TIMER,
> -					  .fd = conn->timer,
> -					  .flow = FLOW_IDX(conn) };
> -		struct epoll_event ev_t = { .data.u64 = ref_t.u64,
> -					    .events = EPOLLIN | EPOLLET };
> -
> -		if (epoll_ctl(flow_epollfd(&conn->f), EPOLL_CTL_MOD,
> -			      conn->timer, &ev_t))
> -			return -errno;
> -	}
> -
>  	return 0;
>  }
>  
> -- 
> 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

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