[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] vhost: Fix last queue index of devices with no cvq
From: |
Jason Wang |
Subject: |
Re: [PATCH v2 1/1] vhost: Fix last queue index of devices with no cvq |
Date: |
Wed, 3 Nov 2021 10:50:31 +0800 |
On Tue, Nov 2, 2021 at 7:41 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> The -1 assumes that all devices with no cvq have an spare vq allocated
> for them, but with no offer of VIRTIO_NET_F_CTRL_VQ. This is an invalid
> device by the standard, so just stick to the right number of device
> models.
>
> This is not a problem to vhost-net, but it is to vhost-vdpa, which
> device model trust to reach the last index to finish starting the
> device.
>
> Tested with vp_vdpa with host's vhost=on and vhost=off.
>
> Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the virtio
> device")
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
> hw/net/vhost_net.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 0d888f29a6..a859cc943d 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -329,10 +329,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState
> *ncs,
> int r, e, i, last_index = data_queue_pairs * 2;
> NetClientState *peer;
>
> - if (!cvq) {
> - last_index -= 1;
> - }
> -
So I think the math is wrong at least from the perspective of virtio:
If we had a device with 1 queue pair without cvq, last_index is 2 but
should be 1.
Another thing is that it may break the device with cvq. If we have a
device with 1 queue pair + cvq, last_index is 2.
We will start the device before cvq vhost_net is initialized. Since
for the first vhost_net device (first queue pair) we meet the:
dev->vq_index + dev->nvqs == dev->last_index (0 + 2 == 2).
Then we set DRIVER_OK before initializing cvq.
Thanks
> if (!k->set_guest_notifiers) {
> error_report("binding does not support guest notifiers");
> return -ENOSYS;
> --
> 2.27.0
>