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=202510 header.b=P6pBZAbq; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 41FD25A0BC2 for ; Fri, 14 Nov 2025 01:26:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1763079985; bh=Spq+X8MkfZOcyto95bWEbA5CZZ6fsyNUPkIc45MzXlA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=P6pBZAbqHlf0yQW97Gd/aIkEngeqB9la2QvUeyCn+gKh9luOX6z+16l6JO8u2xKqp aBBxSXUhNT8guaSCcmb6MYhAVNdlnxo3puLtHUxfGLhlYE5IYMA+vth0aB7JP4e4Uy q0XzhYJoD36dWBFL+/zNih069FKgEErNMdaacd44wo/88Sn4pVvkwUjO13d3UiUpQK g2Gkecz73MfFbAjkP4wKHmRgoIQsdB269YFLn6+VK/Lth3YMQdRAlnJoNrGJfhCbQg J3gbtwdWhLcM3aRQtzwvUlMG6kOX/vFLPA/IZLGcIWTyhpj2Szf9+2fEjwjHl2GzPa h6tvTGBufYong== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4d6ycK1hh1z4wMB; Fri, 14 Nov 2025 11:26:25 +1100 (AEDT) Date: Fri, 14 Nov 2025 11:24:31 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 6/8] util: Fix setting of IPV6_V6ONLY socket option Message-ID: References: <20251029062628.1647051-1-david@gibson.dropbear.id.au> <20251029062628.1647051-7-david@gibson.dropbear.id.au> <20251113073335.3a73f9b9@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="wl1UBylsCLSsHarF" Content-Disposition: inline In-Reply-To: <20251113073335.3a73f9b9@elisabeth> Message-ID-Hash: 4AUIRSCZXVRXW4HCRYXXGF6MXAYXRECC X-Message-ID-Hash: 4AUIRSCZXVRXW4HCRYXXGF6MXAYXRECC 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: 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: --wl1UBylsCLSsHarF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Nov 13, 2025 at 07:33:35AM +0100, Stefano Brivio wrote: > On Wed, 29 Oct 2025 17:26:26 +1100 > David Gibson wrote: >=20 > > Currently we only call setsockopt() on IPV6_V6ONLY when we want to set = it > > to 1, which we typically do on all IPv6 sockets except those explicitly= for > > dual stack listening. That's not quite right in two ways: > >=20 > > * Although IPV6_V6ONLY=3D=3D0 is normally the default on Linux, that c= an be > > changed with the net.ipv6.bindv6only sysctl. It may also have diffe= rent > > defaults on other OSes if we ever support them. We know we need it = off > > for dual stack sockets, so explicitly set it to 0 in that case. > >=20 > > * At the same time setting IPV6_V6ONLY to 1 for IPv6 sockets bound to a > > specific address is harmless but pointless. Don't set the option at= all > > in this case, saving a syscall. >=20 > I haven't checked the implications of this but __inet6_bind() handles > address "types" IPV6_ADDR_ANY and IPV6_ADDR_MAPPED in the same way, for > IPV6_V6ONLY purposes. I'm not sure if this has any influence on > functionality though. AFAICT, technically yes, but not in a way that really matters. IIUC for a v4-mapped address using IPV6_V6ONLY=3D0 will let the socket handle IPv4 traffic with the corresponding address. IPV6_V6ONLY will only handle IPv6 traffic that actually has the v4-mapped address on it. We won't actually get to the point of passing v4-mapped addresses to the kernel in passt/pasta, because we already use those to mean "IPv4" and will go to the IPv4 paths. > > Signed-off-by: David Gibson > > --- > > util.c | 27 +++++++++++++++++++++------ > > 1 file changed, 21 insertions(+), 6 deletions(-) > >=20 > > diff --git a/util.c b/util.c > > index c94efae4..62f43895 100644 > > --- a/util.c > > +++ b/util.c > > @@ -45,14 +45,14 @@ > > * @type: epoll type > > * @sa: Socket address to bind to > > * @ifname: Interface for binding, NULL for any > > - * @v6only: Set IPV6_V6ONLY socket option > > + * @v6only: If >=3D 0, set IPV6_V6ONLY socket option to this value > > * @data: epoll reference portion for protocol handlers > > * > > * Return: newly created socket, negative error code on failure > > */ > > static int sock_l4_(const struct ctx *c, enum epoll_type type, > > const union sockaddr_inany *sa, const char *ifname, > > - bool v6only, uint32_t data) > > + int v6only, uint32_t data) > > { > > sa_family_t af =3D sa->sa_family; > > union epoll_ref ref =3D { .type =3D type, .data =3D data }; > > @@ -101,9 +101,11 @@ static int sock_l4_(const struct ctx *c, enum epol= l_type type, > > =20 > > ref.fd =3D fd; > > =20 > > - if (v6only) > > - if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &y, sizeof(y))) > > - debug("Failed to set IPV6_V6ONLY on socket %i", fd); > > + if (v6only >=3D 0) > > + if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, > > + &v6only, sizeof(v6only))) > > + debug("Failed to set IPV6_V6ONLY to %d on socket %i", > > + v6only, fd); >=20 > Nit: curly brackets (two pairs) for consistency. Fixed. > > =20 > > if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &y, sizeof(y))) > > debug("Failed to set SO_REUSEADDR on socket %i", fd); > > @@ -186,7 +188,16 @@ int sock_l4(const struct ctx *c, enum epoll_type t= ype, > > const union sockaddr_inany *sa, const char *ifname, > > uint32_t data) > > { > > - return sock_l4_(c, type, sa, ifname, sa->sa_family =3D=3D AF_INET6, d= ata); > > + int v6only =3D -1; > > + > > + /* The option doesn't exist for IPv4 sockets, and is irrelevant for I= Pv6 > > + * sockets with a non-wildcard address. >=20 > Same as above: I don't think this is true, strictly speaking, but I > didn't check whether this inaccuracy is in any way relevant. Right, so technically yes, but I don't think it's relevant for the reasons I gave above. I've rephrased to "we don't care about it for IPv6 sockets with ...". Does that help? > > + */ > > + if (sa->sa_family =3D=3D AF_INET6 && > > + IN6_IS_ADDR_UNSPECIFIED(&sa->sa6.sin6_addr)) > > + v6only =3D 1; > > + > > + return sock_l4_(c, type, sa, ifname, v6only, data); > > } > > =20 > > int sock_l4_dualstack(const struct ctx *c, enum epoll_type type, > > @@ -198,6 +209,10 @@ int sock_l4_dualstack(const struct ctx *c, enum ep= oll_type type, > > .sa6.sin6_port =3D htons(port), > > }; > > =20 > > + /* Dual stack sockets require IPV6_V6ONLY =3D=3D 0. Usually that's t= he > > + * default, but sysctl net.ipv6.bindv6only can change that, so set the > > + * sockopt explicitly. > > + */ > > return sock_l4_(c, type, &sa, ifname, 0, data); > > } > > =20 >=20 > The rest of the series looks good to me, including 8/8, but it's not > clear to me what the "secondary reasons" to consider 8/8 at this stage > might be, so I'm not actually sure what to do with it. >=20 > Is it because it drops some code? * It drops some code * I think it will address bug 113 (still need to check) * It might also help for some other systemd-resolved edge cases * It will make life a bit easier to implement bug 171 * I think the improved symmetry will make other flexible forwarding changes a bit easier I'll look into bug 113 and revise the commit message for 8/8 in the next spin. --=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 --wl1UBylsCLSsHarF Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmkWdq0ACgkQzQJF27ox 2GcEWw/+M4isaeXVlzctEH6WPiT+Ij6KHOGpMlC7kE9PQxwsi+0KC4mfxABeqUNU w1dCAi4drfOQj8Kj7y+5Oc2Z5mAHTuPtC8M+iHaDaTrgXRvMjdrhir5QZaRW5bVq h3ZkwcKAzdsDs6qgyvFZNwX9ZjdsgLQGSnlTaFefLgGn9RG+PFkncNPs4KBbCXFg 6mdqKv3HmS1eAIiaxfMky3kH/CGk9nn2vBLcnxIeEneF6s56Ik2dHVB2oGg5sSmx H/MRF6Knh6l0kxD4A+fEnvrbdqjyJLVO4cNDI3XOKwj0pWAjdv59ZwJVZ6Nk5d+1 e1+S84N0E4NhCWrNCvuCpmoX1tXltsik+9zvKUi2BCy4p9re/bDu6oyeXLX5UhvL tlL5JCtWn9QLaf5R/dVrWxYiLdeAO3MuYP/y7Gt5iNZcw64XObfeIN4YPnkTW73n mgBtehnVA97l+XtETProkPr9VUje6rh0IdyS3g5mLhjcQxS9Vz1kzBxkLZVbj300 7KJ8cK2LFcN8T1iYIWASeM3WxRHSkR4LdkDXJM/e4Ga4wcA9CIha+48Q5yO4q1KE W4TmK1NVID180Irk4+T9i9cqTf0Mn8S+xPujX6FevnjflYxHqnDP+6NthblO9Lbe Xij46Ec6TrSGSqXvoiiEfjOHB2hKXvnTALyO0hSM95e7UetYaeY= =of1F -----END PGP SIGNATURE----- --wl1UBylsCLSsHarF--