[PATCH net] octeontx2-af: Validate NIX maximum LFs correctly

[PATCH net] octeontx2-af: Validate NIX maximum LFs correctly

[Subbaraya Sundeep]

NIX maximum number of LFs can be set via devlink command
but that can be done before assigning any LFs to a PF/VF.
The condition used to check whether any LFs are assigned is
incorrect. This patch fixes that condition.

Fixes: dd7842878633 ("octeontx2-af: Add new devlink param to configure maximum usable NIX block LFs")
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
---
 .../marvell/octeontx2/af/rvu_devlink.c        | 21 +++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index 6494a9ee2f0d..8ba4b5ba9d2c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -1510,7 +1510,9 @@ static int rvu_af_dl_nix_maxlf_validate(struct devlink *devlink, u32 id,
 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
 	struct rvu *rvu = rvu_dl->rvu;
 	u16 max_nix0_lf, max_nix1_lf;
-	struct npc_mcam *mcam;
+	struct rvu_block *block;
+	int blkaddr = 0;
+	int free_lfs;
 	u64 cfg;
 
 	cfg = rvu_read64(rvu, BLKADDR_NIX0, NIX_AF_CONST2);
@@ -1521,11 +1523,18 @@ static int rvu_af_dl_nix_maxlf_validate(struct devlink *devlink, u32 id,
 	/* Do not allow user to modify maximum NIX LFs while mcam entries
 	 * have already been assigned.
 	 */
-	mcam = &rvu->hw->mcam;
-	if (mcam->bmap_fcnt < mcam->bmap_entries) {
-		NL_SET_ERR_MSG_MOD(extack,
-				   "mcam entries have already been assigned, can't resize");
-		return -EPERM;
+	blkaddr = rvu_get_next_nix_blkaddr(rvu, blkaddr);
+	while (blkaddr) {
+		block = &rvu->hw->block[blkaddr];
+
+		free_lfs = rvu_rsrc_free_count(&block->lf);
+		if (free_lfs != block->lf.max) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "mcam entries have already been assigned, can't resize");
+			return -EPERM;
+		}
+
+		blkaddr = rvu_get_next_nix_blkaddr(rvu, blkaddr);
 	}
 
 	if (max_nix0_lf && val.vu16 > max_nix0_lf) {
-- 
2.48.1

Re: [PATCH net] octeontx2-af: Validate NIX maximum LFs correctly

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
octeontx2-af: Validate NIX maximum LFs correctly

This patch changes the validation condition for setting the maximum
number of NIX LFs via devlink. It replaces the check for assigned MCAM
entries with a check for assigned NIX LFs to determine if resizing
is allowed.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
> index 6494a9ee2f0dd..8ba4b5ba9d2c7 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
[ ... ]
> @@ -1521,11 +1523,18 @@ static int rvu_af_dl_nix_maxlf_validate(struct devlink *devlink, u32 id,
>  	/* Do not allow user to modify maximum NIX LFs while mcam entries
>  	 * have already been assigned.
>  	 */
> -	mcam = &rvu->hw->mcam;
> -	if (mcam->bmap_fcnt < mcam->bmap_entries) {
> -		NL_SET_ERR_MSG_MOD(extack,
> -				   "mcam entries have already been assigned, can't resize");
> -		return -EPERM;

[Severity: High]
Does removing the check for MCAM entries allow the devlink parameter
set operation to silently destroy active MCAM tracking state?

The .set callback unconditionally calls npc_mcam_rsrcs_deinit() and
npc_mcam_rsrcs_init(), which frees and reallocates the mcam->bmap. Since
MCAM entries can be allocated independently of NIX LFs (e.g. during
rvu_switch_enable() or default switchdev/PTP rules), could this cause
resource leaks and use-after-free bugs when untracked rules are later
accessed?

> +	blkaddr = rvu_get_next_nix_blkaddr(rvu, blkaddr);
> +	while (blkaddr) {
> +		block = &rvu->hw->block[blkaddr];

[Severity: High]
This is a pre-existing issue, but does this locklessly read the resource
states?

It appears that the devlink validate and set callbacks execute without
holding the driver's resource locks (rvu->rsrc_lock or mcam->lock).
Could a concurrent mailbox request allocate an LF immediately after this
lockless validation check succeeds, leading to a race where the .set
callback frees structures that are actively being used?

> +
> +		free_lfs = rvu_rsrc_free_count(&block->lf);
> +		if (free_lfs != block->lf.max) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "mcam entries have already been assigned, can't resize");

[Severity: Medium]
Since the validation logic was updated to evaluate NIX LF allocations,
should this error message and the comment above be updated as well? The
message still states that "mcam entries have already been assigned", which
might confuse users since it's actually evaluating NIX LFs.

> +			return -EPERM;
> +		}
> +
> +		blkaddr = rvu_get_next_nix_blkaddr(rvu, blkaddr);
>  	}

Re: [PATCH net] octeontx2-af: Validate NIX maximum LFs correctly

[Simon Horman]

On Wed, Jun 03, 2026 at 05:59:46PM +0100, Simon Horman wrote:
> From: 'Simon Horman' <horms@kernel.org>
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev

Sorry, my tooling isn't helping me much today.
There was supposed to be some commentary in my previous email.
Let me add it.

> ---
> octeontx2-af: Validate NIX maximum LFs correctly
> 
> This patch changes the validation condition for setting the maximum
> number of NIX LFs via devlink. It replaces the check for assigned MCAM
> entries with a check for assigned NIX LFs to determine if resizing
> is allowed.
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
> > index 6494a9ee2f0dd..8ba4b5ba9d2c7 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
> [ ... ]
> > @@ -1521,11 +1523,18 @@ static int rvu_af_dl_nix_maxlf_validate(struct devlink *devlink, u32 id,
> >  	/* Do not allow user to modify maximum NIX LFs while mcam entries
> >  	 * have already been assigned.
> >  	 */
> > -	mcam = &rvu->hw->mcam;
> > -	if (mcam->bmap_fcnt < mcam->bmap_entries) {
> > -		NL_SET_ERR_MSG_MOD(extack,
> > -				   "mcam entries have already been assigned, can't resize");
> > -		return -EPERM;
> 

The issue below looks like it should be looked at in the context of this
patch.

> [Severity: High]
> Does removing the check for MCAM entries allow the devlink parameter
> set operation to silently destroy active MCAM tracking state?
> 
> The .set callback unconditionally calls npc_mcam_rsrcs_deinit() and
> npc_mcam_rsrcs_init(), which frees and reallocates the mcam->bmap. Since
> MCAM entries can be allocated independently of NIX LFs (e.g. during
> rvu_switch_enable() or default switchdev/PTP rules), could this cause
> resource leaks and use-after-free bugs when untracked rules are later
> accessed?

> 
> > +	blkaddr = rvu_get_next_nix_blkaddr(rvu, blkaddr);
> > +	while (blkaddr) {
> > +		block = &rvu->hw->block[blkaddr];

While the one below seems like it can be assessed in the context
of possible follow-up.

> [Severity: High]
> This is a pre-existing issue, but does this locklessly read the resource
> states?
> 
> It appears that the devlink validate and set callbacks execute without
> holding the driver's resource locks (rvu->rsrc_lock or mcam->lock).
> Could a concurrent mailbox request allocate an LF immediately after this
> lockless validation check succeeds, leading to a race where the .set
> callback frees structures that are actively being used?
> 
> > +
> > +		free_lfs = rvu_rsrc_free_count(&block->lf);
> > +		if (free_lfs != block->lf.max) {
> > +			NL_SET_ERR_MSG_MOD(extack,
> > +					   "mcam entries have already been assigned, can't resize");
> 

The one below seems less important. But perhaps it would be good
to address if there is a v2 of this patch.

> [Severity: Medium]
> Since the validation logic was updated to evaluate NIX LF allocations,
> should this error message and the comment above be updated as well? The
> message still states that "mcam entries have already been assigned", which
> might confuse users since it's actually evaluating NIX LFs.
> 
> > +			return -EPERM;
> > +		}
> > +
> > +		blkaddr = rvu_get_next_nix_blkaddr(rvu, blkaddr);
> >  	}