Re: [Qemu-devel] [PATCH v2 1/5] sockets: ensure we can bind to both ipv4 & ipv6 separately

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2643 bytes --]

On 05/19/2017 01:03 PM, Daniel P. Berrange wrote:
> When binding to an IPv6 socket we currently force the
> IPV6_V6ONLY flag to off. This means that the IPv6 socket
> will accept both IPv4 & IPv6 sockets when QEMU is launched
> with something like
> 
>   -vnc :::1
> 
> While this is good for that case, it is bad for other
> cases. For example if an empty hostname is given,
> getaddrinfo resolves it to 2 addresses 0.0.0.0 and ::,
> in that order. We will thus bind to 0.0.0.0 first, and
> then fail to bind to :: on the same port. The same
> problem can happen if any other hostname lookup causes
> the IPv4 address to be reported before the IPv6 address.
> 
> When we get an IPv6 bind failure, we should re-try the
> same port, but with IPV6_V6ONLY turned on again, to
> avoid clash with any IPv4 listener.
> 
> This ensures that
> 
>   -vnc :1
> 
> will bind successfully to both 0.0.0.0 and ::, and also
> avoid
> 
>   -vnc :1,to=2
> 
> from mistakenly using a 2nd port for the :: listener.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  util/qemu-sockets.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 

>          for (p = port_min; p <= port_max; p++) {
> +#ifdef IPV6_V6ONLY
> +            /* listen on both ipv4 and ipv6 */
> +            int v6only = 0;
> +#endif
>              inet_setport(e, p);
> +#ifdef IPV6_V6ONLY
> +        rebind:
> +            if (e->ai_family == PF_INET6) {

The rebind: label could go here with no change in semantics and one less
conditional when compiled without optimization. But hopefully the
compiler is smart enough to see that under -O2, so I don't see a reason
for you to change the code.

> +                qemu_setsockopt(slisten, IPPROTO_IPV6, IPV6_V6ONLY, &v6only,
> +                                sizeof(v6only));
> +            }
> +#endif
>              if (bind(slisten, e->ai_addr, e->ai_addrlen) == 0) {
>                  goto listen;
>              }
> +
> +#ifdef IPV6_V6ONLY
> +            /* If we got EADDRINUSE from an IPv6 bind & V6ONLY is unset,
> +             * it could be that the IPv4 port is already claimed, so retry
> +             * with V6ONLY set
> +             */
> +            if (e->ai_family == PF_INET6 && errno == EADDRINUSE && !v6only) {
> +                v6only = 1;
> +                goto rebind;
> +            }
> +#endif
> +

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]