Function __flow_hw_translate_actions_template contains several
encapsulated functions that already have internal error handling
process via rte_flow_error_set for each case.

Thus the one (rte_flow_error_set) within the goto statement `err`
at the end of __flow_hw_translate_actions_template function may be
redundant for those failed cases. As a result, the error messages
would all be overwritten as "fail to create rte table", making it
displayed at quite large granularity.

To prevent above error messages overwrite, this patch add a local
variable `struct rte_flow_error sub_error` to the function and pass
this `sub_error` instead of `error` to each sub-function. Under error
handling process (`err` label), if `sub_error` was updated, copy its
contents to `error` and return. If it was not updated, return default
error message (`fail to create rte table`).

Also refactor the logic for SEND_TO_KERNEL, COUNT and AGE actions in
above function to align the error handling process.

Fixes: f13fab23922b ("net/mlx5: add flow jump action")
Cc: suanmi...@nvidia.com
Cc: sta...@dpdk.org

Signed-off-by: Junfeng Guo <junfe...@nvidia.com>
---
 .mailmap                        |  2 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 65 ++++++++++++++++++++-------------
 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/.mailmap b/.mailmap
index a03d3cfb59..9f053334dc 100644
--- a/.mailmap
+++ b/.mailmap
@@ -760,7 +760,7 @@ Julien Hascoet <ju.hasc...@gmail.com>
 Julien Massonneau <julien.massonn...@6wind.com>
 Julien Meunier <julien.meun...@nokia.com> <julien.meun...@6wind.com>
 JĂșlius Milan <jmilan....@gmail.com>
-Junfeng Guo <junfeng....@intel.com>
+Junfeng Guo <junfe...@nvidia.com> <junfeng....@intel.com>
 Junjie Chen <junjie.j.c...@intel.com>
 Junjie Wan <wanjun...@bytedance.com>
 Junlong Wang <wang.junlo...@zte.com.cn>
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 501bf33f94..e72b87d70f 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2543,6 +2543,11 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
        uint8_t *push_data = NULL, *push_data_m = NULL;
        size_t data_size = 0, push_size = 0;
        struct mlx5_hw_modify_header_action mhdr = { 0 };
+       struct rte_flow_error sub_error = {
+               .type = RTE_FLOW_ERROR_TYPE_NONE,
+               .cause = NULL,
+               .message = NULL,
+       };
        bool actions_end = false;
        uint32_t type;
        bool reformat_used = false;
@@ -2662,7 +2667,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                                        ((const struct rte_flow_action_jump *)
                                        actions->conf)->group;
                                acts->jump = flow_hw_jump_action_register
-                                               (dev, cfg, jump_group, error);
+                                               (dev, cfg, jump_group, 
&sub_error);
                                if (!acts->jump)
                                        goto err;
                                acts->rule_acts[dr_pos].action = 
(!!attr->group) ?
@@ -2793,15 +2798,16 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                        break;
                case RTE_FLOW_ACTION_TYPE_SEND_TO_KERNEL:
                        ret = flow_hw_translate_group(dev, cfg, attr->group,
-                                               &target_grp, error);
+                                               &target_grp, &sub_error);
                        if (ret)
-                               return ret;
+                               goto err;
                        if (target_grp == 0) {
                                __flow_hw_action_template_destroy(dev, acts);
-                               return rte_flow_error_set(error, ENOTSUP,
-                                               RTE_FLOW_ERROR_TYPE_ACTION,
-                                               NULL,
-                                               "Send to kernel action on root 
table is not supported in HW steering mode");
+                               rte_flow_error_set(&sub_error, ENOTSUP,
+                                       RTE_FLOW_ERROR_TYPE_ACTION,
+                                       NULL,
+                                       "Send to kernel action on root table is 
not supported in HW steering mode");
+                               goto err;
                        }
                        table_type = attr->ingress ? MLX5DR_TABLE_TYPE_NIC_RX :
                                     ((attr->egress) ? MLX5DR_TABLE_TYPE_NIC_TX 
:
@@ -2811,14 +2817,14 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                case RTE_FLOW_ACTION_TYPE_MODIFY_FIELD:
                        err = flow_hw_modify_field_compile(dev, attr, actions,
                                                           masks, acts, &mhdr,
-                                                          src_pos, error);
+                                                          src_pos, &sub_error);
                        if (err)
                                goto err;
                        break;
                case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
                        if (flow_hw_represented_port_compile
                                        (dev, attr, actions,
-                                        masks, acts, src_pos, dr_pos, error))
+                                        masks, acts, src_pos, dr_pos, 
&sub_error))
                                goto err;
                        break;
                case RTE_FLOW_ACTION_TYPE_METER:
@@ -2832,7 +2838,8 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                            ((const struct rte_flow_action_meter *)
                             masks->conf)->mtr_id) {
                                err = flow_hw_meter_compile(dev, cfg,
-                                                           dr_pos, jump_pos, 
actions, acts, error);
+                                                           dr_pos, jump_pos, 
actions, acts,
+                                                           &sub_error);
                                if (err)
                                        goto err;
                        } else if (__flow_hw_act_data_general_append(priv, acts,
@@ -2843,15 +2850,16 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                        break;
                case RTE_FLOW_ACTION_TYPE_AGE:
                        ret = flow_hw_translate_group(dev, cfg, attr->group,
-                                               &target_grp, error);
+                                               &target_grp, &sub_error);
                        if (ret)
-                               return ret;
+                               goto err;
                        if (target_grp == 0) {
                                __flow_hw_action_template_destroy(dev, acts);
-                               return rte_flow_error_set(error, ENOTSUP,
-                                               RTE_FLOW_ERROR_TYPE_ACTION,
-                                               NULL,
-                                               "Age action on root table is 
not supported in HW steering mode");
+                               rte_flow_error_set(&sub_error, ENOTSUP,
+                                       RTE_FLOW_ERROR_TYPE_ACTION,
+                                       NULL,
+                                       "Age action on root table is not 
supported in HW steering mode");
+                               goto err;
                        }
                        if (__flow_hw_act_data_general_append(priv, acts,
                                                              actions->type,
@@ -2861,15 +2869,16 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                        break;
                case RTE_FLOW_ACTION_TYPE_COUNT:
                        ret = flow_hw_translate_group(dev, cfg, attr->group,
-                                               &target_grp, error);
+                                               &target_grp, &sub_error);
                        if (ret)
-                               return ret;
+                               goto err;
                        if (target_grp == 0) {
                                __flow_hw_action_template_destroy(dev, acts);
-                               return rte_flow_error_set(error, ENOTSUP,
-                                               RTE_FLOW_ERROR_TYPE_ACTION,
-                                               NULL,
-                                               "Counter action on root table 
is not supported in HW steering mode");
+                               rte_flow_error_set(&sub_error, ENOTSUP,
+                                       RTE_FLOW_ERROR_TYPE_ACTION,
+                                       NULL,
+                                       "Counter action on root table is not 
supported in HW steering mode");
+                               goto err;
                        }
                        if ((at->action_flags & MLX5_FLOW_ACTION_AGE) ||
                            (at->action_flags & MLX5_FLOW_ACTION_INDIRECT_AGE))
@@ -2912,7 +2921,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                                                                 
acts->rule_acts,
                                                                 &acts->mtr_id,
                                                                 
MLX5_HW_INV_QUEUE,
-                                                                error);
+                                                                &sub_error);
                                if (err)
                                        goto err;
                        } else if (__flow_hw_act_data_general_append(priv, acts,
@@ -2979,11 +2988,11 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                }
        }
        if (mhdr.pos != UINT16_MAX) {
-               ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx, 
&mhdr, error);
+               ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx, 
&mhdr, &sub_error);
                if (ret)
                        goto err;
                if (!nt_mode && mhdr.shared) {
-                       ret = mlx5_tbl_ensure_shared_modify_header(dev, cfg, 
acts, error);
+                       ret = mlx5_tbl_ensure_shared_modify_header(dev, cfg, 
acts, &sub_error);
                        if (ret)
                                goto err;
                }
@@ -2994,7 +3003,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                                                  encap_data, encap_data_m,
                                                  mp_ctx, data_size,
                                                  reformat_src,
-                                                 refmt_type, error);
+                                                 refmt_type, &sub_error);
                if (ret)
                        goto err;
                if (!nt_mode && acts->encap_decap->shared) {
@@ -3020,6 +3029,10 @@ __flow_hw_translate_actions_template(struct rte_eth_dev 
*dev,
                rte_errno = EINVAL;
        err = rte_errno;
        __flow_hw_action_template_destroy(dev, acts);
+       if (error != NULL && sub_error.type != RTE_FLOW_ERROR_TYPE_NONE) {
+               rte_memcpy(error, &sub_error, sizeof(sub_error));
+               return -EINVAL;
+       }
        return rte_flow_error_set(error, err,
                                  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
                                  "fail to create rte table");
-- 
2.34.1

Reply via email to