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

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

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

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

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.

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

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