patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [dpdk-dev] vhost/crypto: fix initialization.
@ 2020-10-02  8:36 Fan Zhang
  2020-10-02 12:17 ` Maxime Coquelin
  2020-10-02 15:35 ` [dpdk-stable] [dpdk-dev v2 0/2] " Fan Zhang
  0 siblings, 2 replies; 14+ messages in thread
From: Fan Zhang @ 2020-10-02  8:36 UTC (permalink / raw)
  To: dev
  Cc: chenbo.xia, maxime.coquelin, changpeng.liu, ferruh.yigit, stable,
	Fan Zhang

This patch fixes the problem that vhost crypto cannot be
initialized due to the different requirement between
built-in virtio-net and virtio-crypto. The fix includes
the following change:

- Added new internal enum type virtio_backend_type to
distinguish virtio-net, virtio-crypto, and external
device types.
- Added new API rte_vhost_crypto_driver_start to
distinguish between virtio-net and virtio-crypto built-in
drivers initialization.
- Added new internal function for the vhost library
to use different feature flags when initializing
virtio-crypto.

Fixes: 2ab58f20db51 ("vhost: refactor virtio ready check")
Cc: maxime.coquelin@redhat.com

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 examples/vhost_crypto/main.c           |  3 +-
 lib/librte_vhost/rte_vhost_crypto.h    | 12 +++++++
 lib/librte_vhost/rte_vhost_version.map |  1 +
 lib/librte_vhost/socket.c              | 44 +++++++++++++++++++++-----
 lib/librte_vhost/vhost.h               |  1 -
 lib/librte_vhost/vhost_crypto.c        | 35 ++++++++++++--------
 lib/librte_vhost/vhost_user.h          | 12 +++++++
 7 files changed, 84 insertions(+), 24 deletions(-)

diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
index 11b022e81..ef64e96de 100644
--- a/examples/vhost_crypto/main.c
+++ b/examples/vhost_crypto/main.c
@@ -598,7 +598,8 @@ main(int argc, char *argv[])
 			rte_vhost_driver_callback_register(lo->socket_files[j],
 				&virtio_crypto_device_ops);
 
-			ret = rte_vhost_driver_start(lo->socket_files[j]);
+			ret = rte_vhost_crypto_driver_start(
+					lo->socket_files[j]);
 			if (ret < 0)  {
 				RTE_LOG(ERR, USER1, "failed to start vhost.\n");
 				goto error_exit;
diff --git a/lib/librte_vhost/rte_vhost_crypto.h b/lib/librte_vhost/rte_vhost_crypto.h
index b54d61db6..c809c46a2 100644
--- a/lib/librte_vhost/rte_vhost_crypto.h
+++ b/lib/librte_vhost/rte_vhost_crypto.h
@@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
 	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
 };
 
+/**
+ * Start vhost crypto driver
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @return
+ *  0 on success, -1 on failure
+ */
+__rte_experimental
+int
+rte_vhost_crypto_driver_start(const char *path);
+
 /**
  *  Create Vhost-crypto instance
  *
diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 20b4abcb4..a454d5f41 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -48,6 +48,7 @@ EXPERIMENTAL {
 	rte_vhost_get_vring_base;
 	rte_vhost_set_vring_base;
 	rte_vhost_crypto_create;
+	rte_vhost_crypto_driver_start;
 	rte_vhost_crypto_free;
 	rte_vhost_crypto_fetch_requests;
 	rte_vhost_crypto_finalize_requests;
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 73e1dca95..3de2da836 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -39,7 +39,7 @@ struct vhost_user_socket {
 	bool reconnect;
 	bool dequeue_zero_copy;
 	bool iommu_support;
-	bool use_builtin_virtio_net;
+	enum virtio_backend_type backend_type;
 	bool extbuf;
 	bool linearbuf;
 	bool async_copy;
@@ -225,7 +225,15 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
 	size = strnlen(vsocket->path, PATH_MAX);
 	vhost_set_ifname(vid, vsocket->path, size);
 
-	vhost_set_builtin_virtio_net(vid, vsocket->use_builtin_virtio_net);
+	vhost_set_builtin_virtio_net(vid,
+			vsocket->backend_type == VIRTIO_DEV_BUILTIN_NET ?
+					true : false);
+
+	if (vsocket->backend_type == VIRTIO_DEV_BUILTIN_CRYPTO) {
+		vhost_crypto_set_feature_flags(&vsocket->supported_features,
+				&vsocket->protocol_features);
+		vsocket->features = vsocket->supported_features;
+	}
 
 	vhost_attach_vdpa_device(vid, vsocket->vdpa_dev);
 
@@ -636,7 +644,7 @@ rte_vhost_driver_disable_features(const char *path, uint64_t features)
 	pthread_mutex_lock(&vhost_user.mutex);
 	vsocket = find_vhost_user_socket(path);
 
-	/* Note that use_builtin_virtio_net is not affected by this function
+	/* Note that backend type is not affected by this function
 	 * since callers may want to selectively disable features of the
 	 * built-in vhost net device backend.
 	 */
@@ -685,7 +693,7 @@ rte_vhost_driver_set_features(const char *path, uint64_t features)
 		/* Anyone setting feature bits is implementing their own vhost
 		 * device backend.
 		 */
-		vsocket->use_builtin_virtio_net = false;
+		vsocket->backend_type = VIRTIO_DEV_UNKNOWN;
 	}
 	pthread_mutex_unlock(&vhost_user.mutex);
 
@@ -913,7 +921,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
 	 * rte_vhost_driver_set_features(), which will overwrite following
 	 * two values.
 	 */
-	vsocket->use_builtin_virtio_net = true;
+	vsocket->backend_type = VIRTIO_DEV_BUILTIN_NET;
 	vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES;
 	vsocket->features           = VIRTIO_NET_SUPPORTED_FEATURES;
 	vsocket->protocol_features  = VHOST_USER_PROTOCOL_FEATURES;
@@ -1164,10 +1172,17 @@ vhost_driver_callback_get(const char *path)
 }
 
 int
-rte_vhost_driver_start(const char *path)
+vhost_driver_start(const char *path, enum virtio_backend_type backend_type)
 {
 	struct vhost_user_socket *vsocket;
 	static pthread_t fdset_tid;
+	int ret;
+
+	if (backend_type <= VIRTIO_DEV_UNKNOWN ||
+			backend_type > VIRTIO_DEV_BUILTIN_CRYPTO) {
+		VHOST_LOG_CONFIG(ERR, "Wrong backend type\n");
+		return -1;
+	}
 
 	pthread_mutex_lock(&vhost_user.mutex);
 	vsocket = find_vhost_user_socket(path);
@@ -1200,7 +1215,20 @@ rte_vhost_driver_start(const char *path)
 	}
 
 	if (vsocket->is_server)
-		return vhost_user_start_server(vsocket);
+		ret = vhost_user_start_server(vsocket);
 	else
-		return vhost_user_start_client(vsocket);
+		ret = vhost_user_start_client(vsocket);
+
+	if (ret < 0)
+		return ret;
+
+	vsocket->backend_type = backend_type;
+
+	return 0;
+}
+
+int
+rte_vhost_driver_start(const char *path)
+{
+	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_NET);
 }
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 73a1ed889..1da9ed871 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -352,7 +352,6 @@ struct vring_packed_desc_event {
 				(1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
 				(1ULL << VIRTIO_F_RING_PACKED))
 
-
 struct guest_page {
 	uint64_t guest_phys_addr;
 	uint64_t host_phys_addr;
diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index e08f9c6d7..e9c6702c2 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -35,13 +35,13 @@
 #define VC_LOG_DBG(fmt, args...)
 #endif
 
-#define VIRTIO_CRYPTO_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
-		(1 << VIRTIO_RING_F_INDIRECT_DESC) |			\
-		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
-		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |			\
-		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |			\
-		(1 << VIRTIO_NET_F_CTRL_VQ) |				\
-		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
+#define VIRTIO_CRYPTO_FEATURES ((1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
+		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |			\
+		(1ULL << VIRTIO_RING_F_EVENT_IDX) |			\
+		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
+		(1ULL << VIRTIO_F_VERSION_1) |				\
+		(1ULL << VHOST_USER_PROTOCOL_F_CONFIG) |		\
+		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
 
 #define IOVA_TO_VVA(t, r, a, l, p)					\
 	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p))
@@ -1400,6 +1400,20 @@ vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
 	return processed;
 }
 
+void
+vhost_crypto_set_feature_flags(uint64_t *feature_flags,
+		uint64_t *protocol_feature_flags)
+{
+	*feature_flags = VIRTIO_CRYPTO_FEATURES;
+	*protocol_feature_flags |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
+}
+
+int
+rte_vhost_crypto_driver_start(const char *path)
+{
+	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);
+}
+
 int
 rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
 		struct rte_mempool *sess_pool,
@@ -1417,13 +1431,6 @@ rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
 		return -EINVAL;
 	}
 
-	ret = rte_vhost_driver_set_features(dev->ifname,
-			VIRTIO_CRYPTO_FEATURES);
-	if (ret < 0) {
-		VC_LOG_ERR("Error setting features");
-		return -1;
-	}
-
 	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
 			RTE_CACHE_LINE_SIZE, socket_id);
 	if (!vcrypto) {
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 16fe03f88..d28da17dd 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -158,6 +158,12 @@ typedef struct VhostUserMsg {
 /* The version of the protocol we support */
 #define VHOST_USER_VERSION    0x1
 
+/* virtio backend types */
+enum virtio_backend_type {
+	VIRTIO_DEV_UNKNOWN = 0, /* Likely external */
+	VIRTIO_DEV_BUILTIN_NET, /* Virtio-net device */
+	VIRTIO_DEV_BUILTIN_CRYPTO, /* Virtio-crypto device */
+};
 
 /* vhost_user.c */
 int vhost_user_msg_handler(int vid, int fd);
@@ -167,5 +173,11 @@ int vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm);
 int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
 		int *fd_num);
 int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num);
+int vhost_driver_start(const char *path,
+		enum virtio_backend_type backend_type);
+
+/* vhost_crypto.c */
+void vhost_crypto_set_feature_flags(uint64_t *feature_flags,
+		uint64_t *protocol_feature_flags);
 
 #endif
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] vhost/crypto: fix initialization.
  2020-10-02  8:36 [dpdk-stable] [dpdk-dev] vhost/crypto: fix initialization Fan Zhang
@ 2020-10-02 12:17 ` Maxime Coquelin
  2020-10-02 12:38   ` Zhang, Roy Fan
  2020-10-02 15:35 ` [dpdk-stable] [dpdk-dev v2 0/2] " Fan Zhang
  1 sibling, 1 reply; 14+ messages in thread
From: Maxime Coquelin @ 2020-10-02 12:17 UTC (permalink / raw)
  To: Fan Zhang, dev; +Cc: chenbo.xia, changpeng.liu, ferruh.yigit, stable

Hi Fan,

Thanks for working on this.

The commit message should not contain dot, please remove it in v2.

On 10/2/20 10:36 AM, Fan Zhang wrote:
> This patch fixes the problem that vhost crypto cannot be
> initialized due to the different requirement between
> built-in virtio-net and virtio-crypto. The fix includes
> the following change:
> 
> - Added new internal enum type virtio_backend_type to
> distinguish virtio-net, virtio-crypto, and external
> device types.
> - Added new API rte_vhost_crypto_driver_start to
> distinguish between virtio-net and virtio-crypto built-in
> drivers initialization.
> - Added new internal function for the vhost library
> to use different feature flags when initializing
> virtio-crypto.

This last one should be part of a dedicated patch.


> Fixes: 2ab58f20db51 ("vhost: refactor virtio ready check")
Please remove that Fixes tag.
Looking in this patch, we can see it worked by luck. Thanks to the
v20.08 refactoring, we spotted that Vhost crypto was broken.

> Cc: maxime.coquelin@redhat.com
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  examples/vhost_crypto/main.c           |  3 +-
>  lib/librte_vhost/rte_vhost_crypto.h    | 12 +++++++
>  lib/librte_vhost/rte_vhost_version.map |  1 +
>  lib/librte_vhost/socket.c              | 44 +++++++++++++++++++++-----
>  lib/librte_vhost/vhost.h               |  1 -
>  lib/librte_vhost/vhost_crypto.c        | 35 ++++++++++++--------
>  lib/librte_vhost/vhost_user.h          | 12 +++++++
>  7 files changed, 84 insertions(+), 24 deletions(-)
> 
> diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
> index 11b022e81..ef64e96de 100644
> --- a/examples/vhost_crypto/main.c
> +++ b/examples/vhost_crypto/main.c
> @@ -598,7 +598,8 @@ main(int argc, char *argv[])
>  			rte_vhost_driver_callback_register(lo->socket_files[j],
>  				&virtio_crypto_device_ops);
>  
> -			ret = rte_vhost_driver_start(lo->socket_files[j]);
> +			ret = rte_vhost_crypto_driver_start(
> +					lo->socket_files[j]);
>  			if (ret < 0)  {
>  				RTE_LOG(ERR, USER1, "failed to start vhost.\n");
>  				goto error_exit;
> diff --git a/lib/librte_vhost/rte_vhost_crypto.h b/lib/librte_vhost/rte_vhost_crypto.h
> index b54d61db6..c809c46a2 100644
> --- a/lib/librte_vhost/rte_vhost_crypto.h
> +++ b/lib/librte_vhost/rte_vhost_crypto.h
> @@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
>  	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
>  };
>  
> +/**
> + * Start vhost crypto driver
> + *
> + * @param path
> + *  The vhost-user socket file path
> + * @return
> + *  0 on success, -1 on failure
> + */
> +__rte_experimental
> +int
> +rte_vhost_crypto_driver_start(const char *path);
> +
>  /**
>   *  Create Vhost-crypto instance
>   *
> diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
> index 20b4abcb4..a454d5f41 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -48,6 +48,7 @@ EXPERIMENTAL {
>  	rte_vhost_get_vring_base;
>  	rte_vhost_set_vring_base;
>  	rte_vhost_crypto_create;
> +	rte_vhost_crypto_driver_start;
>  	rte_vhost_crypto_free;
>  	rte_vhost_crypto_fetch_requests;
>  	rte_vhost_crypto_finalize_requests;
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 73e1dca95..3de2da836 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -39,7 +39,7 @@ struct vhost_user_socket {
>  	bool reconnect;
>  	bool dequeue_zero_copy;
>  	bool iommu_support;
> -	bool use_builtin_virtio_net;
> +	enum virtio_backend_type backend_type;
>  	bool extbuf;
>  	bool linearbuf;
>  	bool async_copy;
> @@ -225,7 +225,15 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
>  	size = strnlen(vsocket->path, PATH_MAX);
>  	vhost_set_ifname(vid, vsocket->path, size);
>  
> -	vhost_set_builtin_virtio_net(vid, vsocket->use_builtin_virtio_net);
> +	vhost_set_builtin_virtio_net(vid,
> +			vsocket->backend_type == VIRTIO_DEV_BUILTIN_NET ?
> +					true : false);
> +
> +	if (vsocket->backend_type == VIRTIO_DEV_BUILTIN_CRYPTO) {
> +		vhost_crypto_set_feature_flags(&vsocket->supported_features,
> +				&vsocket->protocol_features);

It should not be done like that, we have API for that.
We don't want to call vhost_crypto API in socket.c

Features supported by the application/backend have to be set between
rte_vhost_driver_register() and rte_vhost_driver_start() calls.

It think it can be done in rte_vhost_crypto_driver_start() function,
please see below.

> +		vsocket->features = vsocket->supported_features;
> +	}
>  
>  	vhost_attach_vdpa_device(vid, vsocket->vdpa_dev);
>  
> @@ -636,7 +644,7 @@ rte_vhost_driver_disable_features(const char *path, uint64_t features)
>  	pthread_mutex_lock(&vhost_user.mutex);
>  	vsocket = find_vhost_user_socket(path);
>  
> -	/* Note that use_builtin_virtio_net is not affected by this function
> +	/* Note that backend type is not affected by this function
>  	 * since callers may want to selectively disable features of the
>  	 * built-in vhost net device backend.
>  	 */
> @@ -685,7 +693,7 @@ rte_vhost_driver_set_features(const char *path, uint64_t features)
>  		/* Anyone setting feature bits is implementing their own vhost
>  		 * device backend.
>  		 */
> -		vsocket->use_builtin_virtio_net = false;
> +		vsocket->backend_type = VIRTIO_DEV_UNKNOWN;
>  	}
>  	pthread_mutex_unlock(&vhost_user.mutex);
>  
> @@ -913,7 +921,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
>  	 * rte_vhost_driver_set_features(), which will overwrite following
>  	 * two values.
>  	 */
> -	vsocket->use_builtin_virtio_net = true;
> +	vsocket->backend_type = VIRTIO_DEV_BUILTIN_NET;
>  	vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES;
>  	vsocket->features           = VIRTIO_NET_SUPPORTED_FEATURES;
>  	vsocket->protocol_features  = VHOST_USER_PROTOCOL_FEATURES;
> @@ -1164,10 +1172,17 @@ vhost_driver_callback_get(const char *path)
>  }
>  
>  int
> -rte_vhost_driver_start(const char *path)
> +vhost_driver_start(const char *path, enum virtio_backend_type backend_type)
>  {
>  	struct vhost_user_socket *vsocket;
>  	static pthread_t fdset_tid;
> +	int ret;
> +
> +	if (backend_type <= VIRTIO_DEV_UNKNOWN ||
> +			backend_type > VIRTIO_DEV_BUILTIN_CRYPTO) {
> +		VHOST_LOG_CONFIG(ERR, "Wrong backend type\n");
> +		return -1;
> +	}
>  
>  	pthread_mutex_lock(&vhost_user.mutex);
>  	vsocket = find_vhost_user_socket(path);
> @@ -1200,7 +1215,20 @@ rte_vhost_driver_start(const char *path)
>  	}
>  
>  	if (vsocket->is_server)
> -		return vhost_user_start_server(vsocket);
> +		ret = vhost_user_start_server(vsocket);
>  	else
> -		return vhost_user_start_client(vsocket);
> +		ret = vhost_user_start_client(vsocket);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	vsocket->backend_type = backend_type;
> +
> +	return 0;
> +}
> +
> +int
> +rte_vhost_driver_start(const char *path)
> +{
> +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_NET);
>  }
> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> index 73a1ed889..1da9ed871 100644
> --- a/lib/librte_vhost/vhost.h
> +++ b/lib/librte_vhost/vhost.h
> @@ -352,7 +352,6 @@ struct vring_packed_desc_event {
>  				(1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
>  				(1ULL << VIRTIO_F_RING_PACKED))
>  
> -
>  struct guest_page {
>  	uint64_t guest_phys_addr;
>  	uint64_t host_phys_addr;
> diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
> index e08f9c6d7..e9c6702c2 100644
> --- a/lib/librte_vhost/vhost_crypto.c
> +++ b/lib/librte_vhost/vhost_crypto.c
> @@ -35,13 +35,13 @@
>  #define VC_LOG_DBG(fmt, args...)
>  #endif
>  
> -#define VIRTIO_CRYPTO_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> -		(1 << VIRTIO_RING_F_INDIRECT_DESC) |			\
> -		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
> -		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |			\
> -		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |			\

The two above lines are not Virtio features, how can it have possibly
work in the past?

> -		(1 << VIRTIO_NET_F_CTRL_VQ) |				\
> -		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
> +#define VIRTIO_CRYPTO_FEATURES ((1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> +		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |			\
> +		(1ULL << VIRTIO_RING_F_EVENT_IDX) |			\
> +		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
> +		(1ULL << VIRTIO_F_VERSION_1) |				\
> +		(1ULL << VHOST_USER_PROTOCOL_F_CONFIG) |		\

Ouch, VHOST_USER_PROTOCOL_F_CONFIG is not a Virtio feature, but a Vhost-
user protocol feature. Please remove that in a dedicated patch.

> +		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))

Why is this now necessary? I don't see the link with the purpose of the
patch.

I think I understand now! I bet that it just means that previously,
VHOST_USER_GET_FEATURES was sent before Vhost-crypto would call
rte_vhost_driver_set_features(), which makes a lot of sense as I said
when I was surprised .new_device() was called without features not being
negotiated.

It was really working by luck.

>  
>  #define IOVA_TO_VVA(t, r, a, l, p)					\
>  	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p))
> @@ -1400,6 +1400,20 @@ vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
>  	return processed;
>  }
>  
> +void
> +vhost_crypto_set_feature_flags(uint64_t *feature_flags,
> +		uint64_t *protocol_feature_flags)
> +{
> +	*feature_flags = VIRTIO_CRYPTO_FEATURES;
> +	*protocol_feature_flags |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> +}

So above function can be removed.

> +int
> +rte_vhost_crypto_driver_start(const char *path)
> +{
Here we set supported Virtio and Vhost-usuer protocol features:

	int ret;

	ret = rte_vhost_driver_set_features(path, VIRTIO_CRYPTO_FEATURES);
	if (ret)
		return -1;

	ret = rte_vhost_driver_get_protocol_features(path, &protocol_features);
	if (ret)
		return -1;
	protocol_features |= VHOST_USER_PROTOCOL_F_CONFIG;
	ret = rte_vhost_driver_set_protocol_features(path, protocol_features);
	if (ret)
		return -1;


> +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);
> +}
> +
>  int
>  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>  		struct rte_mempool *sess_pool,
> @@ -1417,13 +1431,6 @@ rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>  		return -EINVAL;
>  	}
>  
> -	ret = rte_vhost_driver_set_features(dev->ifname,
> -			VIRTIO_CRYPTO_FEATURES);
> -	if (ret < 0) {
> -		VC_LOG_ERR("Error setting features");
> -		return -1;
> -	}
> -
>  	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
>  			RTE_CACHE_LINE_SIZE, socket_id);
>  	if (!vcrypto) {
> diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
> index 16fe03f88..d28da17dd 100644
> --- a/lib/librte_vhost/vhost_user.h
> +++ b/lib/librte_vhost/vhost_user.h
> @@ -158,6 +158,12 @@ typedef struct VhostUserMsg {
>  /* The version of the protocol we support */
>  #define VHOST_USER_VERSION    0x1
>  
> +/* virtio backend types */
> +enum virtio_backend_type {
> +	VIRTIO_DEV_UNKNOWN = 0, /* Likely external */
> +	VIRTIO_DEV_BUILTIN_NET, /* Virtio-net device */
> +	VIRTIO_DEV_BUILTIN_CRYPTO, /* Virtio-crypto device */
> +};
>  
>  /* vhost_user.c */
>  int vhost_user_msg_handler(int vid, int fd);
> @@ -167,5 +173,11 @@ int vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm);
>  int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
>  		int *fd_num);
>  int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num);
> +int vhost_driver_start(const char *path,
> +		enum virtio_backend_type backend_type);
> +
> +/* vhost_crypto.c */
> +void vhost_crypto_set_feature_flags(uint64_t *feature_flags,
> +		uint64_t *protocol_feature_flags);
>  
>  #endif
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] vhost/crypto: fix initialization.
  2020-10-02 12:17 ` Maxime Coquelin
@ 2020-10-02 12:38   ` Zhang, Roy Fan
  0 siblings, 0 replies; 14+ messages in thread
From: Zhang, Roy Fan @ 2020-10-02 12:38 UTC (permalink / raw)
  To: Maxime Coquelin, dev; +Cc: Xia, Chenbo, Liu, Changpeng, Yigit, Ferruh, stable

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, October 2, 2020 1:18 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Liu, Changpeng
> <changpeng.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] vhost/crypto: fix initialization.
> 
> Hi Fan,
> 
> Thanks for working on this.
> 
> The commit message should not contain dot, please remove it in v2.
> 
> On 10/2/20 10:36 AM, Fan Zhang wrote:
> > This patch fixes the problem that vhost crypto cannot be
> > initialized due to the different requirement between
> > built-in virtio-net and virtio-crypto. The fix includes
> > the following change:
> >
> > - Added new internal enum type virtio_backend_type to
> > distinguish virtio-net, virtio-crypto, and external
> > device types.
> > - Added new API rte_vhost_crypto_driver_start to
> > distinguish between virtio-net and virtio-crypto built-in
> > drivers initialization.
> > - Added new internal function for the vhost library
> > to use different feature flags when initializing
> > virtio-crypto.
> 
> This last one should be part of a dedicated patch.
Will do.
> 
> 
> > Fixes: 2ab58f20db51 ("vhost: refactor virtio ready check")
> Please remove that Fixes tag.
> Looking in this patch, we can see it worked by luck. Thanks to the
> v20.08 refactoring, we spotted that Vhost crypto was broken.
> 
> > Cc: maxime.coquelin@redhat.com
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> >  examples/vhost_crypto/main.c           |  3 +-
> >  lib/librte_vhost/rte_vhost_crypto.h    | 12 +++++++
> >  lib/librte_vhost/rte_vhost_version.map |  1 +
> >  lib/librte_vhost/socket.c              | 44 +++++++++++++++++++++-----
> >  lib/librte_vhost/vhost.h               |  1 -
> >  lib/librte_vhost/vhost_crypto.c        | 35 ++++++++++++--------
> >  lib/librte_vhost/vhost_user.h          | 12 +++++++
> >  7 files changed, 84 insertions(+), 24 deletions(-)
> >
> > diff --git a/examples/vhost_crypto/main.c
> b/examples/vhost_crypto/main.c
> > index 11b022e81..ef64e96de 100644
> > --- a/examples/vhost_crypto/main.c
> > +++ b/examples/vhost_crypto/main.c
> > @@ -598,7 +598,8 @@ main(int argc, char *argv[])
> >  			rte_vhost_driver_callback_register(lo-
> >socket_files[j],
> >  				&virtio_crypto_device_ops);
> >
> > -			ret = rte_vhost_driver_start(lo->socket_files[j]);
> > +			ret = rte_vhost_crypto_driver_start(
> > +					lo->socket_files[j]);
> >  			if (ret < 0)  {
> >  				RTE_LOG(ERR, USER1, "failed to start
> vhost.\n");
> >  				goto error_exit;
> > diff --git a/lib/librte_vhost/rte_vhost_crypto.h
> b/lib/librte_vhost/rte_vhost_crypto.h
> > index b54d61db6..c809c46a2 100644
> > --- a/lib/librte_vhost/rte_vhost_crypto.h
> > +++ b/lib/librte_vhost/rte_vhost_crypto.h
> > @@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
> >  	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
> >  };
> >
> > +/**
> > + * Start vhost crypto driver
> > + *
> > + * @param path
> > + *  The vhost-user socket file path
> > + * @return
> > + *  0 on success, -1 on failure
> > + */
> > +__rte_experimental
> > +int
> > +rte_vhost_crypto_driver_start(const char *path);
> > +
> >  /**
> >   *  Create Vhost-crypto instance
> >   *
> > diff --git a/lib/librte_vhost/rte_vhost_version.map
> b/lib/librte_vhost/rte_vhost_version.map
> > index 20b4abcb4..a454d5f41 100644
> > --- a/lib/librte_vhost/rte_vhost_version.map
> > +++ b/lib/librte_vhost/rte_vhost_version.map
> > @@ -48,6 +48,7 @@ EXPERIMENTAL {
> >  	rte_vhost_get_vring_base;
> >  	rte_vhost_set_vring_base;
> >  	rte_vhost_crypto_create;
> > +	rte_vhost_crypto_driver_start;
> >  	rte_vhost_crypto_free;
> >  	rte_vhost_crypto_fetch_requests;
> >  	rte_vhost_crypto_finalize_requests;
> > diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> > index 73e1dca95..3de2da836 100644
> > --- a/lib/librte_vhost/socket.c
> > +++ b/lib/librte_vhost/socket.c
> > @@ -39,7 +39,7 @@ struct vhost_user_socket {
> >  	bool reconnect;
> >  	bool dequeue_zero_copy;
> >  	bool iommu_support;
> > -	bool use_builtin_virtio_net;
> > +	enum virtio_backend_type backend_type;
> >  	bool extbuf;
> >  	bool linearbuf;
> >  	bool async_copy;
> > @@ -225,7 +225,15 @@ vhost_user_add_connection(int fd, struct
> vhost_user_socket *vsocket)
> >  	size = strnlen(vsocket->path, PATH_MAX);
> >  	vhost_set_ifname(vid, vsocket->path, size);
> >
> > -	vhost_set_builtin_virtio_net(vid, vsocket->use_builtin_virtio_net);
> > +	vhost_set_builtin_virtio_net(vid,
> > +			vsocket->backend_type ==
> VIRTIO_DEV_BUILTIN_NET ?
> > +					true : false);
> > +
> > +	if (vsocket->backend_type == VIRTIO_DEV_BUILTIN_CRYPTO) {
> > +		vhost_crypto_set_feature_flags(&vsocket-
> >supported_features,
> > +				&vsocket->protocol_features);
> 
> It should not be done like that, we have API for that.
> We don't want to call vhost_crypto API in socket.c
> 
> Features supported by the application/backend have to be set between
> rte_vhost_driver_register() and rte_vhost_driver_start() calls.

I agree. This is actually the place I am not certain for the patch.

> 
> It think it can be done in rte_vhost_crypto_driver_start() function,
> please see below.


> 
> > +		vsocket->features = vsocket->supported_features;
> > +	}
> >
> >  	vhost_attach_vdpa_device(vid, vsocket->vdpa_dev);
> >
> > @@ -636,7 +644,7 @@ rte_vhost_driver_disable_features(const char
> *path, uint64_t features)
> >  	pthread_mutex_lock(&vhost_user.mutex);
> >  	vsocket = find_vhost_user_socket(path);
> >
> > -	/* Note that use_builtin_virtio_net is not affected by this function
> > +	/* Note that backend type is not affected by this function
> >  	 * since callers may want to selectively disable features of the
> >  	 * built-in vhost net device backend.
> >  	 */
> > @@ -685,7 +693,7 @@ rte_vhost_driver_set_features(const char *path,
> uint64_t features)
> >  		/* Anyone setting feature bits is implementing their own
> vhost
> >  		 * device backend.
> >  		 */
> > -		vsocket->use_builtin_virtio_net = false;
> > +		vsocket->backend_type = VIRTIO_DEV_UNKNOWN;
> >  	}
> >  	pthread_mutex_unlock(&vhost_user.mutex);
> >
> > @@ -913,7 +921,7 @@ rte_vhost_driver_register(const char *path,
> uint64_t flags)
> >  	 * rte_vhost_driver_set_features(), which will overwrite following
> >  	 * two values.
> >  	 */
> > -	vsocket->use_builtin_virtio_net = true;
> > +	vsocket->backend_type = VIRTIO_DEV_BUILTIN_NET;
> >  	vsocket->supported_features =
> VIRTIO_NET_SUPPORTED_FEATURES;
> >  	vsocket->features           = VIRTIO_NET_SUPPORTED_FEATURES;
> >  	vsocket->protocol_features  = VHOST_USER_PROTOCOL_FEATURES;
> > @@ -1164,10 +1172,17 @@ vhost_driver_callback_get(const char *path)
> >  }
> >
> >  int
> > -rte_vhost_driver_start(const char *path)
> > +vhost_driver_start(const char *path, enum virtio_backend_type
> backend_type)
> >  {
> >  	struct vhost_user_socket *vsocket;
> >  	static pthread_t fdset_tid;
> > +	int ret;
> > +
> > +	if (backend_type <= VIRTIO_DEV_UNKNOWN ||
> > +			backend_type > VIRTIO_DEV_BUILTIN_CRYPTO) {
> > +		VHOST_LOG_CONFIG(ERR, "Wrong backend type\n");
> > +		return -1;
> > +	}
> >
> >  	pthread_mutex_lock(&vhost_user.mutex);
> >  	vsocket = find_vhost_user_socket(path);
> > @@ -1200,7 +1215,20 @@ rte_vhost_driver_start(const char *path)
> >  	}
> >
> >  	if (vsocket->is_server)
> > -		return vhost_user_start_server(vsocket);
> > +		ret = vhost_user_start_server(vsocket);
> >  	else
> > -		return vhost_user_start_client(vsocket);
> > +		ret = vhost_user_start_client(vsocket);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	vsocket->backend_type = backend_type;
> > +
> > +	return 0;
> > +}
> > +
> > +int
> > +rte_vhost_driver_start(const char *path)
> > +{
> > +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_NET);
> >  }
> > diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> > index 73a1ed889..1da9ed871 100644
> > --- a/lib/librte_vhost/vhost.h
> > +++ b/lib/librte_vhost/vhost.h
> > @@ -352,7 +352,6 @@ struct vring_packed_desc_event {
> >  				(1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
> >  				(1ULL << VIRTIO_F_RING_PACKED))
> >
> > -
> >  struct guest_page {
> >  	uint64_t guest_phys_addr;
> >  	uint64_t host_phys_addr;
> > diff --git a/lib/librte_vhost/vhost_crypto.c
> b/lib/librte_vhost/vhost_crypto.c
> > index e08f9c6d7..e9c6702c2 100644
> > --- a/lib/librte_vhost/vhost_crypto.c
> > +++ b/lib/librte_vhost/vhost_crypto.c
> > @@ -35,13 +35,13 @@
> >  #define VC_LOG_DBG(fmt, args...)
> >  #endif
> >
> > -#define VIRTIO_CRYPTO_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY)
> |	\
> > -		(1 << VIRTIO_RING_F_INDIRECT_DESC) |
> 	\
> > -		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
> > -		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |
> 	\
> > -		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |
> 	\
> 
> The two above lines are not Virtio features, how can it have possibly
> work in the past?

Not sure - amazingly it worked  :-). Sorry for made  the mistake in the
first place. By then it was my first virtio task.

> 
> > -		(1 << VIRTIO_NET_F_CTRL_VQ) |
> 	\
> > -		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
> > +#define VIRTIO_CRYPTO_FEATURES ((1ULL <<
> VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> > +		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
> 	\
> > +		(1ULL << VIRTIO_RING_F_EVENT_IDX) |
> 	\
> > +		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
> > +		(1ULL << VIRTIO_F_VERSION_1) |
> 	\
> > +		(1ULL << VHOST_USER_PROTOCOL_F_CONFIG) |
> 	\
> 
> Ouch, VHOST_USER_PROTOCOL_F_CONFIG is not a Virtio feature, but a
> Vhost-
> user protocol feature. Please remove that in a dedicated patch.
> 
> > +		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
> 
> Why is this now necessary? I don't see the link with the purpose of the
> patch.

Without this line 1 out of 2 times the feature negotiation will fail. I was
using Qemu 3.0.1 for my test.

> 
> I think I understand now! I bet that it just means that previously,
> VHOST_USER_GET_FEATURES was sent before Vhost-crypto would call
> rte_vhost_driver_set_features(), which makes a lot of sense as I said
> when I was surprised .new_device() was called without features not being
> negotiated.
> 
> It was really working by luck.


You are right. 

> 
> >
> >  #define IOVA_TO_VVA(t, r, a, l, p)					\
> >  	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p))
> > @@ -1400,6 +1400,20 @@
> vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
> >  	return processed;
> >  }
> >
> > +void
> > +vhost_crypto_set_feature_flags(uint64_t *feature_flags,
> > +		uint64_t *protocol_feature_flags)
> > +{
> > +	*feature_flags = VIRTIO_CRYPTO_FEATURES;
> > +	*protocol_feature_flags |= (1ULL <<
> VHOST_USER_PROTOCOL_F_CONFIG);
> > +}
> 
> So above function can be removed.
> 
> > +int
> > +rte_vhost_crypto_driver_start(const char *path)
> > +{
> Here we set supported Virtio and Vhost-usuer protocol features:
> 
> 	int ret;
> 
> 	ret = rte_vhost_driver_set_features(path,
> VIRTIO_CRYPTO_FEATURES);
> 	if (ret)
> 		return -1;
> 
> 	ret = rte_vhost_driver_get_protocol_features(path,
> &protocol_features);
> 	if (ret)
> 		return -1;
> 	protocol_features |= VHOST_USER_PROTOCOL_F_CONFIG;
> 	ret = rte_vhost_driver_set_protocol_features(path,
> protocol_features);
> 	if (ret)
> 		return -1;
> 
> 
> > +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);

Thanks. I will prepare v2 right away with your change proposed.

Regards,
Fan

> > +}
> > +
> >  int
> >  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
> >  		struct rte_mempool *sess_pool,
> > @@ -1417,13 +1431,6 @@ rte_vhost_crypto_create(int vid, uint8_t
> cryptodev_id,
> >  		return -EINVAL;
> >  	}
> >
> > -	ret = rte_vhost_driver_set_features(dev->ifname,
> > -			VIRTIO_CRYPTO_FEATURES);
> > -	if (ret < 0) {
> > -		VC_LOG_ERR("Error setting features");
> > -		return -1;
> > -	}
> > -
> >  	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
> >  			RTE_CACHE_LINE_SIZE, socket_id);
> >  	if (!vcrypto) {
> > diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
> > index 16fe03f88..d28da17dd 100644
> > --- a/lib/librte_vhost/vhost_user.h
> > +++ b/lib/librte_vhost/vhost_user.h
> > @@ -158,6 +158,12 @@ typedef struct VhostUserMsg {
> >  /* The version of the protocol we support */
> >  #define VHOST_USER_VERSION    0x1
> >
> > +/* virtio backend types */
> > +enum virtio_backend_type {
> > +	VIRTIO_DEV_UNKNOWN = 0, /* Likely external */
> > +	VIRTIO_DEV_BUILTIN_NET, /* Virtio-net device */
> > +	VIRTIO_DEV_BUILTIN_CRYPTO, /* Virtio-crypto device */
> > +};
> >
> >  /* vhost_user.c */
> >  int vhost_user_msg_handler(int vid, int fd);
> > @@ -167,5 +173,11 @@ int vhost_user_iotlb_miss(struct virtio_net *dev,
> uint64_t iova, uint8_t perm);
> >  int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int
> max_fds,
> >  		int *fd_num);
> >  int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int
> fd_num);
> > +int vhost_driver_start(const char *path,
> > +		enum virtio_backend_type backend_type);
> > +
> > +/* vhost_crypto.c */
> > +void vhost_crypto_set_feature_flags(uint64_t *feature_flags,
> > +		uint64_t *protocol_feature_flags);
> >
> >  #endif
> >


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-stable] [dpdk-dev v2 0/2] vhost/crypto: fix initialization
  2020-10-02  8:36 [dpdk-stable] [dpdk-dev] vhost/crypto: fix initialization Fan Zhang
  2020-10-02 12:17 ` Maxime Coquelin
@ 2020-10-02 15:35 ` Fan Zhang
  2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 1/2] vhost: add backend type in driver start Fan Zhang
  2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation Fan Zhang
  1 sibling, 2 replies; 14+ messages in thread
From: Fan Zhang @ 2020-10-02 15:35 UTC (permalink / raw)
  To: dev
  Cc: chenbo.xia, maxime.coquelin, changpeng.liu, ferruh.yigit, stable,
	Fan Zhang

This patch fixes the problem that vhost crypto cannot be
initialized due to the different requirement between
built-in virtio-net and virtio-crypto. The fix includes
the following change:

- Added new internal enum type virtio_backend_type to
distinguish virtio-net, virtio-crypto, and external
device types.
- Added new API rte_vhost_crypto_driver_start to
distinguish between virtio-net and virtio-crypto built-in
drivers initialization.

v2:
- Removed socket.c setting vhost crypto features.
- Fixed incorrect feature flag.
- Rebased on top of latest master.

Fan Zhang (2):
  vhost: add backend type in driver start
  vhost/crypto: fix feature negotiation

 examples/vhost_crypto/main.c           |  3 +-
 lib/librte_vhost/rte_vhost_crypto.h    | 12 ++++++++
 lib/librte_vhost/rte_vhost_version.map |  1 +
 lib/librte_vhost/socket.c              | 39 +++++++++++++++++++-----
 lib/librte_vhost/vhost_crypto.c        | 41 +++++++++++++++++---------
 lib/librte_vhost/vhost_user.h          |  8 +++++
 6 files changed, 81 insertions(+), 23 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-stable] [dpdk-dev v2 1/2] vhost: add backend type in driver start
  2020-10-02 15:35 ` [dpdk-stable] [dpdk-dev v2 0/2] " Fan Zhang
@ 2020-10-02 15:36   ` Fan Zhang
  2020-10-06  7:53     ` Maxime Coquelin
  2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation Fan Zhang
  1 sibling, 1 reply; 14+ messages in thread
From: Fan Zhang @ 2020-10-02 15:36 UTC (permalink / raw)
  To: dev
  Cc: chenbo.xia, maxime.coquelin, changpeng.liu, ferruh.yigit, stable,
	Fan Zhang

This patch adds an internal driver start function with a newly
added backend type identifier as parameter. With this way
different built-in driver types (net, crypto) can be identified.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 lib/librte_vhost/socket.c     | 39 ++++++++++++++++++++++++++++-------
 lib/librte_vhost/vhost_user.h |  8 +++++++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 0169d3648..a53e536ac 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -38,7 +38,7 @@ struct vhost_user_socket {
 	bool is_server;
 	bool reconnect;
 	bool iommu_support;
-	bool use_builtin_virtio_net;
+	enum virtio_backend_type backend_type;
 	bool extbuf;
 	bool linearbuf;
 	bool async_copy;
@@ -224,7 +224,9 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
 	size = strnlen(vsocket->path, PATH_MAX);
 	vhost_set_ifname(vid, vsocket->path, size);
 
-	vhost_set_builtin_virtio_net(vid, vsocket->use_builtin_virtio_net);
+	vhost_set_builtin_virtio_net(vid,
+			vsocket->backend_type == VIRTIO_DEV_BUILTIN_NET ?
+					true : false);
 
 	vhost_attach_vdpa_device(vid, vsocket->vdpa_dev);
 
@@ -632,7 +634,7 @@ rte_vhost_driver_disable_features(const char *path, uint64_t features)
 	pthread_mutex_lock(&vhost_user.mutex);
 	vsocket = find_vhost_user_socket(path);
 
-	/* Note that use_builtin_virtio_net is not affected by this function
+	/* Note that backend type is not affected by this function
 	 * since callers may want to selectively disable features of the
 	 * built-in vhost net device backend.
 	 */
@@ -681,7 +683,8 @@ rte_vhost_driver_set_features(const char *path, uint64_t features)
 		/* Anyone setting feature bits is implementing their own vhost
 		 * device backend.
 		 */
-		vsocket->use_builtin_virtio_net = false;
+		if (vsocket->backend_type == VIRTIO_DEV_BUILTIN_NET)
+			vsocket->backend_type = VIRTIO_DEV_UNKNOWN;
 	}
 	pthread_mutex_unlock(&vhost_user.mutex);
 
@@ -899,7 +902,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
 	 * rte_vhost_driver_set_features(), which will overwrite following
 	 * two values.
 	 */
-	vsocket->use_builtin_virtio_net = true;
+	vsocket->backend_type = VIRTIO_DEV_BUILTIN_NET;
 	vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES;
 	vsocket->features           = VIRTIO_NET_SUPPORTED_FEATURES;
 	vsocket->protocol_features  = VHOST_USER_PROTOCOL_FEATURES;
@@ -1117,10 +1120,17 @@ vhost_driver_callback_get(const char *path)
 }
 
 int
-rte_vhost_driver_start(const char *path)
+vhost_driver_start(const char *path, enum virtio_backend_type backend_type)
 {
 	struct vhost_user_socket *vsocket;
 	static pthread_t fdset_tid;
+	int ret;
+
+	if (backend_type <= VIRTIO_DEV_UNKNOWN ||
+			backend_type > VIRTIO_DEV_BUILTIN_CRYPTO) {
+		VHOST_LOG_CONFIG(ERR, "Wrong backend type\n");
+		return -1;
+	}
 
 	pthread_mutex_lock(&vhost_user.mutex);
 	vsocket = find_vhost_user_socket(path);
@@ -1153,7 +1163,20 @@ rte_vhost_driver_start(const char *path)
 	}
 
 	if (vsocket->is_server)
-		return vhost_user_start_server(vsocket);
+		ret = vhost_user_start_server(vsocket);
 	else
-		return vhost_user_start_client(vsocket);
+		ret = vhost_user_start_client(vsocket);
+
+	if (ret < 0)
+		return ret;
+
+	vsocket->backend_type = backend_type;
+
+	return 0;
+}
+
+int
+rte_vhost_driver_start(const char *path)
+{
+	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_NET);
 }
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 16fe03f88..9f526e676 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -158,6 +158,12 @@ typedef struct VhostUserMsg {
 /* The version of the protocol we support */
 #define VHOST_USER_VERSION    0x1
 
+/* virtio backend types */
+enum virtio_backend_type {
+	VIRTIO_DEV_UNKNOWN = 0, /* Likely external */
+	VIRTIO_DEV_BUILTIN_NET, /* Virtio-net device */
+	VIRTIO_DEV_BUILTIN_CRYPTO, /* Virtio-crypto device */
+};
 
 /* vhost_user.c */
 int vhost_user_msg_handler(int vid, int fd);
@@ -167,5 +173,7 @@ int vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm);
 int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
 		int *fd_num);
 int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num);
+int vhost_driver_start(const char *path,
+		enum virtio_backend_type backend_type);
 
 #endif
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
  2020-10-02 15:35 ` [dpdk-stable] [dpdk-dev v2 0/2] " Fan Zhang
  2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 1/2] vhost: add backend type in driver start Fan Zhang
@ 2020-10-02 15:36   ` Fan Zhang
  2020-10-06  8:09     ` Maxime Coquelin
                       ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Fan Zhang @ 2020-10-02 15:36 UTC (permalink / raw)
  To: dev
  Cc: chenbo.xia, maxime.coquelin, changpeng.liu, ferruh.yigit, stable,
	Fan Zhang

This patch fixes the feature negotiation for vhost crypto during
initialization. The patch uses the newly created driver start
function to inform the driver type with the fixed vhost features.
In addtion the patch provides a new API specifically used by
the application to start a vhost-crypto driver.

Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
Cc: roy.fan.zhang@intel.com

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 examples/vhost_crypto/main.c           |  3 +-
 lib/librte_vhost/rte_vhost_crypto.h    | 12 ++++++++
 lib/librte_vhost/rte_vhost_version.map |  1 +
 lib/librte_vhost/vhost_crypto.c        | 41 +++++++++++++++++---------
 4 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
index d78fd9b81..11ad49159 100644
--- a/examples/vhost_crypto/main.c
+++ b/examples/vhost_crypto/main.c
@@ -598,7 +598,8 @@ main(int argc, char *argv[])
 			rte_vhost_driver_callback_register(lo->socket_files[j],
 				&virtio_crypto_device_ops);
 
-			ret = rte_vhost_driver_start(lo->socket_files[j]);
+			ret = rte_vhost_crypto_driver_start(
+					lo->socket_files[j]);
 			if (ret < 0)  {
 				RTE_LOG(ERR, USER1, "failed to start vhost.\n");
 				goto error_exit;
diff --git a/lib/librte_vhost/rte_vhost_crypto.h b/lib/librte_vhost/rte_vhost_crypto.h
index b54d61db6..c809c46a2 100644
--- a/lib/librte_vhost/rte_vhost_crypto.h
+++ b/lib/librte_vhost/rte_vhost_crypto.h
@@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
 	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
 };
 
+/**
+ * Start vhost crypto driver
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @return
+ *  0 on success, -1 on failure
+ */
+__rte_experimental
+int
+rte_vhost_crypto_driver_start(const char *path);
+
 /**
  *  Create Vhost-crypto instance
  *
diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 55e98e557..9183d6f2f 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -55,6 +55,7 @@ EXPERIMENTAL {
 	rte_vhost_driver_get_protocol_features;
 	rte_vhost_driver_get_queue_num;
 	rte_vhost_crypto_create;
+	rte_vhost_crypto_driver_start;
 	rte_vhost_crypto_free;
 	rte_vhost_crypto_fetch_requests;
 	rte_vhost_crypto_finalize_requests;
diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index e08f9c6d7..6195958d2 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -35,13 +35,12 @@
 #define VC_LOG_DBG(fmt, args...)
 #endif
 
-#define VIRTIO_CRYPTO_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
-		(1 << VIRTIO_RING_F_INDIRECT_DESC) |			\
-		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
-		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |			\
-		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |			\
-		(1 << VIRTIO_NET_F_CTRL_VQ) |				\
-		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
+#define VIRTIO_CRYPTO_FEATURES ((1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
+		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |			\
+		(1ULL << VIRTIO_RING_F_EVENT_IDX) |			\
+		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
+		(1ULL << VIRTIO_F_VERSION_1) |				\
+		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
 
 #define IOVA_TO_VVA(t, r, a, l, p)					\
 	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p))
@@ -1400,6 +1399,27 @@ vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
 	return processed;
 }
 
+int
+rte_vhost_crypto_driver_start(const char *path)
+{
+	uint64_t protocol_features;
+	int ret;
+
+	ret = rte_vhost_driver_set_features(path, VIRTIO_CRYPTO_FEATURES);
+	if (ret)
+		return -1;
+
+	ret = rte_vhost_driver_get_protocol_features(path, &protocol_features);
+	if (ret)
+		return -1;
+	protocol_features |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
+	ret = rte_vhost_driver_set_protocol_features(path, protocol_features);
+	if (ret)
+		return -1;
+
+	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);
+}
+
 int
 rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
 		struct rte_mempool *sess_pool,
@@ -1417,13 +1437,6 @@ rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
 		return -EINVAL;
 	}
 
-	ret = rte_vhost_driver_set_features(dev->ifname,
-			VIRTIO_CRYPTO_FEATURES);
-	if (ret < 0) {
-		VC_LOG_ERR("Error setting features");
-		return -1;
-	}
-
 	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
 			RTE_CACHE_LINE_SIZE, socket_id);
 	if (!vcrypto) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev v2 1/2] vhost: add backend type in driver start
  2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 1/2] vhost: add backend type in driver start Fan Zhang
@ 2020-10-06  7:53     ` Maxime Coquelin
  2020-10-06  8:56       ` Zhang, Roy Fan
  0 siblings, 1 reply; 14+ messages in thread
From: Maxime Coquelin @ 2020-10-06  7:53 UTC (permalink / raw)
  To: Fan Zhang, dev; +Cc: chenbo.xia, changpeng.liu, ferruh.yigit, stable

Hi Fan,

On 10/2/20 5:36 PM, Fan Zhang wrote:
> This patch adds an internal driver start function with a newly
> added backend type identifier as parameter. With this way
> different built-in driver types (net, crypto) can be identified.
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  lib/librte_vhost/socket.c     | 39 ++++++++++++++++++++++++++++-------
>  lib/librte_vhost/vhost_user.h |  8 +++++++
>  2 files changed, 39 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 0169d3648..a53e536ac 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -38,7 +38,7 @@ struct vhost_user_socket {
>  	bool is_server;
>  	bool reconnect;
>  	bool iommu_support;
> -	bool use_builtin_virtio_net;
> +	enum virtio_backend_type backend_type;
>  	bool extbuf;
>  	bool linearbuf;
>  	bool async_copy;
> @@ -224,7 +224,9 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
>  	size = strnlen(vsocket->path, PATH_MAX);
>  	vhost_set_ifname(vid, vsocket->path, size);
>  
> -	vhost_set_builtin_virtio_net(vid, vsocket->use_builtin_virtio_net);
> +	vhost_set_builtin_virtio_net(vid,
> +			vsocket->backend_type == VIRTIO_DEV_BUILTIN_NET ?
> +					true : false);
>  
>  	vhost_attach_vdpa_device(vid, vsocket->vdpa_dev);
>  
> @@ -632,7 +634,7 @@ rte_vhost_driver_disable_features(const char *path, uint64_t features)
>  	pthread_mutex_lock(&vhost_user.mutex);
>  	vsocket = find_vhost_user_socket(path);
>  
> -	/* Note that use_builtin_virtio_net is not affected by this function
> +	/* Note that backend type is not affected by this function
>  	 * since callers may want to selectively disable features of the
>  	 * built-in vhost net device backend.
>  	 */
> @@ -681,7 +683,8 @@ rte_vhost_driver_set_features(const char *path, uint64_t features)
>  		/* Anyone setting feature bits is implementing their own vhost
>  		 * device backend.
>  		 */
> -		vsocket->use_builtin_virtio_net = false;
> +		if (vsocket->backend_type == VIRTIO_DEV_BUILTIN_NET)
> +			vsocket->backend_type = VIRTIO_DEV_UNKNOWN;
>  	}
>  	pthread_mutex_unlock(&vhost_user.mutex);
>  
> @@ -899,7 +902,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
>  	 * rte_vhost_driver_set_features(), which will overwrite following
>  	 * two values.
>  	 */
> -	vsocket->use_builtin_virtio_net = true;
> +	vsocket->backend_type = VIRTIO_DEV_BUILTIN_NET;
>  	vsocket->supported_features = VIRTIO_NET_SUPPORTED_FEATURES;
>  	vsocket->features           = VIRTIO_NET_SUPPORTED_FEATURES;
>  	vsocket->protocol_features  = VHOST_USER_PROTOCOL_FEATURES;
> @@ -1117,10 +1120,17 @@ vhost_driver_callback_get(const char *path)
>  }
>  
>  int
> -rte_vhost_driver_start(const char *path)
> +vhost_driver_start(const char *path, enum virtio_backend_type backend_type)
>  {
>  	struct vhost_user_socket *vsocket;
>  	static pthread_t fdset_tid;
> +	int ret;
> +
> +	if (backend_type <= VIRTIO_DEV_UNKNOWN ||
> +			backend_type > VIRTIO_DEV_BUILTIN_CRYPTO) {
> +		VHOST_LOG_CONFIG(ERR, "Wrong backend type\n");
> +		return -1;
> +	}
>  
>  	pthread_mutex_lock(&vhost_user.mutex);
>  	vsocket = find_vhost_user_socket(path);
> @@ -1153,7 +1163,20 @@ rte_vhost_driver_start(const char *path)
>  	}
>  
>  	if (vsocket->is_server)
> -		return vhost_user_start_server(vsocket);
> +		ret = vhost_user_start_server(vsocket);
>  	else
> -		return vhost_user_start_client(vsocket);
> +		ret = vhost_user_start_client(vsocket);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	vsocket->backend_type = backend_type;
> +
> +	return 0;

Looking at it again, I think patch 1 is not necessary.
Patch 2 should be enough and would be easier to backport.

What do you think?

Regards,
Maxime

> +}
> +
> +int
> +rte_vhost_driver_start(const char *path)
> +{
> +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_NET);
>  }
> diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
> index 16fe03f88..9f526e676 100644
> --- a/lib/librte_vhost/vhost_user.h
> +++ b/lib/librte_vhost/vhost_user.h
> @@ -158,6 +158,12 @@ typedef struct VhostUserMsg {
>  /* The version of the protocol we support */
>  #define VHOST_USER_VERSION    0x1
>  
> +/* virtio backend types */
> +enum virtio_backend_type {
> +	VIRTIO_DEV_UNKNOWN = 0, /* Likely external */
> +	VIRTIO_DEV_BUILTIN_NET, /* Virtio-net device */
> +	VIRTIO_DEV_BUILTIN_CRYPTO, /* Virtio-crypto device */
> +};
>  
>  /* vhost_user.c */
>  int vhost_user_msg_handler(int vid, int fd);
> @@ -167,5 +173,7 @@ int vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm);
>  int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
>  		int *fd_num);
>  int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num);
> +int vhost_driver_start(const char *path,
> +		enum virtio_backend_type backend_type);
>  
>  #endif
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
  2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation Fan Zhang
@ 2020-10-06  8:09     ` Maxime Coquelin
  2020-10-06  8:37       ` Zhang, Roy Fan
  2020-10-09  6:39     ` [dpdk-stable] " Maxime Coquelin
  2020-10-09  7:24     ` Maxime Coquelin
  2 siblings, 1 reply; 14+ messages in thread
From: Maxime Coquelin @ 2020-10-06  8:09 UTC (permalink / raw)
  To: Fan Zhang, dev; +Cc: chenbo.xia, changpeng.liu, ferruh.yigit, stable



On 10/2/20 5:36 PM, Fan Zhang wrote:
> This patch fixes the feature negotiation for vhost crypto during
> initialization. The patch uses the newly created driver start
> function to inform the driver type with the fixed vhost features.
> In addtion the patch provides a new API specifically used by
> the application to start a vhost-crypto driver.
> 
> Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
> Cc: roy.fan.zhang@intel.com
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  examples/vhost_crypto/main.c           |  3 +-
>  lib/librte_vhost/rte_vhost_crypto.h    | 12 ++++++++
>  lib/librte_vhost/rte_vhost_version.map |  1 +
>  lib/librte_vhost/vhost_crypto.c        | 41 +++++++++++++++++---------
>  4 files changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
> index d78fd9b81..11ad49159 100644
> --- a/examples/vhost_crypto/main.c
> +++ b/examples/vhost_crypto/main.c
> @@ -598,7 +598,8 @@ main(int argc, char *argv[])
>  			rte_vhost_driver_callback_register(lo->socket_files[j],
>  				&virtio_crypto_device_ops);
>  
> -			ret = rte_vhost_driver_start(lo->socket_files[j]);
> +			ret = rte_vhost_crypto_driver_start(
> +					lo->socket_files[j]);
>  			if (ret < 0)  {
>  				RTE_LOG(ERR, USER1, "failed to start vhost.\n");
>  				goto error_exit;
> diff --git a/lib/librte_vhost/rte_vhost_crypto.h b/lib/librte_vhost/rte_vhost_crypto.h
> index b54d61db6..c809c46a2 100644
> --- a/lib/librte_vhost/rte_vhost_crypto.h
> +++ b/lib/librte_vhost/rte_vhost_crypto.h
> @@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
>  	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
>  };
>  
> +/**
> + * Start vhost crypto driver
> + *
> + * @param path
> + *  The vhost-user socket file path
> + * @return
> + *  0 on success, -1 on failure
> + */
> +__rte_experimental
> +int
> +rte_vhost_crypto_driver_start(const char *path);
> +
>  /**
>   *  Create Vhost-crypto instance
>   *
> diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
> index 55e98e557..9183d6f2f 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -55,6 +55,7 @@ EXPERIMENTAL {
>  	rte_vhost_driver_get_protocol_features;
>  	rte_vhost_driver_get_queue_num;
>  	rte_vhost_crypto_create;
> +	rte_vhost_crypto_driver_start;
>  	rte_vhost_crypto_free;
>  	rte_vhost_crypto_fetch_requests;
>  	rte_vhost_crypto_finalize_requests;
> diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
> index e08f9c6d7..6195958d2 100644
> --- a/lib/librte_vhost/vhost_crypto.c
> +++ b/lib/librte_vhost/vhost_crypto.c
> @@ -35,13 +35,12 @@
>  #define VC_LOG_DBG(fmt, args...)
>  #endif
>  
> -#define VIRTIO_CRYPTO_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> -		(1 << VIRTIO_RING_F_INDIRECT_DESC) |			\
> -		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
> -		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |			\
> -		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |			\
> -		(1 << VIRTIO_NET_F_CTRL_VQ) |				\
> -		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
> +#define VIRTIO_CRYPTO_FEATURES ((1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> +		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |			\
> +		(1ULL << VIRTIO_RING_F_EVENT_IDX) |			\
> +		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
> +		(1ULL << VIRTIO_F_VERSION_1) |				\
> +		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
>  
>  #define IOVA_TO_VVA(t, r, a, l, p)					\
>  	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p))
> @@ -1400,6 +1399,27 @@ vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
>  	return processed;
>  }
>  
> +int
> +rte_vhost_crypto_driver_start(const char *path)
> +{
> +	uint64_t protocol_features;
> +	int ret;
> +
> +	ret = rte_vhost_driver_set_features(path, VIRTIO_CRYPTO_FEATURES);
> +	if (ret)
> +		return -1;

As rte_vhost_driver_set_features is now called on time,
use_builtin_virtio_net is set to false before the connection is
established.

So it should be enough.

> +	ret = rte_vhost_driver_get_protocol_features(path, &protocol_features);
> +	if (ret)
> +		return -1;
> +	protocol_features |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> +	ret = rte_vhost_driver_set_protocol_features(path, protocol_features);
> +	if (ret)
> +		return -1;
> +
> +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);

We just have to remove the extra param.

I will to the change and apply it today so that you can test.

Thanks,
Maxime

> +}
> +
>  int
>  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>  		struct rte_mempool *sess_pool,
> @@ -1417,13 +1437,6 @@ rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>  		return -EINVAL;
>  	}
>  
> -	ret = rte_vhost_driver_set_features(dev->ifname,
> -			VIRTIO_CRYPTO_FEATURES);
> -	if (ret < 0) {
> -		VC_LOG_ERR("Error setting features");
> -		return -1;
> -	}
> -
>  	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
>  			RTE_CACHE_LINE_SIZE, socket_id);
>  	if (!vcrypto) {
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
  2020-10-06  8:09     ` Maxime Coquelin
@ 2020-10-06  8:37       ` Zhang, Roy Fan
  2020-10-09  7:36         ` Maxime Coquelin
  0 siblings, 1 reply; 14+ messages in thread
From: Zhang, Roy Fan @ 2020-10-06  8:37 UTC (permalink / raw)
  To: Maxime Coquelin, dev; +Cc: Xia, Chenbo, Liu, Changpeng, Yigit, Ferruh, stable

Hi Maxime,

Thanks I will verify it after you applied the patch.

Regards,
Fan

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, October 6, 2020 9:10 AM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Liu, Changpeng
> <changpeng.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
> 
> 
> 
> On 10/2/20 5:36 PM, Fan Zhang wrote:
> > This patch fixes the feature negotiation for vhost crypto during
> > initialization. The patch uses the newly created driver start
> > function to inform the driver type with the fixed vhost features.
> > In addtion the patch provides a new API specifically used by
> > the application to start a vhost-crypto driver.
> >
> > Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
> > Cc: roy.fan.zhang@intel.com
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> >  examples/vhost_crypto/main.c           |  3 +-
> >  lib/librte_vhost/rte_vhost_crypto.h    | 12 ++++++++
> >  lib/librte_vhost/rte_vhost_version.map |  1 +
> >  lib/librte_vhost/vhost_crypto.c        | 41 +++++++++++++++++---------
> >  4 files changed, 42 insertions(+), 15 deletions(-)
> >
> > diff --git a/examples/vhost_crypto/main.c
> b/examples/vhost_crypto/main.c
> > index d78fd9b81..11ad49159 100644
> > --- a/examples/vhost_crypto/main.c
> > +++ b/examples/vhost_crypto/main.c
> > @@ -598,7 +598,8 @@ main(int argc, char *argv[])
> >  			rte_vhost_driver_callback_register(lo-
> >socket_files[j],
> >  				&virtio_crypto_device_ops);
> >
> > -			ret = rte_vhost_driver_start(lo->socket_files[j]);
> > +			ret = rte_vhost_crypto_driver_start(
> > +					lo->socket_files[j]);
> >  			if (ret < 0)  {
> >  				RTE_LOG(ERR, USER1, "failed to start
> vhost.\n");
> >  				goto error_exit;
> > diff --git a/lib/librte_vhost/rte_vhost_crypto.h
> b/lib/librte_vhost/rte_vhost_crypto.h
> > index b54d61db6..c809c46a2 100644
> > --- a/lib/librte_vhost/rte_vhost_crypto.h
> > +++ b/lib/librte_vhost/rte_vhost_crypto.h
> > @@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
> >  	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
> >  };
> >
> > +/**
> > + * Start vhost crypto driver
> > + *
> > + * @param path
> > + *  The vhost-user socket file path
> > + * @return
> > + *  0 on success, -1 on failure
> > + */
> > +__rte_experimental
> > +int
> > +rte_vhost_crypto_driver_start(const char *path);
> > +
> >  /**
> >   *  Create Vhost-crypto instance
> >   *
> > diff --git a/lib/librte_vhost/rte_vhost_version.map
> b/lib/librte_vhost/rte_vhost_version.map
> > index 55e98e557..9183d6f2f 100644
> > --- a/lib/librte_vhost/rte_vhost_version.map
> > +++ b/lib/librte_vhost/rte_vhost_version.map
> > @@ -55,6 +55,7 @@ EXPERIMENTAL {
> >  	rte_vhost_driver_get_protocol_features;
> >  	rte_vhost_driver_get_queue_num;
> >  	rte_vhost_crypto_create;
> > +	rte_vhost_crypto_driver_start;
> >  	rte_vhost_crypto_free;
> >  	rte_vhost_crypto_fetch_requests;
> >  	rte_vhost_crypto_finalize_requests;
> > diff --git a/lib/librte_vhost/vhost_crypto.c
> b/lib/librte_vhost/vhost_crypto.c
> > index e08f9c6d7..6195958d2 100644
> > --- a/lib/librte_vhost/vhost_crypto.c
> > +++ b/lib/librte_vhost/vhost_crypto.c
> > @@ -35,13 +35,12 @@
> >  #define VC_LOG_DBG(fmt, args...)
> >  #endif
> >
> > -#define VIRTIO_CRYPTO_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY)
> |	\
> > -		(1 << VIRTIO_RING_F_INDIRECT_DESC) |
> 	\
> > -		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
> > -		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |
> 	\
> > -		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |
> 	\
> > -		(1 << VIRTIO_NET_F_CTRL_VQ) |
> 	\
> > -		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
> > +#define VIRTIO_CRYPTO_FEATURES ((1ULL <<
> VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> > +		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
> 	\
> > +		(1ULL << VIRTIO_RING_F_EVENT_IDX) |
> 	\
> > +		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
> > +		(1ULL << VIRTIO_F_VERSION_1) |
> 	\
> > +		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
> >
> >  #define IOVA_TO_VVA(t, r, a, l, p)					\
> >  	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p))
> > @@ -1400,6 +1399,27 @@
> vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
> >  	return processed;
> >  }
> >
> > +int
> > +rte_vhost_crypto_driver_start(const char *path)
> > +{
> > +	uint64_t protocol_features;
> > +	int ret;
> > +
> > +	ret = rte_vhost_driver_set_features(path,
> VIRTIO_CRYPTO_FEATURES);
> > +	if (ret)
> > +		return -1;
> 
> As rte_vhost_driver_set_features is now called on time,
> use_builtin_virtio_net is set to false before the connection is
> established.
> 
> So it should be enough.
> 
> > +	ret = rte_vhost_driver_get_protocol_features(path,
> &protocol_features);
> > +	if (ret)
> > +		return -1;
> > +	protocol_features |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> > +	ret = rte_vhost_driver_set_protocol_features(path,
> protocol_features);
> > +	if (ret)
> > +		return -1;
> > +
> > +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);
> 
> We just have to remove the extra param.
> 
> I will to the change and apply it today so that you can test.
> 
> Thanks,
> Maxime
> 
> > +}
> > +
> >  int
> >  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
> >  		struct rte_mempool *sess_pool,
> > @@ -1417,13 +1437,6 @@ rte_vhost_crypto_create(int vid, uint8_t
> cryptodev_id,
> >  		return -EINVAL;
> >  	}
> >
> > -	ret = rte_vhost_driver_set_features(dev->ifname,
> > -			VIRTIO_CRYPTO_FEATURES);
> > -	if (ret < 0) {
> > -		VC_LOG_ERR("Error setting features");
> > -		return -1;
> > -	}
> > -
> >  	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
> >  			RTE_CACHE_LINE_SIZE, socket_id);
> >  	if (!vcrypto) {
> >


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev v2 1/2] vhost: add backend type in driver start
  2020-10-06  7:53     ` Maxime Coquelin
@ 2020-10-06  8:56       ` Zhang, Roy Fan
  0 siblings, 0 replies; 14+ messages in thread
From: Zhang, Roy Fan @ 2020-10-06  8:56 UTC (permalink / raw)
  To: Maxime Coquelin, dev; +Cc: Xia, Chenbo, Liu, Changpeng, Yigit, Ferruh, stable

Hi Maxime,

I think you are right :-)

Regards,
Fan

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, October 6, 2020 8:54 AM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Liu, Changpeng
> <changpeng.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev v2 1/2] vhost: add backend type in driver start
> 
> Hi Fan,
> 
> On 10/2/20 5:36 PM, Fan Zhang wrote:
> > This patch adds an internal driver start function with a newly
> > added backend type identifier as parameter. With this way
> > different built-in driver types (net, crypto) can be identified.
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> >  lib/librte_vhost/socket.c     | 39 ++++++++++++++++++++++++++++-------
> >  lib/librte_vhost/vhost_user.h |  8 +++++++
> >  2 files changed, 39 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> > index 0169d3648..a53e536ac 100644
> > --- a/lib/librte_vhost/socket.c
> > +++ b/lib/librte_vhost/socket.c
> > @@ -38,7 +38,7 @@ struct vhost_user_socket {
> >  	bool is_server;
> >  	bool reconnect;
> >  	bool iommu_support;
> > -	bool use_builtin_virtio_net;
> > +	enum virtio_backend_type backend_type;
> >  	bool extbuf;
> >  	bool linearbuf;
> >  	bool async_copy;
> > @@ -224,7 +224,9 @@ vhost_user_add_connection(int fd, struct
> vhost_user_socket *vsocket)
> >  	size = strnlen(vsocket->path, PATH_MAX);
> >  	vhost_set_ifname(vid, vsocket->path, size);
> >
> > -	vhost_set_builtin_virtio_net(vid, vsocket->use_builtin_virtio_net);
> > +	vhost_set_builtin_virtio_net(vid,
> > +			vsocket->backend_type ==
> VIRTIO_DEV_BUILTIN_NET ?
> > +					true : false);
> >
> >  	vhost_attach_vdpa_device(vid, vsocket->vdpa_dev);
> >
> > @@ -632,7 +634,7 @@ rte_vhost_driver_disable_features(const char
> *path, uint64_t features)
> >  	pthread_mutex_lock(&vhost_user.mutex);
> >  	vsocket = find_vhost_user_socket(path);
> >
> > -	/* Note that use_builtin_virtio_net is not affected by this function
> > +	/* Note that backend type is not affected by this function
> >  	 * since callers may want to selectively disable features of the
> >  	 * built-in vhost net device backend.
> >  	 */
> > @@ -681,7 +683,8 @@ rte_vhost_driver_set_features(const char *path,
> uint64_t features)
> >  		/* Anyone setting feature bits is implementing their own
> vhost
> >  		 * device backend.
> >  		 */
> > -		vsocket->use_builtin_virtio_net = false;
> > +		if (vsocket->backend_type == VIRTIO_DEV_BUILTIN_NET)
> > +			vsocket->backend_type = VIRTIO_DEV_UNKNOWN;
> >  	}
> >  	pthread_mutex_unlock(&vhost_user.mutex);
> >
> > @@ -899,7 +902,7 @@ rte_vhost_driver_register(const char *path,
> uint64_t flags)
> >  	 * rte_vhost_driver_set_features(), which will overwrite following
> >  	 * two values.
> >  	 */
> > -	vsocket->use_builtin_virtio_net = true;
> > +	vsocket->backend_type = VIRTIO_DEV_BUILTIN_NET;
> >  	vsocket->supported_features =
> VIRTIO_NET_SUPPORTED_FEATURES;
> >  	vsocket->features           = VIRTIO_NET_SUPPORTED_FEATURES;
> >  	vsocket->protocol_features  = VHOST_USER_PROTOCOL_FEATURES;
> > @@ -1117,10 +1120,17 @@ vhost_driver_callback_get(const char *path)
> >  }
> >
> >  int
> > -rte_vhost_driver_start(const char *path)
> > +vhost_driver_start(const char *path, enum virtio_backend_type
> backend_type)
> >  {
> >  	struct vhost_user_socket *vsocket;
> >  	static pthread_t fdset_tid;
> > +	int ret;
> > +
> > +	if (backend_type <= VIRTIO_DEV_UNKNOWN ||
> > +			backend_type > VIRTIO_DEV_BUILTIN_CRYPTO) {
> > +		VHOST_LOG_CONFIG(ERR, "Wrong backend type\n");
> > +		return -1;
> > +	}
> >
> >  	pthread_mutex_lock(&vhost_user.mutex);
> >  	vsocket = find_vhost_user_socket(path);
> > @@ -1153,7 +1163,20 @@ rte_vhost_driver_start(const char *path)
> >  	}
> >
> >  	if (vsocket->is_server)
> > -		return vhost_user_start_server(vsocket);
> > +		ret = vhost_user_start_server(vsocket);
> >  	else
> > -		return vhost_user_start_client(vsocket);
> > +		ret = vhost_user_start_client(vsocket);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	vsocket->backend_type = backend_type;
> > +
> > +	return 0;
> 
> Looking at it again, I think patch 1 is not necessary.
> Patch 2 should be enough and would be easier to backport.
> 
> What do you think?
> 
> Regards,
> Maxime
> 
> > +}
> > +
> > +int
> > +rte_vhost_driver_start(const char *path)
> > +{
> > +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_NET);
> >  }
> > diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
> > index 16fe03f88..9f526e676 100644
> > --- a/lib/librte_vhost/vhost_user.h
> > +++ b/lib/librte_vhost/vhost_user.h
> > @@ -158,6 +158,12 @@ typedef struct VhostUserMsg {
> >  /* The version of the protocol we support */
> >  #define VHOST_USER_VERSION    0x1
> >
> > +/* virtio backend types */
> > +enum virtio_backend_type {
> > +	VIRTIO_DEV_UNKNOWN = 0, /* Likely external */
> > +	VIRTIO_DEV_BUILTIN_NET, /* Virtio-net device */
> > +	VIRTIO_DEV_BUILTIN_CRYPTO, /* Virtio-crypto device */
> > +};
> >
> >  /* vhost_user.c */
> >  int vhost_user_msg_handler(int vid, int fd);
> > @@ -167,5 +173,7 @@ int vhost_user_iotlb_miss(struct virtio_net *dev,
> uint64_t iova, uint8_t perm);
> >  int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int
> max_fds,
> >  		int *fd_num);
> >  int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int
> fd_num);
> > +int vhost_driver_start(const char *path,
> > +		enum virtio_backend_type backend_type);
> >
> >  #endif
> >


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
  2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation Fan Zhang
  2020-10-06  8:09     ` Maxime Coquelin
@ 2020-10-09  6:39     ` Maxime Coquelin
  2020-10-09  7:24     ` Maxime Coquelin
  2 siblings, 0 replies; 14+ messages in thread
From: Maxime Coquelin @ 2020-10-09  6:39 UTC (permalink / raw)
  To: Fan Zhang, dev; +Cc: chenbo.xia, changpeng.liu, ferruh.yigit, stable



On 10/2/20 5:36 PM, Fan Zhang wrote:
> This patch fixes the feature negotiation for vhost crypto during
> initialization. The patch uses the newly created driver start
> function to inform the driver type with the fixed vhost features.
> In addtion the patch provides a new API specifically used by
> the application to start a vhost-crypto driver.
> 
> Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
> Cc: roy.fan.zhang@intel.com
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  examples/vhost_crypto/main.c           |  3 +-
>  lib/librte_vhost/rte_vhost_crypto.h    | 12 ++++++++
>  lib/librte_vhost/rte_vhost_version.map |  1 +
>  lib/librte_vhost/vhost_crypto.c        | 41 +++++++++++++++++---------
>  4 files changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
> index d78fd9b81..11ad49159 100644
> --- a/examples/vhost_crypto/main.c
> +++ b/examples/vhost_crypto/main.c
> @@ -598,7 +598,8 @@ main(int argc, char *argv[])
>  			rte_vhost_driver_callback_register(lo->socket_files[j],
>  				&virtio_crypto_device_ops);
>  
> -			ret = rte_vhost_driver_start(lo->socket_files[j]);
> +			ret = rte_vhost_crypto_driver_start(
> +					lo->socket_files[j]);
>  			if (ret < 0)  {
>  				RTE_LOG(ERR, USER1, "failed to start vhost.\n");
>  				goto error_exit;
> diff --git a/lib/librte_vhost/rte_vhost_crypto.h b/lib/librte_vhost/rte_vhost_crypto.h
> index b54d61db6..c809c46a2 100644
> --- a/lib/librte_vhost/rte_vhost_crypto.h
> +++ b/lib/librte_vhost/rte_vhost_crypto.h
> @@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
>  	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
>  };
>  
> +/**
> + * Start vhost crypto driver
> + *
> + * @param path
> + *  The vhost-user socket file path
> + * @return
> + *  0 on success, -1 on failure
> + */
> +__rte_experimental
> +int
> +rte_vhost_crypto_driver_start(const char *path);
> +
>  /**
>   *  Create Vhost-crypto instance
>   *
> diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
> index 55e98e557..9183d6f2f 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -55,6 +55,7 @@ EXPERIMENTAL {
>  	rte_vhost_driver_get_protocol_features;
>  	rte_vhost_driver_get_queue_num;
>  	rte_vhost_crypto_create;
> +	rte_vhost_crypto_driver_start;
>  	rte_vhost_crypto_free;
>  	rte_vhost_crypto_fetch_requests;
>  	rte_vhost_crypto_finalize_requests;
> diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
> index e08f9c6d7..6195958d2 100644
> --- a/lib/librte_vhost/vhost_crypto.c
> +++ b/lib/librte_vhost/vhost_crypto.c
> @@ -35,13 +35,12 @@
>  #define VC_LOG_DBG(fmt, args...)
>  #endif
>  
> -#define VIRTIO_CRYPTO_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> -		(1 << VIRTIO_RING_F_INDIRECT_DESC) |			\
> -		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
> -		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |			\
> -		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |			\
> -		(1 << VIRTIO_NET_F_CTRL_VQ) |				\
> -		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
> +#define VIRTIO_CRYPTO_FEATURES ((1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> +		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |			\
> +		(1ULL << VIRTIO_RING_F_EVENT_IDX) |			\
> +		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
> +		(1ULL << VIRTIO_F_VERSION_1) |				\
> +		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
>  
>  #define IOVA_TO_VVA(t, r, a, l, p)					\
>  	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p))
> @@ -1400,6 +1399,27 @@ vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
>  	return processed;
>  }
>  
> +int
> +rte_vhost_crypto_driver_start(const char *path)
> +{
> +	uint64_t protocol_features;
> +	int ret;
> +
> +	ret = rte_vhost_driver_set_features(path, VIRTIO_CRYPTO_FEATURES);
> +	if (ret)
> +		return -1;
> +
> +	ret = rte_vhost_driver_get_protocol_features(path, &protocol_features);
> +	if (ret)
> +		return -1;
> +	protocol_features |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> +	ret = rte_vhost_driver_set_protocol_features(path, protocol_features);
> +	if (ret)
> +		return -1;
> +
> +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);
> +}
> +
>  int
>  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>  		struct rte_mempool *sess_pool,
> @@ -1417,13 +1437,6 @@ rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>  		return -EINVAL;
>  	}
>  
> -	ret = rte_vhost_driver_set_features(dev->ifname,
> -			VIRTIO_CRYPTO_FEATURES);
> -	if (ret < 0) {
> -		VC_LOG_ERR("Error setting features");
> -		return -1;
> -	}
> -
>  	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
>  			RTE_CACHE_LINE_SIZE, socket_id);
>  	if (!vcrypto) {
> 

Only for this patch:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
  2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation Fan Zhang
  2020-10-06  8:09     ` Maxime Coquelin
  2020-10-09  6:39     ` [dpdk-stable] " Maxime Coquelin
@ 2020-10-09  7:24     ` Maxime Coquelin
  2 siblings, 0 replies; 14+ messages in thread
From: Maxime Coquelin @ 2020-10-09  7:24 UTC (permalink / raw)
  To: Fan Zhang, dev; +Cc: chenbo.xia, changpeng.liu, ferruh.yigit, stable



On 10/2/20 5:36 PM, Fan Zhang wrote:
> This patch fixes the feature negotiation for vhost crypto during
> initialization. The patch uses the newly created driver start
> function to inform the driver type with the fixed vhost features.
> In addtion the patch provides a new API specifically used by
> the application to start a vhost-crypto driver.
> 
> Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
> Cc: roy.fan.zhang@intel.com
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  examples/vhost_crypto/main.c           |  3 +-
>  lib/librte_vhost/rte_vhost_crypto.h    | 12 ++++++++
>  lib/librte_vhost/rte_vhost_version.map |  1 +
>  lib/librte_vhost/vhost_crypto.c        | 41 +++++++++++++++++---------
>  4 files changed, 42 insertions(+), 15 deletions(-)


Applied to dpdk-next-virtio/main.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
  2020-10-06  8:37       ` Zhang, Roy Fan
@ 2020-10-09  7:36         ` Maxime Coquelin
  2020-10-10  9:28           ` [dpdk-stable] [dpdk-dev] " Jiang, YuX
  0 siblings, 1 reply; 14+ messages in thread
From: Maxime Coquelin @ 2020-10-09  7:36 UTC (permalink / raw)
  To: Zhang, Roy Fan, dev; +Cc: Xia, Chenbo, Liu, Changpeng, Yigit, Ferruh, stable



On 10/6/20 10:37 AM, Zhang, Roy Fan wrote:
> Hi Maxime,
> 
> Thanks I will verify it after you applied the patch.


Thanks,
Your patch is now on dpdk-next-virtio/main.
I would be glad if you could test it before Ferruh merges it.

Thanks,
Maxime
> Regards,
> Fan
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, October 6, 2020 9:10 AM
>> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
>> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Liu, Changpeng
>> <changpeng.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
>> stable@dpdk.org
>> Subject: Re: [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
>>
>>
>>
>> On 10/2/20 5:36 PM, Fan Zhang wrote:
>>> This patch fixes the feature negotiation for vhost crypto during
>>> initialization. The patch uses the newly created driver start
>>> function to inform the driver type with the fixed vhost features.
>>> In addtion the patch provides a new API specifically used by
>>> the application to start a vhost-crypto driver.
>>>
>>> Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
>>> Cc: roy.fan.zhang@intel.com
>>>
>>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
>>> ---
>>>  examples/vhost_crypto/main.c           |  3 +-
>>>  lib/librte_vhost/rte_vhost_crypto.h    | 12 ++++++++
>>>  lib/librte_vhost/rte_vhost_version.map |  1 +
>>>  lib/librte_vhost/vhost_crypto.c        | 41 +++++++++++++++++---------
>>>  4 files changed, 42 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/examples/vhost_crypto/main.c
>> b/examples/vhost_crypto/main.c
>>> index d78fd9b81..11ad49159 100644
>>> --- a/examples/vhost_crypto/main.c
>>> +++ b/examples/vhost_crypto/main.c
>>> @@ -598,7 +598,8 @@ main(int argc, char *argv[])
>>>  			rte_vhost_driver_callback_register(lo-
>>> socket_files[j],
>>>  				&virtio_crypto_device_ops);
>>>
>>> -			ret = rte_vhost_driver_start(lo->socket_files[j]);
>>> +			ret = rte_vhost_crypto_driver_start(
>>> +					lo->socket_files[j]);
>>>  			if (ret < 0)  {
>>>  				RTE_LOG(ERR, USER1, "failed to start
>> vhost.\n");
>>>  				goto error_exit;
>>> diff --git a/lib/librte_vhost/rte_vhost_crypto.h
>> b/lib/librte_vhost/rte_vhost_crypto.h
>>> index b54d61db6..c809c46a2 100644
>>> --- a/lib/librte_vhost/rte_vhost_crypto.h
>>> +++ b/lib/librte_vhost/rte_vhost_crypto.h
>>> @@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
>>>  	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
>>>  };
>>>
>>> +/**
>>> + * Start vhost crypto driver
>>> + *
>>> + * @param path
>>> + *  The vhost-user socket file path
>>> + * @return
>>> + *  0 on success, -1 on failure
>>> + */
>>> +__rte_experimental
>>> +int
>>> +rte_vhost_crypto_driver_start(const char *path);
>>> +
>>>  /**
>>>   *  Create Vhost-crypto instance
>>>   *
>>> diff --git a/lib/librte_vhost/rte_vhost_version.map
>> b/lib/librte_vhost/rte_vhost_version.map
>>> index 55e98e557..9183d6f2f 100644
>>> --- a/lib/librte_vhost/rte_vhost_version.map
>>> +++ b/lib/librte_vhost/rte_vhost_version.map
>>> @@ -55,6 +55,7 @@ EXPERIMENTAL {
>>>  	rte_vhost_driver_get_protocol_features;
>>>  	rte_vhost_driver_get_queue_num;
>>>  	rte_vhost_crypto_create;
>>> +	rte_vhost_crypto_driver_start;
>>>  	rte_vhost_crypto_free;
>>>  	rte_vhost_crypto_fetch_requests;
>>>  	rte_vhost_crypto_finalize_requests;
>>> diff --git a/lib/librte_vhost/vhost_crypto.c
>> b/lib/librte_vhost/vhost_crypto.c
>>> index e08f9c6d7..6195958d2 100644
>>> --- a/lib/librte_vhost/vhost_crypto.c
>>> +++ b/lib/librte_vhost/vhost_crypto.c
>>> @@ -35,13 +35,12 @@
>>>  #define VC_LOG_DBG(fmt, args...)
>>>  #endif
>>>
>>> -#define VIRTIO_CRYPTO_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY)
>> |	\
>>> -		(1 << VIRTIO_RING_F_INDIRECT_DESC) |
>> 	\
>>> -		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
>>> -		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |
>> 	\
>>> -		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |
>> 	\
>>> -		(1 << VIRTIO_NET_F_CTRL_VQ) |
>> 	\
>>> -		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
>>> +#define VIRTIO_CRYPTO_FEATURES ((1ULL <<
>> VIRTIO_F_NOTIFY_ON_EMPTY) |	\
>>> +		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
>> 	\
>>> +		(1ULL << VIRTIO_RING_F_EVENT_IDX) |
>> 	\
>>> +		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
>>> +		(1ULL << VIRTIO_F_VERSION_1) |
>> 	\
>>> +		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
>>>
>>>  #define IOVA_TO_VVA(t, r, a, l, p)					\
>>>  	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p))
>>> @@ -1400,6 +1399,27 @@
>> vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
>>>  	return processed;
>>>  }
>>>
>>> +int
>>> +rte_vhost_crypto_driver_start(const char *path)
>>> +{
>>> +	uint64_t protocol_features;
>>> +	int ret;
>>> +
>>> +	ret = rte_vhost_driver_set_features(path,
>> VIRTIO_CRYPTO_FEATURES);
>>> +	if (ret)
>>> +		return -1;
>>
>> As rte_vhost_driver_set_features is now called on time,
>> use_builtin_virtio_net is set to false before the connection is
>> established.
>>
>> So it should be enough.
>>
>>> +	ret = rte_vhost_driver_get_protocol_features(path,
>> &protocol_features);
>>> +	if (ret)
>>> +		return -1;
>>> +	protocol_features |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>>> +	ret = rte_vhost_driver_set_protocol_features(path,
>> protocol_features);
>>> +	if (ret)
>>> +		return -1;
>>> +
>>> +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);
>>
>> We just have to remove the extra param.
>>
>> I will to the change and apply it today so that you can test.
>>
>> Thanks,
>> Maxime
>>
>>> +}
>>> +
>>>  int
>>>  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>>>  		struct rte_mempool *sess_pool,
>>> @@ -1417,13 +1437,6 @@ rte_vhost_crypto_create(int vid, uint8_t
>> cryptodev_id,
>>>  		return -EINVAL;
>>>  	}
>>>
>>> -	ret = rte_vhost_driver_set_features(dev->ifname,
>>> -			VIRTIO_CRYPTO_FEATURES);
>>> -	if (ret < 0) {
>>> -		VC_LOG_ERR("Error setting features");
>>> -		return -1;
>>> -	}
>>> -
>>>  	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
>>>  			RTE_CACHE_LINE_SIZE, socket_id);
>>>  	if (!vcrypto) {
>>>
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
  2020-10-09  7:36         ` Maxime Coquelin
@ 2020-10-10  9:28           ` Jiang, YuX
  0 siblings, 0 replies; 14+ messages in thread
From: Jiang, YuX @ 2020-10-10  9:28 UTC (permalink / raw)
  To: Maxime Coquelin, Zhang, Roy Fan, dev
  Cc: Xia, Chenbo, Liu, Changpeng, Yigit, Ferruh, stable, Jiang, YuX

Tested-by: Jiang, YuX <yux.jiang@intel.com>

    Best Regards
    Jiang yu

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maxime Coquelin
> Sent: Friday, October 9, 2020 3:36 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Liu, Changpeng
> <changpeng.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-dev v2 2/2] vhost/crypto: fix feature
> negotiation
> 
> 
> 
> On 10/6/20 10:37 AM, Zhang, Roy Fan wrote:
> > Hi Maxime,
> >
> > Thanks I will verify it after you applied the patch.
> 
> 
> Thanks,
> Your patch is now on dpdk-next-virtio/main.
> I would be glad if you could test it before Ferruh merges it.
> 
> Thanks,
> Maxime
> > Regards,
> > Fan
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Tuesday, October 6, 2020 9:10 AM
> >> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> >> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Liu, Changpeng
> >> <changpeng.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> >> stable@dpdk.org
> >> Subject: Re: [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation
> >>
> >>
> >>
> >> On 10/2/20 5:36 PM, Fan Zhang wrote:
> >>> This patch fixes the feature negotiation for vhost crypto during
> >>> initialization. The patch uses the newly created driver start
> >>> function to inform the driver type with the fixed vhost features.
> >>> In addtion the patch provides a new API specifically used by the
> >>> application to start a vhost-crypto driver.
> >>>
> >>> Fixes: 939066d96563 ("vhost/crypto: add public function
> >>> implementation")
> >>> Cc: roy.fan.zhang@intel.com
> >>>
> >>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> >>> ---
> >>>  examples/vhost_crypto/main.c           |  3 +-
> >>>  lib/librte_vhost/rte_vhost_crypto.h    | 12 ++++++++
> >>>  lib/librte_vhost/rte_vhost_version.map |  1 +
> >>>  lib/librte_vhost/vhost_crypto.c        | 41 +++++++++++++++++---------
> >>>  4 files changed, 42 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/examples/vhost_crypto/main.c
> >> b/examples/vhost_crypto/main.c
> >>> index d78fd9b81..11ad49159 100644
> >>> --- a/examples/vhost_crypto/main.c
> >>> +++ b/examples/vhost_crypto/main.c
> >>> @@ -598,7 +598,8 @@ main(int argc, char *argv[])
> >>>  			rte_vhost_driver_callback_register(lo-
> >>> socket_files[j],
> >>>  				&virtio_crypto_device_ops);
> >>>
> >>> -			ret = rte_vhost_driver_start(lo->socket_files[j]);
> >>> +			ret = rte_vhost_crypto_driver_start(
> >>> +					lo->socket_files[j]);
> >>>  			if (ret < 0)  {
> >>>  				RTE_LOG(ERR, USER1, "failed to start
> >> vhost.\n");
> >>>  				goto error_exit;
> >>> diff --git a/lib/librte_vhost/rte_vhost_crypto.h
> >> b/lib/librte_vhost/rte_vhost_crypto.h
> >>> index b54d61db6..c809c46a2 100644
> >>> --- a/lib/librte_vhost/rte_vhost_crypto.h
> >>> +++ b/lib/librte_vhost/rte_vhost_crypto.h
> >>> @@ -20,6 +20,18 @@ enum rte_vhost_crypto_zero_copy {
> >>>  	RTE_VHOST_CRYPTO_MAX_ZERO_COPY_OPTIONS
> >>>  };
> >>>
> >>> +/**
> >>> + * Start vhost crypto driver
> >>> + *
> >>> + * @param path
> >>> + *  The vhost-user socket file path
> >>> + * @return
> >>> + *  0 on success, -1 on failure
> >>> + */
> >>> +__rte_experimental
> >>> +int
> >>> +rte_vhost_crypto_driver_start(const char *path);
> >>> +
> >>>  /**
> >>>   *  Create Vhost-crypto instance
> >>>   *
> >>> diff --git a/lib/librte_vhost/rte_vhost_version.map
> >> b/lib/librte_vhost/rte_vhost_version.map
> >>> index 55e98e557..9183d6f2f 100644
> >>> --- a/lib/librte_vhost/rte_vhost_version.map
> >>> +++ b/lib/librte_vhost/rte_vhost_version.map
> >>> @@ -55,6 +55,7 @@ EXPERIMENTAL {
> >>>  	rte_vhost_driver_get_protocol_features;
> >>>  	rte_vhost_driver_get_queue_num;
> >>>  	rte_vhost_crypto_create;
> >>> +	rte_vhost_crypto_driver_start;
> >>>  	rte_vhost_crypto_free;
> >>>  	rte_vhost_crypto_fetch_requests;
> >>>  	rte_vhost_crypto_finalize_requests;
> >>> diff --git a/lib/librte_vhost/vhost_crypto.c
> >> b/lib/librte_vhost/vhost_crypto.c
> >>> index e08f9c6d7..6195958d2 100644
> >>> --- a/lib/librte_vhost/vhost_crypto.c
> >>> +++ b/lib/librte_vhost/vhost_crypto.c
> >>> @@ -35,13 +35,12 @@
> >>>  #define VC_LOG_DBG(fmt, args...)
> >>>  #endif
> >>>
> >>> -#define VIRTIO_CRYPTO_FEATURES ((1 <<
> VIRTIO_F_NOTIFY_ON_EMPTY)
> >> |	\
> >>> -		(1 << VIRTIO_RING_F_INDIRECT_DESC) |
> >> 	\
> >>> -		(1 << VIRTIO_RING_F_EVENT_IDX) |			\
> >>> -		(1 << VIRTIO_CRYPTO_SERVICE_CIPHER) |
> >> 	\
> >>> -		(1 << VIRTIO_CRYPTO_SERVICE_MAC) |
> >> 	\
> >>> -		(1 << VIRTIO_NET_F_CTRL_VQ) |
> >> 	\
> >>> -		(1 << VHOST_USER_PROTOCOL_F_CONFIG))
> >>> +#define VIRTIO_CRYPTO_FEATURES ((1ULL <<
> >> VIRTIO_F_NOTIFY_ON_EMPTY) |	\
> >>> +		(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
> >> 	\
> >>> +		(1ULL << VIRTIO_RING_F_EVENT_IDX) |
> >> 	\
> >>> +		(1ULL << VIRTIO_NET_F_CTRL_VQ) |			\
> >>> +		(1ULL << VIRTIO_F_VERSION_1) |
> >> 	\
> >>> +		(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
> >>>
> >>>  #define IOVA_TO_VVA(t, r, a, l, p)
> 	\
> >>>  	((t)(uintptr_t)vhost_iova_to_vva(r->dev, r->vq, a, l, p)) @@
> >>> -1400,6 +1399,27 @@
> >> vhost_crypto_complete_one_vm_requests(struct rte_crypto_op **ops,
> >>>  	return processed;
> >>>  }
> >>>
> >>> +int
> >>> +rte_vhost_crypto_driver_start(const char *path) {
> >>> +	uint64_t protocol_features;
> >>> +	int ret;
> >>> +
> >>> +	ret = rte_vhost_driver_set_features(path,
> >> VIRTIO_CRYPTO_FEATURES);
> >>> +	if (ret)
> >>> +		return -1;
> >>
> >> As rte_vhost_driver_set_features is now called on time,
> >> use_builtin_virtio_net is set to false before the connection is
> >> established.
> >>
> >> So it should be enough.
> >>
> >>> +	ret = rte_vhost_driver_get_protocol_features(path,
> >> &protocol_features);
> >>> +	if (ret)
> >>> +		return -1;
> >>> +	protocol_features |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> >>> +	ret = rte_vhost_driver_set_protocol_features(path,
> >> protocol_features);
> >>> +	if (ret)
> >>> +		return -1;
> >>> +
> >>> +	return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_CRYPTO);
> >>
> >> We just have to remove the extra param.
> >>
> >> I will to the change and apply it today so that you can test.
> >>
> >> Thanks,
> >> Maxime
> >>
> >>> +}
> >>> +
> >>>  int
> >>>  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
> >>>  		struct rte_mempool *sess_pool,
> >>> @@ -1417,13 +1437,6 @@ rte_vhost_crypto_create(int vid, uint8_t
> >> cryptodev_id,
> >>>  		return -EINVAL;
> >>>  	}
> >>>
> >>> -	ret = rte_vhost_driver_set_features(dev->ifname,
> >>> -			VIRTIO_CRYPTO_FEATURES);
> >>> -	if (ret < 0) {
> >>> -		VC_LOG_ERR("Error setting features");
> >>> -		return -1;
> >>> -	}
> >>> -
> >>>  	vcrypto = rte_zmalloc_socket(NULL, sizeof(*vcrypto),
> >>>  			RTE_CACHE_LINE_SIZE, socket_id);
> >>>  	if (!vcrypto) {
> >>>
> >


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-10-10  9:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-02  8:36 [dpdk-stable] [dpdk-dev] vhost/crypto: fix initialization Fan Zhang
2020-10-02 12:17 ` Maxime Coquelin
2020-10-02 12:38   ` Zhang, Roy Fan
2020-10-02 15:35 ` [dpdk-stable] [dpdk-dev v2 0/2] " Fan Zhang
2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 1/2] vhost: add backend type in driver start Fan Zhang
2020-10-06  7:53     ` Maxime Coquelin
2020-10-06  8:56       ` Zhang, Roy Fan
2020-10-02 15:36   ` [dpdk-stable] [dpdk-dev v2 2/2] vhost/crypto: fix feature negotiation Fan Zhang
2020-10-06  8:09     ` Maxime Coquelin
2020-10-06  8:37       ` Zhang, Roy Fan
2020-10-09  7:36         ` Maxime Coquelin
2020-10-10  9:28           ` [dpdk-stable] [dpdk-dev] " Jiang, YuX
2020-10-09  6:39     ` [dpdk-stable] " Maxime Coquelin
2020-10-09  7:24     ` Maxime Coquelin

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).