[dpdk-dev] [PATCH 05/11] net/mlx5: unify validation of drop action

[Slava Ovsiienko]

> -----Original Message-----
> From: Dekel Peled <dekelp at mellanox.com>
> Sent: Wednesday, January 22, 2020 16:27
> To: Matan Azrad <matan at mellanox.com>; Slava Ovsiienko
> <viacheslavo at mellanox.com>
> Cc: Raslan Darawsheh <rasland at mellanox.com>; Ori Kam
> <orika at mellanox.com>; dev at dpdk.org
> Subject: [PATCH 05/11] net/mlx5: unify validation of drop action
> 
> According to PRM: "Drop action is mutually-exclusive with any other action,
> except for Count action".
> In current code this limitaion is checked separately in validation function of
> each action.
> 
> This patch removes the discrete checks, and adds a single check common for
> all actions.
> 
> Signed-off-by: Dekel Peled <dekelp at mellanox.com>
> Acked-by: Ori Kam <orika at mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo at mellanox.com>

> ---
>  drivers/net/mlx5/mlx5_flow.c       | 25 +------------------------
>  drivers/net/mlx5/mlx5_flow_dv.c    | 36 ++++++++++++------------------------
>  drivers/net/mlx5/mlx5_flow_verbs.c | 12 ++++++++++++
>  3 files changed, 25 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 970123b..7738cb2 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -905,11 +905,6 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  			       const struct rte_flow_attr *attr,
>  			       struct rte_flow_error *error)  {
> -
> -	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and flag in same flow");
>  	if (action_flags & MLX5_FLOW_ACTION_MARK)
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL, @@ -961,10 +956,6 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  					  &mark->id,
>  					  "mark id must in 0 <= id < "
>  					  RTE_STR(MLX5_FLOW_MARK_MAX));
> -	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and mark in same flow");
>  	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL, @@ -996,24 +987,10 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>   *   0 on success, a negative errno value otherwise and rte_errno is set.
>   */
>  int
> -mlx5_flow_validate_action_drop(uint64_t action_flags,
> +mlx5_flow_validate_action_drop(uint64_t action_flags __rte_unused,
>  			       const struct rte_flow_attr *attr,
>  			       struct rte_flow_error *error)  {
> -	if (action_flags & MLX5_FLOW_ACTION_FLAG)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and flag in same flow");
> -	if (action_flags & MLX5_FLOW_ACTION_MARK)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and mark in same flow");
> -	if (action_flags & (MLX5_FLOW_FATE_ACTIONS |
> -			    MLX5_FLOW_FATE_ESWITCH_ACTIONS))
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't have 2 fate actions in"
> -					  " same flow");
>  	if (attr->egress)
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, diff --git
> a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index
> 59ece01..b0d5688 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1912,10 +1912,6 @@ struct field_modify_info modify_tcp[] = {
>  	if (ret < 0)
>  		return ret;
>  	assert(ret > 0);
> -	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and flag in same flow");
>  	if (action_flags & MLX5_FLOW_ACTION_MARK)
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL, @@ -1985,10 +1981,6 @@ struct field_modify_info modify_tcp[] = {
> 
> RTE_FLOW_ERROR_TYPE_ACTION_CONF,
>  					  &mark->id,
>  					  "mark id exceeds the limit");
> -	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and mark in same flow");
>  	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL, @@ -2173,10 +2165,6 @@ struct field_modify_info modify_tcp[] = {
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
>  					  "configuration cannot be null");
> -	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and encap in same
> flow");
>  	if (action_flags & (MLX5_FLOW_ENCAP_ACTIONS |
> MLX5_FLOW_DECAP_ACTIONS))
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL, @@ -2209,10 +2197,6 @@ struct field_modify_info modify_tcp[] = {
>  				 const struct rte_flow_attr *attr,
>  				 struct rte_flow_error *error)
>  {
> -	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and decap in same
> flow");
>  	if (action_flags & (MLX5_FLOW_ENCAP_ACTIONS |
> MLX5_FLOW_DECAP_ACTIONS))
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL, @@ -2259,10 +2243,6 @@ struct field_modify_info modify_tcp[] = {
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
>  					  "configuration cannot be null");
> -	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and encap in same
> flow");
>  	if (action_flags & MLX5_FLOW_ENCAP_ACTIONS)
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL, @@ -2306,10 +2286,6 @@ struct field_modify_info modify_tcp[] = {  {
>  	const struct rte_flow_action_raw_decap *decap	= action-
> >conf;
> 
> -	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't drop and decap in same
> flow");
>  	if (action_flags & MLX5_FLOW_ENCAP_ACTIONS)
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL, @@ -5051,6 +5027,18 @@ struct field_modify_info modify_tcp[] = {
>  						  "action not supported");
>  		}
>  	}
> +	/*
> +	 * Validate the drop action mutual exclusion with other actions.
> +	 * Drop action is mutually-exclusive with any other action, except for
> +	 * Count action.
> +	 */
> +	if ((action_flags & MLX5_FLOW_ACTION_DROP) &&
> +	    (action_flags & ~(MLX5_FLOW_ACTION_DROP |
> MLX5_FLOW_ACTION_COUNT)))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> +					  "Drop action is mutually-exclusive "
> +					  "with any other action, except for "
> +					  "Count action");
>  	/* Eswitch has few restrictions on using items and actions */
>  	if (attr->transfer) {
>  		if (!mlx5_flow_ext_mreg_supported(dev) && diff --git
> a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
> index c787c98..72fb1e4 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -1255,6 +1255,18 @@
>  						  "action not supported");
>  		}
>  	}
> +	/*
> +	 * Validate the drop action mutual exclusion with other actions.
> +	 * Drop action is mutually-exclusive with any other action, except for
> +	 * Count action.
> +	 */
> +	if ((action_flags & MLX5_FLOW_ACTION_DROP) &&
> +	    (action_flags & ~(MLX5_FLOW_ACTION_DROP |
> MLX5_FLOW_ACTION_COUNT)))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> +					  "Drop action is mutually-exclusive "
> +					  "with any other action, except for "
> +					  "Count action");
>  	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS))
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
> --
> 1.8.3.1