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=Aa43m38j; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 8A92D5A0269 for ; Mon, 13 Jul 2026 07:39:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1783921140; bh=H3RUCigC7r2DOQpunkyRzzWHexC4WoOYfMzK+IeveXU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Aa43m38jmcnVPXqVkC0R04OVLgsl15SY7QstCZU7tSZSYqp4vQzsgTIhD9neMQdix oXkvmLrD432yfY9qz99cNsY/28QauVBrX/YLYeUlTy0cqt22+szTlBa4oWUCF4XEhM yW9kfxVTUq0LmBUPWTuY9N+j3b3PQt2YHZQYRn83YObraIuST8DUN7MTzB4hY070SO osBOXKp3Q4HtginlxsqxkPYCHEjOVPeuSynxFcWn/SCX/k5TvfntjB3qpdlWCPl3UU CEOXtskdC9fOU+5I72VWi8u+O6AZ0AHRJqom7wqafF2bg1NVI93yK8ZHZRI/qI6L+O S6KZAl5xl2dmw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gzB7m1rdVz4wHt; Mon, 13 Jul 2026 15:39:00 +1000 (AEST) Date: Mon, 13 Jul 2026 14:16:47 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v8 02/14] passt, pasta: Introduce unified multi-address data structures Message-ID: References: <20260626024519.3701556-1-jmaloy@redhat.com> <20260626024519.3701556-3-jmaloy@redhat.com> <769f4409-c317-4736-98b3-6fe8f6d4078d@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ps9vs6J9L6PyTc+E" Content-Disposition: inline In-Reply-To: <769f4409-c317-4736-98b3-6fe8f6d4078d@redhat.com> Message-ID-Hash: TKHS4A2RXNS4TRV4E6WRUO2Y7LEVBD2B X-Message-ID-Hash: TKHS4A2RXNS4TRV4E6WRUO2Y7LEVBD2B 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: sbrivio@redhat.com, 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: --ps9vs6J9L6PyTc+E Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 10, 2026 at 05:31:23PM -0400, Jon Maloy wrote: > > > - if (!ip4->prefix_len) { > > > - in_addr_t addr =3D ntohl(ip4->addr.s_addr); > > > - if (IN_CLASSA(addr)) > > > - ip4->prefix_len =3D (32 - IN_CLASSA_NSHIFT); > > > - else if (IN_CLASSB(addr)) > > > - ip4->prefix_len =3D (32 - IN_CLASSB_NSHIFT); > > > - else if (IN_CLASSC(addr)) > > > - ip4->prefix_len =3D (32 - IN_CLASSC_NSHIFT); > > > - else > > > - ip4->prefix_len =3D 32; > > > + fwd_set_addr(c, &inany_from_v4(addr), CONF_ADDR_HOST, > > > + prefix_len + 96); > > > } > > > - ip4->addr_seen =3D ip4->addr; > > > - > > > + a =3D fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); > >=20 > > Nit: maybe assert(a), since I doubt static checkers will be able to > > reason that the fwd_set_addr() above means this fwd_get_addr() can't > > fail. Also, I think that go inside the if block, since the other case > > means we already have a perfectly good a. >=20 > I think my conclusion was that the static checker ignores assert() as a > conditonal test, but I can try it. I'm not really sure what you mean by this. I don't know for certain with this particular case and each static checker. I do know that there are a *heap* of cases where an assert() can fix static checker warnings because it lets the checker know we can't have reached this point in the "bad" condition. [snip] > > > +/** > > > + * struct guest_addr - Unified IPv4/IPv6 address entry > > > + * @addr: IPv4 (as mapped) or IPv6 address > > > + * @prefix_len: Prefix length in IPv6/IPv4-mapped [0,128]/[96,128] f= ormat > > > + * @flags: CONF_ADDR_* flags > > > + */ > > > +struct guest_addr { > > > + union inany_addr addr; > > > + uint8_t prefix_len; > > > + uint8_t flags; > > > +#define CONF_ADDR_USER BIT(0) /* User set via -a */ > > > +#define CONF_ADDR_HOST BIT(1) /* From host interface */ > > > +#define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ > > > +#define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ > > > +#define CONF_ADDR_ANY 0xff /* Match any flag */ > >=20 > > In general I like CONF_ADDR_ANY instead of 0 being special-cased. But > > with LINKLOCAL in the mix it doesn't quite work. ANY meaning "USER or > > HOST or GENERATED" makes sense. ANY meaning "USER or HOST or > > GENERATED or LINKLOCAL" is nonsense. I suggest you move LINKLOCAL > > into a different field, as Stefano has suggested to handle this (maybe > > "provenance" for USER/HOST/GENERATED and "flags" for LINKLOCAL?). Or > > you could remove the LINKLOCAL flag from the structure entirely: the > > information is already present in the address itself. >=20 > I was hoping to postpone this to a commit after this series, but I see yo= ur > point. I can try with a 'scope' field, as suggested earlier, or just skip= it > altogheter and using an is_link_local() function. Sorry, I thought there was already an inany_is_linklocal() function, but I see that there's only inany_is_linklocal6(). So that will need to be extended to handle IPv4 as well. [snip] > > > diff --git a/pasta.c b/pasta.c > > > index 4e7ee542..9ef3ac00 100644 > > > --- a/pasta.c > > > +++ b/pasta.c > > > @@ -330,6 +330,8 @@ void pasta_ns_conf(struct ctx *c) > > > if (c->pasta_conf_ns) { > > > unsigned int flags =3D IFF_UP; > > > + const struct guest_addr *a; > > > + int plen =3D 0; > > > if (c->mtu) > > > nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu); > > > @@ -341,10 +343,17 @@ void pasta_ns_conf(struct ctx *c) > > > if (c->ifi4) { > >=20 > > Nit: I think you can reduce the very deep nesting here, by moving the > > fwd_get_addr() and changing the if below to.. >=20 > This is already done in a later patch, by introducing two subfuntions. > However, this has caused lot of trouble at each rebase, so I think I will > move it to a separate, standalone patch before this series. Sounds like a good idea. --=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 --ps9vs6J9L6PyTc+E Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpUZqQACgkQzQJF27ox 2Gf1Bw/7B4eKVifMjB9rmUaCIVLizIO/zLKeZf/jp/e/e7X+/nr0HodE51bqbOmX wvkjSNAIPfZYTaiHXKJDMkP3B4/utBdp8XH+o/Uhiu3i/QFkELY4+wdH/gl0vHZl jpvPSpvo5w38HjZ+iCPG5rSFmnkYLYkO03hlz8iqE2kQ/wzqilI78c/DA2SJUtoZ UtPzyG31q0dAMHDGFPVYqsKlhfB6Tw7Z+bHT02d3G5sz4UOiG1IXTYfZ9WNrqdyt sHUFNPQ8dI9Z07zOT/Atq9QXcGlABxTypPXl4Gjp8n2F2T1A9mCYUn1f1rRs941x /zEAGN/fQMCjd5KQFWW5HQsXErav8BHRul/wGNnI21dTD2my0zAzm2l9RPPKTfgm /LwfIHMWaXZpuLJmwY3I9V/9QSLgiEg1ETQJ0uk8xsXdopY4b2IPSXGkh13g3z3k MpDRsmIgHX4H9Aj6NgMOCeN/Y7X8iA6rp991KSW/RPElrwWAKZavUAJUfc3ZpFgr AUcOTRC8alEofWMD7ye8UxDXulRRHlN/UAU+6EW+kp97j6WpdPGR/Of9W3UGQBGM oUWD4ZE2UpbWXt+3cmrTuIOLKcxOI9zmXwu2SqzGH/O7THS+Qtm4st3fNXloCkcn l/AT+eUMXOafTqsxxBjyI4BGQoWwkbJ2AVqQRp9p8L68hGocA2s= =75lr -----END PGP SIGNATURE----- --ps9vs6J9L6PyTc+E--