[PATCH net 0/3] octeontx2-af: Fix smatch reported errors

[PATCH net 0/3] octeontx2-af: Fix smatch reported errors

[Ratheesh Kannoth]

This series tightens error handling in the Marvell OcteonTX2 AF CN20K NPC
layer: MCAM paths now respect npc_mcam_idx_2_key_type() failures, debugfs
setup follows the usual "optional, do not fail init" convention, and
defrag rollback reports failure when freeing slots after a partial
allocation does not complete cleanly.

Patch 1 returns early when resolving an MCAM index to a key type fails,
so enable, configure, copy, and read paths do not program or interpret
hardware with a stale or undefined key type.

Patch 2 removes dentry and allocation-failure checks around
debugfs_create_file() in npc_cn20k_debugfs_init().  Debugfs entries are
diagnostic; callers should not abort probe or subsystem init when they
cannot be created (including when debugfs is disabled).

Patch 3 sets the error code when __npc_subbank_free() fails inside the
rollback loop in npc_defrag_alloc_free_slots(), so the function does not
return success after a failed cleanup.

Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

Ratheesh Kannoth (3):
  octeontx2-af: npc: cn20k: Handle npc_mcam_idx_2_key_type() failures
  octeontx2-af: npc: cn20k: Drop debugfs_create_file() error checks in
    init
  octeontx2-af: npc: cn20k: Return error when defrag rollback free fails

---
 .../marvell/octeontx2/af/cn20k/debugfs.c      | 33 ++++++-------------
 .../ethernet/marvell/octeontx2/af/cn20k/npc.c | 21 +++++++++---
 2 files changed, 26 insertions(+), 28 deletions(-)

--
2.43.0

[PATCH net 1/3] octeontx2-af: npc: cn20k: Handle npc_mcam_idx_2_key_type() failures

[Ratheesh Kannoth]

npc_mcam_idx_2_key_type() can fail; ignoring its return value left
kw_type unchecked in MCAM enable, configure, copy, and read paths.
Return early on error so we do not program or interpret MCAM state
with an invalid key type.

CC: Dan Carpenter <error27@gmail.com>
Fixes: 6d1e70282f76 ("octeontx2-af: npc: cn20k: Use common APIs")
Link: https://lore.kernel.org/netdev/adiQJvuKlEhq2ILx@stanley.mountain/
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 .../ethernet/marvell/octeontx2/af/cn20k/npc.c | 20 ++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
index 7291fdb89b03..2fcd0ee2b1e1 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
@@ -808,7 +808,9 @@ npc_cn20k_enable_mcam_entry(struct rvu *rvu, int blkaddr,
 	u64 cfg, hw_prio;
 	u8 kw_type;
 
-	npc_mcam_idx_2_key_type(rvu, index, &kw_type);
+	if (npc_mcam_idx_2_key_type(rvu, index, &kw_type))
+		return;
+
 	if (kw_type == NPC_MCAM_KEY_X2) {
 		cfg = rvu_read64(rvu, blkaddr,
 				 NPC_AF_CN20K_MCAMEX_BANKX_CFG_EXT(mcam_idx,
@@ -1052,10 +1054,12 @@ void npc_cn20k_config_mcam_entry(struct rvu *rvu, int blkaddr, int index,
 	int kw = 0;
 	u8 kw_type;
 
+	if (npc_mcam_idx_2_key_type(rvu, index, &kw_type))
+		return;
+
 	/* Disable before mcam entry update */
 	npc_cn20k_enable_mcam_entry(rvu, blkaddr, index, false);
 
-	npc_mcam_idx_2_key_type(rvu, index, &kw_type);
 	/* CAM1 takes the comparison value and
 	 * CAM0 specifies match for a bit in key being '0' or '1' or 'dontcare'.
 	 * CAM1<n> = 0 & CAM0<n> = 1 => match if key<n> = 0
@@ -1132,8 +1136,13 @@ void npc_cn20k_copy_mcam_entry(struct rvu *rvu, int blkaddr, u16 src, u16 dest)
 
 	dbank = npc_get_bank(mcam, dest);
 	sbank = npc_get_bank(mcam, src);
-	npc_mcam_idx_2_key_type(rvu, src, &src_kwtype);
-	npc_mcam_idx_2_key_type(rvu, dest, &dest_kwtype);
+
+	if (npc_mcam_idx_2_key_type(rvu, src, &src_kwtype))
+		return;
+
+	if (npc_mcam_idx_2_key_type(rvu, dest, &dest_kwtype))
+		return;
+
 	if (src_kwtype != dest_kwtype)
 		return;
 
@@ -1188,7 +1197,8 @@ void npc_cn20k_read_mcam_entry(struct rvu *rvu, int blkaddr, u16 index,
 	int kw = 0, bank;
 	u8 kw_type;
 
-	npc_mcam_idx_2_key_type(rvu, index, &kw_type);
+	if (npc_mcam_idx_2_key_type(rvu, index, &kw_type))
+		return;
 
 	bank = npc_get_bank(mcam, index);
 	index &= (mcam->banksize - 1);
-- 
2.43.0

[PATCH net 2/3] octeontx2-af: npc: cn20k: Drop debugfs_create_file() error checks in init

[Ratheesh Kannoth]

debugfs is not intended to be checked for allocation failures the way
other kernel APIs are: callers should not fail probe or subsystem init
because a debugfs node could not be created, including when debugfs is
disabled in Kconfig.  Replacing NULL checks with IS_ERR() checks is
similarly wrong for optional debugfs.

Remove dentry checks and -EFAULT returns from npc_cn20k_debugfs_init().
https://staticthinking.wordpress.com/2023/07/24/debugfs-functions-are-not-supposed-to-be-checked/

CC: Dan Carpenter <error27@gmail.com>
Link: https://lore.kernel.org/netdev/adjNGPWKMOk3KgWL@stanley.mountain/
Fixes: 528530dff56b ("octeontx2-af: npc: cn20k: add debugfs support")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 .../marvell/octeontx2/af/cn20k/debugfs.c      | 33 ++++++-------------
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c
index 3debf2fae1a4..6f13296303cb 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c
@@ -249,34 +249,21 @@ DEFINE_SHOW_ATTRIBUTE(npc_defrag);
 int npc_cn20k_debugfs_init(struct rvu *rvu)
 {
 	struct npc_priv_t *npc_priv = npc_priv_get();
-	struct dentry *npc_dentry;
 
-	npc_dentry = debugfs_create_file("mcam_layout", 0444, rvu->rvu_dbg.npc,
-					 npc_priv, &npc_mcam_layout_fops);
+	debugfs_create_file("mcam_layout", 0444, rvu->rvu_dbg.npc,
+			    npc_priv, &npc_mcam_layout_fops);
 
-	if (!npc_dentry)
-		return -EFAULT;
+	debugfs_create_file("mcam_default", 0444, rvu->rvu_dbg.npc,
+			    rvu, &npc_mcam_default_fops);
 
-	npc_dentry = debugfs_create_file("mcam_default", 0444, rvu->rvu_dbg.npc,
-					 rvu, &npc_mcam_default_fops);
+	debugfs_create_file("vidx2idx", 0444, rvu->rvu_dbg.npc,
+			    npc_priv, &npc_vidx2idx_map_fops);
 
-	if (!npc_dentry)
-		return -EFAULT;
+	debugfs_create_file("idx2vidx", 0444, rvu->rvu_dbg.npc,
+			    npc_priv, &npc_idx2vidx_map_fops);
 
-	npc_dentry = debugfs_create_file("vidx2idx", 0444, rvu->rvu_dbg.npc,
-					 npc_priv, &npc_vidx2idx_map_fops);
-	if (!npc_dentry)
-		return -EFAULT;
-
-	npc_dentry = debugfs_create_file("idx2vidx", 0444, rvu->rvu_dbg.npc,
-					 npc_priv, &npc_idx2vidx_map_fops);
-	if (!npc_dentry)
-		return -EFAULT;
-
-	npc_dentry = debugfs_create_file("defrag", 0444, rvu->rvu_dbg.npc,
-					 npc_priv, &npc_defrag_fops);
-	if (!npc_dentry)
-		return -EFAULT;
+	debugfs_create_file("defrag", 0444, rvu->rvu_dbg.npc,
+			    npc_priv, &npc_defrag_fops);
 
 	return 0;
 }
-- 
2.43.0

[PATCH net 3/3] octeontx2-af: npc: cn20k: Return error when defrag rollback free fails

[Ratheesh Kannoth]

In npc_defrag_alloc_free_slots(), the fail_free_alloc rollback loop frees
previously allocated MCAM entries after a partial allocation failure.  If
__npc_subbank_free() fails, we break out of the loop but rc was still zero
from the successful npc_mcam_idx_2_subbank_idx() lookup, so the function
incorrectly returned success.  Set rc to -EFAULT so the failure is visible
to callers.

CC: Dan Carpenter <error27@gmail.com>
Link: https://lore.kernel.org/netdev/adjNJEpILRZATB2N@stanley.mountain/
Fixes: 645c6e3c1999 ("octeontx2-af: npc: cn20k: virtual index support")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
index 2fcd0ee2b1e1..df192729ac1d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
@@ -3541,6 +3541,7 @@ static int npc_defrag_alloc_free_slots(struct rvu *rvu,
 			dev_err(rvu->dev,
 				"%s: Error to free mcam idx=%u\n",
 				__func__, save[i]);
+			rc = -EFAULT;
 			break;
 		}
 	}
-- 
2.43.0

Re: [PATCH net 1/3] octeontx2-af: npc: cn20k: Handle npc_mcam_idx_2_key_type() failures

[Dan Carpenter]

On Thu, Apr 16, 2026 at 09:23:50AM +0530, Ratheesh Kannoth wrote:
> npc_mcam_idx_2_key_type() can fail; ignoring its return value left
> kw_type unchecked in MCAM enable, configure, copy, and read paths.
> Return early on error so we do not program or interpret MCAM state
> with an invalid key type.
> 
> CC: Dan Carpenter <error27@gmail.com>
> Fixes: 6d1e70282f76 ("octeontx2-af: npc: cn20k: Use common APIs")
> Link: https://lore.kernel.org/netdev/adiQJvuKlEhq2ILx@stanley.mountain/
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

Thanks.  That silences the uninitialized variable warning.

regards,
dan carpenter

Re: [PATCH net 1/3] octeontx2-af: npc: cn20k: Handle npc_mcam_idx_2_key_type() failures

[Simon Horman]

From: 'Simon Horman' <horms@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 plausible.
Full review at: https://sashiko.dev
---
octeontx2-af: npc: cn20k: Handle npc_mcam_idx_2_key_type() failures

This commit adds error checking to npc_mcam_idx_2_key_type() calls.
It returns early from several void functions to prevent programming
or interpreting MCAM state with an invalid key type.

Link: https://lore.kernel.org/netdev/adiQJvuKlEhq2ILx@stanley.mountain/

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> index 7291fdb89b03f..2fcd0ee2b1e10 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c

[ ... ]

> @@ -1052,10 +1054,12 @@ void npc_cn20k_config_mcam_entry(struct rvu *rvu, int blkaddr, int index,
>  	int kw = 0;
>  	u8 kw_type;
>  
> +	if (npc_mcam_idx_2_key_type(rvu, index, &kw_type))
> +		return;
> +
>  	/* Disable before mcam entry update */
>  	npc_cn20k_enable_mcam_entry(rvu, blkaddr, index, false);
>  

Does returning early from a void function here cause state desynchronization?

For example, in mailbox handlers like
rvu_mbox_handler_npc_cn20k_mcam_write_entry(), the caller assumes success
and returns 0, but the hardware programming would be silently aborted.

[ ... ]

> @@ -1132,8 +1136,13 @@ void npc_cn20k_copy_mcam_entry(struct rvu *rvu, int blkaddr, u16 src, u16 dest)
>  
>  	dbank = npc_get_bank(mcam, dest);
>  	sbank = npc_get_bank(mcam, src);
> -	npc_mcam_idx_2_key_type(rvu, src, &src_kwtype);
> -	npc_mcam_idx_2_key_type(rvu, dest, &dest_kwtype);
> +
> +	if (npc_mcam_idx_2_key_type(rvu, src, &src_kwtype))
> +		return;
> +

If this copy is skipped, could callers like npc_cn20k_defrag() unconditionally
enable the target entry with stale hardware state?

[ ... ]

> @@ -1188,7 +1197,8 @@ void npc_cn20k_read_mcam_entry(struct rvu *rvu, int blkaddr, u16 index,
>  	int kw = 0, bank;
>  	u8 kw_type;
>  
> -	npc_mcam_idx_2_key_type(rvu, index, &kw_type);
> +	if (npc_mcam_idx_2_key_type(rvu, index, &kw_type))
> +		return;

Could this leave caller variables uninitialized?

If a function like npc_update_dmac_value() passes an uninitialized u8 intf
parameter on the stack, and this function returns early, the caller might
branch on garbage data when later calling is_npc_intf_tx(intf).

Would it make sense to refactor these functions to return an int so that
failures can be propagated and gracefully handled by the callers?

Re: [PATCH net 3/3] octeontx2-af: npc: cn20k: Return error when defrag rollback free fails

[Simon Horman]

From: 'Simon Horman' <horms@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 plausible.
Full review at: https://sashiko.dev
---
octeontx2-af: npc: cn20k: Return error when defrag rollback free fails

This commit attempts to fix a bug where an error code wasn't returned
when a rollback failure occurred during partial allocation in
npc_defrag_alloc_free_slots(). It sets rc to -EFAULT if the
__npc_subbank_free() call fails.

Link: https://lore.kernel.org/netdev/adjNJEpILRZATB2N@stanley.mountain/

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> index 2fcd0ee2b1e10..df192729ac1d4 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> @@ -3541,6 +3541,7 @@ static int npc_defrag_alloc_free_slots(struct rvu *rvu,
>  			dev_err(rvu->dev,
>  				"%s: Error to free mcam idx=%u\n",
>  				__func__, save[i]);
> +			rc = -EFAULT;
>  			break;
>  		}
>  	}

While this ensures an error is returned if the rollback fails, does the
function still incorrectly return success when the rollback succeeds?

Earlier in the function, if the allocation cannot fulfill the requested slots,
the code jumps to the rollback loop:

	if (alloc_cnt1 + alloc_cnt2 != cnt) {
		...
		goto fail_free_alloc;
	}

Inside the fail_free_alloc loop, the rc variable is overwritten by the
subbank lookup:

fail_free_alloc:
	for (i = 0; i < alloc_cnt1 + alloc_cnt2; i++) {
		rc = npc_mcam_idx_2_subbank_idx(rvu, save[i], &sb, &sb_off);

If the subbank lookup and the subsequent __npc_subbank_free() succeed,
rc will be 0.

Since the loop finishes normally without hitting the newly added rc = -EFAULT
path, does the function end up returning 0 at the end of the fail_free_alloc
block?

Could this cause the caller to incorrectly assume that all slots were
successfully allocated, leading to operations on the freed or uninitialized
slots in the save array? Using a separate variable for the loop operations
or explicitly setting an error code like -ENOSPC before the goto might help
preserve the failure state.

Re: [PATCH net 2/3] octeontx2-af: npc: cn20k: Drop debugfs_create_file() error checks in init

[Simon Horman]

On Thu, Apr 16, 2026 at 09:23:51AM +0530, Ratheesh Kannoth wrote:
> debugfs is not intended to be checked for allocation failures the way
> other kernel APIs are: callers should not fail probe or subsystem init
> because a debugfs node could not be created, including when debugfs is
> disabled in Kconfig.  Replacing NULL checks with IS_ERR() checks is
> similarly wrong for optional debugfs.
> 
> Remove dentry checks and -EFAULT returns from npc_cn20k_debugfs_init().
> https://staticthinking.wordpress.com/2023/07/24/debugfs-functions-are-not-supposed-to-be-checked/
> 
> CC: Dan Carpenter <error27@gmail.com>
> Link: https://lore.kernel.org/netdev/adjNGPWKMOk3KgWL@stanley.mountain/
> Fixes: 528530dff56b ("octeontx2-af: npc: cn20k: add debugfs support")
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

Reviewed-by: Simon Horman <horms@kernel.org>