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

Ratheesh Kannoth
2026-04-24T09:11:18Z

On 2026-04-23 at 16:13:16, Ratheesh Kannoth (rkannoth@marvell.com) 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.

>