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

[Stefano Brivio <sbrivio@redhat.com>]

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:");

?

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.

> +		fwd_rules_print(c->fwd[i]);
>  	}
>  }
>  
> @@ -2154,18 +2155,24 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  	/* Forwarding options can be parsed now, after IPv4/IPv6 settings */
>  	fwd_probe_ephemeral();
> +	fwd_rule_init(c);
>  	optind = 0;
>  	do {
>  		name = getopt_long(argc, argv, optstring, options, NULL);
>  
> -		if (name == 't')
> -			conf_ports(c, name, optarg, &c->fwd_in, &tcp_in_mode);
> -		else if (name == 'u')
> -			conf_ports(c, name, optarg, &c->fwd_in, &udp_in_mode);
> -		else if (name == 'T')
> -			conf_ports(c, name, optarg, &c->fwd_out, &tcp_out_mode);
> -		else if (name == 'U')
> -			conf_ports(c, name, optarg, &c->fwd_out, &udp_out_mode);
> +		if (name == 't') {
> +			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
> +				   &tcp_in_mode);
> +		} else if (name == 'u') {
> +			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
> +				   &udp_in_mode);
> +		} else if (name == 'T') {
> +			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
> +				   &tcp_out_mode);
> +		} else if (name == 'U') {
> +			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
> +				   &udp_out_mode);
> +		}
>  	} while (name != -1);
>  
>  	if (c->mode == MODE_PASTA)
> @@ -2224,20 +2231,24 @@ void conf(struct ctx *c, int argc, char **argv)
>  		udp_out_mode = fwd_default;
>  
>  	if (tcp_in_mode == FWD_MODE_AUTO) {
> -		conf_ports_range_except(c, 't', "auto", &c->fwd_in, NULL, NULL,
> -					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> +		conf_ports_range_except(c, 't', "auto", c->fwd[PIF_HOST],
> +					NULL, NULL, 1, NUM_PORTS - 1, NULL, 1,
> +					FWD_SCAN);
>  	}
>  	if (tcp_out_mode == FWD_MODE_AUTO) {
> -		conf_ports_range_except(c, 'T', "auto", &c->fwd_out, NULL, "lo",
> -					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> +		conf_ports_range_except(c, 'T', "auto", c->fwd[PIF_SPLICE],
> +					NULL, "lo", 1, NUM_PORTS - 1, NULL, 1,
> +					FWD_SCAN);
>  	}
>  	if (udp_in_mode == FWD_MODE_AUTO) {
> -		conf_ports_range_except(c, 'u', "auto", &c->fwd_in, NULL, NULL,
> -					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> +		conf_ports_range_except(c, 'u', "auto", c->fwd[PIF_HOST],
> +					NULL, NULL, 1, NUM_PORTS - 1, NULL, 1,
> +					FWD_SCAN);
>  	}
>  	if (udp_out_mode == FWD_MODE_AUTO) {
> -		conf_ports_range_except(c, 'U', "auto", &c->fwd_out, NULL, "lo",
> -					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> +		conf_ports_range_except(c, 'U', "auto", c->fwd[PIF_SPLICE],
> +					NULL, "lo", 1, NUM_PORTS - 1, NULL, 1,
> +					FWD_SCAN);
>  	}
>  
>  	if (!c->quiet)
> diff --git a/flow.c b/flow.c
> index 81333dbc..11cd5752 100644
> --- a/flow.c
> +++ b/flow.c
> @@ -503,10 +503,10 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  {
>  	char estr[INANY_ADDRSTRLEN], ostr[INANY_ADDRSTRLEN];
>  	struct flow_common *f = &flow->f;
> +	const struct fwd_table *fwd = c->fwd[f->pif[INISIDE]];
>  	const struct flowside *ini = &f->side[INISIDE];
>  	struct flowside *tgt = &f->side[TGTSIDE];
>  	const struct fwd_rule *rule = NULL;
> -	const struct fwd_table *fwd;
>  	uint8_t tgtpif = PIF_NONE;
>  
>  	assert(flow_new_entry == flow && f->state == FLOW_STATE_INI);
> @@ -514,6 +514,11 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  	assert(f->pif[INISIDE] != PIF_NONE && f->pif[TGTSIDE] == PIF_NONE);
>  	assert(flow->f.state == FLOW_STATE_INI);
>  
> +	if (fwd) {
> +		if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
> +			goto norule;
> +	}
> +
>  	switch (f->pif[INISIDE]) {
>  	case PIF_TAP:
>  		memcpy(f->tap_omac, MAC_UNDEF, ETH_ALEN);
> @@ -521,20 +526,10 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
>  		break;
>  
>  	case PIF_SPLICE:
> -		fwd = &c->fwd_out;
> -
> -		if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
> -			goto norule;
> -
>  		tgtpif = fwd_nat_from_splice(rule, proto, ini, tgt);
>  		break;
>  
>  	case PIF_HOST:
> -		fwd = &c->fwd_in;
> -
> -		if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
> -			goto norule;
> -
>  		tgtpif = fwd_nat_from_host(c, rule, proto, ini, tgt);
>  		fwd_neigh_mac_get(c, &tgt->oaddr, f->tap_omac);
>  		break;
> @@ -1014,8 +1009,7 @@ static int flow_migrate_source_rollback(struct ctx *c, unsigned bound, int ret)
>  
>  	debug("...roll back migration");
>  
> -	if (fwd_listen_sync(c, &c->fwd_in, PIF_HOST,
> -			    &c->tcp.scan_in, &c->udp.scan_in) < 0)
> +	if (fwd_listen_sync(c, PIF_HOST, &c->tcp.scan_in, &c->udp.scan_in) < 0)
>  		die("Failed to re-establish listening sockets");
>  
>  	foreach_established_tcp_flow(flow) {
> @@ -1148,7 +1142,7 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage,
>  	 * fix that is to not allow local to local migration, which arguably we
>  	 * should (use namespaces for testing instead). */
>  	debug("Stop listen()s");
> -	fwd_listen_close(&c->fwd_in);
> +	fwd_listen_close(c->fwd[PIF_HOST]);
>  
>  	debug("Sending %u flows", count);
>  
> diff --git a/fwd.c b/fwd.c
> index 7844a674..3395a28e 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -331,6 +331,21 @@ bool fwd_port_is_ephemeral(in_port_t port)
>  	return (port >= fwd_ephemeral_min) && (port <= fwd_ephemeral_max);
>  }
>  
> +/* Forwarding table storage, generally accessed via pointers in struct ctx */
> +static struct fwd_table fwd_in;
> +static struct fwd_table fwd_out;
> +
> +/**
> + * fwd_rule_init() - Initialise forwarding tables
> + * @c:		Execution context
> + */
> +void fwd_rule_init(struct ctx *c)
> +{
> +	c->fwd[PIF_HOST] = &fwd_in;
> +	if (c->mode == MODE_PASTA)
> +		c->fwd[PIF_SPLICE] = &fwd_out;
> +}
> +
>  /**
>   * fwd_rule_add() - Add a rule to a forwarding table
>   * @fwd:	Table to add to
> @@ -505,19 +520,17 @@ void fwd_rules_print(const struct fwd_table *fwd)
>  
>  /** fwd_sync_one() - Create or remove listening sockets for a forward entry
>   * @c:		Execution context
> - * @fwd:	Forwarding table
> - * @idx:	Rule index
>   * @pif:	Interface to create listening sockets for
> + * @idx:	Rule index
>   * @tcp:	Bitmap of TCP ports to listen for on FWD_SCAN entries
>   * @udp:	Bitmap of UDP ports to listen for on FWD_SCAN entries
>   *
>   * Return: 0 on success, -1 on failure
>   */
> -static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
> -			unsigned idx, uint8_t pif,
> +static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
>  			const uint8_t *tcp, const uint8_t *udp)
>  {
> -	const struct fwd_rule *rule = &fwd->rules[idx];
> +	const struct fwd_rule *rule = &c->fwd[pif]->rules[idx];
>  	const union inany_addr *addr = fwd_rule_addr(rule);
>  	const char *ifname = rule->ifname;
>  	const uint8_t *map = NULL;
> @@ -598,7 +611,6 @@ static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
>  
>  /** struct fwd_listen_args - arguments for fwd_listen_init_()
>   * @c:		Execution context
> - * @fwd:	Forwarding table
>   * @tcpmap:	Bitmap of TCP ports to auto-forward
>   * @udpmap:	Bitmap of TCP ports to auto-forward
>   * @pif:	Interface to create listening sockets for
> @@ -606,7 +618,6 @@ static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
>   */
>  struct fwd_listen_args {
>  	const struct ctx *c;
> -	const struct fwd_table *fwd;
>  	const uint8_t *tcpmap, *udpmap;
>  	uint8_t pif;
>  	int ret;
> @@ -625,9 +636,8 @@ static int fwd_listen_sync_(void *arg)
>  	if (a->pif == PIF_SPLICE)
>  		ns_enter(a->c);
>  
> -	for (i = 0; i < a->fwd->count; i++) {
> -		a->ret = fwd_sync_one(a->c, a->fwd, i, a->pif,
> -				      a->tcpmap, a->udpmap);
> +	for (i = 0; i < a->c->fwd[a->pif]->count; i++) {
> +		a->ret = fwd_sync_one(a->c, a->pif, i, a->tcpmap, a->udpmap);
>  		if (a->ret < 0)
>  			break;
>  	}
> @@ -637,21 +647,17 @@ static int fwd_listen_sync_(void *arg)
>  
>  /** fwd_listen_sync() - Call fwd_listen_sync_() in correct namespace
>   * @c:		Execution context
> - * @fwd:	Forwarding information
>   * @pif:	Interface to create listening sockets for
>   * @tcp:	Scanning state for TCP
>   * @udp:	Scanning state for UDP
>   *
>   * Return: 0 on success, -1 on failure
>   */
> -int fwd_listen_sync(const struct ctx *c, const struct fwd_table *fwd,
> -		    uint8_t pif,
> +int fwd_listen_sync(const struct ctx *c, uint8_t pif,
>  		    const struct fwd_scan *tcp, const struct fwd_scan *udp)
>  {
>  	struct fwd_listen_args a = {
> -		.c = c, .fwd = fwd,
> -		.tcpmap = tcp->map, .udpmap = udp->map,
> -		.pif = pif,
> +		.c = c, .tcpmap = tcp->map, .udpmap = udp->map, .pif = pif,
>  	};
>  
>  	if (pif == PIF_SPLICE)
> @@ -695,12 +701,11 @@ void fwd_listen_close(const struct fwd_table *fwd)
>   */
>  int fwd_listen_init(const struct ctx *c)
>  {
> -	if (fwd_listen_sync(c, &c->fwd_in, PIF_HOST,
> -			    &c->tcp.scan_in, &c->udp.scan_in) < 0)
> +	if (fwd_listen_sync(c, PIF_HOST, &c->tcp.scan_in, &c->udp.scan_in) < 0)
>  		return -1;
>  
>  	if (c->mode == MODE_PASTA) {
> -		if (fwd_listen_sync(c, &c->fwd_out, PIF_SPLICE,
> +		if (fwd_listen_sync(c, PIF_SPLICE,
>  				    &c->tcp.scan_out, &c->udp.scan_out) < 0)
>  			return -1;
>  	}
> @@ -851,16 +856,16 @@ static void fwd_scan_ports(struct ctx *c)
>  	uint8_t excl_tcp_out[PORT_BITMAP_SIZE], excl_udp_out[PORT_BITMAP_SIZE];
>  	uint8_t excl_tcp_in[PORT_BITMAP_SIZE], excl_udp_in[PORT_BITMAP_SIZE];
>  
> -	current_listen_map(excl_tcp_out, &c->fwd_in, IPPROTO_TCP);
> -	current_listen_map(excl_tcp_in, &c->fwd_out, IPPROTO_TCP);
> -	current_listen_map(excl_udp_out, &c->fwd_in, IPPROTO_UDP);
> -	current_listen_map(excl_udp_in, &c->fwd_out, IPPROTO_UDP);
> +	current_listen_map(excl_tcp_out, c->fwd[PIF_HOST], IPPROTO_TCP);
> +	current_listen_map(excl_tcp_in, c->fwd[PIF_SPLICE], IPPROTO_TCP);
> +	current_listen_map(excl_udp_out, c->fwd[PIF_HOST], IPPROTO_UDP);
> +	current_listen_map(excl_udp_in, c->fwd[PIF_SPLICE], IPPROTO_UDP);
>  
> -	fwd_scan_ports_tcp(&c->fwd_out, &c->tcp.scan_out, excl_tcp_out);
> -	fwd_scan_ports_tcp(&c->fwd_in, &c->tcp.scan_in, excl_tcp_in);
> -	fwd_scan_ports_udp(&c->fwd_out, &c->udp.scan_out,
> +	fwd_scan_ports_tcp(c->fwd[PIF_SPLICE], &c->tcp.scan_out, excl_tcp_out);
> +	fwd_scan_ports_tcp(c->fwd[PIF_HOST], &c->tcp.scan_in, excl_tcp_in);
> +	fwd_scan_ports_udp(c->fwd[PIF_SPLICE], &c->udp.scan_out,
>  			   &c->tcp.scan_out, excl_udp_out);
> -	fwd_scan_ports_udp(&c->fwd_in, &c->udp.scan_in,
> +	fwd_scan_ports_udp(c->fwd[PIF_HOST], &c->udp.scan_in,
>  			   &c->tcp.scan_in, excl_udp_in);
>  }
>  
> @@ -912,10 +917,8 @@ void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now)
>  
>  	fwd_scan_ports(c);
>  
> -	fwd_listen_sync(c, &c->fwd_in, PIF_HOST,
> -			&c->tcp.scan_in, &c->udp.scan_in);
> -	fwd_listen_sync(c, &c->fwd_out, PIF_SPLICE,
> -			&c->tcp.scan_out, &c->udp.scan_out);
> +	fwd_listen_sync(c, PIF_HOST, &c->tcp.scan_in, &c->udp.scan_in);
> +	fwd_listen_sync(c, PIF_SPLICE, &c->tcp.scan_out, &c->udp.scan_out);
>  }
>  
>  /**
> diff --git a/fwd.h b/fwd.h
> index 958eee25..b387d926 100644
> --- a/fwd.h
> +++ b/fwd.h
> @@ -108,6 +108,7 @@ struct fwd_scan {
>  
>  #define FWD_PORT_SCAN_INTERVAL		1000	/* ms */
>  
> +void fwd_rule_init(struct ctx *c);
>  void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
>  		  const union inany_addr *addr, const char *ifname,
>  		  in_port_t first, in_port_t last, in_port_t to);
> @@ -119,8 +120,7 @@ void fwd_rules_print(const struct fwd_table *fwd);
>  void fwd_scan_ports_init(struct ctx *c);
>  void fwd_scan_ports_timer(struct ctx * c, const struct timespec *now);
>  
> -int fwd_listen_sync(const struct ctx *c, const struct fwd_table *fwd,
> -		    uint8_t pif,
> +int fwd_listen_sync(const struct ctx *c, uint8_t pif,
>  		    const struct fwd_scan *tcp, const struct fwd_scan *udp);
>  void fwd_listen_close(const struct fwd_table *fwd);
>  int fwd_listen_init(const struct ctx *c);
> 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.

>  
>  	int no_tcp;
>  	struct tcp_ctx tcp;

-- 
Stefano