Skip to contentSkip to conversation

net/mlx5: fix devargs validation for multiclass probe

PATCH In review [email protected] 3 participants 4 messages Nov 25, 2021 – Nov 26, 2021 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 net/mlx5: fix devargs validation for multiclass probe

From: Michael Baum <[email protected]>

The mlx5_args function reads the devargs and checks if they are valid for this driver and if not it returns an error.

This was normal behavior as long as all the devargs come to this driver, but since it is possible to run several drivers together, the function may return an error for another driver's devarg even though it is completely valid.
In addition the function does not allow the user to know which of the devargs he sent is incorrect, but returns an error without printing the unknown devarg.

This patch eliminates the error return in the case of an unknown devarg, and prints a warning for each such devarg specifically.

Fixes 7b4f1e6bd367 ("common/mlx5: introduce common library")
---
 drivers/net/mlx5/mlx5.c | 68 ++++++-----------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)
drivers/net/mlx5/mlx5.c +9 -59
index 4e04817d11..aa5f313c1a 100644
@@ -1975,9 +1975,9 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
 		config->std_delay_drop = !!(tmp & MLX5_DELAY_DROP_STANDARD);
 		config->hp_delay_drop = !!(tmp & MLX5_DELAY_DROP_HAIRPIN);
 	} else {
-		DRV_LOG(WARNING, "%s: unknown parameter", key);
-		rte_errno = EINVAL;
-		return -rte_errno;
+		DRV_LOG(WARNING,
+			"%s: unknown parameter, maybe it's for another class.",
+			key);
 	}
 	return 0;
 }
@@ -1996,75 +1996,25 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
 int
 mlx5_args(struct mlx5_dev_config *config, struct rte_devargs *devargs)
 {
-	const char **params = (const char *[]){
-		MLX5_DRIVER_KEY,
-		MLX5_RXQ_CQE_COMP_EN,
-		MLX5_RXQ_PKT_PAD_EN,
-		MLX5_RX_MPRQ_EN,
-		MLX5_RX_MPRQ_LOG_STRIDE_NUM,
-		MLX5_RX_MPRQ_LOG_STRIDE_SIZE,
-		MLX5_RX_MPRQ_MAX_MEMCPY_LEN,
-		MLX5_RXQS_MIN_MPRQ,
-		MLX5_TXQ_INLINE,
-		MLX5_TXQ_INLINE_MIN,
-		MLX5_TXQ_INLINE_MAX,
-		MLX5_TXQ_INLINE_MPW,
-		MLX5_TXQS_MIN_INLINE,
-		MLX5_TXQS_MAX_VEC,
-		MLX5_TXQ_MPW_EN,
-		MLX5_TXQ_MPW_HDR_DSEG_EN,
-		MLX5_TXQ_MAX_INLINE_LEN,
-		MLX5_TX_DB_NC,
-		MLX5_TX_PP,
-		MLX5_TX_SKEW,
-		MLX5_TX_VEC_EN,
-		MLX5_RX_VEC_EN,
-		MLX5_L3_VXLAN_EN,
-		MLX5_VF_NL_EN,
-		MLX5_DV_ESW_EN,
-		MLX5_DV_FLOW_EN,
-		MLX5_DV_XMETA_EN,
-		MLX5_LACP_BY_USER,
-		MLX5_MR_EXT_MEMSEG_EN,
-		MLX5_REPRESENTOR,
-		MLX5_MAX_DUMP_FILES_NUM,
-		MLX5_LRO_TIMEOUT_USEC,
-		RTE_DEVARGS_KEY_CLASS,
-		MLX5_HP_BUF_SIZE,
-		MLX5_RECLAIM_MEM,
-		MLX5_SYS_MEM_EN,
-		MLX5_DECAP_EN,
-		MLX5_ALLOW_DUPLICATE_PATTERN,
-		MLX5_MR_MEMPOOL_REG_EN,
-		MLX5_DELAY_DROP,
-		NULL,
-	};
 	struct rte_kvargs *kvlist;
 	int ret = 0;
-	int i;
 
 	if (devargs == NULL)
 		return 0;
 	/* Following UGLY cast is done to pass checkpatch. */
-	kvlist = rte_kvargs_parse(devargs->args, params);
+	kvlist = rte_kvargs_parse(devargs->args, NULL);
 	if (kvlist == NULL) {
 		rte_errno = EINVAL;
 		return -rte_errno;
 	}
 	/* Process parameters. */
-	for (i = 0; (params[i] != NULL); ++i) {
-		if (rte_kvargs_count(kvlist, params[i])) {
-			ret = rte_kvargs_process(kvlist, params[i],
-						 mlx5_args_check, config);
-			if (ret) {
-				rte_errno = EINVAL;
-				rte_kvargs_free(kvlist);
-				return -rte_errno;
-			}
-		}
+	ret = rte_kvargs_process(kvlist, NULL, mlx5_args_check, config);
+	if (ret) {
+		rte_errno = EINVAL;
+		ret = -rte_errno;
 	}
 	rte_kvargs_free(kvlist);
-	return 0;
+	return ret;
 }
 
 /**
Signature · 1 line
2.25.1
#1 lore raw

25/11/2021 07:16, [email protected]:

Quoted 16 lines

From: Michael Baum <[email protected]>

The mlx5_args function reads the devargs and checks if they are valid for this driver and if not it returns an error.

This was normal behavior as long as all the devargs come to this driver, but since it is possible to run several drivers together, the function may return an error for another driver's devarg even though it is completely valid.
In addition the function does not allow the user to know which of the devargs he sent is incorrect, but returns an error without printing the unknown devarg.

This patch eliminates the error return in the case of an unknown devarg, and prints a warning for each such devarg specifically.

Quoted 8 lines
Fixes 7b4f1e6bd367 ("common/mlx5: introduce common library")
---
 drivers/net/mlx5/mlx5.c | 68 ++++++-----------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)

It is quite a big patch.
How well it has been tested?
How critical it is to have in 21.11?

#2 lore raw

From: Thomas Monjalon

Quoted 27 lines

25/11/2021 07:16, [email protected]:

Quoted 19 lines

From: Michael Baum <[email protected]>

The mlx5_args function reads the devargs and checks if they are valid for this driver and if not it returns an error.

This was normal behavior as long as all the devargs come to this driver, but since it is possible to run several drivers together, the function may return an error for another driver's devarg even though it is completely valid.
In addition the function does not allow the user to know which of the devargs he sent is incorrect, but returns an error without printing the unknown devarg.

This patch eliminates the error return in the case of an unknown devarg, and prints a warning for each such devarg specifically.

Fixes 7b4f1e6bd367 ("common/mlx5: introduce common library")
Quoted 6 lines
---
 drivers/net/mlx5/mlx5.c | 68
++++++-----------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)
Quoted 2 lines

It is quite a big patch.
How well it has been tested?

It was tested carefully to see that devargs affect the actual configuration and also to allow running !net classes devargs with the net class.

Quoted 1 line

How critical it is to have in 21.11?

CRYPTO class cannot run with NET\ETH on the same device. No significant risk here; I suggest taking.

#3 lore raw

25/11/2021 11:38, Matan Azrad:

Quoted 31 lines

From: Thomas Monjalon

Quoted 27 lines

25/11/2021 07:16, [email protected]:

Quoted 19 lines

From: Michael Baum <[email protected]>

The mlx5_args function reads the devargs and checks if they are valid for this driver and if not it returns an error.

This was normal behavior as long as all the devargs come to this driver, but since it is possible to run several drivers together, the function may return an error for another driver's devarg even though it is completely valid.
In addition the function does not allow the user to know which of the devargs he sent is incorrect, but returns an error without printing the unknown devarg.

This patch eliminates the error return in the case of an unknown devarg, and prints a warning for each such devarg specifically.

Fixes 7b4f1e6bd367 ("common/mlx5: introduce common library")
Quoted 6 lines
---
 drivers/net/mlx5/mlx5.c | 68
++++++-----------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)
Quoted 2 lines

It is quite a big patch.
How well it has been tested?

Quoted 6 lines

It was tested carefully to see that devargs affect the actual configuration and also to allow running !net classes devargs with the net class.

Quoted 1 line

How critical it is to have in 21.11?

CRYPTO class cannot run with NET\ETH on the same device. No significant risk here; I suggest taking.

OK, applied, thanks.