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

David Gibson
2026-04-08T01:37:13Z

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