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