[PATCH HACK] fwd, fwd_rule: Implement configurable destination address mapping

[PATCH HACK] fwd, fwd_rule: Implement configurable destination address mapping

[Stefano Brivio]

This isn't complete, it's rather a quick hack to enable early
integration testing.

Add a 'daddr' field to forwarding rules, and some rudimentary parsing.

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

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 fwd.c      |  4 +++-
 fwd_rule.c | 46 +++++++++++++++++++++++++++++++++++-----------
 fwd_rule.h |  2 ++
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/fwd.c b/fwd.c
index d224c0a..75db350 100644
--- a/fwd.c
+++ b/fwd.c
@@ -1095,7 +1095,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->daddr)) {
+		tgt->eaddr = rule->daddr;
+	} 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 5fc04d7..5bce2fb 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -465,6 +465,7 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw)
  */
 static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
 				  uint8_t proto, const union inany_addr *addr,
+				  const union inany_addr *daddr,
 				  const char *ifname,
 				  uint16_t first, uint16_t last,
 				  const uint8_t *exclude, uint16_t to,
@@ -472,6 +473,7 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
 {
 	struct fwd_rule rule = {
 		.addr = addr ? *addr : inany_any6,
+		.daddr = daddr ? *daddr : inany_any6,
 		.ifname = { 0 },
 		.proto = proto,
 		.flags = flags,
@@ -544,13 +546,13 @@ fail:
  */
 static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
 				 const union inany_addr *addr,
-				 const char *ifname,
-				 const char *spec)
+				 const char *ifname, char *spec)
 {
+	union inany_addr daddr_buf = inany_any6, *daddr = &daddr_buf;
 	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
 	bool exclude_only = true;
-	const char *p, *ep;
 	uint8_t flags = 0;
+	char *p, *ep;
 	unsigned i;
 
 	if (!strcmp(spec, "all")) {
@@ -568,7 +570,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_keyword(p, (const char **)&p, "auto") == 0) {
 			if (p != ep) /* Garbage after the keyword */
 				goto bad;
 
@@ -586,7 +588,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
 			goto bad;
 		p++;
 
-		if (parse_port_range(p, &p, &xrange))
+		if (parse_port_range(p, (const char **)&p, &xrange))
 			goto bad;
 		if (p != ep) /* Garbage after the range */
 			goto bad;
@@ -599,7 +601,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
 		/* Exclude ephemeral ports */
 		fwd_port_map_ephemeral(exclude);
 
-		fwd_rule_range_except(fwd, del, proto, addr, ifname,
+		fwd_rule_range_except(fwd, del, proto, addr, NULL, ifname,
 				      1, NUM_PORTS - 1, exclude,
 				      1, flags | FWD_WEAK);
 		return;
@@ -613,11 +615,32 @@ 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, (const char **)&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 (*p == ':') {
+			/* There's a range or address to map to as well */
+			char *addr_end = strchr(p, '/');
+
+			if (addr_end) {
+				*addr_end = '\0';
+
+				if (*p == '[' && p[strlen(p) - 1] == ']') {
+					p[strlen(p) - 1] = '\0';
+					p++;
+				}
+
+				if (!inany_pton(p + 1, daddr))
+					die("Bad forwarding address '%s'", p);
+
+				p = addr_end;
+			} else {
+				daddr = NULL;
+			}
+
+
+			if (parse_port_range(p + 1, (const char **)&p,
+			    &mapped_range))
 				goto bad;
 			if ((mapped_range.last - mapped_range.first) !=
 			    (orig_range.last - orig_range.first))
@@ -629,7 +652,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
 		if (p != ep) /* Garbage after the ranges */
 			goto bad;
 
-		fwd_rule_range_except(fwd, del, proto, addr, ifname,
+		fwd_rule_range_except(fwd, del, proto, addr, daddr, ifname,
 				      orig_range.first, orig_range.last,
 				      exclude,
 				      mapped_range.first, flags);
@@ -675,7 +698,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
 
 	strncpy(buf, optarg, sizeof(buf) - 1);
 
-	if ((spec = strchr(buf, '/'))) {
+	if ((spec = strchr(buf, '/')) &&
+	    strchr(spec, ':') == strchr(buf, ':')) {
 		*spec = 0;
 		spec++;
 
diff --git a/fwd_rule.h b/fwd_rule.h
index ae9a3cb..3a2a809 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -35,6 +35,7 @@
 /**
  * struct fwd_rule - Forwarding rule governing a range of ports
  * @addr:	Address to forward from
+ * @daddr:	Optional address to set as destination when forwarding
  * @ifname:	Interface to forward from
  * @first:	First port number to forward
  * @last:	Last port number to forward
@@ -47,6 +48,7 @@
  */
 struct fwd_rule {
 	union inany_addr addr;
+	union inany_addr daddr;
 	char ifname[IFNAMSIZ];
 	in_port_t first;
 	in_port_t last;
-- 
2.43.0

Re: [PATCH HACK] fwd, fwd_rule: Implement configurable destination address mapping

[David Gibson]

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

On Thu, May 07, 2026 at 07:50:36AM +0200, Stefano Brivio wrote:
> This isn't complete, it's rather a quick hack to enable early
> integration testing.
> 
> Add a 'daddr' field to forwarding rules, and some rudimentary parsing.
> 
> 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
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Very nice for a quick hack, and it gets surprisingly far with not much
code at all.


> ---
>  fwd.c      |  4 +++-
>  fwd_rule.c | 46 +++++++++++++++++++++++++++++++++++-----------
>  fwd_rule.h |  2 ++
>  3 files changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/fwd.c b/fwd.c
> index d224c0a..75db350 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -1095,7 +1095,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->daddr)) {
> +		tgt->eaddr = rule->daddr;

Longer term, there are at least two options for what we want to do
if the rule doesn't specify a specific destination address:
  * Use the observed guest address (what we do now)
  * Use the host side destination address (potentially useful if we
    have multiple containers each assigned different host addresses)

So we'll probably want to allow for some new rule flags to cover
options like this.

> +	} 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 5fc04d7..5bce2fb 100644
> --- a/fwd_rule.c
> +++ b/fwd_rule.c
> @@ -465,6 +465,7 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw)
>   */
>  static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
>  				  uint8_t proto, const union inany_addr *addr,
> +				  const union inany_addr *daddr,
>  				  const char *ifname,
>  				  uint16_t first, uint16_t last,
>  				  const uint8_t *exclude, uint16_t to,
> @@ -472,6 +473,7 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
>  {
>  	struct fwd_rule rule = {
>  		.addr = addr ? *addr : inany_any6,
> +		.daddr = daddr ? *daddr : inany_any6,
>  		.ifname = { 0 },
>  		.proto = proto,
>  		.flags = flags,
> @@ -544,13 +546,13 @@ fail:
>   */
>  static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
>  				 const union inany_addr *addr,
> -				 const char *ifname,
> -				 const char *spec)
> +				 const char *ifname, char *spec)
>  {
> +	union inany_addr daddr_buf = inany_any6, *daddr = &daddr_buf;
>  	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
>  	bool exclude_only = true;
> -	const char *p, *ep;
>  	uint8_t flags = 0;
> +	char *p, *ep;
>  	unsigned i;
>  
>  	if (!strcmp(spec, "all")) {
> @@ -568,7 +570,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_keyword(p, (const char **)&p, "auto") == 0) {
>  			if (p != ep) /* Garbage after the keyword */
>  				goto bad;
>  
> @@ -586,7 +588,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
>  			goto bad;
>  		p++;
>  
> -		if (parse_port_range(p, &p, &xrange))
> +		if (parse_port_range(p, (const char **)&p, &xrange))
>  			goto bad;
>  		if (p != ep) /* Garbage after the range */
>  			goto bad;
> @@ -599,7 +601,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
>  		/* Exclude ephemeral ports */
>  		fwd_port_map_ephemeral(exclude);
>  
> -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> +		fwd_rule_range_except(fwd, del, proto, addr, NULL, ifname,
>  				      1, NUM_PORTS - 1, exclude,
>  				      1, flags | FWD_WEAK);
>  		return;
> @@ -613,11 +615,32 @@ 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, (const char **)&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 (*p == ':') {
> +			/* There's a range or address to map to as well */
> +			char *addr_end = strchr(p, '/');
> +
> +			if (addr_end) {
> +				*addr_end = '\0';
> +
> +				if (*p == '[' && p[strlen(p) - 1] == ']') {
> +					p[strlen(p) - 1] = '\0';
> +					p++;
> +				}
> +
> +				if (!inany_pton(p + 1, daddr))
> +					die("Bad forwarding address '%s'", p);
> +
> +				p = addr_end;
> +			} else {
> +				daddr = NULL;
> +			}

We probably want to factor some of this address parsing out into a
helper, since we're now doing it twice.

> +
> +
> +			if (parse_port_range(p + 1, (const char **)&p,
> +			    &mapped_range))
>  				goto bad;
>  			if ((mapped_range.last - mapped_range.first) !=
>  			    (orig_range.last - orig_range.first))
> @@ -629,7 +652,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
>  		if (p != ep) /* Garbage after the ranges */
>  			goto bad;
>  
> -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> +		fwd_rule_range_except(fwd, del, proto, addr, daddr, ifname,
>  				      orig_range.first, orig_range.last,
>  				      exclude,
>  				      mapped_range.first, flags);
> @@ -675,7 +698,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
>  
>  	strncpy(buf, optarg, sizeof(buf) - 1);
>  
> -	if ((spec = strchr(buf, '/'))) {
> +	if ((spec = strchr(buf, '/')) &&
> +	    strchr(spec, ':') == strchr(buf, ':')) {

strchr() for ':' needs a lot of caution, since it can appear within
IPv6 addresses.

>  		*spec = 0;
>  		spec++;
>  
> diff --git a/fwd_rule.h b/fwd_rule.h
> index ae9a3cb..3a2a809 100644
> --- a/fwd_rule.h
> +++ b/fwd_rule.h
> @@ -35,6 +35,7 @@
>  /**
>   * struct fwd_rule - Forwarding rule governing a range of ports
>   * @addr:	Address to forward from
> + * @daddr:	Optional address to set as destination when forwarding
>   * @ifname:	Interface to forward from
>   * @first:	First port number to forward
>   * @last:	Last port number to forward
> @@ -47,6 +48,7 @@
>   */
>  struct fwd_rule {
>  	union inany_addr addr;
> +	union inany_addr daddr;

We probably want to rethink the names here.  Both of these are
destination addresses, just one is host side the other is guest side
(or, in general, they're destination addresses for two different
pifs).  "initiating" vs. "target" are probably the terms to use, since
we already use that in the flow table.

At some point I'm pretty sure we'll also want to put a target pif in
the table.  Not sure if we want to introduce that initially, or
whether just an address makes sense for the first cut.

>  	char ifname[IFNAMSIZ];
>  	in_port_t first;
>  	in_port_t last;
> -- 
> 2.43.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH HACK] fwd, fwd_rule: Implement configurable destination address mapping

[Stefano Brivio]

On Thu, 14 May 2026 14:54:33 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, May 07, 2026 at 07:50:36AM +0200, Stefano Brivio wrote:
> > This isn't complete, it's rather a quick hack to enable early
> > integration testing.
> > 
> > Add a 'daddr' field to forwarding rules, and some rudimentary parsing.
> > 
> > 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
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> Very nice for a quick hack, and it gets surprisingly far with not much
> code at all.
> 
> 
> > ---
> >  fwd.c      |  4 +++-
> >  fwd_rule.c | 46 +++++++++++++++++++++++++++++++++++-----------
> >  fwd_rule.h |  2 ++
> >  3 files changed, 40 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fwd.c b/fwd.c
> > index d224c0a..75db350 100644
> > --- a/fwd.c
> > +++ b/fwd.c
> > @@ -1095,7 +1095,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->daddr)) {
> > +		tgt->eaddr = rule->daddr;  
> 
> Longer term, there are at least two options for what we want to do
> if the rule doesn't specify a specific destination address:
>   * Use the observed guest address (what we do now)

I would keep that as default for now.

>   * Use the host side destination address (potentially useful if we
>     have multiple containers each assigned different host addresses)

This could be implemented on top of Jon's "multiple addresses" series I
think, even in terms of using multiple observed guest addresses, if
useful.

> So we'll probably want to allow for some new rule flags to cover
> options like this.

Rather than flags, I've been suggesting address pointers / references.
I'm not sure if flags are generic enough.

At some point, we might want a syntax to refer to "the observed address
of container B". Even if we have just container A at the moment, maybe
we could start implementing something going in that direction.

> > +	} 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 5fc04d7..5bce2fb 100644
> > --- a/fwd_rule.c
> > +++ b/fwd_rule.c
> > @@ -465,6 +465,7 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw)
> >   */
> >  static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> >  				  uint8_t proto, const union inany_addr *addr,
> > +				  const union inany_addr *daddr,
> >  				  const char *ifname,
> >  				  uint16_t first, uint16_t last,
> >  				  const uint8_t *exclude, uint16_t to,
> > @@ -472,6 +473,7 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> >  {
> >  	struct fwd_rule rule = {
> >  		.addr = addr ? *addr : inany_any6,
> > +		.daddr = daddr ? *daddr : inany_any6,
> >  		.ifname = { 0 },
> >  		.proto = proto,
> >  		.flags = flags,
> > @@ -544,13 +546,13 @@ fail:
> >   */
> >  static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> >  				 const union inany_addr *addr,
> > -				 const char *ifname,
> > -				 const char *spec)
> > +				 const char *ifname, char *spec)
> >  {
> > +	union inany_addr daddr_buf = inany_any6, *daddr = &daddr_buf;
> >  	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
> >  	bool exclude_only = true;
> > -	const char *p, *ep;
> >  	uint8_t flags = 0;
> > +	char *p, *ep;
> >  	unsigned i;
> >  
> >  	if (!strcmp(spec, "all")) {
> > @@ -568,7 +570,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_keyword(p, (const char **)&p, "auto") == 0) {
> >  			if (p != ep) /* Garbage after the keyword */
> >  				goto bad;
> >  
> > @@ -586,7 +588,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> >  			goto bad;
> >  		p++;
> >  
> > -		if (parse_port_range(p, &p, &xrange))
> > +		if (parse_port_range(p, (const char **)&p, &xrange))
> >  			goto bad;
> >  		if (p != ep) /* Garbage after the range */
> >  			goto bad;
> > @@ -599,7 +601,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> >  		/* Exclude ephemeral ports */
> >  		fwd_port_map_ephemeral(exclude);
> >  
> > -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > +		fwd_rule_range_except(fwd, del, proto, addr, NULL, ifname,
> >  				      1, NUM_PORTS - 1, exclude,
> >  				      1, flags | FWD_WEAK);
> >  		return;
> > @@ -613,11 +615,32 @@ 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, (const char **)&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 (*p == ':') {
> > +			/* There's a range or address to map to as well */
> > +			char *addr_end = strchr(p, '/');
> > +
> > +			if (addr_end) {
> > +				*addr_end = '\0';
> > +
> > +				if (*p == '[' && p[strlen(p) - 1] == ']') {
> > +					p[strlen(p) - 1] = '\0';
> > +					p++;
> > +				}
> > +
> > +				if (!inany_pton(p + 1, daddr))
> > +					die("Bad forwarding address '%s'", p);
> > +
> > +				p = addr_end;
> > +			} else {
> > +				daddr = NULL;
> > +			}  
> 
> We probably want to factor some of this address parsing out into a
> helper, since we're now doing it twice.

Yes, definitely, and it should have its own small buffer. The in-place
parsing we have is a remnant of my original implementation (originally
intended for much simpler cases) but that's what forced me to
essentially copy and paste this if I wanted to implement this in a few
minutes.

> > +
> > +
> > +			if (parse_port_range(p + 1, (const char **)&p,
> > +			    &mapped_range))
> >  				goto bad;
> >  			if ((mapped_range.last - mapped_range.first) !=
> >  			    (orig_range.last - orig_range.first))
> > @@ -629,7 +652,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> >  		if (p != ep) /* Garbage after the ranges */
> >  			goto bad;
> >  
> > -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > +		fwd_rule_range_except(fwd, del, proto, addr, daddr, ifname,
> >  				      orig_range.first, orig_range.last,
> >  				      exclude,
> >  				      mapped_range.first, flags);
> > @@ -675,7 +698,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
> >  
> >  	strncpy(buf, optarg, sizeof(buf) - 1);
> >  
> > -	if ((spec = strchr(buf, '/'))) {
> > +	if ((spec = strchr(buf, '/')) &&
> > +	    strchr(spec, ':') == strchr(buf, ':')) {  
> 
> strchr() for ':' needs a lot of caution, since it can appear within
> IPv6 addresses.

Absolutely. This patch actually works only for IPv4. See also:

  https://github.com/containers/container-libs/pull/755#issuecomment-4408900718

We probably need separate helpers extracting different parts to keep
this sane (the LLM-sourced workaround shown there is probably correct
but pretty hard to audit and hard to reason about).

> >  		*spec = 0;
> >  		spec++;
> >  
> > diff --git a/fwd_rule.h b/fwd_rule.h
> > index ae9a3cb..3a2a809 100644
> > --- a/fwd_rule.h
> > +++ b/fwd_rule.h
> > @@ -35,6 +35,7 @@
> >  /**
> >   * struct fwd_rule - Forwarding rule governing a range of ports
> >   * @addr:	Address to forward from
> > + * @daddr:	Optional address to set as destination when forwarding
> >   * @ifname:	Interface to forward from
> >   * @first:	First port number to forward
> >   * @last:	Last port number to forward
> > @@ -47,6 +48,7 @@
> >   */
> >  struct fwd_rule {
> >  	union inany_addr addr;
> > +	union inany_addr daddr;  
> 
> We probably want to rethink the names here.  Both of these are
> destination addresses, just one is host side the other is guest side
> (or, in general, they're destination addresses for two different
> pifs).  "initiating" vs. "target" are probably the terms to use, since
> we already use that in the flow table.

I guess we'll want four addresses anyway, eventually, but that probably
needs the implementation of a separate address list with pointers that
I've been suggesting in the past to keep memory usage reasonable.

For the moment, there are two ways to distinguish these two destination
addresses, I think:

1. outside / inside

2. initiating / target

I was thinking that 1. is simpler to grasp as it refers to stable
concepts (host, container / guest) rather than how a connection came to
be (and client / server are much less universal than that).

Going with that convention means that things will be swapped in terms
of 2., though, for outbound forwarding. I still think it's preferable.

> At some point I'm pretty sure we'll also want to put a target pif in
> the table.  Not sure if we want to introduce that initially, or
> whether just an address makes sense for the first cut.

I don't think it's really important for the table itself (I would add
it if it doesn't add cachelines, and otherwise skip it or add it
commented out), but we should make sure the command line extensions
we're introducing will allow for that, or backwards compatibility will
be complicated to achieve later.

> >  	char ifname[IFNAMSIZ];
> >  	in_port_t first;
> >  	in_port_t last;
> > -- 
> > 2.43.0

Note that I'm not working on this right now as it's not immediately
helpful:

  https://github.com/containers/container-libs/pull/755#issuecomment-4421500913

so I don't plan to post further versions of this, at least not any time
soon. Feel free to pick it up from here, if it makes sense.

-- 
Stefano

Re: [PATCH HACK] fwd, fwd_rule: Implement configurable destination address mapping

[David Gibson]

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

On Fri, May 15, 2026 at 01:28:36AM +0200, Stefano Brivio wrote:
> On Thu, 14 May 2026 14:54:33 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Thu, May 07, 2026 at 07:50:36AM +0200, Stefano Brivio wrote:
> > > This isn't complete, it's rather a quick hack to enable early
> > > integration testing.
> > > 
> > > Add a 'daddr' field to forwarding rules, and some rudimentary parsing.
> > > 
> > > 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
> > > 
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> > 
> > Very nice for a quick hack, and it gets surprisingly far with not much
> > code at all.
> > 
> > 
> > > ---
> > >  fwd.c      |  4 +++-
> > >  fwd_rule.c | 46 +++++++++++++++++++++++++++++++++++-----------
> > >  fwd_rule.h |  2 ++
> > >  3 files changed, 40 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/fwd.c b/fwd.c
> > > index d224c0a..75db350 100644
> > > --- a/fwd.c
> > > +++ b/fwd.c
> > > @@ -1095,7 +1095,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->daddr)) {
> > > +		tgt->eaddr = rule->daddr;  
> > 
> > Longer term, there are at least two options for what we want to do
> > if the rule doesn't specify a specific destination address:
> >   * Use the observed guest address (what we do now)
> 
> I would keep that as default for now.

Of course.  My point is that we'll need to have space in our
forwarding rules for saying which approach we're using: Using address == ::
can only encode one of them.

> >   * Use the host side destination address (potentially useful if we
> >     have multiple containers each assigned different host addresses)
> 
> This could be implemented on top of Jon's "multiple addresses" series I
> think, even in terms of using multiple observed guest addresses, if
> useful.

Sure.

> > So we'll probably want to allow for some new rule flags to cover
> > options like this.
> 
> Rather than flags, I've been suggesting address pointers / references.
> I'm not sure if flags are generic enough.

My point is that there's non-address information as well.  If you have
a single translated address, fine, but there are multiple cases where
you don't, so we need some other way to encode which "auto address"
mode we're using.

> At some point, we might want a syntax to refer to "the observed address
> of container B". Even if we have just container A at the moment, maybe
> we could start implementing something going in that direction.

So, how we do this relates to two possible models for handling
multiple containers.  One is that each container has its own pif.  In
that case "observed address on pif XXX" is well defined, but is more
or less equivalent to having a destination pif in the rule, which I
think we want in any case.

The other model is multiple containers (or guests or whatever) on the
same pif, effectively bridged together.  This is arguably less
elegant than one-pif-per-container, but I think it's important to
support for two reasons:
  * Integrates easily with existing solutions that already bridge a
    bunch of containers or VMs together.
  * It allows things routed via the container (e.g. over a VPN) to get
    access via as pasta as well

In this case we can't define "observed address of container B" because
we don't have a way to tell what came from container B other than the
address they use.

> 
> > > +	} 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 5fc04d7..5bce2fb 100644
> > > --- a/fwd_rule.c
> > > +++ b/fwd_rule.c
> > > @@ -465,6 +465,7 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw)
> > >   */
> > >  static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > >  				  uint8_t proto, const union inany_addr *addr,
> > > +				  const union inany_addr *daddr,
> > >  				  const char *ifname,
> > >  				  uint16_t first, uint16_t last,
> > >  				  const uint8_t *exclude, uint16_t to,
> > > @@ -472,6 +473,7 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > >  {
> > >  	struct fwd_rule rule = {
> > >  		.addr = addr ? *addr : inany_any6,
> > > +		.daddr = daddr ? *daddr : inany_any6,
> > >  		.ifname = { 0 },
> > >  		.proto = proto,
> > >  		.flags = flags,
> > > @@ -544,13 +546,13 @@ fail:
> > >   */
> > >  static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > >  				 const union inany_addr *addr,
> > > -				 const char *ifname,
> > > -				 const char *spec)
> > > +				 const char *ifname, char *spec)
> > >  {
> > > +	union inany_addr daddr_buf = inany_any6, *daddr = &daddr_buf;
> > >  	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
> > >  	bool exclude_only = true;
> > > -	const char *p, *ep;
> > >  	uint8_t flags = 0;
> > > +	char *p, *ep;
> > >  	unsigned i;
> > >  
> > >  	if (!strcmp(spec, "all")) {
> > > @@ -568,7 +570,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_keyword(p, (const char **)&p, "auto") == 0) {
> > >  			if (p != ep) /* Garbage after the keyword */
> > >  				goto bad;
> > >  
> > > @@ -586,7 +588,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > >  			goto bad;
> > >  		p++;
> > >  
> > > -		if (parse_port_range(p, &p, &xrange))
> > > +		if (parse_port_range(p, (const char **)&p, &xrange))
> > >  			goto bad;
> > >  		if (p != ep) /* Garbage after the range */
> > >  			goto bad;
> > > @@ -599,7 +601,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > >  		/* Exclude ephemeral ports */
> > >  		fwd_port_map_ephemeral(exclude);
> > >  
> > > -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > > +		fwd_rule_range_except(fwd, del, proto, addr, NULL, ifname,
> > >  				      1, NUM_PORTS - 1, exclude,
> > >  				      1, flags | FWD_WEAK);
> > >  		return;
> > > @@ -613,11 +615,32 @@ 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, (const char **)&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 (*p == ':') {
> > > +			/* There's a range or address to map to as well */
> > > +			char *addr_end = strchr(p, '/');
> > > +
> > > +			if (addr_end) {
> > > +				*addr_end = '\0';
> > > +
> > > +				if (*p == '[' && p[strlen(p) - 1] == ']') {
> > > +					p[strlen(p) - 1] = '\0';
> > > +					p++;
> > > +				}
> > > +
> > > +				if (!inany_pton(p + 1, daddr))
> > > +					die("Bad forwarding address '%s'", p);
> > > +
> > > +				p = addr_end;
> > > +			} else {
> > > +				daddr = NULL;
> > > +			}  
> > 
> > We probably want to factor some of this address parsing out into a
> > helper, since we're now doing it twice.
> 
> Yes, definitely, and it should have its own small buffer. The in-place
> parsing we have is a remnant of my original implementation (originally
> intended for much simpler cases) but that's what forced me to
> essentially copy and paste this if I wanted to implement this in a few
> minutes.

Yes.

> > > +
> > > +
> > > +			if (parse_port_range(p + 1, (const char **)&p,
> > > +			    &mapped_range))
> > >  				goto bad;
> > >  			if ((mapped_range.last - mapped_range.first) !=
> > >  			    (orig_range.last - orig_range.first))
> > > @@ -629,7 +652,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > >  		if (p != ep) /* Garbage after the ranges */
> > >  			goto bad;
> > >  
> > > -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > > +		fwd_rule_range_except(fwd, del, proto, addr, daddr, ifname,
> > >  				      orig_range.first, orig_range.last,
> > >  				      exclude,
> > >  				      mapped_range.first, flags);
> > > @@ -675,7 +698,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
> > >  
> > >  	strncpy(buf, optarg, sizeof(buf) - 1);
> > >  
> > > -	if ((spec = strchr(buf, '/'))) {
> > > +	if ((spec = strchr(buf, '/')) &&
> > > +	    strchr(spec, ':') == strchr(buf, ':')) {  
> > 
> > strchr() for ':' needs a lot of caution, since it can appear within
> > IPv6 addresses.
> 
> Absolutely. This patch actually works only for IPv4. See also:
> 
>   https://github.com/containers/container-libs/pull/755#issuecomment-4408900718
> 
> We probably need separate helpers extracting different parts to keep
> this sane (the LLM-sourced workaround shown there is probably correct
> but pretty hard to audit and hard to reason about).

Makes sense.

> > >  		*spec = 0;
> > >  		spec++;
> > >  
> > > diff --git a/fwd_rule.h b/fwd_rule.h
> > > index ae9a3cb..3a2a809 100644
> > > --- a/fwd_rule.h
> > > +++ b/fwd_rule.h
> > > @@ -35,6 +35,7 @@
> > >  /**
> > >   * struct fwd_rule - Forwarding rule governing a range of ports
> > >   * @addr:	Address to forward from
> > > + * @daddr:	Optional address to set as destination when forwarding
> > >   * @ifname:	Interface to forward from
> > >   * @first:	First port number to forward
> > >   * @last:	Last port number to forward
> > > @@ -47,6 +48,7 @@
> > >   */
> > >  struct fwd_rule {
> > >  	union inany_addr addr;
> > > +	union inany_addr daddr;  
> > 
> > We probably want to rethink the names here.  Both of these are
> > destination addresses, just one is host side the other is guest side
> > (or, in general, they're destination addresses for two different
> > pifs).  "initiating" vs. "target" are probably the terms to use, since
> > we already use that in the flow table.
> 
> I guess we'll want four addresses anyway, eventually, but that probably
> needs the implementation of a separate address list with pointers that
> I've been suggesting in the past to keep memory usage reasonable.

I'm not so sure about this.  Theoretically, yes, we could allow full
four-address, four-port range rules.  But it can get very messy: often
we have NATs that are essentially independent of port remappings, and
port remappings that are essentially independent of address.  To
handle that we either need n*m rules, which is pretty awful or
applying multiple rules per flow which is pretty confusing.  Although
it's less general in principle, I think it will be more tractable in
practice to have several different translation steps with their own
rule tables, but none of them allowing the fully general four-address
matching.

> For the moment, there are two ways to distinguish these two destination
> addresses, I think:
> 
> 1. outside / inside
> 
> 2. initiating / target
> 
> I was thinking that 1. is simpler to grasp as it refers to stable
> concepts (host, container / guest) rather than how a connection came to
> be (and client / server are much less universal than that).

True, but it doesn't generalise well to multiple inside pifs (e.g. one
per container).  That's why I came up with initiating / target for the
flow table, and I think we should re-use it here.

> Going with that convention means that things will be swapped in terms
> of 2., though, for outbound forwarding. I still think it's preferable.
> 
> > At some point I'm pretty sure we'll also want to put a target pif in
> > the table.  Not sure if we want to introduce that initially, or
> > whether just an address makes sense for the first cut.
> 
> I don't think it's really important for the table itself (I would add
> it if it doesn't add cachelines, and otherwise skip it or add it
> commented out), but we should make sure the command line extensions
> we're introducing will allow for that, or backwards compatibility will
> be complicated to achieve later.

Right.

> > >  	char ifname[IFNAMSIZ];
> > >  	in_port_t first;
> > >  	in_port_t last;
> > > -- 
> > > 2.43.0
> 
> Note that I'm not working on this right now as it's not immediately
> helpful:
> 
>   https://github.com/containers/container-libs/pull/755#issuecomment-4421500913
> 
> so I don't plan to post further versions of this, at least not any time
> soon. Feel free to pick it up from here, if it makes sense.

Understood.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH HACK] fwd, fwd_rule: Implement configurable destination address mapping

[Stefano Brivio]

On Mon, 18 May 2026 16:01:22 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, May 15, 2026 at 01:28:36AM +0200, Stefano Brivio wrote:
> > On Thu, 14 May 2026 14:54:33 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Thu, May 07, 2026 at 07:50:36AM +0200, Stefano Brivio wrote:  
> > > > This isn't complete, it's rather a quick hack to enable early
> > > > integration testing.
> > > > 
> > > > Add a 'daddr' field to forwarding rules, and some rudimentary parsing.
> > > > 
> > > > 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
> > > > 
> > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>    
> > > 
> > > Very nice for a quick hack, and it gets surprisingly far with not much
> > > code at all.
> > > 
> > >   
> > > > ---
> > > >  fwd.c      |  4 +++-
> > > >  fwd_rule.c | 46 +++++++++++++++++++++++++++++++++++-----------
> > > >  fwd_rule.h |  2 ++
> > > >  3 files changed, 40 insertions(+), 12 deletions(-)
> > > > 
> > > > diff --git a/fwd.c b/fwd.c
> > > > index d224c0a..75db350 100644
> > > > --- a/fwd.c
> > > > +++ b/fwd.c
> > > > @@ -1095,7 +1095,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->daddr)) {
> > > > +		tgt->eaddr = rule->daddr;    
> > > 
> > > Longer term, there are at least two options for what we want to do
> > > if the rule doesn't specify a specific destination address:
> > >   * Use the observed guest address (what we do now)  
> > 
> > I would keep that as default for now.  
> 
> Of course.  My point is that we'll need to have space in our
> forwarding rules for saying which approach we're using: Using address == ::
> can only encode one of them.
> 
> > >   * Use the host side destination address (potentially useful if we
> > >     have multiple containers each assigned different host addresses)  
> > 
> > This could be implemented on top of Jon's "multiple addresses" series I
> > think, even in terms of using multiple observed guest addresses, if
> > useful.  
> 
> Sure.
> 
> > > So we'll probably want to allow for some new rule flags to cover
> > > options like this.  
> > 
> > Rather than flags, I've been suggesting address pointers / references.
> > I'm not sure if flags are generic enough.  
> 
> My point is that there's non-address information as well.  If you have
> a single translated address, fine, but there are multiple cases where
> you don't, so we need some other way to encode which "auto address"
> mode we're using.

Ah, yes, so we'll probably need flags *on top* of that.

> > At some point, we might want a syntax to refer to "the observed address
> > of container B". Even if we have just container A at the moment, maybe
> > we could start implementing something going in that direction.  
> 
> So, how we do this relates to two possible models for handling
> multiple containers.  One is that each container has its own pif.  In
> that case "observed address on pif XXX" is well defined, but is more
> or less equivalent to having a destination pif in the rule, which I
> think we want in any case.
> 
> The other model is multiple containers (or guests or whatever) on the
> same pif, effectively bridged together.

It doesn't need to be the same PIF, I guess. It could also be that we
bridge different PIFs together. It's a matter of representation rather
than functionality. I'm not sure which one is more convenient at this
stage.

If we allow multiple guests / containers per PIF, then yes, we'll need
to decouple the two concepts / names. Worth it?

> This is arguably less
> elegant than one-pif-per-container, but I think it's important to
> support for two reasons:
>   * Integrates easily with existing solutions that already bridge a
>     bunch of containers or VMs together.
>   * It allows things routed via the container (e.g. over a VPN) to get
>     access via as pasta as well
> 
> In this case we can't define "observed address of container B" because
> we don't have a way to tell what came from container B other than the
> address they use.

We could, using MAC addresses (that's kind of the thing a bridge does).

> > > > +	} 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 5fc04d7..5bce2fb 100644
> > > > --- a/fwd_rule.c
> > > > +++ b/fwd_rule.c
> > > > @@ -465,6 +465,7 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw)
> > > >   */
> > > >  static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > > >  				  uint8_t proto, const union inany_addr *addr,
> > > > +				  const union inany_addr *daddr,
> > > >  				  const char *ifname,
> > > >  				  uint16_t first, uint16_t last,
> > > >  				  const uint8_t *exclude, uint16_t to,
> > > > @@ -472,6 +473,7 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > > >  {
> > > >  	struct fwd_rule rule = {
> > > >  		.addr = addr ? *addr : inany_any6,
> > > > +		.daddr = daddr ? *daddr : inany_any6,
> > > >  		.ifname = { 0 },
> > > >  		.proto = proto,
> > > >  		.flags = flags,
> > > > @@ -544,13 +546,13 @@ fail:
> > > >   */
> > > >  static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > >  				 const union inany_addr *addr,
> > > > -				 const char *ifname,
> > > > -				 const char *spec)
> > > > +				 const char *ifname, char *spec)
> > > >  {
> > > > +	union inany_addr daddr_buf = inany_any6, *daddr = &daddr_buf;
> > > >  	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
> > > >  	bool exclude_only = true;
> > > > -	const char *p, *ep;
> > > >  	uint8_t flags = 0;
> > > > +	char *p, *ep;
> > > >  	unsigned i;
> > > >  
> > > >  	if (!strcmp(spec, "all")) {
> > > > @@ -568,7 +570,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_keyword(p, (const char **)&p, "auto") == 0) {
> > > >  			if (p != ep) /* Garbage after the keyword */
> > > >  				goto bad;
> > > >  
> > > > @@ -586,7 +588,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > >  			goto bad;
> > > >  		p++;
> > > >  
> > > > -		if (parse_port_range(p, &p, &xrange))
> > > > +		if (parse_port_range(p, (const char **)&p, &xrange))
> > > >  			goto bad;
> > > >  		if (p != ep) /* Garbage after the range */
> > > >  			goto bad;
> > > > @@ -599,7 +601,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > >  		/* Exclude ephemeral ports */
> > > >  		fwd_port_map_ephemeral(exclude);
> > > >  
> > > > -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > > > +		fwd_rule_range_except(fwd, del, proto, addr, NULL, ifname,
> > > >  				      1, NUM_PORTS - 1, exclude,
> > > >  				      1, flags | FWD_WEAK);
> > > >  		return;
> > > > @@ -613,11 +615,32 @@ 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, (const char **)&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 (*p == ':') {
> > > > +			/* There's a range or address to map to as well */
> > > > +			char *addr_end = strchr(p, '/');
> > > > +
> > > > +			if (addr_end) {
> > > > +				*addr_end = '\0';
> > > > +
> > > > +				if (*p == '[' && p[strlen(p) - 1] == ']') {
> > > > +					p[strlen(p) - 1] = '\0';
> > > > +					p++;
> > > > +				}
> > > > +
> > > > +				if (!inany_pton(p + 1, daddr))
> > > > +					die("Bad forwarding address '%s'", p);
> > > > +
> > > > +				p = addr_end;
> > > > +			} else {
> > > > +				daddr = NULL;
> > > > +			}    
> > > 
> > > We probably want to factor some of this address parsing out into a
> > > helper, since we're now doing it twice.  
> > 
> > Yes, definitely, and it should have its own small buffer. The in-place
> > parsing we have is a remnant of my original implementation (originally
> > intended for much simpler cases) but that's what forced me to
> > essentially copy and paste this if I wanted to implement this in a few
> > minutes.  
> 
> Yes.
> 
> > > > +
> > > > +
> > > > +			if (parse_port_range(p + 1, (const char **)&p,
> > > > +			    &mapped_range))
> > > >  				goto bad;
> > > >  			if ((mapped_range.last - mapped_range.first) !=
> > > >  			    (orig_range.last - orig_range.first))
> > > > @@ -629,7 +652,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > >  		if (p != ep) /* Garbage after the ranges */
> > > >  			goto bad;
> > > >  
> > > > -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > > > +		fwd_rule_range_except(fwd, del, proto, addr, daddr, ifname,
> > > >  				      orig_range.first, orig_range.last,
> > > >  				      exclude,
> > > >  				      mapped_range.first, flags);
> > > > @@ -675,7 +698,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
> > > >  
> > > >  	strncpy(buf, optarg, sizeof(buf) - 1);
> > > >  
> > > > -	if ((spec = strchr(buf, '/'))) {
> > > > +	if ((spec = strchr(buf, '/')) &&
> > > > +	    strchr(spec, ':') == strchr(buf, ':')) {    
> > > 
> > > strchr() for ':' needs a lot of caution, since it can appear within
> > > IPv6 addresses.  
> > 
> > Absolutely. This patch actually works only for IPv4. See also:
> > 
> >   https://github.com/containers/container-libs/pull/755#issuecomment-4408900718
> > 
> > We probably need separate helpers extracting different parts to keep
> > this sane (the LLM-sourced workaround shown there is probably correct
> > but pretty hard to audit and hard to reason about).  
> 
> Makes sense.
> 
> > > >  		*spec = 0;
> > > >  		spec++;
> > > >  
> > > > diff --git a/fwd_rule.h b/fwd_rule.h
> > > > index ae9a3cb..3a2a809 100644
> > > > --- a/fwd_rule.h
> > > > +++ b/fwd_rule.h
> > > > @@ -35,6 +35,7 @@
> > > >  /**
> > > >   * struct fwd_rule - Forwarding rule governing a range of ports
> > > >   * @addr:	Address to forward from
> > > > + * @daddr:	Optional address to set as destination when forwarding
> > > >   * @ifname:	Interface to forward from
> > > >   * @first:	First port number to forward
> > > >   * @last:	Last port number to forward
> > > > @@ -47,6 +48,7 @@
> > > >   */
> > > >  struct fwd_rule {
> > > >  	union inany_addr addr;
> > > > +	union inany_addr daddr;    
> > > 
> > > We probably want to rethink the names here.  Both of these are
> > > destination addresses, just one is host side the other is guest side
> > > (or, in general, they're destination addresses for two different
> > > pifs).  "initiating" vs. "target" are probably the terms to use, since
> > > we already use that in the flow table.  
> > 
> > I guess we'll want four addresses anyway, eventually, but that probably
> > needs the implementation of a separate address list with pointers that
> > I've been suggesting in the past to keep memory usage reasonable.  
> 
> I'm not so sure about this.  Theoretically, yes, we could allow full
> four-address, four-port range rules.  But it can get very messy: often
> we have NATs that are essentially independent of port remappings, and
> port remappings that are essentially independent of address.  To
> handle that we either need n*m rules, which is pretty awful or
> applying multiple rules per flow which is pretty confusing.

Unless we find a slightly more expressive way to write addresses and
ports and portions / masks / transformations thereof (I'm not sure if
it's really doable though).

> Although
> it's less general in principle, I think it will be more tractable in
> practice to have several different translation steps with their own
> rule tables, but none of them allowing the fully general four-address
> matching.

Ouch, that sounds a lot like "packet recirculation" in Open vSwitch,
which would add quite some complexity in our case.

The biggest loss of generality with multiple steps is that you might
want to filter on two addresses, and if you want to do that without a
four-address table, you'll need some sort of "tag" (like the
recirculation ID in Open vSwitch) to pass around.

Would (exactly) two tables, both with four IP addresses and four ports,
take care of everything? At that point we don't lose generality on
filtering, so no tags to pass around, and at the same time we could
apply one type of NAT independently from the other (if there are two,
in most cases the second table would be a no-op).

> > For the moment, there are two ways to distinguish these two destination
> > addresses, I think:
> > 
> > 1. outside / inside
> > 
> > 2. initiating / target
> > 
> > I was thinking that 1. is simpler to grasp as it refers to stable
> > concepts (host, container / guest) rather than how a connection came to
> > be (and client / server are much less universal than that).  
> 
> True, but it doesn't generalise well to multiple inside pifs (e.g. one
> per container).  That's why I came up with initiating / target for the
> flow table, and I think we should re-use it here.

Ah, right.

> > Going with that convention means that things will be swapped in terms
> > of 2., though, for outbound forwarding. I still think it's preferable.
> >   
> > > At some point I'm pretty sure we'll also want to put a target pif in
> > > the table.  Not sure if we want to introduce that initially, or
> > > whether just an address makes sense for the first cut.  
> > 
> > I don't think it's really important for the table itself (I would add
> > it if it doesn't add cachelines, and otherwise skip it or add it
> > commented out), but we should make sure the command line extensions
> > we're introducing will allow for that, or backwards compatibility will
> > be complicated to achieve later.  
> 
> Right.
> 
> > > >  	char ifname[IFNAMSIZ];
> > > >  	in_port_t first;
> > > >  	in_port_t last;
> > > > -- 
> > > > 2.43.0  
> > 
> > Note that I'm not working on this right now as it's not immediately
> > helpful:
> > 
> >   https://github.com/containers/container-libs/pull/755#issuecomment-4421500913
> > 
> > so I don't plan to post further versions of this, at least not any time
> > soon. Feel free to pick it up from here, if it makes sense.  
> 
> Understood.

-- 
Stefano

Re: [PATCH HACK] fwd, fwd_rule: Implement configurable destination address mapping

[David Gibson]

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

On Wed, May 20, 2026 at 02:37:27AM +0200, Stefano Brivio wrote:
> On Mon, 18 May 2026 16:01:22 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, May 15, 2026 at 01:28:36AM +0200, Stefano Brivio wrote:
> > > On Thu, 14 May 2026 14:54:33 +1000
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > On Thu, May 07, 2026 at 07:50:36AM +0200, Stefano Brivio wrote:  
> > > > > This isn't complete, it's rather a quick hack to enable early
> > > > > integration testing.
> > > > > 
> > > > > Add a 'daddr' field to forwarding rules, and some rudimentary parsing.
> > > > > 
> > > > > 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
> > > > > 
> > > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>    
> > > > 
> > > > Very nice for a quick hack, and it gets surprisingly far with not much
> > > > code at all.
> > > > 
> > > >   
> > > > > ---
> > > > >  fwd.c      |  4 +++-
> > > > >  fwd_rule.c | 46 +++++++++++++++++++++++++++++++++++-----------
> > > > >  fwd_rule.h |  2 ++
> > > > >  3 files changed, 40 insertions(+), 12 deletions(-)
> > > > > 
> > > > > diff --git a/fwd.c b/fwd.c
> > > > > index d224c0a..75db350 100644
> > > > > --- a/fwd.c
> > > > > +++ b/fwd.c
> > > > > @@ -1095,7 +1095,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->daddr)) {
> > > > > +		tgt->eaddr = rule->daddr;    
> > > > 
> > > > Longer term, there are at least two options for what we want to do
> > > > if the rule doesn't specify a specific destination address:
> > > >   * Use the observed guest address (what we do now)  
> > > 
> > > I would keep that as default for now.  
> > 
> > Of course.  My point is that we'll need to have space in our
> > forwarding rules for saying which approach we're using: Using address == ::
> > can only encode one of them.
> > 
> > > >   * Use the host side destination address (potentially useful if we
> > > >     have multiple containers each assigned different host addresses)  
> > > 
> > > This could be implemented on top of Jon's "multiple addresses" series I
> > > think, even in terms of using multiple observed guest addresses, if
> > > useful.  
> > 
> > Sure.
> > 
> > > > So we'll probably want to allow for some new rule flags to cover
> > > > options like this.  
> > > 
> > > Rather than flags, I've been suggesting address pointers / references.
> > > I'm not sure if flags are generic enough.  
> > 
> > My point is that there's non-address information as well.  If you have
> > a single translated address, fine, but there are multiple cases where
> > you don't, so we need some other way to encode which "auto address"
> > mode we're using.
> 
> Ah, yes, so we'll probably need flags *on top* of that.

Exactly.

> > > At some point, we might want a syntax to refer to "the observed address
> > > of container B". Even if we have just container A at the moment, maybe
> > > we could start implementing something going in that direction.  
> > 
> > So, how we do this relates to two possible models for handling
> > multiple containers.  One is that each container has its own pif.  In
> > that case "observed address on pif XXX" is well defined, but is more
> > or less equivalent to having a destination pif in the rule, which I
> > think we want in any case.
> > 
> > The other model is multiple containers (or guests or whatever) on the
> > same pif, effectively bridged together.
> 
> It doesn't need to be the same PIF, I guess. It could also be that we
> bridge different PIFs together. It's a matter of representation rather
> than functionality. I'm not sure which one is more convenient at this
> stage.

Sorry, to clarify, for this model, I'm assuming the guests are bridged
together downstream of passt, so we're not directly aware of the
details.  If we're bridging pifs together ourself, I'd consider that
the pif-per-container model, just with different rules about how we
forward between guest pifs.

> 
> If we allow multiple guests / containers per PIF, then yes, we'll need
> to decouple the two concepts / names. Worth it?
> 
> > This is arguably less
> > elegant than one-pif-per-container, but I think it's important to
> > support for two reasons:
> >   * Integrates easily with existing solutions that already bridge a
> >     bunch of containers or VMs together.
> >   * It allows things routed via the container (e.g. over a VPN) to get
> >     access via as pasta as well
> > 
> > In this case we can't define "observed address of container B" because
> > we don't have a way to tell what came from container B other than the
> > address they use.
> 
> We could, using MAC addresses (that's kind of the thing a bridge does).

Maybe, but only sort of.  We don't know the MAC addresses of the
guest(s) in advance, any more than we do the IP address.  So then it's
a question of how we discover the relevant MAC addresses, or require
that "address for container B" specify container B by way of MAC
address, which is kind of awkward.

And.., this won't work at all is routing (rather than bridging)
traffic from elswhere onto the pif.

> > > > > +	} 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 5fc04d7..5bce2fb 100644
> > > > > --- a/fwd_rule.c
> > > > > +++ b/fwd_rule.c
> > > > > @@ -465,6 +465,7 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw)
> > > > >   */
> > > > >  static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > > > >  				  uint8_t proto, const union inany_addr *addr,
> > > > > +				  const union inany_addr *daddr,
> > > > >  				  const char *ifname,
> > > > >  				  uint16_t first, uint16_t last,
> > > > >  				  const uint8_t *exclude, uint16_t to,
> > > > > @@ -472,6 +473,7 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del,
> > > > >  {
> > > > >  	struct fwd_rule rule = {
> > > > >  		.addr = addr ? *addr : inany_any6,
> > > > > +		.daddr = daddr ? *daddr : inany_any6,
> > > > >  		.ifname = { 0 },
> > > > >  		.proto = proto,
> > > > >  		.flags = flags,
> > > > > @@ -544,13 +546,13 @@ fail:
> > > > >   */
> > > > >  static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > > >  				 const union inany_addr *addr,
> > > > > -				 const char *ifname,
> > > > > -				 const char *spec)
> > > > > +				 const char *ifname, char *spec)
> > > > >  {
> > > > > +	union inany_addr daddr_buf = inany_any6, *daddr = &daddr_buf;
> > > > >  	uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
> > > > >  	bool exclude_only = true;
> > > > > -	const char *p, *ep;
> > > > >  	uint8_t flags = 0;
> > > > > +	char *p, *ep;
> > > > >  	unsigned i;
> > > > >  
> > > > >  	if (!strcmp(spec, "all")) {
> > > > > @@ -568,7 +570,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_keyword(p, (const char **)&p, "auto") == 0) {
> > > > >  			if (p != ep) /* Garbage after the keyword */
> > > > >  				goto bad;
> > > > >  
> > > > > @@ -586,7 +588,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > > >  			goto bad;
> > > > >  		p++;
> > > > >  
> > > > > -		if (parse_port_range(p, &p, &xrange))
> > > > > +		if (parse_port_range(p, (const char **)&p, &xrange))
> > > > >  			goto bad;
> > > > >  		if (p != ep) /* Garbage after the range */
> > > > >  			goto bad;
> > > > > @@ -599,7 +601,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > > >  		/* Exclude ephemeral ports */
> > > > >  		fwd_port_map_ephemeral(exclude);
> > > > >  
> > > > > -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > > > > +		fwd_rule_range_except(fwd, del, proto, addr, NULL, ifname,
> > > > >  				      1, NUM_PORTS - 1, exclude,
> > > > >  				      1, flags | FWD_WEAK);
> > > > >  		return;
> > > > > @@ -613,11 +615,32 @@ 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, (const char **)&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 (*p == ':') {
> > > > > +			/* There's a range or address to map to as well */
> > > > > +			char *addr_end = strchr(p, '/');
> > > > > +
> > > > > +			if (addr_end) {
> > > > > +				*addr_end = '\0';
> > > > > +
> > > > > +				if (*p == '[' && p[strlen(p) - 1] == ']') {
> > > > > +					p[strlen(p) - 1] = '\0';
> > > > > +					p++;
> > > > > +				}
> > > > > +
> > > > > +				if (!inany_pton(p + 1, daddr))
> > > > > +					die("Bad forwarding address '%s'", p);
> > > > > +
> > > > > +				p = addr_end;
> > > > > +			} else {
> > > > > +				daddr = NULL;
> > > > > +			}    
> > > > 
> > > > We probably want to factor some of this address parsing out into a
> > > > helper, since we're now doing it twice.  
> > > 
> > > Yes, definitely, and it should have its own small buffer. The in-place
> > > parsing we have is a remnant of my original implementation (originally
> > > intended for much simpler cases) but that's what forced me to
> > > essentially copy and paste this if I wanted to implement this in a few
> > > minutes.  
> > 
> > Yes.
> > 
> > > > > +
> > > > > +
> > > > > +			if (parse_port_range(p + 1, (const char **)&p,
> > > > > +			    &mapped_range))
> > > > >  				goto bad;
> > > > >  			if ((mapped_range.last - mapped_range.first) !=
> > > > >  			    (orig_range.last - orig_range.first))
> > > > > @@ -629,7 +652,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,
> > > > >  		if (p != ep) /* Garbage after the ranges */
> > > > >  			goto bad;
> > > > >  
> > > > > -		fwd_rule_range_except(fwd, del, proto, addr, ifname,
> > > > > +		fwd_rule_range_except(fwd, del, proto, addr, daddr, ifname,
> > > > >  				      orig_range.first, orig_range.last,
> > > > >  				      exclude,
> > > > >  				      mapped_range.first, flags);
> > > > > @@ -675,7 +698,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg,
> > > > >  
> > > > >  	strncpy(buf, optarg, sizeof(buf) - 1);
> > > > >  
> > > > > -	if ((spec = strchr(buf, '/'))) {
> > > > > +	if ((spec = strchr(buf, '/')) &&
> > > > > +	    strchr(spec, ':') == strchr(buf, ':')) {    
> > > > 
> > > > strchr() for ':' needs a lot of caution, since it can appear within
> > > > IPv6 addresses.  
> > > 
> > > Absolutely. This patch actually works only for IPv4. See also:
> > > 
> > >   https://github.com/containers/container-libs/pull/755#issuecomment-4408900718
> > > 
> > > We probably need separate helpers extracting different parts to keep
> > > this sane (the LLM-sourced workaround shown there is probably correct
> > > but pretty hard to audit and hard to reason about).  
> > 
> > Makes sense.
> > 
> > > > >  		*spec = 0;
> > > > >  		spec++;
> > > > >  
> > > > > diff --git a/fwd_rule.h b/fwd_rule.h
> > > > > index ae9a3cb..3a2a809 100644
> > > > > --- a/fwd_rule.h
> > > > > +++ b/fwd_rule.h
> > > > > @@ -35,6 +35,7 @@
> > > > >  /**
> > > > >   * struct fwd_rule - Forwarding rule governing a range of ports
> > > > >   * @addr:	Address to forward from
> > > > > + * @daddr:	Optional address to set as destination when forwarding
> > > > >   * @ifname:	Interface to forward from
> > > > >   * @first:	First port number to forward
> > > > >   * @last:	Last port number to forward
> > > > > @@ -47,6 +48,7 @@
> > > > >   */
> > > > >  struct fwd_rule {
> > > > >  	union inany_addr addr;
> > > > > +	union inany_addr daddr;    
> > > > 
> > > > We probably want to rethink the names here.  Both of these are
> > > > destination addresses, just one is host side the other is guest side
> > > > (or, in general, they're destination addresses for two different
> > > > pifs).  "initiating" vs. "target" are probably the terms to use, since
> > > > we already use that in the flow table.  
> > > 
> > > I guess we'll want four addresses anyway, eventually, but that probably
> > > needs the implementation of a separate address list with pointers that
> > > I've been suggesting in the past to keep memory usage reasonable.  
> > 
> > I'm not so sure about this.  Theoretically, yes, we could allow full
> > four-address, four-port range rules.  But it can get very messy: often
> > we have NATs that are essentially independent of port remappings, and
> > port remappings that are essentially independent of address.  To
> > handle that we either need n*m rules, which is pretty awful or
> > applying multiple rules per flow which is pretty confusing.
> 
> Unless we find a slightly more expressive way to write addresses and
> ports and portions / masks / transformations thereof (I'm not sure if
> it's really doable though).

Right, I haven't looked at this in detail yet.

> > Although
> > it's less general in principle, I think it will be more tractable in
> > practice to have several different translation steps with their own
> > rule tables, but none of them allowing the fully general four-address
> > matching.
> 
> Ouch, that sounds a lot like "packet recirculation" in Open vSwitch,
> which would add quite some complexity in our case.

Hmm, from the little I understand packet reciculation from our earlier
discussions, this is actually an attempt to avoid it.  The idea is to
design the tables so that each new flow goes through each table
exactly once.  It costs flexibility, but gains comprehensibility.

> The biggest loss of generality with multiple steps is that you might
> want to filter on two addresses, and if you want to do that without a
> four-address table, you'll need some sort of "tag" (like the
> recirculation ID in Open vSwitch) to pass around.

Right.  The open question here is whether we can design these partial
translation tables in a way that covers enough of the use cases we
care about without requiring loops.

> Would (exactly) two tables, both with four IP addresses and four ports,
> take care of everything? At that point we don't lose generality on
> filtering, so no tags to pass around, and at the same time we could
> apply one type of NAT independently from the other (if there are two,
> in most cases the second table would be a no-op).

Maybe?  The trouble is *any* scheme that allows translating the same
fields twice greatly increases the complexity of listening socket
management: for anything forwarded by the second step, we need to do a
reverse translation through the first step to work out where to
listen.

> > > For the moment, there are two ways to distinguish these two destination
> > > addresses, I think:
> > > 
> > > 1. outside / inside
> > > 
> > > 2. initiating / target
> > > 
> > > I was thinking that 1. is simpler to grasp as it refers to stable
> > > concepts (host, container / guest) rather than how a connection came to
> > > be (and client / server are much less universal than that).  
> > 
> > True, but it doesn't generalise well to multiple inside pifs (e.g. one
> > per container).  That's why I came up with initiating / target for the
> > flow table, and I think we should re-use it here.
> 
> Ah, right.
> 
> > > Going with that convention means that things will be swapped in terms
> > > of 2., though, for outbound forwarding. I still think it's preferable.
> > >   
> > > > At some point I'm pretty sure we'll also want to put a target pif in
> > > > the table.  Not sure if we want to introduce that initially, or
> > > > whether just an address makes sense for the first cut.  
> > > 
> > > I don't think it's really important for the table itself (I would add
> > > it if it doesn't add cachelines, and otherwise skip it or add it
> > > commented out), but we should make sure the command line extensions
> > > we're introducing will allow for that, or backwards compatibility will
> > > be complicated to achieve later.  
> > 
> > Right.
> > 
> > > > >  	char ifname[IFNAMSIZ];
> > > > >  	in_port_t first;
> > > > >  	in_port_t last;
> > > > > -- 
> > > > > 2.43.0  
> > > 
> > > Note that I'm not working on this right now as it's not immediately
> > > helpful:
> > > 
> > >   https://github.com/containers/container-libs/pull/755#issuecomment-4421500913
> > > 
> > > so I don't plan to post further versions of this, at least not any time
> > > soon. Feel free to pick it up from here, if it makes sense.  
> > 
> > Understood.
> 
> -- 
> 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 --]