Re: QEMU Sockets Networking Backend Multicast Networking Fix

[Jason Wang <jasowang@redhat.com>]

On 2020/2/15 下午6:39, Markus Armbruster wrote:
> Jason, please have a look.
>
> Faisal Al-Humaimidi <falhumai96@gmail.com> writes:
>
>> Hello QEMU developers,
>>
>> I have noticed a bug in the `mcast` option of the `socket` networking
>> backend, where I simply cannot join a multicast group (tested in Windows 10
>> with QEMU 4.2.0 release). I have found a fix to the problem. The problem
>> was mainly due to the fact that QEMU was binding to the multicast address,
>> and not the local address or the default INADDR_ANY (0.0.0.0) if no local
>> address is used.
>>
>> Here's the patch text (as well as attached with this email), that outlines
>> my fix:
>>
>> ```
>> diff -uarN qemu-4.2.0.original/net/socket.c qemu-4.2.0.modified/net/socket.c
>> --- qemu-4.2.0.original/net/socket.c 2019-12-12 10:20:48.000000000 -0800
>> +++ qemu-4.2.0.modified/net/socket.c 2020-02-14 10:30:16.395973453 -0800
>> @@ -253,6 +253,15 @@
>>           goto fail;
>>       }
>>
>> +    /* Preserve the multicast address, and bind to a non-multicast group
>> (e.g. a
>> +     * local address).
>> +     */
>> +    struct in_addr group_addr = mcastaddr->sin_addr;
>> +    if (localaddr) {
>> +        mcastaddr->sin_addr = *localaddr;
>> +    } else {
>> +        mcastaddr->sin_addr.s_addr = htonl(INADDR_ANY);
>> +    }
>>       ret = bind(fd, (struct sockaddr *)mcastaddr, sizeof(*mcastaddr));


This looks wrong, AFAIK the local address should be added through 
IP_ADD_MEMBERSHIP which is already handled in this function I believe.

Thanks


>>       if (ret < 0) {
>>           error_setg_errno(errp, errno, "can't bind ip=%s to socket",
>> @@ -260,7 +269,10 @@
>>           goto fail;
>>       }
>>
>> -    /* Add host to multicast group */
>> +    /* Restore the multicast address. */
>> +    mcastaddr->sin_addr = group_addr;
>> +
>> +    /* Add host to multicast group. */
>>       imr.imr_multiaddr = mcastaddr->sin_addr;
>>       if (localaddr) {
>>           imr.imr_interface = *localaddr;
>> @@ -277,7 +289,7 @@
>>           goto fail;
>>       }
>>
>> -    /* Force mcast msgs to loopback (eg. several QEMUs in same host */
>> +    /* Force mcast msgs to loopback (eg. several QEMUs in same host). */
>>       loop = 1;
>>       ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP,
>>                             &loop, sizeof(loop));
>> @@ -287,7 +299,7 @@
>>           goto fail;
>>       }
>>
>> -    /* If a bind address is given, only send packets from that address */
>> +    /* If a bind address is given, only send packets from that address. */
>>       if (localaddr != NULL) {
>>           ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF,
>>                                 localaddr, sizeof(*localaddr));
>> ```
>>
>> Regards,
>> Faisal Al-Humaimidi
>