Re: [PATCH v3 06/11] fwd, conf: Add capabilities bits to each forwarding table

[Laurent Vivier <lvivier@redhat.com>]

On 4/17/26 07:05, David Gibson wrote:
> conf_ports_spec() and conf_ports() take the global context structure, but
> their only use for it is seeing if various things are possible: which
> protocols and address formats are allowed in formatting rules.  Localise
> that information into the forwarding table, with a capabilities bitmap.
> 
> For now we set that caps map to the same thing for all tables, but keep it
> per-table to allow for the possibility of different pif types in future
> that might have different capabilities (e.g. if we add a forwarding table
> for the tap interface, it won't be able to accept interface names to bind).
> 
> Use this information to remove the global context parameter from
> conf_ports() and conf_ports_spec().
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   conf.c     | 48 ++++++++++++++++++++++--------------------------
>   fwd.c      | 17 +++++++++++++++++
>   fwd.h      |  2 ++
>   fwd_rule.h |  8 ++++++++
>   4 files changed, 49 insertions(+), 26 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 99542075..ecc3a342 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -218,15 +218,13 @@ fail:
>   
>   /**
>    * conf_ports_spec() - Parse port range(s) specifier
> - * @c:		Execution context
>    * @fwd:	Forwarding table to be updated
>    * @proto:	Protocol to forward
>    * @addr:	Listening address for forwarding
>    * @ifname:	Interface name for listening
>    * @spec:	Port range(s) specifier
>    */
> -static void conf_ports_spec(const struct ctx *c,
> -			    struct fwd_table *fwd, uint8_t proto,
> +static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto,
>   			    const union inany_addr *addr, const char *ifname,
>   			    const char *spec)
>   {
> @@ -255,7 +253,7 @@ static void conf_ports_spec(const struct ctx *c,
>   			if (p != ep) /* Garbage after the keyword */
>   				goto bad;
>   
> -			if (c->mode != MODE_PASTA) {
> +			if (!(fwd->caps & FWD_CAP_SCAN)) {
>   				die(
>   "'auto' port forwarding is only allowed for pasta");
>   			}
> @@ -329,13 +327,11 @@ bad:
>   
>   /**
>    * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets
> - * @c:		Execution context
>    * @optname:	Short option name, t, T, u, or U
>    * @optarg:	Option argument (port specification)
>    * @fwd:	Forwarding table to be updated
>    */
> -static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> -		       struct fwd_table *fwd)
> +static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd)
>   {
>   	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
>   	char buf[BUFSIZ], *spec, *ifname = NULL;
> @@ -360,9 +356,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>   		return;
>   	}
>   
> -	if (proto == IPPROTO_TCP && c->no_tcp)
> +	if (proto == IPPROTO_TCP && !(fwd->caps & FWD_CAP_TCP))
>   		die("TCP port forwarding requested but TCP is disabled");
> -	if (proto == IPPROTO_UDP && c->no_udp)
> +	if (proto == IPPROTO_UDP && !(fwd->caps & FWD_CAP_UDP))
>   		die("UDP port forwarding requested but UDP is disabled");
>   
>   	strncpy(buf, optarg, sizeof(buf) - 1);
> @@ -410,10 +406,10 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>   	}
>   
>   	if (addr) {
> -		if (!c->ifi4 && inany_v4(addr)) {
> +		if (!(fwd->caps & FWD_CAP_IPV4) && inany_v4(addr)) {
>   			die("IPv4 is disabled, can't use -%c %s",
>   			    optname, optarg);
> -		} else if (!c->ifi6 && !inany_v4(addr)) {
> +		} else if (!(fwd->caps & FWD_CAP_IPV6) && !inany_v4(addr)) {
>   			die("IPv6 is disabled, can't use -%c %s",
>   			    optname, optarg);
>   		}
> @@ -422,17 +418,17 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>   	if (optname == 'T' || optname == 'U') {
>   		assert(!addr && !ifname);
>   
> -		if (c->no_bindtodevice) {
> +		if (!(fwd->caps & FWD_CAP_IFNAME)) {
>   			warn(
>   "SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'",
>   			     optname, optarg);
>   
> -			if (c->ifi4) {
> -				conf_ports_spec(c, fwd, proto,
> +			if (fwd->caps & FWD_CAP_IPV4) {
> +				conf_ports_spec(fwd, proto,
>   						&inany_loopback4, NULL, spec);
>   			}
> -			if (c->ifi6) {
> -				conf_ports_spec(c, fwd, proto,
> +			if (fwd->caps & FWD_CAP_IPV6) {
> +				conf_ports_spec(fwd, proto,
>   						&inany_loopback6, NULL, spec);
>   			}
>   			return;
> @@ -441,13 +437,13 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>   		ifname = "lo";
>   	}
>   
> -	if (ifname && c->no_bindtodevice) {
> +	if (ifname && !(fwd->caps & FWD_CAP_IFNAME)) {
>   		die(
>   "Device binding for '-%c %s' unsupported (requires kernel 5.7+)",
>   		    optname, optarg);
>   	}
>   
> -	conf_ports_spec(c, fwd, proto, addr, ifname, spec);
> +	conf_ports_spec(fwd, proto, addr, ifname, spec);
>   }
>   
>   /**
> @@ -2185,16 +2181,16 @@ void conf(struct ctx *c, int argc, char **argv)
>   
>   		if (name == 't') {
>   			opt_t = true;
> -			conf_ports(c, name, optarg, c->fwd[PIF_HOST]);
> +			conf_ports(name, optarg, c->fwd[PIF_HOST]);
>   		} else if (name == 'u') {
>   			opt_u = true;
> -			conf_ports(c, name, optarg, c->fwd[PIF_HOST]);
> +			conf_ports(name, optarg, c->fwd[PIF_HOST]);
>   		} else if (name == 'T') {
>   			opt_T = true;
> -			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE]);
> +			conf_ports(name, optarg, c->fwd[PIF_SPLICE]);
>   		} else if (name == 'U') {
>   			opt_U = true;
> -			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE]);
> +			conf_ports(name, optarg, c->fwd[PIF_SPLICE]);
>   		}
>   	} while (name != -1);
>   
> @@ -2246,13 +2242,13 @@ void conf(struct ctx *c, int argc, char **argv)
>   
>   	if (c->mode == MODE_PASTA) {
>   		if (!opt_t)
> -			conf_ports(c, 't', "auto", c->fwd[PIF_HOST]);
> +			conf_ports('t', "auto", c->fwd[PIF_HOST]);
>   		if (!opt_T)
> -			conf_ports(c, 'T', "auto", c->fwd[PIF_SPLICE]);
> +			conf_ports('T', "auto", c->fwd[PIF_SPLICE]);
>   		if (!opt_u)
> -			conf_ports(c, 'u', "auto", c->fwd[PIF_HOST]);
> +			conf_ports('u', "auto", c->fwd[PIF_HOST]);
>   		if (!opt_U)
> -			conf_ports(c, 'U', "auto", c->fwd[PIF_SPLICE]);
> +			conf_ports('U', "auto", c->fwd[PIF_SPLICE]);
>   	}
>   
>   	if (!c->quiet)
> diff --git a/fwd.c b/fwd.c
> index 3e87169b..c7fd1a9d 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -326,6 +326,23 @@ static struct fwd_table fwd_out;
>    */
>   void fwd_rule_init(struct ctx *c)
>   {
> +	uint32_t caps = 0;
> +
> +	if (c->ifi4)
> +		caps |= FWD_CAP_IPV4;
> +	if (c->ifi6)
> +		caps |= FWD_CAP_IPV6;
> +	if (!c->no_tcp)
> +		caps |= FWD_CAP_TCP;
> +	if (!c->no_udp)
> +		caps |= FWD_CAP_UDP;
> +	if (c->mode == MODE_PASTA)
> +		caps |= FWD_CAP_SCAN;
> +	if (!c->no_bindtodevice)
> +		caps |= FWD_CAP_IFNAME;
> +
> +	fwd_in.caps = fwd_out.caps = caps;
> +
>   	c->fwd[PIF_HOST] = &fwd_in;
>   	if (c->mode == MODE_PASTA)
>   		c->fwd[PIF_SPLICE] = &fwd_out;
> diff --git a/fwd.h b/fwd.h
> index 43bfeadb..3e365d35 100644
> --- a/fwd.h
> +++ b/fwd.h
> @@ -52,6 +52,7 @@ struct fwd_listen_ref {
>   
>   /**
>    * struct fwd_table - Forwarding state (per initiating pif)
> + * @caps:	Forwarding capabilities for this initiating pif
>    * @count:	Number of forwarding rules
>    * @rules:	Array of forwarding rules
>    * @rulesocks:	Parallel array of @rules (@count valid entries) of pointers to
> @@ -61,6 +62,7 @@ struct fwd_listen_ref {
>    * @socks:	Listening sockets for forwarding
>    */
>   struct fwd_table {
> +	uint32_t caps;
>   	unsigned count;
>   	struct fwd_rule rules[MAX_FWD_RULES];
>   	int *rulesocks[MAX_FWD_RULES];
> diff --git a/fwd_rule.h b/fwd_rule.h
> index 8506a0c4..edba6782 100644
> --- a/fwd_rule.h
> +++ b/fwd_rule.h
> @@ -17,6 +17,14 @@
>   #include "inany.h"
>   #include "bitmap.h"
>   
> +/* Forwarding capability bits */
> +#define FWD_CAP_IPV4		BIT(0)
> +#define FWD_CAP_IPV6		BIT(1)
> +#define FWD_CAP_TCP		BIT(2)
> +#define FWD_CAP_UDP		BIT(3)
> +#define FWD_CAP_SCAN		BIT(4)
> +#define FWD_CAP_IFNAME		BIT(5)
> +
>   /**
>    * struct fwd_rule - Forwarding rule governing a range of ports
>    * @addr:	Address to forward from