[PATCH v4 net] octeontx2-pf: reset HTB scheduler topology before freeing queues

[PATCH v4 net] octeontx2-pf: reset HTB scheduler topology before freeing queues

[Ratheesh Kannoth]

HTB offload programs NIX_AF_TLxX_TOPOLOGY on QoS-allocated scheduler
queues via otx2_qos_txschq_set_parent_topology(), but teardown freed
those queues without clearing TOPOLOGY.  The AF only restores PARENT and
SCHEDULE on free, so PRIO_ANCHOR/RR_PRIO settings can survive in the
shared scheduler pool and affect later allocations.

Add otx2_qos_reset_schq_topology() and otx2_qos_free_hw_schq() to zero
TL4 through TL2 TOPOLOGY before each schq is returned to the AF during
hierarchy teardown and cfg rollback.  Skip the aggregation level (TL1):
it is a per-tx-link queue shared by the PF, default Tx hierarchy and VFs,
and is not freed back to the AF by nix_txschq_free_one().

Fixes: 5e6808b4c68d ("octeontx2-pf: Add support for HTB offload")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

---
v3 -> v4: Addressed AI review comments.
	https://lore.kernel.org/netdev/20260902172428.534936-1-horms@kernel.org/
v2 -> v3: Addressed AI review comments.
	https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824072147.3524339-1-rkannoth@marvell.com?part=1
v1 -> v2: Addressed sashiko comments
        https://sashiko.dev/#/patchset/20260821072812.2890922-1-rkannoth%40marvell.com
---
 .../net/ethernet/marvell/octeontx2/nic/qos.c  | 60 +++++++++++++++++--
 1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
index 69c0911e28e9..f160b1618efa 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
@@ -235,13 +235,63 @@ static int otx2_qos_txschq_set_parent_topology(struct otx2_nic *pfvf,
 	return rc;
 }
 
+static int otx2_qos_reset_schq_topology(struct otx2_nic *pfvf, u16 lvl,
+					u16 schq)
+{
+	struct mbox *mbox = &pfvf->mbox;
+	struct nix_txschq_config *cfg;
+	int rc;
+
+	if (lvl < NIX_TXSCH_LVL_TL4 || lvl >= NIX_TXSCH_LVL_TL1)
+		return 0;
+
+	mutex_lock(&mbox->lock);
+
+	cfg = otx2_mbox_alloc_msg_nix_txschq_cfg(mbox);
+	if (!cfg) {
+		mutex_unlock(&mbox->lock);
+		return -ENOMEM;
+	}
+
+	cfg->lvl = lvl;
+	cfg->num_regs = 1;
+
+	if (lvl == NIX_TXSCH_LVL_TL4)
+		cfg->reg[0] = NIX_AF_TL4X_TOPOLOGY(schq);
+	else if (lvl == NIX_TXSCH_LVL_TL3)
+		cfg->reg[0] = NIX_AF_TL3X_TOPOLOGY(schq);
+	else if (lvl == NIX_TXSCH_LVL_TL2)
+		cfg->reg[0] = NIX_AF_TL2X_TOPOLOGY(schq);
+
+	cfg->regval[0] = 0;
+
+	rc = otx2_sync_mbox_msg(mbox);
+
+	mutex_unlock(&mbox->lock);
+
+	return rc;
+}
+
+static void otx2_qos_free_hw_schq(struct otx2_nic *pfvf, u16 lvl, u16 schq)
+{
+	int err;
+
+	err = otx2_qos_reset_schq_topology(pfvf, lvl, schq);
+	if (err)
+		netdev_warn(pfvf->netdev,
+			    "QoS: failed to reset topology for schq %u at level %u: %d\n",
+			    schq, lvl, err);
+
+	otx2_txschq_free_one(pfvf, lvl, schq);
+}
+
 static void otx2_qos_free_hw_node_schq(struct otx2_nic *pfvf,
 				       struct otx2_qos_node *parent)
 {
 	struct otx2_qos_node *node;
 
 	list_for_each_entry_reverse(node, &parent->child_schq_list, list)
-		otx2_txschq_free_one(pfvf, node->level, node->schq);
+		otx2_qos_free_hw_schq(pfvf, node->level, node->schq);
 }
 
 static void otx2_qos_free_hw_node(struct otx2_nic *pfvf,
@@ -252,7 +302,7 @@ static void otx2_qos_free_hw_node(struct otx2_nic *pfvf,
 	list_for_each_entry_safe(node, tmp, &parent->child_list, list) {
 		otx2_qos_free_hw_node(pfvf, node);
 		otx2_qos_free_hw_node_schq(pfvf, node);
-		otx2_txschq_free_one(pfvf, node->level, node->schq);
+		otx2_qos_free_hw_schq(pfvf, node->level, node->schq);
 	}
 }
 
@@ -266,7 +316,7 @@ static void otx2_qos_free_hw_cfg(struct otx2_nic *pfvf,
 	otx2_qos_free_hw_node_schq(pfvf, node);
 
 	/* free node hw mappings */
-	otx2_txschq_free_one(pfvf, node->level, node->schq);
+	otx2_qos_free_hw_schq(pfvf, node->level, node->schq);
 
 	mutex_unlock(&pfvf->qos.qos_lock);
 }
@@ -913,7 +963,7 @@ static void otx2_qos_free_cfg(struct otx2_nic *pfvf, struct otx2_qos_cfg *cfg)
 	for (lvl = 0; lvl < NIX_TXSCH_LVL_CNT; lvl++) {
 		for (idx = 0; idx < cfg->schq[lvl]; idx++) {
 			schq = cfg->schq_list[lvl][idx];
-			otx2_txschq_free_one(pfvf, lvl, schq);
+			otx2_qos_free_hw_schq(pfvf, lvl, schq);
 		}
 	}
 
@@ -921,7 +971,7 @@ static void otx2_qos_free_cfg(struct otx2_nic *pfvf, struct otx2_qos_cfg *cfg)
 		for (idx = 0; idx < cfg->schq_contig[lvl]; idx++) {
 			if (cfg->schq_index_used[lvl][idx]) {
 				schq = cfg->schq_contig_list[lvl][idx];
-				otx2_txschq_free_one(pfvf, lvl, schq);
+				otx2_qos_free_hw_schq(pfvf, lvl, schq);
 			}
 		}
 	}
-- 
2.43.0

Re: [PATCH v4 net] octeontx2-pf: reset HTB scheduler topology before freeing queues

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 1 · Low: 0

- [High] Incomplete fix: the stale-TOPOLOGY bug class the commit claims
  to close stays reachable through the AF's bulk release path.
- [Medium] Set/reset asymmetry at the aggregation level: HTB offload
  programs NIX_AF_TL1X_TOPOLOGY (PRIO_ANCHOR/RR_PRIO) on the shared per-…

--

octeontx2-pf: reset HTB scheduler topology before freeing queues

This adds otx2_qos_reset_schq_topology() and otx2_qos_free_hw_schq() so
that NIX_AF_TL{4,3,2}X_TOPOLOGY is zeroed before each QoS scheduler queue
is handed back to the AF during HTB hierarchy teardown and cfg rollback.
The aggregation level (TL1) is intentionally skipped.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> index 69c0911e28e9..f160b1618efa 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> @@ -235,13 +235,63 @@ static int otx2_qos_txschq_set_parent_topology(struct otx2_nic *pfvf,
>  	return rc;
>  }
>  
> +static int otx2_qos_reset_schq_topology(struct otx2_nic *pfvf, u16 lvl,
> +					u16 schq)
> +{
> +	struct mbox *mbox = &pfvf->mbox;
> +	struct nix_txschq_config *cfg;
> +	int rc;
> +
> +	if (lvl < NIX_TXSCH_LVL_TL4 || lvl >= NIX_TXSCH_LVL_TL1)
> +		return 0;

[Severity: Medium]
Should the TL1 case be handled here too, or at least commented in the
code?  otx2_qos_txschq_set_parent_topology() does program TL1:

	else if (parent->level == NIX_TXSCH_LVL_TL1)
		cfg->reg[0] = NIX_AF_TL1X_TOPOLOGY(parent->schq);

	cfg->regval[0] = (u64)parent->prio_anchor << 32;
	cfg->regval[0] |= ((parent->child_dwrr_prio != OTX2_QOS_DEFAULT_PRIO) ?
			    parent->child_dwrr_prio : 0)  << 1;

and for a PF the HTB root sits at that level, in otx2_qos_alloc_root():

	if (!is_otx2_vf(pfvf->pcifunc)) {
		node->level = NIX_TXSCH_LVL_TL1;

so otx2_qos_txschq_push_cfg_tl() -> otx2_qos_txschq_set_parent_topology()
writes NIX_AF_TL1X_TOPOLOGY with the HTB prio_anchor, which is the base
index of the contiguous TL2 child block.

On tc qdisc del, otx2_qos_root_destroy() -> otx2_qos_destroy_node() ->
otx2_qos_free_hw_cfg() calls otx2_qos_free_hw_schq(pfvf, node->level,
node->schq) with node->level == NIX_TXSCH_LVL_TL1, which takes the early
return above.

Does anything else restore the default TL1 topology while the interface
stays up?  nix_txschq_free_one() early-returns for lvl >=
hw->cap.nix_tx_aggr_lvl, nix_reset_tx_schedule() has no TL1 case, and
nix_tl1_default_cfg() declines to reprogram once the PF has configured
TL1:

	/* Skip if PF has already done the config */
	if (TXSCH_MAP_FLAGS(pfvf_map[schq]) & NIX_TXSCHQ_CFG_DONE)
		return;
	rvu_write64(rvu, blkaddr, NIX_AF_TL1X_TOPOLOGY(schq),
		    (TXSCH_TL1_DFLT_RR_PRIO << 1));

The only writer of the default TL1 value appears to be
otx2_txschq_config():

	req->reg[1] = NIX_AF_TL1X_TOPOLOGY(schq);
	req->regval[1] = hw->txschq_aggr_lvl_rr_prio << 1;

which runs at interface open rather than on TC_HTB_DESTROY.  So after HTB
teardown with the link up, the shared TL1 keeps a PRIO_ANCHOR pointing at
TL2 indices that were just returned to the AF's block-wide TL2 pool and
may be re-allocated to another function.

The commit message says TL1 "is not freed back to the AF by
nix_txschq_free_one()", which rules out re-allocating TL1 itself, but does
it cover the residual anchor pointing into recycled TL2 indices?  Whether
the TL1 arbiter can act on such a stale anchor depends on hardware
semantics that are not visible from the tree, so this may be harmless in
practice, but the set/reset asymmetry looks worth spelling out in the code
rather than only in the changelog.

> +
> +	mutex_lock(&mbox->lock);
> +
> +	cfg = otx2_mbox_alloc_msg_nix_txschq_cfg(mbox);
> +	if (!cfg) {
> +		mutex_unlock(&mbox->lock);
> +		return -ENOMEM;
> +	}
> +
> +	cfg->lvl = lvl;
> +	cfg->num_regs = 1;
> +
> +	if (lvl == NIX_TXSCH_LVL_TL4)
> +		cfg->reg[0] = NIX_AF_TL4X_TOPOLOGY(schq);
> +	else if (lvl == NIX_TXSCH_LVL_TL3)
> +		cfg->reg[0] = NIX_AF_TL3X_TOPOLOGY(schq);
> +	else if (lvl == NIX_TXSCH_LVL_TL2)
> +		cfg->reg[0] = NIX_AF_TL2X_TOPOLOGY(schq);
> +
> +	cfg->regval[0] = 0;
> +
> +	rc = otx2_sync_mbox_msg(mbox);
> +
> +	mutex_unlock(&mbox->lock);
> +
> +	return rc;
> +}
> +
> +static void otx2_qos_free_hw_schq(struct otx2_nic *pfvf, u16 lvl, u16 schq)
> +{
> +	int err;
> +
> +	err = otx2_qos_reset_schq_topology(pfvf, lvl, schq);

[Severity: High]
Does this cover the case where the AF, not the driver, releases the QoS
queues?  The reset only runs on the driver's explicit per-schq frees, but
nix_txschq_free() in af/rvu_nix.c returns the same TL4/TL3/TL2 indices to
the shared pool with only PARENT and SCHEDULE cleared:

	nix_reset_tx_schedule(rvu, blkaddr, lvl, schq);
	rvu_free_rsrc(&txsch->schq, schq);
	txsch->pfvf_map[schq] = TXSCH_MAP(0, NIX_TXSCHQ_FREE);

and nix_reset_tx_schedule() writes TLxX_PARENT and TLxX_SCHEDULE only, so
TOPOLOGY (PRIO_ANCHOR/RR_PRIO) stays programmed.

That path is reachable while an HTB qdisc is still offloaded, for example
on a function reset (echo 1 > /sys/bus/pci/devices/<bdf>/reset) or a VF
FLR from a passthrough guest:

rvu_flr_handler() -> rvu_blklf_teardown() -> rvu_nix_lf_teardown() ->
nix_txschq_free()

In that case otx2_qos_free_hw_schq() never runs, so the state the commit
message describes ("PRIO_ANCHOR/RR_PRIO settings can survive in the
shared scheduler pool and affect later allocations") is unchanged.

The allocation side also looks unsanitized.  nix_txschq_alloc() resets
linkcfg, shaping and schedule for every newly allocated schq:

	nix_reset_tx_linkcfg(rvu, blkaddr, lvl, schq);
	nix_reset_tx_shaping(rvu, blkaddr, nixlf, lvl, schq);
	nix_reset_tx_schedule(rvu, blkaddr, lvl, schq);

but not TOPOLOGY, and otx2_txschq_config() never writes TOPOLOGY for
TL2/TL3/TL4, so a new owner inherits whatever anchor was left behind.

Would adding the TLxX_TOPOLOGY register to nix_reset_tx_schedule() be a
better place for this?  That would cover free_one, TXSCHQ_FREE_ALL, FLR
and allocation-time inheritance for all AF clients under rvu->rsrc_lock,
and the driver-side helper added here would not be needed.

> +	if (err)
> +		netdev_warn(pfvf->netdev,
> +			    "QoS: failed to reset topology for schq %u at level %u: %d\n",
> +			    schq, lvl, err);
> +
> +	otx2_txschq_free_one(pfvf, lvl, schq);
> +}
> +
>  static void otx2_qos_free_hw_node_schq(struct otx2_nic *pfvf,
>  				       struct otx2_qos_node *parent)
>  {
>  	struct otx2_qos_node *node;
>  
>  	list_for_each_entry_reverse(node, &parent->child_schq_list, list)
> -		otx2_txschq_free_one(pfvf, node->level, node->schq);
> +		otx2_qos_free_hw_schq(pfvf, node->level, node->schq);
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903020533.3068041-1-rkannoth%40marvell.com

Re: [PATCH v4 net] octeontx2-pf: reset HTB scheduler topology before freeing queues

[Ratheesh Kannoth]

On 2026-09-08 at 22:37:40, netdev-bot+sashiko@kernel.org (netdev-bot+sashiko@kernel.org) wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 · Medium: 1 · Low: 0
>
> - [High] Incomplete fix: the stale-TOPOLOGY bug class the commit claims
>   to close stays reachable through the AF's bulk release path.
AF's built release path is FLR/PCI reset path. We will fix it as seperate fix.

> - [Medium] Set/reset asymmetry at the aggregation level: HTB offload
>   programs NIX_AF_TL1X_TOPOLOGY (PRIO_ANCHOR/RR_PRIO) on the shared per-…
We can't clear TL1 blindly as it is a shared resource between PF and VF.

>
> --
>
> octeontx2-pf: reset HTB scheduler topology before freeing queues
>
> This adds otx2_qos_reset_schq_topology() and otx2_qos_free_hw_schq() so
> that NIX_AF_TL{4,3,2}X_TOPOLOGY is zeroed before each QoS scheduler queue
> is handed back to the AF during HTB hierarchy teardown and cfg rollback.
> The aggregation level (TL1) is intentionally skipped.
>
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> > index 69c0911e28e9..f160b1618efa 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> > @@ -235,13 +235,63 @@ static int otx2_qos_txschq_set_parent_topology(struct otx2_nic *pfvf,
> >  	return rc;
> >  }
> >
> > +static int otx2_qos_reset_schq_topology(struct otx2_nic *pfvf, u16 lvl,
> > +					u16 schq)
> > +{
> > +	struct mbox *mbox = &pfvf->mbox;
> > +	struct nix_txschq_config *cfg;
> > +	int rc;
> > +
> > +	if (lvl < NIX_TXSCH_LVL_TL4 || lvl >= NIX_TXSCH_LVL_TL1)
> > +		return 0;
>
> [Severity: Medium]
> Should the TL1 case be handled here too, or at least commented in the
> code?  otx2_qos_txschq_set_parent_topology() does program TL1:
>
> 	else if (parent->level == NIX_TXSCH_LVL_TL1)
> 		cfg->reg[0] = NIX_AF_TL1X_TOPOLOGY(parent->schq);
>
> 	cfg->regval[0] = (u64)parent->prio_anchor << 32;
> 	cfg->regval[0] |= ((parent->child_dwrr_prio != OTX2_QOS_DEFAULT_PRIO) ?
> 			    parent->child_dwrr_prio : 0)  << 1;
>
> and for a PF the HTB root sits at that level, in otx2_qos_alloc_root():
>
> 	if (!is_otx2_vf(pfvf->pcifunc)) {
> 		node->level = NIX_TXSCH_LVL_TL1;
>
> so otx2_qos_txschq_push_cfg_tl() -> otx2_qos_txschq_set_parent_topology()
> writes NIX_AF_TL1X_TOPOLOGY with the HTB prio_anchor, which is the base
> index of the contiguous TL2 child block.
>
> On tc qdisc del, otx2_qos_root_destroy() -> otx2_qos_destroy_node() ->
> otx2_qos_free_hw_cfg() calls otx2_qos_free_hw_schq(pfvf, node->level,
> node->schq) with node->level == NIX_TXSCH_LVL_TL1, which takes the early
> return above.
>
> Does anything else restore the default TL1 topology while the interface
> stays up?  nix_txschq_free_one() early-returns for lvl >=
> hw->cap.nix_tx_aggr_lvl, nix_reset_tx_schedule() has no TL1 case, and
> nix_tl1_default_cfg() declines to reprogram once the PF has configured
> TL1:
>
> 	/* Skip if PF has already done the config */
> 	if (TXSCH_MAP_FLAGS(pfvf_map[schq]) & NIX_TXSCHQ_CFG_DONE)
> 		return;
> 	rvu_write64(rvu, blkaddr, NIX_AF_TL1X_TOPOLOGY(schq),
> 		    (TXSCH_TL1_DFLT_RR_PRIO << 1));
>
> The only writer of the default TL1 value appears to be
> otx2_txschq_config():
>
> 	req->reg[1] = NIX_AF_TL1X_TOPOLOGY(schq);
> 	req->regval[1] = hw->txschq_aggr_lvl_rr_prio << 1;
>
> which runs at interface open rather than on TC_HTB_DESTROY.  So after HTB
> teardown with the link up, the shared TL1 keeps a PRIO_ANCHOR pointing at
> TL2 indices that were just returned to the AF's block-wide TL2 pool and
> may be re-allocated to another function.
>
> The commit message says TL1 "is not freed back to the AF by
> nix_txschq_free_one()", which rules out re-allocating TL1 itself, but does
> it cover the residual anchor pointing into recycled TL2 indices?  Whether
> the TL1 arbiter can act on such a stale anchor depends on hardware
> semantics that are not visible from the tree, so this may be harmless in
> practice, but the set/reset asymmetry looks worth spelling out in the code
> rather than only in the changelog.
>
> > +
> > +	mutex_lock(&mbox->lock);
> > +
> > +	cfg = otx2_mbox_alloc_msg_nix_txschq_cfg(mbox);
> > +	if (!cfg) {
> > +		mutex_unlock(&mbox->lock);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	cfg->lvl = lvl;
> > +	cfg->num_regs = 1;
> > +
> > +	if (lvl == NIX_TXSCH_LVL_TL4)
> > +		cfg->reg[0] = NIX_AF_TL4X_TOPOLOGY(schq);
> > +	else if (lvl == NIX_TXSCH_LVL_TL3)
> > +		cfg->reg[0] = NIX_AF_TL3X_TOPOLOGY(schq);
> > +	else if (lvl == NIX_TXSCH_LVL_TL2)
> > +		cfg->reg[0] = NIX_AF_TL2X_TOPOLOGY(schq);
> > +
> > +	cfg->regval[0] = 0;
> > +
> > +	rc = otx2_sync_mbox_msg(mbox);
> > +
> > +	mutex_unlock(&mbox->lock);
> > +
> > +	return rc;
> > +}
> > +
> > +static void otx2_qos_free_hw_schq(struct otx2_nic *pfvf, u16 lvl, u16 schq)
> > +{
> > +	int err;
> > +
> > +	err = otx2_qos_reset_schq_topology(pfvf, lvl, schq);
>
> [Severity: High]
> Does this cover the case where the AF, not the driver, releases the QoS
> queues?  The reset only runs on the driver's explicit per-schq frees, but
> nix_txschq_free() in af/rvu_nix.c returns the same TL4/TL3/TL2 indices to
> the shared pool with only PARENT and SCHEDULE cleared:
>
> 	nix_reset_tx_schedule(rvu, blkaddr, lvl, schq);
> 	rvu_free_rsrc(&txsch->schq, schq);
> 	txsch->pfvf_map[schq] = TXSCH_MAP(0, NIX_TXSCHQ_FREE);
>
> and nix_reset_tx_schedule() writes TLxX_PARENT and TLxX_SCHEDULE only, so
> TOPOLOGY (PRIO_ANCHOR/RR_PRIO) stays programmed.
>
> That path is reachable while an HTB qdisc is still offloaded, for example
> on a function reset (echo 1 > /sys/bus/pci/devices/<bdf>/reset) or a VF
> FLR from a passthrough guest:
>
> rvu_flr_handler() -> rvu_blklf_teardown() -> rvu_nix_lf_teardown() ->
> nix_txschq_free()
>
> In that case otx2_qos_free_hw_schq() never runs, so the state the commit
> message describes ("PRIO_ANCHOR/RR_PRIO settings can survive in the
> shared scheduler pool and affect later allocations") is unchanged.
>
> The allocation side also looks unsanitized.  nix_txschq_alloc() resets
> linkcfg, shaping and schedule for every newly allocated schq:
>
> 	nix_reset_tx_linkcfg(rvu, blkaddr, lvl, schq);
> 	nix_reset_tx_shaping(rvu, blkaddr, nixlf, lvl, schq);
> 	nix_reset_tx_schedule(rvu, blkaddr, lvl, schq);
>
> but not TOPOLOGY, and otx2_txschq_config() never writes TOPOLOGY for
> TL2/TL3/TL4, so a new owner inherits whatever anchor was left behind.
>
> Would adding the TLxX_TOPOLOGY register to nix_reset_tx_schedule() be a
> better place for this?  That would cover free_one, TXSCHQ_FREE_ALL, FLR
> and allocation-time inheritance for all AF clients under rvu->rsrc_lock,
> and the driver-side helper added here would not be needed.
>
> > +	if (err)
> > +		netdev_warn(pfvf->netdev,
> > +			    "QoS: failed to reset topology for schq %u at level %u: %d\n",
> > +			    schq, lvl, err);
> > +
> > +	otx2_txschq_free_one(pfvf, lvl, schq);
> > +}
> > +
> >  static void otx2_qos_free_hw_node_schq(struct otx2_nic *pfvf,
> >  				       struct otx2_qos_node *parent)
> >  {
> >  	struct otx2_qos_node *node;
> >
> >  	list_for_each_entry_reverse(node, &parent->child_schq_list, list)
> > -		otx2_txschq_free_one(pfvf, node->level, node->schq);
> > +		otx2_qos_free_hw_schq(pfvf, node->level, node->schq);
> >  }
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903020533.3068041-1-rkannoth%40marvell.com

Re: [PATCH v4 net] octeontx2-pf: reset HTB scheduler topology before freeing queues

[Simon Horman]

On Wed, Sep 09, 2026 at 08:47:10AM +0530, Ratheesh Kannoth wrote:
> On 2026-09-08 at 22:37:40, netdev-bot+sashiko@kernel.org (netdev-bot+sashiko@kernel.org) wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential
> > issue(s) to consider:
> >
> > Critical: 0 · High: 1 · Medium: 1 · Low: 0
> >
> > - [High] Incomplete fix: the stale-TOPOLOGY bug class the commit claims
> >   to close stays reachable through the AF's bulk release path.
> AF's built release path is FLR/PCI reset path. We will fix it as seperate fix.
> 
> > - [Medium] Set/reset asymmetry at the aggregation level: HTB offload
> >   programs NIX_AF_TL1X_TOPOLOGY (PRIO_ANCHOR/RR_PRIO) on the shared per-…
> We can't clear TL1 blindly as it is a shared resource between PF and VF.

Thanks for looking into those concerns, and moreover addressing Sashiko's
feedback on earlier revisions of this patch.

This version looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

...

Re: [PATCH v4 net] octeontx2-pf: reset HTB scheduler topology before freeing queues

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 3 Sep 2026 07:35:33 +0530 you wrote:
> HTB offload programs NIX_AF_TLxX_TOPOLOGY on QoS-allocated scheduler
> queues via otx2_qos_txschq_set_parent_topology(), but teardown freed
> those queues without clearing TOPOLOGY.  The AF only restores PARENT and
> SCHEDULE on free, so PRIO_ANCHOR/RR_PRIO settings can survive in the
> shared scheduler pool and affect later allocations.
> 
> Add otx2_qos_reset_schq_topology() and otx2_qos_free_hw_schq() to zero
> TL4 through TL2 TOPOLOGY before each schq is returned to the AF during
> hierarchy teardown and cfg rollback.  Skip the aggregation level (TL1):
> it is a per-tx-link queue shared by the PF, default Tx hierarchy and VFs,
> and is not freed back to the AF by nix_txschq_free_one().
> 
> [...]

Here is the summary with links:
  - [v4,net] octeontx2-pf: reset HTB scheduler topology before freeing queues
    https://git.kernel.org/netdev/net/c/ef39fca85085

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html