Re: [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE table

Michal Kubiak
2025-04-10T17:02:25Z

On Tue, Apr 08, 2025 at 05:00:54PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Remove the matcher action STE implementation now that the code uses
> per-queue action STE pools. This also allows simplifying matcher code
> because it is now only handling a single type of RTC/STE.
>
> The matcher resize data is also going away. Matchers were saving old
> action STE data because the rules still used it, but now that data lives
> in the action STE pool and is no longer coupled to a matcher.
>
> Furthermore, matchers no longer need to rehash a due to action template
> addition. If a new action template needs more action STEs, we simply
> update the matcher's num_of_action_stes and future rules will allocate
> the correct number. Existing rules are unaffected by such an operation
> and can continue to use their existing action STEs.
>
> The range action was using the matcher action STE implementation, but
> there was no reason to do this other than the container fitting the
> purpose. Extract that information to a separate structure.
>
> Finally, stop dumping per-matcher information about action RTCs,
> because they no longer exist. A later patch in this series will add
> support for dumping action STE pools.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> .../mellanox/mlx5/core/steering/hws/action.c | 23 +-
> .../mellanox/mlx5/core/steering/hws/action.h | 8 +-
> .../mellanox/mlx5/core/steering/hws/bwc.c | 77 +---
> .../mellanox/mlx5/core/steering/hws/debug.c | 17 +-
> .../mellanox/mlx5/core/steering/hws/matcher.c | 336 ++++--------------
> .../mellanox/mlx5/core/steering/hws/matcher.h | 20 +-
> .../mellanox/mlx5/core/steering/hws/mlx5hws.h | 5 +-
> .../mellanox/mlx5/core/steering/hws/rule.c | 2 +-
> 8 files changed, 87 insertions(+), 401 deletions(-)
>

[...]

> @@ -803,7 +778,6 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
> struct mlx5hws_rule_attr rule_attr;
> struct mutex *queue_lock; /* Protect the queue */
> u32 num_of_rules;
> - bool need_rehash;

This flag (and the function parameter below) were added in the Patch #1 as part
of the fix for unnecessary rehashing. Now it is removed again.
Is this fix really necessary for this series to somehow make it consistent?
Maybe Patch #1 should go separately as an independent fix in the "net"
tree? What do you think?

> int ret = 0;
> int at_idx;
>
> @@ -830,30 +804,11 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
> at_idx = bwc_matcher->num_of_at - 1;
>
> ret = mlx5hws_matcher_attach_at(bwc_matcher->matcher,
> - bwc_matcher->at[at_idx],
> - &need_rehash);
> + bwc_matcher->at[at_idx]);
> if (unlikely(ret)) {
> hws_bwc_unlock_all_queues(ctx);
> return ret;
> }
> - if (unlikely(need_rehash)) {
> - /* The new action template requires more action STEs.
> - * Need to attempt creating new matcher with all
> - * the action templates, including the new one.
> - */

[...]

Thanks,
Michal