Re: [Qemu-devel] [Qemu-stable] [PATCH V2] net: fix qemu_announce_self not emitting packets

[Peter Lieven <pl@kamp.de>]

Am 08.06.2016 um 12:23 schrieb Paolo Bonzini:
>
> ----- Original Message -----
>> From: "Jason Wang" <jasowang@redhat.com>
>> To: "Peter Lieven" <pl@kamp.de>, qemu-devel@nongnu.org
>> Cc: pbonzini@redhat.com, qemu-stable@nongnu.org, "hongyang yang" <hongyang.yang@easystack.cn>
>> Sent: Wednesday, June 8, 2016 9:54:53 AM
>> Subject: Re: [Qemu-devel] [Qemu-stable] [PATCH V2] net: fix qemu_announce_self not emitting packets
>>
>>
>>
>> On 2016年06月08日 15:13, Peter Lieven wrote:
>>> Am 08.06.2016 um 08:39 schrieb Jason Wang:
>>>> On 2016年06月07日 21:00, 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: hongyang.yang@easystack.cn
>>>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>>>> ---
>>>>> v1->v2: assert that only raw packets with a plain buffer come in. [Paolo]
>>>>>
>>>>>    net/net.c | 10 +++++++++-
>>>>>    1 file changed, 9 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/net/net.c b/net/net.c
>>>>> index 5f3e5a9..5e1b5fa 100644
>>>>> --- a/net/net.c
>>>>> +++ b/net/net.c
>>>>> @@ -714,6 +714,11 @@ ssize_t qemu_deliver_packet_iov(NetClientState
>>>>> *sender,
>>>>>        NetClientState *nc = opaque;
>>>>>        int ret;
>>>>>    +    /* we currently only support sending raw packets via
>>>>>    qemu_send_packet_raw.
>>>>> +     * If we want generic raw iov support we need to implement something
>>>>> like
>>>>> +     * .receive_raw_iov in NetClientInfo first. */
>>>>> +    assert(!(flags & QEMU_NET_PACKET_FLAG_RAW) || iovcnt == 1);
>>>>> +
>>>>>        if (nc->link_down) {
>>>>>            return iov_size(iov, iovcnt);
>>>>>        }
>>>>> @@ -722,7 +727,10 @@ ssize_t qemu_deliver_packet_iov(NetClientState
>>>>> *sender,
>>>>>            return 0;
>>>>>        }
>>>>>    -    if (nc->info->receive_iov) {
>>>>> +    if (flags & QEMU_NET_PACKET_FLAG_RAW && 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);
>>>> We still have raw packet support in nv_sendv_compat, can we use it? (and
>>>> then there's no need for the assert above).
>>> Okay, so Version 1 of the patch is more appropiate. I would either use V1
>>> or revert fefe2a78. The issue is quite serious
>>> for live migration.
> Or you can do even simpler:
>
>     if (nc->info->receive_iov && !(flags & QEMU_NET_PACKET_FLAG_RAW)))
>         nc->info->receive_iov
>     else
>         nc_sendv_compat

I should have looked into this nc_sendv_compat function before ;-)
This indeed is the best solution I think.

Will send a v3 tomorrow.

Peter