diff mbox series

[05/11] net/mlx5: unify validation of drop action

Message ID 26679816cc524d1b5665fc79ba48b89b37d85dd7.1579703134.git.dekelp@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: vlan actions validation fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Dekel Peled Jan. 22, 2020, 2:27 p.m. UTC 
  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@mellanox.com>
Acked-by: Ori Kam <orika@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(-)

Comments

Slava Ovsiienko Jan. 23, 2020, 7:57 a.m. UTC | #1 
> -----Original Message-----
> From: Dekel Peled <dekelp@mellanox.com>
> Sent: Wednesday, January 22, 2020 16:27
> To: Matan Azrad <matan@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Cc: Raslan Darawsheh <rasland@mellanox.com>; Ori Kam
> <orika@mellanox.com>; dev@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@mellanox.com>
> Acked-by: Ori Kam <orika@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@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
diff mbox series

Patch

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,