* [PATCH v2 0/2] Target address mapping
@ 2026-07-02 7:32 David Gibson
2026-07-02 7:32 ` [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules David Gibson
2026-07-02 7:32 ` [PATCH v2 2/2] fwd, fwd_rule: Implement configurable target address mapping David Gibson
0 siblings, 2 replies; 6+ messages in thread
From: David Gibson @ 2026-07-02 7:32 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Here's another draft for target address remapping. I've now given
this some basic, though not extensive testing.
Based on v4 of my parsing rework series.
v2:
* Remove the patch altering --host-lo-to-ns-lo; it's not relevant to
the change at hand.
* Allow either just target address, just target port or both to be
specified
* Allow target address to be specified with all
* Prohibit (for now) forwarding between IPv4 and IPv6 (including from
to a specific address of either family).
* Add the target address when formatting rules for debug display.
* Updated man page for new syntax
David Gibson (1):
fwd_rule: Parse target addresses for forwarding rules
Stefano Brivio (1):
fwd, fwd_rule: Implement configurable target address mapping
fwd.c | 8 +++--
fwd_rule.c | 101 +++++++++++++++++++++++++++++++++++++++++++----------
fwd_rule.h | 7 ++--
passt.1 | 31 +++++++++++-----
pesto.h | 6 +++-
5 files changed, 121 insertions(+), 32 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules
2026-07-02 7:32 [PATCH v2 0/2] Target address mapping David Gibson
@ 2026-07-02 7:32 ` David Gibson
2026-07-04 15:27 ` Stefano Brivio
2026-07-02 7:32 ` [PATCH v2 2/2] fwd, fwd_rule: Implement configurable target address mapping David Gibson
1 sibling, 1 reply; 6+ messages in thread
From: David Gibson @ 2026-07-02 7:32 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
Extend the parsing of forwarding rules (-[tu]) to allow the destination
address on the target side to be specified. For now just parse them, and
give an error if we try to create rules with a specified target address.
We'll implement the actual forwarding logic in another patch.
Format (for either command line or pesto):
-t 2222:192.0.2.1/2222
This should work along with all the other bits, that is, say:
-t 192.0.2.1%eth0/2222-2225:192.0.2.2/22-25
FIXME: Ban for -[TU] for now
FIXME: Check interaction with splice handling
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
[dwg: Syntax from Stefano's earlier draft, largely rewritten on top of new
parsing helpers]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
fwd_rule.c | 90 +++++++++++++++++++++++++++++++++++++++++++++---------
passt.1 | 31 +++++++++++++------
2 files changed, 98 insertions(+), 23 deletions(-)
diff --git a/fwd_rule.c b/fwd_rule.c
index ef35e1b4..bed29ed9 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -378,14 +378,17 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
* @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
+ * @tgt_addr: Destination address on the target side
+ * @tgt_first: Destination port to use for @first on the target side
* @flags: Flags for forwarding entries
*/
static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
uint8_t proto, const union inany_addr *addr,
const char *ifname,
uint16_t first, uint16_t last,
- const uint8_t *exclude, uint16_t to,
+ const uint8_t *exclude,
+ const union inany_addr *tgt_addr,
+ uint16_t tgt_first,
uint8_t flags)
{
struct fwd_rule rule = {
@@ -394,10 +397,31 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
.proto = proto,
.flags = flags,
};
+ unsigned delta = tgt_first - first;
char rulestr[FWD_RULE_STRLEN];
- unsigned delta = to - first;
unsigned base, i;
+ if (tgt_addr && !inany_is_unspecified(tgt_addr)) {
+ char astr[INANY_ADDRSTRLEN];
+
+ if (!inany_is_unicast(tgt_addr)) {
+ die("Target address %s is not unicast",
+ inany_ntop(tgt_addr, astr, sizeof(astr)));
+ }
+ if (!addr || !!inany_v4(addr) != !!inany_v4(tgt_addr)) {
+ char bstr[INANY_ADDRSTRLEN];
+
+ die(
+"Forwarding between IP versions (%s => %s) not implemented",
+ inany_ntop(addr, bstr, sizeof(bstr)),
+ inany_ntop(tgt_addr, astr, sizeof(astr)));
+ }
+
+ info("Target address: %s",
+ inany_ntop(tgt_addr, astr, sizeof(astr)));
+ die("Target address remapping not yet implemented");
+ }
+
if (!addr)
rule.flags |= FWD_DUAL_STACK_ANY;
if (ifname) {
@@ -458,19 +482,31 @@ enum fwd_port_chunk_kind {
* @cursor: Parsing point (see parse.c)
* @kindp: Updated with kind of chunk we parsed
* @lrange: Updated with listening port range (for INCLUDE & EXCLUDE)
+ * @taddr: Updated with target address (for INCLUDE & ALL)
* @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,
+ union inany_addr *taddr,
struct port_range *trange)
{
struct port_range lr = { 0 }, tr = { 0 };
+ union inany_addr taddr_tmp = inany_any6;
enum fwd_port_chunk_kind kind;
const char *p = *cursor;
if (parse_literal(&p, "all")) {
+ const char *tgtspec = p;
+
kind = CHUNK_ALL;
+ if (p = tgtspec,
+ parse_literal(&p, ":") &&
+ parse_inany(&p, &taddr_tmp)) {
+ /* Target address */
+ } else {
+ p = tgtspec;
+ }
} else if (parse_literal(&p, "auto")) {
kind = CHUNK_AUTO;
} else if (parse_literal(&p, "~")) {
@@ -478,12 +514,29 @@ static bool parse_port_chunk(const char **cursor,
if (!parse_port_range(&p, &lr))
return false;
} else if (parse_port_range(&p, &lr)) {
- kind = CHUNK_INCLUDE;
+ const char *tgtspec = p;
- if (parse_literal(&p, ":")) {
- if (!parse_port_range(&p, &tr))
- return false;
+ kind = CHUNK_INCLUDE;
+ if (p = tgtspec,
+ parse_literal(&p, ":") &&
+ parse_inany(&p, &taddr_tmp) &&
+ parse_literal(&p, "/") &&
+ parse_port_range(&p, &tr)) {
+ /* Target address & range */
+ } else if (p = tgtspec,
+ parse_literal(&p, ":") &&
+ parse_inany(&p, &taddr_tmp)) {
+ /* Target address only */
+ tr = lr;
+ } else if (p = tgtspec,
+ parse_literal(&p, ":") &&
+ parse_port_range(&p, &tr)) {
+ /* Target range only */
+ taddr_tmp = inany_any6;
} else {
+ p = tgtspec;
+ /* No target specification */
+ taddr_tmp = inany_any6;
tr = lr;
}
} else {
@@ -492,6 +545,8 @@ static bool parse_port_chunk(const char **cursor,
*kindp = kind;
*lrange = lr;
+ if (taddr)
+ *taddr = taddr_tmp;
if (trange)
*trange = tr;
*cursor = p;
@@ -551,6 +606,7 @@ 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 };
+ union inany_addr all_taddr = inany_any6;
enum fwd_port_chunk_kind kind;
struct port_range lrange;
bool exclude_only = true;
@@ -561,7 +617,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
/* Consider excluded ranges and "auto" in the first pass */
p = spec;
do {
- if (!parse_port_chunk(&p, &kind, &lrange, NULL))
+ if (!parse_port_chunk(&p, &kind, &lrange, NULL, NULL))
goto bad;
switch (kind) {
@@ -586,14 +642,19 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
p = spec;
do {
struct port_range trange;
+ union inany_addr taddr;
- if (!parse_port_chunk(&p, &kind, &lrange, &trange))
+ if (!parse_port_chunk(&p, &kind, &lrange, &taddr, &trange))
goto bad;
switch (kind) {
- case CHUNK_AUTO: /* already handled */
- case CHUNK_EXCLUDE: /* already handled */
- case CHUNK_ALL: /* handled later */
+ case CHUNK_AUTO:
+ case CHUNK_EXCLUDE:
+ continue; /* already handled */
+
+ case CHUNK_ALL:
+ /* Save the address to use later */
+ all_taddr = taddr;
continue;
case CHUNK_INCLUDE:
@@ -604,7 +665,8 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
fwd_rule_range_except(fwd, del, proto, addr, ifname,
lrange.first, lrange.last,
- exclude, trange.first, flags);
+ exclude, &taddr, trange.first,
+ flags);
break;
default:
goto bad;
@@ -620,7 +682,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
fwd_rule_range_except(fwd, del, proto, addr, ifname,
1, NUM_PORTS - 1, exclude,
- 1, flags | FWD_WEAK);
+ &all_taddr, 1, flags | FWD_WEAK);
}
return;
bad:
diff --git a/passt.1 b/passt.1
index c3722ef9..9ece0e0c 100644
--- a/passt.1
+++ b/passt.1
@@ -449,12 +449,15 @@ interface name (since Linux 5.7) can be specified.
\fIports\fR is 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]]
+\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR[\fItoaddr\fR\fB/\fR]\fItofirst\fR[\fB-\fR\fItolast\fR]]
+.TP
+\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItoaddr\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.
+(inclusive) to ports between \fItofirst\fR and \fItolast\fR to address
+\fItoaddr\fR. If \fItoaddr\fR is omitted, automatically determine the
+guest or namespace address. 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]
@@ -462,11 +465,13 @@ Exclude range. Don't forward port numbers between \fIfirst\fR and
\fIlast\fR. This takes precedences over include ranges.
.TP
-.BR all
+.BR all\fR[\fB:\fItoaddr\fR]
Forward all unbound, non-ephemeral ports, not covered by exclude
-ranges above, 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.
+ranges above, as permitted by current capabilities, to the
+corresponding ports on address \fItoaddr\fR. If \fItoaddr\fR is
+omitted, automatically determine the guest or namespace address. 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
@@ -516,6 +521,14 @@ Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22
-t %eth0/22
Forward local port 22, bound to any address on interface eth0, to port 22
.TP
+-t 0.0.0.0/5000:192.0.2.5/6000
+Forward local port 5000, bound to any IPv4 address, to port 6000 on address 192.0.2.5.
+.TP
+-t 127.0.0.6/all:192.0.2.6
+For the local address 127.0.0.6 forward all unbound, non-ephemeral
+ports as permitted by current capabilities to the corresponding port
+on 192.0.2.6.
+.TP
-t 2000-5000,~3000-3010
Forward local ports between 2000 and 5000, except for those between 3000 and
3010
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] fwd, fwd_rule: Implement configurable target address mapping
2026-07-02 7:32 [PATCH v2 0/2] Target address mapping David Gibson
2026-07-02 7:32 ` [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules David Gibson
@ 2026-07-02 7:32 ` David Gibson
1 sibling, 0 replies; 6+ messages in thread
From: David Gibson @ 2026-07-02 7:32 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: David Gibson
From: Stefano Brivio <sbrivio@redhat.com>
Add a 'taddr' field to forwarding rules, which controls the destination
address on the target side. Since changing the structure alters the pesto
update protocol, bump the protocol version number
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
[dwg: Split from option parsing code, added protocol version bump,
explicitly exclude splicing with target address for now]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
fwd.c | 8 ++++++--
fwd_rule.c | 19 +++++++++++--------
fwd_rule.h | 7 +++++--
pesto.h | 6 +++++-
4 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/fwd.c b/fwd.c
index 042158cf..3ae25fde 100644
--- a/fwd.c
+++ b/fwd.c
@@ -1023,7 +1023,9 @@ uint8_t fwd_nat_from_host(const struct ctx *c,
/* Common for spliced and non-spliced cases */
tgt->eport = rule->to + (ini->oport - rule->first);
- if (!c->no_splice && inany_is_loopback(&ini->eaddr) &&
+ /* TODO: Allow splicing with specified target address */
+ if (!c->no_splice && inany_is_unspecified(&rule->taddr) &&
+ inany_is_loopback(&ini->eaddr) &&
(proto == IPPROTO_TCP || proto == IPPROTO_UDP)) {
/* spliceable */
@@ -1074,7 +1076,9 @@ uint8_t fwd_nat_from_host(const struct ctx *c,
}
tgt->oport = ini->eport;
- if (inany_v4(&tgt->oaddr)) {
+ if (!inany_is_unspecified(&rule->taddr)) {
+ tgt->eaddr = rule->taddr;
+ } else if (inany_v4(&tgt->oaddr)) {
tgt->eaddr = inany_from_v4(c->ip4.addr_seen);
} else {
if (inany_is_linklocal6(&tgt->oaddr))
diff --git a/fwd_rule.c b/fwd_rule.c
index bed29ed9..d1ecd39e 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -115,10 +115,15 @@ __attribute__((noinline))
const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size)
{
const char *percent = *rule->ifname ? "%" : "";
+ char taddr[INANY_ADDRSTRLEN] = { 0 };
const char *weak = "", *scan = "";
char addr[INANY_ADDRSTRLEN];
int len;
+ if (!inany_is_unspecified(&rule->taddr)) {
+ (void)snprintf(taddr, sizeof(taddr), "%s:",
+ inany_ntop(&rule->taddr, addr, sizeof(addr)));
+ }
inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr));
if (rule->flags & FWD_WEAK)
weak = " (best effort)";
@@ -127,16 +132,17 @@ const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size)
if (rule->first == rule->last) {
len = snprintf(dst, size,
- "%s [%s]%s%s:%hu => %hu %s%s",
+ "%s [%s]%s%s:%hu => %s%hu %s%s",
ipproto_name(rule->proto), addr, percent,
- rule->ifname, rule->first, rule->to, weak, scan);
+ rule->ifname, rule->first,
+ taddr, 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",
+ "%s [%s]%s%s:%hu-%hu => %s%hu-%hu %s%s",
ipproto_name(rule->proto), addr, percent,
rule->ifname, rule->first, rule->last,
- rule->to, tolast, weak, scan);
+ taddr, rule->to, tolast, weak, scan);
}
if (len < 0 || (size_t)len >= size)
@@ -393,6 +399,7 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
{
struct fwd_rule rule = {
.addr = addr ? *addr : inany_any6,
+ .taddr = tgt_addr ? *tgt_addr : inany_any6,
.ifname = { 0 },
.proto = proto,
.flags = flags,
@@ -416,10 +423,6 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
inany_ntop(addr, bstr, sizeof(bstr)),
inany_ntop(tgt_addr, astr, sizeof(astr)));
}
-
- info("Target address: %s",
- inany_ntop(tgt_addr, astr, sizeof(astr)));
- die("Target address remapping not yet implemented");
}
if (!addr)
diff --git a/fwd_rule.h b/fwd_rule.h
index 435be5bd..a8788318 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -33,6 +33,7 @@
/**
* struct fwd_rule - Forwarding rule governing a range of ports
* @addr: Address to forward from
+ * @taddr: Target side destination address
* @ifname: Interface to forward from
* @first: First port number to forward
* @last: Last port number to forward
@@ -45,6 +46,7 @@
*/
struct fwd_rule {
union inany_addr addr;
+ union inany_addr taddr;
char ifname[IFNAMSIZ];
in_port_t first;
in_port_t last;
@@ -91,10 +93,11 @@ void fwd_probe_ephemeral(void);
#define FWD_RULE_STRLEN \
(IPPROTO_STRLEN - 1 \
- + INANY_ADDRSTRLEN - 1 \
+ + INANY_ADDRSTRLEN - 1 /* listen addr */ \
+ + INANY_ADDRSTRLEN - 1 /* target addr */ \
+ IFNAMSIZ - 1 \
+ 4 * (UINT16_STRLEN - 1) \
- + sizeof(" []%:- => - (best effort) (auto-scan)"))
+ + 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);
diff --git a/pesto.h b/pesto.h
index 980cc17d..8db701b4 100644
--- a/pesto.h
+++ b/pesto.h
@@ -15,7 +15,11 @@
#define PESTO_SERVER_MAGIC "basil:s"
/* Version 0 is reserved for unreleased / unsupported experimental versions */
-#define PESTO_PROTOCOL_VERSION 1
+/* Version 1 had no target address field in struct fwd_rule. It was released,
+ * but was little enough used that we decided not to implement backwards
+ * compatiblity code (i.e. a v2 pesto will not work with a v1 pasta)
+ */
+#define PESTO_PROTOCOL_VERSION 2
/* Maximum size of a pif name, including \0 */
#define PIF_NAME_SIZE (128)
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules
2026-07-02 7:32 ` [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules David Gibson
@ 2026-07-04 15:27 ` Stefano Brivio
2026-07-06 3:08 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Brivio @ 2026-07-04 15:27 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev
On Thu, 2 Jul 2026 17:32:14 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> Extend the parsing of forwarding rules (-[tu]) to allow the destination
> address on the target side to be specified. For now just parse them, and
> give an error if we try to create rules with a specified target address.
> We'll implement the actual forwarding logic in another patch.
>
> Format (for either command line or pesto):
> -t 2222:192.0.2.1/2222
>
> This should work along with all the other bits, that is, say:
> -t 192.0.2.1%eth0/2222-2225:192.0.2.2/22-25
>
> FIXME: Ban for -[TU] for now
> FIXME: Check interaction with splice handling
>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> [dwg: Syntax from Stefano's earlier draft, largely rewritten on top of new
> parsing helpers]
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> fwd_rule.c | 90 +++++++++++++++++++++++++++++++++++++++++++++---------
> passt.1 | 31 +++++++++++++------
> 2 files changed, 98 insertions(+), 23 deletions(-)
>
> diff --git a/fwd_rule.c b/fwd_rule.c
> index ef35e1b4..bed29ed9 100644
> --- a/fwd_rule.c
> +++ b/fwd_rule.c
> @@ -378,14 +378,17 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
> * @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
> + * @tgt_addr: Destination address on the target side
> + * @tgt_first: Destination port to use for @first on the target side
> * @flags: Flags for forwarding entries
> */
> static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> uint8_t proto, const union inany_addr *addr,
> const char *ifname,
> uint16_t first, uint16_t last,
> - const uint8_t *exclude, uint16_t to,
> + const uint8_t *exclude,
> + const union inany_addr *tgt_addr,
> + uint16_t tgt_first,
> uint8_t flags)
> {
> struct fwd_rule rule = {
> @@ -394,10 +397,31 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> .proto = proto,
> .flags = flags,
> };
> + unsigned delta = tgt_first - first;
> char rulestr[FWD_RULE_STRLEN];
> - unsigned delta = to - first;
> unsigned base, i;
>
> + if (tgt_addr && !inany_is_unspecified(tgt_addr)) {
> + char astr[INANY_ADDRSTRLEN];
> +
> + if (!inany_is_unicast(tgt_addr)) {
> + die("Target address %s is not unicast",
> + inany_ntop(tgt_addr, astr, sizeof(astr)));
> + }
Nit: an extra newline here would help readability.
> + if (!addr || !!inany_v4(addr) != !!inany_v4(tgt_addr)) {
I wonder: if !addr, shouldn't we replace it, for the moment, with
inany_any4 or inany_any6 depending on !!inany_v4(tgt_addr)?
I see that an empty address already works with -4 or -6, and I
understand that, by doing this, we'll have a change in behaviour once
forwarding between IP versions is implemented.
But it will take a while before we get there, and, meanwhile, I guess
almost all users will just want to do stuff like -t 8080:192.0.2.1/80,
just to hit:
Forwarding between IP versions (* => 192.0.2.1) not implemented
which isn't obvious if you aren't familiar with the implementation. I
can also picture a constant flow of incoming tickets as a result.
The change in behaviour once we implement forwarding between IP
versions, by the way, looks a bit like an extension rather than a real
change that could reasonably cause trouble to anybody.
> + char bstr[INANY_ADDRSTRLEN];
> +
> + die(
> +"Forwarding between IP versions (%s => %s) not implemented",
> + inany_ntop(addr, bstr, sizeof(bstr)),
> + inany_ntop(tgt_addr, astr, sizeof(astr)));
> + }
About both validations: shouldn't they live in fwd_rule_add(), where
we already have this kind of stuff? Or at least be duplicated there?
Now, I tried dropping those in pesto (#ifndef PESTO ...) to see what
happens, and I couldn't really spot anything really bad, not with
multicast and not with mixing IPv4 and IPv6:
14 20.647728 fe80::1 → ::ffff:127.0.0.1 82 TCP 57812 → 5201 [SYN] Seq=0 Win=65535 Len=0 MSS=61440 WS=256
but still I wonder if we shouldn't make it a bit more robust.
> +
> + info("Target address: %s",
> + inany_ntop(tgt_addr, astr, sizeof(astr)));
> + die("Target address remapping not yet implemented");
> + }
> +
> if (!addr)
> rule.flags |= FWD_DUAL_STACK_ANY;
> if (ifname) {
> @@ -458,19 +482,31 @@ enum fwd_port_chunk_kind {
> * @cursor: Parsing point (see parse.c)
> * @kindp: Updated with kind of chunk we parsed
> * @lrange: Updated with listening port range (for INCLUDE & EXCLUDE)
> + * @taddr: Updated with target address (for INCLUDE & ALL)
> * @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,
> + union inany_addr *taddr,
> struct port_range *trange)
> {
> struct port_range lr = { 0 }, tr = { 0 };
> + union inany_addr taddr_tmp = inany_any6;
> enum fwd_port_chunk_kind kind;
> const char *p = *cursor;
>
> if (parse_literal(&p, "all")) {
> + const char *tgtspec = p;
> +
> kind = CHUNK_ALL;
> + if (p = tgtspec,
> + parse_literal(&p, ":") &&
> + parse_inany(&p, &taddr_tmp)) {
> + /* Target address */
> + } else {
> + p = tgtspec;
> + }
> } else if (parse_literal(&p, "auto")) {
> kind = CHUNK_AUTO;
> } else if (parse_literal(&p, "~")) {
> @@ -478,12 +514,29 @@ static bool parse_port_chunk(const char **cursor,
> if (!parse_port_range(&p, &lr))
> return false;
> } else if (parse_port_range(&p, &lr)) {
> - kind = CHUNK_INCLUDE;
> + const char *tgtspec = p;
>
> - if (parse_literal(&p, ":")) {
> - if (!parse_port_range(&p, &tr))
> - return false;
> + kind = CHUNK_INCLUDE;
> + if (p = tgtspec,
> + parse_literal(&p, ":") &&
> + parse_inany(&p, &taddr_tmp) &&
> + parse_literal(&p, "/") &&
> + parse_port_range(&p, &tr)) {
> + /* Target address & range */
> + } else if (p = tgtspec,
> + parse_literal(&p, ":") &&
> + parse_inany(&p, &taddr_tmp)) {
> + /* Target address only */
> + tr = lr;
> + } else if (p = tgtspec,
> + parse_literal(&p, ":") &&
> + parse_port_range(&p, &tr)) {
> + /* Target range only */
> + taddr_tmp = inany_any6;
> } else {
> + p = tgtspec;
> + /* No target specification */
> + taddr_tmp = inany_any6;
> tr = lr;
> }
> } else {
> @@ -492,6 +545,8 @@ static bool parse_port_chunk(const char **cursor,
>
> *kindp = kind;
> *lrange = lr;
> + if (taddr)
> + *taddr = taddr_tmp;
> if (trange)
> *trange = tr;
> *cursor = p;
> @@ -551,6 +606,7 @@ 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 };
> + union inany_addr all_taddr = inany_any6;
> enum fwd_port_chunk_kind kind;
> struct port_range lrange;
> bool exclude_only = true;
> @@ -561,7 +617,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> /* Consider excluded ranges and "auto" in the first pass */
> p = spec;
> do {
> - if (!parse_port_chunk(&p, &kind, &lrange, NULL))
> + if (!parse_port_chunk(&p, &kind, &lrange, NULL, NULL))
> goto bad;
>
> switch (kind) {
> @@ -586,14 +642,19 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> p = spec;
> do {
> struct port_range trange;
> + union inany_addr taddr;
>
> - if (!parse_port_chunk(&p, &kind, &lrange, &trange))
> + if (!parse_port_chunk(&p, &kind, &lrange, &taddr, &trange))
> goto bad;
>
> switch (kind) {
> - case CHUNK_AUTO: /* already handled */
> - case CHUNK_EXCLUDE: /* already handled */
> - case CHUNK_ALL: /* handled later */
> + case CHUNK_AUTO:
> + case CHUNK_EXCLUDE:
> + continue; /* already handled */
> +
> + case CHUNK_ALL:
> + /* Save the address to use later */
> + all_taddr = taddr;
> continue;
>
> case CHUNK_INCLUDE:
> @@ -604,7 +665,8 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
>
> fwd_rule_range_except(fwd, del, proto, addr, ifname,
> lrange.first, lrange.last,
> - exclude, trange.first, flags);
> + exclude, &taddr, trange.first,
> + flags);
> break;
> default:
> goto bad;
> @@ -620,7 +682,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
>
> fwd_rule_range_except(fwd, del, proto, addr, ifname,
> 1, NUM_PORTS - 1, exclude,
> - 1, flags | FWD_WEAK);
> + &all_taddr, 1, flags | FWD_WEAK);
> }
> return;
> bad:
> diff --git a/passt.1 b/passt.1
> index c3722ef9..9ece0e0c 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -449,12 +449,15 @@ interface name (since Linux 5.7) can be specified.
>
> \fIports\fR is 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]]
> +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR[\fItoaddr\fR\fB/\fR]\fItofirst\fR[\fB-\fR\fItolast\fR]]
> +.TP
> +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItoaddr\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.
> +(inclusive) to ports between \fItofirst\fR and \fItolast\fR to address
> +\fItoaddr\fR. If \fItoaddr\fR is omitted, automatically determine the
> +guest or namespace address. 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]
> @@ -462,11 +465,13 @@ Exclude range. Don't forward port numbers between \fIfirst\fR and
> \fIlast\fR. This takes precedences over include ranges.
>
> .TP
> -.BR all
> +.BR all\fR[\fB:\fItoaddr\fR]
> Forward all unbound, non-ephemeral ports, not covered by exclude
> -ranges above, 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.
> +ranges above, as permitted by current capabilities, to the
> +corresponding ports on address \fItoaddr\fR. If \fItoaddr\fR is
> +omitted, automatically determine the guest or namespace address. 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
> @@ -516,6 +521,14 @@ Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22
> -t %eth0/22
> Forward local port 22, bound to any address on interface eth0, to port 22
> .TP
> +-t 0.0.0.0/5000:192.0.2.5/6000
> +Forward local port 5000, bound to any IPv4 address, to port 6000 on address 192.0.2.5.
> +.TP
> +-t 127.0.0.6/all:192.0.2.6
> +For the local address 127.0.0.6 forward all unbound, non-ephemeral
I think this is a bit difficult to follow without a comma, it should be:
For the local address 127.0.0.6, forward all unbound, [...]
also for consistency with the existing:
For the local address 192.0.2.1, forward ports between 20 and 24 [...]
> +ports as permitted by current capabilities to the corresponding port
> +on 192.0.2.6.
> +.TP
> -t 2000-5000,~3000-3010
> Forward local ports between 2000 and 5000, except for those between 3000 and
> 3010
the rest and 2/2 look good to me, no further comments.
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules
2026-07-04 15:27 ` Stefano Brivio
@ 2026-07-06 3:08 ` David Gibson
2026-07-06 22:48 ` Stefano Brivio
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2026-07-06 3:08 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 14510 bytes --]
On Sat, Jul 04, 2026 at 05:27:06PM +0200, Stefano Brivio wrote:
> On Thu, 2 Jul 2026 17:32:14 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > Extend the parsing of forwarding rules (-[tu]) to allow the destination
> > address on the target side to be specified. For now just parse them, and
> > give an error if we try to create rules with a specified target address.
> > We'll implement the actual forwarding logic in another patch.
> >
> > Format (for either command line or pesto):
> > -t 2222:192.0.2.1/2222
> >
> > This should work along with all the other bits, that is, say:
> > -t 192.0.2.1%eth0/2222-2225:192.0.2.2/22-25
> >
> > FIXME: Ban for -[TU] for now
> > FIXME: Check interaction with splice handling
> >
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > [dwg: Syntax from Stefano's earlier draft, largely rewritten on top of new
> > parsing helpers]
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > fwd_rule.c | 90 +++++++++++++++++++++++++++++++++++++++++++++---------
> > passt.1 | 31 +++++++++++++------
> > 2 files changed, 98 insertions(+), 23 deletions(-)
> >
> > diff --git a/fwd_rule.c b/fwd_rule.c
> > index ef35e1b4..bed29ed9 100644
> > --- a/fwd_rule.c
> > +++ b/fwd_rule.c
> > @@ -378,14 +378,17 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
> > * @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
> > + * @tgt_addr: Destination address on the target side
> > + * @tgt_first: Destination port to use for @first on the target side
> > * @flags: Flags for forwarding entries
> > */
> > static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > uint8_t proto, const union inany_addr *addr,
> > const char *ifname,
> > uint16_t first, uint16_t last,
> > - const uint8_t *exclude, uint16_t to,
> > + const uint8_t *exclude,
> > + const union inany_addr *tgt_addr,
> > + uint16_t tgt_first,
> > uint8_t flags)
> > {
> > struct fwd_rule rule = {
> > @@ -394,10 +397,31 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > .proto = proto,
> > .flags = flags,
> > };
> > + unsigned delta = tgt_first - first;
> > char rulestr[FWD_RULE_STRLEN];
> > - unsigned delta = to - first;
> > unsigned base, i;
> >
> > + if (tgt_addr && !inany_is_unspecified(tgt_addr)) {
> > + char astr[INANY_ADDRSTRLEN];
> > +
> > + if (!inany_is_unicast(tgt_addr)) {
> > + die("Target address %s is not unicast",
> > + inany_ntop(tgt_addr, astr, sizeof(astr)));
> > + }
>
> Nit: an extra newline here would help readability.
Done.
> > + if (!addr || !!inany_v4(addr) != !!inany_v4(tgt_addr)) {
>
> I wonder: if !addr, shouldn't we replace it, for the moment, with
> inany_any4 or inany_any6 depending on !!inany_v4(tgt_addr)?
>
> I see that an empty address already works with -4 or -6, and I
> understand that, by doing this, we'll have a change in behaviour once
> forwarding between IP versions is implemented.
>
> But it will take a while before we get there, and, meanwhile, I guess
> almost all users will just want to do stuff like -t 8080:192.0.2.1/80,
> just to hit:
>
> Forwarding between IP versions (* => 192.0.2.1) not implemented
>
> which isn't obvious if you aren't familiar with the implementation. I
> can also picture a constant flow of incoming tickets as a result.
That's a reasonable concern. I did think about this and came to the
tentative conclusion that automatically restricting to one family
would be worse, but I'm open to persuasion otherwise. Reasoning as
follows.
First, it is a little awkward to implement: I think we should still
prevent an explicit *:192.0.2.1, which means we'd have to distinguish
the explicit * and implicit any cases. Mind you, we probably want
something similar when we allow listening addresses for -[TU], so it's
not such a big deal.
More importly this would become the _only_ place where the target
designation affects where and how we listen - in all other ways,
that's fully determined by the first part of the spec plus global
flags like -[46]. This gets particularly striking when looking at
something like:
-t 5000,5001:192.0.2.1
With an automatic restriction, we'd end up listening on *:5000, but
only on 0.0.0.0:5001, which seems pretty confusing.
> The change in behaviour once we implement forwarding between IP
> versions, by the way, looks a bit like an extension rather than a real
> change that could reasonably cause trouble to anybody.
I'd argue that's even more the case with the errors as is: things that
weren't possible before become possible.
> > + char bstr[INANY_ADDRSTRLEN];
> > +
> > + die(
> > +"Forwarding between IP versions (%s => %s) not implemented",
> > + inany_ntop(addr, bstr, sizeof(bstr)),
> > + inany_ntop(tgt_addr, astr, sizeof(astr)));
> > + }
>
> About both validations: shouldn't they live in fwd_rule_add(), where
> we already have this kind of stuff? Or at least be duplicated there?
Yes, yes they should. Which also means it needs to move to the second
patch (or fwd_add_rule() doesn't see the target address - there's
nowhere for it in the structure).
For the next spin, I've moved this validation to fwd_rule_add() in
patch 2. I've also tweaked the messages a bit, which might at least
slightly mitigate the concerns above.
> Now, I tried dropping those in pesto (#ifndef PESTO ...) to see what
> happens, and I couldn't really spot anything really bad, not with
> multicast and not with mixing IPv4 and IPv6:
>
> 14 20.647728 fe80::1 → ::ffff:127.0.0.1 82 TCP 57812 → 5201 [SYN] Seq=0 Win=65535 Len=0 MSS=61440 WS=256
>
> but still I wonder if we shouldn't make it a bit more robust.
No, definitely. Complete thinko on my behalf.
> > +
> > + info("Target address: %s",
> > + inany_ntop(tgt_addr, astr, sizeof(astr)));
> > + die("Target address remapping not yet implemented");
> > + }
> > +
> > if (!addr)
> > rule.flags |= FWD_DUAL_STACK_ANY;
> > if (ifname) {
> > @@ -458,19 +482,31 @@ enum fwd_port_chunk_kind {
> > * @cursor: Parsing point (see parse.c)
> > * @kindp: Updated with kind of chunk we parsed
> > * @lrange: Updated with listening port range (for INCLUDE & EXCLUDE)
> > + * @taddr: Updated with target address (for INCLUDE & ALL)
> > * @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,
> > + union inany_addr *taddr,
> > struct port_range *trange)
> > {
> > struct port_range lr = { 0 }, tr = { 0 };
> > + union inany_addr taddr_tmp = inany_any6;
> > enum fwd_port_chunk_kind kind;
> > const char *p = *cursor;
> >
> > if (parse_literal(&p, "all")) {
> > + const char *tgtspec = p;
> > +
> > kind = CHUNK_ALL;
> > + if (p = tgtspec,
> > + parse_literal(&p, ":") &&
> > + parse_inany(&p, &taddr_tmp)) {
> > + /* Target address */
> > + } else {
> > + p = tgtspec;
> > + }
> > } else if (parse_literal(&p, "auto")) {
> > kind = CHUNK_AUTO;
> > } else if (parse_literal(&p, "~")) {
> > @@ -478,12 +514,29 @@ static bool parse_port_chunk(const char **cursor,
> > if (!parse_port_range(&p, &lr))
> > return false;
> > } else if (parse_port_range(&p, &lr)) {
> > - kind = CHUNK_INCLUDE;
> > + const char *tgtspec = p;
> >
> > - if (parse_literal(&p, ":")) {
> > - if (!parse_port_range(&p, &tr))
> > - return false;
> > + kind = CHUNK_INCLUDE;
> > + if (p = tgtspec,
> > + parse_literal(&p, ":") &&
> > + parse_inany(&p, &taddr_tmp) &&
> > + parse_literal(&p, "/") &&
> > + parse_port_range(&p, &tr)) {
> > + /* Target address & range */
> > + } else if (p = tgtspec,
> > + parse_literal(&p, ":") &&
> > + parse_inany(&p, &taddr_tmp)) {
> > + /* Target address only */
> > + tr = lr;
> > + } else if (p = tgtspec,
> > + parse_literal(&p, ":") &&
> > + parse_port_range(&p, &tr)) {
> > + /* Target range only */
> > + taddr_tmp = inany_any6;
> > } else {
> > + p = tgtspec;
> > + /* No target specification */
> > + taddr_tmp = inany_any6;
> > tr = lr;
> > }
> > } else {
> > @@ -492,6 +545,8 @@ static bool parse_port_chunk(const char **cursor,
> >
> > *kindp = kind;
> > *lrange = lr;
> > + if (taddr)
> > + *taddr = taddr_tmp;
> > if (trange)
> > *trange = tr;
> > *cursor = p;
> > @@ -551,6 +606,7 @@ 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 };
> > + union inany_addr all_taddr = inany_any6;
> > enum fwd_port_chunk_kind kind;
> > struct port_range lrange;
> > bool exclude_only = true;
> > @@ -561,7 +617,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > /* Consider excluded ranges and "auto" in the first pass */
> > p = spec;
> > do {
> > - if (!parse_port_chunk(&p, &kind, &lrange, NULL))
> > + if (!parse_port_chunk(&p, &kind, &lrange, NULL, NULL))
> > goto bad;
> >
> > switch (kind) {
> > @@ -586,14 +642,19 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > p = spec;
> > do {
> > struct port_range trange;
> > + union inany_addr taddr;
> >
> > - if (!parse_port_chunk(&p, &kind, &lrange, &trange))
> > + if (!parse_port_chunk(&p, &kind, &lrange, &taddr, &trange))
> > goto bad;
> >
> > switch (kind) {
> > - case CHUNK_AUTO: /* already handled */
> > - case CHUNK_EXCLUDE: /* already handled */
> > - case CHUNK_ALL: /* handled later */
> > + case CHUNK_AUTO:
> > + case CHUNK_EXCLUDE:
> > + continue; /* already handled */
> > +
> > + case CHUNK_ALL:
> > + /* Save the address to use later */
> > + all_taddr = taddr;
> > continue;
> >
> > case CHUNK_INCLUDE:
> > @@ -604,7 +665,8 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> >
> > fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > lrange.first, lrange.last,
> > - exclude, trange.first, flags);
> > + exclude, &taddr, trange.first,
> > + flags);
> > break;
> > default:
> > goto bad;
> > @@ -620,7 +682,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> >
> > fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > 1, NUM_PORTS - 1, exclude,
> > - 1, flags | FWD_WEAK);
> > + &all_taddr, 1, flags | FWD_WEAK);
> > }
> > return;
> > bad:
> > diff --git a/passt.1 b/passt.1
> > index c3722ef9..9ece0e0c 100644
> > --- a/passt.1
> > +++ b/passt.1
> > @@ -449,12 +449,15 @@ interface name (since Linux 5.7) can be specified.
> >
> > \fIports\fR is 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]]
> > +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR[\fItoaddr\fR\fB/\fR]\fItofirst\fR[\fB-\fR\fItolast\fR]]
> > +.TP
> > +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItoaddr\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.
> > +(inclusive) to ports between \fItofirst\fR and \fItolast\fR to address
> > +\fItoaddr\fR. If \fItoaddr\fR is omitted, automatically determine the
> > +guest or namespace address. 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]
> > @@ -462,11 +465,13 @@ Exclude range. Don't forward port numbers between \fIfirst\fR and
> > \fIlast\fR. This takes precedences over include ranges.
> >
> > .TP
> > -.BR all
> > +.BR all\fR[\fB:\fItoaddr\fR]
> > Forward all unbound, non-ephemeral ports, not covered by exclude
> > -ranges above, 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.
> > +ranges above, as permitted by current capabilities, to the
> > +corresponding ports on address \fItoaddr\fR. If \fItoaddr\fR is
> > +omitted, automatically determine the guest or namespace address. 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
> > @@ -516,6 +521,14 @@ Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22
> > -t %eth0/22
> > Forward local port 22, bound to any address on interface eth0, to port 22
> > .TP
> > +-t 0.0.0.0/5000:192.0.2.5/6000
> > +Forward local port 5000, bound to any IPv4 address, to port 6000 on address 192.0.2.5.
> > +.TP
> > +-t 127.0.0.6/all:192.0.2.6
> > +For the local address 127.0.0.6 forward all unbound, non-ephemeral
>
> I think this is a bit difficult to follow without a comma, it should be:
Done.
> For the local address 127.0.0.6, forward all unbound, [...]
>
> also for consistency with the existing:
>
> For the local address 192.0.2.1, forward ports between 20 and 24 [...]
>
> > +ports as permitted by current capabilities to the corresponding port
> > +on 192.0.2.6.
> > +.TP
> > -t 2000-5000,~3000-3010
> > Forward local ports between 2000 and 5000, except for those between 3000 and
> > 3010
>
> the rest and 2/2 look good to me, no further comments.
>
> --
> Stefano
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules
2026-07-06 3:08 ` David Gibson
@ 2026-07-06 22:48 ` Stefano Brivio
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2026-07-06 22:48 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev
On Mon, 6 Jul 2026 13:08:29 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Sat, Jul 04, 2026 at 05:27:06PM +0200, Stefano Brivio wrote:
> > On Thu, 2 Jul 2026 17:32:14 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > > Extend the parsing of forwarding rules (-[tu]) to allow the destination
> > > address on the target side to be specified. For now just parse them, and
> > > give an error if we try to create rules with a specified target address.
> > > We'll implement the actual forwarding logic in another patch.
> > >
> > > Format (for either command line or pesto):
> > > -t 2222:192.0.2.1/2222
> > >
> > > This should work along with all the other bits, that is, say:
> > > -t 192.0.2.1%eth0/2222-2225:192.0.2.2/22-25
> > >
> > > FIXME: Ban for -[TU] for now
> > > FIXME: Check interaction with splice handling
> > >
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > > [dwg: Syntax from Stefano's earlier draft, largely rewritten on top of new
> > > parsing helpers]
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > > fwd_rule.c | 90 +++++++++++++++++++++++++++++++++++++++++++++---------
> > > passt.1 | 31 +++++++++++++------
> > > 2 files changed, 98 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/fwd_rule.c b/fwd_rule.c
> > > index ef35e1b4..bed29ed9 100644
> > > --- a/fwd_rule.c
> > > +++ b/fwd_rule.c
> > > @@ -378,14 +378,17 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
> > > * @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
> > > + * @tgt_addr: Destination address on the target side
> > > + * @tgt_first: Destination port to use for @first on the target side
> > > * @flags: Flags for forwarding entries
> > > */
> > > static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > > uint8_t proto, const union inany_addr *addr,
> > > const char *ifname,
> > > uint16_t first, uint16_t last,
> > > - const uint8_t *exclude, uint16_t to,
> > > + const uint8_t *exclude,
> > > + const union inany_addr *tgt_addr,
> > > + uint16_t tgt_first,
> > > uint8_t flags)
> > > {
> > > struct fwd_rule rule = {
> > > @@ -394,10 +397,31 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > > .proto = proto,
> > > .flags = flags,
> > > };
> > > + unsigned delta = tgt_first - first;
> > > char rulestr[FWD_RULE_STRLEN];
> > > - unsigned delta = to - first;
> > > unsigned base, i;
> > >
> > > + if (tgt_addr && !inany_is_unspecified(tgt_addr)) {
> > > + char astr[INANY_ADDRSTRLEN];
> > > +
> > > + if (!inany_is_unicast(tgt_addr)) {
> > > + die("Target address %s is not unicast",
> > > + inany_ntop(tgt_addr, astr, sizeof(astr)));
> > > + }
> >
> > Nit: an extra newline here would help readability.
>
> Done.
>
> > > + if (!addr || !!inany_v4(addr) != !!inany_v4(tgt_addr)) {
> >
> > I wonder: if !addr, shouldn't we replace it, for the moment, with
> > inany_any4 or inany_any6 depending on !!inany_v4(tgt_addr)?
> >
> > I see that an empty address already works with -4 or -6, and I
> > understand that, by doing this, we'll have a change in behaviour once
> > forwarding between IP versions is implemented.
> >
> > But it will take a while before we get there, and, meanwhile, I guess
> > almost all users will just want to do stuff like -t 8080:192.0.2.1/80,
> > just to hit:
> >
> > Forwarding between IP versions (* => 192.0.2.1) not implemented
> >
> > which isn't obvious if you aren't familiar with the implementation. I
> > can also picture a constant flow of incoming tickets as a result.
>
> That's a reasonable concern. I did think about this and came to the
> tentative conclusion that automatically restricting to one family
> would be worse, but I'm open to persuasion otherwise. Reasoning as
> follows.
>
> First, it is a little awkward to implement: I think we should still
> prevent an explicit *:192.0.2.1, which means we'd have to distinguish
> the explicit * and implicit any cases.
Fair, and I think we should, but that looks relatively simple judging
from fwd_rule_range_except() and callers.
> Mind you, we probably want
> something similar when we allow listening addresses for -[TU], so it's
> not such a big deal.
>
> More importly this would become the _only_ place where the target
> designation affects where and how we listen - in all other ways,
> that's fully determined by the first part of the spec plus global
> flags like -[46]. This gets particularly striking when looking at
> something like:
> -t 5000,5001:192.0.2.1
>
> With an automatic restriction, we'd end up listening on *:5000, but
> only on 0.0.0.0:5001, which seems pretty confusing.
So... I wasn't really aware or the fact that:
- t x,y
would always be equivalent to:
-t x -t y
as it is now. I originally intended , as a separator for ports and
ports only, but originally we just had ports as port forwarding
specifications. Then forwarding specifiers grew and I missed that, but
it wasn't my intention.
I think that, eventually, we should make -t x_0,...,x_n a mere port
specifier like it already is for ranges, because it's rather intuitive,
as opposed as, say:
-t 5000 -t 5001:192.0.2.1
where it's clear that we want to map port 5001 to 192.0.2.1 as
destination address. If we say -t 5000,5001:192.0.2.1, it's rather
obvious that we do *not*, and yet that's the outcome.
On the other hand, "fixing" this is clearly behind the scope of this
series, so, unless you have in mind a quick way to implement it, let's
forget about this, and go ahead with this approach which is consistent
with the existing situation, I'd say.
> > The change in behaviour once we implement forwarding between IP
> > versions, by the way, looks a bit like an extension rather than a real
> > change that could reasonably cause trouble to anybody.
>
> I'd argue that's even more the case with the errors as is: things that
> weren't possible before become possible.
Indeed. I was just suggesting that even with the other option we would
add an extension at some point, rather than a bug.
> > > + char bstr[INANY_ADDRSTRLEN];
> > > +
> > > + die(
> > > +"Forwarding between IP versions (%s => %s) not implemented",
> > > + inany_ntop(addr, bstr, sizeof(bstr)),
> > > + inany_ntop(tgt_addr, astr, sizeof(astr)));
> > > + }
> >
> > About both validations: shouldn't they live in fwd_rule_add(), where
> > we already have this kind of stuff? Or at least be duplicated there?
>
> Yes, yes they should. Which also means it needs to move to the second
> patch (or fwd_add_rule() doesn't see the target address - there's
> nowhere for it in the structure).
>
> For the next spin, I've moved this validation to fwd_rule_add() in
> patch 2. I've also tweaked the messages a bit, which might at least
> slightly mitigate the concerns above.
Maybe, given that these error messages are temporary, we could indulge
into some extra message like:
"Did you want to say 0.0.0.0/... instead?"
same for ::1, along with the rest of the forwarding specifier. Users
would see that when their containers fail to start and they could fix
their syntax right away.
> > Now, I tried dropping those in pesto (#ifndef PESTO ...) to see what
> > happens, and I couldn't really spot anything really bad, not with
> > multicast and not with mixing IPv4 and IPv6:
> >
> > 14 20.647728 fe80::1 → ::ffff:127.0.0.1 82 TCP 57812 → 5201 [SYN] Seq=0 Win=65535 Len=0 MSS=61440 WS=256
> >
> > but still I wonder if we shouldn't make it a bit more robust.
>
> No, definitely. Complete thinko on my behalf.
>
> > > +
> > > + info("Target address: %s",
> > > + inany_ntop(tgt_addr, astr, sizeof(astr)));
> > > + die("Target address remapping not yet implemented");
> > > + }
> > > +
> > > if (!addr)
> > > rule.flags |= FWD_DUAL_STACK_ANY;
> > > if (ifname) {
> > > @@ -458,19 +482,31 @@ enum fwd_port_chunk_kind {
> > > * @cursor: Parsing point (see parse.c)
> > > * @kindp: Updated with kind of chunk we parsed
> > > * @lrange: Updated with listening port range (for INCLUDE & EXCLUDE)
> > > + * @taddr: Updated with target address (for INCLUDE & ALL)
> > > * @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,
> > > + union inany_addr *taddr,
> > > struct port_range *trange)
> > > {
> > > struct port_range lr = { 0 }, tr = { 0 };
> > > + union inany_addr taddr_tmp = inany_any6;
> > > enum fwd_port_chunk_kind kind;
> > > const char *p = *cursor;
> > >
> > > if (parse_literal(&p, "all")) {
> > > + const char *tgtspec = p;
> > > +
> > > kind = CHUNK_ALL;
> > > + if (p = tgtspec,
> > > + parse_literal(&p, ":") &&
> > > + parse_inany(&p, &taddr_tmp)) {
> > > + /* Target address */
> > > + } else {
> > > + p = tgtspec;
> > > + }
> > > } else if (parse_literal(&p, "auto")) {
> > > kind = CHUNK_AUTO;
> > > } else if (parse_literal(&p, "~")) {
> > > @@ -478,12 +514,29 @@ static bool parse_port_chunk(const char **cursor,
> > > if (!parse_port_range(&p, &lr))
> > > return false;
> > > } else if (parse_port_range(&p, &lr)) {
> > > - kind = CHUNK_INCLUDE;
> > > + const char *tgtspec = p;
> > >
> > > - if (parse_literal(&p, ":")) {
> > > - if (!parse_port_range(&p, &tr))
> > > - return false;
> > > + kind = CHUNK_INCLUDE;
> > > + if (p = tgtspec,
> > > + parse_literal(&p, ":") &&
> > > + parse_inany(&p, &taddr_tmp) &&
> > > + parse_literal(&p, "/") &&
> > > + parse_port_range(&p, &tr)) {
> > > + /* Target address & range */
> > > + } else if (p = tgtspec,
> > > + parse_literal(&p, ":") &&
> > > + parse_inany(&p, &taddr_tmp)) {
> > > + /* Target address only */
> > > + tr = lr;
> > > + } else if (p = tgtspec,
> > > + parse_literal(&p, ":") &&
> > > + parse_port_range(&p, &tr)) {
> > > + /* Target range only */
> > > + taddr_tmp = inany_any6;
> > > } else {
> > > + p = tgtspec;
> > > + /* No target specification */
> > > + taddr_tmp = inany_any6;
> > > tr = lr;
> > > }
> > > } else {
> > > @@ -492,6 +545,8 @@ static bool parse_port_chunk(const char **cursor,
> > >
> > > *kindp = kind;
> > > *lrange = lr;
> > > + if (taddr)
> > > + *taddr = taddr_tmp;
> > > if (trange)
> > > *trange = tr;
> > > *cursor = p;
> > > @@ -551,6 +606,7 @@ 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 };
> > > + union inany_addr all_taddr = inany_any6;
> > > enum fwd_port_chunk_kind kind;
> > > struct port_range lrange;
> > > bool exclude_only = true;
> > > @@ -561,7 +617,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > /* Consider excluded ranges and "auto" in the first pass */
> > > p = spec;
> > > do {
> > > - if (!parse_port_chunk(&p, &kind, &lrange, NULL))
> > > + if (!parse_port_chunk(&p, &kind, &lrange, NULL, NULL))
> > > goto bad;
> > >
> > > switch (kind) {
> > > @@ -586,14 +642,19 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > p = spec;
> > > do {
> > > struct port_range trange;
> > > + union inany_addr taddr;
> > >
> > > - if (!parse_port_chunk(&p, &kind, &lrange, &trange))
> > > + if (!parse_port_chunk(&p, &kind, &lrange, &taddr, &trange))
> > > goto bad;
> > >
> > > switch (kind) {
> > > - case CHUNK_AUTO: /* already handled */
> > > - case CHUNK_EXCLUDE: /* already handled */
> > > - case CHUNK_ALL: /* handled later */
> > > + case CHUNK_AUTO:
> > > + case CHUNK_EXCLUDE:
> > > + continue; /* already handled */
> > > +
> > > + case CHUNK_ALL:
> > > + /* Save the address to use later */
> > > + all_taddr = taddr;
> > > continue;
> > >
> > > case CHUNK_INCLUDE:
> > > @@ -604,7 +665,8 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > >
> > > fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > > lrange.first, lrange.last,
> > > - exclude, trange.first, flags);
> > > + exclude, &taddr, trange.first,
> > > + flags);
> > > break;
> > > default:
> > > goto bad;
> > > @@ -620,7 +682,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > >
> > > fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > > 1, NUM_PORTS - 1, exclude,
> > > - 1, flags | FWD_WEAK);
> > > + &all_taddr, 1, flags | FWD_WEAK);
> > > }
> > > return;
> > > bad:
> > > diff --git a/passt.1 b/passt.1
> > > index c3722ef9..9ece0e0c 100644
> > > --- a/passt.1
> > > +++ b/passt.1
> > > @@ -449,12 +449,15 @@ interface name (since Linux 5.7) can be specified.
> > >
> > > \fIports\fR is 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]]
> > > +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR[\fItoaddr\fR\fB/\fR]\fItofirst\fR[\fB-\fR\fItolast\fR]]
> > > +.TP
> > > +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItoaddr\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.
> > > +(inclusive) to ports between \fItofirst\fR and \fItolast\fR to address
> > > +\fItoaddr\fR. If \fItoaddr\fR is omitted, automatically determine the
> > > +guest or namespace address. 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]
> > > @@ -462,11 +465,13 @@ Exclude range. Don't forward port numbers between \fIfirst\fR and
> > > \fIlast\fR. This takes precedences over include ranges.
> > >
> > > .TP
> > > -.BR all
> > > +.BR all\fR[\fB:\fItoaddr\fR]
> > > Forward all unbound, non-ephemeral ports, not covered by exclude
> > > -ranges above, 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.
> > > +ranges above, as permitted by current capabilities, to the
> > > +corresponding ports on address \fItoaddr\fR. If \fItoaddr\fR is
> > > +omitted, automatically determine the guest or namespace address. 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
> > > @@ -516,6 +521,14 @@ Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22
> > > -t %eth0/22
> > > Forward local port 22, bound to any address on interface eth0, to port 22
> > > .TP
> > > +-t 0.0.0.0/5000:192.0.2.5/6000
> > > +Forward local port 5000, bound to any IPv4 address, to port 6000 on address 192.0.2.5.
> > > +.TP
> > > +-t 127.0.0.6/all:192.0.2.6
> > > +For the local address 127.0.0.6 forward all unbound, non-ephemeral
> >
> > I think this is a bit difficult to follow without a comma, it should be:
>
> Done.
>
> > For the local address 127.0.0.6, forward all unbound, [...]
> >
> > also for consistency with the existing:
> >
> > For the local address 192.0.2.1, forward ports between 20 and 24 [...]
> >
> > > +ports as permitted by current capabilities to the corresponding port
> > > +on 192.0.2.6.
> > > +.TP
> > > -t 2000-5000,~3000-3010
> > > Forward local ports between 2000 and 5000, except for those between 3000 and
> > > 3010
> >
> > the rest and 2/2 look good to me, no further comments.
> >
> > --
> > Stefano
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-06 22:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-02 7:32 [PATCH v2 0/2] Target address mapping David Gibson
2026-07-02 7:32 ` [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules David Gibson
2026-07-04 15:27 ` Stefano Brivio
2026-07-06 3:08 ` David Gibson
2026-07-06 22:48 ` Stefano Brivio
2026-07-02 7:32 ` [PATCH v2 2/2] fwd, fwd_rule: Implement configurable target address mapping David Gibson
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).