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

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

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

On Wed, May 06, 2026 at 10:12:21AM +0200, Laurent Vivier wrote:
> On 5/6/26 01:47, 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>
> > [sbrivio: In fwd_listen_switch(), use the destination size as argument
> >   to memcpy(), instead of sizeof(tmp), as suggested by Laurent]
> > 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 76344da..3f48793 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -2160,15 +2160,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..0697435 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;
> > +	}
> > +
> > +	/* Swap active and pending tables */
> > +	static_assert(sizeof(tmp) == sizeof(c->fwd) &&
> > +		      sizeof(tmp) == sizeof(c->fwd_pending),
> > +		      "Temporary has wrong size");
> 
> At this point:
> 
> c->fwd[PIF_HOST] = &fwd_in;
> c->fwd[PIF_SPLICE] = &fwd_out;
> 
> c->fwd_pending[PIF_HOST] = &fwd_in_pending;
> c->fwd_pending[PIF_SPLICE] = &fwd_out_pending;
> 
> > +	memcpy(&tmp, (void *)c->fwd, sizeof(tmp));
> > +	memcpy((void *)c->fwd, (void *)c->fwd_pending, sizeof(c->fwd));
> > +	memcpy((void *)c->fwd_pending, &tmp, sizeof(c->fwd_pending));
> 
> At this point:
> 
> c->fwd[PIF_HOST] = &fwd_in_pending;
> c->fwd[PIF_SPLICE] = &fwd_out_pending;
> 
> c->fwd_pending[PIF_HOST] = &fwd_in;
> c->fwd_pending[PIF_SPLICE] = &fwd_out;
> 
> Perhaps it should be noted somewhere to avoid confusion in the future?
> Or to copy the content of the rules rather than the pointer to the rules?

Ah, good point.  The problem is the names of the globals - it's not
really one "real" and one "pending" table, just two tables that switch
between those roles.

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