diff mbox series

[v3,2/7] net/virtio-user: add vectorized packed ring parameter

Message ID 20200408085313.4487-3-yong.liu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series add packed ring vectorized datapath |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Marvin Liu April 8, 2020, 8:53 a.m. UTC 
  Add new parameter "packed_vec" which can disable vectorized packed ring
datapath explicitly. When "packed_vec" option is on, driver will check
packed ring vectorized datapath prerequisites. If any one of them not
matched, vectorized datapath won't be selected.

Signed-off-by: Marvin Liu <yong.liu@intel.com>

Comments

Xiaolong Ye April 8, 2020, 6:22 a.m. UTC | #1 
On 04/08, Marvin Liu wrote:
>Add new parameter "packed_vec" which can disable vectorized packed ring
>datapath explicitly. When "packed_vec" option is on, driver will check
>packed ring vectorized datapath prerequisites. If any one of them not
>matched, vectorized datapath won't be selected.
>
>Signed-off-by: Marvin Liu <yong.liu@intel.com>
>
>diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
>index 7433d2f08..8103b7a18 100644
>--- a/drivers/net/virtio/virtio_pci.h
>+++ b/drivers/net/virtio/virtio_pci.h
>@@ -251,6 +251,8 @@ struct virtio_hw {
> 	uint8_t	    use_msix;
> 	uint8_t     modern;
> 	uint8_t     use_simple_rx;
>+	uint8_t     packed_vec_rx;
>+	uint8_t     packed_vec_tx;
> 	uint8_t     use_inorder_rx;
> 	uint8_t     use_inorder_tx;
> 	uint8_t     weak_barriers;
>diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
>index e61af4068..399ac5511 100644
>--- a/drivers/net/virtio/virtio_user_ethdev.c
>+++ b/drivers/net/virtio/virtio_user_ethdev.c
>@@ -450,6 +450,8 @@ static const char *valid_args[] = {
> 	VIRTIO_USER_ARG_IN_ORDER,
> #define VIRTIO_USER_ARG_PACKED_VQ      "packed_vq"
> 	VIRTIO_USER_ARG_PACKED_VQ,
>+#define VIRTIO_USER_ARG_PACKED_VEC     "packed_vec"
>+	VIRTIO_USER_ARG_PACKED_VEC,
> 	NULL
> };
> 
>@@ -552,6 +554,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> 	uint64_t mrg_rxbuf = 1;
> 	uint64_t in_order = 1;
> 	uint64_t packed_vq = 0;
>+	uint64_t packed_vec = 0;
>+
> 	char *path = NULL;
> 	char *ifname = NULL;
> 	char *mac_addr = NULL;
>@@ -668,6 +672,15 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> 		}
> 	}
> 
>+	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PACKED_VEC) == 1) {
>+		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PACKED_VEC,
>+				       &get_integer_arg, &packed_vec) < 0) {
>+			PMD_INIT_LOG(ERR, "error to parse %s",
>+				     VIRTIO_USER_ARG_PACKED_VQ);
>+			goto end;
>+		}
>+	}
>+
> 	if (queues > 1 && cq == 0) {
> 		PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
> 		goto end;
>@@ -705,6 +718,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> 	}
> 
> 	hw = eth_dev->data->dev_private;
>+#if defined(RTE_ARCH_X86) && defined(CC_AVX512_SUPPORT)
>+	if (packed_vec) {
>+		hw->packed_vec_rx = 1;
>+		hw->packed_vec_tx = 1;
>+	}
>+#else
>+	if (packed_vec)
>+		PMD_INIT_LOG(ERR, "building environment not match vectorized "
>+				  "packed ring datapath requirement");

Minor nit:

s/not match/doesn't match/

And better to avoid breaking error message strings across multiple source lines.
It makes it harder to use tools like grep to find errors in source.
E.g. user uses "vectorized packed ring datapath" to grep the code. 

Thanks,
Xiaolong

>+#endif
>+
> 	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
> 			 queue_size, mac_addr, &ifname, server_mode,
> 			 mrg_rxbuf, in_order, packed_vq) < 0) {
>@@ -777,4 +801,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
> 	"server=<0|1> "
> 	"mrg_rxbuf=<0|1> "
> 	"in_order=<0|1> "
>-	"packed_vq=<0|1>");
>+	"packed_vq=<0|1>"
>+	"packed_vec=<0|1>");
>-- 
>2.17.1
>
Marvin Liu April 8, 2020, 7:31 a.m. UTC | #2 
> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> Sent: Wednesday, April 8, 2020 2:23 PM
> To: Liu, Yong <yong.liu@intel.com>
> Cc: maxime.coquelin@redhat.com; Wang, Zhihong
> <zhihong.wang@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH v3 2/7] net/virtio-user: add vectorized packed ring
> parameter
> 
> On 04/08, Marvin Liu wrote:
> >Add new parameter "packed_vec" which can disable vectorized packed
> ring
> >datapath explicitly. When "packed_vec" option is on, driver will check
> >packed ring vectorized datapath prerequisites. If any one of them not
> >matched, vectorized datapath won't be selected.
> >
> >Signed-off-by: Marvin Liu <yong.liu@intel.com>
> >
> >diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> >index 7433d2f08..8103b7a18 100644
> >--- a/drivers/net/virtio/virtio_pci.h
> >+++ b/drivers/net/virtio/virtio_pci.h
> >@@ -251,6 +251,8 @@ struct virtio_hw {
> > 	uint8_t	    use_msix;
> > 	uint8_t     modern;
> > 	uint8_t     use_simple_rx;
> >+	uint8_t     packed_vec_rx;
> >+	uint8_t     packed_vec_tx;
> > 	uint8_t     use_inorder_rx;
> > 	uint8_t     use_inorder_tx;
> > 	uint8_t     weak_barriers;
> >diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> >index e61af4068..399ac5511 100644
> >--- a/drivers/net/virtio/virtio_user_ethdev.c
> >+++ b/drivers/net/virtio/virtio_user_ethdev.c
> >@@ -450,6 +450,8 @@ static const char *valid_args[] = {
> > 	VIRTIO_USER_ARG_IN_ORDER,
> > #define VIRTIO_USER_ARG_PACKED_VQ      "packed_vq"
> > 	VIRTIO_USER_ARG_PACKED_VQ,
> >+#define VIRTIO_USER_ARG_PACKED_VEC     "packed_vec"
> >+	VIRTIO_USER_ARG_PACKED_VEC,
> > 	NULL
> > };
> >
> >@@ -552,6 +554,8 @@ virtio_user_pmd_probe(struct rte_vdev_device
> *dev)
> > 	uint64_t mrg_rxbuf = 1;
> > 	uint64_t in_order = 1;
> > 	uint64_t packed_vq = 0;
> >+	uint64_t packed_vec = 0;
> >+
> > 	char *path = NULL;
> > 	char *ifname = NULL;
> > 	char *mac_addr = NULL;
> >@@ -668,6 +672,15 @@ virtio_user_pmd_probe(struct rte_vdev_device
> *dev)
> > 		}
> > 	}
> >
> >+	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PACKED_VEC) == 1) {
> >+		if (rte_kvargs_process(kvlist,
> VIRTIO_USER_ARG_PACKED_VEC,
> >+				       &get_integer_arg, &packed_vec) < 0) {
> >+			PMD_INIT_LOG(ERR, "error to parse %s",
> >+				     VIRTIO_USER_ARG_PACKED_VQ);
> >+			goto end;
> >+		}
> >+	}
> >+
> > 	if (queues > 1 && cq == 0) {
> > 		PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
> > 		goto end;
> >@@ -705,6 +718,17 @@ virtio_user_pmd_probe(struct rte_vdev_device
> *dev)
> > 	}
> >
> > 	hw = eth_dev->data->dev_private;
> >+#if defined(RTE_ARCH_X86) && defined(CC_AVX512_SUPPORT)
> >+	if (packed_vec) {
> >+		hw->packed_vec_rx = 1;
> >+		hw->packed_vec_tx = 1;
> >+	}
> >+#else
> >+	if (packed_vec)
> >+		PMD_INIT_LOG(ERR, "building environment not match
> vectorized "
> >+				  "packed ring datapath requirement");
> 
> Minor nit:
> 
> s/not match/doesn't match/
> 
> And better to avoid breaking error message strings across multiple source
> lines.
> It makes it harder to use tools like grep to find errors in source.
> E.g. user uses "vectorized packed ring datapath" to grep the code.
> 
> Thanks,
> Xiaolong
> 

Thanks for remind. Will change in next release.

> >+#endif
> >+
> > 	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
> > 			 queue_size, mac_addr, &ifname, server_mode,
> > 			 mrg_rxbuf, in_order, packed_vq) < 0) {
> >@@ -777,4 +801,5 @@
> RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
> > 	"server=<0|1> "
> > 	"mrg_rxbuf=<0|1> "
> > 	"in_order=<0|1> "
> >-	"packed_vq=<0|1>");
> >+	"packed_vq=<0|1>"
> >+	"packed_vec=<0|1>");
> >--
> >2.17.1
> >
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 7433d2f08..8103b7a18 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -251,6 +251,8 @@  struct virtio_hw {
 	uint8_t	    use_msix;
 	uint8_t     modern;
 	uint8_t     use_simple_rx;
+	uint8_t     packed_vec_rx;
+	uint8_t     packed_vec_tx;
 	uint8_t     use_inorder_rx;
 	uint8_t     use_inorder_tx;
 	uint8_t     weak_barriers;
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index e61af4068..399ac5511 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -450,6 +450,8 @@  static const char *valid_args[] = {
 	VIRTIO_USER_ARG_IN_ORDER,
 #define VIRTIO_USER_ARG_PACKED_VQ      "packed_vq"
 	VIRTIO_USER_ARG_PACKED_VQ,
+#define VIRTIO_USER_ARG_PACKED_VEC     "packed_vec"
+	VIRTIO_USER_ARG_PACKED_VEC,
 	NULL
 };
 
@@ -552,6 +554,8 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	uint64_t mrg_rxbuf = 1;
 	uint64_t in_order = 1;
 	uint64_t packed_vq = 0;
+	uint64_t packed_vec = 0;
+
 	char *path = NULL;
 	char *ifname = NULL;
 	char *mac_addr = NULL;
@@ -668,6 +672,15 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 		}
 	}
 
+	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PACKED_VEC) == 1) {
+		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PACKED_VEC,
+				       &get_integer_arg, &packed_vec) < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+				     VIRTIO_USER_ARG_PACKED_VQ);
+			goto end;
+		}
+	}
+
 	if (queues > 1 && cq == 0) {
 		PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
 		goto end;
@@ -705,6 +718,17 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	}
 
 	hw = eth_dev->data->dev_private;
+#if defined(RTE_ARCH_X86) && defined(CC_AVX512_SUPPORT)
+	if (packed_vec) {
+		hw->packed_vec_rx = 1;
+		hw->packed_vec_tx = 1;
+	}
+#else
+	if (packed_vec)
+		PMD_INIT_LOG(ERR, "building environment not match vectorized "
+				  "packed ring datapath requirement");
+#endif
+
 	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
 			 queue_size, mac_addr, &ifname, server_mode,
 			 mrg_rxbuf, in_order, packed_vq) < 0) {
@@ -777,4 +801,5 @@  RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
 	"server=<0|1> "
 	"mrg_rxbuf=<0|1> "
 	"in_order=<0|1> "
-	"packed_vq=<0|1>");
+	"packed_vq=<0|1>"
+	"packed_vec=<0|1>");