Re: [net-next,v5,09/13] octeontx2-af: npc: cn20k: virtual index support
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-28 02:29:29
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- octeontx2-af: npc: cn20k: virtual index support This patch adds support for virtual MCAM index allocation and improves CN20K MCAM defragmentation handling. Virtual indexes allow the hardware to move mapped MCAM entries internally, enabling defragmentation.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c index 8b77ac053d8d..d1a927f3d5db 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
[ ... ]
+static int npc_vidx_maps_modify(struct rvu *rvu, u16 vidx, u16 new_midx)
+{
+ u16 old_midx;
+ void *map;
+ int rc;
+
+ if (!npc_is_vidx(vidx)) {
+ dev_err(rvu->dev,
+ "%s: vidx(%u) does not map to proper mcam idx\n",
+ __func__, vidx);
+ return -ESRCH;
+ }
+
+ map = xa_erase(&npc_priv.xa_vidx2idx_map, vidx);
+ if (!map) {
+ dev_err(rvu->dev,
+ "%s: vidx(%u) could not be deleted from vidx2idx map\n",
+ __func__, vidx);
+ return -ESRCH;
+ }
+
+ old_midx = xa_to_value(map);
+
+ rc = xa_insert(&npc_priv.xa_vidx2idx_map, vidx,
+ xa_mk_value(new_midx), GFP_KERNEL);
+ if (rc) {
+ dev_err(rvu->dev,
+ "%s: vidx(%u) cannot be added to vidx2idx map\n",
+ __func__, vidx);
+ return rc;
+ }
+
+ map = xa_erase(&npc_priv.xa_idx2vidx_map, old_midx);
+ if (!map) {
+ dev_err(rvu->dev,
+ "%s: old_midx(%u, vidx(%u)) cannot be added to idx2vidx map\n",
+ __func__, old_midx, vidx);
+ return -ESRCH;If this error path is taken, xa_vidx2idx_map now has vidx pointing to new_midx, but xa_idx2vidx_map still has old_midx pointing back to vidx. The bidirectional mapping becomes inconsistent. Can this leave the maps in a corrupted state?
+ }
+
+ rc = xa_insert(&npc_priv.xa_idx2vidx_map, new_midx,
+ xa_mk_value(vidx), GFP_KERNEL);
+ if (rc) {
+ dev_err(rvu->dev,
+ "%s: new_midx(%u, vidx(%u)) cannot be added to idx2vidx map\n",
+ __func__, new_midx, vidx);
+ return rc;Similarly here, if xa_insert fails (e.g., memory pressure), the maps are left inconsistent: xa_vidx2idx_map has vidx -> new_midx but xa_idx2vidx_map has neither old_midx nor new_midx pointing to vidx. Subsequent lookups via npc_idx2vidx() for new_midx would fail to find the correct vidx. Should these error paths restore the previous map state?
+ } + + return 0; +}
[ ... ]
+static int npc_defrag_process(struct rvu *rvu, struct list_head *lh)
+{[ ... ]
+err: + /* TODO: how to go back to old state ? */ + return rc; +}
The TODO comment here suggests awareness that error recovery is incomplete. Given that npc_defrag_move_vdx_to_free() calls npc_vidx_maps_modify() which can fail mid-operation, should the defragmentation be made atomic or have proper rollback?