Re: [PATCH] vhost: Fix last queue index of devices with no cvq

[Eugenio Perez Martin]

On Mon, Nov 1, 2021 at 4:34 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Oct 29, 2021 at 10:16 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 may not be the
> > case, and the device may have a pair number of queues.
> >
> > To fix this, just resort to the lower even number of queues.
> >
> > Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the virtio 
> > device")
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  hw/net/vhost_net.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 0d888f29a6..edf56a597f 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -330,7 +330,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> > *ncs,
> >      NetClientState *peer;
> >
> >      if (!cvq) {
> > -        last_index -= 1;
> > +        last_index &= ~1ULL;
> >      }
>
> The math here looks correct but we need to fix vhost_vdpa_dev_start() instead?
>
> if (dev->vq_index + dev->nvqs - 1 != dev->last_index) {
> ...
> }
>

If we just do that, devices that offer an odd number of queues but do
not offer ctrl vq would never enable the last vq pair, isn't it?

Also, I would say that the right place for the solution of this
problem should not be virtio/vhost-vdpa: This is highly dependent on
having cvq, and this implies a knowledge about the use of each
virtqueue. Another kind of device could have an odd number of
virtqueues naturally, and that (-1) would not work for them, isn't it?

Thanks!

> Thanks
>
> >
> >      if (!k->set_guest_notifiers) {
> > --
> > 2.27.0
> >
>