Re: [dpdk-dev] [PATCH 5/7] net/mlx5: fix flow validation for no fate action

[Ori Kam <orika@mellanox.com>]

> -----Original Message-----
> From: Yongseok Koh
> Sent: Monday, October 8, 2018 9:02 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 5/7] net/mlx5: fix flow validation for no fate action
> 
> Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
> Fixes: f7adfffa3de1 ("net/mlx5: add Direct Verbs validation function")
> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
> Cc: Ori Kam <orika@mellanox.com>
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c       | 12 +++---------
>  drivers/net/mlx5/mlx5_flow.h       |  3 +++
>  drivers/net/mlx5/mlx5_flow_dv.c    |  4 ++++
>  drivers/net/mlx5/mlx5_flow_tcf.c   |  4 ++++
>  drivers/net/mlx5/mlx5_flow_verbs.c |  4 ++++
>  5 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index c497cacce..30aa95f14 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -742,9 +742,7 @@ mlx5_flow_validate_action_drop(uint64_t action_flags,
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't drop and mark in same flow");
> -	if (action_flags &
> -	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> -	     MLX5_FLOW_ACTION_RSS))
> +	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions in"
> @@ -776,9 +774,7 @@ mlx5_flow_validate_action_queue(const struct
> rte_flow_action *action,
>  	struct priv *priv = dev->data->dev_private;
>  	const struct rte_flow_action_queue *queue = action->conf;
> 
> -	if (action_flags &
> -	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> -	     MLX5_FLOW_ACTION_RSS))
> +	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions in"
> @@ -821,9 +817,7 @@ mlx5_flow_validate_action_rss(const struct
> rte_flow_action *action,
>  	const struct rte_flow_action_rss *rss = action->conf;
>  	unsigned int i;
> 
> -	if (action_flags &
> -	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> -	     MLX5_FLOW_ACTION_RSS))
> +	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions"
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 309e4d4f2..12de841e8 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -79,6 +79,9 @@
>  #define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
>  #define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
> 
> +#define MLX5_FLOW_FATE_ACTIONS \
> +	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> MLX5_FLOW_ACTION_RSS)
> +
>  #ifndef IPPROTO_MPLS
>  #define IPPROTO_MPLS 137
>  #endif
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index e6c84d444..2fb1d9ee7 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -288,6 +288,10 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
> rte_flow_attr *attr,
>  						  "action not supported");
>  		}
>  	}
> +	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
> +					  "no fate action is found");
>  	return 0;
>  }
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index c87046365..4c660d72b 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -697,6 +697,10 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>  						  "action not supported");
>  		}
>  	}
> +	if (!(action_flags & MLX5_TCF_FATE_ACTIONS))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
> +					  "no fate action is found");
>  	return 0;
>  }
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index 0ecbc8121..3df467214 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -1162,6 +1162,10 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>  						  "action not supported");
>  		}
>  	}
> +	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
> +					  "no fate action is found");
>  	return 0;
>  }
> 
> --
> 2.11.0

Acked-by: Ori Kam <orika@mellanox.com>

Thanks,
Ori Kam