Hi,
> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Matan Azrad
> Sent: Thursday, February 20, 2020 4:44 PM
> To: dev@dpdk.org
> Cc: Slava Ovsiienko <viachesl...@mellanox.com>; sta...@dpdk.org
> Subject: [dpdk-dev] [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: sta...@dpdk.org
>
> Signed-off-by: Matan Azrad <ma...@mellanox.com>
> ---
> 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
Fixed typo in commit log,
And patch applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
