On Sat, Jun 20, 2026 at 12:09:51AM +0200, Stefano Brivio wrote: > On Sun, 12 Apr 2026 20:53:08 -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 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 > > > > --- > > 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. 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). [snip] > > + > > + if (c->addr_count >= MAX_GUEST_ADDRS) > > + return; > > + > > + a = &c->addrs[c->addr_count++]; > > + > > +found: > > + a->addr = *addr; > > + a->prefix_len = inany_prefix_len6(addr, prefix_len); > > + a->flags = flags; > > +} > > + > > +/** > > + * fwd_get_addr() - Get guest address entry matching criteria > > + * @c: Execution context > > + * @af: Address family (AF_INET, AF_INET6, or 0 for any) > > I think AF_UNSPEC (that you seem to use later?) is more common to denote > "any". Not a strong preference, 0 might have some advantages as well. As it happens, AF_UNSPEC has the value 0, but I agree using the name is better. > > + * @incl: Flags that must be present (any-match) > > + * @excl: Flags that must not be present > > + * > > + * Return: first address entry matching criteria, or NULL > > + */ > > +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t af, > > + uint8_t incl, uint8_t excl) > > +{ > > + const struct guest_addr *a; > > + > > + for_each_addr(a, c->addrs, c->addr_count, af) { > > + if (incl && !(a->flags & incl)) > > Regardless of my CONF_ADDR_PICK_ANY suggestion above, it might be a bit > surprising that 0 matches all flags (but so does 0xff). If it needs to > be like that for whatever reason, I think that deserves a mention in > the comment to @incl ("0 means any"). Yeah, I also don't love that special case. [snip] > > +/** > > + * for_each_addr() - Iterate over addresses in array > > + * @a: Pointer variable for current entry (struct guest_addr *) > > + * @addrs: Array of guest addresses (e.g., c->addrs) > > + * @count: Number of addresses (e.g., c->addr_count) > > + * @af: Address family filter: AF_INET, AF_INET6, or 0 for all > > + */ > > +#define for_each_addr(a, addrs, count, af) \ > > + for (int i_ = next_addr_idx_((addrs), (count), 0, (af)); \ > > + i_ < (count) && ((a) = &(addrs)[i_], true); \ > > Why do you need that ", true"? Isn't an extra () pair enough to silence > warnings about evaluating the assignment as a condition? I find it a > bit hard to read like that. I don't think it's there just for the warning. That's the loop condition, so you don't want it to stop if &(addrs)[i_] happens to have value 0. -- 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