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