Re: [PATCH v6 02/13] ip: Introduce unified multi-address data structures

On Fri, Apr 03, 2026 at 08:20:12AM +0200, Stefano Brivio wrote:

On Fri, 3 Apr 2026 12:25:44 +1100 David Gibson wrote:

...

On Mon, Mar 30, 2026 at 11:57:10PM +0200, Stefano Brivio wrote:

...

On Sat, 21 Mar 2026 20:43:22 -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 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

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

[snip]

...

...

- if (!ip4->prefix_len) { - in_addr_t addr = ntohl(ip4->addr.s_addr); - if (IN_CLASSA(addr)) - ip4->prefix_len = (32 - IN_CLASSA_NSHIFT); - else if (IN_CLASSB(addr)) - ip4->prefix_len = (32 - IN_CLASSB_NSHIFT); - else if (IN_CLASSC(addr)) - ip4->prefix_len = (32 - IN_CLASSC_NSHIFT); - else - ip4->prefix_len = 32; + fwd_set_addr(c, &inany_from_v4(addr), CONF_ADDR_HOST, + prefix_len + 96);

Somewhat symmetric to David's comment to inany_prefix_len(): fwd_set_addr() is already checking if the address is IPv4 or IPv6, so you could pass the actual prefix length.

I disagree - in fact I suspect the + 96 is there because of my earlier feedback. When a prefix length is associated with an inany, it should always be encoded as for an IPv6 address, since an inany essentialy is an IPv6 address. There are cases where we can directly use an inany as an IPv6 address, without caring that it might be (dual stack socket interfaces). If we need a prefix length in that context, we need to be able to treat it as an IPv6 prefix length, without having to adjust it for the case of a v4-mapped address.

The +96 here is representing that we're converting from a strictly IPv4 addr+prefix_len to an inany addr+prefix_len.

Hmm, I see. I considered the prefix length argument in this case a separate bit of information compared to the inany stuff, nothing else than a "prefix length" argument. But I see why you might consider that strictly in conjunction with the inany argument.

I mean, fwd_set_addr() is essentially storing a addr+prefix_len as a prefix in the table, so I think it's reasonable to consider the passed address and length to be linked.

...

...

Having to add 96 for IPv4 looks rather bug-prone for the future.

It definitely has gotchas, but I think the ones we get by inconsistently encoding the prefix length for an inany are worse.

I do think the gotches here could be significantly mitigated by having variants of inany_v4(), inany_from_v4() and the like which convert an entire prefix (i.e. address and prefix length in the same call).

Ah, yes, that would definitely help. I'm not sure how complicated it is. Maybe we could as a start have a INANY_PREFIX_LEN_FROM_IPV4(x) macro or something. But I'd say let's leave + 96 here for the moment if we risk adding more interruptions / complexity.

I think it's not too bad to (conditionally) get an IPv4+prefix_len from an inany+prefix_len. The other way is more awkward, but also less common. As noted, having explicit types representing prefixes would probably help. -- 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