Hello Yevgeny Kliteynik,
Commit 472dd792348f ("net/mlx5: HWS, added matchers functionality")
from Jun 20, 2024 (linux-next), leads to the following Smatch static
checker warnings:
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_matcher.c:970 mlx5hws_matcher_attach_at() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_table.c:492 mlx5hws_table_set_default_miss() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_rule.c:754 mlx5hws_rule_destroy() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_rule.c:758 mlx5hws_rule_destroy() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_rule.c:770 mlx5hws_rule_action_update() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_rule.c:779 mlx5hws_rule_action_update() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_bwc_complex.c:36 mlx5hws_bwc_match_params_is_complex() warn: was negative '-E2BIG' intended?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_definer.c:1848 hws_definer_find_best_match_fit() warn: was negative '-E2BIG' intended?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_definer.c:1934 mlx5hws_definer_calc_layout() warn: was negative '-E2BIG' intended?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_matcher.c:678 hws_matcher_bind_mt() warn: was negative '-E2BIG' intended?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_matcher.c
954 int mlx5hws_matcher_attach_at(struct mlx5hws_matcher *matcher,
955 struct mlx5hws_action_template *at)
956 {
957 bool is_jumbo = mlx5hws_matcher_mt_is_jumbo(matcher->mt);
958 struct mlx5hws_context *ctx = matcher->tbl->ctx;
959 u32 required_stes;
960 int ret;
961
962 if (!matcher->attr.max_num_of_at_attach) {
963 mlx5hws_dbg(ctx, "Num of current at (%d) exceed allowed value\n",
964 matcher->num_of_at);
965 return -EOPNOTSUPP;
966 }
967
968 ret = hws_matcher_check_and_process_at(matcher, at);
969 if (ret)
--> 970 return -ret;
The hws_matcher_check_and_process_at() function returns negative error codes.
The caller doesn't care if we return negative or positive.
The function doesn't return E2BIG but it feels like these warnings might be
related. The hws_definer_find_best_match_fit() function returns positive E2BIG
and it's used in mlx5hws_bwc_match_params_is_complex(). I guess the worry here
was that something else would return negative -E2BIG and the error codes would
get mixed up. The E2BIG error code is not super common and I would not worry
about this personally.
None of the other callers use the E2BIG error code except for debugging. The
possitive error code gets mixed with negative error codes and all non-zero
values are treated as errors and eventually it results in a NULL return.
In terms of future proofing the code, I think mixing positive and negative error
codes is risky. Eventually someone will write an error check like if (ret < 0)
and it will be treated as success or someone will pass the positive value to
ERR_PTR() and it will cause an error pointer dereference.
971
972 required_stes = at->num_of_action_stes - (!is_jumbo || at->only_term);
973 if (matcher->action_ste[MLX5HWS_ACTION_STE_IDX_ANY].max_stes < required_stes) {
974 mlx5hws_dbg(ctx, "Required STEs [%d] exceeds initial action template STE [%d]\n",
975 required_stes,
976 matcher->action_ste[MLX5HWS_ACTION_STE_IDX_ANY].max_stes);
977 return -ENOMEM;
978 }
979
980 matcher->at[matcher->num_of_at] = *at;
981 matcher->num_of_at += 1;
982 matcher->attr.max_num_of_at_attach -= 1;
983
984 if (matcher->col_matcher)
985 matcher->col_matcher->num_of_at = matcher->num_of_at;
986
987 return 0;
988 }
regards,
dan carpenter
