netdev - 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/f5hqMDh9eGd0Xc@localhost.localdomain>
Date: Thu, 10 Apr 2025 19:01:58 +0200
From: Michal Kubiak <michal.kubiak@...el.com>
To: Tariq Toukan <tariqt@...dia.com>
CC: "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>, Vlad Dogaru <vdogaru@...dia.com>, Yevgeny Kliteynik
	<kliteyn@...dia.com>
Subject: Re: [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE
 table

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?

>  	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