Hello Jason,

But, the local address is not meant to be added to the group, rather we
listen to it, hence we bind to the local address. The multicast group is a
higher layer that would be requested to join to by the listening host.
Here's a similar example in multicasting that demonstrates this idea in
Python: https://pymotw.com/2/socket/multicast.html.


Regards,
Faisal Al-Humaimidi

On Mon., Feb. 17, 2020, 1:54 a.m. Jason Wang, <jasowang@redhat.com> wrote:

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