Re: [PATCH v3 net 01/11] octeontx2-af: npc: cn20k: Propagate MCAM key-type errors on cn20k

[Ratheesh Kannoth]

On 2026-04-23 at 16:13:07, Ratheesh Kannoth (rkannoth@xxxxxxxxxxx) wrote:
> npc_mcam_idx_2_key_type() can fail; callers used to ignore it and still
> used kw_type when enabling, configuring, copying, and reading MCAM
> entries. That could program or decode hardware with an undefined key
> type.
>
> Return -EINVAL when key-type lookup fails. Return -EINVAL from
> npc_cn20k_copy_mcam_entry() when src and dest key types differ instead
> of failing silently.
>
> Change npc_cn20k_{enable,config,copy,read}_mcam_entry() to return int on
> success or error. Thread those errors through the cn20k MCAM write and
> read mbox handlers, the cn20k baseline steer read path, NPC defrag
> move (disable/copy/enable with dev_err and -EFAULT), and the DMAC
> update path in rvu_npc_fs.c.
>
> Make npc_copy_mcam_entry() return int so the cn20k branch can return
> npc_cn20k_copy_mcam_entry() without a void/int mismatch, and fail
> NPC_MCAM_SHIFT_ENTRY when copy fails.
>
> Cc: Suman Ghosh <sumang@xxxxxxxxxxx>
> Cc: Dan Carpenter <error27@xxxxxxxxx>
> Fixes: 6d1e70282f76 ("octeontx2-af: npc: cn20k: Use common APIs")
> Link: https://lore.kernel.org/netdev/adiQJvuKlEhq2ILx@stanley.mountain/
> Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>
> ---
>  .../ethernet/marvell/octeontx2/af/cn20k/npc.c | 109 ++++++++++++------
>  .../ethernet/marvell/octeontx2/af/cn20k/npc.h |  20 ++--
>  .../ethernet/marvell/octeontx2/af/rvu_npc.c   |  18 ++-
>  .../marvell/octeontx2/af/rvu_npc_fs.c         |  20 ++--
>  4 files changed, 111 insertions(+), 56 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..49b5cd2838b5 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> index 7291fdb89b03..49b5cd2838b5 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c

[ ... ]

> @@ -3607,9 +3627,30 @@ int npc_defrag_move_vdx_to_free(struct rvu *rvu,
>  				   NPC_AF_CN20K_MCAMEX_BANKX_STAT_EXT(midx,
>  								      bank));
>
> -		npc_cn20k_enable_mcam_entry(rvu, blkaddr, old_midx, false);
> -		npc_cn20k_copy_mcam_entry(rvu, blkaddr, old_midx, new_midx);
> -		npc_cn20k_enable_mcam_entry(rvu, blkaddr, new_midx, true);
> +		/* If bug happened during copy/enable mcam, then there is a bug in allocation
> +		 * algorithm itself. There is no point in rewinding and returning, as it
> +		 * will face further issue. Return error after printing error
> +		 */
> +		if (npc_cn20k_enable_mcam_entry(rvu, blkaddr, old_midx, false)) {
> +			dev_err(rvu->dev,
> +				"%s: Error happened while disabling old_mid=%u\n",
> +				__func__, old_midx);
> +			return -EFAULT;
> +		}
> +
> +		if (npc_cn20k_copy_mcam_entry(rvu, blkaddr, old_midx, new_midx)) {
> +			dev_err(rvu->dev,
> +				"%s: Error happened to while copying old_midx=%u new_midx=%u\n",
                                               ^^
This isn't a bug, but there's an extra 'to' here. The message should say
'Error happened while copying' rather than 'Error happened to while copying'.

> +				__func__, old_midx, new_midx);
> +			return -EFAULT;
> +		}
> +
> +		if (npc_cn20k_enable_mcam_entry(rvu, blkaddr, new_midx, true)) {
> +			dev_err(rvu->dev,
> +				"%s: Error happened while enabling new_mid=%u\n",
> +				__func__, new_midx);
> +			return -EFAULT;
> +		}
>
>  		midx = new_midx % mcam->banksize;
>  		bank = new_midx / mcam->banksize;

AI review report: https://netdev-ai.bots.linux.dev/ai-review.html?id=9923f145-31df-46b0-84e8-b15f1e307ad2#patch-10
pw-bot: changes-requested

Grammatical mistake in string will fix it in next patch version.