patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] net/mlx5: disable ConnectX-4 Lx Multi Packet Send by default
@ 2018-08-13  7:00 Shahaf Shuler
  2018-08-14 16:32 ` Yongseok Koh
  2018-08-15 12:17 ` [dpdk-stable] [PATCH v2] " Shahaf Shuler
  0 siblings, 2 replies; 5+ messages in thread
From: Shahaf Shuler @ 2018-08-13  7:00 UTC (permalink / raw)
  To: yskoh; +Cc: dev, rasland, alialnu, wisamm, olgas, stable

On ConnectX-4 Lx the Multi Packet Send (MPW) feature is considered
un-secure, as on some cases were the application provides incorrect mbufs
on the Tx burst the host or NIC can get stuck.

Hence, disabling the feature by default for this specific NIC.
Users can still enable this feature and enjoy the performance gain
(mostly for low number of cores) by using the txq_mpw_en devarg.

This patch will impact the out of the box performance of some application
using ConnectX-4 Lx for the sack of security and robustness.

Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 doc/guides/nics/mlx5.rst       | 7 ++++++-
 drivers/net/mlx5/mlx5.c        | 9 +++++++--
 drivers/net/mlx5/mlx5_ethdev.c | 5 +++--
 drivers/net/mlx5/mlx5_prm.h    | 3 ++-
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 52e1213cf5..dbdb90b59b 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -339,7 +339,12 @@ Run-time configuration
   When those offloads are requested the MPS send function will not be used.
 
   It is currently only supported on the ConnectX-4 Lx, ConnectX-5 and Bluefield
-  families of adapters. Enabled by default.
+  families of adapters.
+  On ConnectX-4 Lx the MPW is considered un-secure hence disabled by default.
+  Users which enable the MPW should be aware that application which provides incorrect
+  mbuf descriptors in the Tx burst can lead to serious errors in the host including, on some cases,
+  NIC to get stuck.
+  On ConnectX-5 and Bluefield the MPW is secure and enabled by default.
 
 - ``txq_mpw_hdr_dseg_en`` parameter [int]
 
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index ec63bc6e22..2e8f906f35 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -477,7 +477,11 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
 	} else if (strcmp(MLX5_TXQS_MIN_INLINE, key) == 0) {
 		config->txqs_inline = tmp;
 	} else if (strcmp(MLX5_TXQ_MPW_EN, key) == 0) {
-		config->mps = !!tmp ? config->mps : 0;
+		tmp = !!tmp;
+		if (tmp && config->mps == MLX5_MPW)
+			config->mps = MLX5_MPW_USER_FORCED;
+		else
+			config->mps = tmp ? config->mps : 0;
 	} else if (strcmp(MLX5_TXQ_MPW_HDR_DSEG_EN, key) == 0) {
 		config->mpw_hdr_dseg = !!tmp;
 	} else if (strcmp(MLX5_TXQ_MAX_INLINE_LEN, key) == 0) {
@@ -1044,7 +1048,8 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	}
 	DRV_LOG(INFO, "%sMPS is %s",
 		config.mps == MLX5_MPW_ENHANCED ? "enhanced " : "",
-		config.mps != MLX5_MPW_DISABLED ? "enabled" : "disabled");
+		(config.mps != MLX5_MPW_DISABLED && config.mps != MLX5_MPW) ?
+		"enabled" : "disabled");
 	if (config.cqe_comp && !cqe_comp) {
 		DRV_LOG(WARNING, "Rx CQE compression isn't supported");
 		config.cqe_comp = 0;
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 34c5b95ee6..48917b0f6b 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1224,11 +1224,12 @@ mlx5_select_tx_function(struct rte_eth_dev *dev)
 				"port %u selected enhanced MPW Tx function",
 				dev->data->port_id);
 		}
-	} else if (config->mps && (config->txq_inline > 0)) {
+	} else if (config->mps == MLX5_MPW_USER_FORCED &&
+		   (config->txq_inline > 0)) {
 		tx_pkt_burst = mlx5_tx_burst_mpw_inline;
 		DRV_LOG(DEBUG, "port %u selected MPW inline Tx function",
 			dev->data->port_id);
-	} else if (config->mps) {
+	} else if (config->mps == MLX5_MPW_USER_FORCED) {
 		tx_pkt_burst = mlx5_tx_burst_mpw;
 		DRV_LOG(DEBUG, "port %u selected MPW Tx function",
 			dev->data->port_id);
diff --git a/drivers/net/mlx5/mlx5_prm.h b/drivers/net/mlx5/mlx5_prm.h
index 0870d32fdb..b64cd45eee 100644
--- a/drivers/net/mlx5/mlx5_prm.h
+++ b/drivers/net/mlx5/mlx5_prm.h
@@ -197,7 +197,8 @@ struct mlx5_wqe64 {
 /* MPW mode. */
 enum mlx5_mpw_mode {
 	MLX5_MPW_DISABLED,
-	MLX5_MPW,
+	MLX5_MPW, /* MPW is supported by the device. */
+	MLX5_MPW_USER_FORCED, /* MPW is supported and selected by the user. */
 	MLX5_MPW_ENHANCED, /* Enhanced Multi-Packet Send WQE, a.k.a MPWv2. */
 };
 
-- 
2.12.0

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

* Re: [dpdk-stable] [PATCH] net/mlx5: disable ConnectX-4 Lx Multi Packet Send by default
  2018-08-13  7:00 [dpdk-stable] [PATCH] net/mlx5: disable ConnectX-4 Lx Multi Packet Send by default Shahaf Shuler
@ 2018-08-14 16:32 ` Yongseok Koh
  2018-08-15 12:17 ` [dpdk-stable] [PATCH v2] " Shahaf Shuler
  1 sibling, 0 replies; 5+ messages in thread
From: Yongseok Koh @ 2018-08-14 16:32 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, rasland, alialnu, wisamm, olgas, stable

On Mon, Aug 13, 2018 at 10:00:18AM +0300, Shahaf Shuler wrote:
> On ConnectX-4 Lx the Multi Packet Send (MPW) feature is considered
> un-secure, as on some cases were the application provides incorrect mbufs
> on the Tx burst the host or NIC can get stuck.
> 
> Hence, disabling the feature by default for this specific NIC.
> Users can still enable this feature and enjoy the performance gain
> (mostly for low number of cores) by using the txq_mpw_en devarg.
> 
> This patch will impact the out of the box performance of some application
> using ConnectX-4 Lx for the sack of security and robustness.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
>  doc/guides/nics/mlx5.rst       | 7 ++++++-
>  drivers/net/mlx5/mlx5.c        | 9 +++++++--
>  drivers/net/mlx5/mlx5_ethdev.c | 5 +++--
>  drivers/net/mlx5/mlx5_prm.h    | 3 ++-
>  4 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
> index 52e1213cf5..dbdb90b59b 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -339,7 +339,12 @@ Run-time configuration
>    When those offloads are requested the MPS send function will not be used.
>  
>    It is currently only supported on the ConnectX-4 Lx, ConnectX-5 and Bluefield
> -  families of adapters. Enabled by default.
> +  families of adapters.
> +  On ConnectX-4 Lx the MPW is considered un-secure hence disabled by default.
> +  Users which enable the MPW should be aware that application which provides incorrect
> +  mbuf descriptors in the Tx burst can lead to serious errors in the host including, on some cases,
> +  NIC to get stuck.
> +  On ConnectX-5 and Bluefield the MPW is secure and enabled by default.
>  
>  - ``txq_mpw_hdr_dseg_en`` parameter [int]
>  
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index ec63bc6e22..2e8f906f35 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -477,7 +477,11 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
>  	} else if (strcmp(MLX5_TXQS_MIN_INLINE, key) == 0) {
>  		config->txqs_inline = tmp;
>  	} else if (strcmp(MLX5_TXQ_MPW_EN, key) == 0) {
> -		config->mps = !!tmp ? config->mps : 0;
> +		tmp = !!tmp;
> +		if (tmp && config->mps == MLX5_MPW)
> +			config->mps = MLX5_MPW_USER_FORCED;
> +		else
> +			config->mps = tmp ? config->mps : 0;
>  	} else if (strcmp(MLX5_TXQ_MPW_HDR_DSEG_EN, key) == 0) {
>  		config->mpw_hdr_dseg = !!tmp;
>  	} else if (strcmp(MLX5_TXQ_MAX_INLINE_LEN, key) == 0) {
> @@ -1044,7 +1048,8 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>  	}
>  	DRV_LOG(INFO, "%sMPS is %s",
>  		config.mps == MLX5_MPW_ENHANCED ? "enhanced " : "",
> -		config.mps != MLX5_MPW_DISABLED ? "enabled" : "disabled");
> +		(config.mps != MLX5_MPW_DISABLED && config.mps != MLX5_MPW) ?
> +		"enabled" : "disabled");
>  	if (config.cqe_comp && !cqe_comp) {
>  		DRV_LOG(WARNING, "Rx CQE compression isn't supported");
>  		config.cqe_comp = 0;
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 34c5b95ee6..48917b0f6b 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1224,11 +1224,12 @@ mlx5_select_tx_function(struct rte_eth_dev *dev)
>  				"port %u selected enhanced MPW Tx function",
>  				dev->data->port_id);
>  		}
> -	} else if (config->mps && (config->txq_inline > 0)) {
> +	} else if (config->mps == MLX5_MPW_USER_FORCED &&
> +		   (config->txq_inline > 0)) {
>  		tx_pkt_burst = mlx5_tx_burst_mpw_inline;
>  		DRV_LOG(DEBUG, "port %u selected MPW inline Tx function",
>  			dev->data->port_id);
> -	} else if (config->mps) {
> +	} else if (config->mps == MLX5_MPW_USER_FORCED) {
>  		tx_pkt_burst = mlx5_tx_burst_mpw;
>  		DRV_LOG(DEBUG, "port %u selected MPW Tx function",
>  			dev->data->port_id);
> diff --git a/drivers/net/mlx5/mlx5_prm.h b/drivers/net/mlx5/mlx5_prm.h
> index 0870d32fdb..b64cd45eee 100644
> --- a/drivers/net/mlx5/mlx5_prm.h
> +++ b/drivers/net/mlx5/mlx5_prm.h
> @@ -197,7 +197,8 @@ struct mlx5_wqe64 {
>  /* MPW mode. */
>  enum mlx5_mpw_mode {
>  	MLX5_MPW_DISABLED,
> -	MLX5_MPW,
> +	MLX5_MPW, /* MPW is supported by the device. */
> +	MLX5_MPW_USER_FORCED, /* MPW is supported and selected by the user. */
>  	MLX5_MPW_ENHANCED, /* Enhanced Multi-Packet Send WQE, a.k.a MPWv2. */
>  };

Any specific reason to save device capability in this case? Currently, all the
configurations made by PMD parameters are static in mlx5. We don't have an API
to change it in runtime. E.g., once MPRQ is disabled (by default), there's no
way to enable it even though HW supports it but to restart the app. How about
making it aligned with behavior of MPRQ (and others)?

Thanks,
Yongseok.

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

* [dpdk-stable] [PATCH v2] net/mlx5: disable ConnectX-4 Lx Multi Packet Send by default
  2018-08-13  7:00 [dpdk-stable] [PATCH] net/mlx5: disable ConnectX-4 Lx Multi Packet Send by default Shahaf Shuler
  2018-08-14 16:32 ` Yongseok Koh
@ 2018-08-15 12:17 ` Shahaf Shuler
  2018-08-15 17:11   ` Yongseok Koh
  1 sibling, 1 reply; 5+ messages in thread
From: Shahaf Shuler @ 2018-08-15 12:17 UTC (permalink / raw)
  To: yskoh; +Cc: dev, rasland, erezf, alialnu, wisamm, olgas, stable

On ConnectX-4 Lx the Multi Packet Send (MPW) feature is considered
un-secure, as on some cases were the application provides incorrect mbufs
on the Tx burst the host or NIC can get stuck.

Hence, disabling the feature by default for this specific NIC.
Users can still enable this feature and enjoy the performance gain
(mostly for low number of cores) by using the txq_mpw_en devarg.

This patch will impact the out of the box performance of some application
using ConnectX-4 Lx for the sack of security and robustness.

Since we need different defaults based on the underlying device the mpw
field in the configuration struct was extended to contain also the
MLX5_ARG_UNSET option.

Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 doc/guides/nics/mlx5.rst |  7 ++++++-
 drivers/net/mlx5/mlx5.c  | 20 +++++++++++---------
 drivers/net/mlx5/mlx5.h  |  2 +-
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 52e1213cf5..dbdb90b59b 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -339,7 +339,12 @@ Run-time configuration
   When those offloads are requested the MPS send function will not be used.
 
   It is currently only supported on the ConnectX-4 Lx, ConnectX-5 and Bluefield
-  families of adapters. Enabled by default.
+  families of adapters.
+  On ConnectX-4 Lx the MPW is considered un-secure hence disabled by default.
+  Users which enable the MPW should be aware that application which provides incorrect
+  mbuf descriptors in the Tx burst can lead to serious errors in the host including, on some cases,
+  NIC to get stuck.
+  On ConnectX-5 and Bluefield the MPW is secure and enabled by default.
 
 - ``txq_mpw_hdr_dseg_en`` parameter [int]
 
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index ec63bc6e22..30d4e70a77 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -477,7 +477,7 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
 	} else if (strcmp(MLX5_TXQS_MIN_INLINE, key) == 0) {
 		config->txqs_inline = tmp;
 	} else if (strcmp(MLX5_TXQ_MPW_EN, key) == 0) {
-		config->mps = !!tmp ? config->mps : 0;
+		config->mps = !!tmp;
 	} else if (strcmp(MLX5_TXQ_MPW_HDR_DSEG_EN, key) == 0) {
 		config->mpw_hdr_dseg = !!tmp;
 	} else if (strcmp(MLX5_TXQ_MAX_INLINE_LEN, key) == 0) {
@@ -702,6 +702,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	struct mlx5dv_context dv_attr = { .comp_mask = 0 };
 	struct mlx5_dev_config config = {
 		.vf = !!vf,
+		.mps = MLX5_ARG_UNSET,
 		.tx_vec_en = 1,
 		.rx_vec_en = 1,
 		.mpw_hdr_dseg = 0,
@@ -791,7 +792,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 		DRV_LOG(DEBUG, "MPW isn't supported");
 		mps = MLX5_MPW_DISABLED;
 	}
-	config.mps = mps;
 #ifdef HAVE_IBV_MLX5_MOD_SWP
 	if (dv_attr.comp_mask & MLX5DV_CONTEXT_MASK_SWP)
 		swp = dv_attr.sw_parsing_caps.sw_parsing_offloads;
@@ -1035,13 +1035,15 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 		       (1 << IBV_QPT_RAW_PACKET)));
 	if (config.tso)
 		config.tso_max_payload_sz = attr.tso_caps.max_tso;
-	if (config.mps && !mps) {
-		DRV_LOG(ERR,
-			"multi-packet send not supported on this device"
-			" (" MLX5_TXQ_MPW_EN ")");
-		err = ENOTSUP;
-		goto error;
-	}
+	/*
+	 * MPW is disabled by default, while the Enhanced MPW is enabled
+	 * by default.
+	 */
+	if (config.mps == MLX5_ARG_UNSET)
+		config.mps = (mps == MLX5_MPW_ENHANCED) ? MLX5_MPW_ENHANCED :
+							  MLX5_MPW_DISABLED;
+	else
+		config.mps = config.mps ? mps : MLX5_MPW_DISABLED;
 	DRV_LOG(INFO, "%sMPS is %s",
 		config.mps == MLX5_MPW_ENHANCED ? "enhanced " : "",
 		config.mps != MLX5_MPW_DISABLED ? "enabled" : "disabled");
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index a3a34cffd8..35a196e76d 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -99,7 +99,6 @@ struct mlx5_dev_config {
 	unsigned int hw_fcs_strip:1; /* FCS stripping is supported. */
 	unsigned int hw_padding:1; /* End alignment padding is supported. */
 	unsigned int vf:1; /* This is a VF. */
-	unsigned int mps:2; /* Multi-packet send supported mode. */
 	unsigned int tunnel_en:1;
 	/* Whether tunnel stateless offloads are supported. */
 	unsigned int mpls_en:1; /* MPLS over GRE/UDP is enabled. */
@@ -122,6 +121,7 @@ struct mlx5_dev_config {
 		unsigned int min_rxqs_num;
 		/* Rx queue count threshold to enable MPRQ. */
 	} mprq; /* Configurations for Multi-Packet RQ. */
+	int mps; /* Multi-packet send supported mode. */
 	unsigned int flow_prio; /* Number of flow priorities. */
 	unsigned int tso_max_payload_sz; /* Maximum TCP payload for TSO. */
 	unsigned int ind_table_max_size; /* Maximum indirection table size. */
-- 
2.12.0

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

* Re: [dpdk-stable] [PATCH v2] net/mlx5: disable ConnectX-4 Lx Multi Packet Send by default
  2018-08-15 12:17 ` [dpdk-stable] [PATCH v2] " Shahaf Shuler
@ 2018-08-15 17:11   ` Yongseok Koh
  2018-08-16 11:57     ` Shahaf Shuler
  0 siblings, 1 reply; 5+ messages in thread
From: Yongseok Koh @ 2018-08-15 17:11 UTC (permalink / raw)
  To: Shahaf Shuler
  Cc: dev, Raslan Darawsheh, Erez Ferber, Ali Alnubani, Wisam Monther,
	Olga Shern, stable


> On Aug 15, 2018, at 5:17 AM, Shahaf Shuler <shahafs@mellanox.com> wrote:
> 
> On ConnectX-4 Lx the Multi Packet Send (MPW) feature is considered
> un-secure, as on some cases were the application provides incorrect mbufs
> on the Tx burst the host or NIC can get stuck.
> 
> Hence, disabling the feature by default for this specific NIC.
> Users can still enable this feature and enjoy the performance gain
> (mostly for low number of cores) by using the txq_mpw_en devarg.
> 
> This patch will impact the out of the box performance of some application
> using ConnectX-4 Lx for the sack of security and robustness.
> 
> Since we need different defaults based on the underlying device the mpw
> field in the configuration struct was extended to contain also the
> MLX5_ARG_UNSET option.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---

Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

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

* Re: [dpdk-stable] [PATCH v2] net/mlx5: disable ConnectX-4 Lx Multi Packet Send by default
  2018-08-15 17:11   ` Yongseok Koh
@ 2018-08-16 11:57     ` Shahaf Shuler
  0 siblings, 0 replies; 5+ messages in thread
From: Shahaf Shuler @ 2018-08-16 11:57 UTC (permalink / raw)
  To: Yongseok Koh
  Cc: dev, Raslan Darawsheh, Erez Ferber, Ali Alnubani, Wisam Monther,
	Olga Shern, stable

Wednesday, August 15, 2018 8:12 PM, Yongseok Koh:
> Subject: Re: [PATCH v2] net/mlx5: disable ConnectX-4 Lx Multi Packet Send
> by default
> 
> 
> > On Aug 15, 2018, at 5:17 AM, Shahaf Shuler <shahafs@mellanox.com>
> wrote:
> >
> > On ConnectX-4 Lx the Multi Packet Send (MPW) feature is considered
> > un-secure, as on some cases were the application provides incorrect
> > mbufs on the Tx burst the host or NIC can get stuck.
> >
> > Hence, disabling the feature by default for this specific NIC.
> > Users can still enable this feature and enjoy the performance gain
> > (mostly for low number of cores) by using the txq_mpw_en devarg.
> >
> > This patch will impact the out of the box performance of some
> > application using ConnectX-4 Lx for the sack of security and robustness.
> >
> > Since we need different defaults based on the underlying device the
> > mpw field in the configuration struct was extended to contain also the
> > MLX5_ARG_UNSET option.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > ---
> 
> Acked-by: Yongseok Koh <yskoh@mellanox.com>

Applied to next-net-mlx, thanks. 

> 
> Thanks

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

end of thread, other threads:[~2018-08-16 11:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-13  7:00 [dpdk-stable] [PATCH] net/mlx5: disable ConnectX-4 Lx Multi Packet Send by default Shahaf Shuler
2018-08-14 16:32 ` Yongseok Koh
2018-08-15 12:17 ` [dpdk-stable] [PATCH v2] " Shahaf Shuler
2018-08-15 17:11   ` Yongseok Koh
2018-08-16 11:57     ` Shahaf Shuler

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