Re: [v2,net,06/11] octeontx2-af: npc: cn20k: Clear MCAM entries by index and key width

[Paolo Abeni <pabeni@redhat.com>]

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: Clear MCAM entries by index and key width

Replace the old four-argument CN20K MCAM clear with a per-bank static
helper and npc_cn20k_clear_mcam_entry() that takes a logical MCAM index,
resolves the key width via npc_mcam_idx_2_key_type(), and clears either
one bank (X2) or every bank (X4). Call it from npc_clear_mcam_entry() on
cn20k and log when key-type lookup fails. Use the per-bank helper from
npc_cn20k_config_mcam_entry() for pre-program clears. For loopback VFs,
use the promisc MCAM index as ucast_idx when copying RSS action for
promisc, matching cn20k default-rule layout.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> index 705fe7a877e6..7bfd39fe0f5e 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c

[ ... ]

> @@ -877,6 +877,32 @@ npc_cn20k_clear_mcam_entry(struct rvu *rvu, int blkaddr, int bank, int index)
>  		    NPC_AF_CN20K_MCAMEX_BANKX_STAT_EXT(index, bank), 0);
>  }
>
> +int
> +npc_cn20k_clear_mcam_entry(struct rvu *rvu, int blkaddr, int mcam_idx)
> +{
> +	struct npc_mcam *mcam = &rvu->hw->mcam;
> +	int bank = npc_get_bank(mcam, mcam_idx);
> +	u8 kw_type, index;
> +
> +	if (npc_mcam_idx_2_key_type(rvu, mcam_idx, &kw_type))
> +		return -EINVAL;
> +
> +	index = mcam_idx & (mcam->banksize - 1);

Can the index calculation overflow a u8 when banksize exceeds 256?

The mcam->banksize field is u16 and can be up to 65535, with typical
values like 512 or 1024 for NPC MCAM hardware. When computing 'mcam_idx &
(mcam->banksize - 1)', the result will be truncated to 8 bits if it
exceeds 255, causing the wrong MCAM entry to be cleared.

Other code in this driver uses u16 or int for the same calculation:

drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c:npc_cn20k_copy_mcam_entry()
    u16 src, dest;
    ...
    src = src_idx & (mcam->banksize - 1);

drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c:npc_clear_mcam_entry()
    index &= (mcam->banksize - 1);

where index is int.

> +
> +	if (kw_type == NPC_MCAM_KEY_X2) {
> +		npc_clear_x2_entry(rvu, blkaddr, bank, index);
> +		return 0;
> +	}
> +
> +	/* For NPC_MCAM_KEY_X4 keys, both the banks
> +	 * need to be programmed with the same value.
> +	 */
> +	for (bank = 0; bank < mcam->banks_per_entry; bank++)
> +		npc_clear_x2_entry(rvu, blkaddr, bank, index);
> +
> +	return 0;
> +}

[ ... ]

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index 762fafd4ccb7..0a2191fda614 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -259,6 +259,13 @@ static void npc_clear_mcam_entry(struct rvu *rvu, struct npc_mcam *mcam,
>  	int bank = npc_get_bank(mcam, index);
>  	int actbank = bank;
>
> +	if (is_cn20k(rvu->pdev)) {
> +		if (npc_cn20k_clear_mcam_entry(rvu, blkaddr, index))
> +			dev_err(rvu->dev, "%s Failed to enable mcam %u\n",
> +				__func__, index);

This isn't a bug, but the error message says "Failed to enable mcam"
when the operation is clearing the entry. Should this be "Failed to clear
mcam"?

> +		return;
> +	}
> +
>  	index &= (mcam->banksize - 1);
>  	for (; bank < (actbank + mcam->banks_per_entry); bank++) {
>  		rvu_write64(rvu, blkaddr,

[ ... ]