> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Thursday, July 6, 2023 4:12 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo....@intel.com>;
> david.march...@redhat.com
> Cc: Maxime Coquelin <maxime.coque...@redhat.com>
> Subject: [PATCH v2 2/2] vhost: fix vduse features negotiation
>
> The series introducing VDUSE support missed the
> application capability to disable supported features.
>
> This results in TSO being negotiated while not supported by
> the application.
>
> Fixes: 0adb8eccc6a6 ("vhost: add VDUSE device creation and destruction")
>
> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
> ---
> lib/vhost/socket.c | 19 +++++++++++++------
> lib/vhost/vduse.c | 29 +++++++----------------------
> lib/vhost/vduse.h | 2 ++
> lib/vhost/vhost.h | 8 +-------
> lib/vhost/vhost_user.h | 9 +++++++++
> 5 files changed, 32 insertions(+), 35 deletions(-)
>
> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
> index 19a7469e45..f55fb299fd 100644
> --- a/lib/vhost/socket.c
> +++ b/lib/vhost/socket.c
> @@ -921,6 +921,10 @@ rte_vhost_driver_register(const char *path, uint64_t
> flags)
> VHOST_LOG_CONFIG(path, ERR, "failed to init connection
> mutex\n");
> goto out_free;
> }
> +
> + if (!strncmp("/dev/vduse/", path, strlen("/dev/vduse/")))
> + vsocket->is_vduse = true;
> +
> vsocket->vdpa_dev = NULL;
> vsocket->max_queue_pairs = VHOST_MAX_QUEUE_PAIRS;
> vsocket->extbuf = flags & RTE_VHOST_USER_EXTBUF_SUPPORT;
> @@ -950,9 +954,14 @@ rte_vhost_driver_register(const char *path, uint64_t
> flags)
> * two values.
> */
> vsocket->use_builtin_virtio_net = true;
> - vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES;
> - vsocket->features = VIRTIO_NET_SUPPORTED_FEATURES;
> - vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES;
> + if (vsocket->is_vduse) {
> + vsocket->supported_features = VDUSE_NET_SUPPORTED_FEATURES;
> + vsocket->features = VDUSE_NET_SUPPORTED_FEATURES;
> + } else {
> + vsocket->supported_features =
> VHOST_USER_NET_SUPPORTED_FEATURES;
> + vsocket->features =
> VHOST_USER_NET_SUPPORTED_FEATURES;
> + vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES;
> + }
>
> if (vsocket->async_copy) {
> vsocket->supported_features &= ~(1ULL << VHOST_F_LOG_ALL);
> @@ -993,9 +1002,7 @@ rte_vhost_driver_register(const char *path, uint64_t
> flags)
> #endif
> }
>
> - if (!strncmp("/dev/vduse/", path, strlen("/dev/vduse/"))) {
> - vsocket->is_vduse = true;
> - } else {
> + if (!vsocket->is_vduse) {
> if ((flags & RTE_VHOST_USER_CLIENT) != 0) {
> vsocket->reconnect = !(flags &
> RTE_VHOST_USER_NO_RECONNECT);
> if (vsocket->reconnect && reconn_tid == 0) {
> diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
> index b9514e6c29..1478562be1 100644
> --- a/lib/vhost/vduse.c
> +++ b/lib/vhost/vduse.c
> @@ -26,27 +26,6 @@
> #define VHOST_VDUSE_API_VERSION 0
> #define VDUSE_CTRL_PATH "/dev/vduse/control"
>
> -#define VDUSE_NET_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) |
> \
> - (1ULL << VIRTIO_F_ANY_LAYOUT) | \
> - (1ULL << VIRTIO_F_VERSION_1) | \
> - (1ULL << VIRTIO_NET_F_GSO) | \
> - (1ULL << VIRTIO_NET_F_HOST_TSO4) | \
> - (1ULL << VIRTIO_NET_F_HOST_TSO6) | \
> - (1ULL << VIRTIO_NET_F_HOST_UFO) | \
> - (1ULL << VIRTIO_NET_F_HOST_ECN) | \
> - (1ULL << VIRTIO_NET_F_CSUM) | \
> - (1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
> - (1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
> - (1ULL << VIRTIO_NET_F_GUEST_TSO6) | \
> - (1ULL << VIRTIO_NET_F_GUEST_UFO) | \
> - (1ULL << VIRTIO_NET_F_GUEST_ECN) | \
> - (1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \
> - (1ULL << VIRTIO_RING_F_EVENT_IDX) | \
> - (1ULL << VIRTIO_F_IN_ORDER) | \
> - (1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
> - (1ULL << VIRTIO_NET_F_CTRL_VQ) | \
> - (1ULL << VIRTIO_NET_F_MQ))
> -
> struct vduse {
> struct fdset fdset;
> };
> @@ -441,7 +420,7 @@ vduse_device_create(const char *path)
> struct virtio_net *dev;
> struct virtio_net_config vnet_config = { 0 };
> uint64_t ver = VHOST_VDUSE_API_VERSION;
> - uint64_t features = VDUSE_NET_SUPPORTED_FEATURES;
> + uint64_t features;
> struct vduse_dev_config *dev_config = NULL;
> const char *name = path + strlen("/dev/vduse/");
>
> @@ -489,6 +468,12 @@ vduse_device_create(const char *path)
> goto out_ctrl_close;
> }
>
> + ret = rte_vhost_driver_get_features(path, &features);
> + if (ret < 0) {
> + VHOST_LOG_CONFIG(name, ERR, "Failed to get backend
> features\n");
> + goto out_free;
> + }
> +
> ret = rte_vhost_driver_get_queue_num(path, &max_queue_pairs);
> if (ret < 0) {
> VHOST_LOG_CONFIG(name, ERR, "Failed to get max queue pairs\n");
> diff --git a/lib/vhost/vduse.h b/lib/vhost/vduse.h
> index a15e5d4c16..d0142694a7 100644
> --- a/lib/vhost/vduse.h
> +++ b/lib/vhost/vduse.h
> @@ -7,6 +7,8 @@
>
> #include "vhost.h"
>
> +#define VDUSE_NET_SUPPORTED_FEATURES VIRTIO_NET_SUPPORTED_FEATURES
> +
> #ifdef VHOST_HAS_VDUSE
>
> int vduse_device_create(const char *path);
> diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
> index 9ecede0f30..f49ce943b0 100644
> --- a/lib/vhost/vhost.h
> +++ b/lib/vhost/vhost.h
> @@ -438,12 +438,8 @@ struct vring_packed_desc_event {
> #define VIRTIO_NET_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) |
> \
> (1ULL << VIRTIO_F_ANY_LAYOUT) | \
> (1ULL << VIRTIO_NET_F_CTRL_VQ) | \
> - (1ULL << VIRTIO_NET_F_CTRL_RX) | \
> - (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \
> (1ULL << VIRTIO_NET_F_MQ) | \
> (1ULL << VIRTIO_F_VERSION_1) | \
> - (1ULL << VHOST_F_LOG_ALL) | \
> - (1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \
> (1ULL << VIRTIO_NET_F_GSO) | \
> (1ULL << VIRTIO_NET_F_HOST_TSO4) | \
> (1ULL << VIRTIO_NET_F_HOST_TSO6) | \
> @@ -457,10 +453,8 @@ struct vring_packed_desc_event {
> (1ULL << VIRTIO_NET_F_GUEST_ECN) | \
> (1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \
> (1ULL << VIRTIO_RING_F_EVENT_IDX) | \
> - (1ULL << VIRTIO_NET_F_MTU) | \
> (1ULL << VIRTIO_F_IN_ORDER) | \
> - (1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
> - (1ULL << VIRTIO_F_RING_PACKED))
> + (1ULL << VIRTIO_F_IOMMU_PLATFORM))
>
>
> struct guest_page {
> diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h
> index 1ffeca92f3..edf7adb3c0 100644
> --- a/lib/vhost/vhost_user.h
> +++ b/lib/vhost/vhost_user.h
> @@ -13,6 +13,15 @@
>
> #define VHOST_MEMORY_MAX_NREGIONS 8
>
> +#define VHOST_USER_NET_SUPPORTED_FEATURES \
> + (VIRTIO_NET_SUPPORTED_FEATURES | \
> + (1ULL << VIRTIO_F_RING_PACKED) | \
> + (1ULL << VIRTIO_NET_F_MTU) | \
> + (1ULL << VHOST_F_LOG_ALL) | \
> + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \
> + (1ULL << VIRTIO_NET_F_CTRL_RX) | \
> + (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE))
> +
> #define VHOST_USER_PROTOCOL_FEATURES ((1ULL <<
> VHOST_USER_PROTOCOL_F_MQ) | \
> (1ULL <<
> VHOST_USER_PROTOCOL_F_LOG_SHMFD)
> |\
> (1ULL << VHOST_USER_PROTOCOL_F_RARP) |
> \
> --
> 2.41.0
Reviewed-by: Chenbo Xia <chenbo....@intel.com>
