From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=QrR6vMxY; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 238C95A0271 for ; Mon, 22 Jun 2026 04:40:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1782096000; bh=5CeJdv5uaEFVBTW1sUHQOHh4lhLGK/bqHGjgJY01QhU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=QrR6vMxYfSl6u01hGpkWeqm80Ry6O0meHA02a9AfiKIl+y/GqE1TdPcyGjz+i0upJ MKShZFS0FHM9VXNGNKqUg15ZbxRa+YKmB9wqahZFk7jRelJMitEMothjQRDJ5K/cvi GiumCDH2vI67nwBFrcbl6r+xrdbP2sj6syU+0/h/KocqIzfxu3/Bt3Mp2YqHw1VKOK WspXjhMwwR0jFIt3YPA+S3P8dWRmyK1vbwH8iF00/xCn7hPjo8GX0dwxKTdy0rHrGr uFsLkJxd+TBB2yzOjYooJYkPHzK5fwiXHswnGsYfkG6NT0TKN3vLDCUgyTf6TQipK8 D0f1tnTa/jLPQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gkC8w3wcMz4wSx; Mon, 22 Jun 2026 12:40:00 +1000 (AEST) Date: Mon, 22 Jun 2026 11:39:48 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v7 02/13] passt, pasta: Introduce unified multi-address data structures Message-ID: References: <20260413005319.3295910-1-jmaloy@redhat.com> <20260413005319.3295910-3-jmaloy@redhat.com> <20260620000951.09b50452@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="36KcZWYaG30G49BG" Content-Disposition: inline In-Reply-To: <20260620000951.09b50452@elisabeth> Message-ID-Hash: HY6FDTLVS2KNNFDQHRYQW54YSXD62UWR X-Message-ID-Hash: HY6FDTLVS2KNNFDQHRYQW54YSXD62UWR X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Jon Maloy , passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --36KcZWYaG30G49BG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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: >=20 > > 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. > >=20 > > 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]. > >=20 > > Signed-off-by: Jon Maloy > >=20 > > --- > > v2: -Using inany_addr instead of protocol specific addresses as > > entry address field. > >=20 > > 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 > >=20 > > v4: -Updated according to changes in previous commits > > -Updated according to feedback from David G. > > -Squashed IP4_MASK macro commit into this one > >=20 > > 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. > >=20 > > 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 addre= ss. > > 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(-) > >=20 > > 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 !=3D htons(ARPHRD_ETHER) || > > ah->ar_pro !=3D htons(ETH_P_IP) || > > ah->ar_hln !=3D ETH_ALEN || > > @@ -54,7 +56,8 @@ static bool ignore_arp(const struct ctx *c, > > return true; > > =20 > > /* Don't resolve the guest's assigned address, either. */ > > - if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) > > + a =3D fwd_get_addr(c, AF_INET, 0, 0); >=20 > 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 >=3D MAX_GUEST_ADDRS) > > + return; > > + > > + a =3D &c->addrs[c->addr_count++]; > > + > > +found: > > + a->addr =3D *addr; > > + a->prefix_len =3D inany_prefix_len6(addr, prefix_len); > > + a->flags =3D flags; > > +} > > + > > +/** > > + * fwd_get_addr() - Get guest address entry matching criteria > > + * @c: Execution context > > + * @af: Address family (AF_INET, AF_INET6, or 0 for any) >=20 > 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)) >=20 > 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_ =3D next_addr_idx_((addrs), (count), 0, (af)); \ > > + i_ < (count) && ((a) =3D &(addrs)[i_], true); \ >=20 > 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. --=20 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 --36KcZWYaG30G49BG Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmo4klAACgkQzQJF27ox 2GdsvBAApe0EeFkaHAK3znUVv06JmANnRcQnhC+hzfezb61zAwN25GW3krSqmaZ+ 67t0y4FMMj4Fh3AgtSlCi+SKEztDj/sm1ZLC/0BoiZwRmGY15MJvCvmsBTc+tsAQ A3NKskVeqQe+oOQboWQ3aAsOX7VIcmr20pkvu3IQJ9hLyWv8cTOmQDY99Z3mv9Yz 9yd09bLBNEIe3iuMH+0SPawXDbK2YIi/E1TL+ZnYDPjg93bRExh52bzqLPyDhNar zmnPfAyUpAdQVTnSuqaCMjyUMR5nMHMt7ry6OE3a1B/e0+pUwCC0DVCzTua7N4Qq pO/+fuxLv4z4sypj5NhWNwFQefhE+LWE9ayJHx2McNdHqSRMJ/MT4pOq/AljXjY6 FDDRta3rswX9gfpoSWPh+2g8ZsU4kiXxIzTGG6QwjRU01sEnEwolPsM5hxxQ3W+U MBIfPTe46RZNa6esTAWn05rz4c5BqvGFy7SJSu+R6xV5bgxj1DNM5OGTyU2bRN/p 97cLcwy2tsVdARLtz9y0N7hAdOLhSeE9xpJRER7YVGAu90KyoDffD9dfgB+kvm+y 1ryy3Sq43hnZJ5DIF/0P+V2st6qdCHNpC/+py3ovv0rxhmSfcCCoVM9h1polajJ9 311L+BWLdBaLPbAyjkc8EicRaas1lPGSnSmjwP2DgaWTqaH1Keg= =oJwp -----END PGP SIGNATURE----- --36KcZWYaG30G49BG--