Re: [PATCH v3 6/8] util: Fix setting of IPV6_V6ONLY socket option

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 29 Oct 2025 17:26:26 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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:
> 
>  * Although IPV6_V6ONLY==0 is normally the default on Linux, that can be
>    changed with the net.ipv6.bindv6only sysctl.  It may also have different
>    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.
> 
>  * 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.

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.

> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  util.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> 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 >= 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 = sa->sa_family;
>  	union epoll_ref ref = { .type = type, .data = data };
> @@ -101,9 +101,11 @@ static int sock_l4_(const struct ctx *c, enum epoll_type type,
>  
>  	ref.fd = fd;
>  
> -	if (v6only)
> -		if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &y, sizeof(y)))
> -			debug("Failed to set IPV6_V6ONLY on socket %i", fd);
> +	if (v6only >= 0)
> +		if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY,
> +			       &v6only, sizeof(v6only)))
> +			debug("Failed to set IPV6_V6ONLY to %d on socket %i",
> +			      v6only, fd);

Nit: curly brackets (two pairs) for consistency.

>  
>  	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 type,
>  	    const union sockaddr_inany *sa, const char *ifname,
>  	    uint32_t data)
>  {
> -	return sock_l4_(c, type, sa, ifname, sa->sa_family == AF_INET6, data);
> +	int v6only = -1;
> +
> +	/* The option doesn't exist for IPv4 sockets, and is irrelevant for IPv6
> +	 * sockets with a non-wildcard address.

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.

> +	 */
> +	if (sa->sa_family == AF_INET6 &&
> +	    IN6_IS_ADDR_UNSPECIFIED(&sa->sa6.sin6_addr))
> +		v6only = 1;
> +
> +	return sock_l4_(c, type, sa, ifname, v6only, data);
>  }
>  
>  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 epoll_type type,
>  		.sa6.sin6_port = htons(port),
>  	};
>  
> +	/* Dual stack sockets require IPV6_V6ONLY == 0.  Usually that's the
> +	 * default, but sysctl net.ipv6.bindv6only can change that, so set the
> +	 * sockopt explicitly.
> +	 */
>  	return sock_l4_(c, type, &sa, ifname, 0, data);
>  }
>  

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.

Is it because it drops some code?

-- 
Stefano