* 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