Re: [PATCH v3 08/25] fwd: Allow FWD_DUAL_STACK_ANY flag to be passed directly to fwd_rule_add()

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

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

On Wed, Mar 25, 2026 at 01:54:33AM +0100, Stefano Brivio wrote:
> Nits:
> 
> On Mon, 23 Mar 2026 18:37:15 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > fwd_rule_add() takes a flags parameter, but it only allows the FWD_WEAK
> > and FWD_SCAN flags to be specified there.  It doesn't allow the
> > FWD_DUAL_STACK_ANY flag to be set, instead expecting a [*] address to be
> > indicated by passing NULL as @addr.
> > 
> > However, for upcoming dynamic rule updates, it's more convenient to be able
> > to explicitly pass FWD_DUAL_STACK_ANY along with an address of ::.  Allow
> > that mode of calling.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  fwd.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fwd.c b/fwd.c
> > index 3395a28e..d73b7ca7 100644
> > --- a/fwd.c
> > +++ b/fwd.c
> > @@ -362,18 +362,21 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
> >  		  in_port_t first, in_port_t last, in_port_t to)
> >  {
> >  	/* Flags which can be set from the caller */
> > -	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN;
> > +	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
> >  	unsigned num = (unsigned)last - first + 1;
> >  	struct fwd_rule *new;
> >  	unsigned i, port;
> >  
> >  	assert(!(flags & ~allowed_flags));
> > -
> 
> Spurious change, I think? The extra newline here looks good for
> readability.

Oops, yes.

> Maybe we should group the second assert with this one?

Good idea, done.

> 
> >  	if (fwd->count >= ARRAY_SIZE(fwd->rules))
> >  		die("Too many port forwarding ranges");
> >  	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks))
> >  		die("Too many listening sockets");
> >  
> > +	/* Passing a non-wildcard address with DUAL_STACK_ANY  is a bug */
> 
> Extra whitespace after DUAL_STACK_ANY.

Fixed.

> 
> > +	assert(!(flags & FWD_DUAL_STACK_ANY) || !addr ||
> > +	       inany_equals(addr, &inany_any6));
> > +
> >  	/* Check for any conflicting entries */
> >  	for (i = 0; i < fwd->count; i++) {
> >  		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
> 
> -- 
> Stefano
> 

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