Re: [v2,net-next,4/5] octeontx2-af: npc: cn20k: dynamically allocate and free default MCAM entries

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
octeontx2-af: npc: cn20k: dynamically allocate and free default MCAM entries

This commit improves MCAM utilization by tying default entry lifetime to
NIX LF usage. Default entries are allocated when NIX LF is allocated and
freed when NIX LF is freed, with a new flag to allow the kernel PF driver
to retain entries across suspend/resume.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> index badfa1d64252..0fb2d71116f4 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c

[ ... ]

> @@ -1684,6 +1685,12 @@ int rvu_mbox_handler_nix_lf_alloc(struct rvu *rvu,
>  	if (is_sdp_pfvf(rvu, pcifunc))
>  		intf = NIX_INTF_TYPE_SDP;
>
> +	if (is_cn20k(rvu->pdev)) {
> +		rc = npc_cn20k_dft_rules_alloc(rvu, pcifunc);
> +		if (rc)
> +			goto free_dft;
> +	}
> +
>  	err = nix_interface_init(rvu, pcifunc, intf, nixlf, rsp,
>  				 !!(req->flags & NIX_LF_LBK_BLK_SEL));
>  	if (err)

Does this leak the cn20k default MCAM entries if nix_interface_init()
fails? The npc_cn20k_dft_rules_alloc() call succeeds and allocates entries
into the xa_pf2dfl_rmap xarray and hardware MCAM slots. When
nix_interface_init() subsequently fails, the error path jumps to 'free_mem'
rather than 'free_dft', skipping npc_cn20k_dft_rules_free().

> @@ -1699,6 +1706,10 @@ int rvu_mbox_handler_nix_lf_alloc(struct rvu *rvu,
>
>  	goto exit;
>
> +free_dft:
> +	if (is_cn20k(rvu->pdev))
> +		npc_cn20k_dft_rules_free(rvu, pcifunc);
> +
>  free_mem:
>  	nix_ctx_free(rvu, pfvf);
>  	rc = -ENOMEM;

The free_dft cleanup path exists and would handle the release, so should
the nix_interface_init() failure case use 'goto free_dft' instead?