[09/11] net/mlx5: fix the set VLAN VID action validation

Message ID 56b43c8675246d3d66ea761b97f6dff8f8417390.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
  Validation function of 'set VLAN VID' action checks twice for existing
same action in flow rule.

This patch updates the validation function logic, to check the same
restrictions more efficiently.

Fixes: 5f163d520cff ("net/mlx5: support modify VLAN ID on existing VLAN header")
Fixes: b8c0372bc5ac ("net/mlx5: fix set VLAN ID/PCP in new header")
Cc: stable@dpdk.org

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)
  

Comments

Slava Ovsiienko Jan. 23, 2020, 7:58 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; stable@dpdk.org
> Subject: [PATCH 09/11] net/mlx5: fix the set VLAN VID action validation
> 
> Validation function of 'set VLAN VID' action checks twice for existing same
> action in flow rule.
> 
> This patch updates the validation function logic, to check the same
> restrictions more efficiently.
> 
> Fixes: 5f163d520cff ("net/mlx5: support modify VLAN ID on existing VLAN
> header")
> Fixes: b8c0372bc5ac ("net/mlx5: fix set VLAN ID/PCP in new header")
> Cc: stable@dpdk.org
> 
> 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_dv.c | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index de4b765..4b6a92c 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1815,10 +1815,10 @@ struct field_modify_info modify_tcp[] = {
>   *
>   * @param[in] item_flags
>   *   Holds the items detected in this rule.
> + * @param[in] action_flags
> + *   Holds the actions detected until now.
>   * @param[in] actions
>   *   Pointer to the list of actions remaining in the flow rule.
> - * @param[in] attr
> - *   Pointer to flow attributes
>   * @param[out] error
>   *   Pointer to error structure.
>   *
> @@ -1838,33 +1838,17 @@ struct field_modify_info modify_tcp[] = {
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
>  					  "VLAN VID value is too big");
> -	/* there is an of_push_vlan action before us */
> -	if (action_flags & MLX5_FLOW_ACTION_OF_PUSH_VLAN) {
> -		if (mlx5_flow_find_action(actions + 1,
> -
> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID))
> -			return rte_flow_error_set(error, ENOTSUP,
> -					RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> -					"Multiple VLAN VID modifications are
> "
> -					"not supported");
> -		else
> -			return 0;
> -	}
> -
> -	/*
> -	 * Action is on an existing VLAN header:
> -	 *    Need to verify this is a single modify CID action.
> -	 *   Rule mast include a match on outer VLAN.
> -	 */
> +	if (!(action_flags & MLX5_FLOW_ACTION_OF_PUSH_VLAN) &&
> +	    !(item_flags & MLX5_FLOW_LAYER_OUTER_VLAN))
> +		return rte_flow_error_set(error, ENOTSUP,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> +					  "set VLAN VID action must follow
> push"
> +					  " VLAN action or match on VLAN
> item");
>  	if (action_flags & MLX5_FLOW_ACTION_OF_SET_VLAN_VID)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
>  					  "Multiple VLAN VID modifications
> are "
>  					  "not supported");
> -	if (!(item_flags & MLX5_FLOW_LAYER_OUTER_VLAN))
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> -					  "match on VLAN is required in order
> "
> -					  "to set VLAN VID");
>  	if (action_flags & MLX5_FLOW_ACTION_PORT_ID)
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> --
> 1.8.3.1
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index de4b765..4b6a92c 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1815,10 +1815,10 @@  struct field_modify_info modify_tcp[] = {
  *
  * @param[in] item_flags
  *   Holds the items detected in this rule.
+ * @param[in] action_flags
+ *   Holds the actions detected until now.
  * @param[in] actions
  *   Pointer to the list of actions remaining in the flow rule.
- * @param[in] attr
- *   Pointer to flow attributes
  * @param[out] error
  *   Pointer to error structure.
  *
@@ -1838,33 +1838,17 @@  struct field_modify_info modify_tcp[] = {
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
 					  "VLAN VID value is too big");
-	/* there is an of_push_vlan action before us */
-	if (action_flags & MLX5_FLOW_ACTION_OF_PUSH_VLAN) {
-		if (mlx5_flow_find_action(actions + 1,
-					  RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID))
-			return rte_flow_error_set(error, ENOTSUP,
-					RTE_FLOW_ERROR_TYPE_ACTION, action,
-					"Multiple VLAN VID modifications are "
-					"not supported");
-		else
-			return 0;
-	}
-
-	/*
-	 * Action is on an existing VLAN header:
-	 *    Need to verify this is a single modify CID action.
-	 *   Rule mast include a match on outer VLAN.
-	 */
+	if (!(action_flags & MLX5_FLOW_ACTION_OF_PUSH_VLAN) &&
+	    !(item_flags & MLX5_FLOW_LAYER_OUTER_VLAN))
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION, action,
+					  "set VLAN VID action must follow push"
+					  " VLAN action or match on VLAN item");
 	if (action_flags & MLX5_FLOW_ACTION_OF_SET_VLAN_VID)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
 					  "Multiple VLAN VID modifications are "
 					  "not supported");
-	if (!(item_flags & MLX5_FLOW_LAYER_OUTER_VLAN))
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, action,
-					  "match on VLAN is required in order "
-					  "to set VLAN VID");
 	if (action_flags & MLX5_FLOW_ACTION_PORT_ID)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,