diff mbox series

net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate

Message ID 1628487855-14030-1-git-send-email-wenxu@ucloud.cn (mailing list archive)
State Not Applicable, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot-build success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

wenxu@ucloud.cn Aug. 9, 2021, 5:44 a.m. UTC 
  From: wenxu <wenxu@ucloud.cn>

If the mlx5_flow_tunnel_validate validate the flow tunnel
rule failed, the err_msg is empty in the rte_flow_error.

Fixes: 4ec6360de37d ("net/mlx5: implement tunnel offload")

Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 drivers/net/mlx5/mlx5_flow.c | 43 ++++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

Comments

Slava Ovsiienko Sept. 21, 2021, 10:09 a.m. UTC | #1 
Hi, Winxu

Thank you for the patch.

> If the mlx5_flow_tunnel_validate validate the flow tunnel rule failed, the
Typo? "validate" -> "validating" ?

> err_msg is empty in the rte_flow_error.
Sorry, what do you mean "empty"? It is NULL, not pointing to empty string "".

Patch looks OK for me, could you, please fix commit message typos and send v2?
 
With best regards,
Slava

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of wenxu@ucloud.cn
> Sent: Monday, August 9, 2021 8:44
> To: Gregory Etelson <getelson@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in
> mlx5_flow_tunnel_validate
> 
> From: wenxu <wenxu@ucloud.cn>
> 
> If the mlx5_flow_tunnel_validate validate the flow tunnel rule failed, the
> err_msg is empty in the rte_flow_error.
> 
> Fixes: 4ec6360de37d ("net/mlx5: implement tunnel offload")
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 43 ++++++++++++++++++++------------------
> -----
>  1 file changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index e63a297..3c5aca0 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -9081,30 +9081,31 @@ int mlx5_alloc_tunnel_hub(struct
> mlx5_dev_ctx_shared *sh)
>  	return err;
>  }
> 
> -static inline bool
> +static inline int
>  mlx5_flow_tunnel_validate(struct rte_eth_dev *dev,
>  			  struct rte_flow_tunnel *tunnel,
> -			  const char *err_msg)
> +			  struct rte_flow_error *error)
>  {
> -	err_msg = NULL;
>  	if (!is_tunnel_offload_active(dev)) {
> -		err_msg = "tunnel offload was not activated";
> -		goto out;
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
> +					  "tunnel offload was not activated");
>  	} else if (!tunnel) {
> -		err_msg = "no application tunnel";
> -		goto out;
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
> +					  "no application tunnel");
>  	}
> 
>  	switch (tunnel->type) {
>  	default:
> -		err_msg = "unsupported tunnel type";
> -		goto out;
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
> +					  "unsupported tunnel type");
>  	case RTE_FLOW_ITEM_TYPE_VXLAN:
>  		break;
>  	}
> 
> -out:
> -	return !err_msg;
> +	return 0;
>  }
> 
>  static int
> @@ -9116,13 +9117,11 @@ int mlx5_alloc_tunnel_hub(struct
> mlx5_dev_ctx_shared *sh)  {
>  	int ret;
>  	struct mlx5_flow_tunnel *tunnel;
> -	const char *err_msg = NULL;
> -	bool verdict = mlx5_flow_tunnel_validate(dev, app_tunnel,
> err_msg);
> 
> -	if (!verdict)
> -		return rte_flow_error_set(error, EINVAL,
> -
> RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
> -					  err_msg);
> +	ret = mlx5_flow_tunnel_validate(dev, app_tunnel, error);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = mlx5_get_flow_tunnel(dev, app_tunnel, &tunnel);
>  	if (ret < 0) {
>  		return rte_flow_error_set(error, ret, @@ -9143,13 +9142,11
> @@ int mlx5_alloc_tunnel_hub(struct mlx5_dev_ctx_shared *sh)  {
>  	int ret;
>  	struct mlx5_flow_tunnel *tunnel;
> -	const char *err_msg = NULL;
> -	bool verdict = mlx5_flow_tunnel_validate(dev, app_tunnel,
> err_msg);
> 
> -	if (!verdict)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_HANDLE,
> NULL,
> -					  err_msg);
> +	ret = mlx5_flow_tunnel_validate(dev, app_tunnel, error);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = mlx5_get_flow_tunnel(dev, app_tunnel, &tunnel);
>  	if (ret < 0) {
>  		return rte_flow_error_set(error, ret,
> --
> 1.8.3.1
Thomas Monjalon Sept. 22, 2021, 7:16 p.m. UTC | #2 
Hi,

> Signed-off-by: wenxu <wenxu@ucloud.cn>

Please could capitalize your name?
Should it be split in 2 words?
Convention in DPDK is to have first name first and family name at the end.

Thanks
Slava Ovsiienko May 30, 2022, 11 a.m. UTC | #3 
Hi,  wenxu

I'm sorry,  it seems the mlx5_flow_tunnel_validate() routine had been
already refactored and fixed, this patch is no longer relevant,

With best regards,
Slava

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of wenxu@ucloud.cn
> Sent: Monday, August 9, 2021 8:44
> To: Gregory Etelson <getelson@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in
> mlx5_flow_tunnel_validate
> 
> From: wenxu <wenxu@ucloud.cn>
> 
> If the mlx5_flow_tunnel_validate validate the flow tunnel rule failed, the
> err_msg is empty in the rte_flow_error.
> 
> Fixes: 4ec6360de37d ("net/mlx5: implement tunnel offload")
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 43 ++++++++++++++++++++-----------------------
>  1 file changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index e63a297..3c5aca0 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -9081,30 +9081,31 @@ int mlx5_alloc_tunnel_hub(struct
> mlx5_dev_ctx_shared *sh)
>  	return err;
>  }
> 
> -static inline bool
> +static inline int
>  mlx5_flow_tunnel_validate(struct rte_eth_dev *dev,
>  			  struct rte_flow_tunnel *tunnel,
> -			  const char *err_msg)
> +			  struct rte_flow_error *error)
>  {
> -	err_msg = NULL;
>  	if (!is_tunnel_offload_active(dev)) {
> -		err_msg = "tunnel offload was not activated";
> -		goto out;
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
> +					  "tunnel offload was not activated");
>  	} else if (!tunnel) {
> -		err_msg = "no application tunnel";
> -		goto out;
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
> +					  "no application tunnel");
>  	}
> 
>  	switch (tunnel->type) {
>  	default:
> -		err_msg = "unsupported tunnel type";
> -		goto out;
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
> +					  "unsupported tunnel type");
>  	case RTE_FLOW_ITEM_TYPE_VXLAN:
>  		break;
>  	}
> 
> -out:
> -	return !err_msg;
> +	return 0;
>  }
> 
>  static int
> @@ -9116,13 +9117,11 @@ int mlx5_alloc_tunnel_hub(struct
> mlx5_dev_ctx_shared *sh)  {
>  	int ret;
>  	struct mlx5_flow_tunnel *tunnel;
> -	const char *err_msg = NULL;
> -	bool verdict = mlx5_flow_tunnel_validate(dev, app_tunnel, err_msg);
> 
> -	if (!verdict)
> -		return rte_flow_error_set(error, EINVAL,
> -
> RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
> -					  err_msg);
> +	ret = mlx5_flow_tunnel_validate(dev, app_tunnel, error);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = mlx5_get_flow_tunnel(dev, app_tunnel, &tunnel);
>  	if (ret < 0) {
>  		return rte_flow_error_set(error, ret, @@ -9143,13 +9142,11
> @@ int mlx5_alloc_tunnel_hub(struct mlx5_dev_ctx_shared *sh)  {
>  	int ret;
>  	struct mlx5_flow_tunnel *tunnel;
> -	const char *err_msg = NULL;
> -	bool verdict = mlx5_flow_tunnel_validate(dev, app_tunnel, err_msg);
> 
> -	if (!verdict)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_HANDLE,
> NULL,
> -					  err_msg);
> +	ret = mlx5_flow_tunnel_validate(dev, app_tunnel, error);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = mlx5_get_flow_tunnel(dev, app_tunnel, &tunnel);
>  	if (ret < 0) {
>  		return rte_flow_error_set(error, ret,
> --
> 1.8.3.1
diff mbox series

Patch

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index e63a297..3c5aca0 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -9081,30 +9081,31 @@  int mlx5_alloc_tunnel_hub(struct mlx5_dev_ctx_shared *sh)
 	return err;
 }
 
-static inline bool
+static inline int
 mlx5_flow_tunnel_validate(struct rte_eth_dev *dev,
 			  struct rte_flow_tunnel *tunnel,
-			  const char *err_msg)
+			  struct rte_flow_error *error)
 {
-	err_msg = NULL;
 	if (!is_tunnel_offload_active(dev)) {
-		err_msg = "tunnel offload was not activated";
-		goto out;
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
+					  "tunnel offload was not activated");
 	} else if (!tunnel) {
-		err_msg = "no application tunnel";
-		goto out;
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
+					  "no application tunnel");
 	}
 
 	switch (tunnel->type) {
 	default:
-		err_msg = "unsupported tunnel type";
-		goto out;
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
+					  "unsupported tunnel type");
 	case RTE_FLOW_ITEM_TYPE_VXLAN:
 		break;
 	}
 
-out:
-	return !err_msg;
+	return 0;
 }
 
 static int
@@ -9116,13 +9117,11 @@  int mlx5_alloc_tunnel_hub(struct mlx5_dev_ctx_shared *sh)
 {
 	int ret;
 	struct mlx5_flow_tunnel *tunnel;
-	const char *err_msg = NULL;
-	bool verdict = mlx5_flow_tunnel_validate(dev, app_tunnel, err_msg);
 
-	if (!verdict)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION_CONF, NULL,
-					  err_msg);
+	ret = mlx5_flow_tunnel_validate(dev, app_tunnel, error);
+	if (ret < 0)
+		return ret;
+
 	ret = mlx5_get_flow_tunnel(dev, app_tunnel, &tunnel);
 	if (ret < 0) {
 		return rte_flow_error_set(error, ret,
@@ -9143,13 +9142,11 @@  int mlx5_alloc_tunnel_hub(struct mlx5_dev_ctx_shared *sh)
 {
 	int ret;
 	struct mlx5_flow_tunnel *tunnel;
-	const char *err_msg = NULL;
-	bool verdict = mlx5_flow_tunnel_validate(dev, app_tunnel, err_msg);
 
-	if (!verdict)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-					  err_msg);
+	ret = mlx5_flow_tunnel_validate(dev, app_tunnel, error);
+	if (ret < 0)
+		return ret;
+
 	ret = mlx5_get_flow_tunnel(dev, app_tunnel, &tunnel);
 	if (ret < 0) {
 		return rte_flow_error_set(error, ret,