Re: [RFC v6] virtio/vsock: add two more queues for datagram types

[Stefano Garzarella]

On Thu, Sep 16, 2021 at 08:26:15AM +0200, Stefano Garzarella wrote:
> On Wed, Sep 15, 2021 at 08:59:17PM -0700, Jiang Wang . wrote:
> > On Tue, Sep 14, 2021 at 5:46 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > > On Mon, Sep 13, 2021 at 10:18:43PM +0000, Jiang Wang wrote:
> > > > Datagram sockets are connectionless and unreliable.
> > > > The sender does not know the capacity of the receiver
> > > > and may send more packets than the receiver can handle.
> > > >
> > > > Add two more dedicate virtqueues for datagram sockets,
> > > > so that it will not unfairly steal resources from
> > > > stream and future connection-oriented sockets.
> > > >
> > > > Signed-off-by: Jiang Wang <jiang.wang@bytedance.com>
> > > > ---
> > > > v1 -> v2: use qemu cmd option to control number of queues,
> > > >         removed configuration settings for dgram.
> > > > v2 -> v3: use ioctl to get features and decide number of
> > > >         virt queues, instead of qemu cmd option.
> > > > v3 -> v4: change DGRAM feature bit value to 2. Add an argument
> > > >         in vhost_vsock_common_realize to indicate dgram is supported or not.
> > > > v4 -> v5: don't open dev to get vhostfd. Removed leftover definition of
> > > >         enable_dgram
> > > > v5 -> v6: fix style errors. Imporve error handling of
> > > >         vhost_vsock_dgram_supported. Rename MAX_VQS_WITH_DGRAM and another one.
> > > >
> > > >  hw/virtio/vhost-user-vsock.c                  |  2 +-
> > > >  hw/virtio/vhost-vsock-common.c                | 25 ++++++++++++--
> > > >  hw/virtio/vhost-vsock.c                       | 34 ++++++++++++++++++-
> > > >  include/hw/virtio/vhost-vsock-common.h        |  6 ++--
> > > >  include/hw/virtio/vhost-vsock.h               |  3 ++
> > > >  include/standard-headers/linux/virtio_vsock.h |  1 +
> > > >  6 files changed, 64 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/hw/virtio/vhost-user-vsock.c b/hw/virtio/vhost-user-vsock.c
> > > > index 6095ed7349..e9ec0e1c00 100644
> > > > --- a/hw/virtio/vhost-user-vsock.c
> > > > +++ b/hw/virtio/vhost-user-vsock.c
> > > > @@ -105,7 +105,7 @@ static void vuv_device_realize(DeviceState *dev, Error 
> > > > **errp)
> > > >          return;
> > > >      }
> > > >
> > > > -    vhost_vsock_common_realize(vdev, "vhost-user-vsock");
> > > > +    vhost_vsock_common_realize(vdev, "vhost-user-vsock", false);
> > >
> > > VIRTIO_VSOCK_F_DGRAM support should work equally well for
> > > vhost-user-vsock. I don't think there is a need to disable it here.
> > Stefano mentioned in previous email ( V3 ) that I can disable dgram
> > for vhost-user-vsock for now. I think we need some extra changes to
> > fully support vhost-vsock-user, like feature discovery?
>
> I think Stefan is suggesting something similar of what we discussed 
> here:
> https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg06689.html
>
> We can allocate all the queues, and choose at runtime which queue to 
> use for events, that is the only queue used by QEMU.
>
> We can check that in vhost_vsock_common_start(), just before starting 
> the device, where we know the features acked by the guest 
> (vdev->guest_features).
>
> And I agree that would be the best approach, since we don't need 
> discovery anymore, and the same code works also for vhost-user-vsock 
> as is.

In second thought, I think we really need to know whether the guest has 
acked the feature or not.

Otherwise if QEMU and host kernel support dgram, but guest kernel 
doesn't, QEMU will use fifth queue instead of third for events, but 
guest doesn't support it.

Thanks,
Stefano