[dpdk-dev] [PATCH] net/mlx5: fix metadata split with encap action

Slava Ovsiienko viacheslavo at mellanox.com
Thu Feb 20 16:02:04 CET 2020


> -----Original Message-----
> From: Matan Azrad <matan at mellanox.com>
> Sent: Thursday, February 20, 2020 16:44
> To: dev at dpdk.org
> Cc: Slava Ovsiienko <viacheslavo at mellanox.com>; stable at dpdk.org
> Subject: [PATCH] net/mlx5: fix metadata split with encap action
> 
> In order to move the mbuf metadata from the WQE to the FDB steering
> domain, the PMD add for each NIC TX flow a new action to copy the metadata
> register REG_A to REG_C_0.
> 
> This copy action is considered as modify header action from HW perspective.
> 
> The HW doesn't support to do modify header action after ant encapsulation
> action.
> 
> The split metadata function wrongly added the copy action in the end of the
> original actions list, hence, NIC egress flow with encapapsulation action failed
> when the PMD worked with dv_xmeta_en mode.
> 
> Move the copy action to be before and back to back with the encapsulation
> action for the aforementioned case.
> 
> Fixes: 71e254bc0294 ("net/mlx5: split Rx flows to provide metadata copy")
> Cc: stable at dpdk.org
> 
> Signed-off-by: Matan Azrad <matan at mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo at mellanox.com>

PS. Raslan - please, fix typos in commit message, thanks


> ---
>  drivers/net/mlx5/mlx5_flow.c | 66 ++++++++++++++++++++++++++++++++++---
> -------
>  1 file changed, 51 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index fa58546..60aab16 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -2744,7 +2744,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,  }
> 
>  /**
> - * Get QUEUE/RSS action from the action list.
> + * Get metadata split action information.
>   *
>   * @param[in] actions
>   *   Pointer to the list of actions.
> @@ -2753,18 +2753,38 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>   * @param[out] qrss_type
>   *   Pointer to the action type to return. RTE_FLOW_ACTION_TYPE_END is
> returned
>   *   if no QUEUE/RSS is found.
> + * @param[out] encap_idx
> + *   Pointer to the index of the encap action if exists, otherwise the last
> + *   action index.
>   *
>   * @return
>   *   Total number of actions.
>   */
>  static int
> -flow_parse_qrss_action(const struct rte_flow_action actions[],
> -		       const struct rte_flow_action **qrss)
> +flow_parse_metadata_split_actions_info(const struct rte_flow_action
> actions[],
> +				       const struct rte_flow_action **qrss,
> +				       int *encap_idx)
>  {
> +	const struct rte_flow_action_raw_encap *raw_encap;
>  	int actions_n = 0;
> +	int raw_decap_idx = -1;
> 
> +	*encap_idx = -1;
>  	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
>  		switch (actions->type) {
> +		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
> +		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
> +			*encap_idx = actions_n;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
> +			raw_decap_idx = actions_n;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> +			raw_encap = actions->conf;
> +			if (raw_encap->size >
> MLX5_ENCAPSULATION_DECISION_SIZE)
> +				*encap_idx = raw_decap_idx != -1 ?
> +						      raw_decap_idx :
> actions_n;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_QUEUE:
>  		case RTE_FLOW_ACTION_TYPE_RSS:
>  			*qrss = actions;
> @@ -2774,6 +2794,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  		}
>  		actions_n++;
>  	}
> +	if (*encap_idx == -1)
> +		*encap_idx = actions_n;
>  	/* Count RTE_FLOW_ACTION_TYPE_END. */
>  	return actions_n + 1;
>  }
> @@ -3739,6 +3761,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>   *   Number of actions in the list.
>   * @param[out] error
>   *   Perform verbose error reporting if not NULL.
> + * @param[in] encap_idx
> + *   The encap action inndex.
>   *
>   * @return
>   *   0 on success, negative value otherwise
> @@ -3747,7 +3771,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,  flow_mreg_tx_copy_prep(struct
> rte_eth_dev *dev,
>  		       struct rte_flow_action *ext_actions,
>  		       const struct rte_flow_action *actions,
> -		       int actions_n, struct rte_flow_error *error)
> +		       int actions_n, struct rte_flow_error *error,
> +		       int encap_idx)
>  {
>  	struct mlx5_flow_action_copy_mreg *cp_mreg =
>  		(struct mlx5_flow_action_copy_mreg *) @@ -3762,15
> +3787,24 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev,
> int32_t priority,
>  	if (ret < 0)
>  		return ret;
>  	cp_mreg->src = ret;
> -	memcpy(ext_actions, actions,
> -			sizeof(*ext_actions) * actions_n);
> -	ext_actions[actions_n - 1] = (struct rte_flow_action){
> -		.type = MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
> -		.conf = cp_mreg,
> -	};
> -	ext_actions[actions_n] = (struct rte_flow_action){
> -		.type = RTE_FLOW_ACTION_TYPE_END,
> -	};
> +	if (encap_idx != 0)
> +		memcpy(ext_actions, actions, sizeof(*ext_actions) *
> encap_idx);
> +	if (encap_idx == actions_n - 1) {
> +		ext_actions[actions_n - 1] = (struct rte_flow_action){
> +			.type = MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
> +			.conf = cp_mreg,
> +		};
> +		ext_actions[actions_n] = (struct rte_flow_action){
> +			.type = RTE_FLOW_ACTION_TYPE_END,
> +		};
> +	} else {
> +		ext_actions[encap_idx] = (struct rte_flow_action){
> +			.type = MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
> +			.conf = cp_mreg,
> +		};
> +		memcpy(ext_actions + encap_idx + 1, actions + encap_idx,
> +				sizeof(*ext_actions) * (actions_n -
> encap_idx));
> +	}
>  	return 0;
>  }
> 
> @@ -3821,6 +3855,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  	int mtr_sfx = 0;
>  	size_t act_size;
>  	int actions_n;
> +	int encap_idx;
>  	int ret;
> 
>  	/* Check whether extensive metadata feature is engaged. */ @@ -
> 3830,7 +3865,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev
> *dev, int32_t priority,
>  		return flow_create_split_inner(dev, flow, NULL, prefix_layers,
>  					       attr, items, actions, external,
>  					       error);
> -	actions_n = flow_parse_qrss_action(actions, &qrss);
> +	actions_n = flow_parse_metadata_split_actions_info(actions, &qrss,
> +							   &encap_idx);
>  	if (qrss) {
>  		/* Exclude hairpin flows from splitting. */
>  		if (qrss->type == RTE_FLOW_ACTION_TYPE_QUEUE) { @@ -
> 3905,7 +3941,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev
> *dev, int32_t priority,
>  						  "metadata flow");
>  		/* Create the action list appended with copy register. */
>  		ret = flow_mreg_tx_copy_prep(dev, ext_actions, actions,
> -					     actions_n, error);
> +					     actions_n, error, encap_idx);
>  		if (ret < 0)
>  			goto exit;
>  	}
> --
> 1.8.3.1



More information about the dev mailing list