Re: [dpdk-dev] [PATCH 09/11] net/mlx5: fix the set VLAN VID action validation

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----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