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

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 27 May 2026 13:40:51 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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.

Hmm, no, wait, I was suggesting something different: instead of two
incl, excl arguments, a single 'flags' argument which is interpreted as
an exclusion mask if an inversion of a single bit is set, so that you
could do:

	fwd_select_addr(c, AF_INET6, CONF_ADDR_OBSERVED);

to get observed addresses, and:

	fwd_select_addr(c, AF_INET6, ~CONF_ADDR_OBSERVED);

to get everything else (see that in use in conn_flag_do(), tcp.c).

But if multiple bits can be filtered on, that won't work.

> But AFACT handling that
> encoding is not actually implemented in fwd_get_addr() in this series.

The usage here is different: this seems to be meant to exclude
link-local addresses (and, I concur, CONF_ADDR_LINKLOCAL should be
another type of flag) if tgt->oaddr is not link-local, and exclude
all other types if it is.

I don't think it will work for all the other types because, if 'll' is
false, we'll exclude CONF_ADDR_USER and CONF_ADDR_HOST anyway (which
are the only ones being selected), but it's rather an issue with how
'excl' is set rather than a particular encoding.

> Plus, that encoding technique doesn't really work any more if you
> allow multiple bits in the excl fields.

Right, the one I suggested won't work. The one that was intended here
should still work, in principle, I guess.

> > +		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;

-- 
Stefano