DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate
@ 2021-08-09  5:44 wenxu
  2021-09-21 10:09 ` Slava Ovsiienko
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: wenxu @ 2021-08-09  5:44 UTC (permalink / raw)
  To: getelson; +Cc: dev

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


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

* Re: [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate
  2021-08-09  5:44 [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate wenxu
@ 2021-09-21 10:09 ` Slava Ovsiienko
  2021-09-22 19:16 ` Thomas Monjalon
  2022-05-30 11:00 ` Slava Ovsiienko
  2 siblings, 0 replies; 4+ messages in thread
From: Slava Ovsiienko @ 2021-09-21 10:09 UTC (permalink / raw)
  To: wenxu, Gregory Etelson; +Cc: dev

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


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

* Re: [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate
  2021-08-09  5:44 [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate wenxu
  2021-09-21 10:09 ` Slava Ovsiienko
@ 2021-09-22 19:16 ` Thomas Monjalon
  2022-05-30 11:00 ` Slava Ovsiienko
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2021-09-22 19:16 UTC (permalink / raw)
  To: wenxu; +Cc: getelson, dev, viacheslavo

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



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

* RE: [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate
  2021-08-09  5:44 [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate wenxu
  2021-09-21 10:09 ` Slava Ovsiienko
  2021-09-22 19:16 ` Thomas Monjalon
@ 2022-05-30 11:00 ` Slava Ovsiienko
  2 siblings, 0 replies; 4+ messages in thread
From: Slava Ovsiienko @ 2022-05-30 11:00 UTC (permalink / raw)
  To: wenxu, Gregory Etelson, Raslan Darawsheh; +Cc: dev

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


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

end of thread, other threads:[~2022-05-30 11:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09  5:44 [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate wenxu
2021-09-21 10:09 ` Slava Ovsiienko
2021-09-22 19:16 ` Thomas Monjalon
2022-05-30 11:00 ` Slava Ovsiienko

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