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: 4142 bytes --]

On Fri, May 15, 2026 at 01:28:33AM +0200, Stefano Brivio wrote:
> On Thu, 14 May 2026 16:30:16 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Sun, Apr 12, 2026 at 08:53:08PM -0400, Jon Maloy wrote:
> >
> > > [...]
> > >
> > > +/** inany_prefix_len() - Convert prefix length to native format
> > > + * @addr:	IPv4 or IPv6 address
> > > + * @prefix_len:	prefix length (any format, auto-detected)  
> > 
> > If it's pertaining to an inany, it should always be IPv6 format.
> > Auto-detecting just masks bugs elsewhere.
> > 
> > > + *
> > > + * Return: prefix length in native format (0-32 for IPv4, 0-128 for IPv6)
> > > + */
> > > +static inline int inany_prefix_len(const union inany_addr *addr,
> > > +				   int prefix_len)
> > > +{
> > > +	if (inany_v4(addr) && prefix_len >= 96)  
> > 
> > Yikes.  Even ignoring the fact I don't like this function at all
> 
> Can you then please suggest / sketch another solution? We went back and
> forth on this topic quite a bit but we couldn't come up with anything
> more elegant so far.

Some of the back of the forth was while I was away, so I've not
actually grasped the argument for this function's existence.  You've
asserted that there are cases where it makes sense, but I haven't yet
seen them.  I'm a bit baffled as to how that can be, because a prefix
length is not a meaningful quantity on its own - it only makes sense
as part of a prefix - i.e. it is always attached to an address.  So,
the format of that address should also dictate the format of that
length: 0..32 for an in_addr, 0..128 for an in6_addr.  I'm arguing
that it should also always be 0..128 for an inany_addr, regardless of
its value, since an inany_addr is at least sometimes interchangeable
with an in6_addr, but not with an in_addr.  That is, I think the
length format should always be determined by the corresponding
address's C type, not it's value (including it's "type" in the network
sense).

There might be some advantages to making the association explicit:
that is defining in4_prefix, in6_prefix and inany_prefix types.  But
that might also have some awkwardness (those will have odd length
leading to poor array packing, for example).  So I'm also ok with
the link between the address and length in a prefix being implicit.

So, I guess the concrete suggestion I have (which I thought I made in
an earlier round) is an inany_prefix_v4() function - a variant on
inany_v4() which converts both the address and length simultaneously
into v4 format, or fails if the input is not IPv4.

> > this
> > isn't good.  If we somehow get a IPv4-mapped inany with a prefix_len
> > that's < 96, something has already gone wrong.  This, however, will
> > just fall back to the IPv6 case.  So if prefix_len is 33..96 it will
> > return something entirely nonsensical, and if it's 0..32 it will
> > return something that looks ok, but ignores the fact that something is
> > already wrong.
> > 
> > > +		return prefix_len - 96;
> > > +
> > > +	return prefix_len;
> > > +}
> > > +
> > > +/** inany_prefix_len6() - Convert prefix length to generic format
> > > + * @addr:	IPv4 or IPv6 address
> > > + * @prefix_len:	prefix length (any format, auto-detected)
> > > + *
> > > + * Return: prefix length in generic format (96-128 for IPv4, 0-128 for IPv6)
> > > + */
> > > +static inline int inany_prefix_len6(const union inany_addr *addr,
> > > +				    int prefix_len)  
> > 
> > This function should not be necessary.  If the prefix_len is attached
> > to an inany, it should already be in IPv6 format.
> > 
> > > +{
> > > +	if (inany_v4(addr) && prefix_len && prefix_len <= 32)
> > > +		return prefix_len + 96;  
> > 
> > Similar comments here.  Also a 0.0.0.0/0 is a valid prefix which
> > should be translated to ::ffff:0:0/96, not to ::ffff:0:0/0 as this
> > function will.
> > 
> > > [...]
> 
> -- 
> 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 --]