Re: [PATCH v7 17/18] conf, fwd: Allow switching to new rules received from pesto

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 5 May 2026 19:53:43 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, May 05, 2026 at 11:08:27AM +0200, Laurent Vivier wrote:
> > On 5/5/26 01:11, Stefano Brivio wrote:  
> > > From: David Gibson <david@gibson.dropbear.id.au>
> > > 
> > > We can now receive updates to the forwarding rules from the pesto client
> > > and store them in a "pending" copy of the forwarding tables.  Implement
> > > switching to using the new rules.
> > > 
> > > The logic is in a new fwd_listen_switch().  For now this closes all
> > > listening sockets related to the old tables, swaps the active and pending
> > > tables, then listens based on the new tables.  In future we look to improve
> > > this so that we don't temporarily stop listening on ports that both the
> > > old and new tables specify.
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > > ---
> > >   conf.c |  5 ++---
> > >   fwd.c  | 34 ++++++++++++++++++++++++++++++++++
> > >   fwd.h  |  1 +
> > >   3 files changed, 37 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/conf.c b/conf.c
> > > index f035fd3..75b8291 100644
> > > --- a/conf.c
> > > +++ b/conf.c
> > > @@ -2159,15 +2159,14 @@ void conf_handler(struct ctx *c, uint32_t events)
> > >   			fwd_rules_dump(info, fwd->rules, fwd->count,
> > >   				       "    ", "");
> > >   		}
> > > +
> > > +		fwd_listen_switch(c);
> > >   	}
> > >   	if (events & EPOLLHUP) {
> > >   		debug("Configuration client hangup");
> > > -		goto close;
> > >   	}
> > > -	return;
> > > -
> > >   close:
> > >   	conf_close(c);
> > > diff --git a/fwd.c b/fwd.c
> > > index d93d2e5..35b9e2b 100644
> > > --- a/fwd.c
> > > +++ b/fwd.c
> > > @@ -534,6 +534,40 @@ int fwd_listen_init(const struct ctx *c)
> > >   	return 0;
> > >   }
> > > +/**
> > > + * fwd_listen_switch() - Switch from current to pending rules table
> > > + * @c:		Execution context
> > > + */
> > > +void fwd_listen_switch(struct ctx *c)
> > > +{
> > > +	struct fwd_table *tmp[PIF_NUM_TYPES];
> > > +	unsigned i;
> > > +
> > > +	/* Stop listening on the old tables */
> > > +	for (i = 0; i < PIF_NUM_TYPES; i++) {
> > > +		struct fwd_table *fwd = c->fwd[i];
> > > +
> > > +		if (!fwd)
> > > +			continue;
> > > +
> > > +		debug("Flushing %u old %s rules", fwd->count, pif_name(i));
> > > +		fwd_listen_close(fwd);
> > > +		fwd->count = fwd->sock_count = 0;  
> > 
> > Perhaps we can reset fwd->count and fwd->sock_count in fwd_listen_close() as
> > after fwd_listen_close() these values are wrong?  
> 
> No, they're not.  fwd_listen_close() closes the listening sockets, but
> it doesn't remove the rules.  fwd->sock_count isn't the number of
> *open* listening sockets, it's the maximum potential number of sockets
> for all the rules.  Having some or all of the sockets close (-1 stored
> in the array) is an allowed state.  It's rare for most rules, but
> routine for SCAN ("auto") rules.

Ah, oops, I didn't realise that would be the case for "auto" rules.
I'll leave this part as it is then.

-- 
Stefano