Re: [net-next,v4,09/13] octeontx2-af: npc: cn20k: virtual index support

[Ratheesh Kannoth <rkannoth@marvell.com>]

On 2026-01-18 at 06:10:27, 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 plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> octeontx2-af: npc: cn20k: virtual index support
>
> This patch adds virtual MCAM index allocation and improves CN20K MCAM
> defragmentation. Virtual indexes allow the hardware to move mapped
> MCAM entries internally while presenting stable indexes to callers.
>
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> > index d549e75b2cb0..334b80ebec69 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
>
> [ ... ]
>
> > +static int npc_vidx_maps_add_entry(struct rvu *rvu, u16 mcam_idx, int pcifunc,
> > +				   u16 *vidx)
> > +{
> > +	int rc, max, min;
> > +	u32 id;
> > +
> > +	/* Virtual index start from maximum mcam index + 1 */
> > +	max = npc_priv.bank_depth * 2 * 2 - 1;
> > +	min = npc_priv.bank_depth * 2;
> > +
> > +	rc = xa_alloc(&npc_priv.xa_vidx2idx_map, &id,
> > +		      xa_mk_value(mcam_idx),
> > +		      XA_LIMIT(min, max), GFP_KERNEL);
> > +	if (rc) {
> > +		dev_err(rvu->dev,
> > +			"%s: Failed to add to vidx2idx map (%u)\n",
> > +			__func__, mcam_idx);
> > +		return rc;
> > +	}
> > +
> > +	rc = xa_insert(&npc_priv.xa_idx2vidx_map, mcam_idx,
> > +		       xa_mk_value(id), GFP_KERNEL);
> > +	if (rc) {
> > +		dev_err(rvu->dev,
> > +			"%s: Failed to add to idx2vidx map (%u)\n",
> > +			__func__, mcam_idx);
> > +		return rc;
>                 ^^^^^^^^^^
>
> If xa_insert() fails here, the entry allocated by xa_alloc() in
> xa_vidx2idx_map is not cleaned up. Should there be an xa_erase() call
> to remove the allocated entry before returning the error?
ACK.

>
> > +	}
> > +
> > +	if (vidx)
> > +		*vidx = id;
> > +
> > +	return 0;
> > +}
>
> [ ... ]
>
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
> > index 0f9953eaf1b0..cc83d4fc5724 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
>
> [ ... ]
>
> > +static const struct devlink_param rvu_af_dl_param_defrag[] = {
> > +	DEVLINK_PARAM_DRIVER(RVU_AF_DEVLINK_PARAM_ID_NPC_DEFRAG,
> > +			     "npc_defrag", DEVLINK_PARAM_TYPE_STRING,
> > +			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
> > +			     rvu_af_npc_defrag_feature_get,
> > +			     rvu_af_npc_defrag,
> > +			     rvu_af_npc_defrag_feature_validate),
> > +};
> > +
> >  int rvu_register_dl(struct rvu *rvu)
> >  {
>
> [ ... ]
>
> > +	if (is_cn20k(rvu->pdev)) {
> > +		err = devlink_params_register(dl, rvu_af_dl_param_defrag,
> > +					      ARRAY_SIZE(rvu_af_dl_param_defrag));
> > +		if (err) {
> > +			dev_err(rvu->dev,
> > +				"devlink defrag params register failed with error %d",
> > +				err);
> > +			goto err_dl_exact_match;
> > +		}
> > +	}
>
> The rvu_af_dl_param_defrag parameters are registered here for CN20K
> devices, but rvu_unregister_dl() does not have a corresponding
> devlink_params_unregister() call for these parameters. Should there be
> a matching unregister call similar to the exact_match handling?
ACK.