* [PATCH] net/mlx5: fix err message overwrite for actions translation
@ 2025-02-20 7:08 Junfeng Guo
2025-02-20 9:49 ` Dariusz Sosnowski
0 siblings, 1 reply; 2+ messages in thread
From: Junfeng Guo @ 2025-02-20 7:08 UTC (permalink / raw)
To: dev
Cc: dsosnowski, viacheslavo, bingz, orika, suanmingm, matan, gavinl,
jiaweiw, stable
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: suanmingm@nvidia.com
Cc: stable@dpdk.org
Signed-off-by: Junfeng Guo <junfengg@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.hascoet@gmail.com>
Julien Massonneau <julien.massonneau@6wind.com>
Julien Meunier <julien.meunier@nokia.com> <julien.meunier@6wind.com>
Július Milan <jmilan.dev@gmail.com>
-Junfeng Guo <junfeng.guo@intel.com>
+Junfeng Guo <junfengg@nvidia.com> <junfeng.guo@intel.com>
Junjie Chen <junjie.j.chen@intel.com>
Junjie Wan <wanjunjie@bytedance.com>
Junlong Wang <wang.junlong1@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
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [PATCH] net/mlx5: fix err message overwrite for actions translation
2025-02-20 7:08 [PATCH] net/mlx5: fix err message overwrite for actions translation Junfeng Guo
@ 2025-02-20 9:49 ` Dariusz Sosnowski
0 siblings, 0 replies; 2+ messages in thread
From: Dariusz Sosnowski @ 2025-02-20 9:49 UTC (permalink / raw)
To: Junfeng Guo, dev
Cc: Slava Ovsiienko, Bing Zhao, Ori Kam, Suanming Mou, Matan Azrad,
Minggang(Gavin) Li, Jiawei(Jonny) Wang, stable
> -----Original Message-----
> From: Junfeng Guo <junfengg@nvidia.com>
> Sent: Thursday, February 20, 2025 08:09
> To: dev@dpdk.org
> Cc: Dariusz Sosnowski <dsosnowski@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Bing Zhao <bingz@nvidia.com>; Ori Kam
> <orika@nvidia.com>; Suanming Mou <suanmingm@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Minggang(Gavin) Li <gavinl@nvidia.com>; Jiawei(Jonny)
> Wang <jiaweiw@nvidia.com>; stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix err message overwrite for actions translation
>
> 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: suanmingm@nvidia.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Junfeng Guo <junfengg@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Best regards,
Dariusz Sosnowski
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-02-20 9:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-20 7:08 [PATCH] net/mlx5: fix err message overwrite for actions translation Junfeng Guo
2025-02-20 9:49 ` Dariusz Sosnowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).