[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260129051309.376665-1-kuba@kernel.org>
Date: Wed, 28 Jan 2026 21:13:09 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: rkannoth@...vell.com
Cc: Jakub Kicinski <kuba@...nel.org>,
edumazet@...gle.com,
sumang@...vell.com,
linux-kernel@...r.kernel.org,
sbhatta@...vell.com,
davem@...emloft.net,
netdev@...r.kernel.org,
pabeni@...hat.com,
andrew+netdev@...n.ch,
sgoutham@...vell.com
Subject: Re: [net-next,v6,10/13] octeontx2-af: npc: cn20k: Allocate MCAM entry for flow installation
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: Allocate MCAM entry for flow installation
This patch extends the install_flow mailbox request message with
additional fields that allow the AF driver to determine the required
NPC MCAM entry type, allocate the entry, and install the flow in a
single step for CN20K.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> index 82afb4993346..7a48ba1db5e9 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
[ ... ]
> @@ -1642,6 +1643,75 @@ static int npc_install_flow(struct rvu *rvu, int blkaddr, u16 target,
> return 0;
> }
>
> +static int
> +rvu_npc_free_entry_for_flow_install(struct rvu *rvu, u16 pcifunc,
> + bool free_entry, int mcam_idx)
> +{
[ ... ]
> +static int
> +rvu_npc_alloc_entry_for_flow_install(struct rvu *rvu,
> + struct npc_install_flow_req *fl_req,
> + u16 *mcam_idx, u8 *kw_type,
> + bool *allocated)
> +{
[ ... ]
> + if (!is_cn20k(rvu->pdev))
> + return 0;
> +
> + if (!fl_req->alloc_entry)
> + return 0;
[ ... ]
> int rvu_mbox_handler_npc_install_flow(struct rvu *rvu,
> struct npc_install_flow_req *req,
> struct npc_install_flow_rsp *rsp)
> {
[ ... ]
> + bool allocated = false;
> bool enable = true;
> + u8 kw_type;
> u16 target;
[ ... ]
> + err = rvu_npc_alloc_entry_for_flow_install(rvu, req, &req->entry,
> + &kw_type, &allocated);
> + if (err) {
> + dev_err(rvu->dev,
> + "%s: Error to alloc mcam entry for pcifunc=%#x\n",
> + __func__, req->hdr.pcifunc);
> + return err;
> + }
[ ... ]
> mutex_lock(&rswitch->switch_lock);
> err = npc_install_flow(rvu, blkaddr, target, nixlf, pfvf,
> req, rsp, enable, pf_set_vfs_mac);
> + if (err)
> + rvu_npc_free_entry_for_flow_install(rvu, req->hdr.pcifunc,
> + allocated, req->entry);
> +
> + rsp->kw_type = kw_type;
> + rsp->entry = req->entry;
^^^^^^^^
Is kw_type initialized in all paths here?
When rvu_npc_alloc_entry_for_flow_install() returns early because either
is_cn20k() is false or alloc_entry is not set, kw_type is never written.
The function returns 0 without setting *kw_type, leaving the local variable
uninitialized.
The uninitialized stack value then gets copied to rsp->kw_type.
Should this be initialized to 0 at declaration?
> mutex_unlock(&rswitch->switch_lock);
>
> return err;
> }
Powered by blists - more mailing lists