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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_gR5YbASnM1JPGm@nvidia.com>
Date: Thu, 10 Apr 2025 20:45:57 +0200
From: Vlad Dogaru <vdogaru@...dia.com>
To: Michal Kubiak <michal.kubiak@...el.com>
Cc: Tariq Toukan <tariqt@...dia.com>,
	"David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Andrew Lunn <andrew+netdev@...n.ch>, Gal Pressman <gal@...dia.com>,
	Leon Romanovsky <leonro@...dia.com>,
	Saeed Mahameed <saeedm@...dia.com>,
	Leon Romanovsky <leon@...nel.org>, netdev@...r.kernel.org,
	linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
	Moshe Shemesh <moshe@...dia.com>, Mark Bloch <mbloch@...dia.com>,
	Yevgeny Kliteynik <kliteyn@...dia.com>
Subject: Re: [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE
 table

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@...dia.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@...dia.com>
> > Reviewed-by: Yevgeny Kliteynik <kliteyn@...dia.com>
> > Reviewed-by: Mark Bloch <mbloch@...dia.com>
> > Signed-off-by: Tariq Toukan <tariqt@...dia.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?

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