* [PATCH v2 01/13] Makefile: Add missing PESTO_HEADERS variable
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 02/13] conf: Use parameter instead of global in conf_nat() David Gibson
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
In several places we use a PESTO_HEADERS variable, with all the headers
that we need to build the pesto binary. However, we never define it.
This looks like an error introduced by a bad rebase of the series
introducing pesto before it was merged.
It turns out the fact we didn't list the headers was the only reason we
weren't getting unusedStructMember cppcheck warnings for pesto as we
already do for passt and passt-repair. So, reinstate that suppression for
pesto as well.
Fixes: 02236db32625 ("pesto: Introduce stub configuration tool")
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
Makefile | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index e8170e90..5ed0f702 100644
--- a/Makefile
+++ b/Makefile
@@ -55,6 +55,7 @@ PASST_HEADERS = arch.h arp.h bitmap.h checksum.h conf.h dhcp.h dhcpv6.h \
virtio.h vu_common.h
QRAP_HEADERS = arp.h ip.h passt.h util.h
PASST_REPAIR_HEADERS = linux_dep.h
+PESTO_HEADERS = bitmap.h common.h fwd_rule.h inany.h ip.h log.h pesto.h serialise.h
C := \#include <sys/random.h>\nint main(){int a=getrandom(0, 0, 0);}
ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
@@ -203,7 +204,8 @@ CPPCHECK_FLAGS = --std=c11 --error-exitcode=1 --enable=all --force \
else \
echo ""; \
fi) \
- --suppress=missingIncludeSystem
+ --suppress=missingIncludeSystem \
+ --suppress=unusedStructMember
cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck
@@ -212,10 +214,8 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck
$(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^
passt.cppcheck: BASE_CPPFLAGS += -UPESTO
-passt.cppcheck: CPPCHECK_FLAGS += --suppress=unusedStructMember
passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h
-passt-repair.cppcheck: CPPCHECK_FLAGS += --suppress=unusedStructMember
passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h
pesto.cppcheck: BASE_CPPFLAGS += -DPESTO
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 02/13] conf: Use parameter instead of global in conf_nat()
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
2026-07-01 5:31 ` [PATCH v2 01/13] Makefile: Add missing PESTO_HEADERS variable David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 03/13] parse: Start splitting out parsing helpers David Gibson
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
conf_nat() takes a parameter @arg for the argument it's parsing.
However on error we print instead optarg, the getopt() global. This
happens to be the same thing at the time of the call, but it's not the
right way to get to it.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/conf.c b/conf.c
index 4755a9f4..6ab8efec 100644
--- a/conf.c
+++ b/conf.c
@@ -1049,7 +1049,7 @@ static void conf_nat(const char *arg, struct in_addr *addr4,
!IN4_IS_ADDR_MULTICAST(addr4))
return;
- die("Invalid address to remap to host: %s", optarg);
+ die("Invalid address to remap to host: %s", arg);
}
/**
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 03/13] parse: Start splitting out parsing helpers
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
2026-07-01 5:31 ` [PATCH v2 01/13] Makefile: Add missing PESTO_HEADERS variable David Gibson
2026-07-01 5:31 ` [PATCH v2 02/13] conf: Use parameter instead of global in conf_nat() David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 04/13] conf: Remove duplicate parsing of -F option David Gibson
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
As we add more complexity to what forwarding rules are allowed, our
existing ad-hoc C parsing is starting to become quite awkward. We already
have some parts that resemble a very simple recursive[0] descent parser,
with composable subfunctions that consume as much input as they need,
rather than pre-splitting based on known delimiters.
Start extending this approach, by creating parse.[ch] with parsing helpers
with a uniform interface. Initially we start with very simple cases: the
parse_keyword() function from fwd_rule.c and another helper to check that
we've reached the end of input.
Rename parse_keyword() to parse_literal(), because it's not just useful
for "keywords" as such. We can use it in a bunch of additional places for
parsing delimiters and other symbols. This doesn't gain us a lot now but
will make things clearer as we use more such parser helpers.
[0] Except that the grammars we have aren't actually recursive, so neither
is the code.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
Makefile | 17 ++++++++------
conf.c | 5 +++-
fwd_rule.c | 33 +++++---------------------
parse.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
parse.h | 14 +++++++++++
5 files changed, 102 insertions(+), 35 deletions(-)
create mode 100644 parse.c
create mode 100644 parse.h
diff --git a/Makefile b/Makefile
index 5ed0f702..bd729c75 100644
--- a/Makefile
+++ b/Makefile
@@ -36,12 +36,13 @@ BASE_CFLAGS += -pedantic -Wall -Wextra -Wno-format-zero-length -Wformat-security
PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \
epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \
isolation.c lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c \
- passt.c pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c \
+ parse.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
-PESTO_SRCS = pesto.c bitmap.c fwd_rule.c inany.c ip.c lineread.c serialise.c
+PESTO_SRCS = pesto.c bitmap.c fwd_rule.c inany.c ip.c lineread.c parse.c \
+ serialise.c
SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_SRCS)
MANPAGES = passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1
@@ -49,13 +50,14 @@ MANPAGES = passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1
PASST_HEADERS = arch.h arp.h bitmap.h checksum.h conf.h dhcp.h dhcpv6.h \
epoll_ctl.h flow.h fwd.h fwd_rule.h flow_table.h icmp.h icmp_flow.h \
inany.h iov.h ip.h isolation.h lineread.h log.h migrate.h ndp.h \
- netlink.h packet.h passt.h pasta.h pcap.h pif.h repair.h serialise.h \
- siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h tcp_internal.h tcp_splice.h \
- tcp_vu.h udp.h udp_flow.h udp_internal.h udp_vu.h util.h vhost_user.h \
- virtio.h vu_common.h
+ netlink.h packet.h parse.h passt.h pasta.h pcap.h pif.h repair.h \
+ serialise.h siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h tcp_internal.h \
+ tcp_splice.h tcp_vu.h udp.h udp_flow.h udp_internal.h udp_vu.h util.h \
+ vhost_user.h virtio.h vu_common.h
QRAP_HEADERS = arp.h ip.h passt.h util.h
PASST_REPAIR_HEADERS = linux_dep.h
-PESTO_HEADERS = bitmap.h common.h fwd_rule.h inany.h ip.h log.h pesto.h serialise.h
+PESTO_HEADERS = bitmap.h common.h fwd_rule.h inany.h ip.h log.h parse.h \
+ pesto.h serialise.h
C := \#include <sys/random.h>\nint main(){int a=getrandom(0, 0, 0);}
ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
@@ -223,6 +225,7 @@ pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:bitmap.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.h
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:ip.h
+pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:parse.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:fwd_rule.c
pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h
diff --git a/conf.c b/conf.c
index 6ab8efec..ca6c859c 100644
--- a/conf.c
+++ b/conf.c
@@ -52,6 +52,7 @@
#include "conf.h"
#include "pesto.h"
#include "serialise.h"
+#include "parse.h"
#define NETNS_RUN_DIR "/run/netns"
@@ -1028,7 +1029,9 @@ static void conf_ugid(char *runas, uid_t *uid, gid_t *gid)
static void conf_nat(const char *arg, struct in_addr *addr4,
struct in6_addr *addr6, int *no_map_gw)
{
- if (strcmp(arg, "none") == 0) {
+ const char *p = arg;
+
+ if (parse_literal(&p, "none") && parse_eoi(p)) {
*addr4 = in4addr_any;
*addr6 = in6addr_any;
if (no_map_gw)
diff --git a/fwd_rule.c b/fwd_rule.c
index 04a0101d..d0e90402 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -24,6 +24,7 @@
#include "fwd_rule.h"
#include "lineread.h"
#include "log.h"
+#include "parse.h"
#include "serialise.h"
/* Ephemeral port range: values from RFC 6335 */
@@ -427,28 +428,6 @@ 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;
-}
-
/**
* fwd_rule_range_except() - Set up forwarding for a range of ports minus a
* bitmap of exclusions
@@ -568,7 +547,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
continue;
}
- if (parse_keyword(p, &p, "auto") == 0) {
+ if (parse_literal(&p, "auto")) {
if (p != ep) /* Garbage after the keyword */
goto bad;
@@ -582,9 +561,8 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
}
/* Should be an exclude range */
- if (*p != '~')
+ if (!parse_literal(&p, "~"))
goto bad;
- p++;
if (parse_port_range(p, &p, &xrange))
goto bad;
@@ -616,8 +594,9 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
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))
+ if (parse_literal(&p, ":")) {
+ /* There's a range to map to as well */
+ if (parse_port_range(p, &p, &mapped_range))
goto bad;
if ((mapped_range.last - mapped_range.first) !=
(orig_range.last - orig_range.first))
diff --git a/parse.c b/parse.c
new file mode 100644
index 00000000..4e3b527d
--- /dev/null
+++ b/parse.c
@@ -0,0 +1,68 @@
+// 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
+ *
+ * parse.c - Composable parsing helpers
+ *
+ * Copyright Red Hat
+ * Author: David Gibson <david@gibson.dropbear.id.au>
+ */
+
+#include <assert.h>
+#include <stddef.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdbool.h>
+#include <unistd.h>
+
+#include "common.h"
+
+/**
+ * DOC: Theory of Operation
+ *
+ * These are a number of primitives which can be combined into parsers for
+ * moderately complex input. For simpler composability, they have common
+ * conventions.
+ *
+ * - Functions return a bool indicating whether they successfully parsed.
+ * - Any specific output from the parse is as output parameters
+ * - First argument is always @cursor, a const char **.
+ * - On entry, *@cursor has the point to start parsing.
+ * - On successful exit, *@cursor is updated to the next character after the
+ * parsed portion of the input
+ * - On failure, *@cursor and any output arguments are not modified
+ *
+ * For brevity the common parameters and return information are omitted from the
+ * individual function documentation comments.
+ */
+
+/**
+ * parse_literal() - Parse a specified literal string
+ * @cursor: Point to parse from, updated on success
+ * @lit: Keyword to accept
+ */
+bool parse_literal(const char **cursor, const char *lit)
+{
+ size_t len = strlen(lit);
+
+ if (strlen(*cursor) < len || memcmp(*cursor, lit, len))
+ return false;
+
+ *cursor += len;
+ return true;
+}
+
+/**
+ * parse_eoi() - Parse end of input
+ * @cursor: Point to parse from
+ *
+ * Return: true if @p is at End of Input (\0), false otherwise
+ */
+bool parse_eoi(const char *cursor)
+{
+ return !(*cursor);
+}
diff --git a/parse.h b/parse.h
new file mode 100644
index 00000000..eb51a469
--- /dev/null
+++ b/parse.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright Red Hat
+ * Author: David Gibson <david@gibson.dropbear.id.au>
+ */
+
+#ifndef PARSE_H
+#define PARSE_H
+
+#include <stdbool.h>
+
+bool parse_literal(const char **cursor, const char *lit);
+bool parse_eoi(const char *cursor);
+
+#endif /* _PARSE_H */
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 04/13] conf: Remove duplicate parsing of -F option
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (2 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 03/13] parse: Start splitting out parsing helpers David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 05/13] conf: Clean up conf_ip4_prefix() David Gibson
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
The -F option is parsed in conf() along with everything else. However,
because it informs what fds we can close at startup, we also have a special
case parse of it in close_open_files().
At present we duplicate the parsing and validation code, which is a bit
risky. Avoid that with a conf_tap_fd() helper.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 29 +++++++++++++++++++----------
conf.h | 1 +
util.c | 12 +++---------
3 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/conf.c b/conf.c
index ca6c859c..32163f56 100644
--- a/conf.c
+++ b/conf.c
@@ -1153,6 +1153,24 @@ static void conf_sock_listen(const struct ctx *c)
die_perror("Couldn't add configuration socket to epoll");
}
+/**
+ * conf_tap_fd() - Read tap fd as supplied by -F command line option
+ * @arg: Argument to -F command line option
+ */
+int conf_tap_fd(const char *arg)
+{
+ long val;
+
+ errno = 0;
+ val = strtol(arg, NULL, 0);
+
+ if (errno || (val != STDIN_FILENO && val <= STDERR_FILENO) ||
+ val > INT_MAX)
+ die("Invalid --fd: %s", arg);
+
+ return val;
+}
+
/**
* conf() - Process command-line arguments and set configuration
* @c: Execution context
@@ -1253,7 +1271,6 @@ void conf(struct ctx *c, int argc, char **argv)
const char *logfile = NULL;
char *runas = NULL;
size_t logsize = 0;
- long fd_tap_opt;
int name, ret;
uid_t uid;
gid_t gid;
@@ -1506,15 +1523,7 @@ void conf(struct ctx *c, int argc, char **argv)
c->fd_control_listen = c->fd_control = -1;
break;
case 'F':
- errno = 0;
- fd_tap_opt = strtol(optarg, NULL, 0);
-
- if (errno ||
- (fd_tap_opt != STDIN_FILENO && fd_tap_opt <= STDERR_FILENO) ||
- fd_tap_opt > INT_MAX)
- die("Invalid --fd: %s", optarg);
-
- c->fd_tap = fd_tap_opt;
+ c->fd_tap = conf_tap_fd(optarg);
c->one_off = true;
*c->sock_path = 0;
break;
diff --git a/conf.h b/conf.h
index 16f97189..1fa1280e 100644
--- a/conf.h
+++ b/conf.h
@@ -7,6 +7,7 @@
#define CONF_H
enum passt_modes conf_mode(int argc, char *argv[]);
+int conf_tap_fd(const char *arg);
void conf(struct ctx *c, int argc, char **argv);
void conf_listen_handler(struct ctx *c, uint32_t events);
void conf_handler(struct ctx *c, uint32_t events);
diff --git a/util.c b/util.c
index 0aa4d42d..4bc5d6f8 100644
--- a/util.c
+++ b/util.c
@@ -38,6 +38,7 @@
#include "epoll_ctl.h"
#include "pasta.h"
#include "serialise.h"
+#include "conf.h"
#ifdef HAS_GETRANDOM
#include <sys/random.h>
#endif
@@ -939,15 +940,8 @@ void close_open_files(int argc, char **argv)
do {
name = getopt_long(argc, argv, "-:F:", optfd, NULL);
- if (name == 'F') {
- errno = 0;
- fd = strtol(optarg, NULL, 0);
-
- if (errno ||
- (fd != STDIN_FILENO && fd <= STDERR_FILENO) ||
- fd > INT_MAX)
- die("Invalid --fd: %s", optarg);
- }
+ if (name == 'F')
+ fd = conf_tap_fd(optarg);
} while (name != -1);
if (fd == -1) {
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 05/13] conf: Clean up conf_ip4_prefix()
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (3 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 04/13] conf: Remove duplicate parsing of -F option David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 06/13] parse: Add helper to parse unsigned integer values David Gibson
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
Restructure conf_ip4_prefix() so that success is the early exit path and
failure is the "default" path. Also move the error handling (die()) into
it from the caller.
This saves a handful of lines for now, and will make integration with
upcoming parsing changes nicer.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/conf.c b/conf.c
index 32163f56..ea299e18 100644
--- a/conf.c
+++ b/conf.c
@@ -350,9 +350,9 @@ static void conf_pasta_ns(int *netns_only, char *userns, char *netns,
/** conf_ip4_prefix() - Parse an IPv4 prefix length or netmask
* @arg: Netmask in dotted decimal or prefix length
*
- * Return: validated prefix length on success, -1 on failure
+ * Return: validated prefix length; dies on bad argument
*/
-static int conf_ip4_prefix(const char *arg)
+static uint8_t conf_ip4_prefix(const char *arg)
{
struct in_addr mask;
unsigned long len;
@@ -360,16 +360,16 @@ static int conf_ip4_prefix(const char *arg)
if (inet_pton(AF_INET, arg, &mask)) {
in_addr_t hmask = ntohl(mask.s_addr);
len = __builtin_popcount(hmask);
- if ((hmask << len) != 0)
- return -1;
+ if ((hmask << len) == 0)
+ return len;
} else {
errno = 0;
len = strtoul(arg, NULL, 0);
- if (len > 32 || errno)
- return -1;
+ if (!errno && len <= 32)
+ return len;
}
- return len;
+ die("Invalid prefix length: %s", arg);
}
/**
@@ -1606,20 +1606,13 @@ void conf(struct ctx *c, int argc, char **argv)
}
break;
}
- case 'n': {
- int plen;
-
+ case 'n':
if (addr_has_prefix_len)
die("Redundant prefix length specification");
- plen = conf_ip4_prefix(optarg);
- if (plen < 0)
- die("Invalid prefix length: %s", optarg);
-
- prefix_len_from_opt = plen + 96;
- c->ip4.prefix_len = plen;
+ c->ip4.prefix_len = conf_ip4_prefix(optarg);
+ prefix_len_from_opt = c->ip4.prefix_len + 96;
break;
- }
case 'M':
parse_mac(c->our_tap_mac, optarg);
break;
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 06/13] parse: Add helper to parse unsigned integer values
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (4 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 05/13] conf: Clean up conf_ip4_prefix() David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 07/13] parse: Move parse_port_range() to new parsing framework David Gibson
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
Most places we need to parse an integer encoded as a string, we use
strtol() or strtoul(). These already work a bit like descent parser
helpers, in that they consume as much as they can, but don't require the
number parsed to be the whole of the einput string. Make a wrapper to
parse unsigned integers as part of our parsing helper. This wrapper
handles the mildly fiddly error checking requirements for strtoul().
We replace a number, though not all, of our existing strtoul() uses with
the new parse_unsigned(). We also replace a number of strtol() use cases,
because, despite using that instead of strtoul() they are only used for
non-negative values. In some cases this makes the logic a little more
straightforward. In some other cases it will catch some error cases we
previously could have missed.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
Makefile | 1 -
conf.c | 51 +++++++++++++++++++++++++--------------------------
fwd_rule.c | 42 ++++++++++++++----------------------------
parse.c | 27 ++++++++++++++++++++++-----
parse.h | 1 +
5 files changed, 62 insertions(+), 60 deletions(-)
diff --git a/Makefile b/Makefile
index bd729c75..85d279c0 100644
--- a/Makefile
+++ b/Makefile
@@ -225,7 +225,6 @@ pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:bitmap.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.h
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:ip.h
-pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:parse.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:fwd_rule.c
pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h
diff --git a/conf.c b/conf.c
index ea299e18..bd357117 100644
--- a/conf.c
+++ b/conf.c
@@ -321,17 +321,16 @@ static void conf_pasta_ns(int *netns_only, char *userns, char *netns,
die("Both --netns and PID or command given");
if (optind + 1 == argc) {
- char *endptr;
- long pidval;
+ const char *p = argv[optind];
+ unsigned long pidval;
- pidval = strtol(argv[optind], &endptr, 10);
- if (!*endptr) {
+ if (parse_unsigned(&p, 10, &pidval) && parse_eoi(p)) {
/* Looks like a pid */
- if (pidval < 0 || pidval > INT_MAX)
+ if (pidval > INT_MAX)
die("Invalid PID %s", argv[optind]);
if (snprintf_check(netns, PATH_MAX,
- "/proc/%ld/ns/net", pidval))
+ "/proc/%lu/ns/net", pidval))
die_perror("Can't build netns path");
if (!*userns) {
@@ -354,6 +353,7 @@ static void conf_pasta_ns(int *netns_only, char *userns, char *netns,
*/
static uint8_t conf_ip4_prefix(const char *arg)
{
+ const char *p = arg;
struct in_addr mask;
unsigned long len;
@@ -362,11 +362,9 @@ static uint8_t conf_ip4_prefix(const char *arg)
len = __builtin_popcount(hmask);
if ((hmask << len) == 0)
return len;
- } else {
- errno = 0;
- len = strtoul(arg, NULL, 0);
- if (!errno && len <= 32)
- return len;
+ } else if (parse_unsigned(&p, 0, &len) && parse_eoi(p) &&
+ len <= 32) {
+ return len;
}
die("Invalid prefix length: %s", arg);
@@ -1159,13 +1157,12 @@ static void conf_sock_listen(const struct ctx *c)
*/
int conf_tap_fd(const char *arg)
{
- long val;
-
- errno = 0;
- val = strtol(arg, NULL, 0);
+ const char *p = arg;
+ unsigned long val;
- if (errno || (val != STDIN_FILENO && val <= STDERR_FILENO) ||
- val > INT_MAX)
+ if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) ||
+ val > INT_MAX ||
+ (val != STDIN_FILENO && val <= STDERR_FILENO))
die("Invalid --fd: %s", arg);
return val;
@@ -1288,6 +1285,8 @@ void conf(struct ctx *c, int argc, char **argv)
optind = 0;
do {
+ const char *p;
+
name = getopt_long(argc, argv, optstring, options, NULL);
switch (name) {
@@ -1368,14 +1367,17 @@ void conf(struct ctx *c, int argc, char **argv)
case 12:
runas = optarg;
break;
- case 13:
- errno = 0;
- logsize = strtol(optarg, NULL, 0);
+ case 13: {
+ unsigned long val;
- if (logsize < LOGFILE_SIZE_MIN || errno)
+ p = optarg;
+ if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) ||
+ val < LOGFILE_SIZE_MIN)
die("Invalid --log-size: %s", optarg);
+ logsize = val;
break;
+ }
case 14:
FPRINTF(stdout,
c->mode == MODE_PASTA ? "pasta " : "passt ");
@@ -1552,12 +1554,9 @@ void conf(struct ctx *c, int argc, char **argv)
break;
case 'm': {
unsigned long mtu;
- char *e;
-
- errno = 0;
- mtu = strtoul(optarg, &e, 0);
- if (errno || *e)
+ p = optarg;
+ if (!parse_unsigned(&p, 0, &mtu) || !parse_eoi(p))
die("Invalid MTU: %s", optarg);
if (mtu > max_mtu) {
diff --git a/fwd_rule.c b/fwd_rule.c
index d0e90402..d4b422d7 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -41,10 +41,11 @@ static in_port_t fwd_ephemeral_max = NUM_PORTS - 1;
*/
void fwd_probe_ephemeral(void)
{
- char *line, *tab, *end;
+ unsigned long min, max;
struct lineread lr;
- long min, max;
+ const char *p;
ssize_t len;
+ char *line;
int fd;
fd = open(PORT_RANGE_SYSCTL, O_RDONLY | O_CLOEXEC);
@@ -57,35 +58,20 @@ void fwd_probe_ephemeral(void)
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;
+ p = line;
+ if (len < 0 ||
+ !parse_unsigned(&p, 10, &min) ||
+ !parse_literal(&p, "\t") ||
+ !parse_unsigned(&p, 10, &max) ||
+ !parse_eoi(p) ||
+ min >= NUM_PORTS ||
+ max >= NUM_PORTS) {
+ warn("Unable to parse %s", PORT_RANGE_SYSCTL);
+ return;
+ }
fwd_ephemeral_min = min;
fwd_ephemeral_max = max;
-
- return;
-
-parse_err:
- warn("Unable to parse %s", PORT_RANGE_SYSCTL);
}
/**
diff --git a/parse.c b/parse.c
index 4e3b527d..9bd0e81f 100644
--- a/parse.c
+++ b/parse.c
@@ -12,14 +12,12 @@
* Author: David Gibson <david@gibson.dropbear.id.au>
*/
-#include <assert.h>
+#include <errno.h>
#include <stddef.h>
-#include <fcntl.h>
+#include <stdlib.h>
#include <string.h>
-#include <stdbool.h>
-#include <unistd.h>
-#include "common.h"
+#include "parse.h"
/**
* DOC: Theory of Operation
@@ -66,3 +64,22 @@ bool parse_eoi(const char *cursor)
{
return !(*cursor);
}
+
+/*
+ * parse_unsigned() - Parse an unsigned integer
+ * @base: Numeric base for string as strtoul(3)
+ * @valp: On success, updated with parsed value
+ */
+bool parse_unsigned(const char **cursor, int base, unsigned long *valp)
+{
+ const char *p = *cursor;
+ unsigned long val;
+
+ errno = 0;
+ val = strtoul(p, (char **)&p, base);
+ if (errno || p == *cursor)
+ return false;
+ *valp = val;
+ *cursor = p;
+ return true;
+}
diff --git a/parse.h b/parse.h
index eb51a469..4f38c28b 100644
--- a/parse.h
+++ b/parse.h
@@ -10,5 +10,6 @@
bool parse_literal(const char **cursor, const char *lit);
bool parse_eoi(const char *cursor);
+bool parse_unsigned(const char **cursor, int base, unsigned long *valp);
#endif /* _PARSE_H */
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 07/13] parse: Move parse_port_range() to new parsing framework
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (5 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 06/13] parse: Add helper to parse unsigned integer values David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 08/13] parse: Add helpers for parsing IP addresses David Gibson
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
parse_port_range() in fwd_rule.c takes an approach similar to that used
by the new parsing helpers in parse.c (and is partially inspiration for
it), but isn't quite the same. Move it into parse.c, and bring it into
line with that file's conventions.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
common.h | 3 +++
fwd_rule.c | 53 +++--------------------------------------------------
fwd_rule.h | 2 --
parse.c | 28 ++++++++++++++++++++++++++++
parse.h | 13 +++++++++++++
5 files changed, 47 insertions(+), 52 deletions(-)
diff --git a/common.h b/common.h
index f2518f66..16b9dc8c 100644
--- a/common.h
+++ b/common.h
@@ -113,4 +113,7 @@ static inline const char *strerror_(int errnum)
#define ntohll(x) (be64toh((x)))
#define htonll(x) (htobe64((x)))
+/* Number of ports for both TCP and UDP */
+#define NUM_PORTS (1U << 16)
+
#endif /* COMMON_H */
diff --git a/fwd_rule.c b/fwd_rule.c
index d4b422d7..0b85b17e 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -367,53 +367,6 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
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;
-}
-
/**
* fwd_rule_range_except() - Set up forwarding for a range of ports minus a
* bitmap of exclusions
@@ -550,7 +503,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
if (!parse_literal(&p, "~"))
goto bad;
- if (parse_port_range(p, &p, &xrange))
+ if (!parse_port_range(&p, &xrange))
goto bad;
if (p != ep) /* Garbage after the range */
goto bad;
@@ -577,12 +530,12 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
/* Already parsed */
continue;
- if (parse_port_range(p, &p, &orig_range))
+ if (!parse_port_range(&p, &orig_range))
goto bad;
if (parse_literal(&p, ":")) {
/* There's a range to map to as well */
- if (parse_port_range(p, &p, &mapped_range))
+ if (!parse_port_range(&p, &mapped_range))
goto bad;
if ((mapped_range.last - mapped_range.first) !=
(orig_range.last - orig_range.first))
diff --git a/fwd_rule.h b/fwd_rule.h
index ae9a3cb0..435be5bd 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -18,8 +18,6 @@
#include "inany.h"
#include "bitmap.h"
-/* 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 */
diff --git a/parse.c b/parse.c
index 9bd0e81f..ad210406 100644
--- a/parse.c
+++ b/parse.c
@@ -17,6 +17,7 @@
#include <stdlib.h>
#include <string.h>
+#include "common.h"
#include "parse.h"
/**
@@ -83,3 +84,30 @@ bool parse_unsigned(const char **cursor, int base, unsigned long *valp)
*cursor = p;
return true;
}
+
+/**
+ * parse_port_range() - Parse a range of port numbers '<first>[-<last>]'
+ * @range: Update with the parsed values on success
+ */
+bool parse_port_range(const char **cursor, struct port_range *range)
+{
+ unsigned long first, last;
+ const char *p = *cursor;
+
+ if (!parse_unsigned(&p, 10, &first))
+ return false;
+
+ last = first;
+
+ if (parse_literal(&p, "-"))
+ if (!parse_unsigned(&p, 10, &last))
+ return false;
+
+ if ((last < first) || (last >= NUM_PORTS))
+ return false;
+
+ range->first = first;
+ range->last = last;
+ *cursor = p;
+ return true;
+}
diff --git a/parse.h b/parse.h
index 4f38c28b..155b3995 100644
--- a/parse.h
+++ b/parse.h
@@ -7,9 +7,22 @@
#define PARSE_H
#include <stdbool.h>
+#include <netinet/in.h>
+
+/**
+ * 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;
+};
bool parse_literal(const char **cursor, const char *lit);
bool parse_eoi(const char *cursor);
bool parse_unsigned(const char **cursor, int base, unsigned long *valp);
+bool parse_port_range(const char **cursor, struct port_range *range);
#endif /* _PARSE_H */
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 08/13] parse: Add helpers for parsing IP addresses
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (6 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 07/13] parse: Move parse_port_range() to new parsing framework David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 09/13] conf: Move address configuration into helper function David Gibson
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson, Jon Maloy
parse_ipv[46]() are wrappers around inet_pton() that are more
convenient for use when the IP is part of a longer string, rather than
the entire string. parse_inany() replaces inany_pton() which again
will become more convenient for strings including IPs that aren't just
an IP.
For now we only have some simple and sometimes awkward use cases,
we'll replace these with more natural uses in future.
Cc: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
Makefile | 1 +
conf.c | 6 ++--
fwd_rule.c | 21 ++++---------
inany.c | 24 ++------------
inany.h | 1 -
parse.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
parse.h | 4 +++
7 files changed, 109 insertions(+), 39 deletions(-)
diff --git a/Makefile b/Makefile
index 85d279c0..e2b22ddf 100644
--- a/Makefile
+++ b/Makefile
@@ -227,4 +227,5 @@ pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:ip.h
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:fwd_rule.c
+pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:parse.c
pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h
diff --git a/conf.c b/conf.c
index bd357117..2d1895d4 100644
--- a/conf.c
+++ b/conf.c
@@ -357,7 +357,7 @@ static uint8_t conf_ip4_prefix(const char *arg)
struct in_addr mask;
unsigned long len;
- if (inet_pton(AF_INET, arg, &mask)) {
+ if (parse_ipv4(&p, &mask) && parse_eoi(p)) {
in_addr_t hmask = ntohl(mask.s_addr);
len = __builtin_popcount(hmask);
if ((hmask << len) == 0)
@@ -1577,7 +1577,9 @@ void conf(struct ctx *c, int argc, char **argv)
if (addr_has_prefix_len && prefix_len_from_opt)
die("Redundant prefix length specification");
- if (!addr_has_prefix_len && !inany_pton(optarg, &addr))
+ p = optarg;
+ if (!addr_has_prefix_len &&
+ !(parse_inany(&p, &addr) && parse_eoi(p)))
die("Invalid address: %s", optarg);
if (prefix_len_from_opt && inany_v4(&addr))
diff --git a/fwd_rule.c b/fwd_rule.c
index 0b85b17e..6d7ec2c5 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -595,6 +595,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
strncpy(buf, optarg, sizeof(buf) - 1);
if ((spec = strchr(buf, '/'))) {
+ const char *p = buf;
+
*spec = 0;
spec++;
@@ -616,22 +618,11 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
}
}
- if (ifname == buf + 1) { /* Interface without address */
+ if (ifname == buf + 1 || /* Interface without address */
+ !strcmp(buf, "*")) /* Explicit wildcard 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 (strcmp(p, "*") == 0)
- addr = NULL;
- else if (!inany_pton(p, &addr_buf))
- die("Bad forwarding address '%s'", p);
- }
+ else if (!parse_inany(&p, &addr_buf) && parse_eoi(p))
+ die("Bad forwarding address '%s'", buf);
} else {
spec = buf;
diff --git a/inany.c b/inany.c
index 23faf3ff..154f08b5 100644
--- a/inany.c
+++ b/inany.c
@@ -27,6 +27,7 @@
#include "ip.h"
#include "inany.h"
#include "fwd.h"
+#include "parse.h"
const union inany_addr inany_loopback4 = INANY_INIT4(IN4ADDR_LOOPBACK_INIT);
const union inany_addr inany_any4 = INANY_INIT4(IN4ADDR_ANY_INIT);
@@ -70,26 +71,6 @@ const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size)
return inet_ntop(AF_INET6, &src->a6, dst, size);
}
-/** inany_pton - Parse an IPv[46] address from text format
- * @src: IPv[46] address
- * @dst: output buffer, filled with parsed address
- *
- * Return: on success, 1, if no parseable address is found, 0
- */
-int inany_pton(const char *src, union inany_addr *dst)
-{
- if (inet_pton(AF_INET, src, &dst->v4mapped.a4)) {
- memset(&dst->v4mapped.zero, 0, sizeof(dst->v4mapped.zero));
- memset(&dst->v4mapped.one, 0xff, sizeof(dst->v4mapped.one));
- return 1;
- }
-
- if (inet_pton(AF_INET6, src, &dst->a6))
- return 1;
-
- return 0;
-}
-
/**
* inany_prefix_pton() - Parse an IPv[46] address with prefix length
* @src: IPv[46] address and prefix length string in CIDR format
@@ -104,6 +85,7 @@ int inany_prefix_pton(const char *src, union inany_addr *dst,
char astr[INANY_ADDRSTRLEN] = { 0 };
size_t alen = strcspn(src, "/");
const char *pstr = &src[alen + 1];
+ const char *p = astr;
unsigned long plen;
char *end;
@@ -129,7 +111,7 @@ int inany_prefix_pton(const char *src, union inany_addr *dst,
return 1;
}
- if (inany_pton(astr, dst)) {
+ if (parse_inany(&p, dst) && parse_eoi(p)) {
if (plen > 32)
return 0;
*prefix_len = plen + 96;
diff --git a/inany.h b/inany.h
index 6bf3ecaa..93d98368 100644
--- a/inany.h
+++ b/inany.h
@@ -303,7 +303,6 @@ static inline int inany_from_sockaddr(union inany_addr *dst, in_port_t *port,
bool inany_matches(const union inany_addr *a, const union inany_addr *b);
const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size);
-int inany_pton(const char *src, union inany_addr *dst);
int inany_prefix_pton(const char *src, union inany_addr *dst,
uint8_t *prefix_len);
diff --git a/parse.c b/parse.c
index ad210406..c8d3a9b0 100644
--- a/parse.c
+++ b/parse.c
@@ -16,9 +16,12 @@
#include <stddef.h>
#include <stdlib.h>
#include <string.h>
+#include <limits.h>
+#include <arpa/inet.h>
#include "common.h"
#include "parse.h"
+#include "inany.h"
/**
* DOC: Theory of Operation
@@ -111,3 +114,91 @@ bool parse_port_range(const char **cursor, struct port_range *range)
*cursor = p;
return true;
}
+
+/**
+ * parse_ipv4() - Parse an IPv4 address from a string
+ * @abuf: On success, updated with parsed address
+ */
+bool parse_ipv4(const char **cursor, struct in_addr *abuf)
+{
+ /* Brackets are not typical on IPv4, but allow for consistency */
+ const char *p = *cursor;
+ bool bracket = parse_literal(&p, "[");
+ char buf[INET_ADDRSTRLEN];
+ struct in_addr addr;
+ size_t len;
+
+ if (bracket)
+ len = strcspn(p, "]");
+ else
+ len = strspn(p, "0123456789.");
+
+ if (len >= sizeof(buf))
+ return false;
+ memcpy(buf, p, len);
+ buf[len] = '\0';
+ p += len;
+
+ if (!inet_pton(AF_INET, buf, &addr))
+ return false;
+
+ if (bracket && !parse_literal(&p, "]"))
+ return false;
+
+ *cursor = p;
+ *abuf = addr;
+ return true;
+}
+
+/**
+ * parse_ipv6() - Parse an IPv6 address from a string
+ * @abuf: On success, updated with parsed address
+ */
+static bool parse_ipv6(const char **cursor, struct in6_addr *abuf)
+{
+ const char *p = *cursor;
+ bool bracket = parse_literal(&p, "[");
+ char buf[INET6_ADDRSTRLEN];
+ struct in6_addr addr;
+ size_t len;
+
+ if (bracket)
+ len = strcspn(p, "]");
+ else
+ len = strspn(p, "0123456789aAbBcCdDeEfF:.");
+
+ if (len >= sizeof(buf))
+ return false;
+ memcpy(buf, p, len);
+ buf[len] = '\0';
+ p += len;
+
+ if (!inet_pton(AF_INET6, buf, &addr))
+ return false;
+
+ if (bracket && !parse_literal(&p, "]"))
+ return false;
+
+ *cursor = p;
+ *abuf = addr;
+ return true;
+}
+
+/**
+ * parse_inany() - Parse an IPv4 or IPv6 address from a string
+ * @addr: On success, updated with parsed address
+ */
+bool parse_inany(const char **cursor, union inany_addr *addr)
+{
+ struct in_addr a4;
+
+ if (parse_ipv6(cursor, &addr->a6))
+ return true;
+
+ if (parse_ipv4(cursor, &a4)) {
+ *addr = inany_from_v4(a4);
+ return true;
+ }
+
+ return false;
+}
diff --git a/parse.h b/parse.h
index 155b3995..2820a065 100644
--- a/parse.h
+++ b/parse.h
@@ -9,6 +9,8 @@
#include <stdbool.h>
#include <netinet/in.h>
+union inany_addr;
+
/**
* port_range() - Represents a non-empty range of ports
* @first: First port number in the range
@@ -24,5 +26,7 @@ bool parse_literal(const char **cursor, const char *lit);
bool parse_eoi(const char *cursor);
bool parse_unsigned(const char **cursor, int base, unsigned long *valp);
bool parse_port_range(const char **cursor, struct port_range *range);
+bool parse_ipv4(const char **cursor, struct in_addr *abuf);
+bool parse_inany(const char **cursor, union inany_addr *addr);
#endif /* _PARSE_H */
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 09/13] conf: Move address configuration into helper function
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (7 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 08/13] parse: Add helpers for parsing IP addresses David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 10/13] conf: Remove unnecessary mode checks from conf_addr() David Gibson
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson, Jon Maloy
The handling of the -a option is getting complex enough that it's pretty
bulky inside its switch label. Move it into a new conf_addr() function.
We also rename the bulky addr_has_prefix_len and prefix_len_from_opt
variables to the terser 'opt_a_is_prefix' and 'opt_n', since they are
specifically about those command line options.
Cc: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 96 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 53 insertions(+), 43 deletions(-)
diff --git a/conf.c b/conf.c
index 2d1895d4..3614776c 100644
--- a/conf.c
+++ b/conf.c
@@ -1168,6 +1168,53 @@ int conf_tap_fd(const char *arg)
return val;
}
+/**
+ * conf_addr() - Configure guest address with -a option
+ * @c: Execution context
+ * @arg: -a command line argument
+ * @opt_n: Value from -n option, if any
+ */
+static bool conf_addr(struct ctx *c, char *arg, uint8_t opt_n)
+{
+ union inany_addr addr;
+ const char *p = arg;
+ uint8_t prefix_len;
+ bool is_prefix;
+
+ is_prefix = inany_prefix_pton(arg, &addr, &prefix_len);
+
+ if (is_prefix && opt_n)
+ die("Redundant prefix length specification");
+
+ if (!is_prefix &&
+ !(parse_inany(&p, &addr) && parse_eoi(p)))
+ die("Invalid address: %s", arg);
+
+ if (opt_n && inany_v4(&addr))
+ prefix_len = opt_n;
+ else if (!is_prefix)
+ prefix_len = inany_default_prefix_len(&addr);
+
+ if (inany_is_unspecified(&addr) || inany_is_multicast(&addr) ||
+ inany_is_loopback(&addr) || IN6_IS_ADDR_V4COMPAT(&addr.a6))
+ die("Invalid address: %s", arg);
+
+ if (inany_v4(&addr)) {
+ c->ip4.addr = *inany_v4(&addr);
+ c->ip4.prefix_len = prefix_len - 96;
+ c->ip4.addr_fixed = true;
+ if (c->mode == MODE_PASTA)
+ c->ip4.no_copy_addrs = true;
+ } else {
+ c->ip6.addr = addr.a6;
+ c->ip6.addr_fixed = true;
+ if (c->mode == MODE_PASTA)
+ c->ip6.no_copy_addrs = true;
+ }
+
+ return is_prefix;
+}
+
/**
* conf() - Process command-line arguments and set configuration
* @c: Execution context
@@ -1262,12 +1309,12 @@ void conf(struct ctx *c, int argc, char **argv)
unsigned dns4_idx = 0, dns6_idx = 0;
unsigned long max_mtu = IP_MAX_MTU;
struct fqdn *dnss = c->dns_search;
- bool addr_has_prefix_len = false;
- uint8_t prefix_len_from_opt = 0;
unsigned int ifi4 = 0, ifi6 = 0;
+ bool opt_a_is_prefix = false;
const char *logfile = NULL;
char *runas = NULL;
size_t logsize = 0;
+ uint8_t opt_n = 0;
int name, ret;
uid_t uid;
gid_t gid;
@@ -1567,52 +1614,15 @@ void conf(struct ctx *c, int argc, char **argv)
c->mtu = mtu;
break;
}
- case 'a': {
- union inany_addr addr;
- uint8_t prefix_len;
-
- addr_has_prefix_len = inany_prefix_pton(optarg, &addr,
- &prefix_len);
-
- if (addr_has_prefix_len && prefix_len_from_opt)
- die("Redundant prefix length specification");
-
- p = optarg;
- if (!addr_has_prefix_len &&
- !(parse_inany(&p, &addr) && parse_eoi(p)))
- die("Invalid address: %s", optarg);
-
- if (prefix_len_from_opt && inany_v4(&addr))
- prefix_len = prefix_len_from_opt;
- else if (!addr_has_prefix_len)
- prefix_len = inany_default_prefix_len(&addr);
-
- if (inany_is_unspecified(&addr) ||
- inany_is_multicast(&addr) ||
- inany_is_loopback(&addr) ||
- IN6_IS_ADDR_V4COMPAT(&addr.a6))
- die("Invalid address: %s", optarg);
-
- if (inany_v4(&addr)) {
- c->ip4.addr = *inany_v4(&addr);
- c->ip4.prefix_len = prefix_len - 96;
- c->ip4.addr_fixed = true;
- if (c->mode == MODE_PASTA)
- c->ip4.no_copy_addrs = true;
- } else {
- c->ip6.addr = addr.a6;
- c->ip6.addr_fixed = true;
- if (c->mode == MODE_PASTA)
- c->ip6.no_copy_addrs = true;
- }
+ case 'a':
+ opt_a_is_prefix = conf_addr(c, optarg, opt_n);
break;
- }
case 'n':
- if (addr_has_prefix_len)
+ if (opt_a_is_prefix)
die("Redundant prefix length specification");
c->ip4.prefix_len = conf_ip4_prefix(optarg);
- prefix_len_from_opt = c->ip4.prefix_len + 96;
+ opt_n = c->ip4.prefix_len + 96;
break;
case 'M':
parse_mac(c->our_tap_mac, optarg);
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 10/13] conf: Remove unnecessary mode checks from conf_addr()
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (8 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 09/13] conf: Move address configuration into helper function David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 11/13] conf: Use new parsing tools to handle -a option David Gibson
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
conf_addr() sets c->ip[46]no_copy_addrs conditional upon being in pasta
mode. That sort of makes sense, since address copying is only a thing for
pasta mode. However, setting the variables anyway is harmless, and
arguably more logically consistent. If we had a way of copying addresses
for passt mode (or some future mode), it would still be incorrect to do so
in these circumstances.
So, make the assignments unconditional.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/conf.c b/conf.c
index 3614776c..d1889e0c 100644
--- a/conf.c
+++ b/conf.c
@@ -1203,13 +1203,11 @@ static bool conf_addr(struct ctx *c, char *arg, uint8_t opt_n)
c->ip4.addr = *inany_v4(&addr);
c->ip4.prefix_len = prefix_len - 96;
c->ip4.addr_fixed = true;
- if (c->mode == MODE_PASTA)
- c->ip4.no_copy_addrs = true;
+ c->ip4.no_copy_addrs = true;
} else {
c->ip6.addr = addr.a6;
c->ip6.addr_fixed = true;
- if (c->mode == MODE_PASTA)
- c->ip6.no_copy_addrs = true;
+ c->ip6.no_copy_addrs = true;
}
return is_prefix;
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 11/13] conf: Use new parsing tools to handle -a option
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (9 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 10/13] conf: Remove unnecessary mode checks from conf_addr() David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 12/13] fwd_rule: Allow "all" port specs to be combined with other options David Gibson
2026-07-01 5:31 ` [PATCH v2 13/13] fwd_rule: Rewrite forward rule parsing using parse.c helpers David Gibson
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson, Jon Maloy
The -a command line option can take either an address prefix, or a bare
address. Current parsing of this is pretty awkward, using the special
purpose helper inany_prefix_pton(). With the new incremental parsing
helpers this can be done more naturally. Rework it to use them.
This does requiring extending parse_inany() to parse_inany_() which also
reports the format of the address as parse, as opposed to the family of
the resulting address. This is so that ::ffff:192.0.1.1/112 will be
correctly interpreted the same as 192.0.1.1/16, rather than the
nonsensical 192.0.0.1/112.
Cc: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
Makefile | 1 -
conf.c | 57 +++++++++++++++++++++++++++++++++++++-------------------
inany.c | 50 -------------------------------------------------
inany.h | 2 --
parse.c | 17 ++++++++++++++---
parse.h | 5 ++++-
6 files changed, 56 insertions(+), 76 deletions(-)
diff --git a/Makefile b/Makefile
index e2b22ddf..5757aeff 100644
--- a/Makefile
+++ b/Makefile
@@ -223,7 +223,6 @@ passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repa
pesto.cppcheck: BASE_CPPFLAGS += -DPESTO
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:bitmap.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.h
-pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:ip.h
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:fwd_rule.c
diff --git a/conf.c b/conf.c
index d1889e0c..c4a36dee 100644
--- a/conf.c
+++ b/conf.c
@@ -1176,31 +1176,45 @@ int conf_tap_fd(const char *arg)
*/
static bool conf_addr(struct ctx *c, char *arg, uint8_t opt_n)
{
+ unsigned long prefix_len;
+ const struct in_addr *a4;
union inany_addr addr;
+ sa_family_t parse_af;
const char *p = arg;
- uint8_t prefix_len;
bool is_prefix;
- is_prefix = inany_prefix_pton(arg, &addr, &prefix_len);
-
- if (is_prefix && opt_n)
- die("Redundant prefix length specification");
-
- if (!is_prefix &&
- !(parse_inany(&p, &addr) && parse_eoi(p)))
- die("Invalid address: %s", arg);
-
- if (opt_n && inany_v4(&addr))
- prefix_len = opt_n;
- else if (!is_prefix)
- prefix_len = inany_default_prefix_len(&addr);
+ if (!parse_inany_(&p, &addr, &parse_af))
+ goto bad;
+ a4 = inany_v4(&addr);
+
+ if ((is_prefix = parse_literal(&p, "/"))) {
+ /* Prefix length included in -a option */
+ if (!parse_unsigned(&p, 10, &prefix_len))
+ goto bad;
+ if (opt_n)
+ die("Redundant prefix length specification");
+ if (parse_af == AF_INET) {
+ if (prefix_len > 32)
+ goto bad_prefix;
+ prefix_len += 96;
+ } else if (prefix_len > 128) {
+ goto bad_prefix;
+ }
+ } else {
+ /* Get prefix length from elsewhere */
+ if (opt_n && a4)
+ prefix_len = opt_n;
+ else
+ prefix_len = inany_default_prefix_len(&addr);
+ }
- if (inany_is_unspecified(&addr) || inany_is_multicast(&addr) ||
- inany_is_loopback(&addr) || IN6_IS_ADDR_V4COMPAT(&addr.a6))
- die("Invalid address: %s", arg);
+ if (!parse_eoi(p) ||
+ !inany_is_unicast(&addr) ||
+ inany_is_loopback(&addr))
+ goto bad;
- if (inany_v4(&addr)) {
- c->ip4.addr = *inany_v4(&addr);
+ if (a4) {
+ c->ip4.addr = *a4;
c->ip4.prefix_len = prefix_len - 96;
c->ip4.addr_fixed = true;
c->ip4.no_copy_addrs = true;
@@ -1211,6 +1225,11 @@ static bool conf_addr(struct ctx *c, char *arg, uint8_t opt_n)
}
return is_prefix;
+
+bad_prefix:
+ die("Invalid prefix length: %s", arg);
+bad:
+ die("Invalid guest address: %s", arg);
}
/**
diff --git a/inany.c b/inany.c
index 154f08b5..120c9387 100644
--- a/inany.c
+++ b/inany.c
@@ -70,53 +70,3 @@ const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size)
return inet_ntop(AF_INET6, &src->a6, dst, size);
}
-
-/**
- * inany_prefix_pton() - Parse an IPv[46] address with prefix length
- * @src: IPv[46] address and prefix length string in CIDR format
- * @dst: Output buffer, filled with parsed address
- * @prefix_len: Prefix length, to be filled in IPv6 format
- *
- * Return: 1 on success, 0 if no parseable address or prefix is found
- */
-int inany_prefix_pton(const char *src, union inany_addr *dst,
- uint8_t *prefix_len)
-{
- char astr[INANY_ADDRSTRLEN] = { 0 };
- size_t alen = strcspn(src, "/");
- const char *pstr = &src[alen + 1];
- const char *p = astr;
- unsigned long plen;
- char *end;
-
- if (alen >= INANY_ADDRSTRLEN)
- return 0;
-
- if (src[alen] != '/')
- return 0;
-
- strncpy(astr, src, alen);
-
- /* Read prefix length */
- errno = 0;
- plen = strtoul(pstr, &end, 10);
- if (errno || *end || plen > 128)
- return 0;
-
- /* Read address */
- if (inet_pton(AF_INET6, astr, dst)) {
- if (inany_v4(dst) && plen < 96)
- return 0;
- *prefix_len = plen;
- return 1;
- }
-
- if (parse_inany(&p, dst) && parse_eoi(p)) {
- if (plen > 32)
- return 0;
- *prefix_len = plen + 96;
- return 1;
- }
-
- return 0;
-}
diff --git a/inany.h b/inany.h
index 93d98368..5b176ccf 100644
--- a/inany.h
+++ b/inany.h
@@ -303,7 +303,5 @@ static inline int inany_from_sockaddr(union inany_addr *dst, in_port_t *port,
bool inany_matches(const union inany_addr *a, const union inany_addr *b);
const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size);
-int inany_prefix_pton(const char *src, union inany_addr *dst,
- uint8_t *prefix_len);
#endif /* INANY_H */
diff --git a/parse.c b/parse.c
index c8d3a9b0..24178873 100644
--- a/parse.c
+++ b/parse.c
@@ -185,18 +185,29 @@ static bool parse_ipv6(const char **cursor, struct in6_addr *abuf)
}
/**
- * parse_inany() - Parse an IPv4 or IPv6 address from a string
+ * parse_inany_() - Parse an IPv4 or IPv6 address from a string
* @addr: On success, updated with parsed address
+ * @parse_af: On success, updated with the format of the parsed address
+ *
+ * @parseaf is updated to reflect the string format, not the final address
+ * family. So "::ffff:192.0.1.1", will set @parseaf to AF_INET6, despite being
+ * a IPv4-mapped address.
*/
-bool parse_inany(const char **cursor, union inany_addr *addr)
+bool parse_inany_(const char **cursor, union inany_addr *addr,
+ sa_family_t *parse_af)
{
struct in_addr a4;
- if (parse_ipv6(cursor, &addr->a6))
+ if (parse_ipv6(cursor, &addr->a6)) {
+ if (parse_af)
+ *parse_af = AF_INET6;
return true;
+ }
if (parse_ipv4(cursor, &a4)) {
*addr = inany_from_v4(a4);
+ if (parse_af)
+ *parse_af = AF_INET;
return true;
}
diff --git a/parse.h b/parse.h
index 2820a065..ab1d5adb 100644
--- a/parse.h
+++ b/parse.h
@@ -27,6 +27,9 @@ bool parse_eoi(const char *cursor);
bool parse_unsigned(const char **cursor, int base, unsigned long *valp);
bool parse_port_range(const char **cursor, struct port_range *range);
bool parse_ipv4(const char **cursor, struct in_addr *abuf);
-bool parse_inany(const char **cursor, union inany_addr *addr);
+bool parse_inany_(const char **cursor, union inany_addr *addr,
+ sa_family_t *parse_af);
+
+#define parse_inany(cursor, addr) parse_inany_((cursor), (addr), NULL)
#endif /* _PARSE_H */
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 12/13] fwd_rule: Allow "all" port specs to be combined with other options
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (10 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 11/13] conf: Use new parsing tools to handle -a option David Gibson
@ 2026-07-01 5:31 ` David Gibson
2026-07-01 5:31 ` [PATCH v2 13/13] fwd_rule: Rewrite forward rule parsing using parse.c helpers David Gibson
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
Currently we handle -t all and the like as a special case, it can't be
combined with other port specifier options. Remove that restriction,
allowing combined options like:
-t all,~9999 # Forward everything non-ephemeral except 9999
-t all,auto # Equivalent to -t auto
-t all,33000 # Forward non-ephemeral plus port 33,000
This isn't particularly useful immediately, but will become important for
destination address specification - it provides a place to attach the
target address for "all" or exclude only mappings. It will also work
better with some parsing reworks we want to make.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
fwd_rule.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/fwd_rule.c b/fwd_rule.c
index 6d7ec2c5..b14df340 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -471,20 +471,13 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
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;
+ /* Include range, parse later */
+ if (parse_literal(&p, "all") || isdigit(*p))
continue;
- }
if (parse_literal(&p, "auto")) {
if (p != ep) /* Garbage after the keyword */
@@ -512,20 +505,18 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
bitmap_set(exclude, i);
}
- if (exclude_only) {
- /* Exclude ephemeral ports */
- fwd_port_map_ephemeral(exclude);
-
- fwd_rule_range_except(fwd, del, 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;
+ /* Handle "all" like exclude only */
+ if (parse_literal(&p, "all")) {
+ if (p != ep) /* Garbage after the keyword */
+ goto bad;
+
+ continue;
+ }
+
if (!isdigit(*p))
/* Already parsed */
continue;
@@ -533,6 +524,8 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
if (!parse_port_range(&p, &orig_range))
goto bad;
+ exclude_only = false;
+
if (parse_literal(&p, ":")) {
/* There's a range to map to as well */
if (!parse_port_range(&p, &mapped_range))
@@ -553,6 +546,14 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
mapped_range.first, flags);
}
+ /* Finally handle "all" and exclude only specs */
+ if (exclude_only) {
+ fwd_port_map_ephemeral(exclude);
+
+ fwd_rule_range_except(fwd, del, proto, addr, ifname,
+ 1, NUM_PORTS - 1, exclude,
+ 1, flags | FWD_WEAK);
+ }
return;
bad:
die("Invalid port specifier '%s'", spec);
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 13/13] fwd_rule: Rewrite forward rule parsing using parse.c helpers
2026-07-01 5:31 [PATCH v2 00/13] Rework option parsing in preparation for destination remapping David Gibson
` (11 preceding siblings ...)
2026-07-01 5:31 ` [PATCH v2 12/13] fwd_rule: Allow "all" port specs to be combined with other options David Gibson
@ 2026-07-01 5:31 ` David Gibson
12 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2026-07-01 5:31 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
Forwarding rules for the -[tTuU] options are parsed in fwd_rule_parse()
and fwd_rule_parse_ports(). Currently those use a mix of loosely
recursive descent parsing helpers, consuming input from left to right,
and ad-hoc C parsing - searching for delimiters with strchr() or the like
and breaking down on that basis.
As well as being a slightly awkward mix, the current approach will not work
for adding target addresses to the rules: we can't easily split the
listening from target parts first, because the ':' delimiter could appear
multiple times for multiple port ranges, or in IPv6 addresses. However,
without splitting the target part first, we can't first split off the
listening address and interface because the '/' delimiter could appear in
the target portion, but not the listening portion, e.g.:
-t 12345:192.0.1.1/12345
To address this, rewrite the entirety of the parsing so we consume the
input left to right in a more-or-less recursive descent manner. This means
we no longer rely on certain delimiters having a single meaning over the
whole input, just the next part of it.
Because of the semantics of port specs, we need to make several passes
over the list of comma separated chunks. Previously, some of the logic was
duplicated between the two passes, making it fragile. Now, we introduce
a parse_port_chunk() helper which we re-use in each pass to make it clearer
we parse exactly the same way on each pass.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
fwd_rule.c | 268 +++++++++++++++++++++++++++++++----------------------
parse.c | 29 ++++++
parse.h | 2 +
3 files changed, 187 insertions(+), 112 deletions(-)
diff --git a/fwd_rule.c b/fwd_rule.c
index b14df340..ca409eaf 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -439,17 +439,102 @@ 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
+/**
+ * enum fwd_port_chunk_kind - Kind of port specifier piece
+ * @CHUNK_ALL "all"
+ * @CHUNK_AUTO "auto"
+ * @CHUNK_EXCLUDE "~1111[-2222]"
+ * @CHUNK_INCLUDE "1111[-2222][:3333[-4444]]"
+ */
+enum fwd_port_chunk_kind {
+ CHUNK_ALL,
+ CHUNK_AUTO,
+ CHUNK_EXCLUDE,
+ CHUNK_INCLUDE,
+};
+
+/**
+ * parse_port_chunk() - Parse one chunk of a port specifier
+ * @cursor: Parsing point (see parse.c)
+ * @kindp: Updated with kind of chunk we parsed
+ * @lrange: Updated with listening port range (for INCLUDE & EXCLUDE)
+ * @trange: Updated with target port range (for INCLUDE)
+ */
+static bool parse_port_chunk(const char **cursor,
+ enum fwd_port_chunk_kind *kindp,
+ struct port_range *lrange,
+ struct port_range *trange)
+{
+ struct port_range lr = { 0 }, tr = { 0 };
+ enum fwd_port_chunk_kind kind;
+ const char *p = *cursor;
+
+ if (parse_literal(&p, "all")) {
+ kind = CHUNK_ALL;
+ } else if (parse_literal(&p, "auto")) {
+ kind = CHUNK_AUTO;
+ } else if (parse_literal(&p, "~")) {
+ kind = CHUNK_EXCLUDE;
+ if (!parse_port_range(&p, &lr))
+ return false;
+ } else if (parse_port_range(&p, &lr)) {
+ kind = CHUNK_INCLUDE;
+
+ if (parse_literal(&p, ":")) {
+ if (!parse_port_range(&p, &tr))
+ return false;
+ } else {
+ tr = lr;
+ }
+ } else {
+ return false;
+ }
+
+ *kindp = kind;
+ *lrange = lr;
+ if (trange)
+ *trange = tr;
+ *cursor = p;
+ return true;
+}
+
+/**
+ * parse_addrifname() - Parse ADDRESS[%IFNAME]/
+ * @cursor: Parsing cursor (see parse.c)
+ * @addr: Updated with parsed inany address (NULL for *)
+ * @abuf: Buffer to store address
+ * @ifname: Updated with parsed interface name ("" if none)
*/
-#define for_each_chunk(p_, ep_, s_, sep_) \
- for ((p_) = (s_); \
- (ep_) = (p_) + strcspn((p_), (sep_)), *(p_); \
- (p_) = *(ep_) ? (ep_) + 1 : (ep_))
+static bool parse_addrifname(const char **cursor,
+ const union inany_addr **addr,
+ union inany_addr *abuf,
+ char *ifname)
+{
+ union inany_addr atmp = inany_any6;
+ char iftmp[IFNAMSIZ] = {0};
+ const char *p;
+
+ if (p = *cursor,
+ parse_inany(&p, &atmp) &&
+ parse_ifspec(&p, iftmp) &&
+ parse_literal(&p, "/")) {
+ /* Specific listening address */
+ *addr = abuf;
+ } else if (p = *cursor,
+ parse_literal(&p, "*"),
+ parse_ifspec(&p, iftmp) &&
+ parse_literal(&p, "/")) {
+ /* Missing or "*" address */
+ *addr = NULL;
+ } else {
+ return false;
+ }
+
+ *abuf = atmp;
+ memcpy(ifname, iftmp, IFNAMSIZ);
+ *cursor = p;
+ return true;
+}
/**
* fwd_rule_parse_ports() - Parse port range(s) specifier
@@ -466,87 +551,70 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
const char *spec)
{
uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
+ enum fwd_port_chunk_kind kind;
+ struct port_range lrange;
bool exclude_only = true;
- const char *p, *ep;
uint8_t flags = 0;
+ const char *p;
unsigned i;
- /* Parse excluded ranges and "auto" in the first pass */
- for_each_chunk(p, ep, spec, ",") {
- struct port_range xrange;
-
- /* Include range, parse later */
- if (parse_literal(&p, "all") || isdigit(*p))
- continue;
-
- if (parse_literal(&p, "auto")) {
- if (p != ep) /* Garbage after the keyword */
- goto bad;
+ /* Consider excluded ranges and "auto" in the first pass */
+ p = spec;
+ do {
+ if (!parse_port_chunk(&p, &kind, &lrange, NULL))
+ goto bad;
+ switch (kind) {
+ case CHUNK_AUTO:
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 (!parse_literal(&p, "~"))
- goto bad;
-
- if (!parse_port_range(&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);
- }
-
- /* Now process base ranges, skipping exclusions */
- for_each_chunk(p, ep, spec, ",") {
- struct port_range orig_range, mapped_range;
-
- /* Handle "all" like exclude only */
- if (parse_literal(&p, "all")) {
- if (p != ep) /* Garbage after the keyword */
- goto bad;
+ break;
- continue;
+ case CHUNK_EXCLUDE:
+ for (i = lrange.first; i <= lrange.last; i++)
+ bitmap_set(exclude, i);
+ break;
+ default:
+ ; /* Handled later */
}
+ } while (parse_literal(&p, ","));
- if (!isdigit(*p))
- /* Already parsed */
- continue;
+ /* Consider included ranges in next pass */
+ p = spec;
+ do {
+ struct port_range trange;
- if (!parse_port_range(&p, &orig_range))
+ if (!parse_port_chunk(&p, &kind, &lrange, &trange))
goto bad;
- exclude_only = false;
+ switch (kind) {
+ case CHUNK_AUTO: /* already handled */
+ case CHUNK_EXCLUDE: /* already handled */
+ case CHUNK_ALL: /* handled later */
+ continue;
- if (parse_literal(&p, ":")) {
- /* There's a range to map to as well */
- if (!parse_port_range(&p, &mapped_range))
- goto bad;
- if ((mapped_range.last - mapped_range.first) !=
- (orig_range.last - orig_range.first))
+ case CHUNK_INCLUDE:
+ exclude_only = false;
+ if (trange.last - trange.first !=
+ lrange.last - lrange.first)
goto bad;
- } else {
- mapped_range = orig_range;
- }
- if (p != ep) /* Garbage after the ranges */
+ fwd_rule_range_except(fwd, del, proto, addr, ifname,
+ lrange.first, lrange.last,
+ exclude, trange.first, flags);
+ break;
+ default:
goto bad;
+ }
+ } while (parse_literal(&p, ","));
- fwd_rule_range_except(fwd, del, proto, addr, ifname,
- orig_range.first, orig_range.last,
- exclude,
- mapped_range.first, flags);
- }
+ if (!parse_eoi(p))
+ goto bad; /* trailing garbage */
- /* Finally handle "all" and exclude only specs */
+ /* Finally handle "all" and exclude only cases */
if (exclude_only) {
fwd_port_map_ephemeral(exclude);
@@ -569,10 +637,11 @@ bad:
void fwd_rule_parse(char optname, bool del, const char *optarg,
struct fwd_table *fwd)
{
- char buf[BUFSIZ], *spec, *ifname = NULL;
- union inany_addr addr_buf = inany_any6;
- const union inany_addr *addr = &addr_buf;
+ const union inany_addr *addr;
+ union inany_addr addr_buf;
+ char ifname[IFNAMSIZ];
uint8_t proto;
+ const char *p;
if (optname == 't' || optname == 'T')
proto = IPPROTO_TCP;
@@ -581,7 +650,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
else
assert(0);
- if (!strcmp(optarg, "none")) {
+ if (p = optarg,
+ parse_literal(&p, "none") && parse_eoi(p)) {
unsigned i;
for (i = 0; i < fwd->count; i++) {
@@ -593,45 +663,19 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
return;
}
- strncpy(buf, optarg, sizeof(buf) - 1);
-
- if ((spec = strchr(buf, '/'))) {
- const char *p = buf;
-
- *spec = 0;
- spec++;
-
- if (optname != 't' && optname != 'u')
+ if (p = optarg,
+ parse_addrifname(&p, &addr, &addr_buf, ifname)) {
+ 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 */
- !strcmp(buf, "*")) /* Explicit wildcard address */
- addr = NULL;
- else if (!parse_inany(&p, &addr_buf) && parse_eoi(p))
- die("Bad forwarding address '%s'", buf);
} else {
- spec = buf;
-
+ /* No address or ifname */
addr = NULL;
+ ifname[0] = '\0';
}
if (optname == 'T' || optname == 'U') {
- assert(!addr && !ifname);
+ assert(!addr && !*ifname);
if (!(fwd->caps & FWD_CAP_IFNAME)) {
warn(
@@ -640,18 +684,18 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
if (fwd->caps & FWD_CAP_IPV4) {
fwd_rule_parse_ports(fwd, del, proto,
- &inany_loopback4, NULL,
- spec);
+ &inany_loopback4, NULL, p);
}
if (fwd->caps & FWD_CAP_IPV6) {
fwd_rule_parse_ports(fwd, del, proto,
- &inany_loopback6, NULL,
- spec);
+ &inany_loopback6, NULL, p);
}
return;
}
- ifname = "lo";
+ static_assert(sizeof("lo") <= sizeof(ifname),
+ "ifname buffer too small");
+ strcpy(ifname, "lo");
}
/* No need for dual stack if we only have one IP version */
@@ -660,13 +704,13 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
else if (!addr && !(fwd->caps & FWD_CAP_IPV6))
addr = &inany_any4;
- if (ifname && !(fwd->caps & FWD_CAP_IFNAME)) {
+ 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, del, proto, addr, ifname, spec);
+ fwd_rule_parse_ports(fwd, del, proto, addr, *ifname ? ifname : NULL, p);
}
/**
diff --git a/parse.c b/parse.c
index 24178873..73f8ffb4 100644
--- a/parse.c
+++ b/parse.c
@@ -18,6 +18,7 @@
#include <string.h>
#include <limits.h>
#include <arpa/inet.h>
+#include <net/if.h>
#include "common.h"
#include "parse.h"
@@ -213,3 +214,31 @@ bool parse_inany_(const char **cursor, union inany_addr *addr,
return false;
}
+
+/**
+ * parse_ifspec() - Parse a interface name specifier (starting with %)
+ * @ifname: On success updated with parsed name (must have IFNAMSIZ space)
+ *
+ * This will accept a missing specifier (empty string), setting ifname to ""
+ */
+bool parse_ifspec(const char **cursor, char *ifname)
+{
+ const char *p = *cursor;
+ size_t len;
+
+ if (!parse_literal(&p, "%")) {
+ /* No interface specifier */
+ ifname[0] = '\0';
+ return true;
+ }
+
+ /* ifnames can have anything that's not '/', '.' or whitespace */
+ len = strcspn(p, "/. \f\n\r\t\v");
+ if (!len || len >= IFNAMSIZ)
+ return false;
+
+ memcpy(ifname, p, len);
+ ifname[len] = '\0';
+ *cursor = p + len;
+ return true;
+}
diff --git a/parse.h b/parse.h
index ab1d5adb..257b9c58 100644
--- a/parse.h
+++ b/parse.h
@@ -32,4 +32,6 @@ bool parse_inany_(const char **cursor, union inany_addr *addr,
#define parse_inany(cursor, addr) parse_inany_((cursor), (addr), NULL)
+bool parse_ifspec(const char **cursor, char *ifname);
+
#endif /* _PARSE_H */
--
2.54.0
^ permalink raw reply [flat|nested] 14+ messages in thread