[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aW2ngo2FwC5sZmWn@rkannoth-OptiPlex-7090>
Date: Mon, 19 Jan 2026 09:09:46 +0530
From: Ratheesh Kannoth <rkannoth@...vell.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [net-next,v4,09/13] octeontx2-af: npc: cn20k: virtual index
support
On 2026-01-18 at 06:10:27, Jakub Kicinski (kuba@...nel.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.
Powered by blists - more mailing lists