* QEMU Sockets Networking Backend Multicast Networking Fix
@ 2020-02-14 18:51 Faisal Al-Humaimidi
2020-02-15 10:39 ` Markus Armbruster
0 siblings, 1 reply; 5+ messages in thread
From: Faisal Al-Humaimidi @ 2020-02-14 18:51 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 2272 bytes --]
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));
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
[-- Attachment #1.2: Type: text/html, Size: 2754 bytes --]
[-- Attachment #2: qemu-4.2.0-fix_socket_mcast.patch --]
[-- Type: application/octet-stream, Size: 1745 bytes --]
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));
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));
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: QEMU Sockets Networking Backend Multicast Networking Fix
2020-02-14 18:51 QEMU Sockets Networking Backend Multicast Networking Fix Faisal Al-Humaimidi
@ 2020-02-15 10:39 ` Markus Armbruster
2020-02-17 9:53 ` Jason Wang
0 siblings, 1 reply; 5+ messages in thread
From: Markus Armbruster @ 2020-02-15 10:39 UTC (permalink / raw)
To: Faisal Al-Humaimidi; +Cc: Jason Wang, qemu-devel
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));
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: QEMU Sockets Networking Backend Multicast Networking Fix
2020-02-15 10:39 ` Markus Armbruster
@ 2020-02-17 9:53 ` Jason Wang
2020-02-17 10:05 ` Faisal Al-Humaimidi
0 siblings, 1 reply; 5+ messages in thread
From: Jason Wang @ 2020-02-17 9:53 UTC (permalink / raw)
To: Markus Armbruster, Faisal Al-Humaimidi; +Cc: qemu-devel
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: QEMU Sockets Networking Backend Multicast Networking Fix
2020-02-17 9:53 ` Jason Wang
@ 2020-02-17 10:05 ` Faisal Al-Humaimidi
2020-02-19 5:22 ` Jason Wang
0 siblings, 1 reply; 5+ messages in thread
From: Faisal Al-Humaimidi @ 2020-02-17 10:05 UTC (permalink / raw)
To: Jason Wang; +Cc: Markus Armbruster, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3524 bytes --]
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
> >
>
>
[-- Attachment #2: Type: text/html, Size: 4837 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: QEMU Sockets Networking Backend Multicast Networking Fix
2020-02-17 10:05 ` Faisal Al-Humaimidi
@ 2020-02-19 5:22 ` Jason Wang
0 siblings, 0 replies; 5+ messages in thread
From: Jason Wang @ 2020-02-19 5:22 UTC (permalink / raw)
To: Faisal Al-Humaimidi; +Cc: Markus Armbruster, qemu-devel
On 2020/2/17 下午6:05, Faisal Al-Humaimidi wrote:
> 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.
Well, I think it kinds of violates the multicast overlay here. It allows
to receive any other traffic (unitcast) that may be received for the
port which is not what we want here.
Thanks
>
>
> Regards,
> Faisal Al-Humaimidi
>
> On Mon., Feb. 17, 2020, 1:54 a.m. Jason Wang, <jasowang@redhat.com
> <mailto:jasowang@redhat.com>> wrote:
>
>
> On 2020/2/15 下午6:39, Markus Armbruster wrote:
> > Jason, please have a look.
> >
> > Faisal Al-Humaimidi <falhumai96@gmail.com
> <mailto: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
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-19 5:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 18:51 QEMU Sockets Networking Backend Multicast Networking Fix Faisal Al-Humaimidi
2020-02-15 10:39 ` Markus Armbruster
2020-02-17 9:53 ` Jason Wang
2020-02-17 10:05 ` Faisal Al-Humaimidi
2020-02-19 5:22 ` Jason Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).