Re: [net-next,v8,09/13] octeontx2-af: npc: cn20k: virtual index support

[Ratheesh Kannoth]

On 2026-02-05 at 07:55:25, Jakub Kicinski (kuba@xxxxxxxxxx) wrote:
> > +		dev_err(rvu->dev,
> > +			"%s: Failed to roll back vidx2idx vidx=%u\n",
> > +			__func__, vidx);
>
> At fail2, xa_erase() returns the erased entry on success (non-NULL) and NULL
> on failure. The error message is printed when xa_erase() succeeds, which
> appears inverted from the intended behavior.
>
> Should the condition check for NULL to detect failure instead?
>
> > +
> > +fail1:
> > +	if (xa_insert(&npc_priv.xa_vidx2idx_map, vidx,
> > +		      xa_mk_value(old_midx), GFP_KERNEL))
> > +		dev_err(rvu->dev,
> > +			"%s: Faled to roll back vidx2idx to old_midx=%u, vidx=%u\n",
> > +			__func__, old_midx, vidx);
>
> This isn't a bug, but there's a typo: "Faled" should be "Failed".
>
> Also, at fail1, the rollback attempts to insert into xa_vidx2idx_map to
> restore old_midx. However, looking at the control flow, fail1 is reached when
> xa_insert() to xa_vidx2idx_map fails. At that point, xa_vidx2idx_map has
> already been erased, so the rollback target is correct.
>
> But fail2 rolls back using xa_erase() on xa_vidx2idx_map, which was just
> inserted successfully. The rollback at fail1 should restore xa_idx2vidx_map
> instead, since that's the only map modified when fail1 is reached via direct
> return.
>
> Could the error path rollback logic be reviewed?
This is getting flagged every time. i dont see any issue in the logic. Am i missing
any thing here.