[v2,2/2] vhost: fix vduse features negotiation
Checks
Commit Message
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(-)
Comments
> -----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>
@@ -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) {
@@ -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");
@@ -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);
@@ -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 {
@@ -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) | \