[PATCH 1/2] net/mlx5: modify PMD args process

[PATCH 1/2] net/mlx5: modify PMD args process

[Shahaf Shuler]

Currently the argument process is done without indication which
parameter was forced by the application and which one is on it
default value.
This becomes problematic when different features requires different
defaults. For example, Enhanced multi packet send and TSO.

This commit modifies the argument process, enabling to differ
which parameter was forced by the application.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5.c | 72 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index b7eb9b56a..9d850dbbd 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -93,6 +93,18 @@
 /* Device parameter to enable hardware TSO offload. */
 #define MLX5_TSO "tso"
 
+/* Default PMD specific parameter value. */
+#define MLX5_UNSET (-1)
+
+struct mlx5_args {
+	int cqe_comp;
+	int txq_inline;
+	int txqs_inline;
+	int mps;
+	int mpw_hdr_dseg;
+	int inline_max_packet_sz;
+	int tso;
+};
 /**
  * Retrieve integer value from environment variable.
  *
@@ -286,7 +298,7 @@ mlx5_dev_idx(struct rte_pci_addr *pci_addr)
 static int
 mlx5_args_check(const char *key, const char *val, void *opaque)
 {
-	struct priv *priv = opaque;
+	struct mlx5_args *args = opaque;
 	unsigned long tmp;
 
 	errno = 0;
@@ -296,19 +308,19 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
 		return errno;
 	}
 	if (strcmp(MLX5_RXQ_CQE_COMP_EN, key) == 0) {
-		priv->cqe_comp = !!tmp;
+		args->cqe_comp = !!tmp;
 	} else if (strcmp(MLX5_TXQ_INLINE, key) == 0) {
-		priv->txq_inline = tmp;
+		args->txq_inline = tmp;
 	} else if (strcmp(MLX5_TXQS_MIN_INLINE, key) == 0) {
-		priv->txqs_inline = tmp;
+		args->txqs_inline = tmp;
 	} else if (strcmp(MLX5_TXQ_MPW_EN, key) == 0) {
-		priv->mps = !!tmp ? priv->mps : MLX5_MPW_DISABLED;
+		args->mps = !!tmp;
 	} else if (strcmp(MLX5_TXQ_MPW_HDR_DSEG_EN, key) == 0) {
-		priv->mpw_hdr_dseg = !!tmp;
+		args->mpw_hdr_dseg = !!tmp;
 	} else if (strcmp(MLX5_TXQ_MAX_INLINE_LEN, key) == 0) {
-		priv->inline_max_packet_sz = tmp;
+		args->inline_max_packet_sz = tmp;
 	} else if (strcmp(MLX5_TSO, key) == 0) {
-		priv->tso = !!tmp;
+		args->tso = !!tmp;
 	} else {
 		WARN("%s: unknown parameter", key);
 		return -EINVAL;
@@ -328,7 +340,7 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
  *   0 on success, errno value on failure.
  */
 static int
-mlx5_args(struct priv *priv, struct rte_devargs *devargs)
+mlx5_args(struct mlx5_args *args, struct rte_devargs *devargs)
 {
 	const char **params = (const char *[]){
 		MLX5_RXQ_CQE_COMP_EN,
@@ -354,7 +366,7 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
 	for (i = 0; (params[i] != NULL); ++i) {
 		if (rte_kvargs_count(kvlist, params[i])) {
 			ret = rte_kvargs_process(kvlist, params[i],
-						 mlx5_args_check, priv);
+						 mlx5_args_check, args);
 			if (ret != 0) {
 				rte_kvargs_free(kvlist);
 				return ret;
@@ -368,6 +380,34 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
 static struct eth_driver mlx5_driver;
 
 /**
+ * Assign parameters from args into priv, only non default
+ * values are considered.
+ *
+ * @param[out] priv
+ *   Pointer to private structure.
+ * @param[in] args
+ *   Pointer to args values.
+ */
+static void
+mlx5_args_assign(struct priv *priv, struct mlx5_args *args)
+{
+	if (args->cqe_comp != MLX5_UNSET)
+		priv->cqe_comp = args->cqe_comp;
+	if (args->txq_inline != MLX5_UNSET)
+		priv->txq_inline = args->txq_inline;
+	if (args->txqs_inline != MLX5_UNSET)
+		priv->txqs_inline = args->txqs_inline;
+	if (args->mps != MLX5_UNSET)
+		priv->mps = args->mps ? priv->mps : 0;
+	if (args->mpw_hdr_dseg != MLX5_UNSET)
+		priv->mpw_hdr_dseg = args->mpw_hdr_dseg;
+	if (args->inline_max_packet_sz != MLX5_UNSET)
+		priv->inline_max_packet_sz = args->inline_max_packet_sz;
+	if (args->tso != MLX5_UNSET)
+		priv->tso = args->tso;
+}
+
+/**
  * DPDK callback to register a PCI device.
  *
  * This function creates an Ethernet device for each port of a given
@@ -502,6 +542,15 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		struct ibv_exp_device_attr exp_device_attr;
 		struct ether_addr mac;
 		uint16_t num_vfs = 0;
+		struct mlx5_args args = {
+			.cqe_comp = MLX5_UNSET,
+			.txq_inline = MLX5_UNSET,
+			.txqs_inline = MLX5_UNSET,
+			.mps = MLX5_UNSET,
+			.mpw_hdr_dseg = MLX5_UNSET,
+			.inline_max_packet_sz = MLX5_UNSET,
+			.tso = MLX5_UNSET,
+		};
 
 		exp_device_attr.comp_mask =
 			IBV_EXP_DEVICE_ATTR_EXP_CAP_FLAGS |
@@ -570,12 +619,13 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		}
 		priv->cqe_comp = 1; /* Enable compression by default. */
 		priv->tunnel_en = tunnel_en;
-		err = mlx5_args(priv, pci_dev->device.devargs);
+		err = mlx5_args(&args, pci_dev->device.devargs);
 		if (err) {
 			ERROR("failed to process device arguments: %s",
 			      strerror(err));
 			goto port_error;
 		}
+		mlx5_args_assign(priv, &args);
 		if (ibv_exp_query_device(ctx, &exp_device_attr)) {
 			ERROR("ibv_exp_query_device() failed");
 			goto port_error;
-- 
2.12.0

[PATCH 2/2] net/mlx5: fix PMD specific parameters defaults

[Shahaf Shuler]

With the Enhanced multi packet send addition, the defaults were made
in order to get the maximum out of the box performance.
Features like tso, don't use the enhanced send, however the defaults
are still valid. This cause Tx queue creation to fail.

Fixes: aea00c008140 ("net/mlx5: add hardware TSO support")

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 9d850dbbd..faf0d3926 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -610,13 +610,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		priv->pd = pd;
 		priv->mtu = ETHER_MTU;
 		priv->mps = mps; /* Enable MPW by default if supported. */
-		/* Set default values for Enhanced MPW, a.k.a MPWv2. */
-		if (mps == MLX5_MPW_ENHANCED) {
-			priv->mpw_hdr_dseg = 0;
-			priv->txqs_inline = MLX5_EMPW_MIN_TXQS;
-			priv->inline_max_packet_sz = MLX5_EMPW_MAX_INLINE_LEN;
-			priv->txq_inline = MLX5_WQE_SIZE_MAX - MLX5_WQE_SIZE;
-		}
 		priv->cqe_comp = 1; /* Enable compression by default. */
 		priv->tunnel_en = tunnel_en;
 		err = mlx5_args(&args, pci_dev->device.devargs);
@@ -688,6 +681,17 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		INFO("%sMPS is %s",
 		     priv->mps == MLX5_MPW_ENHANCED ? "Enhanced " : "",
 		     priv->mps != MLX5_MPW_DISABLED ? "enabled" : "disabled");
+		/* Set default values for Enhanced MPW, a.k.a MPWv2. */
+		if (priv->mps == MLX5_MPW_ENHANCED) {
+			if (args.txqs_inline == MLX5_UNSET)
+				priv->txqs_inline = MLX5_EMPW_MIN_TXQS;
+			if (args.inline_max_packet_sz == MLX5_UNSET)
+				priv->inline_max_packet_sz =
+					MLX5_EMPW_MAX_INLINE_LEN;
+			if (args.txq_inline == MLX5_UNSET)
+				priv->txq_inline = MLX5_WQE_SIZE_MAX -
+						   MLX5_WQE_SIZE;
+		}
 		/* Allocate and register default RSS hash keys. */
 		priv->rss_conf = rte_calloc(__func__, hash_rxq_init_n,
 					    sizeof((*priv->rss_conf)[0]), 0);
-- 
2.12.0

Re: [PATCH 1/2] net/mlx5: modify PMD args process

[Nélio Laranjeiro]

Hi Shahaf,

On Sun, Apr 16, 2017 at 10:46:39AM +0300, Shahaf Shuler wrote:
> Currently the argument process is done without indication which
> parameter was forced by the application and which one is on it
> default value.
> This becomes problematic when different features requires different
> defaults. For example, Enhanced multi packet send and TSO.
> 
> This commit modifies the argument process, enabling to differ
> which parameter was forced by the application.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Acked-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5.c | 72 +++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 61 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index b7eb9b56a..9d850dbbd 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -93,6 +93,18 @@
>  /* Device parameter to enable hardware TSO offload. */
>  #define MLX5_TSO "tso"
>  
> +/* Default PMD specific parameter value. */
> +#define MLX5_UNSET (-1)

I'll suggest a stronger name space like MLX5_ARG_UNSET.

> +struct mlx5_args {
> +	int cqe_comp;
> +	int txq_inline;
> +	int txqs_inline;
> +	int mps;
> +	int mpw_hdr_dseg;
> +	int inline_max_packet_sz;
> +	int tso;
> +};
> +
>  /**
>   * Retrieve integer value from environment variable.
>   *
> @@ -286,7 +298,7 @@ mlx5_dev_idx(struct rte_pci_addr *pci_addr)
>  static int
>  mlx5_args_check(const char *key, const char *val, void *opaque)
>  {
> -	struct priv *priv = opaque;
> +	struct mlx5_args *args = opaque;
>  	unsigned long tmp;
>  
>  	errno = 0;
> @@ -296,19 +308,19 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
>  		return errno;
>  	}
>  	if (strcmp(MLX5_RXQ_CQE_COMP_EN, key) == 0) {
> -		priv->cqe_comp = !!tmp;
> +		args->cqe_comp = !!tmp;
>  	} else if (strcmp(MLX5_TXQ_INLINE, key) == 0) {
> -		priv->txq_inline = tmp;
> +		args->txq_inline = tmp;
>  	} else if (strcmp(MLX5_TXQS_MIN_INLINE, key) == 0) {
> -		priv->txqs_inline = tmp;
> +		args->txqs_inline = tmp;
>  	} else if (strcmp(MLX5_TXQ_MPW_EN, key) == 0) {
> -		priv->mps = !!tmp ? priv->mps : MLX5_MPW_DISABLED;
> +		args->mps = !!tmp;
>  	} else if (strcmp(MLX5_TXQ_MPW_HDR_DSEG_EN, key) == 0) {
> -		priv->mpw_hdr_dseg = !!tmp;
> +		args->mpw_hdr_dseg = !!tmp;
>  	} else if (strcmp(MLX5_TXQ_MAX_INLINE_LEN, key) == 0) {
> -		priv->inline_max_packet_sz = tmp;
> +		args->inline_max_packet_sz = tmp;
>  	} else if (strcmp(MLX5_TSO, key) == 0) {
> -		priv->tso = !!tmp;
> +		args->tso = !!tmp;
>  	} else {
>  		WARN("%s: unknown parameter", key);
>  		return -EINVAL;
> @@ -328,7 +340,7 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
>   *   0 on success, errno value on failure.
>   */
>  static int
> -mlx5_args(struct priv *priv, struct rte_devargs *devargs)
> +mlx5_args(struct mlx5_args *args, struct rte_devargs *devargs)
>  {
>  	const char **params = (const char *[]){
>  		MLX5_RXQ_CQE_COMP_EN,
> @@ -354,7 +366,7 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
>  	for (i = 0; (params[i] != NULL); ++i) {
>  		if (rte_kvargs_count(kvlist, params[i])) {
>  			ret = rte_kvargs_process(kvlist, params[i],
> -						 mlx5_args_check, priv);
> +						 mlx5_args_check, args);
>  			if (ret != 0) {
>  				rte_kvargs_free(kvlist);
>  				return ret;
> @@ -368,6 +380,34 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
>  static struct eth_driver mlx5_driver;
>  
>  /**
> + * Assign parameters from args into priv, only non default
> + * values are considered.
> + *
> + * @param[out] priv
> + *   Pointer to private structure.
> + * @param[in] args
> + *   Pointer to args values.
> + */
> +static void
> +mlx5_args_assign(struct priv *priv, struct mlx5_args *args)
> +{
> +	if (args->cqe_comp != MLX5_UNSET)
> +		priv->cqe_comp = args->cqe_comp;
> +	if (args->txq_inline != MLX5_UNSET)
> +		priv->txq_inline = args->txq_inline;
> +	if (args->txqs_inline != MLX5_UNSET)
> +		priv->txqs_inline = args->txqs_inline;
> +	if (args->mps != MLX5_UNSET)
> +		priv->mps = args->mps ? priv->mps : 0;
> +	if (args->mpw_hdr_dseg != MLX5_UNSET)
> +		priv->mpw_hdr_dseg = args->mpw_hdr_dseg;
> +	if (args->inline_max_packet_sz != MLX5_UNSET)
> +		priv->inline_max_packet_sz = args->inline_max_packet_sz;
> +	if (args->tso != MLX5_UNSET)
> +		priv->tso = args->tso;
> +}
> +
> +/**
>   * DPDK callback to register a PCI device.
>   *
>   * This function creates an Ethernet device for each port of a given
> @@ -502,6 +542,15 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  		struct ibv_exp_device_attr exp_device_attr;
>  		struct ether_addr mac;
>  		uint16_t num_vfs = 0;
> +		struct mlx5_args args = {
> +			.cqe_comp = MLX5_UNSET,
> +			.txq_inline = MLX5_UNSET,
> +			.txqs_inline = MLX5_UNSET,
> +			.mps = MLX5_UNSET,
> +			.mpw_hdr_dseg = MLX5_UNSET,
> +			.inline_max_packet_sz = MLX5_UNSET,
> +			.tso = MLX5_UNSET,
> +		};
>  
>  		exp_device_attr.comp_mask =
>  			IBV_EXP_DEVICE_ATTR_EXP_CAP_FLAGS |
> @@ -570,12 +619,13 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  		}
>  		priv->cqe_comp = 1; /* Enable compression by default. */
>  		priv->tunnel_en = tunnel_en;
> -		err = mlx5_args(priv, pci_dev->device.devargs);
> +		err = mlx5_args(&args, pci_dev->device.devargs);
>  		if (err) {
>  			ERROR("failed to process device arguments: %s",
>  			      strerror(err));
>  			goto port_error;
>  		}
> +		mlx5_args_assign(priv, &args);
>  		if (ibv_exp_query_device(ctx, &exp_device_attr)) {
>  			ERROR("ibv_exp_query_device() failed");
>  			goto port_error;
> -- 
> 2.12.0
> 

-- 
Nélio Laranjeiro
6WIND

[PATCH v2 1/2] net/mlx5: modify PMD args process

[Shahaf Shuler]

Currently the argument process is done without indication which
parameter was forced by the application and which one is on it
default value.
This becomes problematic when different features requires different
defaults. For example, Enhanced multi packet send and TSO.

This commit modifies the argument process, enabling to differ
which parameter was forced by the application.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
on v2:
 * changed MLX5_UNSET to MLX5_ARG_UNSET. 
---
 drivers/net/mlx5/mlx5.c | 72 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index b7eb9b56a..78724a1bb 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -93,6 +93,18 @@
 /* Device parameter to enable hardware TSO offload. */
 #define MLX5_TSO "tso"
 
+/* Default PMD specific parameter value. */
+#define MLX5_ARG_UNSET (-1)
+
+struct mlx5_args {
+	int cqe_comp;
+	int txq_inline;
+	int txqs_inline;
+	int mps;
+	int mpw_hdr_dseg;
+	int inline_max_packet_sz;
+	int tso;
+};
 /**
  * Retrieve integer value from environment variable.
  *
@@ -286,7 +298,7 @@ mlx5_dev_idx(struct rte_pci_addr *pci_addr)
 static int
 mlx5_args_check(const char *key, const char *val, void *opaque)
 {
-	struct priv *priv = opaque;
+	struct mlx5_args *args = opaque;
 	unsigned long tmp;
 
 	errno = 0;
@@ -296,19 +308,19 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
 		return errno;
 	}
 	if (strcmp(MLX5_RXQ_CQE_COMP_EN, key) == 0) {
-		priv->cqe_comp = !!tmp;
+		args->cqe_comp = !!tmp;
 	} else if (strcmp(MLX5_TXQ_INLINE, key) == 0) {
-		priv->txq_inline = tmp;
+		args->txq_inline = tmp;
 	} else if (strcmp(MLX5_TXQS_MIN_INLINE, key) == 0) {
-		priv->txqs_inline = tmp;
+		args->txqs_inline = tmp;
 	} else if (strcmp(MLX5_TXQ_MPW_EN, key) == 0) {
-		priv->mps = !!tmp ? priv->mps : MLX5_MPW_DISABLED;
+		args->mps = !!tmp;
 	} else if (strcmp(MLX5_TXQ_MPW_HDR_DSEG_EN, key) == 0) {
-		priv->mpw_hdr_dseg = !!tmp;
+		args->mpw_hdr_dseg = !!tmp;
 	} else if (strcmp(MLX5_TXQ_MAX_INLINE_LEN, key) == 0) {
-		priv->inline_max_packet_sz = tmp;
+		args->inline_max_packet_sz = tmp;
 	} else if (strcmp(MLX5_TSO, key) == 0) {
-		priv->tso = !!tmp;
+		args->tso = !!tmp;
 	} else {
 		WARN("%s: unknown parameter", key);
 		return -EINVAL;
@@ -328,7 +340,7 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
  *   0 on success, errno value on failure.
  */
 static int
-mlx5_args(struct priv *priv, struct rte_devargs *devargs)
+mlx5_args(struct mlx5_args *args, struct rte_devargs *devargs)
 {
 	const char **params = (const char *[]){
 		MLX5_RXQ_CQE_COMP_EN,
@@ -354,7 +366,7 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
 	for (i = 0; (params[i] != NULL); ++i) {
 		if (rte_kvargs_count(kvlist, params[i])) {
 			ret = rte_kvargs_process(kvlist, params[i],
-						 mlx5_args_check, priv);
+						 mlx5_args_check, args);
 			if (ret != 0) {
 				rte_kvargs_free(kvlist);
 				return ret;
@@ -368,6 +380,34 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
 static struct eth_driver mlx5_driver;
 
 /**
+ * Assign parameters from args into priv, only non default
+ * values are considered.
+ *
+ * @param[out] priv
+ *   Pointer to private structure.
+ * @param[in] args
+ *   Pointer to args values.
+ */
+static void
+mlx5_args_assign(struct priv *priv, struct mlx5_args *args)
+{
+	if (args->cqe_comp != MLX5_ARG_UNSET)
+		priv->cqe_comp = args->cqe_comp;
+	if (args->txq_inline != MLX5_ARG_UNSET)
+		priv->txq_inline = args->txq_inline;
+	if (args->txqs_inline != MLX5_ARG_UNSET)
+		priv->txqs_inline = args->txqs_inline;
+	if (args->mps != MLX5_ARG_UNSET)
+		priv->mps = args->mps ? priv->mps : 0;
+	if (args->mpw_hdr_dseg != MLX5_ARG_UNSET)
+		priv->mpw_hdr_dseg = args->mpw_hdr_dseg;
+	if (args->inline_max_packet_sz != MLX5_ARG_UNSET)
+		priv->inline_max_packet_sz = args->inline_max_packet_sz;
+	if (args->tso != MLX5_ARG_UNSET)
+		priv->tso = args->tso;
+}
+
+/**
  * DPDK callback to register a PCI device.
  *
  * This function creates an Ethernet device for each port of a given
@@ -502,6 +542,15 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		struct ibv_exp_device_attr exp_device_attr;
 		struct ether_addr mac;
 		uint16_t num_vfs = 0;
+		struct mlx5_args args = {
+			.cqe_comp = MLX5_ARG_UNSET,
+			.txq_inline = MLX5_ARG_UNSET,
+			.txqs_inline = MLX5_ARG_UNSET,
+			.mps = MLX5_ARG_UNSET,
+			.mpw_hdr_dseg = MLX5_ARG_UNSET,
+			.inline_max_packet_sz = MLX5_ARG_UNSET,
+			.tso = MLX5_ARG_UNSET,
+		};
 
 		exp_device_attr.comp_mask =
 			IBV_EXP_DEVICE_ATTR_EXP_CAP_FLAGS |
@@ -570,12 +619,13 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		}
 		priv->cqe_comp = 1; /* Enable compression by default. */
 		priv->tunnel_en = tunnel_en;
-		err = mlx5_args(priv, pci_dev->device.devargs);
+		err = mlx5_args(&args, pci_dev->device.devargs);
 		if (err) {
 			ERROR("failed to process device arguments: %s",
 			      strerror(err));
 			goto port_error;
 		}
+		mlx5_args_assign(priv, &args);
 		if (ibv_exp_query_device(ctx, &exp_device_attr)) {
 			ERROR("ibv_exp_query_device() failed");
 			goto port_error;
-- 
2.12.0

[PATCH v2 2/2] net/mlx5: fix PMD specific parameters defaults

[Shahaf Shuler]

With the Enhanced multi packet send addition, the defaults were made
in order to get the maximum out of the box performance.
Features like tso, don't use the enhanced send, however the defaults
are still valid. This cause Tx queue creation to fail.

Fixes: aea00c008140 ("net/mlx5: add hardware TSO support")

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
on v2:
* changed MLX5_UNSET to MLX5_ARG_UNSET.
---
 drivers/net/mlx5/mlx5.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 78724a1bb..c48fea82b 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -610,13 +610,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		priv->pd = pd;
 		priv->mtu = ETHER_MTU;
 		priv->mps = mps; /* Enable MPW by default if supported. */
-		/* Set default values for Enhanced MPW, a.k.a MPWv2. */
-		if (mps == MLX5_MPW_ENHANCED) {
-			priv->mpw_hdr_dseg = 0;
-			priv->txqs_inline = MLX5_EMPW_MIN_TXQS;
-			priv->inline_max_packet_sz = MLX5_EMPW_MAX_INLINE_LEN;
-			priv->txq_inline = MLX5_WQE_SIZE_MAX - MLX5_WQE_SIZE;
-		}
 		priv->cqe_comp = 1; /* Enable compression by default. */
 		priv->tunnel_en = tunnel_en;
 		err = mlx5_args(&args, pci_dev->device.devargs);
@@ -688,6 +681,17 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		INFO("%sMPS is %s",
 		     priv->mps == MLX5_MPW_ENHANCED ? "Enhanced " : "",
 		     priv->mps != MLX5_MPW_DISABLED ? "enabled" : "disabled");
+		/* Set default values for Enhanced MPW, a.k.a MPWv2. */
+		if (priv->mps == MLX5_MPW_ENHANCED) {
+			if (args.txqs_inline == MLX5_ARG_UNSET)
+				priv->txqs_inline = MLX5_EMPW_MIN_TXQS;
+			if (args.inline_max_packet_sz == MLX5_ARG_UNSET)
+				priv->inline_max_packet_sz =
+					MLX5_EMPW_MAX_INLINE_LEN;
+			if (args.txq_inline == MLX5_ARG_UNSET)
+				priv->txq_inline = MLX5_WQE_SIZE_MAX -
+						   MLX5_WQE_SIZE;
+		}
 		/* Allocate and register default RSS hash keys. */
 		priv->rss_conf = rte_calloc(__func__, hash_rxq_init_n,
 					    sizeof((*priv->rss_conf)[0]), 0);
-- 
2.12.0

Re: [PATCH v2 1/2] net/mlx5: modify PMD args process

[Nélio Laranjeiro]

On Tue, Apr 18, 2017 at 01:22:27PM +0300, Shahaf Shuler wrote:
> Currently the argument process is done without indication which
> parameter was forced by the application and which one is on it
> default value.
> This becomes problematic when different features requires different
> defaults. For example, Enhanced multi packet send and TSO.
> 
> This commit modifies the argument process, enabling to differ
> which parameter was forced by the application.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Acked-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> on v2:
>  * changed MLX5_UNSET to MLX5_ARG_UNSET. 
> ---
>  drivers/net/mlx5/mlx5.c | 72 +++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 61 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index b7eb9b56a..78724a1bb 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -93,6 +93,18 @@
>  /* Device parameter to enable hardware TSO offload. */
>  #define MLX5_TSO "tso"
>  
> +/* Default PMD specific parameter value. */
> +#define MLX5_ARG_UNSET (-1)
> +
> +struct mlx5_args {
> +	int cqe_comp;
> +	int txq_inline;
> +	int txqs_inline;
> +	int mps;
> +	int mpw_hdr_dseg;
> +	int inline_max_packet_sz;
> +	int tso;
> +};
>  /**
>   * Retrieve integer value from environment variable.
>   *
> @@ -286,7 +298,7 @@ mlx5_dev_idx(struct rte_pci_addr *pci_addr)
>  static int
>  mlx5_args_check(const char *key, const char *val, void *opaque)
>  {
> -	struct priv *priv = opaque;
> +	struct mlx5_args *args = opaque;
>  	unsigned long tmp;
>  
>  	errno = 0;
> @@ -296,19 +308,19 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
>  		return errno;
>  	}
>  	if (strcmp(MLX5_RXQ_CQE_COMP_EN, key) == 0) {
> -		priv->cqe_comp = !!tmp;
> +		args->cqe_comp = !!tmp;
>  	} else if (strcmp(MLX5_TXQ_INLINE, key) == 0) {
> -		priv->txq_inline = tmp;
> +		args->txq_inline = tmp;
>  	} else if (strcmp(MLX5_TXQS_MIN_INLINE, key) == 0) {
> -		priv->txqs_inline = tmp;
> +		args->txqs_inline = tmp;
>  	} else if (strcmp(MLX5_TXQ_MPW_EN, key) == 0) {
> -		priv->mps = !!tmp ? priv->mps : MLX5_MPW_DISABLED;
> +		args->mps = !!tmp;
>  	} else if (strcmp(MLX5_TXQ_MPW_HDR_DSEG_EN, key) == 0) {
> -		priv->mpw_hdr_dseg = !!tmp;
> +		args->mpw_hdr_dseg = !!tmp;
>  	} else if (strcmp(MLX5_TXQ_MAX_INLINE_LEN, key) == 0) {
> -		priv->inline_max_packet_sz = tmp;
> +		args->inline_max_packet_sz = tmp;
>  	} else if (strcmp(MLX5_TSO, key) == 0) {
> -		priv->tso = !!tmp;
> +		args->tso = !!tmp;
>  	} else {
>  		WARN("%s: unknown parameter", key);
>  		return -EINVAL;
> @@ -328,7 +340,7 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
>   *   0 on success, errno value on failure.
>   */
>  static int
> -mlx5_args(struct priv *priv, struct rte_devargs *devargs)
> +mlx5_args(struct mlx5_args *args, struct rte_devargs *devargs)
>  {
>  	const char **params = (const char *[]){
>  		MLX5_RXQ_CQE_COMP_EN,
> @@ -354,7 +366,7 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
>  	for (i = 0; (params[i] != NULL); ++i) {
>  		if (rte_kvargs_count(kvlist, params[i])) {
>  			ret = rte_kvargs_process(kvlist, params[i],
> -						 mlx5_args_check, priv);
> +						 mlx5_args_check, args);
>  			if (ret != 0) {
>  				rte_kvargs_free(kvlist);
>  				return ret;
> @@ -368,6 +380,34 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
>  static struct eth_driver mlx5_driver;
>  
>  /**
> + * Assign parameters from args into priv, only non default
> + * values are considered.
> + *
> + * @param[out] priv
> + *   Pointer to private structure.
> + * @param[in] args
> + *   Pointer to args values.
> + */
> +static void
> +mlx5_args_assign(struct priv *priv, struct mlx5_args *args)
> +{
> +	if (args->cqe_comp != MLX5_ARG_UNSET)
> +		priv->cqe_comp = args->cqe_comp;
> +	if (args->txq_inline != MLX5_ARG_UNSET)
> +		priv->txq_inline = args->txq_inline;
> +	if (args->txqs_inline != MLX5_ARG_UNSET)
> +		priv->txqs_inline = args->txqs_inline;
> +	if (args->mps != MLX5_ARG_UNSET)
> +		priv->mps = args->mps ? priv->mps : 0;
> +	if (args->mpw_hdr_dseg != MLX5_ARG_UNSET)
> +		priv->mpw_hdr_dseg = args->mpw_hdr_dseg;
> +	if (args->inline_max_packet_sz != MLX5_ARG_UNSET)
> +		priv->inline_max_packet_sz = args->inline_max_packet_sz;
> +	if (args->tso != MLX5_ARG_UNSET)
> +		priv->tso = args->tso;
> +}
> +
> +/**
>   * DPDK callback to register a PCI device.
>   *
>   * This function creates an Ethernet device for each port of a given
> @@ -502,6 +542,15 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  		struct ibv_exp_device_attr exp_device_attr;
>  		struct ether_addr mac;
>  		uint16_t num_vfs = 0;
> +		struct mlx5_args args = {
> +			.cqe_comp = MLX5_ARG_UNSET,
> +			.txq_inline = MLX5_ARG_UNSET,
> +			.txqs_inline = MLX5_ARG_UNSET,
> +			.mps = MLX5_ARG_UNSET,
> +			.mpw_hdr_dseg = MLX5_ARG_UNSET,
> +			.inline_max_packet_sz = MLX5_ARG_UNSET,
> +			.tso = MLX5_ARG_UNSET,
> +		};
>  
>  		exp_device_attr.comp_mask =
>  			IBV_EXP_DEVICE_ATTR_EXP_CAP_FLAGS |
> @@ -570,12 +619,13 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  		}
>  		priv->cqe_comp = 1; /* Enable compression by default. */
>  		priv->tunnel_en = tunnel_en;
> -		err = mlx5_args(priv, pci_dev->device.devargs);
> +		err = mlx5_args(&args, pci_dev->device.devargs);
>  		if (err) {
>  			ERROR("failed to process device arguments: %s",
>  			      strerror(err));
>  			goto port_error;
>  		}
> +		mlx5_args_assign(priv, &args);
>  		if (ibv_exp_query_device(ctx, &exp_device_attr)) {
>  			ERROR("ibv_exp_query_device() failed");
>  			goto port_error;
> -- 
> 2.12.0
> 

For the series,

Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

-- 
Nélio Laranjeiro
6WIND

Re: [PATCH v2 2/2] net/mlx5: fix PMD specific parameters defaults

[Ferruh Yigit]

On 4/18/2017 11:22 AM, Shahaf Shuler wrote:
> With the Enhanced multi packet send addition, the defaults were made
> in order to get the maximum out of the box performance.
> Features like tso, don't use the enhanced send, however the defaults
> are still valid. This cause Tx queue creation to fail.
> 
> Fixes: aea00c008140 ("net/mlx5: add hardware TSO support")

Can you please confirm the Fixes line, the code modified below added
with commit [1], and above commit (correct one is: 3f13f8c23a7c) doesn't
touch those lines?

[1]
Fixes: 6ce84bd88919 ("net/mlx5: add enhanced multi-packet send for
ConnectX-5")

Re: [PATCH v2 2/2] net/mlx5: fix PMD specific parameters defaults

[Shahaf Shuler]

Wednesday, April 19, 2017 1:57 PM, Ferruh Yigit:
> On 4/18/2017 11:22 AM, Shahaf Shuler wrote:
> > With the Enhanced multi packet send addition, the defaults were made
> > in order to get the maximum out of the box performance.
> > Features like tso, don't use the enhanced send, however the defaults
> > are still valid. This cause Tx queue creation to fail.
> >
> > Fixes: aea00c008140 ("net/mlx5: add hardware TSO support")
> 
> Can you please confirm the Fixes line, the code modified below added with
> commit [1], and above commit (correct one is: 3f13f8c23a7c) doesn't touch
> those lines?

Well I guess it can be either. 
The commit Fixes TSO because [1] caused TSO to fail on some cases.
The commit Fixes multi-packet send since it is the one which caused the fault.

Since you pointed that out, I guess other will follow your logic as well. So let's take you suggestion.
Do you need me to post a v3? 

> 
> [1]
> Fixes: 6ce84bd88919 ("net/mlx5: add enhanced multi-packet send for
> ConnectX-5")

Re: [PATCH v2 2/2] net/mlx5: fix PMD specific parameters defaults

[Ferruh Yigit]

On 4/19/2017 12:32 PM, Shahaf Shuler wrote:
> Wednesday, April 19, 2017 1:57 PM, Ferruh Yigit:
>> On 4/18/2017 11:22 AM, Shahaf Shuler wrote:
>>> With the Enhanced multi packet send addition, the defaults were made
>>> in order to get the maximum out of the box performance.
>>> Features like tso, don't use the enhanced send, however the defaults
>>> are still valid. This cause Tx queue creation to fail.
>>>
>>> Fixes: aea00c008140 ("net/mlx5: add hardware TSO support")
>>
>> Can you please confirm the Fixes line, the code modified below added with
>> commit [1], and above commit (correct one is: 3f13f8c23a7c) doesn't touch
>> those lines?
> 
> Well I guess it can be either. 
> The commit Fixes TSO because [1] caused TSO to fail on some cases.
> The commit Fixes multi-packet send since it is the one which caused the fault.
> 
> Since you pointed that out, I guess other will follow your logic as well. So let's take you suggestion.

Does it make sense to add both?

> Do you need me to post a v3? 

If you confirm, I can update while applying, not a new patch required.

> 
>>
>> [1]
>> Fixes: 6ce84bd88919 ("net/mlx5: add enhanced multi-packet send for
>> ConnectX-5")

Re: [PATCH v2 2/2] net/mlx5: fix PMD specific parameters defaults

[Shahaf Shuler]

Wednesday, April 19, 2017 3:25 PM, Ferruh Yigit:
 
> On 4/19/2017 12:32 PM, Shahaf Shuler wrote:
> > Wednesday, April 19, 2017 1:57 PM, Ferruh Yigit:
> >> On 4/18/2017 11:22 AM, Shahaf Shuler wrote:
> >>> With the Enhanced multi packet send addition, the defaults were made
> >>> in order to get the maximum out of the box performance.
> >>> Features like tso, don't use the enhanced send, however the defaults
> >>> are still valid. This cause Tx queue creation to fail.
> >>>
> >>> Fixes: aea00c008140 ("net/mlx5: add hardware TSO support")
> >>
> >> Can you please confirm the Fixes line, the code modified below added
> >> with commit [1], and above commit (correct one is: 3f13f8c23a7c)
> >> doesn't touch those lines?
> >
> > Well I guess it can be either.
> > The commit Fixes TSO because [1] caused TSO to fail on some cases.
> > The commit Fixes multi-packet send since it is the one which caused the
> fault.
> >
> > Since you pointed that out, I guess other will follow your logic as well. So
> let's take you suggestion.
> 
> Does it make sense to add both?
> 
> > Do you need me to post a v3?
> 
> If you confirm, I can update while applying, not a new patch required.

Agreed. Let's have both. 

> 
> >
> >>
> >> [1]
> >> Fixes: 6ce84bd88919 ("net/mlx5: add enhanced multi-packet send for
> >> ConnectX-5")

Re: [PATCH v2 1/2] net/mlx5: modify PMD args process

[Ferruh Yigit]

On 4/19/2017 7:50 AM, Nélio Laranjeiro wrote:
> On Tue, Apr 18, 2017 at 01:22:27PM +0300, Shahaf Shuler wrote:
>> Currently the argument process is done without indication which
>> parameter was forced by the application and which one is on it
>> default value.
>> This becomes problematic when different features requires different
>> defaults. For example, Enhanced multi packet send and TSO.
>>
>> This commit modifies the argument process, enabling to differ
>> which parameter was forced by the application.
>>
>> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
>> Acked-by: Yongseok Koh <yskoh@mellanox.com>

> For the series,
> 
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Series applied to dpdk-next-net/master, thanks.