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

[Vitaly Cheptsov]

Gentle ping :)

> On 3 May 2022, at 19:10, Vitaly Cheptsov <cheptsov@ispras.ru> wrote:
> 
> Hi Daniel,
> 
> Thank you for your comment. Socket implementation on all the systems is 
> rather complicated, and while I am fine to update the patch with better 
> reasoning, it needs to work on macOS. Given the situation with Windows, I 
> think we may want to ifdef the change to be macOS-specific.
> 
> Other than that, perhaps, we can come to something better if you give some 
> ideas what should we try. So far this was the only working combination, 
> however. If you do not have the hardware, I am can perform these tests for 
> you.
> 
> To simplify the situation, I attached two minimal python scripts, which 
> closely resemble QEMU actions, and allow testing bidirectional multicast 
> sockets.
> 
>> 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]
> 
> I also saw this description, and it is possible it is a little outdated. I 
> can safely (entirely) drop setting SO_REUSEADDR on macOS, but it will not 
> work without socket.SO_REUSEPORT. This can be easily proven with a.py/b.py.
> 
>> 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.
> 
> I made this judgement from the following part of ip(4):
> 
>> If the IP_BINDANY option is enabled on a SOCK_STREAM, SOCK_DGRAM or a
>> SOCK_RAW socket, one can bind(2) to any address, even one not bound to
>> any available network interface in the system.
> 
> 
> This makes some sense, because if I change a.py and b.py
> - to bind to MCAST_GRP instead of “0.0.0.0”
> - to not set SO_REUSEPORT
> 
> I get "Can't assign requested address” error at sendto in b.py. Same thing 
> happens in QEMU.
> 
> Best regards,
> Vitaly
> 
> 
> # a.py
> import socket
> import struct
> import scapy.all as scapy
> 
> MCAST_GRP = '230.0.0.1'
> MCAST_PORT = 1234
> MULTICAST_TTL = 2
> MAX_PACKET_SIZE = 65535
> 
> sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
> sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
> sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
> sock.bind(("0.0.0.0", MCAST_PORT))
> mreq = struct.pack('4sL', socket.inet_aton(MCAST_GRP), socket.INADDR_ANY)
> sock.setsockopt(socket.IPPROTO_IP, socket.IP_ADD_MEMBERSHIP, mreq)
> 
> data, _ = sock.recvfrom(MAX_PACKET_SIZE)
> print(data)
> sock.sendto(b"message_from_a", (MCAST_GRP, MCAST_PORT))
> 
> # b.py
> import socket
> import struct
> import scapy.all as scapy
> 
> MCAST_GRP = '230.0.0.1'
> MCAST_PORT = 1234
> MULTICAST_TTL = 2
> MAX_PACKET_SIZE = 65535
> 
> sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
> sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
> sock.bind(("0.0.0.0", MCAST_PORT))
> mreq = struct.pack('4sL', socket.inet_aton(MCAST_GRP), socket.INADDR_ANY)
> sock.setsockopt(socket.IPPROTO_IP, socket.IP_ADD_MEMBERSHIP, mreq)
> 
> sock.sendto(b"message_from_b", (MCAST_GRP, MCAST_PORT))
> 
> data, _ = sock.recvfrom(MAX_PACKET_SIZE)
> print(data)
> 
> data, _ = sock.recvfrom(MAX_PACKET_SIZE)
> print(data)
> 
> % python3 ./a.py
> WARNING: No IPv4 address found on en2 !
> WARNING: No IPv4 address found on en1 !
> WARNING: more No IPv4 address found on p2p0 !
> b'message_from_b'
> 
> % python3 ./b.py
> WARNING: No IPv4 address found on en2 !
> WARNING: No IPv4 address found on en1 !
> WARNING: more No IPv4 address found on p2p0 !
> b'message_from_b'
> b'message_from_a'
> 
>> On 3 May 2022, at 16:13, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> 
>> 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 :|
>

signature.asc
Description: Message signed with OpenPGP