Re: [PATCH v2 06/15] fwd: Store forwarding tables indexed by (origin) pif

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

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

On Fri, Mar 20, 2026 at 09:58:34PM +0100, Stefano Brivio wrote:
> On Thu, 19 Mar 2026 17:11:48 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Currently we store the inbound (PIF_HOST) and outbound (PIF_SPLICE)
> > forwarding tables in separate fields of struct ctx.  In a number of places
> > this requires somewhat awkward if or switch constructs to select the
> > right table for updates.  Conceptually simplify that by using an index of
> > forwarding tables by pif, which as a bonus keeps track generically which
> > pifs have implemented forwarding tables so far.
> > 
> > For now this doesn't simplify a lot textually, because many places that
> > need this also have other special cases to apply by pif.  It does simplify
> > a few crucial places though, and we expect it will become more useful as
> > the flexibility of the forwarding table is improved.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  conf.c  | 53 +++++++++++++++++++++++++++-------------------
> >  flow.c  | 22 +++++++------------
> >  fwd.c   | 65 ++++++++++++++++++++++++++++++---------------------------
> >  fwd.h   |  4 ++--
> >  passt.h |  3 +--
> >  5 files changed, 77 insertions(+), 70 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index 940fb9e9..6af3c8a5 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -1252,11 +1252,12 @@ dns6:
> >  		}
> >  	}
> >  
> > -	info("Inbound forwarding:");
> > -	fwd_rules_print(&c->fwd_in);
> > -	if (c->mode == MODE_PASTA) {
> > -		info("Outbound forwarding:");
> > -		fwd_rules_print(&c->fwd_out);
> > +	for (i = 0; i < PIF_NUM_TYPES; i++) {
> > +		if (!c->fwd[i])
> > +			continue;
> > +
> > +		info("Forwarding from %s:", pif_name(i));
> 
> I don't have a good solution to propose but it's slightly annoying that
> we're changing very clear "Inbound forwarding" and "Outbound
> forwarding" indications to "Forwarding from: " HOST | TAP | SPLICE.
> 
> Should we perhaps introduce a PIF_INBOUND_MAX value that's the same as
> HOST, and then:
> 
> 		if (i <= PIF_INBOUND_MAX)
> 			info("Inbound forwarding:");
> 		else
> 			info("Outbound forwarding:");
> 
> ?

That will become ambiguous as soon as we add tables for TAP.

> I'm fine keeping it as it is, I would just have a slight preference to
> make it as clear as it was before. This is something users can now look
> at to double check things and I have the feeling we're avoid a bunch of
> bug reports as a result.

For now I've gone with:
	{Inbound|Outbound} forwarding rules (<pif name>):

> > diff --git a/passt.h b/passt.h
> > index b614bdf0..5fc4e07f 100644
> > --- a/passt.h
> > +++ b/passt.h
> > @@ -264,8 +264,7 @@ struct ctx {
> >  	unsigned int pasta_ifi;
> >  	int pasta_conf_ns;
> >  
> > -	struct fwd_table fwd_in;
> > -	struct fwd_table fwd_out;
> > +	struct fwd_table *fwd[PIF_NUM_TYPES];
> 
> Nit: the struct documentation should be updated accordingly.

Fixed.

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