Re: [PATCH 02/18] conf: Simplify handling of default forwarding mode

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

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

On Wed, Apr 08, 2026 at 01:14:36AM +0200, Stefano Brivio wrote:
> On Tue,  7 Apr 2026 13:16:14 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > For passt, the default forwarding mode is "none", which falls out naturally
> > from the other handling: if we don't get any options, we get empty
> > forwarding tables, which corresponds to "none" behaviour.  However, for
> > pasta the default is "auto".  This is handled a bit oddly: in conf_ports()
> > we set the mode variable, but don't set up the rules we need for "auto"
> > mode.  Instead we want until nearly the end of conf() and if the mode is
> > FWD_MODE_AUTO or unset, we make conf_ports_range_except() calls to set up
> > the "auto" rules.
> > 
> > Simplify this a bit, by creating the rules within conf_ports() itself when
> > we parse -[tuTU] auto.  For the case of no forwarding options we call
> > into conf_ports() itself with synthetic arguments.  As well as making the
> > code a little shorter, this makes it more obvious that giving no arguments
> > really is equivalent to -[tuTU] auto.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  conf.c | 54 ++++++++++++++++++++++--------------------------------
> >  1 file changed, 22 insertions(+), 32 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index c515480b..7d718f91 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -345,6 +345,10 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> >  			die("'auto' port forwarding is only allowed for pasta");
> >  
> >  		*mode = FWD_MODE_AUTO;
> > +
> > +		conf_ports_range_except(c, optname, optarg, fwd, NULL, NULL,
> > +					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> > +
> >  		return;
> >  	}
> 
> Pre-existing, it goes away in 12/18 of the current series, and it's
> harmless unless somebody should ever touch 'enum fwd_mode' in a rather
> unlikely way: the code here checks:
> 
> 		if (*mode)
> 			...
> 
> relying on the fact that FWD_MODE_UNSPEC is 0 (and we also have a _NONE
> value, which makes it somewhat risky) instead of directly checking if
> (*mode == FWD_MODE_UNSPEC).
> 
> I thought I would note it here in the unlikely case we'd need to back to
> mode constants for whatever reason.

Right.  I noticed this, and my solution was to remove enum fwd_mode
entirely in 12/18 as noted :).

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