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

[Michal Kubiak]

On Tue, Apr 08, 2025 at 05:00:54PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@xxxxxxxxxx>
> Reviewed-by: Mark Bloch <mbloch@xxxxxxxxxx>
> Signed-off-by: Tariq Toukan <tariqt@xxxxxxxxxx>
> ---
>  .../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