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

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