[dpdk-dev,v2,3/7] net/virtio_user: move vhost user specific code
Checks
Commit Message
To support vhost kernel as the backend of net_virtio_user in coming
patches, we move vhost_user specific structs and macros into
vhost_user.c, and only keep common definitions in vhost.h.
Besides, remove VHOST_USER_MQ feature check, it will be added back
in following multiqueue patch.
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
drivers/net/virtio/virtio_user/vhost.h | 36 ------------------------
drivers/net/virtio/virtio_user/vhost_user.c | 32 +++++++++++++++++++++
drivers/net/virtio/virtio_user/virtio_user_dev.c | 9 ------
3 files changed, 32 insertions(+), 45 deletions(-)
Comments
On Fri, Dec 23, 2016 at 07:14:22AM +0000, Jianfeng Tan wrote:
> To support vhost kernel as the backend of net_virtio_user in coming
> patches, we move vhost_user specific structs and macros into
> vhost_user.c, and only keep common definitions in vhost.h.
Good.
> Besides, remove VHOST_USER_MQ feature check, it will be added back
> in following multiqueue patch.
Why then?
--yliu
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, December 26, 2016 2:28 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; Yigit, Ferruh; Liang, Cunming
> Subject: Re: [PATCH v2 3/7] net/virtio_user: move vhost user specific code
>
> On Fri, Dec 23, 2016 at 07:14:22AM +0000, Jianfeng Tan wrote:
> > To support vhost kernel as the backend of net_virtio_user in coming
> > patches, we move vhost_user specific structs and macros into
> > vhost_user.c, and only keep common definitions in vhost.h.
>
> Good.
>
> > Besides, remove VHOST_USER_MQ feature check, it will be added back
> > in following multiqueue patch.
>
> Why then?
Only vhost user recognizes this feature bit, and vhost kernel does not. My intension is to put those vhost user specific code inside vhost user. And in fact, I forget to add it back. I should add it back in this patch.
Thanks,
Jianfeng
>
> --yliu
On Mon, Dec 26, 2016 at 06:58:58AM +0000, Tan, Jianfeng wrote:
>
>
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Monday, December 26, 2016 2:28 PM
> > To: Tan, Jianfeng
> > Cc: dev@dpdk.org; Yigit, Ferruh; Liang, Cunming
> > Subject: Re: [PATCH v2 3/7] net/virtio_user: move vhost user specific code
> >
> > On Fri, Dec 23, 2016 at 07:14:22AM +0000, Jianfeng Tan wrote:
> > > To support vhost kernel as the backend of net_virtio_user in coming
> > > patches, we move vhost_user specific structs and macros into
> > > vhost_user.c, and only keep common definitions in vhost.h.
> >
> > Good.
> >
> > > Besides, remove VHOST_USER_MQ feature check, it will be added back
> > > in following multiqueue patch.
> >
> > Why then?
>
> Only vhost user recognizes this feature bit, and vhost kernel does not. My intension is to put those vhost user specific code inside vhost user.
That's okay. But why you have to remove it and then add it back?
--yliu
> And in fact, I forget to add it back. I should add it back in this patch.
>
> Thanks,
> Jianfeng
> >
> > --yliu
Hi Yuanhan,
On 12/26/2016 3:57 PM, Yuanhan Liu wrote:
> On Mon, Dec 26, 2016 at 06:58:58AM +0000, Tan, Jianfeng wrote:
>>
>>> -----Original Message-----
>>> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
>>> Sent: Monday, December 26, 2016 2:28 PM
>>> To: Tan, Jianfeng
>>> Cc: dev@dpdk.org; Yigit, Ferruh; Liang, Cunming
>>> Subject: Re: [PATCH v2 3/7] net/virtio_user: move vhost user specific code
>>>
>>> On Fri, Dec 23, 2016 at 07:14:22AM +0000, Jianfeng Tan wrote:
>>>> To support vhost kernel as the backend of net_virtio_user in coming
>>>> patches, we move vhost_user specific structs and macros into
>>>> vhost_user.c, and only keep common definitions in vhost.h.
>>> Good.
>>>
>>>> Besides, remove VHOST_USER_MQ feature check, it will be added back
>>>> in following multiqueue patch.
>>> Why then?
>> Only vhost user recognizes this feature bit, and vhost kernel does not. My intension is to put those vhost user specific code inside vhost user.
> That's okay. But why you have to remove it and then add it back?
After second thought , I agree that it's necessary to add it back.
Currently, our vhost does not implement the semantics of
VHOST_USER_F_PROTOCOL_FEATURES.
Besides, all other comments from you on this patch series will be
addressed on next version. Thank you very much for those suggestions!
Thanks,
Jianfeng
>
> --yliu
>
>> And in fact, I forget to add it back. I should add it back in this patch.
>>
>> Thanks,
>> Jianfeng
>>> --yliu
@@ -42,8 +42,6 @@
#include "../virtio_logs.h"
#include "../virtqueue.h"
-#define VHOST_MEMORY_MAX_NREGIONS 8
-
struct vhost_vring_state {
unsigned int index;
unsigned int num;
@@ -105,40 +103,6 @@ struct vhost_memory_region {
uint64_t mmap_offset;
};
-struct vhost_memory {
- uint32_t nregions;
- uint32_t padding;
- struct vhost_memory_region regions[VHOST_MEMORY_MAX_NREGIONS];
-};
-
-struct vhost_user_msg {
- enum vhost_user_request request;
-
-#define VHOST_USER_VERSION_MASK 0x3
-#define VHOST_USER_REPLY_MASK (0x1 << 2)
- uint32_t flags;
- uint32_t size; /* the following payload size */
- union {
-#define VHOST_USER_VRING_IDX_MASK 0xff
-#define VHOST_USER_VRING_NOFD_MASK (0x1 << 8)
- uint64_t u64;
- struct vhost_vring_state state;
- struct vhost_vring_addr addr;
- struct vhost_memory memory;
- } payload;
- int fds[VHOST_MEMORY_MAX_NREGIONS];
-} __attribute((packed));
-
-#define VHOST_USER_HDR_SIZE offsetof(struct vhost_user_msg, payload.u64)
-#define VHOST_USER_PAYLOAD_SIZE \
- (sizeof(struct vhost_user_msg) - VHOST_USER_HDR_SIZE)
-
-/* The version of the protocol we support */
-#define VHOST_USER_VERSION 0x1
-
-#define VHOST_USER_F_PROTOCOL_FEATURES 30
-#define VHOST_USER_MQ (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)
-
int vhost_user_sock(int vhostfd, enum vhost_user_request req, void *arg);
int vhost_user_setup(const char *path);
int vhost_user_enable_queue_pair(int vhostfd, uint16_t pair_idx, int enable);
@@ -42,6 +42,38 @@
#include "vhost.h"
+/* The version of the protocol we support */
+#define VHOST_USER_VERSION 0x1
+
+#define VHOST_MEMORY_MAX_NREGIONS 8
+struct vhost_memory {
+ uint32_t nregions;
+ uint32_t padding;
+ struct vhost_memory_region regions[VHOST_MEMORY_MAX_NREGIONS];
+};
+
+struct vhost_user_msg {
+ enum vhost_user_request request;
+
+#define VHOST_USER_VERSION_MASK 0x3
+#define VHOST_USER_REPLY_MASK (0x1 << 2)
+ uint32_t flags;
+ uint32_t size; /* the following payload size */
+ union {
+#define VHOST_USER_VRING_IDX_MASK 0xff
+#define VHOST_USER_VRING_NOFD_MASK (0x1 << 8)
+ uint64_t u64;
+ struct vhost_vring_state state;
+ struct vhost_vring_addr addr;
+ struct vhost_memory memory;
+ } payload;
+ int fds[VHOST_MEMORY_MAX_NREGIONS];
+} __attribute((packed));
+
+#define VHOST_USER_HDR_SIZE offsetof(struct vhost_user_msg, payload.u64)
+#define VHOST_USER_PAYLOAD_SIZE \
+ (sizeof(struct vhost_user_msg) - VHOST_USER_HDR_SIZE)
+
static int
vhost_user_write(int fd, void *buf, int len, int *fds, int fd_num)
{
@@ -151,8 +151,6 @@ virtio_user_start_device(struct virtio_user_dev *dev)
* and VIRTIO_NET_F_MAC is stripped.
*/
features = dev->features;
- if (dev->max_queue_pairs > 1)
- features |= VHOST_USER_MQ;
features &= ~(1ull << VIRTIO_NET_F_MAC);
ret = vhost_user_sock(dev->vhostfd, VHOST_USER_SET_FEATURES, &features);
if (ret < 0)
@@ -250,13 +248,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
dev->device_features &= ~(1ull << VIRTIO_NET_F_CTRL_MAC_ADDR);
}
- if (dev->max_queue_pairs > 1) {
- if (!(dev->features & VHOST_USER_MQ)) {
- PMD_INIT_LOG(ERR, "MQ not supported by the backend");
- return -1;
- }
- }
-
return 0;
}