Re: [PATCH net-next 4/5] cteontx2-af: npc: cn20k: dynamically allocate and free default MCAM entries
From: Simon Horman <horms@kernel.org>
Date: 2026-03-06 13:21:47
Also in:
lkml
On Mon, Mar 02, 2026 at 02:28:02PM +0530, Ratheesh Kannoth wrote: ...
quoted hunk ↗ jump to hunk
@@ -4220,21 +4242,46 @@ void npc_cn20k_dft_rules_free(struct rvu *rvu, u16 pcifunc) index = NPC_DFT_RULE_ID_MK(pcifunc, i); map = xa_erase(&npc_priv.xa_pf2dfl_rmap, index); if (!map) - dev_dbg(rvu->dev, + dev_err(rvu->dev, "%s: Err from delete %s mcam idx from xarray (pcifunc=%#x\n", __func__, npc_dft_rule_name[i], pcifunc); } free_rules: + blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0); + if (blkaddr < 0) + return; - 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); + for (int i = 0; i < 4; i++) { + if (ptr[i] == USHRT_MAX) + continue; + + 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) + dev_err(rvu->dev, + "%s:%d Error deleting default entries (pcifunc=%#x) mcam_idx=%u\n", + __func__, __LINE__, pcifunc, ptr[i]); + } + + 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;
Hi Ratheesh, I assume that the condition above should match exactly once for each iteration of the for loop. But if not rule may either not be freed or freed twice. Would it make sense to move freeing rule to outside the for loop? I think this would address both problems. But perhaps it leads to rule being freed when it should not be. Alternatively, should there be a break after the call to kfree()? I think that would address only the (theoretical possibility of) a double free. Flagged by Smatch.
+ + list_del(&rule->list); + kfree(rule); + } + } + mutex_unlock(&mcam->lock); }
...