Re: [PATCH] vhost-vdpa: fix assert !virtio_net_get_subqueue(nc)->async_tx.elem in virtio_net_reset

[Si-Wei Liu]

    Hi Jason,
    
    This one is a one-line simple bug fix but seems to be missed from
    the pull request. If there's a v2 for the PULL, would appreciate if
    you can piggyback. Thanks in advance!
    
    Regards,
    -Siwei

On 10/7/2022 8:42 AM, Eugenio Perez Martin wrote:

> On Tue, Oct 4, 2022 at 11:05 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
> > The citing commit has incorrect code in vhost_vdpa_receive() that returns
> > zero instead of full packet size to the caller. This renders pending packets
> > unable to be freed so then get clogged in the tx queue forever. When device
> > is being reset later on, below assertion failure ensues:
> >
> > 0  0x00007f86d53bb387 in raise () from /lib64/libc.so.6
> > 1  0x00007f86d53bca78 in abort () from /lib64/libc.so.6
> > 2  0x00007f86d53b41a6 in __assert_fail_base () from /lib64/libc.so.6
> > 3  0x00007f86d53b4252 in __assert_fail () from /lib64/libc.so.6
> > 4  0x000055b8f6ff6fcc in virtio_net_reset (vdev=<optimized out>) at /usr/src/debug/qemu/hw/net/virtio-net.c:563
> > 5  0x000055b8f7012fcf in virtio_reset (opaque=0x55b8faf881f0) at /usr/src/debug/qemu/hw/virtio/virtio.c:1993
> > 6  0x000055b8f71f0086 in virtio_bus_reset (bus=bus@entry=0x55b8faf88178) at /usr/src/debug/qemu/hw/virtio/virtio-bus.c:102
> > 7  0x000055b8f71f1620 in virtio_pci_reset (qdev=<optimized out>) at /usr/src/debug/qemu/hw/virtio/virtio-pci.c:1845
> > 8  0x000055b8f6fafc6c in memory_region_write_accessor (mr=<optimized out>, addr=<optimized out>, value=<optimized out>,
> >    size=<optimized out>, shift=<optimized out>, mask=<optimized out>, attrs=...) at /usr/src/debug/qemu/memory.c:483
> > 9  0x000055b8f6fadce9 in access_with_adjusted_size (addr=addr@entry=20, value=value@entry=0x7f867e7fb7e8, size=size@entry=1,
> >    access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x55b8f6fafc20 <memory_region_write_accessor>,
> >    mr=0x55b8faf80a50, attrs=...) at /usr/src/debug/qemu/memory.c:544
> > 10 0x000055b8f6fb1d0b in memory_region_dispatch_write (mr=mr@entry=0x55b8faf80a50, addr=addr@entry=20, data="" op=<optimized out>,
> >    attrs=attrs@entry=...) at /usr/src/debug/qemu/memory.c:1470
> > 11 0x000055b8f6f62ada in flatview_write_continue (fv=fv@entry=0x7f86ac04cd20, addr=addr@entry=549755813908, attrs=...,
> >    attrs@entry=..., buf=buf@entry=0x7f86d0223028 <Address 0x7f86d0223028 out of bounds>, len=len@entry=1, addr1=20, l=1,
> >    mr=0x55b8faf80a50) at /usr/src/debug/qemu/exec.c:3266
> > 12 0x000055b8f6f62c8f in flatview_write (fv=0x7f86ac04cd20, addr=549755813908, attrs=...,
> >    buf=0x7f86d0223028 <Address 0x7f86d0223028 out of bounds>, len=1) at /usr/src/debug/qemu/exec.c:3306
> > 13 0x000055b8f6f674cb in address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=<optimized out>,
> >    len=<optimized out>) at /usr/src/debug/qemu/exec.c:3396
> > 14 0x000055b8f6f67575 in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., attrs@entry=...,
> >    buf=buf@entry=0x7f86d0223028 <Address 0x7f86d0223028 out of bounds>, len=<optimized out>, is_write=<optimized out>)
> >    at /usr/src/debug/qemu/exec.c:3406
> > 15 0x000055b8f6fc1cc8 in kvm_cpu_exec (cpu=cpu@entry=0x55b8f9aa0e10) at /usr/src/debug/qemu/accel/kvm/kvm-all.c:2410
> > 16 0x000055b8f6fa5f5e in qemu_kvm_cpu_thread_fn (arg=0x55b8f9aa0e10) at /usr/src/debug/qemu/cpus.c:1318
> > 17 0x000055b8f7336e16 in qemu_thread_start (args=0x55b8f9ac8480) at /usr/src/debug/qemu/util/qemu-thread-posix.c:519
> > 18 0x00007f86d575aea5 in start_thread () from /lib64/libpthread.so.0
> > 19 0x00007f86d5483b2d in clone () from /lib64/libc.so.6
> >
> > Make vhost_vdpa_receive() return the size passed in as is, so that the
> > caller qemu_deliver_packet_iov() would eventually propagate it back to
> > virtio_net_flush_tx() to release pending packets from the async_tx queue.
> > Which corresponds to the drop path where qemu_sendv_packet_async() returns
> > non-zero in virtio_net_flush_tx().
> >
> >       
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
>
>
> > Fixes: 846a1e85da64 ("vdpa: Add dummy receive callback")
> > Cc: Eugenio Perez Martin <eperezma@redhat.com>
> > Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> > ---
> >  net/vhost-vdpa.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > index 4bc3fd0..182b3a1 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -211,7 +211,7 @@ static bool vhost_vdpa_check_peer_type(NetClientState *nc, ObjectClass *oc,
> >  static ssize_t vhost_vdpa_receive(NetClientState *nc, const uint8_t *buf,
> >                                    size_t size)
> >  {
> > -    return 0;
> > +    return size;
> >  }
> >
> >  static NetClientInfo net_vhost_vdpa_info = {
> > --
> > 1.8.3.1
> >
> >       
>