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

Jiang Wang posted 1 patch 2 days, 22 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/next-importer-push tags/patchew/20210610001424.209158-1-jiang.wang@bytedance.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.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(-)

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

Posted by Jiang Wang 2 days, 22 hours ago
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"
+  ;;
   --disable-vhost-user-blk-server) vhost_user_blk_server="disabled"
   ;;
   --enable-vhost-user-blk-server) vhost_user_blk_server="enabled"
@@ -1839,6 +1844,7 @@ disabled with --disable-FEATURE, default is enabled if available
   attr            attr and xattr support
   vhost-net       vhost-net kernel acceleration support
   vhost-vsock     virtio sockets device support
+  vhost-vsock-dgram     virtio sockets datagram type support
   vhost-scsi      vhost-scsi kernel target support
   vhost-crypto    vhost-user-crypto backend support
   vhost-kernel    vhost kernel backend support
@@ -2389,6 +2395,10 @@ test "$vhost_vsock" = "" && vhost_vsock=$vhost_kernel
 if test "$vhost_vsock" = "yes" && test "$vhost_kernel" != "yes"; then
   error_exit "--enable-vhost-vsock requires --enable-vhost-kernel"
 fi
+test "$vhost_vsock_dgram" = "" && vhost_vsock_dgram=$vhost_vsock
+if test "$vhost_vsock_dgram" = "yes" && test "$vhost_vsock" != "yes"; then
+  error_exit "--enable-vhost-vsock-dgram requires --enable-vhost-vsock"
+fi
 
 # vhost-user backends
 test "$vhost_net_user" = "" && vhost_net_user=$vhost_user
@@ -5810,6 +5820,9 @@ if test "$vhost_vsock" = "yes" ; then
   if test "$vhost_user" = "yes" ; then
     echo "CONFIG_VHOST_USER_VSOCK=y" >> $config_host_mak
   fi
+  if test "$vhost_vsock_dgram" = "yes" ; then
+    echo "CONFIG_VHOST_VSOCK_DGRAM=y" >> $config_host_mak
+  fi
 fi
 if test "$vhost_kernel" = "yes" ; then
   echo "CONFIG_VHOST_KERNEL=y" >> $config_host_mak
diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
index 4ad6e234ad..fff8d12d91 100644
--- a/hw/virtio/vhost-vsock-common.c
+++ b/hw/virtio/vhost-vsock-common.c
@@ -208,7 +208,12 @@ void vhost_vsock_common_realize(VirtIODevice *vdev, const char *name)
                                       vhost_vsock_common_handle_output);
     vvc->trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
                                        vhost_vsock_common_handle_output);
-
+#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
     /* The event queue belongs to QEMU */
     vvc->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
                                        vhost_vsock_common_handle_output);
@@ -227,6 +232,10 @@ void vhost_vsock_common_unrealize(VirtIODevice *vdev)
 
     virtio_delete_queue(vvc->recv_vq);
     virtio_delete_queue(vvc->trans_vq);
+#ifdef CONFIG_VHOST_VSOCK_DGRAM
+    virtio_delete_queue(vvc->dgram_recv_vq);
+    virtio_delete_queue(vvc->dgram_trans_vq);
+#endif
     virtio_delete_queue(vvc->event_vq);
     virtio_cleanup(vdev);
 }
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 8ddfb9abfe..f6066a69bd 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -105,11 +105,13 @@ static void vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
 }
 
 static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
-                                         uint64_t requested_features,
+                                         uint64_t features,
                                          Error **errp)
 {
-    /* No feature bits used yet */
-    return requested_features;
+#ifdef CONFIG_VHOST_VSOCK_DGRAM
+    virtio_add_feature(&features, VIRTIO_VSOCK_F_DGRAM);
+#endif
+    return features;
 }
 
 static const VMStateDescription vmstate_virtio_vhost_vsock = {
diff --git a/include/hw/virtio/vhost-vsock-common.h b/include/hw/virtio/vhost-vsock-common.h
index e412b5ee98..647ec8c8b3 100644
--- a/include/hw/virtio/vhost-vsock-common.h
+++ b/include/hw/virtio/vhost-vsock-common.h
@@ -27,13 +27,21 @@ enum {
 struct VHostVSockCommon {
     VirtIODevice parent;
 
+#ifdef CONFIG_VHOST_VSOCK_DGRAM
+    struct vhost_virtqueue vhost_vqs[4];
+#else
     struct vhost_virtqueue vhost_vqs[2];
+#endif
+
     struct vhost_dev vhost_dev;
 
     VirtQueue *event_vq;
     VirtQueue *recv_vq;
     VirtQueue *trans_vq;
-
+#ifdef CONFIG_VHOST_VSOCK_DGRAM
+    VirtQueue *dgram_recv_vq;
+    VirtQueue *dgram_trans_vq;
+#endif
     QEMUTimer *post_load_timer;
 };
 
diff --git a/include/standard-headers/linux/virtio_vsock.h b/include/standard-headers/linux/virtio_vsock.h
index be443211ce..abcf2a8adf 100644
--- a/include/standard-headers/linux/virtio_vsock.h
+++ b/include/standard-headers/linux/virtio_vsock.h
@@ -38,6 +38,9 @@
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/virtio_config.h"
 
+/* Feature bits */
+#define VIRTIO_VSOCK_F_DGRAM 0 /*Does vsock support dgram */
+
 struct virtio_vsock_config {
 	uint64_t guest_cid;
 } QEMU_PACKED;
diff --git a/meson.build b/meson.build
index 3d889857a0..9d170e0476 100644
--- a/meson.build
+++ b/meson.build
@@ -2430,6 +2430,7 @@ summary_info += {'vhost-net support': config_host.has_key('CONFIG_VHOST_NET')}
 summary_info += {'vhost-crypto support': config_host.has_key('CONFIG_VHOST_CRYPTO')}
 summary_info += {'vhost-scsi support': config_host.has_key('CONFIG_VHOST_SCSI')}
 summary_info += {'vhost-vsock support': config_host.has_key('CONFIG_VHOST_VSOCK')}
+summary_info += {'vhost-vsock-dgram support': config_host.has_key('CONFIG_VHOST_VSOCK_DGRAM')}
 summary_info += {'vhost-user support': config_host.has_key('CONFIG_VHOST_USER')}
 summary_info += {'vhost-user-blk server support': have_vhost_user_blk_server}
 summary_info += {'vhost-user-fs support': config_host.has_key('CONFIG_VHOST_USER_FS')}
-- 
2.11.0


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

Posted by Stefano Garzarella 2 days, 13 hours ago
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().

Thanks,
Stefano


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

Posted by Jiang Wang . 2 days, 5 hours ago
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
>