Re: [dpdk-dev] [PATCH] net/mlx5: fix meter header modify before decap issue

[Raslan Darawsheh <rasland@mellanox.com>]

Hi,
> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Thursday, February 6, 2020 6:14 AM
> To: Matan Azrad <matan@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; xiangxia.m.yue@gmail.com; Raslan Darawsheh
> <rasland@mellanox.com>; stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix meter header modify before decap issue
> 
> The meter flows are split into three subflows each, the prefix subflow
> with meter action color the packet, the meter subflow filters out the
> colored packets, the suffix subflow applies all the remaining actions
> to the passed packets. The tag header modify action is added to the
> prefix subflow to make the suffix subflow to match the packets from the
> prefix subflow.
> 
> Currently, the tag header modify action is added at the beginning in the
> prefix subflow even before decap action. The header modify action does
> not make sense to the later decap action, so the flow create will be
> validated as incorrect flow rule and failed.
> 
> Move the tag header modify action just before meter action in the prefix
> subflow to make the flow with decap action to do the decap first, then
> do the tag and meter to fix that issue.
> 
> Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")
> Cc: stable@dpdk.org
> 
> Reported-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 144e07c..7f1830f 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -3476,21 +3476,25 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  		 struct rte_flow_action actions_sfx[],
>  		 struct rte_flow_action actions_pre[])
>  {
> -	struct rte_flow_action *tag_action;
> +	struct rte_flow_action *tag_action = NULL;
>  	struct mlx5_rte_flow_action_set_tag *set_tag;
>  	struct rte_flow_error error;
>  	const struct rte_flow_action_raw_encap *raw_encap;
>  	const struct rte_flow_action_raw_decap *raw_decap;
>  	uint32_t tag_id;
> 
> -	/* Add the extra tag action first. */
> -	tag_action = actions_pre;
> -	tag_action->type = MLX5_RTE_FLOW_ACTION_TYPE_TAG;
> -	actions_pre++;
>  	/* Prepare the actions for prefix and suffix flow. */
>  	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
>  		switch (actions->type) {
>  		case RTE_FLOW_ACTION_TYPE_METER:
> +			/* Add the extra tag action first. */
> +			tag_action = actions_pre;
> +			tag_action->type =
> MLX5_RTE_FLOW_ACTION_TYPE_TAG;
> +			actions_pre++;
> +			memcpy(actions_pre, actions,
> +			       sizeof(struct rte_flow_action));
> +			actions_pre++;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP:
>  		case RTE_FLOW_ACTION_TYPE_NVGRE_DECAP:
>  			memcpy(actions_pre, actions,
> @@ -3545,6 +3549,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  	 */
>  	tag_id = flow_qrss_get_id(dev);
>  	set_tag->data = tag_id << MLX5_MTR_COLOR_BITS;
> +	assert(tag_action);
>  	tag_action->conf = set_tag;
>  	return tag_id;
>  }
> --
> 1.8.3.1

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh