Re: [PATCH v3 net 10/11] octeontx2-af: npc: cn20k: Tear down default MCAM rules explicitly on free

[Ratheesh Kannoth <rkannoth@marvell.com>]

On 2026-04-23 at 16:13:16, Ratheesh Kannoth (rkannoth@marvell.com) wrote:
> npc_cn20k_dft_rules_free() used the NPC MCAM mbox "free all" path, which
> does not match how cn20k tracks default-rule MCAM slots indexes.
>
> Resolve the default-rule indices, then for each valid slot clear the
> bitmap entry, drop the PF/VF map, disable the MCAM line, clear the
> target function, and npc_cn20k_idx_free(). Remove any
> matching software mcam_rules nodes. On hard failure from idx_free, WARN
> and stop so the box stays up for analysis.
>
> In npc_mcam_free_all_entries(), prefetch the same default-rule indices
> and, on cn20k, skip bitmap clear and idx_free when the scanned entry is
> one of those reserved defaults (they are released by
> npc_cn20k_dft_rules_free). Still disable the entry and tear down counter
> mapping for every matching index.
>
> Fixes: 09d3b7a1403f ("octeontx2-af: npc: cn20k: Allocate default MCAM indexes")
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

>> octeontx2-af: npc: cn20k: Tear down default MCAM rules explicitly on free
>[ ... ]
>> Resolve the default-rule indices, then for each valid slot clear the
>> bitmap entry, drop the PF/VF map, disable the MCAM line, clear the
>> target function, and npc_cn20k_idx_free(). Remove any
>> matching software mcam_rules nodes. On hard failure from idx_free, WARN
>> and stop so the box stays up for analysis.
>The commit message says "WARN and stop", but looking at the code below,
>there doesn't appear to be a stop.

ACK. will add a "return" from here. This issue indicates fundemental error in mcam allocation
algorithm. There is no point in graceful handling of error. Returning from here would allow the
debugger to understand the current state of the system and debug the issue.

>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
>> index 6d3166960a3f2a..54a25d9c55053e 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
>> @@ -4228,14 +4228,47 @@ void npc_cn20k_dft_rules_free(struct rvu *rvu, u16 pcifunc)
>[ ... ]
>> +	for (int i = 0; i < 4; i++) {
>> +		if (ptr[i] == USHRT_MAX)
>> +			continue;
>>
>> -	free_req.hdr.pcifunc = pcifunc;
>> -	free_req.all = 1;
>> -	rc = rvu_mbox_handler_npc_mcam_free_entry(rvu, &free_req, &rsp);
>> -	if (rc)
>> -		dev_err(rvu->dev,
>> -			"%s: Error deleting default entries (pcifunc=%#x\n",
>> -			__func__, pcifunc);
>> +		mutex_lock(&mcam->lock);
>> +		npc_mcam_clear_bit(mcam, ptr[i]);
>Can this lead to a double-free of the default rules?
>If an unprivileged VF manually frees its default rule by sending an
>NPC_MCAM_FREE_ENTRY mailbox message,

default entiries are always allocated by Kernel; not by an unprivileged VF manually.

>the single-entry free handler
>(rvu_mbox_handler_npc_mcam_free_entry) frees the hardware index and clears
>the software bitmap, but it does not remove the index from the AF's internal
>xa_pf2dfl_rmap xarray.
>When the VF is later detached, this code fetches the same index from the
>xarray and unconditionally calls npc_mcam_clear_bit(), which increments
>mcam->bmap_fcnt++ and artificially inflates the free count.
>> +		mcam->entry2pfvf_map[ptr[i]] = NPC_MCAM_INVALID_MAP;
>> +		npc_cn20k_enable_mcam_entry(rvu, blkaddr, ptr[i], false);
>> +		mcam->entry2target_pffunc[ptr[i]] = 0x0;
>> +		mutex_unlock(&mcam->lock);
>> +
>> +		rc = npc_cn20k_idx_free(rvu, &ptr[i], 1);
>Since the subbank bitmap was already cleared by the single-entry free
>handler, wouldn't npc_cn20k_idx_free() return -EINVAL for the already
>freed index?
>> +		if (rc) {
>> +			/* Non recoverable error. Let us WARN and return. Keep system alive to
>> +			 * enable debugging
>> +			 */
>> +			WARN(1, "%s Error deleting default entries (pcifunc=%#x) mcam_idx=%u\n",
>> +			     __func__, pcifunc, ptr[i]);
>Does this allow an unprivileged VF to trigger kernel warnings (causing a DoS
>if panic_on_warn is enabled) by freeing its default rules before detaching?

default entiries are always allocated by Kernel; not by an unprivileged VF manually.

>Also, the inline comment says "Let us WARN and return", but there is no
>return or break statement here. Does this just continue the loop instead of
>stopping as intended?

>> +
>> +			/* Clear entries from mcam_rules, whose indexes are successfully freed
>> +			 * by npc_cn20k_idx_free().
>> +			 */
>> +			ptr[i] = USHRT_MAX;
>> +		}
>> +	}