[PATCH 00/18] Rework forwarding option parsing

[PATCH 00/18] Rework forwarding option parsing

[David Gibson]

This series makes a number of significant reworks to how we process
forwarding options (-t, -u, -T and -U) in passt & pasta.  This is
largely motivated by moving towards being able to share this code with
a configuration update tool.  However, along the way it also enables
some forwarding configurations that were technically possible with the
forwarding table but couldn't be specified on the command line, in
particular bug 180.

There is still a bunch of work needed to make the parsing code truly
shareable with pesto, but this is a solid start.

David Gibson (18):
  conf: Split parsing of port specifiers from the rest of -[tuTU]
    parsing
  conf: Simplify handling of default forwarding mode
  conf: Move first pass handling of -[TU] next to handling of -[tu]
  doc: Consolidate -[tu] option descriptions for passt and pasta
  conf: Permit -[tTuU] all in pasta mode
  fwd: Better split forwarding rule specification from associated
    sockets
  fwd_rule: Move forwarding rule formatting
  conf: Pass protocol explicitly to conf_ports_range_except()
  fwd: Split rule building from rule adding
  fwd_rule: Move rule conflict checking from fwd_rule_add() to caller
  fwd: Improve error handling in fwd_rule_add()
  conf: Don't be strict about exclusivity of forwarding mode
  conf: Rework stepping through chunks of port specifiers
  conf: Rework checking for garbage after a range
  conf: Move "all" handling to port specifier
  conf: Allow user-specified auto-scanned port forwarding ranges
  conf: Move SO_BINDTODEVICE workaround to conf_ports()
  conf: Don't pass raw commandline argument to conf_ports_spec()

 Makefile   |  10 +-
 conf.c     | 541 ++++++++++++++++++++++++++---------------------------
 fwd.c      | 190 ++++++-------------
 fwd.h      |  21 +--
 fwd_rule.c | 132 +++++++++++++
 fwd_rule.h |  15 ++
 passt.1    | 214 +++++++--------------
 7 files changed, 542 insertions(+), 581 deletions(-)
 create mode 100644 fwd_rule.c

-- 
2.53.0

[PATCH 01/18] conf: Split parsing of port specifiers from the rest of -[tuTU] parsing

[David Gibson]

conf_ports() is extremely long, but we want to refactor it so that parts
can be shared with the upcoming configuration client.  Make a small start
by separating out the section that parses just the port specification
(not including address and/or interface).

This also allows us to constify a few extra things, and while we're there
replace a few vague error messages with more specific ones.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 203 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 116 insertions(+), 87 deletions(-)

diff --git a/conf.c b/conf.c
index ae37bf96..c515480b 100644
--- a/conf.c
+++ b/conf.c
@@ -74,7 +74,7 @@ const char *pasta_default_ifn = "tap0";
  *	   character *after* the delimiter, if no further @c is in @s,
  *	   return NULL
  */
-static char *next_chunk(const char *s, char c)
+static const char *next_chunk(const char *s, char c)
 {
 	char *sep = strchr(s, c);
 	return sep ? sep + 1 : NULL;
@@ -101,18 +101,19 @@ struct port_range {
  * Return: -EINVAL on parsing error, -ERANGE on out of range port
  *	   numbers, 0 on success
  */
-static int parse_port_range(const char *s, char **endptr,
+static int parse_port_range(const char *s, const char **endptr,
 			    struct port_range *range)
 {
 	unsigned long first, last;
+	char *ep;
 
-	last = first = strtoul(s, endptr, 10);
-	if (*endptr == s) /* Parsed nothing */
+	last = first = strtoul(s, &ep, 10);
+	if (ep == s) /* Parsed nothing */
 		return -EINVAL;
-	if (**endptr == '-') { /* we have a last value too */
-		const char *lasts = *endptr + 1;
-		last = strtoul(lasts, endptr, 10);
-		if (*endptr == lasts) /* Parsed nothing */
+	if (*ep == '-') { /* we have a last value too */
+		const char *lasts = ep + 1;
+		last = strtoul(lasts, &ep, 10);
+		if (ep == lasts) /* Parsed nothing */
 			return -EINVAL;
 	}
 
@@ -121,6 +122,7 @@ static int parse_port_range(const char *s, char **endptr,
 
 	range->first = first;
 	range->last = last;
+	*endptr = ep;
 
 	return 0;
 }
@@ -213,6 +215,101 @@ enum fwd_mode {
 	FWD_MODE_ALL,
 };
 
+/**
+ * conf_ports_spec() - Parse port range(s) specifier
+ * @c:		Execution context
+ * @optname:	Short option name, t, T, u, or U
+ * @optarg:	Option argument (port specification)
+ * @fwd:	Forwarding table to be updated
+ * @addr:	Listening address for forwarding
+ * @ifname:	Interface name for listening
+ * @spec:	Port range(s) specifier
+ */
+static void conf_ports_spec(const struct ctx *c,
+			    char optname, const char *optarg,
+			    struct fwd_table *fwd,
+			    const union inany_addr *addr, const char *ifname,
+			    const char *spec)
+{
+	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
+	bool exclude_only = true;
+	const char *p;
+	unsigned i;
+
+	/* Mark all exclusions first, they might be given after base ranges */
+	p = spec;
+	do {
+		struct port_range xrange;
+
+		if (*p != '~') {
+			/* Not an exclude range, parse later */
+			exclude_only = false;
+			continue;
+		}
+		p++;
+
+		if (parse_port_range(p, &p, &xrange))
+			goto bad;
+		if ((*p != '\0')  && (*p != ',')) /* Garbage after the range */
+			goto bad;
+
+		for (i = xrange.first; i <= xrange.last; i++)
+			bitmap_set(exclude, i);
+	} while ((p = next_chunk(p, ',')));
+
+	if (exclude_only) {
+		/* Exclude ephemeral ports */
+		fwd_port_map_ephemeral(exclude);
+
+		conf_ports_range_except(c, optname, optarg, fwd,
+					addr, ifname,
+					1, NUM_PORTS - 1, exclude,
+					1, FWD_WEAK);
+		return;
+	}
+
+	/* Now process base ranges, skipping exclusions */
+	p = spec;
+	do {
+		struct port_range orig_range, mapped_range;
+
+		if (*p == '~')
+			/* Exclude range, already parsed */
+			continue;
+
+		if (parse_port_range(p, &p, &orig_range))
+			goto bad;
+
+		if (*p == ':') { /* There's a range to map to as well */
+			if (parse_port_range(p + 1, &p, &mapped_range))
+				goto bad;
+			if ((mapped_range.last - mapped_range.first) !=
+			    (orig_range.last - orig_range.first))
+				goto bad;
+		} else {
+			mapped_range = orig_range;
+		}
+
+		if ((*p != '\0')  && (*p != ',')) /* Garbage after the ranges */
+			goto bad;
+
+		if (orig_range.first == 0) {
+			die("Can't forward port 0 for option '-%c %s'",
+			    optname, optarg);
+		}
+
+		conf_ports_range_except(c, optname, optarg, fwd,
+					addr, ifname,
+					orig_range.first, orig_range.last,
+					exclude,
+					mapped_range.first, 0);
+	} while ((p = next_chunk(p, ',')));
+
+	return;
+bad:
+	die("Invalid port specifier %s", optarg);
+}
+
 /**
  * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets
  * @c:		Execution context
@@ -226,9 +323,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 {
 	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
 	char buf[BUFSIZ], *spec, *ifname = NULL, *p;
-	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
-	bool exclude_only = true;
-	unsigned i;
 
 	if (!strcmp(optarg, "none")) {
 		if (*mode)
@@ -255,6 +349,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 	}
 
 	if (!strcmp(optarg, "all")) {
+		uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
+
 		if (*mode)
 			goto mode_conflict;
 
@@ -285,7 +381,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		spec++;
 
 		if (optname != 't' && optname != 'u')
-			goto bad;
+			die("Listening address not allowed for -%c %s",
+			    optname, optarg);
 
 		if ((ifname = strchr(buf, '%'))) {
 			*ifname = 0;
@@ -295,9 +392,10 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 			 * so the length of the given ifname is:
 			 * (spec - ifname - 1)
 			 */
-			if (spec - ifname - 1 >= IFNAMSIZ)
-				goto bad;
-
+			if (spec - ifname - 1 >= IFNAMSIZ) {
+				die("Interface name '%s' is too long (max %u)",
+				    ifname, IFNAMSIZ - 1);
+			}
 		}
 
 		if (ifname == buf + 1) {	/* Interface without address */
@@ -312,7 +410,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 			}
 
 			if (!inany_pton(p, addr))
-				goto bad;
+				die("Bad forwarding address '%s'", p);
 		}
 	} else {
 		spec = buf;
@@ -330,27 +428,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		}
 	}
 
-	/* Mark all exclusions first, they might be given after base ranges */
-	p = spec;
-	do {
-		struct port_range xrange;
-
-		if (*p != '~') {
-			/* Not an exclude range, parse later */
-			exclude_only = false;
-			continue;
-		}
-		p++;
-
-		if (parse_port_range(p, &p, &xrange))
-			goto bad;
-		if ((*p != '\0')  && (*p != ',')) /* Garbage after the range */
-			goto bad;
-
-		for (i = xrange.first; i <= xrange.last; i++)
-			bitmap_set(exclude, i);
-	} while ((p = next_chunk(p, ',')));
-
 	if (ifname && c->no_bindtodevice) {
 		die(
 "Device binding for '-%c %s' unsupported (requires kernel 5.7+)",
@@ -360,57 +437,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 	if ((optname == 'T' || optname == 'U') && !ifname)
 		ifname = "lo";
 
-	if (exclude_only) {
-		/* Exclude ephemeral ports */
-		fwd_port_map_ephemeral(exclude);
-
-		conf_ports_range_except(c, optname, optarg, fwd,
-					addr, ifname,
-					1, NUM_PORTS - 1, exclude,
-					1, FWD_WEAK);
-		return;
-	}
-
-	/* Now process base ranges, skipping exclusions */
-	p = spec;
-	do {
-		struct port_range orig_range, mapped_range;
-
-		if (*p == '~')
-			/* Exclude range, already parsed */
-			continue;
-
-		if (parse_port_range(p, &p, &orig_range))
-			goto bad;
-
-		if (*p == ':') { /* There's a range to map to as well */
-			if (parse_port_range(p + 1, &p, &mapped_range))
-				goto bad;
-			if ((mapped_range.last - mapped_range.first) !=
-			    (orig_range.last - orig_range.first))
-				goto bad;
-		} else {
-			mapped_range = orig_range;
-		}
-
-		if ((*p != '\0')  && (*p != ',')) /* Garbage after the ranges */
-			goto bad;
-
-		if (orig_range.first == 0) {
-			die("Can't forward port 0 for option '-%c %s'",
-			    optname, optarg);
-		}
-
-		conf_ports_range_except(c, optname, optarg, fwd,
-					addr, ifname,
-					orig_range.first, orig_range.last,
-					exclude,
-					mapped_range.first, 0);
-	} while ((p = next_chunk(p, ',')));
-
+	conf_ports_spec(c, optname, optarg, fwd, addr, ifname, spec);
 	return;
-bad:
-	die("Invalid port specifier %s", optarg);
+
 mode_conflict:
 	die("Port forwarding mode '%s' conflicts with previous mode", optarg);
 }
-- 
2.53.0

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

[David Gibson]

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;
 	}
 
@@ -1576,7 +1580,6 @@ void conf(struct ctx *c, int argc, char **argv)
 	enum fwd_mode udp_out_mode = FWD_MODE_UNSET;
 	enum fwd_mode tcp_in_mode = FWD_MODE_UNSET;
 	enum fwd_mode udp_in_mode = FWD_MODE_UNSET;
-	enum fwd_mode fwd_default = FWD_MODE_NONE;
 	bool v4_only = false, v6_only = false;
 	unsigned dns4_idx = 0, dns6_idx = 0;
 	unsigned long max_mtu = IP_MAX_MTU;
@@ -1593,10 +1596,8 @@ void conf(struct ctx *c, int argc, char **argv)
 	gid_t gid;
 	
 
-	if (c->mode == MODE_PASTA) {
+	if (c->mode == MODE_PASTA)
 		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
-		fwd_default = FWD_MODE_AUTO;
-	}
 
 	if (tap_l2_max_len(c) - ETH_HLEN < max_mtu)
 		max_mtu = tap_l2_max_len(c) - ETH_HLEN;
@@ -2244,34 +2245,23 @@ void conf(struct ctx *c, int argc, char **argv)
 			if_indextoname(c->ifi6, c->pasta_ifn);
 	}
 
-	if (!tcp_in_mode)
-		tcp_in_mode = fwd_default;
-	if (!tcp_out_mode)
-		tcp_out_mode = fwd_default;
-	if (!udp_in_mode)
-		udp_in_mode = fwd_default;
-	if (!udp_out_mode)
-		udp_out_mode = fwd_default;
-
-	if (tcp_in_mode == FWD_MODE_AUTO) {
-		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[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[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[PIF_SPLICE],
-					NULL, "lo", 1, NUM_PORTS - 1, NULL, 1,
-					FWD_SCAN);
+	if (c->mode == MODE_PASTA) {
+		if (!tcp_in_mode) {
+			conf_ports(c, 't', "auto",
+				   c->fwd[PIF_HOST], &tcp_in_mode);
+		}
+		if (!tcp_out_mode) {
+			conf_ports(c, 'T', "auto",
+				   c->fwd[PIF_SPLICE], &tcp_out_mode);
+		}
+		if (!udp_in_mode) {
+			conf_ports(c, 'u', "auto",
+				   c->fwd[PIF_HOST], &udp_in_mode);
+		}
+		if (!udp_out_mode) {
+			conf_ports(c, 'U', "auto",
+				   c->fwd[PIF_SPLICE], &udp_out_mode);
+		}
 	}
 
 	if (!c->quiet)
-- 
2.53.0

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

[Stefano Brivio]

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.

-- 
Stefano

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

[David Gibson]

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

[PATCH 03/18] conf: Move first pass handling of -[TU] next to handling of -[tu]

[David Gibson]

The forwarding options -[tTuU] can't be fully handled in our first pass
over the command line options, they need to wait until the second, once
we know more about the interface configuration.  However, we do need stub
handling, so they don't cause an error.  For historical reasons the
-[TU] options are handled a fair way apart from the -[tu] options.  Move
them next to each other for clarity.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/conf.c b/conf.c
index 7d718f91..f3b36bb6 100644
--- a/conf.c
+++ b/conf.c
@@ -2024,6 +2024,12 @@ void conf(struct ctx *c, int argc, char **argv)
 
 			c->one_off = true;
 			break;
+		case 'T':
+		case 'U':
+			if (c->mode != MODE_PASTA)
+				die("-%c is for pasta mode only", name);
+
+			/* fall through */
 		case 't':
 		case 'u':
 			/* Handle these later, once addresses are configured */
@@ -2064,13 +2070,6 @@ void conf(struct ctx *c, int argc, char **argv)
 			die("Cannot use DNS address %s", optarg);
 		}
 			break;
-		case 'T':
-		case 'U':
-			if (c->mode != MODE_PASTA)
-				die("-%c is for pasta mode only", name);
-
-			/* Handle properly later, once addresses are configured */
-			break;
 		case 'h':
 			usage(argv[0], stdout, EXIT_SUCCESS);
 			break;
-- 
2.53.0

[PATCH 04/18] doc: Consolidate -[tu] option descriptions for passt and pasta

[David Gibson]

The man page currently has two fairly large, near-identical sections
separately describing the -t and -u options for passt and pasta.  This is
bulky and potentially confusing.  It will make this information more
tedious to update as we alter what's possible here with the forwarding
table.  Consolidate both descriptions to a single one in the common
options, noting the few passt/pasta difference inline.

There's similar duplication usage(), consolidate that as well.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c  |  54 +++++---------
 passt.1 | 216 ++++++++++++++++++--------------------------------------
 2 files changed, 85 insertions(+), 185 deletions(-)

diff --git a/conf.c b/conf.c
index f3b36bb6..751e500f 100644
--- a/conf.c
+++ b/conf.c
@@ -1023,18 +1023,12 @@ static void usage(const char *name, FILE *f, int status)
 		"  --freebind		Bind to any address for forwarding\n"
 		"  --no-map-gw		Don't map gateway address to host\n"
 		"  -4, --ipv4-only	Enable IPv4 operation only\n"
-		"  -6, --ipv6-only	Enable IPv6 operation only\n");
-
-	if (strstr(name, "pasta"))
-		goto pasta_opts;
-
-	FPRINTF(f,
-		"  -1, --one-off	Quit after handling one single client\n"
+		"  -6, --ipv6-only	Enable IPv6 operation only\n"
 		"  -t, --tcp-ports SPEC	TCP port forwarding to guest\n"
 		"    can be specified multiple times\n"
 		"    SPEC can be:\n"
 		"      'none': don't forward any ports\n"
-		"      'all': forward all unbound, non-ephemeral ports\n"
+		"%s"
 		"      a comma-separated list, optionally ranged with '-'\n"
 		"        and optional target ports after ':', with optional\n"
 		"        address specification suffixed by '/' and optional\n"
@@ -1050,43 +1044,29 @@ static void usage(const char *name, FILE *f, int status)
 		"        -t 192.0.2.1/5	Bind port 5 of 192.0.2.1 to guest\n"
 		"        -t 5-25,~10-20	Forward ports 5 to 9, and 21 to 25\n"
 		"        -t ~25		Forward all ports except for 25\n"
-		"    default: none\n"
+		"    default: %s\n"
 		"  -u, --udp-ports SPEC	UDP port forwarding to guest\n"
 		"    SPEC is as described for TCP above\n"
-		"    default: none\n");
+		"    default: %s\n",
+		strstr(name, "pasta") ?
+		"      'auto': forward all ports currently bound in namespace\n"
+		:
+		"      'all': forward all unbound, non-ephemeral ports\n",
+		strstr(name, "pasta") ? "auto" : "none",
+		strstr(name, "pasta") ? "auto" : "none");
+
+	if (strstr(name, "pasta"))
+		goto pasta_opts;
+
+	FPRINTF(f,
+		"  -1, --one-off	Quit after handling one single client\n"
+		);
 
 	passt_exit(status);
 
 pasta_opts:
 
 	FPRINTF(f,
-		"  -t, --tcp-ports SPEC	TCP port forwarding to namespace\n"
-		"    can be specified multiple times\n"
-		"    SPEC can be:\n"
-		"      'none': don't forward any ports\n"
-		"      'auto': forward all ports currently bound in namespace\n"
-		"      a comma-separated list, optionally ranged with '-'\n"
-		"        and optional target ports after ':', with optional\n"
-		"        address specification suffixed by '/' and optional\n"
-		"        interface prefixed by '%%'. Examples:\n"
-		"        -t 22	Forward local port 22 to port 22 in netns\n"
-		"        -t 22:23	Forward local port 22 to port 23\n"
-		"        -t 22,25	Forward ports 22, 25 to ports 22, 25\n"
-		"        -t 22-80	Forward ports 22 to 80\n"
-		"        -t 22-80:32-90	Forward ports 22 to 80 to\n"
-		"			corresponding port numbers plus 10\n"
-		"        -t 192.0.2.1/5	Bind port 5 of 192.0.2.1 to namespace\n"
-		"        -t 5-25,~10-20	Forward ports 5 to 9, and 21 to 25\n"
-		"        -t ~25		Forward all bound ports except for 25\n"
-		"    default: auto\n"
-		"    IPv6 bound ports are also forwarded for IPv4\n"
-		"  -u, --udp-ports SPEC	UDP port forwarding to namespace\n"
-		"    SPEC is as described for TCP above\n"
-		"    default: auto\n"
-		"    IPv6 bound ports are also forwarded for IPv4\n"
-		"    unless specified, with '-t auto', UDP ports with numbers\n"
-		"    corresponding to forwarded TCP port numbers are\n"
-		"    forwarded too\n"
 		"  -T, --tcp-ns SPEC	TCP port forwarding to init namespace\n"
 		"    SPEC is as described above\n"
 		"    default: auto\n"
diff --git a/passt.1 b/passt.1
index 13e8df9d..a9a8a42a 100644
--- a/passt.1
+++ b/passt.1
@@ -425,78 +425,6 @@ Send \fIname\fR as DHCP option 12 (hostname).
 FQDN to configure the client with.
 Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39.
 
-.SS \fBpasst\fR-only options
-
-.TP
-.BR \-s ", " \-\-socket-path ", " \-\-socket " " \fIpath
-Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to
-\fBpasst\fR.
-Default is to probe a free socket, not accepting connections, starting from
-\fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR.
-
-.TP
-.BR \-\-vhost-user
-Enable vhost-user. The vhost-user command socket is provided by \fB--socket\fR.
-
-.TP
-.BR \-\-print-capabilities
-Print back-end capabilities in JSON format, only meaningful for vhost-user mode.
-
-.TP
-.BR \-\-repair-path " " \fIpath
-Path for UNIX domain socket used by the \fBpasst-repair\fR(1) helper to connect
-to \fBpasst\fR in order to set or clear the TCP_REPAIR option on sockets, during
-migration. \fB--repair-path none\fR disables this interface (if you need to
-specify a socket path called "none" you can prefix the path by \fI./\fR).
-
-Default, for \-\-vhost-user mode only, is to append \fI.repair\fR to the path
-chosen for the hypervisor UNIX domain socket. No socket is created if not in
-\-\-vhost-user mode.
-
-.TP
-.BR \-\-migrate-exit " " (DEPRECATED)
-Exit after a completed migration as source. By default, \fBpasst\fR keeps
-running and the migrated guest can continue using its connection, or a new guest
-can connect.
-
-Note that this configuration option is \fBdeprecated\fR and will be removed in a
-future version. It is not expected to be of any use, and it simply reflects a
-legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
-below.
-
-.TP
-.BR \-\-migrate-no-linger " " (DEPRECATED)
-Close TCP sockets on the source instance once migration completes.
-
-By default, sockets are kept open, and events on data sockets are ignored, so
-that any further message reaching sockets after the source migrated is silently
-ignored, to avoid connection resets in case data is received after migration.
-
-Note that this configuration option is \fBdeprecated\fR and will be removed in a
-future version. It is not expected to be of any use, and it simply reflects a
-legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
-below.
-
-.TP
-.BR \-F ", " \-\-fd " " \fIFD
-Pass a pre-opened, connected socket to \fBpasst\fR. Usually the socket is opened
-in the parent process and \fBpasst\fR inherits it when run as a child. This
-allows the parent process to open sockets using another address family or
-requiring special privileges.
-
-This option implies the behaviour described for \-\-one-off, once this socket
-is closed.
-
-.TP
-.BR \-1 ", " \-\-one-off
-Quit after handling a single client connection, that is, once the client closes
-the socket, or once we get a socket error.
-
-\fBNote\fR: this option has no effect after \fBpasst\fR completes a migration as
-source, because, in that case, exiting would close sockets for active
-connections, which would in turn cause connection resets if any further data is
-received. See also the description of \fI\-\-migrate-no-linger\fR.
-
 .TP
 .BR \-t ", " \-\-tcp-ports " " \fIspec
 Configure TCP port forwarding to guest. \fIspec\fR can be one of:
@@ -507,11 +435,17 @@ Configure TCP port forwarding to guest. \fIspec\fR can be one of:
 Don't forward any ports
 
 .TP
-.BR all
+.BR all (\fBpasst\fR only)
 Forward all unbound, non-ephemeral ports, as permitted by current capabilities.
 For low (< 1024) ports, see \fBNOTES\fR. No failures are reported for
 unavailable ports, unless no ports could be forwarded at all.
 
+.TP
+.BR auto (\fBpasta\fR only)
+Dynamically forward ports bound in the namespace. The list of ports is
+periodically derived (every second) from listening sockets reported by
+\fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5).
+
 .TP
 .BR ports
 A comma-separated list of ports, optionally ranged with \fI-\fR, and,
@@ -563,7 +497,7 @@ and 30
 Forward all ports to the guest, except for the range from 20000 to 20010
 .RE
 
-Default is \fBnone\fR.
+Default is \fBnone\fR for \fBpasst\fR and \fBauto\fR for \fBpasta\fR.
 .RE
 
 .TP
@@ -575,101 +509,87 @@ Note: unless overridden, UDP ports with numbers corresponding to forwarded TCP
 port numbers are forwarded too, without, however, any port translation. IPv6
 bound ports are also forwarded for IPv4.
 
-Default is \fBnone\fR.
+Default is \fBnone\fR for \fBpasst\fR and \fBauto\fR for \fBpasta\fR.
 
-.SS \fBpasta\fR-only options
+.SS \fBpasst\fR-only options
 
 .TP
-.BR \-I ", " \-\-ns-ifname " " \fIname
-Name of tap interface to be created in target namespace.
-By default, the same interface name as the external, routable interface is used.
-If no such interface exists, the name \fItap0\fR will be used instead.
+.BR \-s ", " \-\-socket-path ", " \-\-socket " " \fIpath
+Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to
+\fBpasst\fR.
+Default is to probe a free socket, not accepting connections, starting from
+\fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR.
 
 .TP
-.BR \-t ", " \-\-tcp-ports " " \fIspec
-Configure TCP port forwarding to namespace. \fIspec\fR can be one of:
-.RS
+.BR \-\-vhost-user
+Enable vhost-user. The vhost-user command socket is provided by \fB--socket\fR.
 
 .TP
-.BR none
-Don't forward any ports
+.BR \-\-print-capabilities
+Print back-end capabilities in JSON format, only meaningful for vhost-user mode.
 
 .TP
-.BR auto
-Dynamically forward ports bound in the namespace. The list of ports is
-periodically derived (every second) from listening sockets reported by
-\fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5).
+.BR \-\-repair-path " " \fIpath
+Path for UNIX domain socket used by the \fBpasst-repair\fR(1) helper to connect
+to \fBpasst\fR in order to set or clear the TCP_REPAIR option on sockets, during
+migration. \fB--repair-path none\fR disables this interface (if you need to
+specify a socket path called "none" you can prefix the path by \fI./\fR).
+
+Default, for \-\-vhost-user mode only, is to append \fI.repair\fR to the path
+chosen for the hypervisor UNIX domain socket. No socket is created if not in
+\-\-vhost-user mode.
 
 .TP
-.BR ports
-A comma-separated list of ports, optionally ranged with \fI-\fR, and,
-optionally, with target ports after \fI:\fR, if they differ. Specific addresses
-can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited
-to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports
-and ranges can be excluded by an additional specification prefixed by \fI~\fR.
+.BR \-\-migrate-exit " " (DEPRECATED)
+Exit after a completed migration as source. By default, \fBpasst\fR keeps
+running and the migrated guest can continue using its connection, or a new guest
+can connect.
 
-Specifying excluded ranges only implies that all other ports are forwarded. In
-this case, no failures are reported for unavailable ports, unless no ports could
-be forwarded at all.
+Note that this configuration option is \fBdeprecated\fR and will be removed in a
+future version. It is not expected to be of any use, and it simply reflects a
+legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
+below.
 
-Examples:
-.RS
-.TP
--t 22
-Forward local port 22 to 22 in the target namespace
-.TP
--t 22:23
-Forward local port 22 to port 23 in the target namespace
-.TP
--t 22,25
-Forward local ports 22 and 25 to ports 22 and 25 in the target namespace
-.TP
--t 22-80
-Forward local ports between 22 and 80 to corresponding ports in the target
-namespace
-.TP
--t 22-80:32-90
-Forward local ports between 22 and 80 to ports between 32 and 90 in the target
-namespace
-.TP
--t 192.0.2.1/22
-Forward local port 22, bound to 192.0.2.1, to port 22 in the target namespace
-.TP
--t 192.0.2.1%eth0/22
-Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22
-.TP
--t %eth0/22
-Forward local port 22, bound to any address on interface eth0, to port 22
-.TP
--t 2000-5000,~3000-3010
-Forward local ports between 2000 and 5000, except for those between 3000 and
-3010
-.TP
--t 192.0.2.1/20-30,~25
-For the local address 192.0.2.1, forward ports between 20 and 24 and between 26
-and 30
 .TP
--t ~20000-20010
-Forward all ports to the namespace, except for those between 20000 and 20010
-.RE
+.BR \-\-migrate-no-linger " " (DEPRECATED)
+Close TCP sockets on the source instance once migration completes.
 
-IPv6 bound ports are also forwarded for IPv4.
+By default, sockets are kept open, and events on data sockets are ignored, so
+that any further message reaching sockets after the source migrated is silently
+ignored, to avoid connection resets in case data is received after migration.
 
-Default is \fBauto\fR.
-.RE
+Note that this configuration option is \fBdeprecated\fR and will be removed in a
+future version. It is not expected to be of any use, and it simply reflects a
+legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
+below.
 
 .TP
-.BR \-u ", " \-\-udp-ports " " \fIspec
-Configure UDP port forwarding to namespace. \fIspec\fR is as described for TCP
-above, and the list of ports is derived from listening sockets reported by
-\fI/proc/net/udp\fR and \fI/proc/net/udp6\fR, see \fBproc\fR(5).
+.BR \-F ", " \-\-fd " " \fIFD
+Pass a pre-opened, connected socket to \fBpasst\fR. Usually the socket is opened
+in the parent process and \fBpasst\fR inherits it when run as a child. This
+allows the parent process to open sockets using another address family or
+requiring special privileges.
 
-Note: unless overridden, UDP ports with numbers corresponding to forwarded TCP
-port numbers are forwarded too, without, however, any port translation. 
+This option implies the behaviour described for \-\-one-off, once this socket
+is closed.
 
-IPv6 bound ports are also forwarded for IPv4.
+.TP
+.BR \-1 ", " \-\-one-off
+Quit after handling a single client connection, that is, once the client closes
+the socket, or once we get a socket error.
 
-Default is \fBauto\fR.
+\fBNote\fR: this option has no effect after \fBpasst\fR completes a migration as
+source, because, in that case, exiting would close sockets for active
+connections, which would in turn cause connection resets if any further data is
+received. See also the description of \fI\-\-migrate-no-linger\fR.
+
+.SS \fBpasta\fR-only options
+
+.TP
+.BR \-I ", " \-\-ns-ifname " " \fIname
+Name of tap interface to be created in target namespace.
+By default, the same interface name as the external, routable interface is used.
+If no such interface exists, the name \fItap0\fR will be used instead.
 
 .TP
 .BR \-T ", " \-\-tcp-ns " " \fIspec
-- 
2.53.0

Re: [PATCH 04/18] doc: Consolidate -[tu] option descriptions for passt and pasta

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:16 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The man page currently has two fairly large, near-identical sections

Not entirely identical also because:

> separately describing the -t and -u options for passt and pasta.  This is
> bulky and potentially confusing.  It will make this information more
> tedious to update as we alter what's possible here with the forwarding
> table.  Consolidate both descriptions to a single one in the common
> options, noting the few passt/pasta difference inline.
> 
> There's similar duplication usage(), consolidate that as well.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  conf.c  |  54 +++++---------
>  passt.1 | 216 ++++++++++++++++++--------------------------------------
>  2 files changed, 85 insertions(+), 185 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index f3b36bb6..751e500f 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1023,18 +1023,12 @@ static void usage(const char *name, FILE *f, int status)
>  		"  --freebind		Bind to any address for forwarding\n"
>  		"  --no-map-gw		Don't map gateway address to host\n"
>  		"  -4, --ipv4-only	Enable IPv4 operation only\n"
> -		"  -6, --ipv6-only	Enable IPv6 operation only\n");
> -
> -	if (strstr(name, "pasta"))
> -		goto pasta_opts;
> -
> -	FPRINTF(f,
> -		"  -1, --one-off	Quit after handling one single client\n"
> +		"  -6, --ipv6-only	Enable IPv6 operation only\n"
>  		"  -t, --tcp-ports SPEC	TCP port forwarding to guest\n"
>  		"    can be specified multiple times\n"
>  		"    SPEC can be:\n"
>  		"      'none': don't forward any ports\n"
> -		"      'all': forward all unbound, non-ephemeral ports\n"
> +		"%s"
>  		"      a comma-separated list, optionally ranged with '-'\n"
>  		"        and optional target ports after ':', with optional\n"
>  		"        address specification suffixed by '/' and optional\n"
> @@ -1050,43 +1044,29 @@ static void usage(const char *name, FILE *f, int status)
>  		"        -t 192.0.2.1/5	Bind port 5 of 192.0.2.1 to guest\n"
>  		"        -t 5-25,~10-20	Forward ports 5 to 9, and 21 to 25\n"
>  		"        -t ~25		Forward all ports except for 25\n"
> -		"    default: none\n"
> +		"    default: %s\n"
>  		"  -u, --udp-ports SPEC	UDP port forwarding to guest\n"

...this is "guest" for passt and "namespace" for pasta. Now, I know
that you use those terms interchangeably and the code does as well, but
this is directly visible to users so I think we shouldn't confuse them
with "guests" for... containers.

To reaffirm the difference between terms used in the code and terms
used in documentation, could we perhaps do something like:

	char *guest = (mode == MODE_PASTA) ? "namespace" : "guest";

? And then use that later? It might be worth doing that for the
"default" strings too.

>  		"    SPEC is as described for TCP above\n"
> -		"    default: none\n");
> +		"    default: %s\n",
> +		strstr(name, "pasta") ?
> +		"      'auto': forward all ports currently bound in namespace\n"
> +		:
> +		"      'all': forward all unbound, non-ephemeral ports\n",
> +		strstr(name, "pasta") ? "auto" : "none",
> +		strstr(name, "pasta") ? "auto" : "none");
> +
> +	if (strstr(name, "pasta"))
> +		goto pasta_opts;
> +
> +	FPRINTF(f,
> +		"  -1, --one-off	Quit after handling one single client\n"
> +		);
>  
>  	passt_exit(status);
>  
>  pasta_opts:
>  
>  	FPRINTF(f,
> -		"  -t, --tcp-ports SPEC	TCP port forwarding to namespace\n"
> -		"    can be specified multiple times\n"
> -		"    SPEC can be:\n"
> -		"      'none': don't forward any ports\n"
> -		"      'auto': forward all ports currently bound in namespace\n"
> -		"      a comma-separated list, optionally ranged with '-'\n"
> -		"        and optional target ports after ':', with optional\n"
> -		"        address specification suffixed by '/' and optional\n"
> -		"        interface prefixed by '%%'. Examples:\n"
> -		"        -t 22	Forward local port 22 to port 22 in netns\n"
> -		"        -t 22:23	Forward local port 22 to port 23\n"
> -		"        -t 22,25	Forward ports 22, 25 to ports 22, 25\n"
> -		"        -t 22-80	Forward ports 22 to 80\n"
> -		"        -t 22-80:32-90	Forward ports 22 to 80 to\n"
> -		"			corresponding port numbers plus 10\n"
> -		"        -t 192.0.2.1/5	Bind port 5 of 192.0.2.1 to namespace\n"
> -		"        -t 5-25,~10-20	Forward ports 5 to 9, and 21 to 25\n"
> -		"        -t ~25		Forward all bound ports except for 25\n"
> -		"    default: auto\n"
> -		"    IPv6 bound ports are also forwarded for IPv4\n"
> -		"  -u, --udp-ports SPEC	UDP port forwarding to namespace\n"
> -		"    SPEC is as described for TCP above\n"
> -		"    default: auto\n"
> -		"    IPv6 bound ports are also forwarded for IPv4\n"
> -		"    unless specified, with '-t auto', UDP ports with numbers\n"
> -		"    corresponding to forwarded TCP port numbers are\n"
> -		"    forwarded too\n"
>  		"  -T, --tcp-ns SPEC	TCP port forwarding to init namespace\n"
>  		"    SPEC is as described above\n"
>  		"    default: auto\n"
> diff --git a/passt.1 b/passt.1
> index 13e8df9d..a9a8a42a 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -425,78 +425,6 @@ Send \fIname\fR as DHCP option 12 (hostname).
>  FQDN to configure the client with.
>  Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39.
>  
> -.SS \fBpasst\fR-only options
> -
> -.TP
> -.BR \-s ", " \-\-socket-path ", " \-\-socket " " \fIpath
> -Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to
> -\fBpasst\fR.
> -Default is to probe a free socket, not accepting connections, starting from
> -\fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR.
> -
> -.TP
> -.BR \-\-vhost-user
> -Enable vhost-user. The vhost-user command socket is provided by \fB--socket\fR.
> -
> -.TP
> -.BR \-\-print-capabilities
> -Print back-end capabilities in JSON format, only meaningful for vhost-user mode.
> -
> -.TP
> -.BR \-\-repair-path " " \fIpath
> -Path for UNIX domain socket used by the \fBpasst-repair\fR(1) helper to connect
> -to \fBpasst\fR in order to set or clear the TCP_REPAIR option on sockets, during
> -migration. \fB--repair-path none\fR disables this interface (if you need to
> -specify a socket path called "none" you can prefix the path by \fI./\fR).
> -
> -Default, for \-\-vhost-user mode only, is to append \fI.repair\fR to the path
> -chosen for the hypervisor UNIX domain socket. No socket is created if not in
> -\-\-vhost-user mode.
> -
> -.TP
> -.BR \-\-migrate-exit " " (DEPRECATED)
> -Exit after a completed migration as source. By default, \fBpasst\fR keeps
> -running and the migrated guest can continue using its connection, or a new guest
> -can connect.
> -
> -Note that this configuration option is \fBdeprecated\fR and will be removed in a
> -future version. It is not expected to be of any use, and it simply reflects a
> -legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
> -below.
> -
> -.TP
> -.BR \-\-migrate-no-linger " " (DEPRECATED)
> -Close TCP sockets on the source instance once migration completes.
> -
> -By default, sockets are kept open, and events on data sockets are ignored, so
> -that any further message reaching sockets after the source migrated is silently
> -ignored, to avoid connection resets in case data is received after migration.
> -
> -Note that this configuration option is \fBdeprecated\fR and will be removed in a
> -future version. It is not expected to be of any use, and it simply reflects a
> -legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
> -below.
> -
> -.TP
> -.BR \-F ", " \-\-fd " " \fIFD
> -Pass a pre-opened, connected socket to \fBpasst\fR. Usually the socket is opened
> -in the parent process and \fBpasst\fR inherits it when run as a child. This
> -allows the parent process to open sockets using another address family or
> -requiring special privileges.
> -
> -This option implies the behaviour described for \-\-one-off, once this socket
> -is closed.
> -
> -.TP
> -.BR \-1 ", " \-\-one-off
> -Quit after handling a single client connection, that is, once the client closes
> -the socket, or once we get a socket error.
> -
> -\fBNote\fR: this option has no effect after \fBpasst\fR completes a migration as
> -source, because, in that case, exiting would close sockets for active
> -connections, which would in turn cause connection resets if any further data is
> -received. See also the description of \fI\-\-migrate-no-linger\fR.
> -
>  .TP
>  .BR \-t ", " \-\-tcp-ports " " \fIspec
>  Configure TCP port forwarding to guest. \fIspec\fR can be one of:
> @@ -507,11 +435,17 @@ Configure TCP port forwarding to guest. \fIspec\fR can be one of:
>  Don't forward any ports
>  
>  .TP
> -.BR all
> +.BR all (\fBpasst\fR only)

This is rendered as "passtonly". You need \fBpasst\fR " " only. While
this one goes away in 5/18,

>  Forward all unbound, non-ephemeral ports, as permitted by current capabilities.
>  For low (< 1024) ports, see \fBNOTES\fR. No failures are reported for
>  unavailable ports, unless no ports could be forwarded at all.
>  
> +.TP
> +.BR auto (\fBpasta\fR only)

...this one doesn't.

> +Dynamically forward ports bound in the namespace. The list of ports is
> +periodically derived (every second) from listening sockets reported by
> +\fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5).
> +
>  .TP
>  .BR ports
>  A comma-separated list of ports, optionally ranged with \fI-\fR, and,
> @@ -563,7 +497,7 @@ and 30
>  Forward all ports to the guest, except for the range from 20000 to 20010
>  .RE
>  
> -Default is \fBnone\fR.
> +Default is \fBnone\fR for \fBpasst\fR and \fBauto\fR for \fBpasta\fR.
>  .RE
>  
>  .TP
> @@ -575,101 +509,87 @@ Note: unless overridden, UDP ports with numbers corresponding to forwarded TCP
>  port numbers are forwarded too, without, however, any port translation. IPv6
>  bound ports are also forwarded for IPv4.
>  
> -Default is \fBnone\fR.
> +Default is \fBnone\fR for \fBpasst\fR and \fBauto\fR for \fBpasta\fR.
>  
> -.SS \fBpasta\fR-only options
> +.SS \fBpasst\fR-only options
>  
>  .TP
> -.BR \-I ", " \-\-ns-ifname " " \fIname
> -Name of tap interface to be created in target namespace.
> -By default, the same interface name as the external, routable interface is used.
> -If no such interface exists, the name \fItap0\fR will be used instead.
> +.BR \-s ", " \-\-socket-path ", " \-\-socket " " \fIpath
> +Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to
> +\fBpasst\fR.
> +Default is to probe a free socket, not accepting connections, starting from
> +\fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR.
>  
>  .TP
> -.BR \-t ", " \-\-tcp-ports " " \fIspec
> -Configure TCP port forwarding to namespace. \fIspec\fR can be one of:
> -.RS
> +.BR \-\-vhost-user
> +Enable vhost-user. The vhost-user command socket is provided by \fB--socket\fR.
>  
>  .TP
> -.BR none
> -Don't forward any ports
> +.BR \-\-print-capabilities
> +Print back-end capabilities in JSON format, only meaningful for vhost-user mode.
>  
>  .TP
> -.BR auto
> -Dynamically forward ports bound in the namespace. The list of ports is
> -periodically derived (every second) from listening sockets reported by
> -\fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5).
> +.BR \-\-repair-path " " \fIpath
> +Path for UNIX domain socket used by the \fBpasst-repair\fR(1) helper to connect
> +to \fBpasst\fR in order to set or clear the TCP_REPAIR option on sockets, during
> +migration. \fB--repair-path none\fR disables this interface (if you need to
> +specify a socket path called "none" you can prefix the path by \fI./\fR).
> +
> +Default, for \-\-vhost-user mode only, is to append \fI.repair\fR to the path
> +chosen for the hypervisor UNIX domain socket. No socket is created if not in
> +\-\-vhost-user mode.
>  
>  .TP
> -.BR ports
> -A comma-separated list of ports, optionally ranged with \fI-\fR, and,
> -optionally, with target ports after \fI:\fR, if they differ. Specific addresses
> -can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited
> -to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports
> -and ranges can be excluded by an additional specification prefixed by \fI~\fR.
> +.BR \-\-migrate-exit " " (DEPRECATED)
> +Exit after a completed migration as source. By default, \fBpasst\fR keeps
> +running and the migrated guest can continue using its connection, or a new guest
> +can connect.
>  
> -Specifying excluded ranges only implies that all other ports are forwarded. In
> -this case, no failures are reported for unavailable ports, unless no ports could
> -be forwarded at all.
> +Note that this configuration option is \fBdeprecated\fR and will be removed in a
> +future version. It is not expected to be of any use, and it simply reflects a
> +legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
> +below.
>  
> -Examples:
> -.RS
> -.TP
> --t 22
> -Forward local port 22 to 22 in the target namespace

...the passt equivalent says "guest". I think we should replace all
these occurrences by "guest or namespace" at this point.

-- 
Stefano

Re: [PATCH 04/18] doc: Consolidate -[tu] option descriptions for passt and pasta

[David Gibson]

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

On Wed, Apr 08, 2026 at 01:14:41AM +0200, Stefano Brivio wrote:
11;rgb:ffff/ffff/ffff> On Tue,  7 Apr 2026 13:16:16 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The man page currently has two fairly large, near-identical sections
> 
> Not entirely identical also because:
> 
> > separately describing the -t and -u options for passt and pasta.  This is
> > bulky and potentially confusing.  It will make this information more
> > tedious to update as we alter what's possible here with the forwarding
> > table.  Consolidate both descriptions to a single one in the common
> > options, noting the few passt/pasta difference inline.
> > 
> > There's similar duplication usage(), consolidate that as well.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  conf.c  |  54 +++++---------
> >  passt.1 | 216 ++++++++++++++++++--------------------------------------
> >  2 files changed, 85 insertions(+), 185 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index f3b36bb6..751e500f 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -1023,18 +1023,12 @@ static void usage(const char *name, FILE *f, int status)
> >  		"  --freebind		Bind to any address for forwarding\n"
> >  		"  --no-map-gw		Don't map gateway address to host\n"
> >  		"  -4, --ipv4-only	Enable IPv4 operation only\n"
> > -		"  -6, --ipv6-only	Enable IPv6 operation only\n");
> > -
> > -	if (strstr(name, "pasta"))
> > -		goto pasta_opts;
> > -
> > -	FPRINTF(f,
> > -		"  -1, --one-off	Quit after handling one single client\n"
> > +		"  -6, --ipv6-only	Enable IPv6 operation only\n"
> >  		"  -t, --tcp-ports SPEC	TCP port forwarding to guest\n"
> >  		"    can be specified multiple times\n"
> >  		"    SPEC can be:\n"
> >  		"      'none': don't forward any ports\n"
> > -		"      'all': forward all unbound, non-ephemeral ports\n"
> > +		"%s"
> >  		"      a comma-separated list, optionally ranged with '-'\n"
> >  		"        and optional target ports after ':', with optional\n"
> >  		"        address specification suffixed by '/' and optional\n"
> > @@ -1050,43 +1044,29 @@ static void usage(const char *name, FILE *f, int status)
> >  		"        -t 192.0.2.1/5	Bind port 5 of 192.0.2.1 to guest\n"
> >  		"        -t 5-25,~10-20	Forward ports 5 to 9, and 21 to 25\n"
> >  		"        -t ~25		Forward all ports except for 25\n"
> > -		"    default: none\n"
> > +		"    default: %s\n"
> >  		"  -u, --udp-ports SPEC	UDP port forwarding to guest\n"
> 
> ...this is "guest" for passt and "namespace" for pasta. Now, I know
> that you use those terms interchangeably and the code does as well, but
> this is directly visible to users so I think we shouldn't confuse them
> with "guests" for... containers.
> 
> To reaffirm the difference between terms used in the code and terms
> used in documentation, could we perhaps do something like:
> 
> 	char *guest = (mode == MODE_PASTA) ? "namespace" : "guest";
> 
> ? And then use that later? It might be worth doing that for the
> "default" strings too.

Sure, done.

> 
> >  		"    SPEC is as described for TCP above\n"
> > -		"    default: none\n");
> > +		"    default: %s\n",
> > +		strstr(name, "pasta") ?
> > +		"      'auto': forward all ports currently bound in namespace\n"
> > +		:
> > +		"      'all': forward all unbound, non-ephemeral ports\n",
> > +		strstr(name, "pasta") ? "auto" : "none",
> > +		strstr(name, "pasta") ? "auto" : "none");
> > +
> > +	if (strstr(name, "pasta"))
> > +		goto pasta_opts;
> > +
> > +	FPRINTF(f,
> > +		"  -1, --one-off	Quit after handling one single client\n"
> > +		);
> >  
> >  	passt_exit(status);
> >  
> >  pasta_opts:
> >  
> >  	FPRINTF(f,
> > -		"  -t, --tcp-ports SPEC	TCP port forwarding to namespace\n"
> > -		"    can be specified multiple times\n"
> > -		"    SPEC can be:\n"
> > -		"      'none': don't forward any ports\n"
> > -		"      'auto': forward all ports currently bound in namespace\n"
> > -		"      a comma-separated list, optionally ranged with '-'\n"
> > -		"        and optional target ports after ':', with optional\n"
> > -		"        address specification suffixed by '/' and optional\n"
> > -		"        interface prefixed by '%%'. Examples:\n"
> > -		"        -t 22	Forward local port 22 to port 22 in netns\n"
> > -		"        -t 22:23	Forward local port 22 to port 23\n"
> > -		"        -t 22,25	Forward ports 22, 25 to ports 22, 25\n"
> > -		"        -t 22-80	Forward ports 22 to 80\n"
> > -		"        -t 22-80:32-90	Forward ports 22 to 80 to\n"
> > -		"			corresponding port numbers plus 10\n"
> > -		"        -t 192.0.2.1/5	Bind port 5 of 192.0.2.1 to namespace\n"
> > -		"        -t 5-25,~10-20	Forward ports 5 to 9, and 21 to 25\n"
> > -		"        -t ~25		Forward all bound ports except for 25\n"
> > -		"    default: auto\n"
> > -		"    IPv6 bound ports are also forwarded for IPv4\n"
> > -		"  -u, --udp-ports SPEC	UDP port forwarding to namespace\n"
> > -		"    SPEC is as described for TCP above\n"
> > -		"    default: auto\n"
> > -		"    IPv6 bound ports are also forwarded for IPv4\n"
> > -		"    unless specified, with '-t auto', UDP ports with numbers\n"
> > -		"    corresponding to forwarded TCP port numbers are\n"
> > -		"    forwarded too\n"
> >  		"  -T, --tcp-ns SPEC	TCP port forwarding to init namespace\n"
> >  		"    SPEC is as described above\n"
> >  		"    default: auto\n"
> > diff --git a/passt.1 b/passt.1
> > index 13e8df9d..a9a8a42a 100644
> > --- a/passt.1
> > +++ b/passt.1
> > @@ -425,78 +425,6 @@ Send \fIname\fR as DHCP option 12 (hostname).
> >  FQDN to configure the client with.
> >  Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39.
> >  
> > -.SS \fBpasst\fR-only options
> > -
> > -.TP
> > -.BR \-s ", " \-\-socket-path ", " \-\-socket " " \fIpath
> > -Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to
> > -\fBpasst\fR.
> > -Default is to probe a free socket, not accepting connections, starting from
> > -\fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR.
> > -
> > -.TP
> > -.BR \-\-vhost-user
> > -Enable vhost-user. The vhost-user command socket is provided by \fB--socket\fR.
> > -
> > -.TP
> > -.BR \-\-print-capabilities
> > -Print back-end capabilities in JSON format, only meaningful for vhost-user mode.
> > -
> > -.TP
> > -.BR \-\-repair-path " " \fIpath
> > -Path for UNIX domain socket used by the \fBpasst-repair\fR(1) helper to connect
> > -to \fBpasst\fR in order to set or clear the TCP_REPAIR option on sockets, during
> > -migration. \fB--repair-path none\fR disables this interface (if you need to
> > -specify a socket path called "none" you can prefix the path by \fI./\fR).
> > -
> > -Default, for \-\-vhost-user mode only, is to append \fI.repair\fR to the path
> > -chosen for the hypervisor UNIX domain socket. No socket is created if not in
> > -\-\-vhost-user mode.
> > -
> > -.TP
> > -.BR \-\-migrate-exit " " (DEPRECATED)
> > -Exit after a completed migration as source. By default, \fBpasst\fR keeps
> > -running and the migrated guest can continue using its connection, or a new guest
> > -can connect.
> > -
> > -Note that this configuration option is \fBdeprecated\fR and will be removed in a
> > -future version. It is not expected to be of any use, and it simply reflects a
> > -legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
> > -below.
> > -
> > -.TP
> > -.BR \-\-migrate-no-linger " " (DEPRECATED)
> > -Close TCP sockets on the source instance once migration completes.
> > -
> > -By default, sockets are kept open, and events on data sockets are ignored, so
> > -that any further message reaching sockets after the source migrated is silently
> > -ignored, to avoid connection resets in case data is received after migration.
> > -
> > -Note that this configuration option is \fBdeprecated\fR and will be removed in a
> > -future version. It is not expected to be of any use, and it simply reflects a
> > -legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
> > -below.
> > -
> > -.TP
> > -.BR \-F ", " \-\-fd " " \fIFD
> > -Pass a pre-opened, connected socket to \fBpasst\fR. Usually the socket is opened
> > -in the parent process and \fBpasst\fR inherits it when run as a child. This
> > -allows the parent process to open sockets using another address family or
> > -requiring special privileges.
> > -
> > -This option implies the behaviour described for \-\-one-off, once this socket
> > -is closed.
> > -
> > -.TP
> > -.BR \-1 ", " \-\-one-off
> > -Quit after handling a single client connection, that is, once the client closes
> > -the socket, or once we get a socket error.
> > -
> > -\fBNote\fR: this option has no effect after \fBpasst\fR completes a migration as
> > -source, because, in that case, exiting would close sockets for active
> > -connections, which would in turn cause connection resets if any further data is
> > -received. See also the description of \fI\-\-migrate-no-linger\fR.
> > -
> >  .TP
> >  .BR \-t ", " \-\-tcp-ports " " \fIspec
> >  Configure TCP port forwarding to guest. \fIspec\fR can be one of:
> > @@ -507,11 +435,17 @@ Configure TCP port forwarding to guest. \fIspec\fR can be one of:
> >  Don't forward any ports
> >  
> >  .TP
> > -.BR all
> > +.BR all (\fBpasst\fR only)
> 
> This is rendered as "passtonly". You need \fBpasst\fR " " only. While
> this one goes away in 5/18,

Fixed.  (In fact I think I might have fixed that before, then
accidentally discarded it; oops).

> >  Forward all unbound, non-ephemeral ports, as permitted by current capabilities.
> >  For low (< 1024) ports, see \fBNOTES\fR. No failures are reported for
> >  unavailable ports, unless no ports could be forwarded at all.
> >  
> > +.TP
> > +.BR auto (\fBpasta\fR only)
> 
> ...this one doesn't.
> 
> > +Dynamically forward ports bound in the namespace. The list of ports is
> > +periodically derived (every second) from listening sockets reported by
> > +\fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5).
> > +
> >  .TP
> >  .BR ports
> >  A comma-separated list of ports, optionally ranged with \fI-\fR, and,
> > @@ -563,7 +497,7 @@ and 30
> >  Forward all ports to the guest, except for the range from 20000 to 20010
> >  .RE
> >  
> > -Default is \fBnone\fR.
> > +Default is \fBnone\fR for \fBpasst\fR and \fBauto\fR for \fBpasta\fR.
> >  .RE
> >  
> >  .TP
> > @@ -575,101 +509,87 @@ Note: unless overridden, UDP ports with numbers corresponding to forwarded TCP
> >  port numbers are forwarded too, without, however, any port translation. IPv6
> >  bound ports are also forwarded for IPv4.
> >  
> > -Default is \fBnone\fR.
> > +Default is \fBnone\fR for \fBpasst\fR and \fBauto\fR for \fBpasta\fR.
> >  
> > -.SS \fBpasta\fR-only options
> > +.SS \fBpasst\fR-only options
> >  
> >  .TP
> > -.BR \-I ", " \-\-ns-ifname " " \fIname
> > -Name of tap interface to be created in target namespace.
> > -By default, the same interface name as the external, routable interface is used.
> > -If no such interface exists, the name \fItap0\fR will be used instead.
> > +.BR \-s ", " \-\-socket-path ", " \-\-socket " " \fIpath
> > +Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to
> > +\fBpasst\fR.
> > +Default is to probe a free socket, not accepting connections, starting from
> > +\fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR.
> >  
> >  .TP
> > -.BR \-t ", " \-\-tcp-ports " " \fIspec
> > -Configure TCP port forwarding to namespace. \fIspec\fR can be one of:
> > -.RS
> > +.BR \-\-vhost-user
> > +Enable vhost-user. The vhost-user command socket is provided by \fB--socket\fR.
> >  
> >  .TP
> > -.BR none
> > -Don't forward any ports
> > +.BR \-\-print-capabilities
> > +Print back-end capabilities in JSON format, only meaningful for vhost-user mode.
> >  
> >  .TP
> > -.BR auto
> > -Dynamically forward ports bound in the namespace. The list of ports is
> > -periodically derived (every second) from listening sockets reported by
> > -\fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5).
> > +.BR \-\-repair-path " " \fIpath
> > +Path for UNIX domain socket used by the \fBpasst-repair\fR(1) helper to connect
> > +to \fBpasst\fR in order to set or clear the TCP_REPAIR option on sockets, during
> > +migration. \fB--repair-path none\fR disables this interface (if you need to
> > +specify a socket path called "none" you can prefix the path by \fI./\fR).
> > +
> > +Default, for \-\-vhost-user mode only, is to append \fI.repair\fR to the path
> > +chosen for the hypervisor UNIX domain socket. No socket is created if not in
> > +\-\-vhost-user mode.
> >  
> >  .TP
> > -.BR ports
> > -A comma-separated list of ports, optionally ranged with \fI-\fR, and,
> > -optionally, with target ports after \fI:\fR, if they differ. Specific addresses
> > -can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited
> > -to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports
> > -and ranges can be excluded by an additional specification prefixed by \fI~\fR.
> > +.BR \-\-migrate-exit " " (DEPRECATED)
> > +Exit after a completed migration as source. By default, \fBpasst\fR keeps
> > +running and the migrated guest can continue using its connection, or a new guest
> > +can connect.
> >  
> > -Specifying excluded ranges only implies that all other ports are forwarded. In
> > -this case, no failures are reported for unavailable ports, unless no ports could
> > -be forwarded at all.
> > +Note that this configuration option is \fBdeprecated\fR and will be removed in a
> > +future version. It is not expected to be of any use, and it simply reflects a
> > +legacy behaviour. If you have any use for this, refer to \fBREPORTING BUGS\fR
> > +below.
> >  
> > -Examples:
> > -.RS
> > -.TP
> > --t 22
> > -Forward local port 22 to 22 in the target namespace
> 
> ...the passt equivalent says "guest". I think we should replace all
> these occurrences by "guest or namespace" at this point.

I started doing that, but thought it looked awkwardly bulky, without
adding much clarity, so I left it at least for the first draft (there
are also some existing places in the man page which use "guest" to
refer to either a VM or namespace).  But, given your preference I've
substituted "guest or namespace" for v2.

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

[PATCH 05/18] conf: Permit -[tTuU] all in pasta mode

[David Gibson]

Currently we explicitly forbid -[tTuU] all in pasta mode.  While these are
primarily useful for passt, there's no particular reason they can't be
used in pasta mode as well.  Indeed you can do the same thing in pasta
by using "-t ~32768-60999" (assuming default Linux configuration of
ephemeral ports).  For consistency, permit "all" for pasta as well.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c  | 7 ++-----
 passt.1 | 2 +-
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/conf.c b/conf.c
index 751e500f..870712af 100644
--- a/conf.c
+++ b/conf.c
@@ -358,9 +358,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		if (*mode)
 			goto mode_conflict;
 
-		if (c->mode == MODE_PASTA)
-			die("'all' port forwarding is only allowed for passt");
-
 		*mode = FWD_MODE_ALL;
 
 		/* Exclude ephemeral ports */
@@ -1028,6 +1025,7 @@ static void usage(const char *name, FILE *f, int status)
 		"    can be specified multiple times\n"
 		"    SPEC can be:\n"
 		"      'none': don't forward any ports\n"
+		"      'all': forward all unbound, non-ephemeral ports\n"
 		"%s"
 		"      a comma-separated list, optionally ranged with '-'\n"
 		"        and optional target ports after ':', with optional\n"
@@ -1050,8 +1048,7 @@ static void usage(const char *name, FILE *f, int status)
 		"    default: %s\n",
 		strstr(name, "pasta") ?
 		"      'auto': forward all ports currently bound in namespace\n"
-		:
-		"      'all': forward all unbound, non-ephemeral ports\n",
+		: "",
 		strstr(name, "pasta") ? "auto" : "none",
 		strstr(name, "pasta") ? "auto" : "none");
 
diff --git a/passt.1 b/passt.1
index a9a8a42a..44113929 100644
--- a/passt.1
+++ b/passt.1
@@ -435,7 +435,7 @@ Configure TCP port forwarding to guest. \fIspec\fR can be one of:
 Don't forward any ports
 
 .TP
-.BR all (\fBpasst\fR only)
+.BR all
 Forward all unbound, non-ephemeral ports, as permitted by current capabilities.
 For low (< 1024) ports, see \fBNOTES\fR. No failures are reported for
 unavailable ports, unless no ports could be forwarded at all.
-- 
2.53.0

[PATCH 06/18] fwd: Better split forwarding rule specification from associated sockets

[David Gibson]

6dad076df037 ("fwd: Split forwarding rule specification from its
implementation state") created struct fwd_rule_state with a forwarding rule
plus the table of sockets used for its implementation.  It turns out this
is quite awkward for sharing rule parsing code between passt and the
upcoming configuration client.

Instead keep the index of listening sockets in a parallel array in
struct fwd_table.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c | 73 ++++++++++++++++++++++++++++++-----------------------------
 fwd.h | 16 ++++---------
 2 files changed, 41 insertions(+), 48 deletions(-)

diff --git a/fwd.c b/fwd.c
index e09b42fe..7e0edc38 100644
--- a/fwd.c
+++ b/fwd.c
@@ -363,7 +363,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 	/* Flags which can be set from the caller */
 	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
 	unsigned num = (unsigned)last - first + 1;
-	struct fwd_rule_state *new;
+	struct fwd_rule *new;
 	unsigned i, port;
 
 	assert(!(flags & ~allowed_flags));
@@ -379,7 +379,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 	/* Check for any conflicting entries */
 	for (i = 0; i < fwd->count; i++) {
 		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
-		const struct fwd_rule *rule = &fwd->rules[i].rule;
+		const struct fwd_rule *rule = &fwd->rules[i];
 
 		if (proto != rule->proto)
 			/* Non-conflicting protocols */
@@ -399,38 +399,38 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 		    rule->first, rule->last);
 	}
 
-	new = &fwd->rules[fwd->count++];
-	new->rule.proto = proto;
-	new->rule.flags = flags;
+	new = &fwd->rules[fwd->count];
+	new->proto = proto;
+	new->flags = flags;
 
 	if (addr) {
-		new->rule.addr = *addr;
+		new->addr = *addr;
 	} else {
-		new->rule.addr = inany_any6;
-		new->rule.flags |= FWD_DUAL_STACK_ANY;
+		new->addr = inany_any6;
+		new->flags |= FWD_DUAL_STACK_ANY;
 	}
 
-	memset(new->rule.ifname, 0, sizeof(new->rule.ifname));
+	memset(new->ifname, 0, sizeof(new->ifname));
 	if (ifname) {
 		int ret;
 
-		ret = snprintf(new->rule.ifname, sizeof(new->rule.ifname),
+		ret = snprintf(new->ifname, sizeof(new->ifname),
 			       "%s", ifname);
-		if (ret <= 0 || (size_t)ret >= sizeof(new->rule.ifname))
+		if (ret <= 0 || (size_t)ret >= sizeof(new->ifname))
 			die("Invalid interface name: %s", ifname);
 	}
 
 	assert(first <= last);
-	new->rule.first = first;
-	new->rule.last = last;
+	new->first = first;
+	new->last = last;
+	new->to = to;
 
-	new->rule.to = to;
+	fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
+	for (port = new->first; port <= new->last; port++)
+		fwd->rulesocks[fwd->count][port - new->first] = -1;
 
-	new->socks = &fwd->socks[fwd->sock_count];
+	fwd->count++;
 	fwd->sock_count += num;
-
-	for (port = new->rule.first; port <= new->rule.last; port++)
-		new->socks[port - new->rule.first] = -1;
 }
 
 /**
@@ -466,7 +466,7 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
 
 	if (hint >= 0) {
 		char ostr[INANY_ADDRSTRLEN], rstr[INANY_ADDRSTRLEN];
-		const struct fwd_rule *rule = &fwd->rules[hint].rule;
+		const struct fwd_rule *rule = &fwd->rules[hint];
 
 		assert((unsigned)hint < fwd->count);
 		if (fwd_rule_match(rule, ini, proto))
@@ -480,8 +480,8 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
 	}
 
 	for (i = 0; i < fwd->count; i++) {
-		if (fwd_rule_match(&fwd->rules[i].rule, ini, proto))
-			return &fwd->rules[i].rule;
+		if (fwd_rule_match(&fwd->rules[i], ini, proto))
+			return &fwd->rules[i];
 	}
 
 	return NULL;
@@ -496,7 +496,7 @@ void fwd_rules_print(const struct fwd_table *fwd)
 	unsigned i;
 
 	for (i = 0; i < fwd->count; i++) {
-		const struct fwd_rule *rule = &fwd->rules[i].rule;
+		const struct fwd_rule *rule = &fwd->rules[i];
 		const char *percent = *rule->ifname ? "%" : "";
 		const char *weak = "", *scan = "";
 		char addr[INANY_ADDRSTRLEN];
@@ -533,9 +533,9 @@ void fwd_rules_print(const struct fwd_table *fwd)
 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_state *rs = &c->fwd[pif]->rules[idx];
-	const struct fwd_rule *rule = &rs->rule;
+	const struct fwd_rule *rule = &c->fwd[pif]->rules[idx];
 	const union inany_addr *addr = fwd_rule_addr(rule);
+	int *socks = c->fwd[pif]->rulesocks[idx];
 	const char *ifname = rule->ifname;
 	const uint8_t *map = NULL;
 	bool bound_one = false;
@@ -555,7 +555,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
 	}
 
 	for (port = rule->first; port <= rule->last; port++) {
-		int fd = rs->socks[port - rule->first];
+		int fd = socks[port - rule->first];
 
 		if (map && !bitmap_isset(map, port)) {
 			/* We don't want to listen on this port */
@@ -563,7 +563,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
 				/* We already are, so stop */
 				epoll_del(c->epollfd, fd);
 				close(fd);
-				rs->socks[port - rule->first] = -1;
+				socks[port - rule->first] = -1;
 			}
 			continue;
 		}
@@ -595,7 +595,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
 			continue;
 		}
 
-		rs->socks[port - rule->first] = fd;
+		socks[port - rule->first] = fd;
 		bound_one = true;
 	}
 
@@ -685,11 +685,12 @@ void fwd_listen_close(const struct fwd_table *fwd)
 	unsigned i;
 
 	for (i = 0; i < fwd->count; i++) {
-		const struct fwd_rule_state *rs = &fwd->rules[i];
+		const struct fwd_rule *rule = &fwd->rules[i];
+		int *socks = fwd->rulesocks[i];
 		unsigned port;
 
-		for (port = rs->rule.first; port <= rs->rule.last; port++) {
-			int *fdp = &rs->socks[port - rs->rule.first];
+		for (port = rule->first; port <= rule->last; port++) {
+			int *fdp = &socks[port - rule->first];
 			if (*fdp >= 0) {
 				close(*fdp);
 				*fdp = -1;
@@ -769,8 +770,8 @@ static bool has_scan_rules(const struct fwd_table *fwd, uint8_t proto)
 	unsigned i;
 
 	for (i = 0; i < fwd->count; i++) {
-		if (fwd->rules[i].rule.proto == proto &&
-		    fwd->rules[i].rule.flags & FWD_SCAN)
+		if (fwd->rules[i].proto == proto &&
+		    fwd->rules[i].flags & FWD_SCAN)
 			return true;
 	}
 	return false;
@@ -838,14 +839,14 @@ static void current_listen_map(uint8_t *map, const struct fwd_table *fwd,
 	memset(map, 0, PORT_BITMAP_SIZE);
 
 	for (i = 0; i < fwd->count; i++) {
-		const struct fwd_rule_state *rs = &fwd->rules[i];
+		const struct fwd_rule *rule = &fwd->rules[i];
 		unsigned port;
 
-		if (rs->rule.proto != proto)
+		if (rule->proto != proto)
 			continue;
 
-		for (port = rs->rule.first; port <= rs->rule.last; port++) {
-			if (rs->socks[port - rs->rule.first] >= 0)
+		for (port = rule->first; port <= rule->last; port++) {
+			if (fwd->rulesocks[i][port - rule->first] >= 0)
 				bitmap_set(map, port);
 		}
 	}
diff --git a/fwd.h b/fwd.h
index 33600cbf..83ee9b2e 100644
--- a/fwd.h
+++ b/fwd.h
@@ -26,16 +26,6 @@ struct flowside;
 void fwd_probe_ephemeral(void);
 void fwd_port_map_ephemeral(uint8_t *map);
 
-/**
- * struct fwd_rule_state - Forwarding rule and associated state
- * @rule:	Rule specification
- * @socks:	Array of listening sockets for this entry
- */
-struct fwd_rule_state {
-	struct fwd_rule rule;
-	int *socks;
-};
-
 #define FWD_RULE_BITS	8
 #define MAX_FWD_RULES	MAX_FROM_BITS(FWD_RULE_BITS)
 #define FWD_NO_HINT	(-1)
@@ -61,15 +51,17 @@ struct fwd_listen_ref {
 #define MAX_LISTEN_SOCKS	(NUM_PORTS * 5)
 
 /**
- * struct fwd_table - Table of forwarding rules (per initiating pif)
+ * struct fwd_table - Forwarding state (per initiating pif)
  * @count:	Number of forwarding rules
  * @rules:	Array of forwarding rules
+ * @rulesocks:	Pointers to socket arrays per-rule
  * @sock_count:	Number of entries used in @socks
  * @socks:	Listening sockets for forwarding
  */
 struct fwd_table {
 	unsigned count;
-	struct fwd_rule_state rules[MAX_FWD_RULES];
+	struct fwd_rule rules[MAX_FWD_RULES];
+	int *rulesocks[MAX_FWD_RULES];
 	unsigned sock_count;
 	int socks[MAX_LISTEN_SOCKS];
 };
-- 
2.53.0

Re: [PATCH 06/18] fwd: Better split forwarding rule specification from associated sockets

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:18 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> 6dad076df037 ("fwd: Split forwarding rule specification from its
> implementation state") created struct fwd_rule_state with a forwarding rule
> plus the table of sockets used for its implementation.  It turns out this
> is quite awkward for sharing rule parsing code between passt and the
> upcoming configuration client.

Indeed, I hated it, in that short moment I had to fiddle with it. Thanks
for coming up with a cleaner solution.

> 
> Instead keep the index of listening sockets in a parallel array in
> struct fwd_table.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  fwd.c | 73 ++++++++++++++++++++++++++++++-----------------------------
>  fwd.h | 16 ++++---------
>  2 files changed, 41 insertions(+), 48 deletions(-)
> 
> diff --git a/fwd.c b/fwd.c
> index e09b42fe..7e0edc38 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -363,7 +363,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
>  	/* Flags which can be set from the caller */
>  	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
>  	unsigned num = (unsigned)last - first + 1;
> -	struct fwd_rule_state *new;
> +	struct fwd_rule *new;
>  	unsigned i, port;
>  
>  	assert(!(flags & ~allowed_flags));
> @@ -379,7 +379,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
>  	/* Check for any conflicting entries */
>  	for (i = 0; i < fwd->count; i++) {
>  		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
> -		const struct fwd_rule *rule = &fwd->rules[i].rule;
> +		const struct fwd_rule *rule = &fwd->rules[i];
>  
>  		if (proto != rule->proto)
>  			/* Non-conflicting protocols */
> @@ -399,38 +399,38 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
>  		    rule->first, rule->last);
>  	}
>  
> -	new = &fwd->rules[fwd->count++];
> -	new->rule.proto = proto;
> -	new->rule.flags = flags;
> +	new = &fwd->rules[fwd->count];
> +	new->proto = proto;
> +	new->flags = flags;
>  
>  	if (addr) {
> -		new->rule.addr = *addr;
> +		new->addr = *addr;
>  	} else {
> -		new->rule.addr = inany_any6;
> -		new->rule.flags |= FWD_DUAL_STACK_ANY;
> +		new->addr = inany_any6;
> +		new->flags |= FWD_DUAL_STACK_ANY;
>  	}
>  
> -	memset(new->rule.ifname, 0, sizeof(new->rule.ifname));
> +	memset(new->ifname, 0, sizeof(new->ifname));
>  	if (ifname) {
>  		int ret;
>  
> -		ret = snprintf(new->rule.ifname, sizeof(new->rule.ifname),
> +		ret = snprintf(new->ifname, sizeof(new->ifname),
>  			       "%s", ifname);
> -		if (ret <= 0 || (size_t)ret >= sizeof(new->rule.ifname))
> +		if (ret <= 0 || (size_t)ret >= sizeof(new->ifname))
>  			die("Invalid interface name: %s", ifname);
>  	}
>  
>  	assert(first <= last);
> -	new->rule.first = first;
> -	new->rule.last = last;
> +	new->first = first;
> +	new->last = last;
> +	new->to = to;
>  
> -	new->rule.to = to;
> +	fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
> +	for (port = new->first; port <= new->last; port++)
> +		fwd->rulesocks[fwd->count][port - new->first] = -1;
>  
> -	new->socks = &fwd->socks[fwd->sock_count];
> +	fwd->count++;
>  	fwd->sock_count += num;
> -
> -	for (port = new->rule.first; port <= new->rule.last; port++)
> -		new->socks[port - new->rule.first] = -1;
>  }
>  
>  /**
> @@ -466,7 +466,7 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
>  
>  	if (hint >= 0) {
>  		char ostr[INANY_ADDRSTRLEN], rstr[INANY_ADDRSTRLEN];
> -		const struct fwd_rule *rule = &fwd->rules[hint].rule;
> +		const struct fwd_rule *rule = &fwd->rules[hint];
>  
>  		assert((unsigned)hint < fwd->count);
>  		if (fwd_rule_match(rule, ini, proto))
> @@ -480,8 +480,8 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
>  	}
>  
>  	for (i = 0; i < fwd->count; i++) {
> -		if (fwd_rule_match(&fwd->rules[i].rule, ini, proto))
> -			return &fwd->rules[i].rule;
> +		if (fwd_rule_match(&fwd->rules[i], ini, proto))
> +			return &fwd->rules[i];
>  	}
>  
>  	return NULL;
> @@ -496,7 +496,7 @@ void fwd_rules_print(const struct fwd_table *fwd)
>  	unsigned i;
>  
>  	for (i = 0; i < fwd->count; i++) {
> -		const struct fwd_rule *rule = &fwd->rules[i].rule;
> +		const struct fwd_rule *rule = &fwd->rules[i];
>  		const char *percent = *rule->ifname ? "%" : "";
>  		const char *weak = "", *scan = "";
>  		char addr[INANY_ADDRSTRLEN];
> @@ -533,9 +533,9 @@ void fwd_rules_print(const struct fwd_table *fwd)
>  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_state *rs = &c->fwd[pif]->rules[idx];
> -	const struct fwd_rule *rule = &rs->rule;
> +	const struct fwd_rule *rule = &c->fwd[pif]->rules[idx];
>  	const union inany_addr *addr = fwd_rule_addr(rule);
> +	int *socks = c->fwd[pif]->rulesocks[idx];
>  	const char *ifname = rule->ifname;
>  	const uint8_t *map = NULL;
>  	bool bound_one = false;
> @@ -555,7 +555,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
>  	}
>  
>  	for (port = rule->first; port <= rule->last; port++) {
> -		int fd = rs->socks[port - rule->first];
> +		int fd = socks[port - rule->first];
>  
>  		if (map && !bitmap_isset(map, port)) {
>  			/* We don't want to listen on this port */
> @@ -563,7 +563,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
>  				/* We already are, so stop */
>  				epoll_del(c->epollfd, fd);
>  				close(fd);
> -				rs->socks[port - rule->first] = -1;
> +				socks[port - rule->first] = -1;
>  			}
>  			continue;
>  		}
> @@ -595,7 +595,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
>  			continue;
>  		}
>  
> -		rs->socks[port - rule->first] = fd;
> +		socks[port - rule->first] = fd;
>  		bound_one = true;
>  	}
>  
> @@ -685,11 +685,12 @@ void fwd_listen_close(const struct fwd_table *fwd)
>  	unsigned i;
>  
>  	for (i = 0; i < fwd->count; i++) {
> -		const struct fwd_rule_state *rs = &fwd->rules[i];
> +		const struct fwd_rule *rule = &fwd->rules[i];
> +		int *socks = fwd->rulesocks[i];
>  		unsigned port;
>  
> -		for (port = rs->rule.first; port <= rs->rule.last; port++) {
> -			int *fdp = &rs->socks[port - rs->rule.first];
> +		for (port = rule->first; port <= rule->last; port++) {
> +			int *fdp = &socks[port - rule->first];
>  			if (*fdp >= 0) {
>  				close(*fdp);
>  				*fdp = -1;
> @@ -769,8 +770,8 @@ static bool has_scan_rules(const struct fwd_table *fwd, uint8_t proto)
>  	unsigned i;
>  
>  	for (i = 0; i < fwd->count; i++) {
> -		if (fwd->rules[i].rule.proto == proto &&
> -		    fwd->rules[i].rule.flags & FWD_SCAN)
> +		if (fwd->rules[i].proto == proto &&
> +		    fwd->rules[i].flags & FWD_SCAN)
>  			return true;
>  	}
>  	return false;
> @@ -838,14 +839,14 @@ static void current_listen_map(uint8_t *map, const struct fwd_table *fwd,
>  	memset(map, 0, PORT_BITMAP_SIZE);
>  
>  	for (i = 0; i < fwd->count; i++) {
> -		const struct fwd_rule_state *rs = &fwd->rules[i];
> +		const struct fwd_rule *rule = &fwd->rules[i];
>  		unsigned port;
>  
> -		if (rs->rule.proto != proto)
> +		if (rule->proto != proto)
>  			continue;
>  
> -		for (port = rs->rule.first; port <= rs->rule.last; port++) {
> -			if (rs->socks[port - rs->rule.first] >= 0)
> +		for (port = rule->first; port <= rule->last; port++) {
> +			if (fwd->rulesocks[i][port - rule->first] >= 0)
>  				bitmap_set(map, port);
>  		}
>  	}
> diff --git a/fwd.h b/fwd.h
> index 33600cbf..83ee9b2e 100644
> --- a/fwd.h
> +++ b/fwd.h
> @@ -26,16 +26,6 @@ struct flowside;
>  void fwd_probe_ephemeral(void);
>  void fwd_port_map_ephemeral(uint8_t *map);
>  
> -/**
> - * struct fwd_rule_state - Forwarding rule and associated state
> - * @rule:	Rule specification
> - * @socks:	Array of listening sockets for this entry
> - */
> -struct fwd_rule_state {
> -	struct fwd_rule rule;
> -	int *socks;
> -};
> -
>  #define FWD_RULE_BITS	8
>  #define MAX_FWD_RULES	MAX_FROM_BITS(FWD_RULE_BITS)
>  #define FWD_NO_HINT	(-1)
> @@ -61,15 +51,17 @@ struct fwd_listen_ref {
>  #define MAX_LISTEN_SOCKS	(NUM_PORTS * 5)
>  
>  /**
> - * struct fwd_table - Table of forwarding rules (per initiating pif)
> + * struct fwd_table - Forwarding state (per initiating pif)
>   * @count:	Number of forwarding rules
>   * @rules:	Array of forwarding rules
> + * @rulesocks:	Pointers to socket arrays per-rule

I don't see this as particularly descriptive (which sockets? What's
the array size?). I'm thinking of something like:

 @socks_ref:	Per-rule pointers to associated @socks, @sock_count of them

>   * @sock_count:	Number of entries used in @socks
>   * @socks:	Listening sockets for forwarding
>   */
>  struct fwd_table {
>  	unsigned count;
> -	struct fwd_rule_state rules[MAX_FWD_RULES];
> +	struct fwd_rule rules[MAX_FWD_RULES];
> +	int *rulesocks[MAX_FWD_RULES];
>  	unsigned sock_count;
>  	int socks[MAX_LISTEN_SOCKS];
>  };

-- 
Stefano

Re: [PATCH 06/18] fwd: Better split forwarding rule specification from associated sockets

[David Gibson]

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

On Wed, Apr 08, 2026 at 01:14:46AM +0200, Stefano Brivio wrote:
> On Tue,  7 Apr 2026 13:16:18 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > 6dad076df037 ("fwd: Split forwarding rule specification from its
> > implementation state") created struct fwd_rule_state with a forwarding rule
> > plus the table of sockets used for its implementation.  It turns out this
> > is quite awkward for sharing rule parsing code between passt and the
> > upcoming configuration client.
> 
> Indeed, I hated it, in that short moment I had to fiddle with it. Thanks
> for coming up with a cleaner solution.

Yeah, mea culpa.  Seemed like a good idea at the time, but it wasn't.

[snip]
> >  /**
> > - * struct fwd_table - Table of forwarding rules (per initiating pif)
> > + * struct fwd_table - Forwarding state (per initiating pif)
> >   * @count:	Number of forwarding rules
> >   * @rules:	Array of forwarding rules
> > + * @rulesocks:	Pointers to socket arrays per-rule
> 
> I don't see this as particularly descriptive (which sockets? What's
> the array size?). I'm thinking of something like:
> 
>  @socks_ref:	Per-rule pointers to associated @socks, @sock_count of them

There are @count of them, not @sock_count... which I guess just
emphasises the need for a better description.  How's this:

 * struct fwd_table - Forwarding state (per initiating pif)
 * @count:	Number of forwarding rules
 * @rules:	Array of forwarding rules
 * @rulesocks:	Array of @count pointers within @socks giving the start of the
 *		corresponding rule's listening sockets within the larger array
 * @sock_count:	Number of entries used in @socks (for all rules combined)
 * @socks:	Listening sockets for forwarding

> >   * @sock_count:	Number of entries used in @socks
> >   * @socks:	Listening sockets for forwarding
> >   */
> >  struct fwd_table {
> >  	unsigned count;
> > -	struct fwd_rule_state rules[MAX_FWD_RULES];
> > +	struct fwd_rule rules[MAX_FWD_RULES];
> > +	int *rulesocks[MAX_FWD_RULES];
> >  	unsigned sock_count;
> >  	int socks[MAX_LISTEN_SOCKS];
> >  };
> 
> -- 
> Stefano
> 

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

Re: [PATCH 06/18] fwd: Better split forwarding rule specification from associated sockets

[Stefano Brivio]

On Wed, 8 Apr 2026 11:30:29 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Apr 08, 2026 at 01:14:46AM +0200, Stefano Brivio wrote:
> > On Tue,  7 Apr 2026 13:16:18 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > 6dad076df037 ("fwd: Split forwarding rule specification from its
> > > implementation state") created struct fwd_rule_state with a forwarding rule
> > > plus the table of sockets used for its implementation.  It turns out this
> > > is quite awkward for sharing rule parsing code between passt and the
> > > upcoming configuration client.  
> > 
> > Indeed, I hated it, in that short moment I had to fiddle with it. Thanks
> > for coming up with a cleaner solution.  
> 
> Yeah, mea culpa.  Seemed like a good idea at the time, but it wasn't.
> 
> [snip]
> > >  /**
> > > - * struct fwd_table - Table of forwarding rules (per initiating pif)
> > > + * struct fwd_table - Forwarding state (per initiating pif)
> > >   * @count:	Number of forwarding rules
> > >   * @rules:	Array of forwarding rules
> > > + * @rulesocks:	Pointers to socket arrays per-rule  
> > 
> > I don't see this as particularly descriptive (which sockets? What's
> > the array size?). I'm thinking of something like:
> > 
> >  @socks_ref:	Per-rule pointers to associated @socks, @sock_count of them  
> 
> There are @count of them, not @sock_count...

Oops, "of course"...

> which I guess just
> emphasises the need for a better description.  How's this:
> 
>  * struct fwd_table - Forwarding state (per initiating pif)
>  * @count:	Number of forwarding rules
>  * @rules:	Array of forwarding rules
>  * @rulesocks:	Array of @count pointers within @socks giving the start of the
>  *		corresponding rule's listening sockets within the larger array

"Array of @count pointers" is ambiguous in English as it might refer to
pointers to @count. It obviously doesn't, but it might take a couple of
readings to realise that. Simple fix: "Array of pointers (@count of
them) ...".

For the rest, yes, it's better, but I started wondering if we could
simplify the representation a bit by, either:

1. storing indices instead of int *, or

2. storing start and end. I'm not sure if it's usable, but it would
   actually look easier to describe

if neither of these applies (I didn't really think it through), maybe
this is slightly more intuitive:

 * Pointers to entry in @socks (@count of them) with first socket for each rule

? If not, I think the version you just proposed is better than the
original and sufficiently clear anyway.

>  * @sock_count:	Number of entries used in @socks (for all rules combined)
>  * @socks:	Listening sockets for forwarding
> 
> > >   * @sock_count:	Number of entries used in @socks
> > >   * @socks:	Listening sockets for forwarding
> > >   */
> > >  struct fwd_table {
> > >  	unsigned count;
> > > -	struct fwd_rule_state rules[MAX_FWD_RULES];
> > > +	struct fwd_rule rules[MAX_FWD_RULES];
> > > +	int *rulesocks[MAX_FWD_RULES];
> > >  	unsigned sock_count;
> > >  	int socks[MAX_LISTEN_SOCKS];
> > >  };  
> > 
> > -- 
> > Stefano
> >   
> 

-- 
Stefano

Re: [PATCH 06/18] fwd: Better split forwarding rule specification from associated sockets

[David Gibson]

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

On Wed, Apr 08, 2026 at 11:39:55PM +0200, Stefano Brivio wrote:
> On Wed, 8 Apr 2026 11:30:29 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Wed, Apr 08, 2026 at 01:14:46AM +0200, Stefano Brivio wrote:
> > > On Tue,  7 Apr 2026 13:16:18 +1000
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > 6dad076df037 ("fwd: Split forwarding rule specification from its
> > > > implementation state") created struct fwd_rule_state with a forwarding rule
> > > > plus the table of sockets used for its implementation.  It turns out this
> > > > is quite awkward for sharing rule parsing code between passt and the
> > > > upcoming configuration client.  
> > > 
> > > Indeed, I hated it, in that short moment I had to fiddle with it. Thanks
> > > for coming up with a cleaner solution.  
> > 
> > Yeah, mea culpa.  Seemed like a good idea at the time, but it wasn't.
> > 
> > [snip]
> > > >  /**
> > > > - * struct fwd_table - Table of forwarding rules (per initiating pif)
> > > > + * struct fwd_table - Forwarding state (per initiating pif)
> > > >   * @count:	Number of forwarding rules
> > > >   * @rules:	Array of forwarding rules
> > > > + * @rulesocks:	Pointers to socket arrays per-rule  
> > > 
> > > I don't see this as particularly descriptive (which sockets? What's
> > > the array size?). I'm thinking of something like:
> > > 
> > >  @socks_ref:	Per-rule pointers to associated @socks, @sock_count of them  
> > 
> > There are @count of them, not @sock_count...
> 
> Oops, "of course"...
> 
> > which I guess just
> > emphasises the need for a better description.  How's this:
> > 
> >  * struct fwd_table - Forwarding state (per initiating pif)
> >  * @count:	Number of forwarding rules
> >  * @rules:	Array of forwarding rules
> >  * @rulesocks:	Array of @count pointers within @socks giving the start of the
> >  *		corresponding rule's listening sockets within the larger array
> 
> "Array of @count pointers" is ambiguous in English as it might refer to
> pointers to @count. It obviously doesn't, but it might take a couple of
> readings to realise that. Simple fix: "Array of pointers (@count of
> them) ...".

Good point.

> For the rest, yes, it's better, but I started wondering if we could
> simplify the representation a bit by, either:
> 
> 1. storing indices instead of int *, or

We could do that, but it makes lookups of a rule's sockets more
awkward for minimal benefit

> 2. storing start and end. I'm not sure if it's usable, but it would
>    actually look easier to describe

We could do that, but it means maintaining redundant information that
we never actually have a reason to consult

> if neither of these applies (I didn't really think it through), maybe
> this is slightly more intuitive:
> 
>  * Pointers to entry in @socks (@count of them) with first socket for each rule
> 
> ? If not, I think the version you just proposed is better than the
> original and sufficiently clear anyway.

I have this version now:

/**
 * struct fwd_table - Forwarding state (per initiating pif)
 * @count:	Number of forwarding rules
 * @rules:	Array of forwarding rules
 * @rulesocks:	Parallel array ro @rules (@count valid entries) of pointers to
 *		@socks entries giving the start of the corresponding rule's
 *		sockets within the larger array
 * @sock_count:	Number of entries used in @socks (for all rules combined)
 * @socks:	Listening sockets for forwarding
 */

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

[PATCH 07/18] fwd_rule: Move forwarding rule formatting

[David Gibson]

In order to be shared with the upcoming configuration client, we want to
split code which deals with forwarding rules as standalone objects from
those which deal with forwarding rules as they're actually used to
implement forwarding in passt/pasta.

Create fwd_rule.c to contain code from the first category, and start off
by moving code to format rules into text for human display into it.  While
we're at it, we rework that formatting code a little to make it more
flexible.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile   | 10 +++---
 conf.c     |  2 +-
 fwd.c      | 48 -----------------------------
 fwd.h      |  1 -
 fwd_rule.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fwd_rule.h | 12 ++++++++
 6 files changed, 107 insertions(+), 55 deletions(-)
 create mode 100644 fwd_rule.c

diff --git a/Makefile b/Makefile
index dd647b10..f697c12b 100644
--- a/Makefile
+++ b/Makefile
@@ -38,11 +38,11 @@ FLAGS += -DVERSION=\"$(VERSION)\"
 FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
 
 PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \
-	epoll_ctl.c flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c \
-	lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c \
-	pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c \
-	tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c vhost_user.c \
-	virtio.c vu_common.c
+	epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \
+	isolation.c lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c \
+	passt.c pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c \
+	tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c \
+	vhost_user.c virtio.c vu_common.c
 QRAP_SRCS = qrap.c
 PASST_REPAIR_SRCS = passt-repair.c
 SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS)
diff --git a/conf.c b/conf.c
index 870712af..a3452bc9 100644
--- a/conf.c
+++ b/conf.c
@@ -1261,7 +1261,7 @@ dns6:
 			dir = "Inbound";
 
 		info("%s forwarding rules (%s):", dir, pif_name(i));
-		fwd_rules_print(c->fwd[i]);
+		fwd_rules_info(c->fwd[i]->rules, c->fwd[i]->count);
 	}
 }
 
diff --git a/fwd.c b/fwd.c
index 7e0edc38..39a14c40 100644
--- a/fwd.c
+++ b/fwd.c
@@ -304,20 +304,6 @@ parse_err:
 	warn("Unable to parse %s", PORT_RANGE_SYSCTL);
 }
 
-/**
- * fwd_rule_addr() - Return match address for a rule
- * @rule:	Forwarding rule
- *
- * Return: matching address for rule, NULL if it matches all addresses
- */
-static const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule)
-{
-	if (rule->flags & FWD_DUAL_STACK_ANY)
-		return NULL;
-
-	return &rule->addr;
-}
-
 /**
  * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap
  * @map:	Bitmap to update
@@ -487,40 +473,6 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
 	return NULL;
 }
 
-/**
- * fwd_rules_print() - Print forwarding rules for debugging
- * @fwd:	Table to print
- */
-void fwd_rules_print(const struct fwd_table *fwd)
-{
-	unsigned i;
-
-	for (i = 0; i < fwd->count; i++) {
-		const struct fwd_rule *rule = &fwd->rules[i];
-		const char *percent = *rule->ifname ? "%" : "";
-		const char *weak = "", *scan = "";
-		char addr[INANY_ADDRSTRLEN];
-
-		inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr));
-		if (rule->flags & FWD_WEAK)
-			weak = " (best effort)";
-		if (rule->flags & FWD_SCAN)
-			scan = " (auto-scan)";
-
-		if (rule->first == rule->last) {
-			info("    %s [%s]%s%s:%hu  =>  %hu %s%s",
-			     ipproto_name(rule->proto), addr, percent,
-			     rule->ifname, rule->first, rule->to, weak, scan);
-		} else {
-			info("    %s [%s]%s%s:%hu-%hu  =>  %hu-%hu %s%s",
-			     ipproto_name(rule->proto), addr, percent,
-			     rule->ifname, rule->first, rule->last,
-			     rule->to, rule->last - rule->first + rule->to,
-			     weak, scan);
-		}
-	}
-}
-
 /** fwd_sync_one() - Create or remove listening sockets for a forward entry
  * @c:		Execution context
  * @pif:	Interface to create listening sockets for
diff --git a/fwd.h b/fwd.h
index 83ee9b2e..c5f6d554 100644
--- a/fwd.h
+++ b/fwd.h
@@ -89,7 +89,6 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
 				       const struct flowside *ini,
 				       uint8_t proto, int hint);
-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);
diff --git a/fwd_rule.c b/fwd_rule.c
new file mode 100644
index 00000000..abe9dfbf
--- /dev/null
+++ b/fwd_rule.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/* PASST - Plug A Simple Socket Transport
+ *  for qemu/UNIX domain socket mode
+ *
+ * PASTA - Pack A Subtle Tap Abstraction
+ *  for network namespace/tap device mode
+ *
+ * PESTO - Programmable Extensible Socket Translation Orchestrator
+ *  front-end for passt(1) and pasta(1) forwarding configuration
+ *
+ * fwd_rule.c - Helpers for working with forwarding rule specifications
+ *
+ * Copyright Red Hat
+ * Author: David Gibson <david@gibson.dropbear.id.au>
+ */
+
+#include <stdio.h>
+
+#include "fwd_rule.h"
+
+/**
+ * fwd_rule_addr() - Return match address for a rule
+ * @rule:	Forwarding rule
+ *
+ * Return: matching address for rule, NULL if it matches all addresses
+ */
+const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule)
+{
+	if (rule->flags & FWD_DUAL_STACK_ANY)
+		return NULL;
+
+	return &rule->addr;
+}
+
+/**
+ * fwd_rule_fmt() - Prettily format forwarding rule as a string
+ * @rule:	Rule to format
+ * @dst:	Buffer to store output (should have FWD_RULE_STRLEN bytes)
+ * @size:	Size of @dst
+ */
+static const char *fwd_rule_fmt(const struct fwd_rule *rule,
+				char *dst, size_t size)
+{
+	const char *percent = *rule->ifname ? "%" : "";
+	const char *weak = "", *scan = "";
+	char addr[INANY_ADDRSTRLEN];
+	int len;
+
+	inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr));
+	if (rule->flags & FWD_WEAK)
+		weak = " (best effort)";
+	if (rule->flags & FWD_SCAN)
+		scan = " (auto-scan)";
+
+	if (rule->first == rule->last) {
+		len = snprintf(dst, size,
+			       "%s [%s]%s%s:%hu  =>  %hu %s%s",
+			       ipproto_name(rule->proto), addr, percent,
+			       rule->ifname, rule->first, rule->to, weak, scan);
+	} else {
+		in_port_t tolast = rule->last - rule->first + rule->to;
+		len = snprintf(dst, size,
+			       "%s [%s]%s%s:%hu-%hu  =>  %hu-%hu %s%s",
+			       ipproto_name(rule->proto), addr, percent,
+			       rule->ifname, rule->first, rule->last,
+			       rule->to, tolast, weak, scan);
+	}
+
+	if (len < 0 || (size_t)len >= size)
+		return NULL;
+
+	return dst;
+}
+
+/**
+ * fwd_rules_info() - Print forwarding rules for debugging
+ * @fwd:	Table to print
+ */
+void fwd_rules_info(const struct fwd_rule *rules, size_t count)
+{
+	unsigned i;
+
+	for (i = 0; i < count; i++) {
+		char buf[FWD_RULE_STRLEN];
+
+		info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
+	}
+}
diff --git a/fwd_rule.h b/fwd_rule.h
index ae0e1d61..e92efb6d 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -13,7 +13,9 @@
 #include <net/if.h>
 #include <netinet/in.h>
 
+#include "ip.h"
 #include "inany.h"
+#include "bitmap.h"
 
 /**
  * struct fwd_rule - Forwarding rule governing a range of ports
@@ -41,4 +43,14 @@ struct fwd_rule {
 	uint8_t flags;
 };
 
+#define FWD_RULE_STRLEN					    \
+	(IPPROTO_STRLEN - 1				    \
+	 + INANY_ADDRSTRLEN - 1				    \
+	 + IFNAMSIZ - 1					    \
+	 + 4 * (UINT16_STRLEN - 1)			    \
+	 + sizeof(" []%:-  =>  - (best effort) (auto-scan)"))
+
+const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule);
+void fwd_rules_info(const struct fwd_rule *rules, size_t count);
+
 #endif /* FWD_RULE_H */
-- 
2.53.0

[PATCH 08/18] conf: Pass protocol explicitly to conf_ports_range_except()

[David Gibson]

Currently conf_ports_range_except() deduces the protocol to use from the
option name.  This is correct, but a DRY violation, since we check the
option name at several points in the callchain.

Instead pass the protocol explicitly to conf_ports_range_except() and
conf_ports_spec(), computing it from the option name in conf_ports().  This
is redundant for now, but means the optname and optarg parameters to the
lower-level functions are used only for debugging output and will be
removable in future.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/conf.c b/conf.c
index a3452bc9..23125d2c 100644
--- a/conf.c
+++ b/conf.c
@@ -134,6 +134,7 @@ static int parse_port_range(const char *s, const char **endptr,
  * @optname:	Short option name, t, T, u, or U
  * @optarg:	Option argument (port specification)
  * @fwd:	Forwarding table to be updated
+ * @proto:	Protocol to forward
  * @addr:	Listening address
  * @ifname:	Listening interface
  * @first:	First port to forward
@@ -144,7 +145,7 @@ static int parse_port_range(const char *s, const char **endptr,
  */
 static void conf_ports_range_except(const struct ctx *c, char optname,
 				    const char *optarg, struct fwd_table *fwd,
-				    const union inany_addr *addr,
+				    uint8_t proto, const union inany_addr *addr,
 				    const char *ifname,
 				    uint16_t first, uint16_t last,
 				    const uint8_t *exclude, uint16_t to,
@@ -152,17 +153,9 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 {
 	unsigned delta = to - first;
 	unsigned base, i;
-	uint8_t proto;
 
 	assert(first != 0);
 
-	if (optname == 't' || optname == 'T')
-		proto = IPPROTO_TCP;
-	else if (optname == 'u' || optname == 'U')
-		proto = IPPROTO_UDP;
-	else
-		assert(0);
-
 	for (base = first; base <= last; base++) {
 		if (exclude && bitmap_isset(exclude, base))
 			continue;
@@ -221,13 +214,14 @@ enum fwd_mode {
  * @optname:	Short option name, t, T, u, or U
  * @optarg:	Option argument (port specification)
  * @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,
 			    char optname, const char *optarg,
-			    struct fwd_table *fwd,
+			    struct fwd_table *fwd, uint8_t proto,
 			    const union inany_addr *addr, const char *ifname,
 			    const char *spec)
 {
@@ -262,7 +256,7 @@ static void conf_ports_spec(const struct ctx *c,
 		fwd_port_map_ephemeral(exclude);
 
 		conf_ports_range_except(c, optname, optarg, fwd,
-					addr, ifname,
+					proto, addr, ifname,
 					1, NUM_PORTS - 1, exclude,
 					1, FWD_WEAK);
 		return;
@@ -299,7 +293,7 @@ static void conf_ports_spec(const struct ctx *c,
 		}
 
 		conf_ports_range_except(c, optname, optarg, fwd,
-					addr, ifname,
+					proto, addr, ifname,
 					orig_range.first, orig_range.last,
 					exclude,
 					mapped_range.first, 0);
@@ -323,6 +317,14 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 {
 	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
 	char buf[BUFSIZ], *spec, *ifname = NULL, *p;
+	uint8_t proto;
+
+	if (optname == 't' || optname == 'T')
+		proto = IPPROTO_TCP;
+	else if (optname == 'u' || optname == 'U')
+		proto = IPPROTO_UDP;
+	else
+		assert(0);
 
 	if (!strcmp(optarg, "none")) {
 		if (*mode)
@@ -332,9 +334,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		return;
 	}
 
-	if ((optname == 't' || optname == 'T') && c->no_tcp)
+	if (proto == IPPROTO_TCP && c->no_tcp)
 		die("TCP port forwarding requested but TCP is disabled");
-	if ((optname == 'u' || optname == 'U') && c->no_udp)
+	if (proto == IPPROTO_UDP && c->no_udp)
 		die("UDP port forwarding requested but UDP is disabled");
 
 	if (!strcmp(optarg, "auto")) {
@@ -346,7 +348,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 
 		*mode = FWD_MODE_AUTO;
 
-		conf_ports_range_except(c, optname, optarg, fwd, NULL, NULL,
+		conf_ports_range_except(c, optname, optarg, fwd,
+					proto, NULL, NULL,
 					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
 
 		return;
@@ -364,7 +367,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		fwd_port_map_ephemeral(exclude);
 
 		conf_ports_range_except(c, optname, optarg, fwd,
-					NULL, NULL,
+					proto, NULL, NULL,
 					1, NUM_PORTS - 1, exclude,
 					1, FWD_WEAK);
 		return;
@@ -438,7 +441,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 	if ((optname == 'T' || optname == 'U') && !ifname)
 		ifname = "lo";
 
-	conf_ports_spec(c, optname, optarg, fwd, addr, ifname, spec);
+	conf_ports_spec(c, optname, optarg, fwd, proto, addr, ifname, spec);
 	return;
 
 mode_conflict:
-- 
2.53.0

[PATCH 09/18] fwd: Split rule building from rule adding

[David Gibson]

Currently fwd_rule_add() both builds the struct fwd_rule and inserts it
into the table.  This will be inconvenient when we want to dynamically add
rules from a configuration client.  Alter fwd_rule_add() to take a
pre-constructed struct fwd_rule, which we build in the caller.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 42 ++++++++++++++++++++++++++++++++--------
 fwd.c  | 61 ++++++++++++++--------------------------------------------
 fwd.h  |  4 +---
 3 files changed, 49 insertions(+), 58 deletions(-)

diff --git a/conf.c b/conf.c
index 23125d2c..c9ee8c59 100644
--- a/conf.c
+++ b/conf.c
@@ -151,9 +151,26 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 				    const uint8_t *exclude, uint16_t to,
 				    uint8_t flags)
 {
+	struct fwd_rule rule = {
+		.addr = addr ? *addr : inany_any6,
+		.ifname = { 0 },
+		.proto = proto,
+		.flags = flags,
+	};
 	unsigned delta = to - first;
 	unsigned base, i;
 
+	if (!addr)
+		rule.flags |= FWD_DUAL_STACK_ANY;
+	if (ifname) {
+		int ret;
+
+		ret = snprintf(rule.ifname, sizeof(rule.ifname),
+			       "%s", ifname);
+		if (ret <= 0 || (size_t)ret >= sizeof(rule.ifname))
+			die("Invalid interface name: %s", ifname);
+	}
+
 	assert(first != 0);
 
 	for (base = first; base <= last; base++) {
@@ -165,28 +182,37 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 				break;
 		}
 
+		rule.first = base;
+		rule.last = i - 1;
+		rule.to = base + delta;
+
 		if ((optname == 'T' || optname == 'U') && c->no_bindtodevice) {
 			/* FIXME: Once the fwd bitmaps are removed, move this
 			 * workaround to the caller
 			 */
+			struct fwd_rule rulev = {
+				.ifname = { 0 },
+				.flags = flags,
+				.first = base,
+				.last = i - 1,
+				.to = base + delta,
+			};
+
 			assert(!addr && ifname && !strcmp(ifname, "lo"));
 			warn(
 "SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'",
 			     optname, optarg);
 
 			if (c->ifi4) {
-				fwd_rule_add(fwd, proto, flags,
-					     &inany_loopback4, NULL,
-					     base, i - 1, base + delta);
+				rulev.addr = inany_loopback4;
+				fwd_rule_add(fwd, &rulev);
 			}
 			if (c->ifi6) {
-				fwd_rule_add(fwd, proto, flags,
-					     &inany_loopback6, NULL,
-					     base, i - 1, base + delta);
+				rulev.addr = inany_loopback6;
+				fwd_rule_add(fwd, &rulev);
 			}
 		} else {
-			fwd_rule_add(fwd, proto, flags, addr, ifname,
-				     base, i - 1, base + delta);
+			fwd_rule_add(fwd, &rule);
 		}
 		base = i - 1;
 	}
diff --git a/fwd.c b/fwd.c
index 39a14c40..c05107d1 100644
--- a/fwd.c
+++ b/fwd.c
@@ -332,30 +332,22 @@ void fwd_rule_init(struct ctx *c)
 }
 
 /**
- * fwd_rule_add() - Add a rule to a forwarding table
+ * fwd_rule_add() - Validate and add a rule to a forwarding table
  * @fwd:	Table to add to
- * @proto:	Protocol to forward
- * @flags:	Flags for this entry
- * @addr:	Our address to forward (NULL for both 0.0.0.0 and ::)
- * @ifname:	Only forward from this interface name, if non-empty
- * @first:	First port number to forward
- * @last:	Last port number to forward
- * @to:		First port of target port range to map to
+ * @new:	Rule to add
  */
-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)
+void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
 {
 	/* Flags which can be set from the caller */
 	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
-	unsigned num = (unsigned)last - first + 1;
-	struct fwd_rule *new;
+	unsigned num = (unsigned)new->last - new->first + 1;
 	unsigned i, port;
 
-	assert(!(flags & ~allowed_flags));
+	assert(!(new->flags & ~allowed_flags));
 	/* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */
-	assert(!(flags & FWD_DUAL_STACK_ANY) || !addr ||
-	       inany_equals(addr, &inany_any6));
+	assert(!(new->flags & FWD_DUAL_STACK_ANY) ||
+	       inany_equals(&new->addr, &inany_any6));
+	assert(new->first <= new->last);
 
 	if (fwd->count >= ARRAY_SIZE(fwd->rules))
 		die("Too many port forwarding ranges");
@@ -367,55 +359,30 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
 		const struct fwd_rule *rule = &fwd->rules[i];
 
-		if (proto != rule->proto)
+		if (new->proto != rule->proto)
 			/* Non-conflicting protocols */
 			continue;
 
-		if (!inany_matches(addr, fwd_rule_addr(rule)))
+		if (!inany_matches(fwd_rule_addr(new), fwd_rule_addr(rule)))
 			/* Non-conflicting addresses */
 			continue;
 
-		if (last < rule->first || rule->last < first)
+		if (new->last < rule->first || rule->last < new->first)
 			/* Port ranges don't overlap */
 			continue;
 
 		die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u",
-		    inany_ntop(addr, newstr, sizeof(newstr)), first, last,
+		    inany_ntop(fwd_rule_addr(new), newstr, sizeof(newstr)),
+		    new->first, new->last,
 		    inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)),
 		    rule->first, rule->last);
 	}
 
-	new = &fwd->rules[fwd->count];
-	new->proto = proto;
-	new->flags = flags;
-
-	if (addr) {
-		new->addr = *addr;
-	} else {
-		new->addr = inany_any6;
-		new->flags |= FWD_DUAL_STACK_ANY;
-	}
-
-	memset(new->ifname, 0, sizeof(new->ifname));
-	if (ifname) {
-		int ret;
-
-		ret = snprintf(new->ifname, sizeof(new->ifname),
-			       "%s", ifname);
-		if (ret <= 0 || (size_t)ret >= sizeof(new->ifname))
-			die("Invalid interface name: %s", ifname);
-	}
-
-	assert(first <= last);
-	new->first = first;
-	new->last = last;
-	new->to = to;
-
 	fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
 	for (port = new->first; port <= new->last; port++)
 		fwd->rulesocks[fwd->count][port - new->first] = -1;
 
-	fwd->count++;
+	fwd->rules[fwd->count++] = *new;
 	fwd->sock_count += num;
 }
 
diff --git a/fwd.h b/fwd.h
index c5f6d554..1d74cbd2 100644
--- a/fwd.h
+++ b/fwd.h
@@ -83,9 +83,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);
+void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new);
 const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
 				       const struct flowside *ini,
 				       uint8_t proto, int hint);
-- 
2.53.0

[PATCH 10/18] fwd_rule: Move rule conflict checking from fwd_rule_add() to caller

[David Gibson]

Amongst other checks, fwd_rule_add() checks that the newly added rule
doesn't conflict with any existing rules.  However, unlike the other things
we verify, this isn't really required for safe operation.  Rule conflicts
are a useful thing for the user to know about, but the forwarding logic
is perfectly sound with conflicting rules (the first one will win).

In order to support dynamic rule updates, we want fwd_rule_add() to become
a more low-level function, only checking the things it really needs to.
So, move rule conflict checking to its caller via new helpers in
fwd_rule.c.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c     |  5 +++++
 fwd.c      | 26 +-------------------------
 fwd_rule.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 fwd_rule.h |  2 ++
 4 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/conf.c b/conf.c
index c9ee8c59..a93837cc 100644
--- a/conf.c
+++ b/conf.c
@@ -205,13 +205,18 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 
 			if (c->ifi4) {
 				rulev.addr = inany_loopback4;
+				fwd_rule_conflict_check(&rulev,
+							fwd->rules, fwd->count);
 				fwd_rule_add(fwd, &rulev);
 			}
 			if (c->ifi6) {
 				rulev.addr = inany_loopback6;
+				fwd_rule_conflict_check(&rulev,
+							fwd->rules, fwd->count);
 				fwd_rule_add(fwd, &rulev);
 			}
 		} else {
+			fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
 			fwd_rule_add(fwd, &rule);
 		}
 		base = i - 1;
diff --git a/fwd.c b/fwd.c
index c05107d1..c9637525 100644
--- a/fwd.c
+++ b/fwd.c
@@ -341,7 +341,7 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
 	/* Flags which can be set from the caller */
 	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
 	unsigned num = (unsigned)new->last - new->first + 1;
-	unsigned i, port;
+	unsigned port;
 
 	assert(!(new->flags & ~allowed_flags));
 	/* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */
@@ -354,30 +354,6 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
 	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks))
 		die("Too many listening sockets");
 
-	/* Check for any conflicting entries */
-	for (i = 0; i < fwd->count; i++) {
-		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
-		const struct fwd_rule *rule = &fwd->rules[i];
-
-		if (new->proto != rule->proto)
-			/* Non-conflicting protocols */
-			continue;
-
-		if (!inany_matches(fwd_rule_addr(new), fwd_rule_addr(rule)))
-			/* Non-conflicting addresses */
-			continue;
-
-		if (new->last < rule->first || rule->last < new->first)
-			/* Port ranges don't overlap */
-			continue;
-
-		die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u",
-		    inany_ntop(fwd_rule_addr(new), newstr, sizeof(newstr)),
-		    new->first, new->last,
-		    inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)),
-		    rule->first, rule->last);
-	}
-
 	fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
 	for (port = new->first; port <= new->last; port++)
 		fwd->rulesocks[fwd->count][port - new->first] = -1;
diff --git a/fwd_rule.c b/fwd_rule.c
index abe9dfbf..4d5048f9 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -87,3 +87,47 @@ void fwd_rules_info(const struct fwd_rule *rules, size_t count)
 		info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
 	}
 }
+
+/**
+ * fwd_rule_conflicts() - Test if two rules conflict with each other
+ * @a, @b:	Rules to test
+ */
+static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule *b)
+{
+	if (a->proto != b->proto)
+		/* Non-conflicting protocols */
+		return false;
+
+	if (!inany_matches(fwd_rule_addr(a), fwd_rule_addr(b)))
+		/* Non-conflicting addresses */
+		return false;
+
+	assert(a->first <= a->last && b->first <= b->last);
+	if (a->last < b->first || b->last < a->first)
+		/* Port ranges don't overlap */
+		return false;
+
+	return true;
+}
+
+/* fwd_rule_conflict_check() - Die with errir if rule conflicts with any in list
+ * @new:	New rule
+ * @rules:	Existing rules against which to test
+ * @count:	Number of rules in @rules
+ */
+void fwd_rule_conflict_check(const struct fwd_rule *new,
+			     const struct fwd_rule *rules, size_t count)
+{
+	unsigned i;
+
+	for (i = 0; i < count; i++) {
+		char newstr[FWD_RULE_STRLEN], rulestr[FWD_RULE_STRLEN];
+
+		if (!fwd_rule_conflicts(new, &rules[i]))
+			continue;
+
+		die("Forwarding configuration conflict: %s versus %s",
+		    fwd_rule_fmt(new, newstr, sizeof(newstr)),
+		    fwd_rule_fmt(&rules[i], rulestr, sizeof(rulestr)));
+	}
+}
diff --git a/fwd_rule.h b/fwd_rule.h
index e92efb6d..f852be39 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -52,5 +52,7 @@ struct fwd_rule {
 
 const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule);
 void fwd_rules_info(const struct fwd_rule *rules, size_t count);
+void fwd_rule_conflict_check(const struct fwd_rule *new,
+			     const struct fwd_rule *rules, size_t count);
 
 #endif /* FWD_RULE_H */
-- 
2.53.0

Re: [PATCH 10/18] fwd_rule: Move rule conflict checking from fwd_rule_add() to caller

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:22 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Amongst other checks, fwd_rule_add() checks that the newly added rule
> doesn't conflict with any existing rules.  However, unlike the other things
> we verify, this isn't really required for safe operation.  Rule conflicts
> are a useful thing for the user to know about, but the forwarding logic
> is perfectly sound with conflicting rules (the first one will win).
> 
> In order to support dynamic rule updates, we want fwd_rule_add() to become
> a more low-level function, only checking the things it really needs to.
> So, move rule conflict checking to its caller via new helpers in
> fwd_rule.c.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  conf.c     |  5 +++++
>  fwd.c      | 26 +-------------------------
>  fwd_rule.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  fwd_rule.h |  2 ++
>  4 files changed, 52 insertions(+), 25 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index c9ee8c59..a93837cc 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -205,13 +205,18 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
>  
>  			if (c->ifi4) {
>  				rulev.addr = inany_loopback4;
> +				fwd_rule_conflict_check(&rulev,
> +							fwd->rules, fwd->count);
>  				fwd_rule_add(fwd, &rulev);
>  			}
>  			if (c->ifi6) {
>  				rulev.addr = inany_loopback6;
> +				fwd_rule_conflict_check(&rulev,
> +							fwd->rules, fwd->count);
>  				fwd_rule_add(fwd, &rulev);
>  			}
>  		} else {
> +			fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
>  			fwd_rule_add(fwd, &rule);
>  		}
>  		base = i - 1;
> diff --git a/fwd.c b/fwd.c
> index c05107d1..c9637525 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -341,7 +341,7 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
>  	/* Flags which can be set from the caller */
>  	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
>  	unsigned num = (unsigned)new->last - new->first + 1;
> -	unsigned i, port;
> +	unsigned port;
>  
>  	assert(!(new->flags & ~allowed_flags));
>  	/* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */
> @@ -354,30 +354,6 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
>  	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks))
>  		die("Too many listening sockets");
>  
> -	/* Check for any conflicting entries */
> -	for (i = 0; i < fwd->count; i++) {
> -		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
> -		const struct fwd_rule *rule = &fwd->rules[i];
> -
> -		if (new->proto != rule->proto)
> -			/* Non-conflicting protocols */
> -			continue;
> -
> -		if (!inany_matches(fwd_rule_addr(new), fwd_rule_addr(rule)))
> -			/* Non-conflicting addresses */
> -			continue;
> -
> -		if (new->last < rule->first || rule->last < new->first)
> -			/* Port ranges don't overlap */
> -			continue;
> -
> -		die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u",
> -		    inany_ntop(fwd_rule_addr(new), newstr, sizeof(newstr)),
> -		    new->first, new->last,
> -		    inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)),
> -		    rule->first, rule->last);
> -	}
> -
>  	fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
>  	for (port = new->first; port <= new->last; port++)
>  		fwd->rulesocks[fwd->count][port - new->first] = -1;
> diff --git a/fwd_rule.c b/fwd_rule.c
> index abe9dfbf..4d5048f9 100644
> --- a/fwd_rule.c
> +++ b/fwd_rule.c
> @@ -87,3 +87,47 @@ void fwd_rules_info(const struct fwd_rule *rules, size_t count)
>  		info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
>  	}
>  }
> +
> +/**
> + * fwd_rule_conflicts() - Test if two rules conflict with each other
> + * @a, @b:	Rules to test
> + */
> +static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule *b)
> +{
> +	if (a->proto != b->proto)
> +		/* Non-conflicting protocols */
> +		return false;
> +
> +	if (!inany_matches(fwd_rule_addr(a), fwd_rule_addr(b)))
> +		/* Non-conflicting addresses */
> +		return false;
> +
> +	assert(a->first <= a->last && b->first <= b->last);
> +	if (a->last < b->first || b->last < a->first)
> +		/* Port ranges don't overlap */
> +		return false;
> +
> +	return true;
> +}
> +
> +/* fwd_rule_conflict_check() - Die with errir if rule conflicts with any in list

Nit: an errir happens only when Mimir (ask Jon) makes a mistake, which
is quite rare. :)

> + * @new:	New rule
> + * @rules:	Existing rules against which to test
> + * @count:	Number of rules in @rules
> + */
> +void fwd_rule_conflict_check(const struct fwd_rule *new,
> +			     const struct fwd_rule *rules, size_t count)
> +{
> +	unsigned i;
> +
> +	for (i = 0; i < count; i++) {
> +		char newstr[FWD_RULE_STRLEN], rulestr[FWD_RULE_STRLEN];
> +
> +		if (!fwd_rule_conflicts(new, &rules[i]))
> +			continue;
> +
> +		die("Forwarding configuration conflict: %s versus %s",
> +		    fwd_rule_fmt(new, newstr, sizeof(newstr)),
> +		    fwd_rule_fmt(&rules[i], rulestr, sizeof(rulestr)));
> +	}
> +}
> diff --git a/fwd_rule.h b/fwd_rule.h
> index e92efb6d..f852be39 100644
> --- a/fwd_rule.h
> +++ b/fwd_rule.h
> @@ -52,5 +52,7 @@ struct fwd_rule {
>  
>  const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule);
>  void fwd_rules_info(const struct fwd_rule *rules, size_t count);
> +void fwd_rule_conflict_check(const struct fwd_rule *new,
> +			     const struct fwd_rule *rules, size_t count);
>  
>  #endif /* FWD_RULE_H */

I reviewed only up to here so far, the rest will come in a bit.

I had a quick look at the whole series and it all looks good to me so
far but that wasn't quite a review.

Meanwhile, I noticed some warnings that strangely appear only during the
build of passt.avx2:

cc -Wall -Wextra -Wno-format-zero-length -Wformat-security -pedantic -std=c11 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -pie -fPIE -DPAGE_SIZE=4096 -DVERSION=\"2026_01_20.386b5f5-84-ge87c74f\" -DDUAL_STACK_SOCKETS=1 -DHAS_GETRANDOM -fstack-protector-strong -Ofast -mavx2 -ftree-vectorize -funroll-loops   \
	arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c -o passt.avx2 
In file included from util.h:22,
                 from ip.h:12,
                 from fwd_rule.h:16,
                 from fwd_rule.c:20:
fwd_rule.c: In function ‘fwd_rules_info’:
fwd_rule.c:86:22: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
   86 |                 info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
      |                      ^~~~~~~~
log.h:31:66: note: in definition of macro ‘info’
   31 | #define info(...)               logmsg(true, false, LOG_INFO,    __VA_ARGS__)
      |                                                                  ^~~~~~~~~~~
fwd_rule.c:86:27: note: format string is defined here
   86 |                 info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
      |                           ^~

...but I don't think I looked at the code changes causing them, yet (and
didn't bisect the series either). This is with gcc 13.3.0.

-- 
Stefano

Re: [PATCH 10/18] fwd_rule: Move rule conflict checking from fwd_rule_add() to caller

[David Gibson]

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

On Wed, Apr 08, 2026 at 01:14:50AM +0200, Stefano Brivio wrote:
> On Tue,  7 Apr 2026 13:16:22 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Amongst other checks, fwd_rule_add() checks that the newly added rule
> > doesn't conflict with any existing rules.  However, unlike the other things
> > we verify, this isn't really required for safe operation.  Rule conflicts
> > are a useful thing for the user to know about, but the forwarding logic
> > is perfectly sound with conflicting rules (the first one will win).
> > 
> > In order to support dynamic rule updates, we want fwd_rule_add() to become
> > a more low-level function, only checking the things it really needs to.
> > So, move rule conflict checking to its caller via new helpers in
> > fwd_rule.c.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  conf.c     |  5 +++++
> >  fwd.c      | 26 +-------------------------
> >  fwd_rule.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  fwd_rule.h |  2 ++
> >  4 files changed, 52 insertions(+), 25 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index c9ee8c59..a93837cc 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -205,13 +205,18 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
> >  
> >  			if (c->ifi4) {
> >  				rulev.addr = inany_loopback4;
> > +				fwd_rule_conflict_check(&rulev,
> > +							fwd->rules, fwd->count);
> >  				fwd_rule_add(fwd, &rulev);
> >  			}
> >  			if (c->ifi6) {
> >  				rulev.addr = inany_loopback6;
> > +				fwd_rule_conflict_check(&rulev,
> > +							fwd->rules, fwd->count);
> >  				fwd_rule_add(fwd, &rulev);
> >  			}
> >  		} else {
> > +			fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
> >  			fwd_rule_add(fwd, &rule);
> >  		}
> >  		base = i - 1;
> > diff --git a/fwd.c b/fwd.c
> > index c05107d1..c9637525 100644
> > --- a/fwd.c
> > +++ b/fwd.c
> > @@ -341,7 +341,7 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
> >  	/* Flags which can be set from the caller */
> >  	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
> >  	unsigned num = (unsigned)new->last - new->first + 1;
> > -	unsigned i, port;
> > +	unsigned port;
> >  
> >  	assert(!(new->flags & ~allowed_flags));
> >  	/* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */
> > @@ -354,30 +354,6 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
> >  	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks))
> >  		die("Too many listening sockets");
> >  
> > -	/* Check for any conflicting entries */
> > -	for (i = 0; i < fwd->count; i++) {
> > -		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
> > -		const struct fwd_rule *rule = &fwd->rules[i];
> > -
> > -		if (new->proto != rule->proto)
> > -			/* Non-conflicting protocols */
> > -			continue;
> > -
> > -		if (!inany_matches(fwd_rule_addr(new), fwd_rule_addr(rule)))
> > -			/* Non-conflicting addresses */
> > -			continue;
> > -
> > -		if (new->last < rule->first || rule->last < new->first)
> > -			/* Port ranges don't overlap */
> > -			continue;
> > -
> > -		die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u",
> > -		    inany_ntop(fwd_rule_addr(new), newstr, sizeof(newstr)),
> > -		    new->first, new->last,
> > -		    inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)),
> > -		    rule->first, rule->last);
> > -	}
> > -
> >  	fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
> >  	for (port = new->first; port <= new->last; port++)
> >  		fwd->rulesocks[fwd->count][port - new->first] = -1;
> > diff --git a/fwd_rule.c b/fwd_rule.c
> > index abe9dfbf..4d5048f9 100644
> > --- a/fwd_rule.c
> > +++ b/fwd_rule.c
> > @@ -87,3 +87,47 @@ void fwd_rules_info(const struct fwd_rule *rules, size_t count)
> >  		info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
> >  	}
> >  }
> > +
> > +/**
> > + * fwd_rule_conflicts() - Test if two rules conflict with each other
> > + * @a, @b:	Rules to test
> > + */
> > +static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule *b)
> > +{
> > +	if (a->proto != b->proto)
> > +		/* Non-conflicting protocols */
> > +		return false;
> > +
> > +	if (!inany_matches(fwd_rule_addr(a), fwd_rule_addr(b)))
> > +		/* Non-conflicting addresses */
> > +		return false;
> > +
> > +	assert(a->first <= a->last && b->first <= b->last);
> > +	if (a->last < b->first || b->last < a->first)
> > +		/* Port ranges don't overlap */
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +/* fwd_rule_conflict_check() - Die with errir if rule conflicts with any in list
> 
> Nit: an errir happens only when Mimir (ask Jon) makes a mistake, which
> is quite rare. :)

Fixed.  Also the incorrect formatting for the start of the comment
block.

> > + * @new:	New rule
> > + * @rules:	Existing rules against which to test
> > + * @count:	Number of rules in @rules
> > + */
> > +void fwd_rule_conflict_check(const struct fwd_rule *new,
> > +			     const struct fwd_rule *rules, size_t count)
> > +{
> > +	unsigned i;
> > +
> > +	for (i = 0; i < count; i++) {
> > +		char newstr[FWD_RULE_STRLEN], rulestr[FWD_RULE_STRLEN];
> > +
> > +		if (!fwd_rule_conflicts(new, &rules[i]))
> > +			continue;
> > +
> > +		die("Forwarding configuration conflict: %s versus %s",
> > +		    fwd_rule_fmt(new, newstr, sizeof(newstr)),
> > +		    fwd_rule_fmt(&rules[i], rulestr, sizeof(rulestr)));
> > +	}
> > +}
> > diff --git a/fwd_rule.h b/fwd_rule.h
> > index e92efb6d..f852be39 100644
> > --- a/fwd_rule.h
> > +++ b/fwd_rule.h
> > @@ -52,5 +52,7 @@ struct fwd_rule {
> >  
> >  const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule);
> >  void fwd_rules_info(const struct fwd_rule *rules, size_t count);
> > +void fwd_rule_conflict_check(const struct fwd_rule *new,
> > +			     const struct fwd_rule *rules, size_t count);
> >  
> >  #endif /* FWD_RULE_H */
> 
> I reviewed only up to here so far, the rest will come in a bit.
> 
> I had a quick look at the whole series and it all looks good to me so
> far but that wasn't quite a review.
> 
> Meanwhile, I noticed some warnings that strangely appear only during the
> build of passt.avx2:
> 
> cc -Wall -Wextra -Wno-format-zero-length -Wformat-security -pedantic -std=c11 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -pie -fPIE -DPAGE_SIZE=4096 -DVERSION=\"2026_01_20.386b5f5-84-ge87c74f\" -DDUAL_STACK_SOCKETS=1 -DHAS_GETRANDOM -fstack-protector-strong -Ofast -mavx2 -ftree-vectorize -funroll-loops   \
> 	arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c -o passt.avx2 
> In file included from util.h:22,
>                  from ip.h:12,
>                  from fwd_rule.h:16,
>                  from fwd_rule.c:20:
> fwd_rule.c: In function ‘fwd_rules_info’:
> fwd_rule.c:86:22: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
>    86 |                 info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
>       |                      ^~~~~~~~
> log.h:31:66: note: in definition of macro ‘info’
>    31 | #define info(...)               logmsg(true, false, LOG_INFO,    __VA_ARGS__)
>       |                                                                  ^~~~~~~~~~~
> fwd_rule.c:86:27: note: format string is defined here
>    86 |                 info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
>       |                           ^~
> 
> ...but I don't think I looked at the code changes causing them, yet (and
> didn't bisect the series either). This is with gcc 13.3.0.

Huh.  I don't see those those with gcc 15.2.1.  I _think_ it's a false
positive.  Investigating...

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

Re: [PATCH 10/18] fwd_rule: Move rule conflict checking from fwd_rule_add() to caller

[David Gibson]

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

On Wed, Apr 08, 2026 at 11:37:04AM +1000, David Gibson wrote:
> On Wed, Apr 08, 2026 at 01:14:50AM +0200, Stefano Brivio wrote:
> > On Tue,  7 Apr 2026 13:16:22 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
[snip]
> > Meanwhile, I noticed some warnings that strangely appear only during the
> > build of passt.avx2:
> > 
> > cc -Wall -Wextra -Wno-format-zero-length -Wformat-security -pedantic -std=c11 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -pie -fPIE -DPAGE_SIZE=4096 -DVERSION=\"2026_01_20.386b5f5-84-ge87c74f\" -DDUAL_STACK_SOCKETS=1 -DHAS_GETRANDOM -fstack-protector-strong -Ofast -mavx2 -ftree-vectorize -funroll-loops   \
> > 	arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c -o passt.avx2 
> > In file included from util.h:22,
> >                  from ip.h:12,
> >                  from fwd_rule.h:16,
> >                  from fwd_rule.c:20:
> > fwd_rule.c: In function ‘fwd_rules_info’:
> > fwd_rule.c:86:22: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
> >    86 |                 info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
> >       |                      ^~~~~~~~
> > log.h:31:66: note: in definition of macro ‘info’
> >    31 | #define info(...)               logmsg(true, false, LOG_INFO,    __VA_ARGS__)
> >       |                                                                  ^~~~~~~~~~~
> > fwd_rule.c:86:27: note: format string is defined here
> >    86 |                 info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
> >       |                           ^~
> > 
> > ...but I don't think I looked at the code changes causing them, yet (and
> > didn't bisect the series either). This is with gcc 13.3.0.
> 
> Huh.  I don't see those those with gcc 15.2.1.  I _think_ it's a false
> positive.  Investigating...

Ok, I think I have a workaround for this.  I reproduced the problem,
and unsurprisingly it's introduced in patch 7/18 that introduces the
new rule formatting helpers.

It appears to occur with gcc 12, 13 & 14, but not 15.  It's not the
AVX2 per-se that triggers it, but the fact we use -Ofast for the
passt.avx2 build - it appears to trigger if we go above -O2.  The gcc
man page does warn that high optimization levels might cause false
positives for this warning:

    -Wformat-overflow=level
        Warn about calls to formatted input/output functions such as
	"sprintf" and "vsprintf" that might overflow the destination
	buffer.  When the exact number of bytes written by a format
	directive cannot be determined at compile-time it is estimated
	based on heuristics that depend on the level argument and on
	optimization.  While enabling optimization will in most cases
	improve the accuracy of the warning, it may also result in
	false positives.

It's theoretically true that fwd_rule_fmt() can return NULL.  However
the buffer is sized so that shouldn't ever happen.  However gcc seems
to think it knows it *is* returning NULL, not just that it could.

I'm pretty sure my calculation for the max buffer size is correct.
Even if it's not, I'm pretty sure there is a gcc bug in play here:
I get the same warning if I replace the guts of fwd_rule_fmt() with an
snprintf() of a short fixed string to the output buffer, then check
the (now statically known) return value against the buffer size.

I _think_ the reason it's occurring here but not the many other places
we use a similar pattern (e.g. every inet_ntop() called as a printf
argument) is because here the caller is in the same translation unit
as fwd_rule_fmt(), so gcc thinks it can look into fwd_rule_fmt() and
reason about it, but it's reasoning wrong.

In fact, looking closer it seems the problem is triggered when this
function is inlined, which I guess it's trying to do on the higher
optimization levels.  In v2 I've added a ((noinline)) attribute if
__GNUC__ < 15, which appears to do the trick.

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

[PATCH 11/18] fwd: Improve error handling in fwd_rule_add()

[David Gibson]

fwd_rule_add() sanity checks the given rule, however all errors are fatal:
either they're assert()s in the case of things that callers should have
already verified, or die()s if we run out of space for the new rule.

This won't suffice any more when we allow rule updates from a configuraiton
client.  We don't want to trust the input we get from the client any more
than we have to.

Replace the assert()s and die()s with a return value.  Also include warn()s
so that the user gets a more specific idea of the problem in the logs or
stderr.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c     | 15 ++++++++++++---
 fwd.c      | 40 ++++++++++++++++++++++++++++++----------
 fwd.h      |  2 +-
 fwd_rule.c |  3 +--
 fwd_rule.h |  1 +
 5 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/conf.c b/conf.c
index a93837cc..cd30f2f8 100644
--- a/conf.c
+++ b/conf.c
@@ -157,6 +157,7 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 		.proto = proto,
 		.flags = flags,
 	};
+	char rulestr[FWD_RULE_STRLEN];
 	unsigned delta = to - first;
 	unsigned base, i;
 
@@ -207,20 +208,28 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 				rulev.addr = inany_loopback4;
 				fwd_rule_conflict_check(&rulev,
 							fwd->rules, fwd->count);
-				fwd_rule_add(fwd, &rulev);
+				if (fwd_rule_add(fwd, &rulev) < 0)
+					goto fail;
 			}
 			if (c->ifi6) {
 				rulev.addr = inany_loopback6;
 				fwd_rule_conflict_check(&rulev,
 							fwd->rules, fwd->count);
-				fwd_rule_add(fwd, &rulev);
+				if (fwd_rule_add(fwd, &rulev) < 0)
+					goto fail;
 			}
 		} else {
 			fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
-			fwd_rule_add(fwd, &rule);
+			if (fwd_rule_add(fwd, &rule) < 0)
+				goto fail;
 		}
 		base = i - 1;
 	}
+	return;
+
+fail:
+	die("Unable to add rule %s",
+	    fwd_rule_fmt(&rule, rulestr, sizeof(rulestr)));
 }
 
 /**
diff --git a/fwd.c b/fwd.c
index c9637525..cee2c7f6 100644
--- a/fwd.c
+++ b/fwd.c
@@ -335,24 +335,43 @@ void fwd_rule_init(struct ctx *c)
  * fwd_rule_add() - Validate and add a rule to a forwarding table
  * @fwd:	Table to add to
  * @new:	Rule to add
+ *
+ * Return: 0 on success, negative error code on failure
  */
-void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
+int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
 {
 	/* Flags which can be set from the caller */
 	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
 	unsigned num = (unsigned)new->last - new->first + 1;
 	unsigned port;
 
-	assert(!(new->flags & ~allowed_flags));
-	/* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */
-	assert(!(new->flags & FWD_DUAL_STACK_ANY) ||
-	       inany_equals(&new->addr, &inany_any6));
-	assert(new->first <= new->last);
+	if (new->first > new->last) {
+		warn("Forwarding rule has invalid port range %u-%u",
+		     new->first, new->last);
+		return -EINVAL;
+	}
+	if (new->flags & ~allowed_flags) {
+		warn("Forwarding rule has invalid flags 0x%x",
+		     new->flags & ~allowed_flags);
+		return -EINVAL;
+	}
+	if (new->flags & FWD_DUAL_STACK_ANY &&
+	    !inany_equals(&new->addr, &inany_any6)) {
+		warn("Dual stack forward must have unspecified address");
+		return -EINVAL;
+	}
 
-	if (fwd->count >= ARRAY_SIZE(fwd->rules))
-		die("Too many port forwarding ranges");
-	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks))
-		die("Too many listening sockets");
+	if (fwd->count >= ARRAY_SIZE(fwd->rules)) {
+		warn("Too many forwarding rules (maximum %u)",
+		     ARRAY_SIZE(fwd->rules));
+		return -ENOSPC;
+	}
+	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) {
+		warn(
+"Forwarding rules require too many listening sockets (maximum %u)",
+		     ARRAY_SIZE(fwd->socks));
+		return -ENOSPC;
+	}
 
 	fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
 	for (port = new->first; port <= new->last; port++)
@@ -360,6 +379,7 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
 
 	fwd->rules[fwd->count++] = *new;
 	fwd->sock_count += num;
+	return 0;
 }
 
 /**
diff --git a/fwd.h b/fwd.h
index 1d74cbd2..c56749e1 100644
--- a/fwd.h
+++ b/fwd.h
@@ -83,7 +83,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, const struct fwd_rule *new);
+int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new);
 const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
 				       const struct flowside *ini,
 				       uint8_t proto, int hint);
diff --git a/fwd_rule.c b/fwd_rule.c
index 4d5048f9..ef9308c2 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -39,8 +39,7 @@ const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule)
  * @dst:	Buffer to store output (should have FWD_RULE_STRLEN bytes)
  * @size:	Size of @dst
  */
-static const char *fwd_rule_fmt(const struct fwd_rule *rule,
-				char *dst, size_t size)
+const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size)
 {
 	const char *percent = *rule->ifname ? "%" : "";
 	const char *weak = "", *scan = "";
diff --git a/fwd_rule.h b/fwd_rule.h
index f852be39..8506a0c4 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -51,6 +51,7 @@ struct fwd_rule {
 	 + sizeof(" []%:-  =>  - (best effort) (auto-scan)"))
 
 const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule);
+const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size);
 void fwd_rules_info(const struct fwd_rule *rules, size_t count);
 void fwd_rule_conflict_check(const struct fwd_rule *new,
 			     const struct fwd_rule *rules, size_t count);
-- 
2.53.0

Re: [PATCH 11/18] fwd: Improve error handling in fwd_rule_add()

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:23 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> fwd_rule_add() sanity checks the given rule, however all errors are fatal:
> either they're assert()s in the case of things that callers should have
> already verified, or die()s if we run out of space for the new rule.
> 
> This won't suffice any more when we allow rule updates from a configuraiton

Nit, as I guess you'll respin: configuration.

-- 
Stefano

Re: [PATCH 11/18] fwd: Improve error handling in fwd_rule_add()

[David Gibson]

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

On Wed, Apr 08, 2026 at 11:40:07PM +0200, Stefano Brivio wrote:
> On Tue,  7 Apr 2026 13:16:23 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > fwd_rule_add() sanity checks the given rule, however all errors are fatal:
> > either they're assert()s in the case of things that callers should have
> > already verified, or die()s if we run out of space for the new rule.
> > 
> > This won't suffice any more when we allow rule updates from a configuraiton
> 
> Nit, as I guess you'll respin: configuration.

Oops, fixed.


> 
> -- 
> Stefano
> 

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

[PATCH 12/18] conf: Don't be strict about exclusivity of forwarding mode

[David Gibson]

Currently as well as building the forwarding tables, conf() maintains a
"forwarding mode" value for each protocol and direction.  This prevents,
for example "-t all" and "-t 40000" being given on the same command line.

This restriction predates the forwarding table and is no longer really
necessary.  Remove the restriction, instead doing our best to apply all the
given options simultaneously.

 * Many combinations previously disallowed will still be disallowed because
   of conflicts between the specific generated rules, e.g.
        -t all -t 8888
   (because -t all already listens on port 8888)
 * Some new combinations are now allowed and will work, e.g.
        -t all -t 40000
   because 'all' excludes ephemeral ports (which includes 40000 on default
   Linux configurations).
 * We remove our mode variables, but keep boolean variables to track if
   any forwarding config option has been given.  This is needed in order to
   correctly default to -t auto -T auto -u auto -U auto for pasta.
 * -[tTuU] none after any other rules is still considered an error.
   However -t none *before* other rules is allowed.  This is potentially
   confusing, but is awkward to avoid for the time being.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 97 ++++++++++++++++------------------------------------------
 1 file changed, 27 insertions(+), 70 deletions(-)

diff --git a/conf.c b/conf.c
index cd30f2f8..1e456361 100644
--- a/conf.c
+++ b/conf.c
@@ -232,22 +232,6 @@ fail:
 	    fwd_rule_fmt(&rule, rulestr, sizeof(rulestr)));
 }
 
-/**
- * enum fwd_mode - Overall forwarding mode for a direction and protocol
- * @FWD_MODE_UNSET	Initial value, not parsed/configured yet
- * @FWD_MODE_SPEC	Forward specified ports
- * @FWD_MODE_NONE	No forwarded ports
- * @FWD_MODE_AUTO	Automatic detection and forwarding based on bound ports
- * @FWD_MODE_ALL	Bind all free ports
- */
-enum fwd_mode {
-	FWD_MODE_UNSET = 0,
-	FWD_MODE_SPEC,
-	FWD_MODE_NONE,
-	FWD_MODE_AUTO,
-	FWD_MODE_ALL,
-};
-
 /**
  * conf_ports_spec() - Parse port range(s) specifier
  * @c:		Execution context
@@ -350,13 +334,12 @@ bad:
  * @optname:	Short option name, t, T, u, or U
  * @optarg:	Option argument (port specification)
  * @fwd:	Forwarding table to be updated
- * @mode:	Overall port forwarding mode (updated)
  */
 static void conf_ports(const struct ctx *c, char optname, const char *optarg,
-		       struct fwd_table *fwd, enum fwd_mode *mode)
+		       struct fwd_table *fwd)
 {
 	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
-	char buf[BUFSIZ], *spec, *ifname = NULL, *p;
+	char buf[BUFSIZ], *spec, *ifname = NULL;
 	uint8_t proto;
 
 	if (optname == 't' || optname == 'T')
@@ -367,10 +350,14 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		assert(0);
 
 	if (!strcmp(optarg, "none")) {
-		if (*mode)
-			goto mode_conflict;
+		unsigned i;
 
-		*mode = FWD_MODE_NONE;
+		for (i = 0; i < fwd->count; i++) {
+			if (fwd->rules[i].proto == proto) {
+				die("-%c none conflicts with previous options",
+					optname);
+			}
+		}
 		return;
 	}
 
@@ -380,14 +367,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		die("UDP port forwarding requested but UDP is disabled");
 
 	if (!strcmp(optarg, "auto")) {
-		if (*mode)
-			goto mode_conflict;
-
 		if (c->mode != MODE_PASTA)
 			die("'auto' port forwarding is only allowed for pasta");
 
-		*mode = FWD_MODE_AUTO;
-
 		conf_ports_range_except(c, optname, optarg, fwd,
 					proto, NULL, NULL,
 					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
@@ -398,11 +380,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 	if (!strcmp(optarg, "all")) {
 		uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
 
-		if (*mode)
-			goto mode_conflict;
-
-		*mode = FWD_MODE_ALL;
-
 		/* Exclude ephemeral ports */
 		fwd_port_map_ephemeral(exclude);
 
@@ -413,11 +390,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		return;
 	}
 
-	if (*mode > FWD_MODE_SPEC)
-		die("Specific ports cannot be specified together with all/none/auto");
-
-	*mode = FWD_MODE_SPEC;
-
 	strncpy(buf, optarg, sizeof(buf) - 1);
 
 	if ((spec = strchr(buf, '/'))) {
@@ -445,7 +417,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		if (ifname == buf + 1) {	/* Interface without address */
 			addr = NULL;
 		} else {
-			p = buf;
+			char *p = buf;
 
 			/* Allow square brackets for IPv4 too for convenience */
 			if (*p == '[' && p[strlen(p) - 1] == ']') {
@@ -482,10 +454,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		ifname = "lo";
 
 	conf_ports_spec(c, optname, optarg, fwd, proto, addr, ifname, spec);
-	return;
-
-mode_conflict:
-	die("Port forwarding mode '%s' conflicts with previous mode", optarg);
 }
 
 /**
@@ -1594,12 +1562,9 @@ void conf(struct ctx *c, int argc, char **argv)
 	};
 	const char *optstring = "+dqfel:hs:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:";
 	const char *logname = (c->mode == MODE_PASTA) ? "pasta" : "passt";
+	bool opt_t = false, opt_T = false, opt_u = false, opt_U = false;
 	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
 	bool copy_addrs_opt = false, copy_routes_opt = false;
-	enum fwd_mode tcp_out_mode = FWD_MODE_UNSET;
-	enum fwd_mode udp_out_mode = FWD_MODE_UNSET;
-	enum fwd_mode tcp_in_mode = FWD_MODE_UNSET;
-	enum fwd_mode udp_in_mode = FWD_MODE_UNSET;
 	bool v4_only = false, v6_only = false;
 	unsigned dns4_idx = 0, dns6_idx = 0;
 	unsigned long max_mtu = IP_MAX_MTU;
@@ -2204,17 +2169,17 @@ void conf(struct ctx *c, int argc, char **argv)
 		name = getopt_long(argc, argv, optstring, options, NULL);
 
 		if (name == 't') {
-			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
-				   &tcp_in_mode);
+			opt_t = true;
+			conf_ports(c, name, optarg, c->fwd[PIF_HOST]);
 		} else if (name == 'u') {
-			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
-				   &udp_in_mode);
+			opt_u = true;
+			conf_ports(c, name, optarg, c->fwd[PIF_HOST]);
 		} else if (name == 'T') {
-			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
-				   &tcp_out_mode);
+			opt_T = true;
+			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE]);
 		} else if (name == 'U') {
-			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
-				   &udp_out_mode);
+			opt_U = true;
+			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE]);
 		}
 	} while (name != -1);
 
@@ -2265,22 +2230,14 @@ void conf(struct ctx *c, int argc, char **argv)
 	}
 
 	if (c->mode == MODE_PASTA) {
-		if (!tcp_in_mode) {
-			conf_ports(c, 't', "auto",
-				   c->fwd[PIF_HOST], &tcp_in_mode);
-		}
-		if (!tcp_out_mode) {
-			conf_ports(c, 'T', "auto",
-				   c->fwd[PIF_SPLICE], &tcp_out_mode);
-		}
-		if (!udp_in_mode) {
-			conf_ports(c, 'u', "auto",
-				   c->fwd[PIF_HOST], &udp_in_mode);
-		}
-		if (!udp_out_mode) {
-			conf_ports(c, 'U', "auto",
-				   c->fwd[PIF_SPLICE], &udp_out_mode);
-		}
+		if (!opt_t)
+			conf_ports(c, 't', "auto", c->fwd[PIF_HOST]);
+		if (!opt_T)
+			conf_ports(c, 'T', "auto", c->fwd[PIF_SPLICE]);
+		if (!opt_u)
+			conf_ports(c, 'u', "auto", c->fwd[PIF_HOST]);
+		if (!opt_U)
+			conf_ports(c, 'U', "auto", c->fwd[PIF_SPLICE]);
 	}
 
 	if (!c->quiet)
-- 
2.53.0

Re: [PATCH 12/18] conf: Don't be strict about exclusivity of forwarding mode

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:24 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Currently as well as building the forwarding tables, conf() maintains a
> "forwarding mode" value for each protocol and direction.  This prevents,
> for example "-t all" and "-t 40000" being given on the same command line.
> 
> This restriction predates the forwarding table and is no longer really
> necessary.  Remove the restriction, instead doing our best to apply all the
> given options simultaneously.
> 
>  * Many combinations previously disallowed will still be disallowed because
>    of conflicts between the specific generated rules, e.g.
>         -t all -t 8888
>    (because -t all already listens on port 8888)
>  * Some new combinations are now allowed and will work, e.g.
>         -t all -t 40000
>    because 'all' excludes ephemeral ports (which includes 40000 on default
>    Linux configurations).

This is slightly confusing though:

$ ./pasta -t auto -t 31337
Forwarding configuration conflict: TCP [*]:31337  =>  31337  versus TCP [*]:1-32767  =>  1-32767  (best effort) (auto-scan)

but I don't see a practical way to "fix" it for the moment being, and
overall I'd say the new behaviour is better than the original one, so I
don't really care.

>  * We remove our mode variables, but keep boolean variables to track if
>    any forwarding config option has been given.  This is needed in order to
>    correctly default to -t auto -T auto -u auto -U auto for pasta.
>  * -[tTuU] none after any other rules is still considered an error.
>    However -t none *before* other rules is allowed.  This is potentially
>    confusing, but is awkward to avoid for the time being.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  conf.c | 97 ++++++++++++++++------------------------------------------
>  1 file changed, 27 insertions(+), 70 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index cd30f2f8..1e456361 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -232,22 +232,6 @@ fail:
>  	    fwd_rule_fmt(&rule, rulestr, sizeof(rulestr)));
>  }
>  
> -/**
> - * enum fwd_mode - Overall forwarding mode for a direction and protocol
> - * @FWD_MODE_UNSET	Initial value, not parsed/configured yet
> - * @FWD_MODE_SPEC	Forward specified ports
> - * @FWD_MODE_NONE	No forwarded ports
> - * @FWD_MODE_AUTO	Automatic detection and forwarding based on bound ports
> - * @FWD_MODE_ALL	Bind all free ports
> - */
> -enum fwd_mode {
> -	FWD_MODE_UNSET = 0,
> -	FWD_MODE_SPEC,
> -	FWD_MODE_NONE,
> -	FWD_MODE_AUTO,
> -	FWD_MODE_ALL,
> -};
> -
>  /**
>   * conf_ports_spec() - Parse port range(s) specifier
>   * @c:		Execution context
> @@ -350,13 +334,12 @@ bad:
>   * @optname:	Short option name, t, T, u, or U
>   * @optarg:	Option argument (port specification)
>   * @fwd:	Forwarding table to be updated
> - * @mode:	Overall port forwarding mode (updated)
>   */
>  static void conf_ports(const struct ctx *c, char optname, const char *optarg,
> -		       struct fwd_table *fwd, enum fwd_mode *mode)
> +		       struct fwd_table *fwd)
>  {
>  	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
> -	char buf[BUFSIZ], *spec, *ifname = NULL, *p;
> +	char buf[BUFSIZ], *spec, *ifname = NULL;
>  	uint8_t proto;
>  
>  	if (optname == 't' || optname == 'T')
> @@ -367,10 +350,14 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>  		assert(0);
>  
>  	if (!strcmp(optarg, "none")) {
> -		if (*mode)
> -			goto mode_conflict;
> +		unsigned i;
>  
> -		*mode = FWD_MODE_NONE;
> +		for (i = 0; i < fwd->count; i++) {
> +			if (fwd->rules[i].proto == proto) {
> +				die("-%c none conflicts with previous options",
> +					optname);
> +			}
> +		}
>  		return;
>  	}
>  
> @@ -380,14 +367,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>  		die("UDP port forwarding requested but UDP is disabled");
>  
>  	if (!strcmp(optarg, "auto")) {
> -		if (*mode)
> -			goto mode_conflict;
> -
>  		if (c->mode != MODE_PASTA)
>  			die("'auto' port forwarding is only allowed for pasta");
>  
> -		*mode = FWD_MODE_AUTO;
> -
>  		conf_ports_range_except(c, optname, optarg, fwd,
>  					proto, NULL, NULL,
>  					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> @@ -398,11 +380,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>  	if (!strcmp(optarg, "all")) {
>  		uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
>  
> -		if (*mode)
> -			goto mode_conflict;
> -
> -		*mode = FWD_MODE_ALL;
> -
>  		/* Exclude ephemeral ports */
>  		fwd_port_map_ephemeral(exclude);
>  
> @@ -413,11 +390,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>  		return;
>  	}
>  
> -	if (*mode > FWD_MODE_SPEC)
> -		die("Specific ports cannot be specified together with all/none/auto");
> -
> -	*mode = FWD_MODE_SPEC;
> -
>  	strncpy(buf, optarg, sizeof(buf) - 1);
>  
>  	if ((spec = strchr(buf, '/'))) {
> @@ -445,7 +417,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>  		if (ifname == buf + 1) {	/* Interface without address */
>  			addr = NULL;
>  		} else {
> -			p = buf;
> +			char *p = buf;
>  
>  			/* Allow square brackets for IPv4 too for convenience */
>  			if (*p == '[' && p[strlen(p) - 1] == ']') {
> @@ -482,10 +454,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>  		ifname = "lo";
>  
>  	conf_ports_spec(c, optname, optarg, fwd, proto, addr, ifname, spec);
> -	return;
> -
> -mode_conflict:
> -	die("Port forwarding mode '%s' conflicts with previous mode", optarg);
>  }
>  
>  /**
> @@ -1594,12 +1562,9 @@ void conf(struct ctx *c, int argc, char **argv)
>  	};
>  	const char *optstring = "+dqfel:hs:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:";
>  	const char *logname = (c->mode == MODE_PASTA) ? "pasta" : "passt";
> +	bool opt_t = false, opt_T = false, opt_u = false, opt_U = false;
>  	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
>  	bool copy_addrs_opt = false, copy_routes_opt = false;
> -	enum fwd_mode tcp_out_mode = FWD_MODE_UNSET;
> -	enum fwd_mode udp_out_mode = FWD_MODE_UNSET;
> -	enum fwd_mode tcp_in_mode = FWD_MODE_UNSET;
> -	enum fwd_mode udp_in_mode = FWD_MODE_UNSET;
>  	bool v4_only = false, v6_only = false;
>  	unsigned dns4_idx = 0, dns6_idx = 0;
>  	unsigned long max_mtu = IP_MAX_MTU;
> @@ -2204,17 +2169,17 @@ void conf(struct ctx *c, int argc, char **argv)
>  		name = getopt_long(argc, argv, optstring, options, NULL);
>  
>  		if (name == 't') {
> -			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
> -				   &tcp_in_mode);
> +			opt_t = true;
> +			conf_ports(c, name, optarg, c->fwd[PIF_HOST]);
>  		} else if (name == 'u') {
> -			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
> -				   &udp_in_mode);
> +			opt_u = true;
> +			conf_ports(c, name, optarg, c->fwd[PIF_HOST]);
>  		} else if (name == 'T') {
> -			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
> -				   &tcp_out_mode);
> +			opt_T = true;
> +			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE]);
>  		} else if (name == 'U') {
> -			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
> -				   &udp_out_mode);
> +			opt_U = true;
> +			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE]);
>  		}
>  	} while (name != -1);
>  
> @@ -2265,22 +2230,14 @@ void conf(struct ctx *c, int argc, char **argv)
>  	}
>  
>  	if (c->mode == MODE_PASTA) {
> -		if (!tcp_in_mode) {
> -			conf_ports(c, 't', "auto",
> -				   c->fwd[PIF_HOST], &tcp_in_mode);
> -		}
> -		if (!tcp_out_mode) {
> -			conf_ports(c, 'T', "auto",
> -				   c->fwd[PIF_SPLICE], &tcp_out_mode);
> -		}
> -		if (!udp_in_mode) {
> -			conf_ports(c, 'u', "auto",
> -				   c->fwd[PIF_HOST], &udp_in_mode);
> -		}
> -		if (!udp_out_mode) {
> -			conf_ports(c, 'U', "auto",
> -				   c->fwd[PIF_SPLICE], &udp_out_mode);
> -		}
> +		if (!opt_t)
> +			conf_ports(c, 't', "auto", c->fwd[PIF_HOST]);
> +		if (!opt_T)
> +			conf_ports(c, 'T', "auto", c->fwd[PIF_SPLICE]);
> +		if (!opt_u)
> +			conf_ports(c, 'u', "auto", c->fwd[PIF_HOST]);
> +		if (!opt_U)
> +			conf_ports(c, 'U', "auto", c->fwd[PIF_SPLICE]);
>  	}
>  
>  	if (!c->quiet)

-- 
Stefano

Re: [PATCH 12/18] conf: Don't be strict about exclusivity of forwarding mode

[David Gibson]

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

On Wed, Apr 08, 2026 at 11:40:16PM +0200, Stefano Brivio wrote:
> On Tue,  7 Apr 2026 13:16:24 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Currently as well as building the forwarding tables, conf() maintains a
> > "forwarding mode" value for each protocol and direction.  This prevents,
> > for example "-t all" and "-t 40000" being given on the same command line.
> > 
> > This restriction predates the forwarding table and is no longer really
> > necessary.  Remove the restriction, instead doing our best to apply all the
> > given options simultaneously.
> > 
> >  * Many combinations previously disallowed will still be disallowed because
> >    of conflicts between the specific generated rules, e.g.
> >         -t all -t 8888
> >    (because -t all already listens on port 8888)
> >  * Some new combinations are now allowed and will work, e.g.
> >         -t all -t 40000
> >    because 'all' excludes ephemeral ports (which includes 40000 on default
> >    Linux configurations).
> 
> This is slightly confusing though:
> 
> $ ./pasta -t auto -t 31337
> Forwarding configuration conflict: TCP [*]:31337  =>  31337  versus TCP [*]:1-32767  =>  1-32767  (best effort) (auto-scan)

You mean because the single port rule is redundant, but doesn't do
something different, so not strictly speaking conflicting?

> but I don't see a practical way to "fix" it for the moment being, and
> overall I'd say the new behaviour is better than the original one, so I
> don't really care.

Ok.

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

[PATCH 13/18] conf: Rework stepping through chunks of port specifiers

[David Gibson]

Port specifier strings are made up of ',' separated chunks.  Rework the
logic we use to step through the chunks.

Specifically, maintain a pointer to the end of each chunk as well as the
start.  This is not really used yet, but will be useful in future.

This also has side effect on semantics.  Previously an empty specifier (0
chunks) was not accepted.  Now it is, and will be treated as an "exclude
only" spec which excludes only ephemeral ports.  This seems a bit odd, and
I don't expect it to be (directly) used in practice.  However, it falls
naturally out of the existing semantics, and will combine well with some
upcoming changes.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/conf.c b/conf.c
index 1e456361..1ab1b071 100644
--- a/conf.c
+++ b/conf.c
@@ -65,21 +65,6 @@
 
 const char *pasta_default_ifn = "tap0";
 
-/**
- * next_chunk() - Return the next piece of a string delimited by a character
- * @s:		String to search
- * @c:		Delimiter character
- *
- * Return: if another @c is found in @s, returns a pointer to the
- *	   character *after* the delimiter, if no further @c is in @s,
- *	   return NULL
- */
-static const char *next_chunk(const char *s, char c)
-{
-	char *sep = strchr(s, c);
-	return sep ? sep + 1 : NULL;
-}
-
 /**
  * port_range() - Represents a non-empty range of ports
  * @first:	First port number in the range
@@ -232,6 +217,18 @@ fail:
 	    fwd_rule_fmt(&rule, rulestr, sizeof(rulestr)));
 }
 
+/*
+ * for_each_chunk - Step through delimited chunks of a string
+ * @p_:		Pointer to start of each chunk (updated)
+ * @ep_:	Pointer to end of each chunk (updated)
+ * @s_:		String to step through
+ * @sep_:	String of all allowed delimiters
+ */
+#define for_each_chunk(p_, ep_, s_, sep_)			\
+	for ((p_) = (s_);					\
+	     (ep_) = (p_) + strcspn((p_), (sep_)), *(p_);	\
+	     (p_) = *(ep_) ? (ep_) + 1 : (ep_))
+
 /**
  * conf_ports_spec() - Parse port range(s) specifier
  * @c:		Execution context
@@ -251,12 +248,11 @@ static void conf_ports_spec(const struct ctx *c,
 {
 	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
 	bool exclude_only = true;
-	const char *p;
+	const char *p, *ep;
 	unsigned i;
 
 	/* Mark all exclusions first, they might be given after base ranges */
-	p = spec;
-	do {
+	for_each_chunk(p, ep, spec, ",") {
 		struct port_range xrange;
 
 		if (*p != '~') {
@@ -273,7 +269,7 @@ static void conf_ports_spec(const struct ctx *c,
 
 		for (i = xrange.first; i <= xrange.last; i++)
 			bitmap_set(exclude, i);
-	} while ((p = next_chunk(p, ',')));
+	}
 
 	if (exclude_only) {
 		/* Exclude ephemeral ports */
@@ -287,8 +283,7 @@ static void conf_ports_spec(const struct ctx *c,
 	}
 
 	/* Now process base ranges, skipping exclusions */
-	p = spec;
-	do {
+	for_each_chunk(p, ep, spec, ",") {
 		struct port_range orig_range, mapped_range;
 
 		if (*p == '~')
@@ -321,7 +316,7 @@ static void conf_ports_spec(const struct ctx *c,
 					orig_range.first, orig_range.last,
 					exclude,
 					mapped_range.first, 0);
-	} while ((p = next_chunk(p, ',')));
+	}
 
 	return;
 bad:
-- 
2.53.0

Re: [PATCH 13/18] conf: Rework stepping through chunks of port specifiers

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:25 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Port specifier strings are made up of ',' separated chunks.  Rework the
> logic we use to step through the chunks.
> 
> Specifically, maintain a pointer to the end of each chunk as well as the
> start.  This is not really used yet, but will be useful in future.
> 
> This also has side effect on semantics.  Previously an empty specifier (0
> chunks) was not accepted.  Now it is, and will be treated as an "exclude
> only" spec which excludes only ephemeral ports.

Not really, because getopt_long() wants an argument for those options
anyway.

-- 
Stefano

Re: [PATCH 13/18] conf: Rework stepping through chunks of port specifiers

[David Gibson]

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

On Wed, Apr 08, 2026 at 11:40:22PM +0200, Stefano Brivio wrote:
> On Tue,  7 Apr 2026 13:16:25 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Port specifier strings are made up of ',' separated chunks.  Rework the
> > logic we use to step through the chunks.
> > 
> > Specifically, maintain a pointer to the end of each chunk as well as the
> > start.  This is not really used yet, but will be useful in future.
> > 
> > This also has side effect on semantics.  Previously an empty specifier (0
> > chunks) was not accepted.  Now it is, and will be treated as an "exclude
> > only" spec which excludes only ephemeral ports.
> 
> Not really, because getopt_long() wants an argument for those options
> anyway.

This allows an *empty* specifier, not a missing one.  i.e.
	$ pasta -t '' -u ''

That's a kind of weird thing to do, but it does work after this patch.

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

[PATCH 14/18] conf: Rework checking for garbage after a range

[David Gibson]

After parsing port ranges conf_ports_spec() checks if we've reached a
chunk delimiter (',') to verify that there isn't extra garbage there.
Rework how we do this to use the recently introduces chunk-end pointer.
This has two advantages:

1) Small, but practical: we don't need to repeat what the valid delimiters
   are, that's already handled in the chunk splitting code.

2) Large, if theoretical: this will also given an error if port parsing
   overruns a chunk boundary.  We don't really expect that to happen, but
   it would give very confusing behaviour if it did.  strtoul(3), on which
   parse_port_range() is based does say it may accept thousands separators
   based on locale which means can't entirely be sure it will only accept
   strings of digits.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/conf.c b/conf.c
index 1ab1b071..51612047 100644
--- a/conf.c
+++ b/conf.c
@@ -264,7 +264,7 @@ static void conf_ports_spec(const struct ctx *c,
 
 		if (parse_port_range(p, &p, &xrange))
 			goto bad;
-		if ((*p != '\0')  && (*p != ',')) /* Garbage after the range */
+		if (p != ep) /* Garbage after the range */
 			goto bad;
 
 		for (i = xrange.first; i <= xrange.last; i++)
@@ -303,7 +303,7 @@ static void conf_ports_spec(const struct ctx *c,
 			mapped_range = orig_range;
 		}
 
-		if ((*p != '\0')  && (*p != ',')) /* Garbage after the ranges */
+		if (p != ep) /* Garbage after the ranges */
 			goto bad;
 
 		if (orig_range.first == 0) {
-- 
2.53.0

Re: [PATCH 14/18] conf: Rework checking for garbage after a range

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:26 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> After parsing port ranges conf_ports_spec() checks if we've reached a
> chunk delimiter (',') to verify that there isn't extra garbage there.
> Rework how we do this to use the recently introduces chunk-end pointer.

Nit: introduced

> This has two advantages:
> 
> 1) Small, but practical: we don't need to repeat what the valid delimiters
>    are, that's already handled in the chunk splitting code.
> 
> 2) Large, if theoretical: this will also given an error if port parsing
>    overruns a chunk boundary.  We don't really expect that to happen, but
>    it would give very confusing behaviour if it did.  strtoul(3), on which
>    parse_port_range() is based does say it may accept thousands separators
>    based on locale which means can't entirely be sure it will only accept

Nit: we can't

>    strings of digits.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  conf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 1ab1b071..51612047 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -264,7 +264,7 @@ static void conf_ports_spec(const struct ctx *c,
>  
>  		if (parse_port_range(p, &p, &xrange))
>  			goto bad;
> -		if ((*p != '\0')  && (*p != ',')) /* Garbage after the range */
> +		if (p != ep) /* Garbage after the range */
>  			goto bad;
>  
>  		for (i = xrange.first; i <= xrange.last; i++)
> @@ -303,7 +303,7 @@ static void conf_ports_spec(const struct ctx *c,
>  			mapped_range = orig_range;
>  		}
>  
> -		if ((*p != '\0')  && (*p != ',')) /* Garbage after the ranges */
> +		if (p != ep) /* Garbage after the ranges */
>  			goto bad;
>  
>  		if (orig_range.first == 0) {

-- 
Stefano

Re: [PATCH 14/18] conf: Rework checking for garbage after a range

[David Gibson]

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

On Wed, Apr 08, 2026 at 11:40:27PM +0200, Stefano Brivio wrote:
> On Tue,  7 Apr 2026 13:16:26 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > After parsing port ranges conf_ports_spec() checks if we've reached a
> > chunk delimiter (',') to verify that there isn't extra garbage there.
> > Rework how we do this to use the recently introduces chunk-end pointer.
> 
> Nit: introduced
> 
> > This has two advantages:
> > 
> > 1) Small, but practical: we don't need to repeat what the valid delimiters
> >    are, that's already handled in the chunk splitting code.
> > 
> > 2) Large, if theoretical: this will also given an error if port parsing
> >    overruns a chunk boundary.  We don't really expect that to happen, but
> >    it would give very confusing behaviour if it did.  strtoul(3), on which
> >    parse_port_range() is based does say it may accept thousands separators
> >    based on locale which means can't entirely be sure it will only accept
> 
> Nit: we can't

Both fixed.  Plus a few more errors fixed and some edits for brevity.

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

[PATCH 15/18] conf: Move "all" handling to port specifier

[David Gibson]

Currently -[tTuU] all is handled separately in conf_ports() before calling
conf_ports_spec().  Earlier changes mean we can now move this handling to
conf_ports_spec().  This makes the code slightly simpler, but more
importantly it allows some useful combinations we couldn't previously do,
such as
	-t 127.0.0.1/all
or
	-u %eth2/all

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/conf.c b/conf.c
index 51612047..fcc75d25 100644
--- a/conf.c
+++ b/conf.c
@@ -251,6 +251,11 @@ static void conf_ports_spec(const struct ctx *c,
 	const char *p, *ep;
 	unsigned i;
 
+	if (!strcmp(spec, "all")) {
+		/* Treat "all" as equivalent to "": all non-ephemeral ports */
+		spec = "";
+	}
+
 	/* Mark all exclusions first, they might be given after base ranges */
 	for_each_chunk(p, ep, spec, ",") {
 		struct port_range xrange;
@@ -372,19 +377,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		return;
 	}
 
-	if (!strcmp(optarg, "all")) {
-		uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
-
-		/* Exclude ephemeral ports */
-		fwd_port_map_ephemeral(exclude);
-
-		conf_ports_range_except(c, optname, optarg, fwd,
-					proto, NULL, NULL,
-					1, NUM_PORTS - 1, exclude,
-					1, FWD_WEAK);
-		return;
-	}
-
 	strncpy(buf, optarg, sizeof(buf) - 1);
 
 	if ((spec = strchr(buf, '/'))) {
-- 
2.53.0

Re: [PATCH 15/18] conf: Move "all" handling to port specifier

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:27 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Currently -[tTuU] all is handled separately in conf_ports() before calling
> conf_ports_spec().  Earlier changes mean we can now move this handling to
> conf_ports_spec().  This makes the code slightly simpler, but more
> importantly it allows some useful combinations we couldn't previously do,
> such as
> 	-t 127.0.0.1/all
> or
> 	-u %eth2/all

It would probably be good to update the man page at some point with
this (we can also leave it undocumented for a while though, as it
doesn't break any existing functionality).

> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  conf.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 51612047..fcc75d25 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -251,6 +251,11 @@ static void conf_ports_spec(const struct ctx *c,
>  	const char *p, *ep;
>  	unsigned i;
>  
> +	if (!strcmp(spec, "all")) {
> +		/* Treat "all" as equivalent to "": all non-ephemeral ports */
> +		spec = "";
> +	}
> +
>  	/* Mark all exclusions first, they might be given after base ranges */
>  	for_each_chunk(p, ep, spec, ",") {
>  		struct port_range xrange;
> @@ -372,19 +377,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>  		return;
>  	}
>  
> -	if (!strcmp(optarg, "all")) {
> -		uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
> -
> -		/* Exclude ephemeral ports */
> -		fwd_port_map_ephemeral(exclude);
> -
> -		conf_ports_range_except(c, optname, optarg, fwd,
> -					proto, NULL, NULL,
> -					1, NUM_PORTS - 1, exclude,
> -					1, FWD_WEAK);
> -		return;
> -	}
> -
>  	strncpy(buf, optarg, sizeof(buf) - 1);
>  
>  	if ((spec = strchr(buf, '/'))) {

-- 
Stefano

[PATCH 16/18] conf: Allow user-specified auto-scanned port forwarding ranges

[David Gibson]

The forwarding table now allows for arbitrary port ranges to be marked as
FWD_SCAN, meaning we don't open sockets for every port, but only those we
scan as listening on the target side.  However, there's currently no way
to create such rules, except -[tTuU] auto which always scans every port
with an unspecified listening address and interface.

Allow user-specified "auto" ranges by moving the parsing of the "auto"
keyword from conf_ports(), to conf_ports_spec() as part of the port
specified.  "auto" can be combined freely with other port ranges, e.g.
    -t 127.0.0.1/auto
    -u %lo/5000-7000,auto
    -T auto,12345
    -U auto,~1-9000

Note that any address and interface given only affects where the automatic
forwards listen, not what addresses we consider when scanning.  That is,
if the target side is listening on *any* address, we will create a forward
on the specified address.

Link: https://bugs.passt.top/show_bug.cgi?id=180

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 66 ++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 18 deletions(-)

diff --git a/conf.c b/conf.c
index fcc75d25..86c30c7f 100644
--- a/conf.c
+++ b/conf.c
@@ -13,6 +13,7 @@
  */
 
 #include <arpa/inet.h>
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <getopt.h>
@@ -112,6 +113,28 @@ static int parse_port_range(const char *s, const char **endptr,
 	return 0;
 }
 
+/**
+ * parse_keyword() - Parse a literal keyword
+ * @s:		String to parse
+ * @endptr:	Update to the character after the keyword
+ * @kw:		Keyword to accept
+ *
+ * Return: 0, if @s starts with @kw, -EINVAL if it does not
+ */
+static int parse_keyword(const char *s, const char **endptr, const char *kw)
+{
+	size_t len = strlen(kw);
+
+	if (strlen(s) < len)
+		return -EINVAL;
+
+	if (memcmp(s, kw, len))
+		return -EINVAL;
+
+	*endptr = s + len;
+	return 0;
+}
+
 /**
  * conf_ports_range_except() - Set up forwarding for a range of ports minus a
  *                             bitmap of exclusions
@@ -249,6 +272,7 @@ static void conf_ports_spec(const struct ctx *c,
 	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
 	bool exclude_only = true;
 	const char *p, *ep;
+	uint8_t flags = 0;
 	unsigned i;
 
 	if (!strcmp(spec, "all")) {
@@ -256,15 +280,32 @@ static void conf_ports_spec(const struct ctx *c,
 		spec = "";
 	}
 
-	/* Mark all exclusions first, they might be given after base ranges */
+	/* Parse excluded ranges and "auto" in the first pass */
 	for_each_chunk(p, ep, spec, ",") {
 		struct port_range xrange;
 
-		if (*p != '~') {
-			/* Not an exclude range, parse later */
+		if (isdigit(*p))  {
+			/* Include range, parse later */
 			exclude_only = false;
 			continue;
 		}
+
+		if (parse_keyword(p, &p, "auto") == 0) {
+			if (p != ep) /* Garbage after the keyword */
+				goto bad;
+
+			if (c->mode != MODE_PASTA) {
+				die(
+"'auto' port forwarding is only allowed for pasta");
+			}
+
+			flags |= FWD_SCAN;
+			continue;
+		}
+
+		/* Should be an exclude range */
+		if (*p != '~')
+			goto bad;
 		p++;
 
 		if (parse_port_range(p, &p, &xrange))
@@ -283,7 +324,7 @@ static void conf_ports_spec(const struct ctx *c,
 		conf_ports_range_except(c, optname, optarg, fwd,
 					proto, addr, ifname,
 					1, NUM_PORTS - 1, exclude,
-					1, FWD_WEAK);
+					1, flags | FWD_WEAK);
 		return;
 	}
 
@@ -291,8 +332,8 @@ static void conf_ports_spec(const struct ctx *c,
 	for_each_chunk(p, ep, spec, ",") {
 		struct port_range orig_range, mapped_range;
 
-		if (*p == '~')
-			/* Exclude range, already parsed */
+		if (!isdigit(*p))
+			/* Already parsed */
 			continue;
 
 		if (parse_port_range(p, &p, &orig_range))
@@ -320,7 +361,7 @@ static void conf_ports_spec(const struct ctx *c,
 					proto, addr, ifname,
 					orig_range.first, orig_range.last,
 					exclude,
-					mapped_range.first, 0);
+					mapped_range.first, flags);
 	}
 
 	return;
@@ -366,17 +407,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 	if (proto == IPPROTO_UDP && c->no_udp)
 		die("UDP port forwarding requested but UDP is disabled");
 
-	if (!strcmp(optarg, "auto")) {
-		if (c->mode != MODE_PASTA)
-			die("'auto' port forwarding is only allowed for pasta");
-
-		conf_ports_range_except(c, optname, optarg, fwd,
-					proto, NULL, NULL,
-					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
-
-		return;
-	}
-
 	strncpy(buf, optarg, sizeof(buf) - 1);
 
 	if ((spec = strchr(buf, '/'))) {
-- 
2.53.0

Re: [PATCH 16/18] conf: Allow user-specified auto-scanned port forwarding ranges

[Stefano Brivio]

On Tue,  7 Apr 2026 13:16:28 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The forwarding table now allows for arbitrary port ranges to be marked as
> FWD_SCAN, meaning we don't open sockets for every port, but only those we
> scan as listening on the target side.  However, there's currently no way
> to create such rules, except -[tTuU] auto which always scans every port
> with an unspecified listening address and interface.
> 
> Allow user-specified "auto" ranges by moving the parsing of the "auto"
> keyword from conf_ports(), to conf_ports_spec() as part of the port
> specified.  "auto" can be combined freely with other port ranges, e.g.
>     -t 127.0.0.1/auto
>     -u %lo/5000-7000,auto
>     -T auto,12345
>     -U auto,~1-9000
> 
> Note that any address and interface given only affects where the automatic
> forwards listen, not what addresses we consider when scanning.  That is,
> if the target side is listening on *any* address, we will create a forward
> on the specified address.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=180
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  conf.c | 66 ++++++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 48 insertions(+), 18 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index fcc75d25..86c30c7f 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <arpa/inet.h>
> +#include <ctype.h>
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <getopt.h>
> @@ -112,6 +113,28 @@ static int parse_port_range(const char *s, const char **endptr,
>  	return 0;
>  }
>  
> +/**
> + * parse_keyword() - Parse a literal keyword

Here, "parse" sounds overly generic. If I understand this correctly,
it's a strstr() / memmem() implementation with the added 'endptr'
functionality, so maybe... "Find the end of a substring"?

> + * @s:		String to parse
> + * @endptr:	Update to the character after the keyword
> + * @kw:		Keyword to accept
> + *
> + * Return: 0, if @s starts with @kw, -EINVAL if it does not
> + */
> +static int parse_keyword(const char *s, const char **endptr, const char *kw)
> +{
> +	size_t len = strlen(kw);
> +
> +	if (strlen(s) < len)
> +		return -EINVAL;
> +
> +	if (memcmp(s, kw, len))
> +		return -EINVAL;
> +
> +	*endptr = s + len;
> +	return 0;
> +}
> +
>  /**
>   * conf_ports_range_except() - Set up forwarding for a range of ports minus a
>   *                             bitmap of exclusions
> @@ -249,6 +272,7 @@ static void conf_ports_spec(const struct ctx *c,
>  	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
>  	bool exclude_only = true;
>  	const char *p, *ep;
> +	uint8_t flags = 0;
>  	unsigned i;
>  
>  	if (!strcmp(spec, "all")) {
> @@ -256,15 +280,32 @@ static void conf_ports_spec(const struct ctx *c,
>  		spec = "";
>  	}
>  
> -	/* Mark all exclusions first, they might be given after base ranges */
> +	/* Parse excluded ranges and "auto" in the first pass */
>  	for_each_chunk(p, ep, spec, ",") {
>  		struct port_range xrange;
>  
> -		if (*p != '~') {
> -			/* Not an exclude range, parse later */
> +		if (isdigit(*p))  {
> +			/* Include range, parse later */
>  			exclude_only = false;
>  			continue;
>  		}
> +
> +		if (parse_keyword(p, &p, "auto") == 0) {
> +			if (p != ep) /* Garbage after the keyword */
> +				goto bad;
> +
> +			if (c->mode != MODE_PASTA) {
> +				die(
> +"'auto' port forwarding is only allowed for pasta");
> +			}
> +
> +			flags |= FWD_SCAN;
> +			continue;
> +		}
> +
> +		/* Should be an exclude range */
> +		if (*p != '~')
> +			goto bad;
>  		p++;
>  
>  		if (parse_port_range(p, &p, &xrange))
> @@ -283,7 +324,7 @@ static void conf_ports_spec(const struct ctx *c,
>  		conf_ports_range_except(c, optname, optarg, fwd,
>  					proto, addr, ifname,
>  					1, NUM_PORTS - 1, exclude,
> -					1, FWD_WEAK);
> +					1, flags | FWD_WEAK);
>  		return;
>  	}
>  
> @@ -291,8 +332,8 @@ static void conf_ports_spec(const struct ctx *c,
>  	for_each_chunk(p, ep, spec, ",") {
>  		struct port_range orig_range, mapped_range;
>  
> -		if (*p == '~')
> -			/* Exclude range, already parsed */
> +		if (!isdigit(*p))
> +			/* Already parsed */
>  			continue;
>  
>  		if (parse_port_range(p, &p, &orig_range))
> @@ -320,7 +361,7 @@ static void conf_ports_spec(const struct ctx *c,
>  					proto, addr, ifname,
>  					orig_range.first, orig_range.last,
>  					exclude,
> -					mapped_range.first, 0);
> +					mapped_range.first, flags);
>  	}
>  
>  	return;
> @@ -366,17 +407,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
>  	if (proto == IPPROTO_UDP && c->no_udp)
>  		die("UDP port forwarding requested but UDP is disabled");
>  
> -	if (!strcmp(optarg, "auto")) {
> -		if (c->mode != MODE_PASTA)
> -			die("'auto' port forwarding is only allowed for pasta");
> -
> -		conf_ports_range_except(c, optname, optarg, fwd,
> -					proto, NULL, NULL,
> -					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> -
> -		return;
> -	}
> -
>  	strncpy(buf, optarg, sizeof(buf) - 1);
>  
>  	if ((spec = strchr(buf, '/'))) {

The rest of the series looks good to me!

-- 
Stefano

[PATCH 17/18] conf: Move SO_BINDTODEVICE workaround to conf_ports()

[David Gibson]

For historical reasons we apply our workaround for -[TU] handling when
SO_BINDTODEVICE is unavailable inside conf_ports_range_except().  We've
now removed the reasons it had to be there, so it can move to conf_ports(),
the caller's caller.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 77 ++++++++++++++++++++++------------------------------------
 1 file changed, 29 insertions(+), 48 deletions(-)

diff --git a/conf.c b/conf.c
index 86c30c7f..de262ef5 100644
--- a/conf.c
+++ b/conf.c
@@ -138,9 +138,6 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw)
 /**
  * conf_ports_range_except() - Set up forwarding for a range of ports minus a
  *                             bitmap of exclusions
- * @c:		Execution context
- * @optname:	Short option name, t, T, u, or U
- * @optarg:	Option argument (port specification)
  * @fwd:	Forwarding table to be updated
  * @proto:	Protocol to forward
  * @addr:	Listening address
@@ -151,9 +148,8 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw)
  * @to:		Port to translate @first to when forwarding
  * @flags:	Flags for forwarding entries
  */
-static void conf_ports_range_except(const struct ctx *c, char optname,
-				    const char *optarg, struct fwd_table *fwd,
-				    uint8_t proto, const union inany_addr *addr,
+static void conf_ports_range_except(struct fwd_table *fwd, uint8_t proto,
+				    const union inany_addr *addr,
 				    const char *ifname,
 				    uint16_t first, uint16_t last,
 				    const uint8_t *exclude, uint16_t to,
@@ -195,42 +191,10 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 		rule.last = i - 1;
 		rule.to = base + delta;
 
-		if ((optname == 'T' || optname == 'U') && c->no_bindtodevice) {
-			/* FIXME: Once the fwd bitmaps are removed, move this
-			 * workaround to the caller
-			 */
-			struct fwd_rule rulev = {
-				.ifname = { 0 },
-				.flags = flags,
-				.first = base,
-				.last = i - 1,
-				.to = base + delta,
-			};
-
-			assert(!addr && ifname && !strcmp(ifname, "lo"));
-			warn(
-"SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'",
-			     optname, optarg);
+		fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
+		if (fwd_rule_add(fwd, &rule) < 0)
+			goto fail;
 
-			if (c->ifi4) {
-				rulev.addr = inany_loopback4;
-				fwd_rule_conflict_check(&rulev,
-							fwd->rules, fwd->count);
-				if (fwd_rule_add(fwd, &rulev) < 0)
-					goto fail;
-			}
-			if (c->ifi6) {
-				rulev.addr = inany_loopback6;
-				fwd_rule_conflict_check(&rulev,
-							fwd->rules, fwd->count);
-				if (fwd_rule_add(fwd, &rulev) < 0)
-					goto fail;
-			}
-		} else {
-			fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
-			if (fwd_rule_add(fwd, &rule) < 0)
-				goto fail;
-		}
 		base = i - 1;
 	}
 	return;
@@ -321,8 +285,7 @@ static void conf_ports_spec(const struct ctx *c,
 		/* Exclude ephemeral ports */
 		fwd_port_map_ephemeral(exclude);
 
-		conf_ports_range_except(c, optname, optarg, fwd,
-					proto, addr, ifname,
+		conf_ports_range_except(fwd, proto, addr, ifname,
 					1, NUM_PORTS - 1, exclude,
 					1, flags | FWD_WEAK);
 		return;
@@ -357,8 +320,7 @@ static void conf_ports_spec(const struct ctx *c,
 			    optname, optarg);
 		}
 
-		conf_ports_range_except(c, optname, optarg, fwd,
-					proto, addr, ifname,
+		conf_ports_range_except(fwd, proto, addr, ifname,
 					orig_range.first, orig_range.last,
 					exclude,
 					mapped_range.first, flags);
@@ -461,14 +423,33 @@ 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) {
+			warn(
+"SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'",
+			     optname, optarg);
+
+			if (c->ifi4) {
+				conf_ports_spec(c, optname, optarg, fwd, proto,
+						&inany_loopback4, NULL, spec);
+			}
+			if (c->ifi6) {
+				conf_ports_spec(c, optname, optarg, fwd, proto,
+						&inany_loopback6, NULL, spec);
+			}
+			return;
+		}
+
+		ifname = "lo";
+	}
+
 	if (ifname && c->no_bindtodevice) {
 		die(
 "Device binding for '-%c %s' unsupported (requires kernel 5.7+)",
 		    optname, optarg);
 	}
-	/* Outbound forwards come from guest loopback */
-	if ((optname == 'T' || optname == 'U') && !ifname)
-		ifname = "lo";
 
 	conf_ports_spec(c, optname, optarg, fwd, proto, addr, ifname, spec);
 }
-- 
2.53.0

[PATCH 18/18] conf: Don't pass raw commandline argument to conf_ports_spec()

[David Gibson]

We only use the optname and optarg parameters for printing error messages,
and they're not even particularly necessary there.  Remove them.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/conf.c b/conf.c
index de262ef5..4c4d9c2f 100644
--- a/conf.c
+++ b/conf.c
@@ -219,8 +219,6 @@ fail:
 /**
  * conf_ports_spec() - Parse port range(s) specifier
  * @c:		Execution context
- * @optname:	Short option name, t, T, u, or U
- * @optarg:	Option argument (port specification)
  * @fwd:	Forwarding table to be updated
  * @proto:	Protocol to forward
  * @addr:	Listening address for forwarding
@@ -228,7 +226,6 @@ fail:
  * @spec:	Port range(s) specifier
  */
 static void conf_ports_spec(const struct ctx *c,
-			    char optname, const char *optarg,
 			    struct fwd_table *fwd, uint8_t proto,
 			    const union inany_addr *addr, const char *ifname,
 			    const char *spec)
@@ -316,8 +313,7 @@ static void conf_ports_spec(const struct ctx *c,
 			goto bad;
 
 		if (orig_range.first == 0) {
-			die("Can't forward port 0 for option '-%c %s'",
-			    optname, optarg);
+			die("Can't forward port 0 included in '%s'", spec);
 		}
 
 		conf_ports_range_except(fwd, proto, addr, ifname,
@@ -328,7 +324,7 @@ static void conf_ports_spec(const struct ctx *c,
 
 	return;
 bad:
-	die("Invalid port specifier %s", optarg);
+	die("Invalid port specifier '%s'", spec);
 }
 
 /**
@@ -432,11 +428,11 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 			     optname, optarg);
 
 			if (c->ifi4) {
-				conf_ports_spec(c, optname, optarg, fwd, proto,
+				conf_ports_spec(c, fwd, proto,
 						&inany_loopback4, NULL, spec);
 			}
 			if (c->ifi6) {
-				conf_ports_spec(c, optname, optarg, fwd, proto,
+				conf_ports_spec(c, fwd, proto,
 						&inany_loopback6, NULL, spec);
 			}
 			return;
@@ -451,7 +447,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		    optname, optarg);
 	}
 
-	conf_ports_spec(c, optname, optarg, fwd, proto, addr, ifname, spec);
+	conf_ports_spec(c, fwd, proto, addr, ifname, spec);
 }
 
 /**
-- 
2.53.0