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

[Michal Kubiak]

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

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@xxxxxxxxx>