qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets
@ 2016-06-07  7:37 Peter Lieven
  2016-06-07  7:55 ` Paolo Bonzini
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Peter Lieven @ 2016-06-07  7:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: jasowang, s.priebe, Peter Lieven, qemu-stable, yanghy

commit fefe2a78 accidently dropped the code path for injecting
raw packets. This feature is needed for sending gratuitous ARPs
after an incoming migration has completed. The result is increased
network downtime for vservers where the network card is not virtio-net
with the VIRTIO_NET_F_GUEST_ANNOUNCE feature.

Fixes: fefe2a78abde932e0f340b21bded2c86def1d242
Cc: qemu-stable@nongnu.org
Cc: yanghy@cn.fujitsu.com
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 net/net.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index 5f3e5a9..d5834ea 100644
--- a/net/net.c
+++ b/net/net.c
@@ -722,7 +722,11 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
         return 0;
     }
 
-    if (nc->info->receive_iov) {
+    if (flags & QEMU_NET_PACKET_FLAG_RAW && iovcnt == 1 &&
+        nc->info->receive_raw) {
+        /* this is required for qemu_announce_self() */
+        ret = nc->info->receive_raw(nc, iov[0].iov_base, iov[0].iov_len);
+    } else  if (nc->info->receive_iov) {
         ret = nc->info->receive_iov(nc, iov, iovcnt);
     } else {
         ret = nc_sendv_compat(nc, iov, iovcnt, flags);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets
  2016-06-07  7:37 [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets Peter Lieven
@ 2016-06-07  7:55 ` Paolo Bonzini
  2016-06-07  8:02   ` Peter Lieven
  2016-06-07  9:33 ` Stefan Priebe - Profihost AG
  2016-06-07 11:35 ` Yang Hongyang
  2 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2016-06-07  7:55 UTC (permalink / raw)
  To: Peter Lieven, qemu-devel; +Cc: jasowang, yanghy, qemu-stable, s.priebe



On 07/06/2016 09:37, Peter Lieven wrote:
> commit fefe2a78 accidently dropped the code path for injecting
> raw packets. This feature is needed for sending gratuitous ARPs
> after an incoming migration has completed. The result is increased
> network downtime for vservers where the network card is not virtio-net
> with the VIRTIO_NET_F_GUEST_ANNOUNCE feature.
> 
> Fixes: fefe2a78abde932e0f340b21bded2c86def1d242
> Cc: qemu-stable@nongnu.org
> Cc: yanghy@cn.fujitsu.com
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  net/net.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 5f3e5a9..d5834ea 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -722,7 +722,11 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
>          return 0;
>      }
>  
> -    if (nc->info->receive_iov) {
> +    if (flags & QEMU_NET_PACKET_FLAG_RAW && iovcnt == 1 &&
> +        nc->info->receive_raw) {
> +        /* this is required for qemu_announce_self() */
> +        ret = nc->info->receive_raw(nc, iov[0].iov_base, iov[0].iov_len);

Possibly move "iovcnt == 1" from the if condition to an assertion inside it?

Thanks,

Paolo

> +    } else  if (nc->info->receive_iov) {
>          ret = nc->info->receive_iov(nc, iov, iovcnt);
>      } else {
>          ret = nc_sendv_compat(nc, iov, iovcnt, flags);
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets
  2016-06-07  7:55 ` Paolo Bonzini
@ 2016-06-07  8:02   ` Peter Lieven
  2016-06-07  8:23     ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Lieven @ 2016-06-07  8:02 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: jasowang, yanghy, qemu-stable, s.priebe

Am 07.06.2016 um 09:55 schrieb Paolo Bonzini:
>
> On 07/06/2016 09:37, Peter Lieven wrote:
>> commit fefe2a78 accidently dropped the code path for injecting
>> raw packets. This feature is needed for sending gratuitous ARPs
>> after an incoming migration has completed. The result is increased
>> network downtime for vservers where the network card is not virtio-net
>> with the VIRTIO_NET_F_GUEST_ANNOUNCE feature.
>>
>> Fixes: fefe2a78abde932e0f340b21bded2c86def1d242
>> Cc: qemu-stable@nongnu.org
>> Cc: yanghy@cn.fujitsu.com
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>   net/net.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/net.c b/net/net.c
>> index 5f3e5a9..d5834ea 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -722,7 +722,11 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
>>           return 0;
>>       }
>>   
>> -    if (nc->info->receive_iov) {
>> +    if (flags & QEMU_NET_PACKET_FLAG_RAW && iovcnt == 1 &&
>> +        nc->info->receive_raw) {
>> +        /* this is required for qemu_announce_self() */
>> +        ret = nc->info->receive_raw(nc, iov[0].iov_base, iov[0].iov_len);
> Possibly move "iovcnt == 1" from the if condition to an assertion inside it?

I do not know if there is any other user that emits raw packets than qemu_announce_self.

The question is do you want to abort or just return -1 (which is the case without this patch).

Peter

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets
  2016-06-07  8:02   ` Peter Lieven
@ 2016-06-07  8:23     ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2016-06-07  8:23 UTC (permalink / raw)
  To: Peter Lieven, qemu-devel; +Cc: jasowang, yanghy, qemu-stable, s.priebe



On 07/06/2016 10:02, Peter Lieven wrote:
> Am 07.06.2016 um 09:55 schrieb Paolo Bonzini:
>>
>> On 07/06/2016 09:37, Peter Lieven wrote:
>>> commit fefe2a78 accidently dropped the code path for injecting
>>> raw packets. This feature is needed for sending gratuitous ARPs
>>> after an incoming migration has completed. The result is increased
>>> network downtime for vservers where the network card is not virtio-net
>>> with the VIRTIO_NET_F_GUEST_ANNOUNCE feature.
>>>
>>> Fixes: fefe2a78abde932e0f340b21bded2c86def1d242
>>> Cc: qemu-stable@nongnu.org
>>> Cc: yanghy@cn.fujitsu.com
>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>> ---
>>>   net/net.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 5f3e5a9..d5834ea 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -722,7 +722,11 @@ ssize_t qemu_deliver_packet_iov(NetClientState
>>> *sender,
>>>           return 0;
>>>       }
>>>   -    if (nc->info->receive_iov) {
>>> +    if (flags & QEMU_NET_PACKET_FLAG_RAW && iovcnt == 1 &&
>>> +        nc->info->receive_raw) {
>>> +        /* this is required for qemu_announce_self() */
>>> +        ret = nc->info->receive_raw(nc, iov[0].iov_base,
>>> iov[0].iov_len);
>> Possibly move "iovcnt == 1" from the if condition to an assertion
>> inside it?
> 
> I do not know if there is any other user that emits raw packets than
> qemu_announce_self.

Currently all senders of raw packets go through qemu_send_packet_raw,
which has iovcnt == 1.

> The question is do you want to abort or just return -1 (which is the
> case without this patch).

I want to abort, since it's currently impossible to get into that
situation.  If someone wants to introduce
qemu_sendv_packet_async_with_flags, they must also introduce
nc->info->receive_raw_iov or something like that.

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets
  2016-06-07  7:37 [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets Peter Lieven
  2016-06-07  7:55 ` Paolo Bonzini
@ 2016-06-07  9:33 ` Stefan Priebe - Profihost AG
  2016-06-07 11:35 ` Yang Hongyang
  2 siblings, 0 replies; 6+ messages in thread
From: Stefan Priebe - Profihost AG @ 2016-06-07  9:33 UTC (permalink / raw)
  To: Peter Lieven, qemu-devel; +Cc: jasowang, qemu-stable, yanghy


Am 07.06.2016 um 09:37 schrieb Peter Lieven:
> commit fefe2a78 accidently dropped the code path for injecting
> raw packets. This feature is needed for sending gratuitous ARPs
> after an incoming migration has completed. The result is increased
> network downtime for vservers where the network card is not virtio-net
> with the VIRTIO_NET_F_GUEST_ANNOUNCE feature.
> 
> Fixes: fefe2a78abde932e0f340b21bded2c86def1d242
> Cc: qemu-stable@nongnu.org
> Cc: yanghy@cn.fujitsu.com
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  net/net.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 5f3e5a9..d5834ea 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -722,7 +722,11 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
>          return 0;
>      }
>  
> -    if (nc->info->receive_iov) {
> +    if (flags & QEMU_NET_PACKET_FLAG_RAW && iovcnt == 1 &&
> +        nc->info->receive_raw) {
> +        /* this is required for qemu_announce_self() */
> +        ret = nc->info->receive_raw(nc, iov[0].iov_base, iov[0].iov_len);
> +    } else  if (nc->info->receive_iov) {
>          ret = nc->info->receive_iov(nc, iov, iovcnt);
>      } else {
>          ret = nc_sendv_compat(nc, iov, iovcnt, flags);
> 

Thanks for the patch. Sadly it does not fix our problem. So we still
have another Problem.

Thanks!

Greets,
Stefan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets
  2016-06-07  7:37 [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets Peter Lieven
  2016-06-07  7:55 ` Paolo Bonzini
  2016-06-07  9:33 ` Stefan Priebe - Profihost AG
@ 2016-06-07 11:35 ` Yang Hongyang
  2 siblings, 0 replies; 6+ messages in thread
From: Yang Hongyang @ 2016-06-07 11:35 UTC (permalink / raw)
  To: Peter Lieven; +Cc: qemu devel, Jason Wang, Yang Hongyang, qemu-stable, s.priebe

Hi Peter,

Sorry for the inconvenience and thank you for the patch.

On Tue, Jun 7, 2016 at 3:37 PM, Peter Lieven <pl@kamp.de> wrote:

> commit fefe2a78 accidently dropped the code path for injecting
> raw packets. This feature is needed for sending gratuitous ARPs
> after an incoming migration has completed. The result is increased
> network downtime for vservers where the network card is not virtio-net
> with the VIRTIO_NET_F_GUEST_ANNOUNCE feature.
>
> Fixes: fefe2a78abde932e0f340b21bded2c86def1d242
> Cc: qemu-stable@nongnu.org
> Cc: yanghy@cn.fujitsu.com
> Signed-off-by: Peter Lieven <pl@kamp.de>
>

After addressing Paolo's comments
Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>


> ---
>  net/net.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/net.c b/net/net.c
> index 5f3e5a9..d5834ea 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -722,7 +722,11 @@ ssize_t qemu_deliver_packet_iov(NetClientState
> *sender,
>          return 0;
>      }
>
> -    if (nc->info->receive_iov) {
> +    if (flags & QEMU_NET_PACKET_FLAG_RAW && iovcnt == 1 &&
> +        nc->info->receive_raw) {
> +        /* this is required for qemu_announce_self() */
> +        ret = nc->info->receive_raw(nc, iov[0].iov_base, iov[0].iov_len);
> +    } else  if (nc->info->receive_iov) {
>          ret = nc->info->receive_iov(nc, iov, iovcnt);
>      } else {
>          ret = nc_sendv_compat(nc, iov, iovcnt, flags);
> --
> 1.9.1
>
>
>


-- 
Thanks,
Yang

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-06-07 11:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-07  7:37 [Qemu-devel] [PATCH] net: fix qemu_announce_self not emitting packets Peter Lieven
2016-06-07  7:55 ` Paolo Bonzini
2016-06-07  8:02   ` Peter Lieven
2016-06-07  8:23     ` Paolo Bonzini
2016-06-07  9:33 ` Stefan Priebe - Profihost AG
2016-06-07 11:35 ` Yang Hongyang

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