Re: [net-next,v3,05/13] octeontx2-af: npc: cn20k: Allocate default MCAM indexes

[Jakub Kicinski <kuba@kernel.org>]

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

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
octeontx2-af: npc: cn20k: Allocate default MCAM indexes

This patch allocates MCAM indexes for promiscuous, multicast, broadcast,
and unicast traffic when the NIX LF is attached to the PF/VF, rather than
reserving entries upfront.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> index 88d7f65d246c..cc5a2fde02b9 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c

[ ... ]

> +int npc_cn20k_dft_rules_alloc(struct rvu *rvu, u16 pcifunc)
> +{
> +	u16 mcam_idx[4] = { 0 }, pf_ucast, pf_pcifunc;
> +	struct npc_mcam_alloc_entry_req req = { 0 };
> +	struct npc_mcam_alloc_entry_rsp rsp = { 0 };

[ ... ]

> +	ret = rvu_mbox_handler_npc_mcam_alloc_entry(rvu, &req, &rsp);
> +
> +	/* successfully allocated index */
> +	if (!ret) {
> +		/* Copy indexes to local array */
> +		for (i = 0; i < cnt; i++)
> +			mcam_idx[i] = rsp.entry_list[i];
> +
> +		goto chk_sanity;
> +	}

[ ... ]

> +chk_sanity:

[ ... ]

> +	/* LBK */
> +	if (is_lbk_vf(rvu, pcifunc)) {
> +		index = NPC_DFT_RULE_ID_MK(pcifunc, NPC_DFT_RULE_PROMISC_ID);
> +		ret = xa_insert(&npc_priv.xa_pf2dfl_rmap, index,
> +				xa_mk_value(mcam_idx[0]), GFP_KERNEL);
> +		if (ret) {

[ ... ]

> +			ret = -EFAULT;
> +			goto err;
> +		}
> +
> +		goto done;
> +	}
> +
> +	/* VF */
> +	if (is_vf(pcifunc)) {
> +		index = NPC_DFT_RULE_ID_MK(pcifunc, NPC_DFT_RULE_UCAST_ID);
> +		ret = xa_insert(&npc_priv.xa_pf2dfl_rmap, index,
> +				xa_mk_value(mcam_idx[0]), GFP_KERNEL);
> +		if (ret) {

[ ... ]

> +			ret = -EFAULT;
> +			goto err;
> +		}
> +
> +		goto done;
> +	}
> +
> +	/* PF */
> +	for (i = NPC_DFT_RULE_START_ID, k = 0; i < NPC_DFT_RULE_MAX_ID &&
> +	     k < cnt; i++, k++) {
> +		index = NPC_DFT_RULE_ID_MK(pcifunc, i);
> +		ret = xa_insert(&npc_priv.xa_pf2dfl_rmap, index,
> +				xa_mk_value(mcam_idx[k]), GFP_KERNEL);
> +		if (ret) {

[ ... ]

> +			ret = -EFAULT;
> +			goto err;
> +		}
> +	}
> +
> +done:
> +	return 0;
> +err:
> +	/* TODO: handle errors */
> +	return ret;
> +}

If xa_insert() fails here, the MCAM entries that were allocated earlier
via rvu_mbox_handler_npc_mcam_alloc_entry() are not freed before
returning the error. Could this leak the MCAM entries?

The comment indicates this is known incomplete error handling. For the
PF case, partial xa_insert failures in the loop could also leave some
entries inserted in the xarray while others are not, leading to an
inconsistent state.