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

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

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

On Thu, Jul 09, 2026 at 03:56:08PM +1000, David Gibson wrote:
> On Thu, Jul 09, 2026 at 02:05:11PM +1000, David Gibson wrote:
> > On Thu, Jun 25, 2026 at 10:45:07PM -0400, Jon Maloy 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 some 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].
> > > 
> > > In conf_ip6(), the explicit IN6_IS_ADDR_UNSPECIFIED() check on the
> > > address at the end of the function is no longer needed: for
> > > host-discovered addresses, it is now checked inside the if (!a) block
> > > before calling fwd_set_addr(); for user-provided addresses (via -a),
> > > validation already rejects unspecified addresses at parse time.
> 
> Following up with a more general observation.  We've now been through
> a bunch of iterations with various problems in the filtering logic of
> fwd_get_addr().  On reflection I think the whole idea of a single
> "get" function with a couple of mask parameters is flawed.  The
> conditions we want to filter on are more complex than this, because we
> do have both different provenances of address and types of address.
> 
> The major use of fwd_get_addr() in these early patches is to replace
> direct references to c->ip4.addr, c->ip6.addr or c->ip6.addr_ll.  In
> later patches, many of these go away, because you instead iterate
> through and do something for *all* relevant addresses.  So, I'd
> suggest instead of trying to make this general-but-actually-it's-not
> lookup function you instead insert 3 specific functions: one that
> retreives an ip4.addr equivalent address, one an ip6.addr equivalent
> and so forth.
> 
> That removes the evidently confusing filtering logic of fwd_get_addr()
> and makes for smaller and clearer changes in the many places that need
> to be changed to use the new data structure.  Through the rest of the
> patches callers of those will mostly go away, and they can be removed
> again.

Oh, and finally.  I'm not going to review the rest of this series.  I
think we need to get the basics of the data structure right, in this
early patch before it's worthwhile having another long revise/review
cycle on the rest.

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