Re: [PATCH] net: fix multicast support with BSD (macOS) socket implementations

[Daniel P . Berrangé]

On Mon, May 02, 2022 at 03:38:30AM +0300, Vitaly Cheptsov wrote:
> This patch fixes socket communication with QEMU -> host on macOS,
> which was originally impossible due to QEMU and host program
> having to bind to the same ip/port in a way not supported by BSD
> sockets. The change was tested on both Linux and macOS.
> 
> As per BSD manual pages SO_REUSEPORT allows completely duplicate
> bindings by multiple processes, permitting multiple instances of
> a program to each receive UDP/IP multicast datagrams destined
> for the bound port. Without this option macOS, unlike Linux,
> which (ab)uses SO_REUSEADDR for this purpose, will return
> "Address already in use" on bind().


When looking in Google there's a comprehensive looking
description of SO_REUSEADDR + SO_REUSEPORT across all the
different OS which insists that SO_REUSEPORT and SO_REUSEADDR
are functionally equivalent for multicast IP addresses:

  
https://stackoverflow.com/questions/14388706/how-do-so-reuseaddr-and-so-reuseport-differ

And AFAIK, macOS should behave the same way, which suggests
this patch is not needed.

Oddly though, I don't find this in the FreeBSD man page - its
description seems fairly clear that SO_REUSEPORT is needed for
multicast

[quote]
     SO_REUSEPORT allows completely duplicate bindings by multiple processes
     if they all set SO_REUSEPORT before binding the port.  This option
     permits multiple instances of a program to each receive UDP/IP multicast
     or broadcast datagrams destined for the bound port.
[/quote]


> 
> As per BSD manual pages binding to any address, even one not bound
> to any available network interface in the system, should be
> IP_BINDANY. Without binding to INADDR_ANY macOS will return
> "Can't assign requested address" on send().

I didn't find a quote about this in the FreeBSD man pages I looked
at, and it feels dubious to me. If the user gives QEMU a address to
bind to, we should surely be honouring that, not changing it to
INADDR_ANY.

If using INADDR_ANY though, thsi could explain the need for
SO_REUSEPORT, since INADDR_ANY is not a designated mcast address.

> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Vitaly Cheptsov <cheptsov@ispras.ru>
> ---
>  net/socket.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/net/socket.c b/net/socket.c
> index ea5220a2eb..8b2c6c4bb8 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -252,10 +252,24 @@ static int net_socket_mcast_create(struct sockaddr_in 
> *mcastaddr,
>          goto fail;
>      }
>  
> -    ret = bind(fd, (struct sockaddr *)mcastaddr, sizeof(*mcastaddr));
> +    val = 1;
> +    ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
> +    if (ret < 0) {
> +        error_setg_errno(errp, errno,
> +                         "can't set socket option SO_REUSEPORT");
> +        goto fail;
> +    }

AFAIK, this likely won't compile on Windows since it lacks SO_REUSEPORT

> +
> +    struct sockaddr_in bindaddr;
> +    memset(&bindaddr, 0, sizeof(bindaddr));
> +    bindaddr.sin_family = AF_INET;
> +    bindaddr.sin_addr.s_addr = htonl(INADDR_ANY);
> +    bindaddr.sin_port = mcastaddr->sin_port;
> +    ret = bind(fd, (struct sockaddr *)&bindaddr, sizeof(bindaddr));
> +
>      if (ret < 0) {
>          error_setg_errno(errp, errno, "can't bind ip=%s to socket",
> -                         inet_ntoa(mcastaddr->sin_addr));
> +                         inet_ntoa(bindaddr.sin_addr));
>          goto fail;
>      }


With 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 :|