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

On Wed, May 06, 2026 at 10:49:55AM +0200, Stefano Brivio wrote:

On Wed, 6 May 2026 10:39:30 +0200 Stefano Brivio wrote:

...

On Wed, 6 May 2026 10:12:21 +0200 Laurent Vivier wrote:

...

On 5/6/26 01:47, Stefano Brivio wrote:

...

From: David Gibson

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 [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 --- 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;

Yeah, makes sense, I can change that in v9.

...

Perhaps it should be noted somewhere to avoid confusion in the future?

What do you think should be noted exactly, and where? Can you show a practical example of the change you're proposing?

...I'm leaving like it is in v9 to make sure I'm not misinterpreting you, and also because the current (v8) version is obviously correct and I also tested it fairly heavily by now.

I'd suggest optimising this (and commenting as needed) in a separate patch later.

As noted in another branch of this thread, I think all it really needs is renaming the globals that c->fwd and c->fwd_pending point to. They should just be fwd_in_[12] (or an array of 2 tables), instead of implying a semantic difference between the plain and "pending" copies. -- 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