netdev - Re: [PATCH net-next 08/12] net/mlx5: HWS, Implement action STE pool

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/ffJUkWbS15sPAs@localhost.localdomain>
Date: Thu, 10 Apr 2025 17:09:25 +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 08/12] net/mlx5: HWS, Implement action STE pool

On Tue, Apr 08, 2025 at 05:00:52PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@...dia.com>
> 
> Implement a per-queue pool of action STEs that match STEs can link to,
> regardless of matcher.
> 
> The code relies on hints to optimize whether a given rule is added to
> rx-only, tx-only or both. Correspondingly, action STEs need to be added
> to different RTC for ingress or egress paths. For rx-and-tx rules, the
> current rule implementation dictates that the offsets for a given rule
> must be the same in both RTCs.
> 
> To avoid wasting STEs, each action STE pool element holds 3 pools:
> rx-only, tx-only, and rx-and-tx, corresponding to the possible values of
> the pool optimization enum. The implementation then chooses at rule
> creation / update which of these elements to allocate from.
> 
> Each element holds multiple action STE tables, which wrap an RTC, an STE
> range, the logic to buddy-allocate offsets from the range, and an STC
> that allows match STEs to point to this table. When allocating offsets
> from an element, we iterate through available action STE tables and, if
> needed, create a new table.
> 
> Similar to the previous implementation, this iteration does not free any
> resources. This is implemented in a subsequent patch.
> 
> 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>
> ---

The patch looks OK to me. It corresponds with the idea described in the
cover letter and commit message.
No new issues found.

> +static int hws_action_ste_table_create_single_rtc(
> +	struct mlx5hws_context *ctx,
> +	struct mlx5hws_action_ste_table *action_tbl,
> +	enum mlx5hws_pool_optimize opt, size_t log_sz, bool tx)
> +{
> +	struct mlx5hws_cmd_rtc_create_attr rtc_attr = { 0 };
> +	u32 *rtc_id;
> +
> +	rtc_attr.log_depth = 0;
> +	rtc_attr.update_index_mode = MLX5_IFC_RTC_STE_UPDATE_MODE_BY_OFFSET;
> +	/* Action STEs use the default always hit definer. */
> +	rtc_attr.match_definer_0 = ctx->caps->trivial_match_definer;
> +	rtc_attr.is_frst_jumbo = false;
> +	rtc_attr.miss_ft_id = 0;
> +	rtc_attr.pd = ctx->pd_num;
> +	rtc_attr.ste_offset = 0;

As I mentioned in my review for patch #5, it's always zero.
Anyway, you've already said you're going to remove it in v2.

Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@...el.com>