Re: [dpdk-dev] [PATCH] net/mlx5: fix empty err msg in mlx5_flow_tunnel_validate

[Slava Ovsiienko <viacheslavo@nvidia.com>]

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