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

[Ratheesh Kannoth <rkannoth@marvell.com>]

On 2026-01-11 at 04:29:35, Jakub Kicinski (kuba@kernel.org) wrote:
> 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.
ACK