From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev@dpdk.org, chenbo.xia@intel.com
Subject: Re: [PATCH 1/2] vhost: fix vduse features negotiation
Date: Wed, 5 Jul 2023 19:02:07 +0200 [thread overview]
Message-ID: <3cc8e20a-70d7-871c-c799-5adf37306167@redhat.com> (raw)
In-Reply-To: <CAJFAV8yyrDPpaG_f1=FOWHmpoZcLFB_L8kRAmd4yvVLGg+REeg@mail.gmail.com>
Hi David,
On 7/5/23 15:36, David Marchand wrote:
> On Wed, Jul 5, 2023 at 3:22 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>> @@ -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 = VIRTIO_NET_SUPPORTED_FEATURES;
>> + vsocket->features = VIRTIO_NET_SUPPORTED_FEATURES;
>> + vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES;
>> + }
>>
>
> Would it make sense to split those features in a set of features
> shared by vhost-user and vduse, and one dedicated set for each of
> them?
> For example, the VIRTIO_NET_F_GUEST/HOST_* features are not bound to
> vhost-user / vduse, are they?
Most of the features are the same, but there are a few differences:
- VIRTIO_F_RING_PACKED: this is not really Vhost or VDUSE specific. The
issue is we just lack packed ring support in the control queue
implementation, only used by VDUSE for now. I expect to add it in
v23.11.
- VHOST_USER_F_PROTOCOL_FEATURES: This feature is Vhost-user specific
- VHOST_F_LOG_ALL: This is for live-migration, maybe it will be
advertised by VDUSE in the future, but we miss live-migration support in
Kernel VDUSE at the moment.
- VIRTIO_NET_F_CTRL_RX: This is advertised by Vhost-user, but it does
not do anything specific. While if we do it in VDUSE, we need to
implement its support in CVQ.
To summarize, we have some features specific to Vhost-user, but for the
other differences, this is just some features are not yet
implemented/tested with VDUSE.
Maybe we could have something like this, which has the advantage to
highlight the gaps we have in VDUSE (not compiled/tested):
------------------------------------------------------------
diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
index a65d301406..f55fb299fd 100644
--- a/lib/vhost/socket.c
+++ b/lib/vhost/socket.c
@@ -958,8 +958,8 @@ rte_vhost_driver_register(const char *path, uint64_t
flags)
vsocket->supported_features = VDUSE_NET_SUPPORTED_FEATURES;
vsocket->features = VDUSE_NET_SUPPORTED_FEATURES;
} else {
- vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES;
- vsocket->features = VIRTIO_NET_SUPPORTED_FEATURES;
+ vsocket->supported_features =
VHOST_USER_NET_SUPPORTED_FEATURES;
+ vsocket->features =
VHOST_USER_NET_SUPPORTED_FEATURES;
vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES;
}
diff --git a/lib/vhost/vduse.h b/lib/vhost/vduse.h
index 46753fec73..5a8b37ec67 100644
--- a/lib/vhost/vduse.h
+++ b/lib/vhost/vduse.h
@@ -7,28 +7,7 @@
#include "vhost.h"
-#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))
-
-#ifdef VHOST_HAS_VDUSE
+#define VDUSE_NET_SUPPORTED_FEATURES VIRTIO_NET_SUPPORTED_FEATURES
int vduse_device_create(const char *path);
int vduse_device_destroy(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) | \
------------------------------------------------------------
WDYT?
Thanks,
Maxime
>
>
next prev parent reply other threads:[~2023-07-05 17:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-05 13:22 [PATCH 0/2] VDUSE fixes for v23.07 Maxime Coquelin
2023-07-05 13:22 ` [PATCH 1/2] vhost: fix vduse features negotiation Maxime Coquelin
2023-07-05 13:36 ` David Marchand
2023-07-05 17:02 ` Maxime Coquelin [this message]
2023-07-05 17:15 ` David Marchand
2023-07-05 13:22 ` [PATCH 2/2] vduse: fix missing event index features 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=3cc8e20a-70d7-871c-c799-5adf37306167@redhat.com \
--to=maxime.coquelin@redhat.com \
--cc=chenbo.xia@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
/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).