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

[Jiang Wang .]

Hi Stefano,

I checked virtio_net_set_multiqueue(), which will help with following
changes in my patch:

#ifdef CONFIG_VHOST_VSOCK_DGRAM
vvc->dgram_recv_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
vhost_vsock_common_handle_output);
vvc->dgram_trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
vhost_vsock_common_handle_output);
#endif

But I think there is still an issue with the following lines, right?

#ifdef CONFIG_VHOST_VSOCK_DGRAM
struct vhost_virtqueue vhost_vqs[4];
#else
struct vhost_virtqueue vhost_vqs[2];
#endif

I think the problem with feature bits is that they are set and get after
vhost_vsock_common_realize() and after vhost_dev_init() in drivers/vhost/vsock.c
But those virtqueues need to be set up correctly beforehand.

I tried to test with the host kernel allocating 4 vqs, but qemu only
allocated 2 vqs, and
guest kernel will not be able to send even the vsock stream packets. I
think the host
kernel and the qemu have to agree on the number of vhost_vqs. Do you agree?
Did I miss something?

Another idea to make the setting in runtime instead of compiling time is to use
qemu cmd-line options, then qemu can allocate 2 or 4 queues depending on
the cmd line. This will solve the issue when the host kernel is an old
one( no dgram
support) and the qemu is a new one.

But there is still an issue when the host kernel is a new one, while the qemu
is an old one.  I am not sure how to make the virtqueues numbers to
change in run-time
for the host kernel. In another email thread, you mentioned removing kconfig
in the linux kernel, I believe that is related to this qemu patch,
right?  If so,
any ideas that I can make the host kernel change the number of vqs in
the run-time
or when starting up vsock? The only way I can think of is to use a
kernel module parameter
for the vsock_vhost module. Any other ideas? Thanks.

btw, I searched Linux kernel code but did not find any examples.

Regards,

Jiang

On Thu, Jun 10, 2021 at 10:29 AM Jiang Wang . <jiang.wang@bytedance.com> wrote:
>
> On Thu, Jun 10, 2021 at 2:40 AM Stefano Garzarella <sgarzare@redhat.com> 
> wrote:
> >
> > On Thu, Jun 10, 2021 at 12:14:24AM +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.
> > >
> > >The virtio spec patch is here:
> > >https://www.spinics.net/lists/linux-virtualization/msg50027.html
> > >
> > >Here is the link for the linux kernel git repo with patches
> > >to support dgram sockets:
> > >https://github.com/Jiang1155/linux/tree/vsock-dgram-v1
> > >
> > >Signed-off-by: Jiang Wang <jiang.wang@bytedance.com>
> > >---
> > > configure                                     | 13 +++++++++++++
> > > hw/virtio/vhost-vsock-common.c                | 11 ++++++++++-
> > > hw/virtio/vhost-vsock.c                       |  8 +++++---
> > > include/hw/virtio/vhost-vsock-common.h        | 10 +++++++++-
> > > include/standard-headers/linux/virtio_vsock.h |  3 +++
> > > meson.build                                   |  1 +
> > > 6 files changed, 41 insertions(+), 5 deletions(-)
> > >
> > >diff --git a/configure b/configure
> > >index 9f016b06b5..6455b283a5 100755
> > >--- a/configure
> > >+++ b/configure
> > >@@ -343,6 +343,7 @@ vhost_net="$default_feature"
> > > vhost_crypto="$default_feature"
> > > vhost_scsi="$default_feature"
> > > vhost_vsock="$default_feature"
> > >+vhost_vsock_dgram="no"
> > > vhost_user="no"
> > > vhost_user_blk_server="auto"
> > > vhost_user_fs="$default_feature"
> > >@@ -1272,6 +1273,10 @@ for opt do
> > >   ;;
> > >   --enable-vhost-vsock) vhost_vsock="yes"
> > >   ;;
> > >+  --disable-vhost-vsock-dgram) vhost_vsock_dgram="no"
> > >+  ;;
> > >+  --enable-vhost-vsock-dgram) vhost_vsock_dgram="yes"
> > >+  ;;
> >
> > I don't think we should add a configuration option to enable/disable the
> > dgram support at build time.
> >
> > I think we should do it at runtime looking at the features negiotated.
> >
> > Take a look at virtio_net_set_multiqueue().
>
> Got it. Will check. Thanks.
>
> > Thanks,
> > Stefano
> >