Re: [PATCH 3/7] udp: Unconditionally clear act flag in udp_timer_one()

On Tue, Feb 27, 2024 at 12:56:32PM +0100, Stefano Brivio wrote:

[Sorry, I wrote this comment a while ago and just now realised I didn't send this out...]

On Thu, 22 Feb 2024 10:21:11 +1100 David Gibson wrote:

...

udp_timer_one() first checks for expiries of various sorts of sockets and if necessary sets the 'sockp' variable to trigger a cleanup at the end. If sockp is *not* set then, correctly, we don't attempt to close a non-existent socket. However, we also don't clear the flag in the udp_act[] map, in which case we'll come back here and there will, again, be nothing to be done.

Well, but that's intended: if we didn't reach UDP_CONN_TIMEOUT for this port, the socket should still be in udp_act[], meaning we'll check it against activity timeouts and close on timeout.

Otherwise, we'll never check that port for the activity timeout, right?

Ah, bother. Yes, my patch is entirely wrong, I'll drop.

...

So, clear the udp_act[] flag, even if we don't need to clean up a socket.

Signed-off-by: David Gibson --- udp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/udp.c b/udp.c index 03a5936e..4200f849 100644 --- a/udp.c +++ b/udp.c @@ -1123,8 +1123,9 @@ static void udp_timer_one(struct ctx *c, int v6, enum udp_act_type type, *sockp = -1; epoll_ctl(c->epollfd, EPOLL_CTL_DEL, s, NULL); close(s); - bitmap_clear(udp_act[v6 ? V6 : V4][type], port); } + + bitmap_clear(udp_act[v6 ? V6 : V4][type], port); }

/**

-- David Gibson | 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