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

[Laurent Vivier <lvivier@redhat.com>]

On 5/5/26 12:15, Stefano Brivio wrote:
> 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.
> 

Then add my:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>