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

[Vlad Dogaru]

On Thu, Apr 10, 2025 at 07:01:58PM +0200, Michal Kubiak wrote:
> 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?

Although patch 1 is indeed a bugfix, we've never seen it triggered in
our regressions. Since all of the code is relatively new and it's
unlikely we will see this in the wild, let's keep the series as is, I
hope that's ok.

Cheers,
Vlad