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

Ratheesh Kannoth
2026-04-27T10:09:24Z

On 2026-04-27 at 12:02:12, 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>

>> 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]);
>> + return;
>When npc_cn20k_idx_free() fails on some ptr[i], this path WARNs and
>returns from inside the per-entry loop. At that point, the earlier
>block in npc_cn20k_dft_rules_free() has already xa_erase()d the
>xa_pf2dfl_rmap entries for all four default rule IDs for this pcifunc,
>so npc_cn20k_dft_rules_idx_get() on a retry would return -ESRCH and
>could not reclaim anything.
>Given that, could the remaining ptr[i+1..3] entries leak on this path?
>Their bitmap bits stay cleared only if they were processed before the
>failure, but any entries after i still have their bitmap slot allocated,
>entry2pfvf_map still referencing pcifunc, MCAM line still enabled,
>entry2target_pffunc still set, and the cn20k idx allocator still holding
>the slots.
No. This is a non recoverable error and no way we can recover and proceed. So this warning
and returning would enable the user to debug the system.

>> + }
>> + }
>> +
>> + mutex_lock(&mcam->lock);
>> + list_for_each_entry_safe(rule, tmp, &mcam->mcam_rules, list) {
>> + for (int i = 0; i < 4; i++) {
>> + if (ptr[i] != rule->entry)
>> + continue;
>> +
>> + list_del(&rule->list);
>> + kfree(rule);
>> + break;
>> + }
>> + }
>> + mutex_unlock(&mcam->lock);
>> }
>On the same error path, the list_for_each_entry_safe() walk over
>mcam->mcam_rules below is skipped entirely because of the early return,
>so rvu_npc_mcam_rule nodes for all four default indices (including the
>ones that were successfully torn down before the failure) stay on the
>list.
>The commit message says:
> 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.
>Is the "Remove any matching software mcam_rules nodes" step intended to
>also be skipped whenever idx_free() fails? If not, would it make sense
>to move the mcam_rules cleanup ahead of the per-entry loop, or continue
>the loop best-effort and WARN once at the end, so the software list and
>the already-torn-down slots do not silently diverge on the error path?
No. mcam list will show more information in debugfs. I dont think, freeing 4 entries (in default entries)
would save us anything. So better to keep this way.