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 <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> > ---
> >  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