From: "Xia, Chenbo" <chenbo.xia@intel.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"david.marchand@redhat.com" <david.marchand@redhat.com>
Subject: RE: [PATCH v2 2/2] vhost: fix vduse features negotiation
Date: Thu, 6 Jul 2023 08:39:38 +0000 [thread overview]
Message-ID: <SN6PR11MB3504323BBE9D2F4DE18AEEA69C2CA@SN6PR11MB3504.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230706081223.151832-3-maxime.coquelin@redhat.com>
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, July 6, 2023 4:12 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
> david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@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.coquelin@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.xia@intel.com>
next prev parent reply other threads:[~2023-07-06 8:39 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-06 8:12 [PATCH v2 0/2] VDUSE fixes for v23.07 Maxime Coquelin
2023-07-06 8:12 ` [PATCH v2 1/2] vduse: fix missing event index features Maxime Coquelin
2023-07-06 8:38 ` Xia, Chenbo
2023-07-06 8:12 ` [PATCH v2 2/2] vhost: fix vduse features negotiation Maxime Coquelin
2023-07-06 8:39 ` Xia, Chenbo [this message]
2023-07-06 8:16 ` [PATCH v2 0/2] VDUSE fixes for v23.07 David Marchand
2023-07-06 9:06 ` Maxime Coquelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SN6PR11MB3504323BBE9D2F4DE18AEEA69C2CA@SN6PR11MB3504.namprd11.prod.outlook.com \
--to=chenbo.xia@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=maxime.coquelin@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).