From: "Daniel P. Berrange" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: Gerd Hoffmann <kraxel@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] sockets: ensure we can bind to both ipv4 & ipv6 separately
Date: Tue, 16 May 2017 13:39:51 +0100 [thread overview]
Message-ID: <20170516123951.GB16341@redhat.com> (raw)
In-Reply-To: <20170428121553.22408-1-berrange@redhat.com>
On Fri, Apr 28, 2017 at 01:15:53PM +0100, 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 | 30 ++++++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 8 deletions(-)
Ping Paolo or Gerd - any comments on this patch ?
Separately from this patch, I noticed one further possible
problem,
Currently, the ipv4=on|off/ipv6=on|off settings are only
used to determine what getaddrinfo results we request/use.
This leads to the somewhat odd situation where if you set
ipv4=off,ipv6=on, QEMU won't be listening on an IPv4
socket, but *will still* accept IPv4 clients over the IPv6
socket due to our use of IPV6_V6ONLY=off.
So I'm thinking that when ipv4=off, we should in fact
set IPV6_V6ONLY=on. My fear though is that this is a
semantic change that could cause regression. ie someone
might be accidentally relying on fact that ipv4=off,ipv6=on
still lets IPv4 clients connection, despite it being a
somewhat nonsensical scenario
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 8188d9a..75d1e0f 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -207,22 +207,36 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
> }
>
> socket_set_fast_reuse(slisten);
> -#ifdef IPV6_V6ONLY
> - if (e->ai_family == PF_INET6) {
> - /* listen on both ipv4 and ipv6 */
> - const int off = 0;
> - qemu_setsockopt(slisten, IPPROTO_IPV6, IPV6_V6ONLY, &off,
> - sizeof(off));
> - }
> -#endif
>
> port_min = inet_getport(e);
> port_max = saddr->has_to ? saddr->to + port_offset : port_min;
> for (p = port_min; p <= port_max; p++) {
> inet_setport(e, p);
> +#ifdef IPV6_V6ONLY
> + if (e->ai_family == PF_INET6) {
> + /* listen on both ipv4 and ipv6 */
> + const int off = 0;
> + qemu_setsockopt(slisten, IPPROTO_IPV6, IPV6_V6ONLY, &off,
> + sizeof(off));
> + }
> +#endif
> if (bind(slisten, e->ai_addr, e->ai_addrlen) == 0) {
> goto listen;
> }
> +
> +#ifdef IPV6_V6ONLY
> + if (e->ai_family == PF_INET6 && errno == EADDRINUSE) {
> + /* listen on only ipv6 */
> + const int on = 1;
> + qemu_setsockopt(slisten, IPPROTO_IPV6, IPV6_V6ONLY, &on,
> + sizeof(on));
> +
> + if (bind(slisten, e->ai_addr, e->ai_addrlen) == 0) {
> + goto listen;
> + }
> + }
> +#endif
> +
> if (p == port_max) {
> if (!e->ai_next) {
> error_setg_errno(errp, errno, "Failed to bind socket");
> --
> 2.9.3
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
next prev parent reply other threads:[~2017-05-16 12:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-28 12:15 [Qemu-devel] [PATCH] sockets: ensure we can bind to both ipv4 & ipv6 separately Daniel P. Berrange
2017-04-28 14:57 ` Eric Blake
2017-04-28 14:59 ` Daniel P. Berrange
2017-04-28 15:31 ` Markus Armbruster
2017-05-16 12:39 ` Daniel P. Berrange [this message]
2017-05-16 13:29 ` Gerd Hoffmann
2017-05-16 13:35 ` Daniel P. Berrange
2017-05-16 13:55 ` Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170516123951.GB16341@redhat.com \
--to=berrange@redhat.com \
--cc=kraxel@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.