[PATCH v2 00/23] Rework forwarding option parsing

[PATCH v2 00/23] 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.

v2:
 * Assorted minor changes based on Stefano's review, including
   * Explicitly state "guest or namespace" in the manpage
   * Clearer description of @rulesocks field
 * Worked around a gcc < 15 bug causing a false positive warning
 * Update man page and usage() for new capabilities
 * Additional patches moving rule parsing out of conf.c

David Gibson (23):
  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
  doc: Rework man page description of port specifiers
  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()
  fwd, conf: Add capabilities bits to each forwarding table
  conf, fwd: Stricter rule checking in fwd_rule_add()
  fwd_rule: Move ephemeral port probing to fwd_rule.c
  fwd, conf: Move rule parsing code to fwd_rule.[ch]

 Makefile   |  10 +-
 conf.c     | 524 ++++++-----------------------------------
 fwd.c      | 261 +++------------------
 fwd.h      |  46 ----
 fwd_rule.c | 676 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fwd_rule.h |  59 +++++
 passt.1    | 288 ++++++++++-------------
 7 files changed, 973 insertions(+), 891 deletions(-)
 create mode 100644 fwd_rule.c

-- 
2.53.0

[PATCH v2 01/23] 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 v2 02/23] 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

[PATCH v2 03/23] 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 v2 04/23] 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  |  72 ++++++++----------
 passt.1 | 230 ++++++++++++++++++--------------------------------------
 2 files changed, 105 insertions(+), 197 deletions(-)

diff --git a/conf.c b/conf.c
index f3b36bb6..64ee8f00 100644
--- a/conf.c
+++ b/conf.c
@@ -888,6 +888,8 @@ static void conf_ip6_local(struct ip6_ctx *ip6)
  */
 static void usage(const char *name, FILE *f, int status)
 {
+	const char *guest, *fwd_default;
+
 	if (strstr(name, "pasta")) {
 		FPRINTF(f, "Usage: %s [OPTION]... [COMMAND] [ARGS]...\n", name);
 		FPRINTF(f, "       %s [OPTION]... PID\n", name);
@@ -897,8 +899,14 @@ static void usage(const char *name, FILE *f, int status)
 			"Without PID or --netns, run the given command or a\n"
 			"default shell in a new network and user namespace, and\n"
 			"connect it via pasta.\n");
+
+		guest = "namespace";
+		fwd_default = "auto";
 	} else {
 		FPRINTF(f, "Usage: %s [OPTION]...\n", name);
+
+		guest = "guest";
+		fwd_default = "none";
 	}
 
 	FPRINTF(f,
@@ -1023,70 +1031,50 @@ 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"
-		"  -t, --tcp-ports SPEC	TCP port forwarding to guest\n"
+		"  -6, --ipv6-only	Enable IPv6 operation only\n"
+		"  -t, --tcp-ports SPEC	TCP port forwarding to %s\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"
 		"        interface prefixed by '%%'. Ranges can be reduced by\n"
 		"        excluding ports or ranges prefixed by '~'\n"
 		"        Examples:\n"
-		"        -t 22		Forward local port 22 to 22 on guest\n"
-		"        -t 22:23	Forward local port 22 to 23 on guest\n"
+		"        -t 22		Forward local port 22 to 22 on %s\n"
+		"        -t 22:23	Forward local port 22 to 23 on %s\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 guest\n"
+		"        -t 192.0.2.1/5	Bind port 5 of 192.0.2.1 to %s\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"
-		"  -u, --udp-ports SPEC	UDP port forwarding to guest\n"
+		"    default: %s\n"
+		"  -u, --udp-ports SPEC	UDP port forwarding to %s\n"
 		"    SPEC is as described for TCP above\n"
-		"    default: none\n");
+		"    default: %s\n",
+		guest,
+		strstr(name, "pasta") ?
+		"      'auto': forward all ports currently bound in namespace\n"
+		:
+		"      'all': forward all unbound, non-ephemeral ports\n",
+		guest, guest, guest, fwd_default, guest, fwd_default);
+
+	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..976f3f0c 100644
--- a/passt.1
+++ b/passt.1
@@ -425,81 +425,9 @@ 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:
+Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of:
 .RS
 
 .TP
@@ -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,
@@ -528,22 +462,22 @@ Examples:
 .RS
 .TP
 -t 22
-Forward local port 22 to port 22 on the guest
+Forward local port 22 to port 22 on the guest or namespace
 .TP
 -t 22:23
-Forward local port 22 to port 23 on the guest
+Forward local port 22 to port 23 on the guest or namespace
 .TP
 -t 22,25
-Forward local ports 22 and 25 to ports 22 and 25 on the guest
+Forward local ports 22 and 25 to ports 22 and 25 on the guest or namespace
 .TP
 -t 22-80
-Forward local ports between 22 and 80 to corresponding ports on the guest
+Forward local ports between 22 and 80 to corresponding ports on the guest or namespace
 .TP
 -t 22-80:32-90
-Forward local ports between 22 and 80 to ports between 32 and 90 on the guest
+Forward local ports between 22 and 80 to ports between 32 and 90 on the guest or namespace
 .TP
 -t 192.0.2.1/22
-Forward local port 22, bound to 192.0.2.1, to port 22 on the guest
+Forward local port 22, bound to 192.0.2.1, to port 22 on the guest or 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
@@ -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

[PATCH v2 05/23] 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 64ee8f00..15044f3c 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 */
@@ -1036,6 +1033,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"
@@ -1059,8 +1057,7 @@ static void usage(const char *name, FILE *f, int status)
 		guest,
 		strstr(name, "pasta") ?
 		"      'auto': forward all ports currently bound in namespace\n"
-		:
-		"      'all': forward all unbound, non-ephemeral ports\n",
+		: "",
 		guest, guest, guest, fwd_default, guest, fwd_default);
 
 	if (strstr(name, "pasta"))
diff --git a/passt.1 b/passt.1
index 976f3f0c..7da4fe5f 100644
--- a/passt.1
+++ b/passt.1
@@ -435,7 +435,7 @@ Configure TCP port forwarding to guest or namespace. \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 v2 06/23] 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 | 20 ++++++----------
 2 files changed, 44 insertions(+), 49 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..7e9ec49e 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,19 @@ 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
- * @sock_count:	Number of entries used in @socks
+ * @rulesocks:	Parallel array of @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
  */
 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

[PATCH v2 07/23] 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 | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fwd_rule.h | 12 +++++++
 6 files changed, 113 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 15044f3c..318bb915 100644
--- a/conf.c
+++ b/conf.c
@@ -1269,7 +1269,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 7e9ec49e..805fabd0 100644
--- a/fwd.h
+++ b/fwd.h
@@ -91,7 +91,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..a034d5d1
--- /dev/null
+++ b/fwd_rule.c
@@ -0,0 +1,95 @@
+// 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
+ */
+#if defined(__GNUC__) && __GNUC__ < 15
+/* Workaround bug in gcc 12, 13 & 14 (at least) which gives a false positive
+ * -Wformat-overflow message if this function is inlined.
+ */
+__attribute__((noinline))
+#endif
+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 v2 08/23] 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 318bb915..a0cf9454 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 v2 09/23] 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 a0cf9454..027bbac9 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 805fabd0..96b8c608 100644
--- a/fwd.h
+++ b/fwd.h
@@ -85,9 +85,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 v2 10/23] 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 | 45 +++++++++++++++++++++++++++++++++++++++++++++
 fwd_rule.h |  2 ++
 4 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/conf.c b/conf.c
index 027bbac9..b871646f 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 a034d5d1..5bc94efe 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -93,3 +93,48 @@ 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 if given 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

[PATCH v2 11/23] 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
configuration 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      | 41 +++++++++++++++++++++++++++++++----------
 fwd.h      |  2 +-
 fwd_rule.c |  3 +--
 fwd_rule.h |  1 +
 5 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/conf.c b/conf.c
index b871646f..5c913820 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..3e87169b 100644
--- a/fwd.c
+++ b/fwd.c
@@ -335,24 +335,44 @@ 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("Rule has invalid port range %u-%u",
+		     new->first, new->last);
+		return -EINVAL;
+	}
+	if (new->flags & ~allowed_flags) {
+		warn("Rule has invalid flags 0x%hhx",
+		     new->flags & ~allowed_flags);
+		return -EINVAL;
+	}
+	if (new->flags & FWD_DUAL_STACK_ANY &&
+	    !inany_equals(&new->addr, &inany_any6)) {
+		char astr[INANY_ADDRSTRLEN];
 
-	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");
+		warn("Dual stack rule has non-wildcard address %s",
+		     inany_ntop(&new->addr, astr, sizeof(astr)));
+		return -EINVAL;
+	}
+
+	if (fwd->count >= ARRAY_SIZE(fwd->rules)) {
+		warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules));
+		return -ENOSPC;
+	}
+	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) {
+		warn("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 +380,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 96b8c608..43bfeadb 100644
--- a/fwd.h
+++ b/fwd.h
@@ -85,7 +85,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 5bc94efe..47d8df1c 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -45,8 +45,7 @@ const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule)
  */
 __attribute__((noinline))
 #endif
-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

[PATCH v2 12/23] 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 5c913820..67d88ee2 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);
 }
 
 /**
@@ -1602,12 +1570,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;
@@ -2212,17 +2177,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);
 
@@ -2273,22 +2238,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

[PATCH v2 13/23] 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 67d88ee2..0bc74e95 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

[PATCH v2 14/23] 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 introduced 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 give an error if port parsing
   overruns a chunk boundary.  We don't really expect that to happen,
   but it would be very confusing if it did.  strtoul(3), on which
   parse_port_range() is based does say it may accept thousands
   separators based on locale which means we can't 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 0bc74e95..c3655824 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

[PATCH v2 15/23] doc: Rework man page description of port specifiers

[David Gibson]

Currently the man page describes the internal syntax of port specifiers
in prose, which isn't particularly easy to follow.  Rework it to use
more syntax "diagrams" to show how it works.  This will also allow us to
more easily update the manual page for some coming changes in syntax.

usage() output is updated similarly, though more briefly.

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

diff --git a/conf.c b/conf.c
index c3655824..5d6517c3 100644
--- a/conf.c
+++ b/conf.c
@@ -1041,11 +1041,11 @@ static void usage(const char *name, FILE *f, int status)
 		"      '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"
-		"        interface prefixed by '%%'. Ranges can be reduced by\n"
-		"        excluding ports or ranges prefixed by '~'\n"
+		"      [ADDR[%%IFACE]/]PORTS: forward specific ports\n"
+		"        PORTS is a comma-separated list of ports, optionally\n"
+		"        ranged with '-' and optional target ports after ':'.\n"
+		"        Ranges can be reduced by excluding ports or ranges\n"
+		"        prefixed by '~'\n"
 		"        Examples:\n"
 		"        -t 22		Forward local port 22 to 22 on %s\n"
 		"        -t 22:23	Forward local port 22 to 23 on %s\n"
diff --git a/passt.1 b/passt.1
index 7da4fe5f..d329f8f0 100644
--- a/passt.1
+++ b/passt.1
@@ -447,16 +447,28 @@ 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,
-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.
-
-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.
+[\fIaddress\fR[\fB%\fR\fIinterface\fR]\fB/\fR]\fIports\fR ...
+Specific ports to forward.  Optionally, a specific listening address
+and interface name (since Linux 5.7) can be specified.  \fIports\fR is
+a comma-separated list of entries which may be any of:
+.RS
+.TP
+\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItofirst\fR[\fB-\fR\fItolast\fR]]
+Include range. Forward port numbers between \fIfirst\fR and \fIlast\fR
+(inclusive) to ports between \fItofirst\fR and \fItolast\fR.  If
+\fItofirst\fR and \fItolast\fR are omitted, assume the same as
+\fIfirst\fR and \fIlast\fR.  If \fIlast\fR is omitted, assume the same
+as \fIfirst\fR.
+
+.TP
+\fB~\fR\fIfirst\fR[\fB-\fR\fIlast\fR]
+Exclude range.  Exclude port numbers between \fIfirst\fR and
+\fIlast\fR from.  This takes precedences over include ranges.
+.RE
+
+Specifying excluded ranges only implies that all other non-ephemeral
+ports are forwarded. In this case, no failures are reported for
+unavailable ports, unless no ports could be forwarded at all.
 
 Examples:
 .RS
-- 
2.53.0

[PATCH v2 16/23] 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  | 25 ++++++++++---------------
 passt.1 | 28 ++++++++++++++++++++--------
 2 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/conf.c b/conf.c
index 5d6517c3..f62109b5 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, '/'))) {
@@ -1039,14 +1031,17 @@ 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"
 		"      [ADDR[%%IFACE]/]PORTS: forward specific ports\n"
-		"        PORTS is a comma-separated list of ports, optionally\n"
+		"        PORTS is either 'all' (forward all unbound, non-ephemeral\n"
+		"        ports), or a comma-separated list of ports, optionally\n"
 		"        ranged with '-' and optional target ports after ':'.\n"
 		"        Ranges can be reduced by excluding ports or ranges\n"
 		"        prefixed by '~'\n"
 		"        Examples:\n"
+		"        -t all         Forward all ports\n"
+		"        -t 127.0.0.1/all Forward all ports from local address\n"
+		"                         127.0.0.1\n"
 		"        -t 22		Forward local port 22 to 22 on %s\n"
 		"        -t 22:23	Forward local port 22 to 23 on %s\n"
 		"        -t 22,25	Forward ports 22, 25 to ports 22, 25\n"
diff --git a/passt.1 b/passt.1
index d329f8f0..3ba447d5 100644
--- a/passt.1
+++ b/passt.1
@@ -434,12 +434,6 @@ Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of:
 .BR none
 Don't forward any ports
 
-.TP
-.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.
-
 .TP
 .BR auto " " (\fBpasta\fR " " only)
 Dynamically forward ports bound in the namespace. The list of ports is
@@ -449,10 +443,20 @@ periodically derived (every second) from listening sockets reported by
 .TP
 [\fIaddress\fR[\fB%\fR\fIinterface\fR]\fB/\fR]\fIports\fR ...
 Specific ports to forward.  Optionally, a specific listening address
-and interface name (since Linux 5.7) can be specified.  \fIports\fR is
-a comma-separated list of entries which may be any of:
+and interface name (since Linux 5.7) can be specified.  \fIports\fR
+may be either:
 .RS
 .TP
+\fBall\fR
+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.
+.RE
+
+.RS
+or a comma-separated list of entries which may be any of:
+.TP
 \fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItofirst\fR[\fB-\fR\fItolast\fR]]
 Include range. Forward port numbers between \fIfirst\fR and \fIlast\fR
 (inclusive) to ports between \fItofirst\fR and \fItolast\fR.  If
@@ -473,6 +477,14 @@ unavailable ports, unless no ports could be forwarded at all.
 Examples:
 .RS
 .TP
+-t all
+Forward all unbound, non-ephemeral ports as permitted by current
+capabilities to the corresponding port on the guest or namespace
+.TP
+-t 127.0.0.1/all
+For the local address 127.0.0.1, forward all unbound, non-ephemeral
+ports as permitted by current capabilities.
+.TP
 -t 22
 Forward local port 22 to port 22 on the guest or namespace
 .TP
-- 
2.53.0

[PATCH v2 17/23] 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  | 85 ++++++++++++++++++++++++++++++++++++++++++---------------
 passt.1 | 30 ++++++++++++++------
 2 files changed, 85 insertions(+), 30 deletions(-)

diff --git a/conf.c b/conf.c
index f62109b5..8e3b4b20 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, '/'))) {
@@ -1031,13 +1061,13 @@ 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"
-		"%s"
 		"      [ADDR[%%IFACE]/]PORTS: forward specific ports\n"
 		"        PORTS is either 'all' (forward all unbound, non-ephemeral\n"
 		"        ports), or a comma-separated list of ports, optionally\n"
 		"        ranged with '-' and optional target ports after ':'.\n"
 		"        Ranges can be reduced by excluding ports or ranges\n"
-		"        prefixed by '~'\n"
+		"        prefixed by '~'.\n"
+		"%s"
 		"        Examples:\n"
 		"        -t all         Forward all ports\n"
 		"        -t 127.0.0.1/all Forward all ports from local address\n"
@@ -1051,15 +1081,26 @@ 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 %s\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"
+		"%s"
 		"    default: %s\n"
 		"  -u, --udp-ports SPEC	UDP port forwarding to %s\n"
 		"    SPEC is as described for TCP above\n"
 		"    default: %s\n",
 		guest,
 		strstr(name, "pasta") ?
-		"      'auto': forward all ports currently bound in namespace\n"
+		"        The 'auto' keyword may be given to only forward\n"
+		"        ports which are bound in the target namespace\n"
+		: "",
+		guest, guest, guest,
+		strstr(name, "pasta") ?
+		"        -t auto	Forward all ports bound in namespace\n"
+		"        -t 192.0.2.2/auto Forward ports from 192.0.2.2 if\n"
+		"                          they are bound in the namespace\n"
+		"        -t 8000-8010,auto Forward ports 8000-8010 if they\n"
+		"                          are bound in the namespace\n"
 		: "",
-		guest, guest, guest, fwd_default, guest, fwd_default);
+
+		fwd_default, guest, fwd_default);
 
 	if (strstr(name, "pasta"))
 		goto pasta_opts;
diff --git a/passt.1 b/passt.1
index 3ba447d5..eeecc0fb 100644
--- a/passt.1
+++ b/passt.1
@@ -434,12 +434,6 @@ Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of:
 .BR none
 Don't forward any ports
 
-.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
 [\fIaddress\fR[\fB%\fR\fIinterface\fR]\fB/\fR]\fIports\fR ...
 Specific ports to forward.  Optionally, a specific listening address
@@ -468,11 +462,20 @@ as \fIfirst\fR.
 \fB~\fR\fIfirst\fR[\fB-\fR\fIlast\fR]
 Exclude range.  Exclude port numbers between \fIfirst\fR and
 \fIlast\fR from.  This takes precedences over include ranges.
+
+.TP
+.BR auto
+(\fBpasta\fR " " only).  Only forward ports in the specified set if
+the target ports are 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).
 .RE
 
 Specifying excluded ranges only implies that all other non-ephemeral
-ports are forwarded. In this case, no failures are reported for
-unavailable ports, unless no ports could be forwarded at all.
+ports are forwarded. Specifying no ranges at all implies forwarding
+all non-ephemeral ports permitted by current capabilities.  In this
+case, no failures are reported for unavailable ports, unless no ports
+could be forwarded at all.
 
 Examples:
 .RS
@@ -519,6 +522,17 @@ and 30
 .TP
 -t ~20000-20010
 Forward all ports to the guest, except for the range from 20000 to 20010
+.TP
+-t auto
+Automatically forward any ports which are bound in the namespace.
+.TP
+-t 192.0.2.2/auto
+Automatically forward any ports which are bound in the namespace,
+listening only on local port 192.0.2.2.
+.TP
+-t 8000-8010,auto
+Forward ports in the range 8000-8010 if and only if they are bound in
+the namespace.
 .RE
 
 Default is \fBnone\fR for \fBpasst\fR and \fBauto\fR for \fBpasta\fR.
-- 
2.53.0

[PATCH v2 18/23] 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 8e3b4b20..5629b3dc 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 v2 19/23] 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 5629b3dc..89f68de7 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

[PATCH v2 20/23] fwd, conf: Add capabilities bits to each forwarding table

[David Gibson]

conf_ports_spec() and conf_ports() take the global context structure, but
their only use for it is seeing if various things are possible: which
protocols and address formats are allowed in formatting rules.  Localise
that information into the forwarding table, with a capabilities bitmap.

For now we set that caps map to the same thing for all tables, but keep it
per-table to allow for the possibility of different pif types in future
that might have different capabilities (e.g. if we add a forwarding table
for the tap interface, it won't be able to accept interface names to bind).

Use this information to remove the global context parameter from
conf_ports() and conf_ports_spec().

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

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

[PATCH v2 21/23] conf, fwd: Stricter rule checking in fwd_rule_add()

[David Gibson]

Although fwd_rule_add() performs some sanity checks on the rule it is
given, there are invalid rules we don't check for, assuming that its
callers will do that.

That won't be enough when we can get rules inserted by a dynamic update
client without going through the existing parsing code.  So, add stricter
checks to fwd_rule_add(), which is now possible thanks to the capabilities
bits in the struct fwd_table.  Where those duplicate existing checks in the
callers, remove the old copies.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 19 -------------------
 fwd.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/conf.c b/conf.c
index b7a4459e..31627209 100644
--- a/conf.c
+++ b/conf.c
@@ -310,10 +310,6 @@ static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto,
 		if (p != ep) /* Garbage after the ranges */
 			goto bad;
 
-		if (orig_range.first == 0) {
-			die("Can't forward port 0 included in '%s'", spec);
-		}
-
 		conf_ports_range_except(fwd, proto, addr, ifname,
 					orig_range.first, orig_range.last,
 					exclude,
@@ -356,11 +352,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd)
 		return;
 	}
 
-	if (proto == IPPROTO_TCP && !(fwd->caps & FWD_CAP_TCP))
-		die("TCP port forwarding requested but TCP is disabled");
-	if (proto == IPPROTO_UDP && !(fwd->caps & FWD_CAP_UDP))
-		die("UDP port forwarding requested but UDP is disabled");
-
 	strncpy(buf, optarg, sizeof(buf) - 1);
 
 	if ((spec = strchr(buf, '/'))) {
@@ -405,16 +396,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd)
 		addr = NULL;
 	}
 
-	if (addr) {
-		if (!(fwd->caps & FWD_CAP_IPV4) && inany_v4(addr)) {
-			die("IPv4 is disabled, can't use -%c %s",
-			    optname, optarg);
-		} else if (!(fwd->caps & FWD_CAP_IPV6) && !inany_v4(addr)) {
-			die("IPv6 is disabled, can't use -%c %s",
-			    optname, optarg);
-		}
-	}
-
 	if (optname == 'T' || optname == 'U') {
 		assert(!addr && !ifname);
 
diff --git a/fwd.c b/fwd.c
index c7fd1a9d..98b04d0c 100644
--- a/fwd.c
+++ b/fwd.c
@@ -367,17 +367,58 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
 		     new->first, new->last);
 		return -EINVAL;
 	}
+	if (!new->first) {
+		warn("Forwarding rule atttempts to map from port 0");
+		return -EINVAL;
+	}
+	if (!new->to || (new->to + new->last - new->first) < new->to) {
+		warn("Forwarding rule atttempts to map to port 0");
+		return -EINVAL;
+	}
 	if (new->flags & ~allowed_flags) {
 		warn("Rule has invalid flags 0x%hhx",
 		     new->flags & ~allowed_flags);
 		return -EINVAL;
 	}
-	if (new->flags & FWD_DUAL_STACK_ANY &&
-	    !inany_equals(&new->addr, &inany_any6)) {
-		char astr[INANY_ADDRSTRLEN];
+	if (new->flags & FWD_DUAL_STACK_ANY) {
+		if (!inany_equals(&new->addr, &inany_any6)) {
+			char astr[INANY_ADDRSTRLEN];
 
-		warn("Dual stack rule has non-wildcard address %s",
-		     inany_ntop(&new->addr, astr, sizeof(astr)));
+			warn("Dual stack rule has non-wildcard address %s",
+			     inany_ntop(&new->addr, astr, sizeof(astr)));
+			return -EINVAL;
+		}
+		if (!(fwd->caps & FWD_CAP_IPV4)) {
+			warn("Dual stack forward, but IPv4 not enabled");
+			return -EINVAL;
+		}
+		if (!(fwd->caps & FWD_CAP_IPV6)) {
+			warn("Dual stack forward, but IPv6 not enabled");
+			return -EINVAL;
+		}
+	} else {
+		if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) {
+			warn("IPv4 forward, but IPv4 not enabled");
+			return -EINVAL;
+		}
+		if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) {
+			warn("IPv6 forward, but IPv6 not enabled");
+			return -EINVAL;
+		}
+	}
+	if (new->proto == IPPROTO_TCP) {
+		if (!(fwd->caps & FWD_CAP_TCP)) {
+			warn("Can't add TCP forwarding rule, TCP not enabled");
+			return -EINVAL;
+		}
+	} else if (new->proto == IPPROTO_UDP) {
+		if (!(fwd->caps & FWD_CAP_UDP)) {
+			warn("Can't add UDP forwarding rule, UDP not enabled");
+			return -EINVAL;
+		}
+	} else {
+		warn("Unsupported protocol 0x%hhx (%s) for forwarding rule",
+		     new->proto, ipproto_name(new->proto));
 		return -EINVAL;
 	}
 
-- 
2.53.0

[PATCH v2 22/23] fwd_rule: Move ephemeral port probing to fwd_rule.c

[David Gibson]

We want to move parsing of forward rule options to fwd_rule.c so it can
eventually be shared with a configuration client.  As a preliminary step,
move the ephemeral port probing there, which that will need to use.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c      | 73 --------------------------------------------------
 fwd.h      |  6 -----
 fwd_rule.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fwd_rule.h |  6 +++++
 4 files changed, 84 insertions(+), 79 deletions(-)

diff --git a/fwd.c b/fwd.c
index 98b04d0c..f2f7b648 100644
--- a/fwd.c
+++ b/fwd.c
@@ -34,12 +34,6 @@
 #include "arp.h"
 #include "ndp.h"
 
-/* Ephemeral port range: values from RFC 6335 */
-static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14);
-static in_port_t fwd_ephemeral_max = NUM_PORTS - 1;
-
-#define PORT_RANGE_SYSCTL	"/proc/sys/net/ipv4/ip_local_port_range"
-
 #define NEIGH_TABLE_SLOTS    1024
 #define NEIGH_TABLE_SIZE     (NEIGH_TABLE_SLOTS / 2)
 static_assert((NEIGH_TABLE_SLOTS & (NEIGH_TABLE_SLOTS - 1)) == 0,
@@ -249,73 +243,6 @@ void fwd_neigh_table_init(const struct ctx *c)
 		fwd_neigh_table_update(c, &mga, c->our_tap_mac, true);
 }
 
-/** fwd_probe_ephemeral() - Determine what ports this host considers ephemeral
- *
- * Work out what ports the host thinks are emphemeral and record it for later
- * use by fwd_port_is_ephemeral().  If we're unable to probe, assume the range
- * recommended by RFC 6335.
- */
-void fwd_probe_ephemeral(void)
-{
-	char *line, *tab, *end;
-	struct lineread lr;
-	long min, max;
-	ssize_t len;
-	int fd;
-
-	fd = open(PORT_RANGE_SYSCTL, O_RDONLY | O_CLOEXEC);
-	if (fd < 0) {
-		warn_perror("Unable to open %s", PORT_RANGE_SYSCTL);
-		return;
-	}
-
-	lineread_init(&lr, fd);
-	len = lineread_get(&lr, &line);
-	close(fd);
-
-	if (len < 0)
-		goto parse_err;
-
-	tab = strchr(line, '\t');
-	if (!tab)
-		goto parse_err;
-	*tab = '\0';
-
-	errno = 0;
-	min = strtol(line, &end, 10);
-	if (*end || errno)
-		goto parse_err;
-
-	errno = 0;
-	max = strtol(tab + 1, &end, 10);
-	if (*end || errno)
-		goto parse_err;
-
-	if (min < 0 || min >= (long)NUM_PORTS ||
-	    max < 0 || max >= (long)NUM_PORTS)
-		goto parse_err;
-
-	fwd_ephemeral_min = min;
-	fwd_ephemeral_max = max;
-
-	return;
-
-parse_err:
-	warn("Unable to parse %s", PORT_RANGE_SYSCTL);
-}
-
-/**
- * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap
- * @map:	Bitmap to update
- */
-void fwd_port_map_ephemeral(uint8_t *map)
-{
-	unsigned port;
-
-	for (port = fwd_ephemeral_min; port <= fwd_ephemeral_max; port++)
-		bitmap_set(map, port);
-}
-
 /* Forwarding table storage, generally accessed via pointers in struct ctx */
 static struct fwd_table fwd_in;
 static struct fwd_table fwd_out;
diff --git a/fwd.h b/fwd.h
index 3e365d35..e664d1d0 100644
--- a/fwd.h
+++ b/fwd.h
@@ -20,12 +20,6 @@
 
 struct flowside;
 
-/* Number of ports for both TCP and UDP */
-#define	NUM_PORTS	(1U << 16)
-
-void fwd_probe_ephemeral(void);
-void fwd_port_map_ephemeral(uint8_t *map);
-
 #define FWD_RULE_BITS	8
 #define MAX_FWD_RULES	MAX_FROM_BITS(FWD_RULE_BITS)
 #define FWD_NO_HINT	(-1)
diff --git a/fwd_rule.c b/fwd_rule.c
index 47d8df1c..9d489827 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -15,9 +15,87 @@
  * Author: David Gibson <david@gibson.dropbear.id.au>
  */
 
+#include <errno.h>
+#include <fcntl.h>
 #include <stdio.h>
+#include <unistd.h>
 
 #include "fwd_rule.h"
+#include "lineread.h"
+#include "log.h"
+
+/* Ephemeral port range: values from RFC 6335 */
+static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14);
+static in_port_t fwd_ephemeral_max = NUM_PORTS - 1;
+
+#define PORT_RANGE_SYSCTL	"/proc/sys/net/ipv4/ip_local_port_range"
+
+/** fwd_probe_ephemeral() - Determine what ports this host considers ephemeral
+ *
+ * Work out what ports the host thinks are emphemeral and record it for later
+ * use by fwd_port_is_ephemeral().  If we're unable to probe, assume the range
+ * recommended by RFC 6335.
+ */
+void fwd_probe_ephemeral(void)
+{
+	char *line, *tab, *end;
+	struct lineread lr;
+	long min, max;
+	ssize_t len;
+	int fd;
+
+	fd = open(PORT_RANGE_SYSCTL, O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		warn_perror("Unable to open %s", PORT_RANGE_SYSCTL);
+		return;
+	}
+
+	lineread_init(&lr, fd);
+	len = lineread_get(&lr, &line);
+	close(fd);
+
+	if (len < 0)
+		goto parse_err;
+
+	tab = strchr(line, '\t');
+	if (!tab)
+		goto parse_err;
+	*tab = '\0';
+
+	errno = 0;
+	min = strtol(line, &end, 10);
+	if (*end || errno)
+		goto parse_err;
+
+	errno = 0;
+	max = strtol(tab + 1, &end, 10);
+	if (*end || errno)
+		goto parse_err;
+
+	if (min < 0 || min >= (long)NUM_PORTS ||
+	    max < 0 || max >= (long)NUM_PORTS)
+		goto parse_err;
+
+	fwd_ephemeral_min = min;
+	fwd_ephemeral_max = max;
+
+	return;
+
+parse_err:
+	warn("Unable to parse %s", PORT_RANGE_SYSCTL);
+}
+
+/**
+ * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap
+ * @map:	Bitmap to update
+ */
+void fwd_port_map_ephemeral(uint8_t *map)
+{
+	unsigned port;
+
+	for (port = fwd_ephemeral_min; port <= fwd_ephemeral_max; port++)
+		bitmap_set(map, port);
+}
 
 /**
  * fwd_rule_addr() - Return match address for a rule
diff --git a/fwd_rule.h b/fwd_rule.h
index edba6782..5c7b67aa 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -17,6 +17,9 @@
 #include "inany.h"
 #include "bitmap.h"
 
+/* Number of ports for both TCP and UDP */
+#define	NUM_PORTS	(1U << 16)
+
 /* Forwarding capability bits */
 #define FWD_CAP_IPV4		BIT(0)
 #define FWD_CAP_IPV6		BIT(1)
@@ -51,6 +54,9 @@ struct fwd_rule {
 	uint8_t flags;
 };
 
+void fwd_probe_ephemeral(void);
+void fwd_port_map_ephemeral(uint8_t *map);
+
 #define FWD_RULE_STRLEN					    \
 	(IPPROTO_STRLEN - 1				    \
 	 + INANY_ADDRSTRLEN - 1				    \
-- 
2.53.0

[PATCH v2 23/23] fwd, conf: Move rule parsing code to fwd_rule.[ch]

[David Gibson]

The code parsing command line options into forwarding rules has now been
decoupled from most of passt/pasta's internals.  This is good, because
we'll soon want to share it with a configuration update client.

Make the next step by moving this code into fwd_rule.[ch].

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c     | 378 +------------------------------------------
 fwd.c      |  93 -----------
 fwd.h      |  33 ----
 fwd_rule.c | 465 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 fwd_rule.h |  36 ++++-
 5 files changed, 503 insertions(+), 502 deletions(-)

diff --git a/conf.c b/conf.c
index 31627209..5f38a287 100644
--- a/conf.c
+++ b/conf.c
@@ -13,7 +13,6 @@
  */
 
 #include <arpa/inet.h>
-#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <getopt.h>
@@ -66,367 +65,6 @@
 
 const char *pasta_default_ifn = "tap0";
 
-/**
- * port_range() - Represents a non-empty range of ports
- * @first:	First port number in the range
- * @last:	Last port number in the range (inclusive)
- *
- * Invariant:	@last >= @first
- */
-struct port_range {
-	in_port_t first, last;
-};
-
-/**
- * parse_port_range() - Parse a range of port numbers '<first>[-<last>]'
- * @s:		String to parse
- * @endptr:	Update to the character after the parsed range (similar to
- *		strtol() etc.)
- * @range:	Update with the parsed values on success
- *
- * Return: -EINVAL on parsing error, -ERANGE on out of range port
- *	   numbers, 0 on success
- */
-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, &ep, 10);
-	if (ep == s) /* Parsed nothing */
-		return -EINVAL;
-	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;
-	}
-
-	if ((last < first) || (last >= NUM_PORTS))
-		return -ERANGE;
-
-	range->first = first;
-	range->last = last;
-	*endptr = ep;
-
-	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
- * @fwd:	Forwarding table to be updated
- * @proto:	Protocol to forward
- * @addr:	Listening address
- * @ifname:	Listening interface
- * @first:	First port to forward
- * @last:	Last port to forward
- * @exclude:	Bitmap of ports to exclude (may be NULL)
- * @to:		Port to translate @first to when forwarding
- * @flags:	Flags for forwarding entries
- */
-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,
-				    uint8_t flags)
-{
-	struct fwd_rule rule = {
-		.addr = addr ? *addr : inany_any6,
-		.ifname = { 0 },
-		.proto = proto,
-		.flags = flags,
-	};
-	char rulestr[FWD_RULE_STRLEN];
-	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++) {
-		if (exclude && bitmap_isset(exclude, base))
-			continue;
-
-		for (i = base; i <= last; i++) {
-			if (exclude && bitmap_isset(exclude, i))
-				break;
-		}
-
-		rule.first = base;
-		rule.last = i - 1;
-		rule.to = base + delta;
-
-		fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
-		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)));
-}
-
-/*
- * 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
- * @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(struct fwd_table *fwd, uint8_t proto,
-			    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, *ep;
-	uint8_t flags = 0;
-	unsigned i;
-
-	if (!strcmp(spec, "all")) {
-		/* Treat "all" as equivalent to "": all non-ephemeral ports */
-		spec = "";
-	}
-
-	/* Parse excluded ranges and "auto" in the first pass */
-	for_each_chunk(p, ep, spec, ",") {
-		struct port_range xrange;
-
-		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 (!(fwd->caps & FWD_CAP_SCAN)) {
-				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))
-			goto bad;
-		if (p != ep) /* Garbage after the range */
-			goto bad;
-
-		for (i = xrange.first; i <= xrange.last; i++)
-			bitmap_set(exclude, i);
-	}
-
-	if (exclude_only) {
-		/* Exclude ephemeral ports */
-		fwd_port_map_ephemeral(exclude);
-
-		conf_ports_range_except(fwd, proto, addr, ifname,
-					1, NUM_PORTS - 1, exclude,
-					1, flags | FWD_WEAK);
-		return;
-	}
-
-	/* Now process base ranges, skipping exclusions */
-	for_each_chunk(p, ep, spec, ",") {
-		struct port_range orig_range, mapped_range;
-
-		if (!isdigit(*p))
-			/* 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 != ep) /* Garbage after the ranges */
-			goto bad;
-
-		conf_ports_range_except(fwd, proto, addr, ifname,
-					orig_range.first, orig_range.last,
-					exclude,
-					mapped_range.first, flags);
-	}
-
-	return;
-bad:
-	die("Invalid port specifier '%s'", spec);
-}
-
-/**
- * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets
- * @optname:	Short option name, t, T, u, or U
- * @optarg:	Option argument (port specification)
- * @fwd:	Forwarding table to be updated
- */
-static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd)
-{
-	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
-	char buf[BUFSIZ], *spec, *ifname = NULL;
-	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")) {
-		unsigned i;
-
-		for (i = 0; i < fwd->count; i++) {
-			if (fwd->rules[i].proto == proto) {
-				die("-%c none conflicts with previous options",
-					optname);
-			}
-		}
-		return;
-	}
-
-	strncpy(buf, optarg, sizeof(buf) - 1);
-
-	if ((spec = strchr(buf, '/'))) {
-		*spec = 0;
-		spec++;
-
-		if (optname != 't' && optname != 'u')
-			die("Listening address not allowed for -%c %s",
-			    optname, optarg);
-
-		if ((ifname = strchr(buf, '%'))) {
-			*ifname = 0;
-			ifname++;
-
-			/* spec is already advanced one past the '/',
-			 * so the length of the given ifname is:
-			 * (spec - ifname - 1)
-			 */
-			if (spec - ifname - 1 >= IFNAMSIZ) {
-				die("Interface name '%s' is too long (max %u)",
-				    ifname, IFNAMSIZ - 1);
-			}
-		}
-
-		if (ifname == buf + 1) {	/* Interface without address */
-			addr = NULL;
-		} else {
-			char *p = buf;
-
-			/* Allow square brackets for IPv4 too for convenience */
-			if (*p == '[' && p[strlen(p) - 1] == ']') {
-				p[strlen(p) - 1] = '\0';
-				p++;
-			}
-
-			if (!inany_pton(p, addr))
-				die("Bad forwarding address '%s'", p);
-		}
-	} else {
-		spec = buf;
-
-		addr = NULL;
-	}
-
-	if (optname == 'T' || optname == 'U') {
-		assert(!addr && !ifname);
-
-		if (!(fwd->caps & FWD_CAP_IFNAME)) {
-			warn(
-"SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'",
-			     optname, optarg);
-
-			if (fwd->caps & FWD_CAP_IPV4) {
-				conf_ports_spec(fwd, proto,
-						&inany_loopback4, NULL, spec);
-			}
-			if (fwd->caps & FWD_CAP_IPV6) {
-				conf_ports_spec(fwd, proto,
-						&inany_loopback6, NULL, spec);
-			}
-			return;
-		}
-
-		ifname = "lo";
-	}
-
-	if (ifname && !(fwd->caps & FWD_CAP_IFNAME)) {
-		die(
-"Device binding for '-%c %s' unsupported (requires kernel 5.7+)",
-		    optname, optarg);
-	}
-
-	conf_ports_spec(fwd, proto, addr, ifname, spec);
-}
-
 /**
  * add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration
  * @c:		Execution context
@@ -2163,16 +1801,16 @@ void conf(struct ctx *c, int argc, char **argv)
 
 		if (name == 't') {
 			opt_t = true;
-			conf_ports(name, optarg, c->fwd[PIF_HOST]);
+			fwd_rule_parse(name, optarg, c->fwd[PIF_HOST]);
 		} else if (name == 'u') {
 			opt_u = true;
-			conf_ports(name, optarg, c->fwd[PIF_HOST]);
+			fwd_rule_parse(name, optarg, c->fwd[PIF_HOST]);
 		} else if (name == 'T') {
 			opt_T = true;
-			conf_ports(name, optarg, c->fwd[PIF_SPLICE]);
+			fwd_rule_parse(name, optarg, c->fwd[PIF_SPLICE]);
 		} else if (name == 'U') {
 			opt_U = true;
-			conf_ports(name, optarg, c->fwd[PIF_SPLICE]);
+			fwd_rule_parse(name, optarg, c->fwd[PIF_SPLICE]);
 		}
 	} while (name != -1);
 
@@ -2224,13 +1862,13 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	if (c->mode == MODE_PASTA) {
 		if (!opt_t)
-			conf_ports('t', "auto", c->fwd[PIF_HOST]);
+			fwd_rule_parse('t', "auto", c->fwd[PIF_HOST]);
 		if (!opt_T)
-			conf_ports('T', "auto", c->fwd[PIF_SPLICE]);
+			fwd_rule_parse('T', "auto", c->fwd[PIF_SPLICE]);
 		if (!opt_u)
-			conf_ports('u', "auto", c->fwd[PIF_HOST]);
+			fwd_rule_parse('u', "auto", c->fwd[PIF_HOST]);
 		if (!opt_U)
-			conf_ports('U', "auto", c->fwd[PIF_SPLICE]);
+			fwd_rule_parse('U', "auto", c->fwd[PIF_SPLICE]);
 	}
 
 	if (!c->quiet)
diff --git a/fwd.c b/fwd.c
index f2f7b648..728a783c 100644
--- a/fwd.c
+++ b/fwd.c
@@ -275,99 +275,6 @@ void fwd_rule_init(struct ctx *c)
 		c->fwd[PIF_SPLICE] = &fwd_out;
 }
 
-/**
- * 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
- */
-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;
-
-	if (new->first > new->last) {
-		warn("Rule has invalid port range %u-%u",
-		     new->first, new->last);
-		return -EINVAL;
-	}
-	if (!new->first) {
-		warn("Forwarding rule atttempts to map from port 0");
-		return -EINVAL;
-	}
-	if (!new->to || (new->to + new->last - new->first) < new->to) {
-		warn("Forwarding rule atttempts to map to port 0");
-		return -EINVAL;
-	}
-	if (new->flags & ~allowed_flags) {
-		warn("Rule has invalid flags 0x%hhx",
-		     new->flags & ~allowed_flags);
-		return -EINVAL;
-	}
-	if (new->flags & FWD_DUAL_STACK_ANY) {
-		if (!inany_equals(&new->addr, &inany_any6)) {
-			char astr[INANY_ADDRSTRLEN];
-
-			warn("Dual stack rule has non-wildcard address %s",
-			     inany_ntop(&new->addr, astr, sizeof(astr)));
-			return -EINVAL;
-		}
-		if (!(fwd->caps & FWD_CAP_IPV4)) {
-			warn("Dual stack forward, but IPv4 not enabled");
-			return -EINVAL;
-		}
-		if (!(fwd->caps & FWD_CAP_IPV6)) {
-			warn("Dual stack forward, but IPv6 not enabled");
-			return -EINVAL;
-		}
-	} else {
-		if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) {
-			warn("IPv4 forward, but IPv4 not enabled");
-			return -EINVAL;
-		}
-		if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) {
-			warn("IPv6 forward, but IPv6 not enabled");
-			return -EINVAL;
-		}
-	}
-	if (new->proto == IPPROTO_TCP) {
-		if (!(fwd->caps & FWD_CAP_TCP)) {
-			warn("Can't add TCP forwarding rule, TCP not enabled");
-			return -EINVAL;
-		}
-	} else if (new->proto == IPPROTO_UDP) {
-		if (!(fwd->caps & FWD_CAP_UDP)) {
-			warn("Can't add UDP forwarding rule, UDP not enabled");
-			return -EINVAL;
-		}
-	} else {
-		warn("Unsupported protocol 0x%hhx (%s) for forwarding rule",
-		     new->proto, ipproto_name(new->proto));
-		return -EINVAL;
-	}
-
-	if (fwd->count >= ARRAY_SIZE(fwd->rules)) {
-		warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules));
-		return -ENOSPC;
-	}
-	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) {
-		warn("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++)
-		fwd->rulesocks[fwd->count][port - new->first] = -1;
-
-	fwd->rules[fwd->count++] = *new;
-	fwd->sock_count += num;
-	return 0;
-}
-
 /**
  * fwd_rule_match() - Does a prospective flow match a given forwarding rule?
  * @rule:	Forwarding rule
diff --git a/fwd.h b/fwd.h
index e664d1d0..8f845d09 100644
--- a/fwd.h
+++ b/fwd.h
@@ -20,8 +20,6 @@
 
 struct flowside;
 
-#define FWD_RULE_BITS	8
-#define MAX_FWD_RULES	MAX_FROM_BITS(FWD_RULE_BITS)
 #define FWD_NO_HINT	(-1)
 
 /**
@@ -36,36 +34,6 @@ struct fwd_listen_ref {
 	unsigned	rule :FWD_RULE_BITS;
 };
 
-/* Maximum number of listening sockets (per pif)
- *
- * Rationale: This lets us listen on every port for two addresses and two
- * protocols (which we need for -T auto -U auto without SO_BINDTODEVICE), plus a
- * comfortable number of extras.
- */
-#define MAX_LISTEN_SOCKS	(NUM_PORTS * 5)
-
-/**
- * struct fwd_table - Forwarding state (per initiating pif)
- * @caps:	Forwarding capabilities for this initiating pif
- * @count:	Number of forwarding rules
- * @rules:	Array of forwarding rules
- * @rulesocks:	Parallel array of @rules (@count valid entries) of pointers to
- *		@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
- */
-struct fwd_table {
-	uint32_t caps;
-	unsigned count;
-	struct fwd_rule rules[MAX_FWD_RULES];
-	int *rulesocks[MAX_FWD_RULES];
-	unsigned sock_count;
-	int socks[MAX_LISTEN_SOCKS];
-};
-
-#define PORT_BITMAP_SIZE	DIV_ROUND_UP(NUM_PORTS, 8)
-
 /**
  * struct fwd_scan - Port scanning state for a protocol+direction
  * @scan4:	/proc/net fd to scan for IPv4 ports when in AUTO mode
@@ -81,7 +49,6 @@ struct fwd_scan {
 #define FWD_PORT_SCAN_INTERVAL		1000	/* ms */
 
 void fwd_rule_init(struct ctx *c);
-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 9d489827..2b0bd4db 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -15,6 +15,7 @@
  * Author: David Gibson <david@gibson.dropbear.id.au>
  */
 
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
@@ -89,7 +90,7 @@ parse_err:
  * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap
  * @map:	Bitmap to update
  */
-void fwd_port_map_ephemeral(uint8_t *map)
+static void fwd_port_map_ephemeral(uint8_t *map)
 {
 	unsigned port;
 
@@ -123,6 +124,7 @@ const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule)
  */
 __attribute__((noinline))
 #endif
+/* cppcheck-suppress staticFunction */
 const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size)
 {
 	const char *percent = *rule->ifname ? "%" : "";
@@ -199,8 +201,8 @@ static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_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)
+static void fwd_rule_conflict_check(const struct fwd_rule *new,
+				    const struct fwd_rule *rules, size_t count)
 {
 	unsigned i;
 
@@ -215,3 +217,460 @@ void fwd_rule_conflict_check(const struct fwd_rule *new,
 		    fwd_rule_fmt(&rules[i], rulestr, sizeof(rulestr)));
 	}
 }
+
+/**
+ * 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
+ */
+static 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;
+
+	if (new->first > new->last) {
+		warn("Rule has invalid port range %u-%u",
+		     new->first, new->last);
+		return -EINVAL;
+	}
+	if (!new->first) {
+		warn("Forwarding rule atttempts to map from port 0");
+		return -EINVAL;
+	}
+	if (!new->to || (new->to + new->last - new->first) < new->to) {
+		warn("Forwarding rule atttempts to map to port 0");
+		return -EINVAL;
+	}
+	if (new->flags & ~allowed_flags) {
+		warn("Rule has invalid flags 0x%hhx",
+		     new->flags & ~allowed_flags);
+		return -EINVAL;
+	}
+	if (new->flags & FWD_DUAL_STACK_ANY) {
+		if (!inany_equals(&new->addr, &inany_any6)) {
+			char astr[INANY_ADDRSTRLEN];
+
+			warn("Dual stack rule has non-wildcard address %s",
+			     inany_ntop(&new->addr, astr, sizeof(astr)));
+			return -EINVAL;
+		}
+		if (!(fwd->caps & FWD_CAP_IPV4)) {
+			warn("Dual stack forward, but IPv4 not enabled");
+			return -EINVAL;
+		}
+		if (!(fwd->caps & FWD_CAP_IPV6)) {
+			warn("Dual stack forward, but IPv6 not enabled");
+			return -EINVAL;
+		}
+	} else {
+		if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) {
+			warn("IPv4 forward, but IPv4 not enabled");
+			return -EINVAL;
+		}
+		if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) {
+			warn("IPv6 forward, but IPv6 not enabled");
+			return -EINVAL;
+		}
+	}
+	if (new->proto == IPPROTO_TCP) {
+		if (!(fwd->caps & FWD_CAP_TCP)) {
+			warn("Can't add TCP forwarding rule, TCP not enabled");
+			return -EINVAL;
+		}
+	} else if (new->proto == IPPROTO_UDP) {
+		if (!(fwd->caps & FWD_CAP_UDP)) {
+			warn("Can't add UDP forwarding rule, UDP not enabled");
+			return -EINVAL;
+		}
+	} else {
+		warn("Unsupported protocol 0x%hhx (%s) for forwarding rule",
+		     new->proto, ipproto_name(new->proto));
+		return -EINVAL;
+	}
+
+	if (fwd->count >= ARRAY_SIZE(fwd->rules)) {
+		warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules));
+		return -ENOSPC;
+	}
+	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) {
+		warn("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++)
+		fwd->rulesocks[fwd->count][port - new->first] = -1;
+
+	fwd->rules[fwd->count++] = *new;
+	fwd->sock_count += num;
+	return 0;
+}
+
+/**
+ * port_range() - Represents a non-empty range of ports
+ * @first:	First port number in the range
+ * @last:	Last port number in the range (inclusive)
+ *
+ * Invariant:	@last >= @first
+ */
+struct port_range {
+	in_port_t first, last;
+};
+
+/**
+ * parse_port_range() - Parse a range of port numbers '<first>[-<last>]'
+ * @s:		String to parse
+ * @endptr:	Update to the character after the parsed range (similar to
+ *		strtol() etc.)
+ * @range:	Update with the parsed values on success
+ *
+ * Return: -EINVAL on parsing error, -ERANGE on out of range port
+ *	   numbers, 0 on success
+ */
+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, &ep, 10);
+	if (ep == s) /* Parsed nothing */
+		return -EINVAL;
+	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;
+	}
+
+	if ((last < first) || (last >= NUM_PORTS))
+		return -ERANGE;
+
+	range->first = first;
+	range->last = last;
+	*endptr = ep;
+
+	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;
+}
+
+/**
+ * fwd_rule_range_except() - Set up forwarding for a range of ports minus a
+ *                           bitmap of exclusions
+ * @fwd:	Forwarding table to be updated
+ * @proto:	Protocol to forward
+ * @addr:	Listening address
+ * @ifname:	Listening interface
+ * @first:	First port to forward
+ * @last:	Last port to forward
+ * @exclude:	Bitmap of ports to exclude (may be NULL)
+ * @to:		Port to translate @first to when forwarding
+ * @flags:	Flags for forwarding entries
+ */
+static void fwd_rule_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,
+				  uint8_t flags)
+{
+	struct fwd_rule rule = {
+		.addr = addr ? *addr : inany_any6,
+		.ifname = { 0 },
+		.proto = proto,
+		.flags = flags,
+	};
+	char rulestr[FWD_RULE_STRLEN];
+	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++) {
+		if (exclude && bitmap_isset(exclude, base))
+			continue;
+
+		for (i = base; i <= last; i++) {
+			if (exclude && bitmap_isset(exclude, i))
+				break;
+		}
+
+		rule.first = base;
+		rule.last = i - 1;
+		rule.to = base + delta;
+
+		fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
+		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)));
+}
+
+/*
+ * 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_))
+
+/**
+ * fwd_rule_parse_ports() - Parse port range(s) specifier
+ * @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 fwd_rule_parse_ports(struct fwd_table *fwd, uint8_t proto,
+				 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, *ep;
+	uint8_t flags = 0;
+	unsigned i;
+
+	if (!strcmp(spec, "all")) {
+		/* Treat "all" as equivalent to "": all non-ephemeral ports */
+		spec = "";
+	}
+
+	/* Parse excluded ranges and "auto" in the first pass */
+	for_each_chunk(p, ep, spec, ",") {
+		struct port_range xrange;
+
+		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 (!(fwd->caps & FWD_CAP_SCAN)) {
+				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))
+			goto bad;
+		if (p != ep) /* Garbage after the range */
+			goto bad;
+
+		for (i = xrange.first; i <= xrange.last; i++)
+			bitmap_set(exclude, i);
+	}
+
+	if (exclude_only) {
+		/* Exclude ephemeral ports */
+		fwd_port_map_ephemeral(exclude);
+
+		fwd_rule_range_except(fwd, proto, addr, ifname,
+				      1, NUM_PORTS - 1, exclude,
+				      1, flags | FWD_WEAK);
+		return;
+	}
+
+	/* Now process base ranges, skipping exclusions */
+	for_each_chunk(p, ep, spec, ",") {
+		struct port_range orig_range, mapped_range;
+
+		if (!isdigit(*p))
+			/* 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 != ep) /* Garbage after the ranges */
+			goto bad;
+
+		fwd_rule_range_except(fwd, proto, addr, ifname,
+				      orig_range.first, orig_range.last,
+				      exclude,
+				      mapped_range.first, flags);
+	}
+
+	return;
+bad:
+	die("Invalid port specifier '%s'", spec);
+}
+
+/**
+ * fwd_rule_parse() - Parse port configuration option
+ * @optname:	Short option name, t, T, u, or U
+ * @optarg:	Option argument (port specification)
+ * @fwd:	Forwarding table to be updated
+ */
+void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd)
+{
+	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
+	char buf[BUFSIZ], *spec, *ifname = NULL;
+	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")) {
+		unsigned i;
+
+		for (i = 0; i < fwd->count; i++) {
+			if (fwd->rules[i].proto == proto) {
+				die("-%c none conflicts with previous options",
+					optname);
+			}
+		}
+		return;
+	}
+
+	strncpy(buf, optarg, sizeof(buf) - 1);
+
+	if ((spec = strchr(buf, '/'))) {
+		*spec = 0;
+		spec++;
+
+		if (optname != 't' && optname != 'u')
+			die("Listening address not allowed for -%c %s",
+			    optname, optarg);
+
+		if ((ifname = strchr(buf, '%'))) {
+			*ifname = 0;
+			ifname++;
+
+			/* spec is already advanced one past the '/',
+			 * so the length of the given ifname is:
+			 * (spec - ifname - 1)
+			 */
+			if (spec - ifname - 1 >= IFNAMSIZ) {
+				die("Interface name '%s' is too long (max %u)",
+				    ifname, IFNAMSIZ - 1);
+			}
+		}
+
+		if (ifname == buf + 1) {	/* Interface without address */
+			addr = NULL;
+		} else {
+			char *p = buf;
+
+			/* Allow square brackets for IPv4 too for convenience */
+			if (*p == '[' && p[strlen(p) - 1] == ']') {
+				p[strlen(p) - 1] = '\0';
+				p++;
+			}
+
+			if (!inany_pton(p, addr))
+				die("Bad forwarding address '%s'", p);
+		}
+	} else {
+		spec = buf;
+
+		addr = NULL;
+	}
+
+	if (optname == 'T' || optname == 'U') {
+		assert(!addr && !ifname);
+
+		if (!(fwd->caps & FWD_CAP_IFNAME)) {
+			warn(
+"SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'",
+			     optname, optarg);
+
+			if (fwd->caps & FWD_CAP_IPV4) {
+				fwd_rule_parse_ports(fwd, proto,
+						     &inany_loopback4, NULL,
+						     spec);
+			}
+			if (fwd->caps & FWD_CAP_IPV6) {
+				fwd_rule_parse_ports(fwd, proto,
+						     &inany_loopback6, NULL,
+						     spec);
+			}
+			return;
+		}
+
+		ifname = "lo";
+	}
+
+	if (ifname && !(fwd->caps & FWD_CAP_IFNAME)) {
+		die(
+"Device binding for '-%c %s' unsupported (requires kernel 5.7+)",
+		    optname, optarg);
+	}
+
+	fwd_rule_parse_ports(fwd, proto, addr, ifname, spec);
+}
diff --git a/fwd_rule.h b/fwd_rule.h
index 5c7b67aa..f0f4efda 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -19,6 +19,7 @@
 
 /* Number of ports for both TCP and UDP */
 #define	NUM_PORTS	(1U << 16)
+#define PORT_BITMAP_SIZE	DIV_ROUND_UP(NUM_PORTS, 8)
 
 /* Forwarding capability bits */
 #define FWD_CAP_IPV4		BIT(0)
@@ -54,8 +55,38 @@ struct fwd_rule {
 	uint8_t flags;
 };
 
+#define FWD_RULE_BITS	8
+#define MAX_FWD_RULES	MAX_FROM_BITS(FWD_RULE_BITS)
+
+/* Maximum number of listening sockets (per pif)
+ *
+ * Rationale: This lets us listen on every port for two addresses and two
+ * protocols (which we need for -T auto -U auto without SO_BINDTODEVICE), plus a
+ * comfortable number of extras.
+ */
+#define MAX_LISTEN_SOCKS	(NUM_PORTS * 5)
+
+/**
+ * struct fwd_table - Forwarding state (per initiating pif)
+ * @caps:	Forwarding capabilities for this initiating pif
+ * @count:	Number of forwarding rules
+ * @rules:	Array of forwarding rules
+ * @rulesocks:	Parallel array of @rules (@count valid entries) of pointers to
+ *		@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
+ */
+struct fwd_table {
+	uint32_t caps;
+	unsigned count;
+	struct fwd_rule rules[MAX_FWD_RULES];
+	int *rulesocks[MAX_FWD_RULES];
+	unsigned sock_count;
+	int socks[MAX_LISTEN_SOCKS];
+};
+
 void fwd_probe_ephemeral(void);
-void fwd_port_map_ephemeral(uint8_t *map);
 
 #define FWD_RULE_STRLEN					    \
 	(IPPROTO_STRLEN - 1				    \
@@ -67,7 +98,6 @@ void fwd_port_map_ephemeral(uint8_t *map);
 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);
+void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd);
 
 #endif /* FWD_RULE_H */
-- 
2.53.0