From: Arnd Bergmann <[email protected]>
The debugfs files always in this driver all use an extra round-trip
through an snprintf() before getting put into a mlx5dr_dbg_dump_buff()
rather than the normal seq_printf().
Zhu Yanjun noticed that the buffers are not initialized before being
filled or reused and requested them to always be zeroed as a
preparation for having more reused between the buffers.
Requested-by: Zhu Yanjun <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
---
.../mellanox/mlx5/core/steering/dr_dbg.c | 35 +++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
index 64f4cc284aea..be7a8481d7d2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
@@ -217,6 +217,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
switch (action->action_type) {
case DR_ACTION_TYP_DROP:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx\n",
DR_DUMP_REC_TYPE_ACTION_DROP, action_id,
@@ -229,6 +230,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_FT:
+ memset(buff, 0, sizeof(buff));
if (action->dest_tbl->is_fw_tbl)
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x,0x%x\n",
@@ -250,6 +252,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_CTR:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_CTR, action_id, rule_id,
@@ -262,6 +265,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_TAG:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_TAG, action_id, rule_id,
@@ -283,6 +287,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
ptrn_arg = !action->rewrite->single_action_opt && ptrn && arg;
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x,%d,0x%x,0x%x,0x%x",
DR_DUMP_REC_TYPE_ACTION_MODIFY_HDR, action_id,
@@ -300,6 +305,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
if (ptrn_arg) {
for (i = 0; i < action->rewrite->num_of_actions; i++) {
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
",0x%016llx",
be64_to_cpu(((__be64 *)rewrite_data)[i]));
@@ -321,6 +327,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
break;
}
case DR_ACTION_TYP_VPORT:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_VPORT, action_id, rule_id,
@@ -333,6 +340,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_TNL_L2_TO_L2:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx\n",
DR_DUMP_REC_TYPE_ACTION_DECAP_L2, action_id,
@@ -345,6 +353,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_TNL_L3_TO_L2:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_DECAP_L3, action_id,
@@ -360,6 +369,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_L2_TO_TNL_L2:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_ENCAP_L2, action_id,
@@ -372,6 +382,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_L2_TO_TNL_L3:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_ENCAP_L3, action_id,
@@ -384,6 +395,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_POP_VLAN:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx\n",
DR_DUMP_REC_TYPE_ACTION_POP_VLAN, action_id,
@@ -396,6 +408,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_PUSH_VLAN:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_PUSH_VLAN, action_id,
@@ -408,6 +421,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_INSERT_HDR:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x,0x%x,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_INSERT_HDR, action_id,
@@ -422,6 +436,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_REMOVE_HDR:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x,0x%x,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_REMOVE_HDR, action_id,
@@ -436,6 +451,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
return ret;
break;
case DR_ACTION_TYP_SAMPLER:
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x,0x%x,0x%x,0x%llx,0x%llx\n",
DR_DUMP_REC_TYPE_ACTION_SAMPLER, action_id,
@@ -468,6 +484,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
DR_DBG_PTR_TO_ID(action->range->miss_tbl_action->dest_tbl->tbl);
}
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x,0x%llx,0x%x,0x%llx,0x%x\n",
DR_DUMP_REC_TYPE_ACTION_MATCH_RANGE, action_id,
@@ -507,6 +524,7 @@ dr_dump_rule_mem(struct seq_file *file, struct mlx5dr_ste *ste,
dr_dump_hex_print(hw_ste_dump, (char *)mlx5dr_ste_get_hw_ste(ste),
DR_STE_SIZE_REDUCED);
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,%s\n", mem_rec_type,
dr_dump_icm_to_idx(mlx5dr_ste_get_icm_addr(ste)),
@@ -554,6 +572,7 @@ static int dr_dump_rule(struct seq_file *file, struct mlx5dr_rule *rule)
format_ver = rule->matcher->tbl->dmn->info.caps.sw_format_ver;
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx\n", DR_DUMP_REC_TYPE_RULE,
rule_id, DR_DBG_PTR_TO_ID(rule->matcher));
@@ -593,6 +612,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
char dump[DR_HEX_SIZE];
int ret;
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH, "%d,0x%llx,",
DR_DUMP_REC_TYPE_MATCHER_MASK, matcher_id);
if (ret < 0)
@@ -602,6 +622,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
if (ret)
return ret;
+ memset(buff, 0, sizeof(buff));
if (criteria & DR_MATCHER_CRITERIA_OUTER) {
dr_dump_hex_print(dump, (char *)&mask->outer, sizeof(mask->outer));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
@@ -617,6 +638,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
if (ret)
return ret;
+ memset(buff, 0, sizeof(buff));
if (criteria & DR_MATCHER_CRITERIA_INNER) {
dr_dump_hex_print(dump, (char *)&mask->inner, sizeof(mask->inner));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
@@ -632,6 +654,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
if (ret)
return ret;
+ memset(buff, 0, sizeof(buff));
if (criteria & DR_MATCHER_CRITERIA_MISC) {
dr_dump_hex_print(dump, (char *)&mask->misc, sizeof(mask->misc));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
@@ -647,6 +670,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
if (ret)
return ret;
+ memset(buff, 0, sizeof(buff));
if (criteria & DR_MATCHER_CRITERIA_MISC2) {
dr_dump_hex_print(dump, (char *)&mask->misc2, sizeof(mask->misc2));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
@@ -662,6 +686,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
if (ret)
return ret;
+ memset(buff, 0, sizeof(buff));
if (criteria & DR_MATCHER_CRITERIA_MISC3) {
dr_dump_hex_print(dump, (char *)&mask->misc3, sizeof(mask->misc3));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
@@ -687,6 +712,7 @@ dr_dump_matcher_builder(struct seq_file *file, struct mlx5dr_ste_build *builder,
char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
int ret;
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,%d,%d,0x%x\n",
DR_DUMP_REC_TYPE_MATCHER_BUILDER, matcher_id, index,
@@ -716,6 +742,7 @@ dr_dump_matcher_rx_tx(struct seq_file *file, bool is_rx,
s_icm_addr = mlx5dr_icm_pool_get_chunk_icm_addr(matcher_rx_tx->s_htbl->chunk);
e_icm_addr = mlx5dr_icm_pool_get_chunk_icm_addr(matcher_rx_tx->e_anchor->chunk);
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,%d,0x%llx,0x%llx\n",
rec_type, DR_DBG_PTR_TO_ID(matcher_rx_tx),
@@ -752,6 +779,7 @@ dr_dump_matcher(struct seq_file *file, struct mlx5dr_matcher *matcher)
matcher_id = DR_DBG_PTR_TO_ID(matcher);
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,%d\n", DR_DUMP_REC_TYPE_MATCHER,
matcher_id, DR_DBG_PTR_TO_ID(matcher->tbl),
@@ -816,6 +844,7 @@ dr_dump_table_rx_tx(struct seq_file *file, bool is_rx,
DR_DUMP_REC_TYPE_TABLE_TX;
s_icm_addr = mlx5dr_icm_pool_get_chunk_icm_addr(table_rx_tx->s_anchor->chunk);
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx\n", rec_type, table_id,
dr_dump_icm_to_idx(s_icm_addr));
@@ -836,6 +865,7 @@ static int dr_dump_table(struct seq_file *file, struct mlx5dr_table *table)
char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
int ret;
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,%d,%d\n", DR_DUMP_REC_TYPE_TABLE,
DR_DBG_PTR_TO_ID(table), DR_DBG_PTR_TO_ID(table->dmn),
@@ -887,6 +917,7 @@ dr_dump_send_ring(struct seq_file *file, struct mlx5dr_send_ring *ring,
char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
int ret;
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%llx,0x%x,0x%x\n",
DR_DUMP_REC_TYPE_DOMAIN_SEND_RING,
@@ -911,6 +942,7 @@ dr_dump_domain_info_flex_parser(struct seq_file *file,
char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
int ret;
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,%s,0x%x\n",
DR_DUMP_REC_TYPE_DOMAIN_INFO_FLEX_PARSER, domain_id,
@@ -937,6 +969,7 @@ dr_dump_domain_info_caps(struct seq_file *file, struct mlx5dr_cmd_caps *caps,
xa_for_each(&caps->vports.vports_caps_xa, vports_num, vport_caps)
; /* count the number of vports in xarray */
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,0x%x,0x%llx,0x%llx,0x%x,%lu,%d\n",
DR_DUMP_REC_TYPE_DOMAIN_INFO_CAPS, domain_id, caps->gvmi,
@@ -952,6 +985,7 @@ dr_dump_domain_info_caps(struct seq_file *file, struct mlx5dr_cmd_caps *caps,
xa_for_each(&caps->vports.vports_caps_xa, i, vport_caps) {
vport_caps = xa_load(&caps->vports.vports_caps_xa, i);
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,%lu,0x%x,0x%llx,0x%llx\n",
DR_DUMP_REC_TYPE_DOMAIN_INFO_VPORT,
@@ -1012,6 +1046,7 @@ dr_dump_domain(struct seq_file *file, struct mlx5dr_domain *dmn)
u64 domain_id = DR_DBG_PTR_TO_ID(dmn);
int ret;
+ memset(buff, 0, sizeof(buff));
ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
"%d,0x%llx,%d,0%x,%d,%u.%u.%u,%s,%d,%u,%u,%u\n",
DR_DUMP_REC_TYPE_DOMAIN,
--
2.39.2
From: Arnd Bergmann <[email protected]>
A couple of debug functions use a 512 byte temporary buffer and call another
function that has another buffer of the same size, which in turn exceeds the
usual warning limit for excessive stack usage:
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c:1073:1: error: stack frame size (1448) exceeds limit (1024) in 'dr_dump_start' [-Werror,-Wframe-larger-than]
dr_dump_start(struct seq_file *file, loff_t *pos)
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c:1009:1: error: stack frame size (1120) exceeds limit (1024) in 'dr_dump_domain' [-Werror,-Wframe-larger-than]
dr_dump_domain(struct seq_file *file, struct mlx5dr_domain *dmn)
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c:705:1: error: stack frame size (1104) exceeds limit (1024) in 'dr_dump_matcher_rx_tx' [-Werror,-Wframe-larger-than]
dr_dump_matcher_rx_tx(struct seq_file *file, bool is_rx,
Rework these so that each of the various code paths only ever has one of
these buffers in it, and exactly the functions that declare one have
the 'noinline_for_stack' annotation that prevents them from all being
inlined into the same caller.
Fixes: 917d1e799ddf ("net/mlx5: DR, Change SWS usage to debug fs seq_file interface")
Signed-off-by: Arnd Bergmann <[email protected]>
---
[v2] no changes, just based on patch 1/2 but can still be applied independently
---
.../mellanox/mlx5/core/steering/dr_dbg.c | 82 +++++++++----------
1 file changed, 41 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
index be7a8481d7d2..eae04f66b8f4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
@@ -205,12 +205,11 @@ dr_dump_hex_print(char hex[DR_HEX_SIZE], char *src, u32 size)
}
static int
-dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
+dr_dump_rule_action_mem(struct seq_file *file, char *buff, const u64 rule_id,
struct mlx5dr_rule_action_member *action_mem)
{
struct mlx5dr_action *action = action_mem->action;
const u64 action_id = DR_DBG_PTR_TO_ID(action);
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
u64 hit_tbl_ptr, miss_tbl_ptr;
u32 hit_tbl_id, miss_tbl_id;
int ret;
@@ -505,10 +504,9 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
}
static int
-dr_dump_rule_mem(struct seq_file *file, struct mlx5dr_ste *ste,
+dr_dump_rule_mem(struct seq_file *file, char *buff, struct mlx5dr_ste *ste,
bool is_rx, const u64 rule_id, u8 format_ver)
{
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
char hw_ste_dump[DR_HEX_SIZE];
u32 mem_rec_type;
int ret;
@@ -540,7 +538,8 @@ dr_dump_rule_mem(struct seq_file *file, struct mlx5dr_ste *ste,
}
static int
-dr_dump_rule_rx_tx(struct seq_file *file, struct mlx5dr_rule_rx_tx *rule_rx_tx,
+dr_dump_rule_rx_tx(struct seq_file *file, char *buff,
+ struct mlx5dr_rule_rx_tx *rule_rx_tx,
bool is_rx, const u64 rule_id, u8 format_ver)
{
struct mlx5dr_ste *ste_arr[DR_RULE_MAX_STES + DR_ACTION_MAX_STES];
@@ -551,7 +550,7 @@ dr_dump_rule_rx_tx(struct seq_file *file, struct mlx5dr_rule_rx_tx *rule_rx_tx,
return 0;
while (i--) {
- ret = dr_dump_rule_mem(file, ste_arr[i], is_rx, rule_id,
+ ret = dr_dump_rule_mem(file, buff, ste_arr[i], is_rx, rule_id,
format_ver);
if (ret < 0)
return ret;
@@ -560,7 +559,8 @@ dr_dump_rule_rx_tx(struct seq_file *file, struct mlx5dr_rule_rx_tx *rule_rx_tx,
return 0;
}
-static int dr_dump_rule(struct seq_file *file, struct mlx5dr_rule *rule)
+static noinline_for_stack int
+dr_dump_rule(struct seq_file *file, struct mlx5dr_rule *rule)
{
struct mlx5dr_rule_action_member *action_mem;
const u64 rule_id = DR_DBG_PTR_TO_ID(rule);
@@ -584,19 +584,19 @@ static int dr_dump_rule(struct seq_file *file, struct mlx5dr_rule *rule)
return ret;
if (rx->nic_matcher) {
- ret = dr_dump_rule_rx_tx(file, rx, true, rule_id, format_ver);
+ ret = dr_dump_rule_rx_tx(file, buff, rx, true, rule_id, format_ver);
if (ret < 0)
return ret;
}
if (tx->nic_matcher) {
- ret = dr_dump_rule_rx_tx(file, tx, false, rule_id, format_ver);
+ ret = dr_dump_rule_rx_tx(file, buff, tx, false, rule_id, format_ver);
if (ret < 0)
return ret;
}
list_for_each_entry(action_mem, &rule->rule_actions_list, list) {
- ret = dr_dump_rule_action_mem(file, rule_id, action_mem);
+ ret = dr_dump_rule_action_mem(file, buff, rule_id, action_mem);
if (ret < 0)
return ret;
}
@@ -605,10 +605,10 @@ static int dr_dump_rule(struct seq_file *file, struct mlx5dr_rule *rule)
}
static int
-dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
+dr_dump_matcher_mask(struct seq_file *file, char *buff,
+ struct mlx5dr_match_param *mask,
u8 criteria, const u64 matcher_id)
{
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
char dump[DR_HEX_SIZE];
int ret;
@@ -706,10 +706,10 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
}
static int
-dr_dump_matcher_builder(struct seq_file *file, struct mlx5dr_ste_build *builder,
+dr_dump_matcher_builder(struct seq_file *file, char *buff,
+ struct mlx5dr_ste_build *builder,
u32 index, bool is_rx, const u64 matcher_id)
{
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
int ret;
memset(buff, 0, sizeof(buff));
@@ -728,11 +728,10 @@ dr_dump_matcher_builder(struct seq_file *file, struct mlx5dr_ste_build *builder,
}
static int
-dr_dump_matcher_rx_tx(struct seq_file *file, bool is_rx,
+dr_dump_matcher_rx_tx(struct seq_file *file, char *buff, bool is_rx,
struct mlx5dr_matcher_rx_tx *matcher_rx_tx,
const u64 matcher_id)
{
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
enum dr_dump_rec_type rec_type;
u64 s_icm_addr, e_icm_addr;
int i, ret;
@@ -758,7 +757,7 @@ dr_dump_matcher_rx_tx(struct seq_file *file, bool is_rx,
return ret;
for (i = 0; i < matcher_rx_tx->num_of_builders; i++) {
- ret = dr_dump_matcher_builder(file,
+ ret = dr_dump_matcher_builder(file, buff,
&matcher_rx_tx->ste_builder[i],
i, is_rx, matcher_id);
if (ret < 0)
@@ -768,7 +767,7 @@ dr_dump_matcher_rx_tx(struct seq_file *file, bool is_rx,
return 0;
}
-static int
+static noinline_for_stack int
dr_dump_matcher(struct seq_file *file, struct mlx5dr_matcher *matcher)
{
struct mlx5dr_matcher_rx_tx *rx = &matcher->rx;
@@ -791,19 +790,19 @@ dr_dump_matcher(struct seq_file *file, struct mlx5dr_matcher *matcher)
if (ret)
return ret;
- ret = dr_dump_matcher_mask(file, &matcher->mask,
+ ret = dr_dump_matcher_mask(file, buff, &matcher->mask,
matcher->match_criteria, matcher_id);
if (ret < 0)
return ret;
if (rx->nic_tbl) {
- ret = dr_dump_matcher_rx_tx(file, true, rx, matcher_id);
+ ret = dr_dump_matcher_rx_tx(file, buff, true, rx, matcher_id);
if (ret < 0)
return ret;
}
if (tx->nic_tbl) {
- ret = dr_dump_matcher_rx_tx(file, false, tx, matcher_id);
+ ret = dr_dump_matcher_rx_tx(file, buff, false, tx, matcher_id);
if (ret < 0)
return ret;
}
@@ -831,11 +830,10 @@ dr_dump_matcher_all(struct seq_file *file, struct mlx5dr_matcher *matcher)
}
static int
-dr_dump_table_rx_tx(struct seq_file *file, bool is_rx,
+dr_dump_table_rx_tx(struct seq_file *file, char *buff, bool is_rx,
struct mlx5dr_table_rx_tx *table_rx_tx,
const u64 table_id)
{
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
enum dr_dump_rec_type rec_type;
u64 s_icm_addr;
int ret;
@@ -858,7 +856,8 @@ dr_dump_table_rx_tx(struct seq_file *file, bool is_rx,
return 0;
}
-static int dr_dump_table(struct seq_file *file, struct mlx5dr_table *table)
+static noinline_for_stack int
+dr_dump_table(struct seq_file *file, struct mlx5dr_table *table)
{
struct mlx5dr_table_rx_tx *rx = &table->rx;
struct mlx5dr_table_rx_tx *tx = &table->tx;
@@ -878,14 +877,14 @@ static int dr_dump_table(struct seq_file *file, struct mlx5dr_table *table)
return ret;
if (rx->nic_dmn) {
- ret = dr_dump_table_rx_tx(file, true, rx,
+ ret = dr_dump_table_rx_tx(file, buff, true, rx,
DR_DBG_PTR_TO_ID(table));
if (ret < 0)
return ret;
}
if (tx->nic_dmn) {
- ret = dr_dump_table_rx_tx(file, false, tx,
+ ret = dr_dump_table_rx_tx(file, buff, false, tx,
DR_DBG_PTR_TO_ID(table));
if (ret < 0)
return ret;
@@ -911,10 +910,10 @@ static int dr_dump_table_all(struct seq_file *file, struct mlx5dr_table *tbl)
}
static int
-dr_dump_send_ring(struct seq_file *file, struct mlx5dr_send_ring *ring,
+dr_dump_send_ring(struct seq_file *file, char *buff,
+ struct mlx5dr_send_ring *ring,
const u64 domain_id)
{
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
int ret;
memset(buff, 0, sizeof(buff));
@@ -933,13 +932,13 @@ dr_dump_send_ring(struct seq_file *file, struct mlx5dr_send_ring *ring,
return 0;
}
-static noinline_for_stack int
+static int
dr_dump_domain_info_flex_parser(struct seq_file *file,
+ char *buff,
const char *flex_parser_name,
const u8 flex_parser_value,
const u64 domain_id)
{
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
int ret;
memset(buff, 0, sizeof(buff));
@@ -957,11 +956,11 @@ dr_dump_domain_info_flex_parser(struct seq_file *file,
return 0;
}
-static noinline_for_stack int
-dr_dump_domain_info_caps(struct seq_file *file, struct mlx5dr_cmd_caps *caps,
+static int
+dr_dump_domain_info_caps(struct seq_file *file, char *buff,
+ struct mlx5dr_cmd_caps *caps,
const u64 domain_id)
{
- char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
struct mlx5dr_cmd_vport_cap *vport_caps;
unsigned long i, vports_num;
int ret;
@@ -1003,34 +1002,35 @@ dr_dump_domain_info_caps(struct seq_file *file, struct mlx5dr_cmd_caps *caps,
}
static int
-dr_dump_domain_info(struct seq_file *file, struct mlx5dr_domain_info *info,
+dr_dump_domain_info(struct seq_file *file, char *buff,
+ struct mlx5dr_domain_info *info,
const u64 domain_id)
{
int ret;
- ret = dr_dump_domain_info_caps(file, &info->caps, domain_id);
+ ret = dr_dump_domain_info_caps(file, buff, &info->caps, domain_id);
if (ret < 0)
return ret;
- ret = dr_dump_domain_info_flex_parser(file, "icmp_dw0",
+ ret = dr_dump_domain_info_flex_parser(file, buff, "icmp_dw0",
info->caps.flex_parser_id_icmp_dw0,
domain_id);
if (ret < 0)
return ret;
- ret = dr_dump_domain_info_flex_parser(file, "icmp_dw1",
+ ret = dr_dump_domain_info_flex_parser(file, buff, "icmp_dw1",
info->caps.flex_parser_id_icmp_dw1,
domain_id);
if (ret < 0)
return ret;
- ret = dr_dump_domain_info_flex_parser(file, "icmpv6_dw0",
+ ret = dr_dump_domain_info_flex_parser(file, buff, "icmpv6_dw0",
info->caps.flex_parser_id_icmpv6_dw0,
domain_id);
if (ret < 0)
return ret;
- ret = dr_dump_domain_info_flex_parser(file, "icmpv6_dw1",
+ ret = dr_dump_domain_info_flex_parser(file, buff, "icmpv6_dw1",
info->caps.flex_parser_id_icmpv6_dw1,
domain_id);
if (ret < 0)
@@ -1067,12 +1067,12 @@ dr_dump_domain(struct seq_file *file, struct mlx5dr_domain *dmn)
if (ret)
return ret;
- ret = dr_dump_domain_info(file, &dmn->info, domain_id);
+ ret = dr_dump_domain_info(file, buff, &dmn->info, domain_id);
if (ret < 0)
return ret;
if (dmn->info.supp_sw_steering) {
- ret = dr_dump_send_ring(file, dmn->send_ring, domain_id);
+ ret = dr_dump_send_ring(file, buff, dmn->send_ring, domain_id);
if (ret < 0)
return ret;
}
--
2.39.2
在 2024/2/19 18:04, Arnd Bergmann 写道:
> From: Arnd Bergmann <[email protected]>
>
> The debugfs files always in this driver all use an extra round-trip
> through an snprintf() before getting put into a mlx5dr_dbg_dump_buff()
> rather than the normal seq_printf().
>
> Zhu Yanjun noticed that the buffers are not initialized before being
> filled or reused and requested them to always be zeroed as a
> preparation for having more reused between the buffers.
I think that you are the first to find this.
Thanks,
Zhu Yanjun
>
> Requested-by: Zhu Yanjun <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> .../mellanox/mlx5/core/steering/dr_dbg.c | 35 +++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
> index 64f4cc284aea..be7a8481d7d2 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
> @@ -217,6 +217,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
>
> switch (action->action_type) {
> case DR_ACTION_TYP_DROP:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx\n",
> DR_DUMP_REC_TYPE_ACTION_DROP, action_id,
> @@ -229,6 +230,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_FT:
> + memset(buff, 0, sizeof(buff));
> if (action->dest_tbl->is_fw_tbl)
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x,0x%x\n",
> @@ -250,6 +252,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_CTR:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_CTR, action_id, rule_id,
> @@ -262,6 +265,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_TAG:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_TAG, action_id, rule_id,
> @@ -283,6 +287,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
>
> ptrn_arg = !action->rewrite->single_action_opt && ptrn && arg;
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x,%d,0x%x,0x%x,0x%x",
> DR_DUMP_REC_TYPE_ACTION_MODIFY_HDR, action_id,
> @@ -300,6 +305,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
>
> if (ptrn_arg) {
> for (i = 0; i < action->rewrite->num_of_actions; i++) {
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> ",0x%016llx",
> be64_to_cpu(((__be64 *)rewrite_data)[i]));
> @@ -321,6 +327,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> break;
> }
> case DR_ACTION_TYP_VPORT:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_VPORT, action_id, rule_id,
> @@ -333,6 +340,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_TNL_L2_TO_L2:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx\n",
> DR_DUMP_REC_TYPE_ACTION_DECAP_L2, action_id,
> @@ -345,6 +353,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_TNL_L3_TO_L2:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_DECAP_L3, action_id,
> @@ -360,6 +369,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_L2_TO_TNL_L2:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_ENCAP_L2, action_id,
> @@ -372,6 +382,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_L2_TO_TNL_L3:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_ENCAP_L3, action_id,
> @@ -384,6 +395,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_POP_VLAN:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx\n",
> DR_DUMP_REC_TYPE_ACTION_POP_VLAN, action_id,
> @@ -396,6 +408,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_PUSH_VLAN:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_PUSH_VLAN, action_id,
> @@ -408,6 +421,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_INSERT_HDR:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x,0x%x,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_INSERT_HDR, action_id,
> @@ -422,6 +436,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_REMOVE_HDR:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x,0x%x,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_REMOVE_HDR, action_id,
> @@ -436,6 +451,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> return ret;
> break;
> case DR_ACTION_TYP_SAMPLER:
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x,0x%x,0x%x,0x%llx,0x%llx\n",
> DR_DUMP_REC_TYPE_ACTION_SAMPLER, action_id,
> @@ -468,6 +484,7 @@ dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> DR_DBG_PTR_TO_ID(action->range->miss_tbl_action->dest_tbl->tbl);
> }
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x,0x%llx,0x%x,0x%llx,0x%x\n",
> DR_DUMP_REC_TYPE_ACTION_MATCH_RANGE, action_id,
> @@ -507,6 +524,7 @@ dr_dump_rule_mem(struct seq_file *file, struct mlx5dr_ste *ste,
> dr_dump_hex_print(hw_ste_dump, (char *)mlx5dr_ste_get_hw_ste(ste),
> DR_STE_SIZE_REDUCED);
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,%s\n", mem_rec_type,
> dr_dump_icm_to_idx(mlx5dr_ste_get_icm_addr(ste)),
> @@ -554,6 +572,7 @@ static int dr_dump_rule(struct seq_file *file, struct mlx5dr_rule *rule)
>
> format_ver = rule->matcher->tbl->dmn->info.caps.sw_format_ver;
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx\n", DR_DUMP_REC_TYPE_RULE,
> rule_id, DR_DBG_PTR_TO_ID(rule->matcher));
> @@ -593,6 +612,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
> char dump[DR_HEX_SIZE];
> int ret;
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH, "%d,0x%llx,",
> DR_DUMP_REC_TYPE_MATCHER_MASK, matcher_id);
> if (ret < 0)
> @@ -602,6 +622,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
> if (ret)
> return ret;
>
> + memset(buff, 0, sizeof(buff));
> if (criteria & DR_MATCHER_CRITERIA_OUTER) {
> dr_dump_hex_print(dump, (char *)&mask->outer, sizeof(mask->outer));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> @@ -617,6 +638,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
> if (ret)
> return ret;
>
> + memset(buff, 0, sizeof(buff));
> if (criteria & DR_MATCHER_CRITERIA_INNER) {
> dr_dump_hex_print(dump, (char *)&mask->inner, sizeof(mask->inner));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> @@ -632,6 +654,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
> if (ret)
> return ret;
>
> + memset(buff, 0, sizeof(buff));
> if (criteria & DR_MATCHER_CRITERIA_MISC) {
> dr_dump_hex_print(dump, (char *)&mask->misc, sizeof(mask->misc));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> @@ -647,6 +670,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
> if (ret)
> return ret;
>
> + memset(buff, 0, sizeof(buff));
> if (criteria & DR_MATCHER_CRITERIA_MISC2) {
> dr_dump_hex_print(dump, (char *)&mask->misc2, sizeof(mask->misc2));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> @@ -662,6 +686,7 @@ dr_dump_matcher_mask(struct seq_file *file, struct mlx5dr_match_param *mask,
> if (ret)
> return ret;
>
> + memset(buff, 0, sizeof(buff));
> if (criteria & DR_MATCHER_CRITERIA_MISC3) {
> dr_dump_hex_print(dump, (char *)&mask->misc3, sizeof(mask->misc3));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> @@ -687,6 +712,7 @@ dr_dump_matcher_builder(struct seq_file *file, struct mlx5dr_ste_build *builder,
> char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
> int ret;
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,%d,%d,0x%x\n",
> DR_DUMP_REC_TYPE_MATCHER_BUILDER, matcher_id, index,
> @@ -716,6 +742,7 @@ dr_dump_matcher_rx_tx(struct seq_file *file, bool is_rx,
>
> s_icm_addr = mlx5dr_icm_pool_get_chunk_icm_addr(matcher_rx_tx->s_htbl->chunk);
> e_icm_addr = mlx5dr_icm_pool_get_chunk_icm_addr(matcher_rx_tx->e_anchor->chunk);
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,%d,0x%llx,0x%llx\n",
> rec_type, DR_DBG_PTR_TO_ID(matcher_rx_tx),
> @@ -752,6 +779,7 @@ dr_dump_matcher(struct seq_file *file, struct mlx5dr_matcher *matcher)
>
> matcher_id = DR_DBG_PTR_TO_ID(matcher);
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,%d\n", DR_DUMP_REC_TYPE_MATCHER,
> matcher_id, DR_DBG_PTR_TO_ID(matcher->tbl),
> @@ -816,6 +844,7 @@ dr_dump_table_rx_tx(struct seq_file *file, bool is_rx,
> DR_DUMP_REC_TYPE_TABLE_TX;
>
> s_icm_addr = mlx5dr_icm_pool_get_chunk_icm_addr(table_rx_tx->s_anchor->chunk);
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx\n", rec_type, table_id,
> dr_dump_icm_to_idx(s_icm_addr));
> @@ -836,6 +865,7 @@ static int dr_dump_table(struct seq_file *file, struct mlx5dr_table *table)
> char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
> int ret;
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,%d,%d\n", DR_DUMP_REC_TYPE_TABLE,
> DR_DBG_PTR_TO_ID(table), DR_DBG_PTR_TO_ID(table->dmn),
> @@ -887,6 +917,7 @@ dr_dump_send_ring(struct seq_file *file, struct mlx5dr_send_ring *ring,
> char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
> int ret;
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%llx,0x%x,0x%x\n",
> DR_DUMP_REC_TYPE_DOMAIN_SEND_RING,
> @@ -911,6 +942,7 @@ dr_dump_domain_info_flex_parser(struct seq_file *file,
> char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
> int ret;
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,%s,0x%x\n",
> DR_DUMP_REC_TYPE_DOMAIN_INFO_FLEX_PARSER, domain_id,
> @@ -937,6 +969,7 @@ dr_dump_domain_info_caps(struct seq_file *file, struct mlx5dr_cmd_caps *caps,
> xa_for_each(&caps->vports.vports_caps_xa, vports_num, vport_caps)
> ; /* count the number of vports in xarray */
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,0x%x,0x%llx,0x%llx,0x%x,%lu,%d\n",
> DR_DUMP_REC_TYPE_DOMAIN_INFO_CAPS, domain_id, caps->gvmi,
> @@ -952,6 +985,7 @@ dr_dump_domain_info_caps(struct seq_file *file, struct mlx5dr_cmd_caps *caps,
> xa_for_each(&caps->vports.vports_caps_xa, i, vport_caps) {
> vport_caps = xa_load(&caps->vports.vports_caps_xa, i);
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,%lu,0x%x,0x%llx,0x%llx\n",
> DR_DUMP_REC_TYPE_DOMAIN_INFO_VPORT,
> @@ -1012,6 +1046,7 @@ dr_dump_domain(struct seq_file *file, struct mlx5dr_domain *dmn)
> u64 domain_id = DR_DBG_PTR_TO_ID(dmn);
> int ret;
>
> + memset(buff, 0, sizeof(buff));
> ret = snprintf(buff, MLX5DR_DEBUG_DUMP_BUFF_LENGTH,
> "%d,0x%llx,%d,0%x,%d,%u.%u.%u,%s,%d,%u,%u,%u\n",
> DR_DUMP_REC_TYPE_DOMAIN,
On 20-Feb-24 07:50, Zhu Yanjun wrote:
> External email: Use caution opening links or attachments
>
>
> 在 2024/2/19 18:04, Arnd Bergmann 写道:
>> From: Arnd Bergmann <[email protected]>
>>
>> The debugfs files always in this driver all use an extra round-trip
>> through an snprintf() before getting put into a mlx5dr_dbg_dump_buff()
>> rather than the normal seq_printf().
>>
>> Zhu Yanjun noticed that the buffers are not initialized before being
>> filled or reused and requested them to always be zeroed as a
>> preparation for having more reused between the buffers.
>
> I think that you are the first to find this.
The buffers are not initialized intentionally.
The content is overwritten from the buffer's beginning.
Initializing is not needed as it will only cause perf degradation.
-- YK
On Mon, Feb 19, 2024 at 11:04:56AM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> A couple of debug functions use a 512 byte temporary buffer and call another
> function that has another buffer of the same size, which in turn exceeds the
> usual warning limit for excessive stack usage:
>
> drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c:1073:1: error: stack frame size (1448) exceeds limit (1024) in 'dr_dump_start' [-Werror,-Wframe-larger-than]
> dr_dump_start(struct seq_file *file, loff_t *pos)
> drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c:1009:1: error: stack frame size (1120) exceeds limit (1024) in 'dr_dump_domain' [-Werror,-Wframe-larger-than]
> dr_dump_domain(struct seq_file *file, struct mlx5dr_domain *dmn)
> drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c:705:1: error: stack frame size (1104) exceeds limit (1024) in 'dr_dump_matcher_rx_tx' [-Werror,-Wframe-larger-than]
> dr_dump_matcher_rx_tx(struct seq_file *file, bool is_rx,
>
> Rework these so that each of the various code paths only ever has one of
> these buffers in it, and exactly the functions that declare one have
> the 'noinline_for_stack' annotation that prevents them from all being
> inlined into the same caller.
>
> Fixes: 917d1e799ddf ("net/mlx5: DR, Change SWS usage to debug fs seq_file interface")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> [v2] no changes, just based on patch 1/2 but can still be applied independently
> ---
> .../mellanox/mlx5/core/steering/dr_dbg.c | 82 +++++++++----------
> 1 file changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
> index be7a8481d7d2..eae04f66b8f4 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_dbg.c
> @@ -205,12 +205,11 @@ dr_dump_hex_print(char hex[DR_HEX_SIZE], char *src, u32 size)
> }
>
> static int
> -dr_dump_rule_action_mem(struct seq_file *file, const u64 rule_id,
> +dr_dump_rule_action_mem(struct seq_file *file, char *buff, const u64 rule_id,
> struct mlx5dr_rule_action_member *action_mem)
> {
> struct mlx5dr_action *action = action_mem->action;
> const u64 action_id = DR_DBG_PTR_TO_ID(action);
> - char buff[MLX5DR_DEBUG_DUMP_BUFF_LENGTH];
> u64 hit_tbl_ptr, miss_tbl_ptr;
> u32 hit_tbl_id, miss_tbl_id;
> int ret;
Hi Arnd,
With patch 1/2 in place this code goes on as:
switch (action->action_type) {
case DR_ACTION_TYP_DROP:
memset(buff, 0, sizeof(buff));
buff is now a char * rather than an array of char.
siceof(buff) doesn't seem right here anymore.
Flagged by Coccinelle.
On Tue, Feb 20, 2024, at 09:06, Simon Horman wrote:
> On Mon, Feb 19, 2024 at 11:04:56AM +0100, Arnd Bergmann wrote:
> Hi Arnd,
>
> With patch 1/2 in place this code goes on as:
>
> switch (action->action_type) {
> case DR_ACTION_TYP_DROP:
> memset(buff, 0, sizeof(buff));
>
> buff is now a char * rather than an array of char.
> siceof(buff) doesn't seem right here anymore.
>
> Flagged by Coccinelle.
Rihgt, that would be bad. It sounds like we won't use patch 1/2
after all though, so I think it's going to be fine after all.
If the mlx5 maintainers still want both patches, I'll rework
it to use the fixed size.
Arnd
On 20-Feb-24 10:11, Arnd Bergmann wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, Feb 20, 2024, at 09:06, Simon Horman wrote:
>> On Mon, Feb 19, 2024 at 11:04:56AM +0100, Arnd Bergmann wrote:
>
>> Hi Arnd,
>>
>> With patch 1/2 in place this code goes on as:
>>
>> switch (action->action_type) {
>> case DR_ACTION_TYP_DROP:
>> memset(buff, 0, sizeof(buff));
>>
>> buff is now a char * rather than an array of char.
>> siceof(buff) doesn't seem right here anymore.
>>
>> Flagged by Coccinelle.
>
> Rihgt, that would be bad. It sounds like we won't use patch 1/2
> after all though, so I think it's going to be fine after all.
> If the mlx5 maintainers still want both patches, I'll rework
> it to use the fixed size.
No need for the first patch, so only the stack frame limit
fix is needed.
Thanks,
-- YK
> Arnd
在 2024/2/20 14:57, Yevgeny Kliteynik 写道:
> On 20-Feb-24 07:50, Zhu Yanjun wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 在 2024/2/19 18:04, Arnd Bergmann 写道:
>>> From: Arnd Bergmann <[email protected]>
>>>
>>> The debugfs files always in this driver all use an extra round-trip
>>> through an snprintf() before getting put into a mlx5dr_dbg_dump_buff()
>>> rather than the normal seq_printf().
>>>
>>> Zhu Yanjun noticed that the buffers are not initialized before being
>>> filled or reused and requested them to always be zeroed as a
>>> preparation for having more reused between the buffers.
>>
>> I think that you are the first to find this.
>
> The buffers are not initialized intentionally.
> The content is overwritten from the buffer's beginning.
> Initializing is not needed as it will only cause perf degradation.
If the mentioned functions are not in the critical path, this
initialization will not make too much difference on the performance.
But if this function knows how many bytes are written and read, it is
not necessary to initialize the buffer. Or else we should initialize the
buffer before it is used again.
Zhu Yanjun
>
> -- YK
>
>
On Tue, Feb 20, 2024 at 09:11:51AM +0100, Arnd Bergmann wrote:
> On Tue, Feb 20, 2024, at 09:06, Simon Horman wrote:
> > On Mon, Feb 19, 2024 at 11:04:56AM +0100, Arnd Bergmann wrote:
>
> > Hi Arnd,
> >
> > With patch 1/2 in place this code goes on as:
> >
> > switch (action->action_type) {
> > case DR_ACTION_TYP_DROP:
> > memset(buff, 0, sizeof(buff));
> >
> > buff is now a char * rather than an array of char.
> > siceof(buff) doesn't seem right here anymore.
> >
> > Flagged by Coccinelle.
>
> Rihgt, that would be bad. It sounds like we won't use patch 1/2
> after all though, so I think it's going to be fine after all.
> If the mlx5 maintainers still want both patches, I'll rework
> it to use the fixed size.
Ack. I agree that this patch is fine if 1/2 is dropped.
If that is the case feel free to add.
Reviewed-by: Simon Horman <[email protected]>