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

[Ratheesh Kannoth]

On 2026-04-23 at 16:13:16, Ratheesh Kannoth (rkannoth@xxxxxxxxxxx) wrote:
>
octeontx2-af: npc: cn20k: Tear down default MCAM rules explicitly on free

This patch changes npc_cn20k_dft_rules_free() to explicitly tear down
each default MCAM rule by clearing the bitmap, disabling the MCAM line,
and calling npc_cn20k_idx_free(), instead of using the "free all" path.
It also updates npc_mcam_free_all_entries() to skip bitmap clear and
idx_free for cn20k default-rule indices.

>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
>> index 6d3166960a3f..54a25d9c5505 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)
>>  	}
>>
>>  free_rules:
>> +	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
>> +	if (blkaddr < 0)
>> +		return;
>> +	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]);
>> +		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);
>> +		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]);
>> +
>> +			/* Clear entries from mcam_rules, whose indexes are successfully freed
>> +			 * by npc_cn20k_idx_free().
>> +			 */
>> +			ptr[i] = USHRT_MAX;
>> +		}
>
>The comment says "Let us WARN and return" and the commit message says
>"WARN and stop so the box stays up for analysis," but after setting
>ptr[i] = USHRT_MAX, the loop continues processing remaining indices.
>Should the code break or return here to match the comment and commit
>message? Or if continuing is intentional, should the comment say
>"WARN and continue" instead?
AI review report: https://netdev-ai.bots.linux.dev/ai-review.html?id=9923f145-31df-46b0-84e8-b15f1e307ad2#patch-10
pw-bot: changes-requested

Will fix it by returning error code.

>