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

[Paolo Bonzini <pbonzini@redhat.com>]

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

Paolo

> >
> > Peter
> >
> >
> 
> V1 checks iovcnt and call receive_raw with hard coded first iov. Since
> we have raw packet support in nc_sendv_compat(), I mean when we detect
> QEMU_NET_PACKET_FLAG_RAW, call nc_sendv_compat() here.
>