[PATCH net-next v3 0/2] net: stmmac: Introduce hw queue priority offload

[PATCH net-next v3 0/2] net: stmmac: Introduce hw queue priority offload

[Lorenzo Bianconi]

Implement the offload of the tc mqprio hw queue priority in the stmmac
driver. This feature requires the mqprio to be configured in channel
mode.
Do not overwrite real_num_tx_queues configured by the qdisc in
stmmac_hw_setup().

---
Changes in v3:
- Fix cbs overwriting removing mqprio qdisc configured in dcb mode.
- Return an error in stmmac_reinit_queues() if configured number of tx
  queues is lower than xmit_qdisc.num_tx_queues and mqprio offloading is
  enabled.
- Link to v2: https://lore.kernel.org/r/20260924-stmmac-mqprio-prio-offload-v2-0-fdd8b69efccf@oss.qualcomm.com

Changes in v2:
- Forbid cbs queue offload if mqprio is already enabled.
- SP requires 1:1 TC to TXQ map.
- Fix the real_num_tx_queue overwrite during device open.
- Require the qdisc to be configured in channel mode to enable hw queue
  priority offload in order to not introduce any regression in the
  previous driver behaviour.
- Link to v1: https://lore.kernel.org/r/20260918-stmmac-mqprio-prio-offload-v1-1-5328157fcb58@oss.qualcomm.com

---
Lorenzo Bianconi (2):
      net: stmmac: add tc mqprio hw queue priority offload
      net: stmmac: align real_num_tx_queues to mqprio configuration

 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   9 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c  |   2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  37 ++++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 144 ++++++++++++++++++++--
 4 files changed, 169 insertions(+), 23 deletions(-)
---
base-commit: 014d795c73837ea2339a4ea8e8f82c6e959b845d
change-id: 20260918-stmmac-mqprio-prio-offload-3d82d87f7bc8

Best regards,
-- 
Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

[PATCH net-next v3 1/2] net: stmmac: add tc mqprio hw queue priority offload

[Lorenzo Bianconi]

Implement the offload of the tc mqprio hw queue priority in the stmmac
driver. When the mqprio qdisc is configured in channel mode, the MTL TX
scheduler is switched to strict priority, and the PSTQX/PSTC priority
bitmask of each TX queue is programmed from the set of frame priorities
mapped to the owning traffic class (qopt->prio_tc_map).

The channel mode offload requires the DCB hw feature and a 1:1 TC to TX
queue mapping, with a single queue per TC. Configurations with AVB queues
are rejected, since forcing strict priority conflicts with the CBS
algorithm.

In the default DCB mode, only the netdev TC map and the FPE preemption
class mapping are offloaded, leaving the MTL scheduler and the per-queue
priorities to the device-tree configuration.

Reviewed-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   7 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c  |   2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  25 ++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 138 ++++++++++++++++++++--
 4 files changed, 150 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 4fc96b317d79..f1bdb65d0fe6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -299,6 +299,13 @@ struct stmmac_priv {
 	/* Protect est parameters */
 	struct mutex est_lock;
 	struct stmmac_est *est;
+
+	struct {
+		u32 prio[MTL_MAX_TX_QUEUES];
+		bool prio_offload;
+		u8 algo;
+	} xmit_qdisc;
+
 	struct dma_features dma_cap;
 	struct stmmac_counters mmc;
 	int hw_cap_support;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
index c889204a7aa5..b6b5ef7c8fc4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
@@ -230,7 +230,7 @@ int dwmac5_fpe_map_preemption_class(struct net_device *ndev,
 		if (count == 1)
 			continue;
 
-		if (priv->plat->tx_sched_algorithm == MTL_TX_ALGORITHM_SP) {
+		if (priv->xmit_qdisc.algo == MTL_TX_ALGORITHM_SP) {
 			NL_SET_ERR_MSG_MOD(extack, ALG_ERR_MSG);
 			return -EINVAL;
 		}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3ad9252bf6ae..27e4e86e3c8b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3533,17 +3533,11 @@ static void stmmac_mac_config_rx_queues_prio(struct stmmac_priv *priv)
  */
 static void stmmac_mac_config_tx_queues_prio(struct stmmac_priv *priv)
 {
-	u8 tx_queues_count = priv->plat->tx_queues_to_use;
-	u8 queue;
-	u32 prio;
-
-	for (queue = 0; queue < tx_queues_count; queue++) {
-		if (!priv->plat->tx_queues_cfg[queue].use_prio)
-			continue;
+	int i;
 
-		prio = priv->plat->tx_queues_cfg[queue].prio;
-		stmmac_tx_queue_prio(priv, priv->hw, prio, queue);
-	}
+	for (i = 0; i < priv->plat->tx_queues_to_use; i++)
+		stmmac_tx_queue_prio(priv, priv->hw,
+				     priv->xmit_qdisc.prio[i], i);
 }
 
 /**
@@ -3604,7 +3598,7 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
 	/* Configure MTL TX algorithms */
 	if (tx_queues_count > 1)
 		stmmac_prog_mtl_tx_algorithms(priv, priv->hw,
-				priv->plat->tx_sched_algorithm);
+					      priv->xmit_qdisc.algo);
 
 	/* Configure CBS in AVB TX queues */
 	if (tx_queues_count > 1)
@@ -7924,6 +7918,15 @@ static int __stmmac_dvr_probe(struct device *device,
 	priv->wol_irq = res->wol_irq;
 	priv->sfty_irq = res->sfty_irq;
 
+	/* Default xmit qdisc configuration */
+	for (i = 0; i < MTL_MAX_TX_QUEUES; i++) {
+		if (!priv->plat->tx_queues_cfg[i].use_prio)
+			continue;
+
+		priv->xmit_qdisc.prio[i] = priv->plat->tx_queues_cfg[i].prio;
+	}
+	priv->xmit_qdisc.algo = priv->plat->tx_sched_algorithm;
+
 	if (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN) {
 		ret = stmmac_msi_init(priv, res);
 		if (ret)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 42a00446e9b4..07cf4582ed76 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -346,6 +346,9 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
 	if (!priv->dma_cap.av)
 		return -EOPNOTSUPP;
 
+	if (qopt->enable && priv->xmit_qdisc.prio_offload)
+		return -EOPNOTSUPP;
+
 	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
 
 	if (qopt->enable) {
@@ -1266,12 +1269,109 @@ static int stmmac_reset_tc_mqprio(struct net_device *ndev,
 {
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
+	if (priv->xmit_qdisc.prio_offload) {
+		int i;
+
+		for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
+			u32 prio;
+
+			if (priv->plat->tx_queues_cfg[i].use_prio)
+				prio = priv->plat->tx_queues_cfg[i].prio;
+			else
+				prio = 0;
+
+			stmmac_tx_queue_prio(priv, priv->hw, prio, i);
+			priv->xmit_qdisc.prio[i] = prio;
+		}
+
+		stmmac_prog_mtl_tx_algorithms(priv, priv->hw,
+					      priv->plat->tx_sched_algorithm);
+		priv->xmit_qdisc.algo = priv->plat->tx_sched_algorithm;
+		priv->xmit_qdisc.prio_offload = false;
+	}
+
 	netdev_reset_tc(ndev);
 	netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
 
 	return stmmac_fpe_map_preemption_class(priv, ndev, extack, 0);
 }
 
+static void tc_mqprio_config_queue_prio(struct stmmac_priv *priv,
+					struct tc_mqprio_qopt *qopt)
+{
+	int i;
+
+	for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
+		u32 prio = 0;
+		int j;
+
+		for (j = 0; j < qopt->num_tc; j++) {
+			int p;
+
+			if (qopt->offset[j] != i)
+				continue;
+
+			/* The PSTQX/PSTC priority map is 8 bits wide, so only
+			 * priorities 0-7 can be represented in hardware.
+			 * Priorities 8-15 are handled in software by the
+			 * kernel through the netdev prio_tc_map.
+			 */
+			for (p = 0; p < 8; p++) {
+				if (qopt->prio_tc_map[p] == j)
+					prio |= BIT(p);
+			}
+			break;
+		}
+
+		stmmac_tx_queue_prio(priv, priv->hw, prio, i);
+		priv->xmit_qdisc.prio[i] = prio;
+	}
+
+	stmmac_prog_mtl_tx_algorithms(priv, priv->hw, MTL_TX_ALGORITHM_SP);
+	priv->xmit_qdisc.algo = MTL_TX_ALGORITHM_SP;
+	priv->xmit_qdisc.prio_offload = true;
+}
+
+static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
+					struct tc_mqprio_qopt_offload *mqprio)
+{
+	struct plat_stmmacenet_data *pdata = priv->plat;
+	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
+	int i;
+
+	if (!priv->dma_cap.dcben) {
+		NL_SET_ERR_MSG_MOD(mqprio->extack,
+				   "hw DCB is required to offload mqprio");
+		return -EOPNOTSUPP;
+	}
+
+	/* Forcing strict priority conflicts with the CBS algorithm
+	 * of AVB queues, so reject the offload when any queue is
+	 * configured as AVB.
+	 */
+	for (i = 0; i < pdata->tx_queues_to_use; i++) {
+		if (pdata->tx_queues_cfg[i].mode_to_use == MTL_QUEUE_AVB) {
+			NL_SET_ERR_MSG_MOD(mqprio->extack,
+					   "SP conflicts with AVB queues");
+			return -EOPNOTSUPP;
+		}
+	}
+
+	for (i = 0; i < qopt->num_tc; i++) {
+		/* The offload switches the MTL scheduler to strict
+		 * priority, which only supports a 1:1 TC to TX queue
+		 * mapping.
+		 */
+		if (qopt->count[i] > 1 || qopt->offset[i] != i) {
+			NL_SET_ERR_MSG_MOD(mqprio->extack,
+					   "SP requires 1:1 TXQ map");
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return 0;
+}
+
 static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 				    struct tc_mqprio_qopt_offload *mqprio)
 {
@@ -1282,23 +1382,22 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
 	struct net_device *ndev = priv->dev;
 	u8 ndev_prio_tc_map[TC_BITMASK + 1];
-	int i, err, ndev_ntc;
+	int i, err, ndev_ntc, mode;
 
 	if (!qopt->num_tc)
 		return stmmac_reset_tc_mqprio(ndev, extack);
 
-	if (qopt->num_tc > ARRAY_SIZE(tc_to_txq))
+	if (qopt->num_tc > priv->plat->tx_queues_to_use)
 		return -EINVAL;
 
-	/* save current tc values for reset */
-	ndev_ntc = netdev_get_num_tc(ndev);
-	for (i = 0; i < ARRAY_SIZE(ndev->tc_to_txq); i++)
-		ndev_tc_to_txq[i].combined =
-			READ_ONCE(ndev->tc_to_txq[i].combined);
-	for (i = 0; i < ARRAY_SIZE(ndev_prio_tc_map); i++)
-		ndev_prio_tc_map[i] = READ_ONCE(ndev->prio_tc_map[i]);
-
 	for (i = 0; i < qopt->num_tc; i++) {
+		if (qopt->offset[i] + qopt->count[i] >
+		    priv->plat->tx_queues_to_use) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Queues exceed hw TX queues");
+			return -EINVAL;
+		}
+
 		tc_to_txq[i] = (struct netdev_tc_txq) {
 			.count = qopt->count[i],
 			.offset = qopt->offset[i],
@@ -1306,6 +1405,22 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 		num_tx_queues += qopt->count[i];
 	}
 
+	mode = mqprio->flags & TC_MQPRIO_F_MODE ? mqprio->mode
+						: TC_MQPRIO_MODE_DCB;
+	if (mode == TC_MQPRIO_MODE_CHANNEL) {
+		err = tc_mqprio_validate_chan_mode(priv, mqprio);
+		if (err)
+			return err;
+	}
+
+	/* save current tc values for reset */
+	ndev_ntc = netdev_get_num_tc(ndev);
+	for (i = 0; i < ARRAY_SIZE(ndev->tc_to_txq); i++)
+		ndev_tc_to_txq[i].combined =
+			READ_ONCE(ndev->tc_to_txq[i].combined);
+	for (i = 0; i < ARRAY_SIZE(ndev_prio_tc_map); i++)
+		ndev_prio_tc_map[i] = READ_ONCE(ndev->prio_tc_map[i]);
+
 	err = stmmac_set_ndev_tcs(ndev, qopt->num_tc, tc_to_txq);
 	if (err)
 		goto error_reset_tc;
@@ -1320,6 +1435,9 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 	if (err)
 		goto error_reset_num_tx_queues;
 
+	if (mode == TC_MQPRIO_MODE_CHANNEL)
+		tc_mqprio_config_queue_prio(priv, qopt);
+
 	return 0;
 
 error_reset_num_tx_queues:

-- 
2.55.0

Re: [PATCH net-next v3 1/2] net: stmmac: add tc mqprio hw queue priority offload

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 (3) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Stale shadow priorities in xmit_qdisc.prio[] get written back
  to hardware.
- [Medium] net: stmmac: state corruption when switching mqprio mode from
  CHANNEL to DCB

Pre-existing issues:
- [Medium] Channel-mode shaper and rate requests are accepted but never
  applied.
- [Medium] DCB mappings with queue gaps quietly disable TC
  classification.
- [Medium] The mqprio path accesses MAC/MTL registers without a runtime-
  PM reference.

--

net: stmmac: add tc mqprio hw queue priority offload

This patch adds mqprio channel mode offload to stmmac. In that mode the
MTL TX scheduler is switched to strict priority and the per-queue
PSTQx/PSTC priority bitmasks are programmed from prio_tc_map. The patch
also adds a shadow copy of the TX algorithm and the per-queue priorities
in priv->xmit_qdisc, which is programmed into the hardware on open.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 42a00446e9b4..07cf4582ed76 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

[ ... ]

> @@ -1266,12 +1269,109 @@ static int stmmac_reset_tc_mqprio(struct net_device *ndev,
>  {
>  	struct stmmac_priv *priv = netdev_priv(ndev);
>  
> +	if (priv->xmit_qdisc.prio_offload) {
> +		int i;
> +
> +		for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
> +			u32 prio;
> +
> +			if (priv->plat->tx_queues_cfg[i].use_prio)
> +				prio = priv->plat->tx_queues_cfg[i].prio;
> +			else
> +				prio = 0;
> +
> +			stmmac_tx_queue_prio(priv, priv->hw, prio, i);
> +			priv->xmit_qdisc.prio[i] = prio;
> +		}

[Severity: Medium]
Can stale entries in priv->xmit_qdisc.prio[] get written back to the
hardware?

This loop, and the one in tc_mqprio_config_queue_prio(), only update
prio[i] for i below the tx_queues_to_use value at that moment. With this
patch, stmmac_mac_config_tx_queues_prio() programs PSTQx/PSTC from the
shadow array on every open, where it used to read tx_queues_cfg[]:

stmmac_main.c:stmmac_mac_config_tx_queues_prio() {
	for (i = 0; i < priv->plat->tx_queues_to_use; i++)
		stmmac_tx_queue_prio(priv, priv->hw,
				     priv->xmit_qdisc.prio[i], i);
}

With 8 TX queues from DT, each with snps,priority, this sequence looks
possible:

1) tc qdisc add ... mqprio num_tc 4 queues 1@0 1@1 1@2 1@3 hw 1 mode channel
   tc_mqprio_config_queue_prio() sets prio[4..7] = 0
2) ethtool -L <dev> tx 4
   tx_queues_to_use becomes 4
3) tc qdisc del ...
   stmmac_reset_tc_mqprio() restores only prio[0..3]
4) ethtool -L <dev> tx 8
   stmmac_open()->stmmac_hw_setup()->stmmac_mtl_configuration()->
   stmmac_mac_config_tx_queues_prio() writes 0 for queues 4..7

After that, queues 4..7 keep a zero priority map in place of their DT
values until the driver is re-probed.

The follow-up commit "net: stmmac: align real_num_tx_queues to mqprio
configuration" makes stmmac_reinit_queues() reject tx_cnt below
xmit_qdisc.num_tx_queues while mqprio is enabled. That covers shrinking
below the TC queues. Shrinking to exactly num_tx_queues is still allowed,
though, so step 2 still works at the end of the series.

Should the restore loop here cover all MTL_MAX_TX_QUEUES entries?

> +
> +		stmmac_prog_mtl_tx_algorithms(priv, priv->hw,
> +					      priv->plat->tx_sched_algorithm);
> +		priv->xmit_qdisc.algo = priv->plat->tx_sched_algorithm;
> +		priv->xmit_qdisc.prio_offload = false;
> +	}
> +
>  	netdev_reset_tc(ndev);
>  	netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
>  
>  	return stmmac_fpe_map_preemption_class(priv, ndev, extack, 0);
>  }
>  
> +static void tc_mqprio_config_queue_prio(struct stmmac_priv *priv,
> +					struct tc_mqprio_qopt *qopt)
> +{

[ ... ]

> +		stmmac_tx_queue_prio(priv, priv->hw, prio, i);
> +		priv->xmit_qdisc.prio[i] = prio;
> +	}
> +
> +	stmmac_prog_mtl_tx_algorithms(priv, priv->hw, MTL_TX_ALGORITHM_SP);
> +	priv->xmit_qdisc.algo = MTL_TX_ALGORITHM_SP;
> +	priv->xmit_qdisc.prio_offload = true;
> +}

[Severity: Medium]
This is a pre-existing issue, but is it safe to access the MTL registers
here without a runtime PM reference?

stmmac_setup_tc() passes TC_SETUP_QDISC_MQPRIO to
stmmac_tc_setup_mqprio() without calling pm_runtime_resume_and_get().
While the interface is down, stmmac_runtime_suspend() can gate
stmmac_clk and pclk through stmmac_bus_clks_config(priv, false).

A tc qdisc add or del on a down interface would then reach
stmmac_tx_queue_prio() and stmmac_prog_mtl_tx_algorithms() here, and in
the prio_offload block of stmmac_reset_tc_mqprio(), with those clocks
off.

This path already did unguarded MMIO before this patch, through
dwmac5_fpe_map_preemption_class():

	val = readl(priv->ioaddr + GMAC5_MTL_FPE_CTRL_STS);

Other register-touching ndos in this driver, such as set_mac_address and
the VLAN add/kill ops, call pm_runtime_resume_and_get() first.

The xmit_qdisc shadow is reapplied on open. Could the register writes be
skipped when !netif_running(), or could a PM reference be taken around
them?

> +
> +static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
> +					struct tc_mqprio_qopt_offload *mqprio)
> +{
> +	struct plat_stmmacenet_data *pdata = priv->plat;
> +	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
> +	int i;
> +
> +	if (!priv->dma_cap.dcben) {
> +		NL_SET_ERR_MSG_MOD(mqprio->extack,
> +				   "hw DCB is required to offload mqprio");
> +		return -EOPNOTSUPP;
> +	}

[ ... ]

> +	for (i = 0; i < qopt->num_tc; i++) {
> +		/* The offload switches the MTL scheduler to strict
> +		 * priority, which only supports a 1:1 TC to TX queue
> +		 * mapping.
> +		 */
> +		if (qopt->count[i] > 1 || qopt->offset[i] != i) {
> +			NL_SET_ERR_MSG_MOD(mqprio->extack,
> +					   "SP requires 1:1 TXQ map");
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
> +	return 0;
> +}

[Severity: Medium]
This is a pre-existing issue, but should channel mode reject shaper and
rate requests?

In channel mode, mqprio_enable_offload() in net/sched/sch_mqprio.c
copies the flags, shaper, min_rate[] and max_rate[] into struct
tc_mqprio_qopt_offload. If the driver returns 0, it sets hw_offload and
later dumps TCA_MQPRIO_SHAPER.

tc_setup_dwmac510_mqprio() only checks TC_MQPRIO_F_MODE, and this
validator does not check TC_MQPRIO_F_SHAPER, TC_MQPRIO_F_MIN_RATE or
TC_MQPRIO_F_MAX_RATE.

So a request like "mqprio ... hw 1 mode channel shaper bw_rlimit
max_rate ..." with a 1:1 mapping succeeds. Only SP and PSTQ get
programmed, and the qdisc reports rate limiting the hardware isn't
doing.

The baseline handler ignored mode and flags entirely. Would this new
validator be a good place to return -EOPNOTSUPP for those flags?

[ ... ]

> @@ -1306,6 +1405,22 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
>  		num_tx_queues += qopt->count[i];
>  	}

[Severity: Medium]
This is a pre-existing issue, but is num_tx_queues too small when the
queue ranges have gaps?

mqprio_validate_queue_counts() does not require the ranges to be
contiguous from queue 0. Here num_tx_queues is the sum of the counts,
not the highest offset + count.

On a 4 queue device, a DCB request with a single TC at 1@2 passes both
that check and the new offset + count check in this function. It then
calls netif_set_real_num_tx_queues(ndev, 1), and netif_setup_tc() does:

	/* If TC0 is invalidated disable TC mapping */
	if (res.offset + res.count > txq) {
		netdev_warn(dev, "Number of in use tx queues changed invalidating tc mappings. Priority traffic classification disabled!\n");
		WRITE_ONCE(dev->num_tc, 0);
		return;
	}

The driver still returns 0, so mqprio reports a working offload while
the device runs on one queue with no TC mapping. With 1@0 1@2, the
priorities for TC1 are quietly remapped to TC0 instead.

The baseline code computed num_tx_queues the same way. Should this use
the highest offset + count, or reject non-contiguous ranges?

>  
> +	mode = mqprio->flags & TC_MQPRIO_F_MODE ? mqprio->mode
> +						: TC_MQPRIO_MODE_DCB;
> +	if (mode == TC_MQPRIO_MODE_CHANNEL) {
> +		err = tc_mqprio_validate_chan_mode(priv, mqprio);
> +		if (err)
> +			return err;
> +	}

[Severity: Medium]
Does a DCB mode request clean up after a previous channel mode
offload?

The xmit_qdisc state is only restored in stmmac_reset_tc_mqprio(),
which runs only when num_tc is 0. When tc_setup_dwmac510_mqprio() is
called with num_tc > 0 and mode resolves to TC_MQPRIO_MODE_DCB, nothing
touches priv->xmit_qdisc. A prior channel mode configuration therefore
leaves three things behind: xmit_qdisc.algo stays at
MTL_TX_ALGORITHM_SP, xmit_qdisc.prio_offload stays set, and the channel
mode PSTQx/PSTC maps stay programmed in the hardware.

One way to get here is replacing a channel mode mqprio with a DCB one
under a new handle. qdisc_graft() runs mqprio_init() for the new qdisc,
and so the driver offload, before mqprio_destroy() of the old qdisc
calls back with num_tc 0.

With this patch, dwmac5_fpe_map_preemption_class() tests
xmit_qdisc.algo instead of plat->tx_sched_algorithm. So on that path, a
valid DCB request with a TC spanning more than one queue is rejected
with ALG_ERR_MSG, even when DT selected a non-SP algorithm.

Any DCB request that arrives without an intervening reset has the same
problem. The MTL scheduler stays in SP with the old priority maps, and
tc_setup_cbs() keeps refusing CBS because prio_offload is still true.
That contradicts the commit message, which says DCB mode leaves the
scheduler and per-queue priorities to the device-tree configuration.

Could the prio_offload restore block in stmmac_reset_tc_mqprio() be
moved into a helper and also called on the DCB path, before
stmmac_fpe_map_preemption_class() runs? If so, the error unwind in
tc_setup_dwmac510_mqprio() would need to put the channel mode state
back when the FPE mapping fails. How would you prefer to handle that?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928-stmmac-mqprio-prio-offload-v3-0-abbe181f5024%40oss.qualcomm.com

[PATCH net-next v3 2/2] net: stmmac: align real_num_tx_queues to mqprio configuration

[Lorenzo Bianconi]

stmmac_hw_setup() unconditionally sets the number of real TX queues to
plat->tx_queues_to_use on every device open. When a tc-mqprio qdisc is
offloaded the driver reduces netif_set_real_num_tx_queues() to the number
of queues enabled by the offload, but the next device open reverts it to
plat->tx_queues_to_use while the netdev TC map is still the one programmed
by the qdisc, leaving the two inconsistent.

Track the number of TX queues enabled by the current qdisc configuration
in the per-qdisc state (priv->xmit_qdisc.num_tx_queues) and use it in
stmmac_hw_setup(). The field defaults to plat->tx_queues_to_use at probe
time and when the mqprio qdisc is destroyed, meaning no offload is active
and the number of real TX queues must match the number of queues the
driver allocated, and it is updated to the offloaded queue count on a
successful mqprio setup.

Refuse to lower the number of TX queues below the count required by the
active mqprio offload in stmmac_reinit_queues(), otherwise
netif_set_real_num_tx_queues() would leave real_num_tx_queues
inconsistent with the netdev TC map.

Fixes: 195e4f409a40 ("net: stmmac: support fp parameter of tc-mqprio")
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |  2 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++++++++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   |  6 ++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index f1bdb65d0fe6..40eab899027b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -301,8 +301,10 @@ struct stmmac_priv {
 	struct stmmac_est *est;
 
 	struct {
+		bool enabled;
 		u32 prio[MTL_MAX_TX_QUEUES];
 		bool prio_offload;
+		u32 num_tx_queues;
 		u8 algo;
 	} xmit_qdisc;
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 27e4e86e3c8b..1c25d1c129d4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3772,7 +3772,7 @@ static int stmmac_hw_setup(struct net_device *dev)
 
 	/* Configure real RX and TX queues */
 	netif_set_real_num_rx_queues(dev, priv->plat->rx_queues_to_use);
-	netif_set_real_num_tx_queues(dev, priv->plat->tx_queues_to_use);
+	netif_set_real_num_tx_queues(dev, priv->xmit_qdisc.num_tx_queues);
 
 	/* Start the ball rolling... */
 	stmmac_start_all_dma(priv);
@@ -7676,6 +7676,10 @@ int stmmac_reinit_queues(struct net_device *dev, u8 rx_cnt, u8 tx_cnt)
 	struct stmmac_priv *priv = netdev_priv(dev);
 	int ret = 0, i;
 
+	if (priv->xmit_qdisc.enabled &&
+	    tx_cnt < priv->xmit_qdisc.num_tx_queues)
+		return -EINVAL;
+
 	if (netif_running(dev))
 		stmmac_release(dev);
 
@@ -7683,6 +7687,9 @@ int stmmac_reinit_queues(struct net_device *dev, u8 rx_cnt, u8 tx_cnt)
 
 	priv->plat->rx_queues_to_use = rx_cnt;
 	priv->plat->tx_queues_to_use = tx_cnt;
+	if (!priv->xmit_qdisc.enabled)
+		priv->xmit_qdisc.num_tx_queues = tx_cnt;
+
 	if (!netif_is_rxfh_configured(dev))
 		for (i = 0; i < ARRAY_SIZE(priv->rss.table); i++)
 			priv->rss.table[i] = ethtool_rxfh_indir_default(i,
@@ -8003,6 +8010,9 @@ static int __stmmac_dvr_probe(struct device *device,
 	ndev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 			     NETDEV_XDP_ACT_XSK_ZEROCOPY;
 
+	/* Default qdisc num_tx_queues */
+	priv->xmit_qdisc.num_tx_queues = priv->plat->tx_queues_to_use;
+
 	ret = stmmac_tc_init(priv, priv);
 	if (!ret) {
 		ndev->hw_features |= NETIF_F_HW_TC;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 07cf4582ed76..8bdc411e902f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -1290,6 +1290,9 @@ static int stmmac_reset_tc_mqprio(struct net_device *ndev,
 		priv->xmit_qdisc.prio_offload = false;
 	}
 
+	priv->xmit_qdisc.num_tx_queues = priv->plat->tx_queues_to_use;
+	priv->xmit_qdisc.enabled = false;
+
 	netdev_reset_tc(ndev);
 	netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
 
@@ -1438,6 +1441,9 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 	if (mode == TC_MQPRIO_MODE_CHANNEL)
 		tc_mqprio_config_queue_prio(priv, qopt);
 
+	priv->xmit_qdisc.num_tx_queues = num_tx_queues;
+	priv->xmit_qdisc.enabled = true;
+
 	return 0;
 
 error_reset_num_tx_queues:

-- 
2.55.0

Re: [PATCH net-next v3 2/2] net: stmmac: align real_num_tx_queues to mqprio configuration

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The value this patch stores as "the count required by the
  active mqprio offload" (`priv->xmit_qdisc.num_tx_queues`) is…
- [Low] This patch carries `Fixes: 195e4f409a40` ("net: stmmac: support
  fp parameter of tc-mqprio", Sep 2024), but it depends on the commit…
- [Low] The hardware TX queue priority map (PSTQx on dwmac4/5, PSTC on
  XGMAC) becomes stale or inconsistent after `ethtool -L` while a…

--

net: stmmac: align real_num_tx_queues to mqprio configuration

This patch stores the TX queue count used by the active mqprio offload
in priv->xmit_qdisc.num_tx_queues. stmmac_hw_setup() now uses that value
instead of plat->tx_queues_to_use for real_num_tx_queues. The patch also
makes stmmac_reinit_queues() reject TX counts below the count the
offload needs.

> Fixes: 195e4f409a40 ("net: stmmac: support fp parameter of tc-mqprio")

[Severity: Low]
Can this patch be applied to a tree that only has 195e4f409a40?

It depends on the earlier commit in this series, "net: stmmac: add tc
mqprio hw queue priority offload", which is a new feature. That commit
added the xmit_qdisc struct in struct stmmac_priv, which this patch
extends.

It also added the hunk context used here. That context is the
"if (priv->xmit_qdisc.prio_offload) {" restore block in
stmmac_reset_tc_mqprio() and the tc_mqprio_config_queue_prio() call in
tc_setup_dwmac510_mqprio().

The bug being fixed already exists at 195e4f409a40. In that tree,
stmmac_hw_setup() calls
netif_set_real_num_tx_queues(dev, priv->plat->tx_queues_to_use), and
tc_setup_dwmac510_mqprio() lowers real_num_tx_queues to the offloaded
count.

Stable tooling that uses the Fixes: tag will not be able to apply this
patch unless it also takes the feature commit. Could the fix be reworked
to apply to net on its own? If not, could the commit message mention
the dependency?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 27e4e86e3c8b..1c25d1c129d4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -7676,6 +7676,10 @@ int stmmac_reinit_queues(struct net_device *dev, u8 rx_cnt, u8 tx_cnt)
>  	struct stmmac_priv *priv = netdev_priv(dev);
>  	int ret = 0, i;
>  
> +	if (priv->xmit_qdisc.enabled &&
> +	    tx_cnt < priv->xmit_qdisc.num_tx_queues)
> +		return -EINVAL;
> +

[Severity: Low]
While a channel-mode mqprio offload is active, this check still lets
tx_cnt grow, or shrink as far as num_tx_queues. Does xmit_qdisc.prio[]
stay in sync with the new queue count?

At install time, tc_mqprio_config_queue_prio() fills prio[i] only for
i < tx_queues_to_use. After ethtool -L raises the TX count, the path
stmmac_open()->stmmac_hw_setup()->stmmac_mtl_configuration()->
stmmac_mac_config_tx_queues_prio() programs every queue from prio[]:

	for (i = 0; i < priv->plat->tx_queues_to_use; i++)
		stmmac_tx_queue_prio(priv, priv->hw,
				     priv->xmit_qdisc.prio[i], i);

For the new queues, this writes the probe-time DT priority
(plat->tx_queues_cfg[i].prio). Channel mode would give those queues 0,
since they are outside every TC.

dwmac4_tx_queue_priority() clears only the target queue's field:

	value &= ~GMAC_TXQCTRL_PSTQX_MASK(queue);
	value |= (prio << GMAC_TXQCTRL_PSTQX_SHIFT(queue)) &
					GMAC_TXQCTRL_PSTQX_MASK(queue);

As a result, one 802.1p priority can be mapped to both a mqprio TC
queue and a newly added queue.

The reverse sequence looks possible too: shrink tx_cnt while mqprio is
active, delete the qdisc, then grow again. stmmac_reset_tc_mqprio()
restores defaults only up to the reduced count:

	for (i = 0; i < priv->plat->tx_queues_to_use; i++) {

That leaves the mqprio-era 0 values in prio[] for the upper queues. The
later increase writes those values while prio_offload is false, so the
DT-configured priorities for those queues are lost.

The prio[] state was added by the earlier "net: stmmac: add tc mqprio
hw queue priority offload" commit in this series. Before that commit,
stmmac_mac_config_tx_queues_prio() read plat->tx_queues_cfg directly.

The runtime impact may be small. These fields mostly select which
queues a received PFC frame pauses, and stmmac does not enable PFC
reception.

Should stmmac_reinit_queues() recompute prio[] for the new count? The
alternative would be to refuse any tx_cnt change while prio_offload is
set.

[ ... ]

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 07cf4582ed76..8bdc411e902f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

[ ... ]

> @@ -1438,6 +1441,9 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
>  	if (mode == TC_MQPRIO_MODE_CHANNEL)
>  		tc_mqprio_config_queue_prio(priv, qopt);
>  
> +	priv->xmit_qdisc.num_tx_queues = num_tx_queues;
> +	priv->xmit_qdisc.enabled = true;
> +

[Severity: Medium]
Is num_tx_queues the right value to store here? Earlier in
tc_setup_dwmac510_mqprio(), it is computed as the sum of the counts,
but the validation checks offset + count:

	if (qopt->offset[i] + qopt->count[i] >
	    priv->plat->tx_queues_to_use) {
	...
	num_tx_queues += qopt->count[i];

mqprio_validate_queue_counts() rejects ranges that are out of bounds or
overlap. It still allows gaps and a non-zero first offset.
tc_mqprio_validate_chan_mode() requires offset[i] == i only in channel
mode, so DCB-mode layouts like "queues 1@0 1@2" or "queues 2@1" are
accepted.

For those layouts, netif_set_real_num_tx_queues(ndev, num_tx_queues)
reaches netif_setup_tc():

	/* If TC0 is invalidated disable TC mapping */
	if (res.offset + res.count > txq) {
		...
		WRITE_ONCE(dev->num_tc, 0);
		return;
	}
	...
		if (res.offset + res.count > txq) {
			...
			netdev_set_prio_tc_map(dev, i, 0);

With "1@0 1@2" the sum is 2, so every priority mapped to TC1 moves to
TC0. With "2@1", TC0 needs 3 queues but only 2 are real, so num_tc is
set to 0.

In both cases the function still returns success, with enabled = true
and num_tx_queues = 2.

This patch now keeps that value. stmmac_hw_setup() reapplies it on
every open and resume, and the new check in stmmac_reinit_queues()
compares tx_cnt against it.

That check allows sum <= tx_cnt < max(offset + count), which leaves
tx_cnt below queues that tc_to_txq[] still references. For "2@1", it
rejects tx_cnt values below 2 even though num_tc is already 0.

The sum-of-counts calculation predates this patch. Here it becomes the
"count required by the active mqprio offload" that the commit message
depends on.

Would storing max(offset[i] + count[i]) make the saved value, the
stmmac_reinit_queues() check and the stmmac_hw_setup() restore agree
with the validation?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928-stmmac-mqprio-prio-offload-v3-0-abbe181f5024%40oss.qualcomm.com

Re: [PATCH net-next v3 0/2] net: stmmac: Introduce hw queue priority offload

[Maxime Chevallier]

Hi Lorenzo,

On 9/28/26 18:48, Lorenzo Bianconi wrote:
> Implement the offload of the tc mqprio hw queue priority in the stmmac
> driver. This feature requires the mqprio to be configured in channel
> mode.
> Do not overwrite real_num_tx_queues configured by the qdisc in
> stmmac_hw_setup().

I've tested that on AgileX5 with:

tc qdisc replace dev eth0 root handle 1: mqprio num_tc 2 \
map 0 0 0 0 1 1 1 1 queues 1@0 1@1 hw 1 mode channel

I do see the right prio -> queue mapping being used :)

Also checked that doing a down->up keeps the config. This is
great, thanks !

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

Re: [PATCH net-next v3 0/2] net: stmmac: Introduce hw queue priority offload

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

> Implement the offload of the tc mqprio hw queue priority in the stmmac
> driver. This feature requires the mqprio to be configured in channel
> mode.
> Do not overwrite real_num_tx_queues configured by the qdisc in
> stmmac_hw_setup().
> 
> ---
> Changes in v3:
> - Fix cbs overwriting removing mqprio qdisc configured in dcb mode.
> - Return an error in stmmac_reinit_queues() if configured number of tx
>   queues is lower than xmit_qdisc.num_tx_queues and mqprio offloading is
>   enabled.
> - Link to v2: https://lore.kernel.org/r/20260924-stmmac-mqprio-prio-offload-v2-0-fdd8b69efccf@oss.qualcomm.com
> 
> Changes in v2:
> - Forbid cbs queue offload if mqprio is already enabled.
> - SP requires 1:1 TC to TXQ map.
> - Fix the real_num_tx_queue overwrite during device open.
> - Require the qdisc to be configured in channel mode to enable hw queue
>   priority offload in order to not introduce any regression in the
>   previous driver behaviour.
> - Link to v1: https://lore.kernel.org/r/20260918-stmmac-mqprio-prio-offload-v1-1-5328157fcb58@oss.qualcomm.com

I will address sashiko's comment in v4.

Regards,
Lorenzo

> 
> ---
> Lorenzo Bianconi (2):
>       net: stmmac: add tc mqprio hw queue priority offload
>       net: stmmac: align real_num_tx_queues to mqprio configuration
> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   9 ++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c  |   2 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  37 ++++--
>  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 144 ++++++++++++++++++++--
>  4 files changed, 169 insertions(+), 23 deletions(-)
> ---
> base-commit: 014d795c73837ea2339a4ea8e8f82c6e959b845d
> change-id: 20260918-stmmac-mqprio-prio-offload-3d82d87f7bc8
> 
> Best regards,
> -- 
> Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]