Re: [PATCH v7 09/13] conf, pasta: Track observed guest IPv6 addresses in unified address array

[David Gibson <david@gibson.dropbear.id.au>]

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

On Sun, Apr 12, 2026 at 08:53:15PM -0400, Jon Maloy wrote:
> We remove the addr_seen and addr_ll_seen fields in struct ip6_ctx
> and replace them by setting CONF_ADDR_OBSERVED and CONF_ADDR_LINKLOCAL
> flags in the corresponding entry in the unified address array.
> 
> The observed IPv6 address is always added/moved to position 0
> in the array, improving chances for fast lookup.
> 
> The separate check against addr_seen in fwd_guest_accessible() can now
> be removed because the observed address is now in the unified array,
> and the existing for_each_addr() loop already checks against all
> addresses, including this one.
> 
> This completes the unification of address storage for both IPv4 and
> IPv6, enabling future support for multiple guest addresses per family.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> 
> ---
> v5: - Made to use same algorithm and function as IPv4 for inserting
>       observed into the array.
> 
> v6: - Re-introduced code that by accident had been moved to the
>       previous commit.
>     - Some fixes based on feedback from David G.
> 
> v7: - Added a commit at the beginning of the series addressing
>       Stefano's concern about DHCPv6 reply addresses.
>     - Some other updates based on feedback from David and Stefano.
> ---
>  conf.c    |  4 ----
>  dhcpv6.c  |  4 +---
>  fwd.c     | 38 +++++++++++++++++++++++---------------
>  inany.h   |  3 +++
>  migrate.c | 37 +++++++++++++++++++++++++++----------
>  passt.h   |  4 ----
>  pasta.c   | 11 +++++++----
>  tap.c     | 17 +++++------------
>  8 files changed, 66 insertions(+), 52 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index f503d0f..3cb3553 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -827,7 +827,6 @@ static unsigned int conf_ip6(struct ctx *c, unsigned int ifi)
>  			      strerror_(-rc));
>  			return 0;
>  		}
> -		a = fwd_get_addr(c, AF_INET6, CONF_ADDR_HOST, 0);
>  	} else {
>  		rc = nl_addr_get_ll(nl_sock, ifi, &ip6->our_tap_ll);
>  		if (rc < 0) {
> @@ -836,9 +835,6 @@ static unsigned int conf_ip6(struct ctx *c, unsigned int ifi)
>  		}
>  	}
>  
> -	if (a)
> -		ip6->addr_seen = a->addr.a6;
> -
>  	if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw))
>  		ip6->our_tap_ll = ip6->guest_gw;
>  
> diff --git a/dhcpv6.c b/dhcpv6.c
> index 0a064a9..447aaba 100644
> --- a/dhcpv6.c
> +++ b/dhcpv6.c
> @@ -567,8 +567,8 @@ int dhcpv6(struct ctx *c, struct iov_tail *data,
>  	struct opt_hdr client_id_storage;
>  	/* cppcheck-suppress [variableScope,unmatchedSuppression] */
>  	struct opt_ia_na ia_storage;
> -	const struct guest_addr *a;
>  	const struct in6_addr *src;
> +	const struct guest_addr *a;
>  	struct msg_hdr mh_storage;
>  	const struct msg_hdr *mh;
>  	struct udphdr uh_storage;
> @@ -683,8 +683,6 @@ int dhcpv6(struct ctx *c, struct iov_tail *data,
>  	resp.hdr.xid = mh->xid;
>  
>  	tap_udp6_send(c, src, 547, saddr, 546, mh->xid, &resp, n);
> -	if (a)
> -		c->ip6.addr_seen = a->addr.a6;
>  
>  	return 1;
>  }
> diff --git a/fwd.c b/fwd.c
> index 8c7bf91..b177be9 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -1095,14 +1095,6 @@ static bool fwd_guest_accessible(const struct ctx *c,
>  			return false;
>  	}
>  
> -	/* For IPv6, addr_seen starts unspecified, because we don't know what LL
> -	 * address the guest will take until we see it.  Only check against it
> -	 * if it has been set to a real address.
> -	 */
> -	if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen) &&
> -	    inany_equals6(addr, &c->ip6.addr_seen))
> -		return false;
> -
>  	return true;
>  }
>  
> @@ -1305,10 +1297,20 @@ uint8_t fwd_nat_from_host(const struct ctx *c,
>  			}
>  			tgt->oaddr = inany_any4;
>  		} else {
> -			if (c->host_lo_to_ns_lo)
> +			if (c->host_lo_to_ns_lo) {
>  				tgt->eaddr = inany_loopback6;
> -			else
> -				tgt->eaddr.a6 = c->ip6.addr_seen;
> +			} else {
> +				const struct guest_addr *a;
> +
> +				a = fwd_select_addr(c, AF_INET6,
> +						    CONF_ADDR_OBSERVED,
> +						    CONF_ADDR_USER |
> +						    CONF_ADDR_HOST,
> +						    CONF_ADDR_LINKLOCAL);

As for IPv4, do we actually need the two-phase search, or is the
ordering of addresses in the array enough?

> +				if (!a)
> +					return PIF_NONE;
> +				tgt->eaddr = a->addr;
> +			}
>  			tgt->oaddr = inany_any6;
>  		}
>  
> @@ -1346,10 +1348,16 @@ uint8_t fwd_nat_from_host(const struct ctx *c,
>  
>  		tgt->eaddr = a->addr;
>  	} else {
> -		if (inany_is_linklocal6(&tgt->oaddr))
> -			tgt->eaddr.a6 = c->ip6.addr_ll_seen;
> -		else
> -			tgt->eaddr.a6 = c->ip6.addr_seen;
> +		bool ll = inany_is_linklocal6(&tgt->oaddr);
> +		uint8_t excl = ll ? ~CONF_ADDR_LINKLOCAL : CONF_ADDR_LINKLOCAL;

Uuhhh... I recall Stefano suggested using this encoding technique of
using ~flag to indicate a different meaning.  But AFACT handling that
encoding is not actually implemented in fwd_get_addr() in this series.
Plus, that encoding technique doesn't really work any more if you
allow multiple bits in the excl fields.

> +		const struct guest_addr *a;
> +
> +		a = fwd_select_addr(c, AF_INET6, CONF_ADDR_OBSERVED,
> +				    CONF_ADDR_USER | CONF_ADDR_HOST, excl);
> +		if (!a)
> +			return PIF_NONE;
> +
> +		tgt->eaddr = a->addr;
>  	}
>  
>  	return PIF_TAP;
> diff --git a/inany.h b/inany.h
> index 0450c45..ddcf93d 100644
> --- a/inany.h
> +++ b/inany.h
> @@ -60,6 +60,9 @@ extern const union inany_addr inany_any4;
>  #define inany_from_v4(a4)	\
>  	((union inany_addr)INANY_INIT4((a4)))
>  
> +#define inany_from_v6(v6)	\
> +	((union inany_addr){ .a6 = (v6) })
> +
>  /** union sockaddr_inany - Either a sockaddr_in or a sockaddr_in6
>   * @sa_family:	Address family, AF_INET or AF_INET6
>   * @sa:		Plain struct sockaddr (useful to avoid casts)
> diff --git a/migrate.c b/migrate.c
> index 1e02720..2dc4dd9 100644
> --- a/migrate.c
> +++ b/migrate.c
> @@ -56,21 +56,30 @@ struct migrate_seen_addrs_v2 {
>  static int seen_addrs_source_v2(struct ctx *c,
>  				const struct migrate_stage *stage, int fd)
>  {
> -	struct migrate_seen_addrs_v2 addrs = {
> -		.addr6 = c->ip6.addr_seen,
> -		.addr6_ll = c->ip6.addr_ll_seen,
> -	};
> +	struct migrate_seen_addrs_v2 addrs = { 0 };
>  	const struct guest_addr *a;
>  
>  	(void)stage;
>  
> -	/* IPv4 observed address, with fallback to configured address */
> +	/* IPv4 observed address, with fallback to any other non-LL address */

Seems like this change belongs in the previous patch.
>  	a = fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED,
>  			    CONF_ADDR_USER | CONF_ADDR_HOST,
>  			    CONF_ADDR_LINKLOCAL);
>  	if (a)
>  		addrs.addr4 = *inany_v4(&a->addr);
>  
> +	/* IPv6 observed address, with fallback to any other non-LL address */
> +	a = fwd_select_addr(c, AF_INET6, CONF_ADDR_OBSERVED,
> +			    CONF_ADDR_USER | CONF_ADDR_HOST,
> +			    CONF_ADDR_LINKLOCAL);
> +	if (a)
> +		addrs.addr6 = a->addr.a6;
> +
> +	/* IPv6 link-local address */
> +	a = fwd_get_addr(c, AF_INET6, CONF_ADDR_LINKLOCAL, 0);
> +	if (a)
> +		addrs.addr6_ll = a->addr.a6;
> +
>  	memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac));
>  
>  	if (write_all_buf(fd, &addrs, sizeof(addrs)))
> @@ -91,19 +100,27 @@ static int seen_addrs_target_v2(struct ctx *c,
>  				const struct migrate_stage *stage, int fd)
>  {
>  	struct migrate_seen_addrs_v2 addrs;
> +	struct in6_addr addr6, addr6_ll;
>  
>  	(void)stage;
>  
>  	if (read_all_buf(fd, &addrs, sizeof(addrs)))
>  		return errno;
>  
> -	c->ip6.addr_seen = addrs.addr6;
> -	c->ip6.addr_ll_seen = addrs.addr6_ll;
> -
> -	if (addrs.addr4.s_addr)
> +	if (addrs.addr4.s_addr) {
>  		fwd_set_addr(c, &inany_from_v4(addrs.addr4),
>  			     CONF_ADDR_OBSERVED, 0);
> -
> +	}

Another change that belongs in the previous patch.

> +	addr6 = addrs.addr6;
> +	if (!IN6_IS_ADDR_UNSPECIFIED(&addr6)) {
> +		fwd_set_addr(c, &inany_from_v6(addr6),
> +			     CONF_ADDR_OBSERVED, 0);
> +	}
> +	addr6_ll = addrs.addr6_ll;
> +	if (!IN6_IS_ADDR_UNSPECIFIED(&addr6_ll)) {
> +		fwd_set_addr(c, &inany_from_v6(addr6_ll),
> +			     CONF_ADDR_OBSERVED | CONF_ADDR_LINKLOCAL, 0);
> +	}
>  	memcpy(c->guest_mac, addrs.mac, sizeof(c->guest_mac));
>  
>  	return 0;
> diff --git a/passt.h b/passt.h
> index 5da1d55..3ef84eb 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -121,8 +121,6 @@ struct ip4_ctx {
>  
>  /**
>   * struct ip6_ctx - IPv6 execution context
> - * @addr_seen:		Latest IPv6 global/site address seen as source from tap
> - * @addr_ll_seen:	Latest IPv6 link-local address seen as source from tap
>   * @guest_gw:		IPv6 gateway as seen by the guest
>   * @map_host_loopback:	Outbound connections to this address are NATted to the
>   *                      host's [::1]
> @@ -138,8 +136,6 @@ struct ip4_ctx {
>   * @no_copy_addrs:	Don't copy all addresses when configuring namespace
>   */
>  struct ip6_ctx {
> -	struct in6_addr addr_seen;
> -	struct in6_addr addr_ll_seen;
>  	struct in6_addr guest_gw;
>  	struct in6_addr map_host_loopback;
>  	struct in6_addr map_guest_addr;
> diff --git a/pasta.c b/pasta.c
> index f949dc2..f7f1e07 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -366,6 +366,7 @@ static int pasta_conf_routes(struct ctx *c, sa_family_t af, int ifi,
>  void pasta_ns_conf(struct ctx *c)
>  {
>  	unsigned int flags = IFF_UP;
> +	struct in6_addr addr_ll;
>  	int rc;
>  
>  	rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP);
> @@ -415,12 +416,14 @@ void pasta_ns_conf(struct ctx *c)
>  	if (!c->ifi6)
>  		return;
>  
> -	rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi,
> -			    &c->ip6.addr_ll_seen);
> -	if (rc < 0)
> +	rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, &addr_ll);
> +	if (rc < 0) {
>  		warn("Can't get LL address from namespace: %s",
>  		     strerror_(-rc));
> -
> +	} else {
> +		fwd_set_addr(c, &inany_from_v6(addr_ll),
> +			     CONF_ADDR_LINKLOCAL | CONF_ADDR_OBSERVED, 0);
> +	}
>  	rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi);
>  	if (rc < 0)
>  		warn("Can't set nodad for LL in namespace: %s",
> diff --git a/tap.c b/tap.c
> index 7f04e12..7f7f0ce 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -933,20 +933,13 @@ resume:
>  			continue;
>  		}
>  
> -		if (IN6_IS_ADDR_LINKLOCAL(saddr)) {
> -			c->ip6.addr_ll_seen = *saddr;
> +		if (!IN6_IS_ADDR_UNSPECIFIED(saddr)) {
> +			uint8_t flags = CONF_ADDR_OBSERVED;
>  
> -			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen)) {
> -				c->ip6.addr_seen = *saddr;
> -			}
> -
> -			if (!fwd_get_addr(c, AF_INET6, 0, 0)) {
> -				union inany_addr addr = { .a6 = *saddr };
> +			if (IN6_IS_ADDR_LINKLOCAL(saddr))
> +				flags |= CONF_ADDR_LINKLOCAL;
>  
> -				fwd_set_addr(c, &addr, CONF_ADDR_LINKLOCAL, 64);
> -			}
> -		} else if (!IN6_IS_ADDR_UNSPECIFIED(saddr)){
> -			c->ip6.addr_seen = *saddr;
> +			fwd_set_addr(c, &inany_from_v6(*saddr), flags, 0);
>  		}
>  
>  		if (proto == IPPROTO_ICMPV6) {
> -- 
> 2.52.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 --]