Re: [PATCH v7 02/13] passt, pasta: Introduce unified multi-address data structures

[Stefano Brivio <sbrivio@redhat.com>]

On Mon, 29 Jun 2026 18:23:21 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> >On Thu, Jun 25, 2026 at 12:56:54AM +0200, Stefano Brivio wrote:
> > On Mon, 22 Jun 2026 11:39:48 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Sat, Jun 20, 2026 at 12:09:51AM +0200, Stefano Brivio wrote:  
> > > > On Sun, 12 Apr 2026 20:53:08 -0400
> > > > Jon Maloy <jmaloy@redhat.com> wrote:
> > > >     
> > > > > As preparation for supporting multiple addresses per interface,
> > > > > we replace the single addr/prefix_len fields with an array. The
> > > > > array consists of a new struct inany_addr_entry containing an
> > > > > address and prefix length, both in inany_addr format.
> > > > > 
> > > > > Despite a lot of code refactoring, there are only two real functional
> > > > > changes:
> > > > > - The indicated IPv6 prefix length is now properly stored, instead
> > > > >   of being ignored and overridden with the hardcoded value 64, as
> > > > >   has been the case until now.
> > > > > - Since even IPv4 addresses now are stored in IPv6 format, we
> > > > >   also store the corresponding prefix length in that format,
> > > > >   i.e. using the range [96,128] instead of [0,32].
> > > > > 
> > > > > Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> > > > > 
> > > > > ---
> > > > > v2: -Using inany_addr instead of protocol specific addresses as
> > > > >      entry address field.
> > > > > 
> > > > > v3: -Merging into one array, directly in struct ctx
> > > > >     -Changed prefix_len and flags fields in struct inany_addr_entry
> > > > >      to uint8_t, since that makes the struct directly migratable
> > > > > 
> > > > > v4: -Updated according to changes in previous commits
> > > > >     -Updated according to feedback from David G.
> > > > >     -Squashed IP4_MASK macro commit into this one
> > > > > 
> > > > > v6: -Renamed and moved some definitions
> > > > >     -Introduced fwd_set_addr() and fwd_get_addr() already in this commit
> > > > >     -Eliminated first_v4/v6() functions, replaced with fwd_get_addr()
> > > > >     -Some other changes as suggested by David G.
> > > > >     -I kept the flag CONF_ADDR_LINKLOCAL, since it will be
> > > > >      needed later in an address selection function.
> > > > > 
> > > > > v7: -Introduced CONF_ADDR_GENERATED flag
> > > > >     -Other fixes based on feedback from David and Stefano.
> > > > >     -I changed signature of inany_prefix_len(), but I did not change
> > > > >      its semantics, since the premise of David's comment is wrong: the
> > > > >      caller does *not* explicitly know he is dealing with an IPv4 address.
> > > > >      In fact, there are examples later in this series where it may be an
> > > > >      IPv6 address, and the caller just trusts he gets the return value in
> > > > >      the appropriate format.
> > > > >     -Introduced the inverse of inany_prefix_len(), called inany_prefix_len6()
> > > > >      which always returns the prefix in IPv6 or mapped IPv4 format.
> > > > >      The name of the function isn't great, but any alternative I came up
> > > > >      with became too long to be practical.
> > > > > ---
> > > > >  arp.c    |  12 ++++-
> > > > >  conf.c   | 143 ++++++++++++++++++++++++++++++-------------------------
> > > > >  dhcp.c   |  14 ++++--
> > > > >  dhcpv6.c |  15 ++++--
> > > > >  fwd.c    | 109 ++++++++++++++++++++++++++++++++++--------
> > > > >  fwd.h    |   4 ++
> > > > >  inany.h  |  41 ++++++++++++++++
> > > > >  ip.h     |   2 +
> > > > >  ndp.c    |  16 +++++--
> > > > >  passt.h  |  67 ++++++++++++++++++++++----
> > > > >  pasta.c  |  25 ++++++----
> > > > >  tap.c    |   7 ++-
> > > > >  12 files changed, 340 insertions(+), 115 deletions(-)
> > > > > 
> > > > > diff --git a/arp.c b/arp.c
> > > > > index bb042e9..a7fd82f 100644
> > > > > --- a/arp.c
> > > > > +++ b/arp.c
> > > > > @@ -41,6 +41,8 @@
> > > > >  static bool ignore_arp(const struct ctx *c,
> > > > >  		       const struct arphdr *ah, const struct arpmsg *am)
> > > > >  {
> > > > > +	const struct guest_addr *a;
> > > > > +
> > > > >  	if (ah->ar_hrd != htons(ARPHRD_ETHER)	||
> > > > >  	    ah->ar_pro != htons(ETH_P_IP)	||
> > > > >  	    ah->ar_hln != ETH_ALEN		||
> > > > > @@ -54,7 +56,8 @@ static bool ignore_arp(const struct ctx *c,
> > > > >  		return true;
> > > > >  
> > > > >  	/* Don't resolve the guest's assigned address, either. */
> > > > > -	if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip)))
> > > > > +	a = fwd_get_addr(c, AF_INET, 0, 0);    
> > > > 
> > > > I guess it's not strictly needed right now to avoid breaking things,
> > > > but, eventually, if we support multiple assigned / configured /
> > > > observed addresses for the guest, we should make sure we don't resolve
> > > > any of them. That is, we should eventually pass am->tip to a lookup
> > > > function. It might be in scope for this series but not necessarily.    
> > > 
> > > I think one of the later patches already does that.  
> > 
> > Ah, right, I actually realised but I then decided to keep this comment
> > (and forgot to update it) because this is another argument in favour of
> > a lookup function (which doesn't appear in the series at all).
> >   
> > > I do wonder if for supporting multiple guest addresses it makes sense
> > > at some point to switch from resolving everything _except_ a known
> > > guest address to only resolving addresses that passt "owns" on the
> > > guest link (basically the gateway address, maybe some NATs).  
> > 
> > That will break compatibility with containers / guests that don't use
> > the address of the default gateway we set (anymore), assuming we set
> > one, or that we advertise.  
> 
> Well, yes.  Trying to keep guests working when they don't use the
> network routing they're supposed to seems even more perverse to me
> than keeping them working when they don't use the address they're
> supposed to, which I've already expressed my opinion on.

But the reason behind it is the same: guests without a DHCP client, so
I don't think it's more or less perverse in its intention.

> I guess it does preclude the option of telling the guest that the
> entire internet is on its subnet, no gateway needed.  Odd as it seems,
> that might be a potentially useful option to have.

Hmm, what precludes what exactly? That actually seems to work with
pasta, people tried "device routes" with Wireguard successfully if I
recall correctly.

> > We also had some reports of users setting up a device route only
> > (perhaps for Wireguard, even though that would more naturally be a
> > point-to-point configuration). I can't find them right now but I have a
> > rather clear memory. If those are *not* point-to-point (just like in
> > those setups if I recall correctly), pasta resolving any IP address is
> > rather convenient.

-- 
Stefano