Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/3] net: stmmac: Introduce hw queue priority offload
@ 2026-10-01 13:50 Lorenzo Bianconi
  2026-10-01 13:50 ` [PATCH net-next v4 1/3] net: stmmac: set real_num_tx_queues to max(offset + count) Lorenzo Bianconi
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-01 13:50 UTC (permalink / raw)
  To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Vladimir Oltean, Furong Xu
  Cc: Daniel Thompson, netdev, linux-stm32, linux-arm-kernel,
	Lorenzo Bianconi, Davide Caratti

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().

The misbehaviour has been reported by Shashank and the patch has been
tested on a Qualcomm RB3-Gen2 board.

---
Changes in v4:
- Move real_num_tx_queues fix before mqprio offload patch in order to
  allow easy backporting.
- Add patch 'net: stmmac: set real_num_tx_queues from max(offset +
  count)' to fix the real_num_tx_queues configuration.
- Link to v3: https://lore.kernel.org/r/20260928-stmmac-mqprio-prio-offload-v3-0-abbe181f5024@oss.qualcomm.com

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 (3):
      net: stmmac: set real_num_tx_queues to max(offset + count)
      net: stmmac: align real_num_tx_queues to mqprio configuration
      net: stmmac: add tc mqprio hw queue priority offload

 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   | 147 ++++++++++++++++++++--
 4 files changed, 172 insertions(+), 23 deletions(-)
---
base-commit: eb0c18404c8943356f4aa164e5db9067b79ea93c
change-id: 20260918-stmmac-mqprio-prio-offload-3d82d87f7bc8

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v4 1/3] net: stmmac: set real_num_tx_queues to max(offset + count)
  2026-10-01 13:50 [PATCH net-next v4 0/3] net: stmmac: Introduce hw queue priority offload Lorenzo Bianconi
@ 2026-10-01 13:50 ` Lorenzo Bianconi
  2026-10-05 14:13   ` netdev-bot+sashiko
  2026-10-01 13:50 ` [PATCH net-next v4 2/3] net: stmmac: align real_num_tx_queues to mqprio configuration Lorenzo Bianconi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-01 13:50 UTC (permalink / raw)
  To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Vladimir Oltean, Furong Xu
  Cc: Daniel Thompson, netdev, linux-stm32, linux-arm-kernel,
	Lorenzo Bianconi

tc_setup_dwmac510_mqprio() computes the number of real TX queues as the
sum of the per-TC queue counts. mqprio_validate_queue_counts() only
rejects out-of-bounds or overlapping ranges, so layouts with gaps or a
non-zero first offset are accepted, and in those cases the sum is smaller
than the highest queue referenced by the netdev TC map.

netif_set_real_num_tx_queues() then passes this too-small count to
netif_setup_tc(), which invalidates the mapping.

Compute num_tx_queues as the maximum of offset[i] + count[i] instead of
the sum of the counts, so real_num_tx_queues always covers the highest
queue referenced by the TC map. This is a no-op for the contiguous
channel-mode layouts where offset[i] == i and count[i] == 1.

The issue has been reported by sashiko and the patch has been tested on
Qualcomm RB3-Gen2 board.

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_tc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 42a00446e9b4..1aa80d51ab33 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -1303,7 +1303,8 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 			.count = qopt->count[i],
 			.offset = qopt->offset[i],
 		};
-		num_tx_queues += qopt->count[i];
+		num_tx_queues = max(num_tx_queues,
+				    qopt->offset[i] + qopt->count[i]);
 	}
 
 	err = stmmac_set_ndev_tcs(ndev, qopt->num_tc, tc_to_txq);

-- 
2.55.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v4 2/3] net: stmmac: align real_num_tx_queues to mqprio configuration
  2026-10-01 13:50 [PATCH net-next v4 0/3] net: stmmac: Introduce hw queue priority offload Lorenzo Bianconi
  2026-10-01 13:50 ` [PATCH net-next v4 1/3] net: stmmac: set real_num_tx_queues to max(offset + count) Lorenzo Bianconi
@ 2026-10-01 13:50 ` Lorenzo Bianconi
  2026-10-05 14:14   ` netdev-bot+sashiko
  2026-10-01 13:50 ` [PATCH net-next v4 3/3] net: stmmac: add tc mqprio hw queue priority offload Lorenzo Bianconi
  2026-10-06 10:11 ` [PATCH net-next v4 0/3] net: stmmac: Introduce " Lorenzo Bianconi
  3 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-01 13:50 UTC (permalink / raw)
  To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Vladimir Oltean, Furong Xu
  Cc: Daniel Thompson, netdev, linux-stm32, linux-arm-kernel,
	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.

The issue has been reported by sashiko and the patch has been tested on
a Qualcomm RB3-Gen2 board.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 4fc96b317d79..0052f2129297 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -299,6 +299,12 @@ struct stmmac_priv {
 	/* Protect est parameters */
 	struct mutex est_lock;
 	struct stmmac_est *est;
+
+	struct {
+		bool enabled;
+		u32 num_tx_queues;
+	} xmit_qdisc;
+
 	struct dma_features dma_cap;
 	struct stmmac_counters mmc;
 	int hw_cap_support;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d95482b7f6e5..1dc9218bd4e9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3778,7 +3778,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);
@@ -7692,6 +7692,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);
 
@@ -7699,6 +7703,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,
@@ -8010,6 +8017,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 1aa80d51ab33..d0c3feb45789 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -1266,6 +1266,9 @@ static int stmmac_reset_tc_mqprio(struct net_device *ndev,
 {
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
+	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);
 
@@ -1321,6 +1324,9 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 	if (err)
 		goto error_reset_num_tx_queues;
 
+	priv->xmit_qdisc.num_tx_queues = num_tx_queues;
+	priv->xmit_qdisc.enabled = true;
+
 	return 0;
 
 error_reset_num_tx_queues:

-- 
2.55.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v4 3/3] net: stmmac: add tc mqprio hw queue priority offload
  2026-10-01 13:50 [PATCH net-next v4 0/3] net: stmmac: Introduce hw queue priority offload Lorenzo Bianconi
  2026-10-01 13:50 ` [PATCH net-next v4 1/3] net: stmmac: set real_num_tx_queues to max(offset + count) Lorenzo Bianconi
  2026-10-01 13:50 ` [PATCH net-next v4 2/3] net: stmmac: align real_num_tx_queues to mqprio configuration Lorenzo Bianconi
@ 2026-10-01 13:50 ` Lorenzo Bianconi
  2026-10-05 14:14   ` netdev-bot+sashiko
  2026-10-06 10:11 ` [PATCH net-next v4 0/3] net: stmmac: Introduce " Lorenzo Bianconi
  3 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-01 13:50 UTC (permalink / raw)
  To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Vladimir Oltean, Furong Xu
  Cc: Daniel Thompson, netdev, linux-stm32, linux-arm-kernel,
	Davide Caratti, 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>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   3 +
 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   | 140 ++++++++++++++++++++--
 4 files changed, 148 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 0052f2129297..40eab899027b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -302,7 +302,10 @@ struct stmmac_priv {
 
 	struct {
 		bool enabled;
+		u32 prio[MTL_MAX_TX_QUEUES];
+		bool prio_offload;
 		u32 num_tx_queues;
+		u8 algo;
 	} xmit_qdisc;
 
 	struct dma_features dma_cap;
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 1dc9218bd4e9..faf7e82d5cde 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)
@@ -7941,6 +7935,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 < ARRAY_SIZE(priv->plat->tx_queues_cfg); 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 d0c3feb45789..e697e1c5bd6e 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,6 +1269,28 @@ 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 < ARRAY_SIZE(priv->plat->tx_queues_cfg); i++) {
+			u32 prio;
+
+			if (priv->plat->tx_queues_cfg[i].use_prio)
+				prio = priv->plat->tx_queues_cfg[i].prio;
+			else
+				prio = 0;
+
+			priv->xmit_qdisc.prio[i] = prio;
+			if (i < priv->plat->tx_queues_to_use)
+				stmmac_tx_queue_prio(priv, priv->hw, prio, i);
+		}
+
+		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;
+	}
+
 	priv->xmit_qdisc.num_tx_queues = priv->plat->tx_queues_to_use;
 	priv->xmit_qdisc.enabled = false;
 
@@ -1275,6 +1300,83 @@ static int stmmac_reset_tc_mqprio(struct net_device *ndev,
 	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 < ARRAY_SIZE(priv->plat->tx_queues_cfg); 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;
+		}
+
+		priv->xmit_qdisc.prio[i] = prio;
+		if (i < priv->plat->tx_queues_to_use)
+			stmmac_tx_queue_prio(priv, priv->hw, prio, i);
+	}
+
+	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)
 {
@@ -1285,23 +1387,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],
@@ -1310,6 +1411,22 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 				    qopt->offset[i] + 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;
@@ -1324,6 +1441,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);
+
 	priv->xmit_qdisc.num_tx_queues = num_tx_queues;
 	priv->xmit_qdisc.enabled = true;
 

-- 
2.55.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v4 1/3] net: stmmac: set real_num_tx_queues to max(offset + count)
  2026-10-01 13:50 ` [PATCH net-next v4 1/3] net: stmmac: set real_num_tx_queues to max(offset + count) Lorenzo Bianconi
@ 2026-10-05 14:13   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-05 14:13 UTC (permalink / raw)
  To: lorenzo.bianconi
  Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32, alexandre.torgue, olteanv, 0x1207, daniel,
	netdev, linux-stm32, linux-arm-kernel, dcaratti

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message describes only one of two failure modes.

--

net: stmmac: set real_num_tx_queues to max(offset + count)

This patch changes how tc_setup_dwmac510_mqprio() computes the number of
real TX queues. It now uses the maximum of offset[i] + count[i] instead
of the sum of the per-TC counts. As a result, real_num_tx_queues covers
the highest queue in the TC map even when the mqprio layout has gaps or
a non-zero first offset.

> netif_set_real_num_tx_queues() then passes this too-small count to
> netif_setup_tc(), which invalidates the mapping.

[Severity: Low]
Is this description complete? It seems to cover only layouts where TC0
itself ends up out of range.

In tc_setup_dwmac510_mqprio(), stmmac_set_ndev_tcs() runs before
netif_set_real_num_tx_queues(). It starts by calling netdev_reset_tc(),
which zeroes dev->prio_tc_map:

drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:stmmac_set_ndev_tcs() {
	...
	netdev_reset_tc(ndev);
	...
}

So when netif_setup_tc() runs, every priority maps to TC0, and the TC0
check is the only one that can fire:

net/core/dev.c:netif_setup_tc() {
	...
	/* 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;
	}
	...
}

mqprio_init() installs the user's prio_tc_map only after the offload
callback returns.

Take a gapped layout where TC0 fits, such as "queues 1@0 1@2". The old
sum gives real_num_tx_queues = 2, and nothing gets invalidated. TC1
keeps offset 2, so on transmit:

netdev_core_pick_tx()
  netdev_pick_tx()
    skb_tx_hash()          /* returns 2 */
  netdev_cap_txqueue()     /* 2 >= real_num_tx_queues, warns, returns 0 */

stmmac does not implement ndo_select_queue. That traffic class,
including express or preemptible FPE traffic, would therefore go out on
queue 0 with only a ratelimited warning. Its mapping would not be
rejected.

The code change handles both cases. Could the commit message also
describe this misrouting case, so backporters can see the full impact of
the bug being fixed?

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v4 2/3] net: stmmac: align real_num_tx_queues to mqprio configuration
  2026-10-01 13:50 ` [PATCH net-next v4 2/3] net: stmmac: align real_num_tx_queues to mqprio configuration Lorenzo Bianconi
@ 2026-10-05 14:14   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-05 14:14 UTC (permalink / raw)
  To: lorenzo.bianconi
  Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32, alexandre.torgue, olteanv, 0x1207, daniel,
	netdev, linux-stm32, linux-arm-kernel, dcaratti

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

Critical: 0 · High: 1 · Medium: 0 (1) · 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).

- [High] At this commit, stmmac_hw_setup() (stmmac_main.c:3781) calls
  netif_set_real_num_tx_queues(dev, priv->xmit_qdisc.num_tx_queues) and…

Pre-existing issues:
- [Medium] After this patch, a device open (ifdown/ifup,
  stmmac_reinit_queues()/ethtool -L, resume) restores the software half…

--

net: stmmac: align real_num_tx_queues to mqprio configuration

This commit records the TX queue count of the current mqprio offload in
priv->xmit_qdisc.num_tx_queues. stmmac_hw_setup() then uses that value
for real_num_tx_queues on every open, so it stays consistent with the
netdev TC map. It also stops stmmac_reinit_queues() from lowering the TX
queue count below what an active mqprio offload needs.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d95482b7f6e5d..1dc9218bd4e9b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3778,7 +3778,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);

[Severity: High]
Can this expose TX queues that have no rings allocated?

tc_setup_dwmac510_mqprio() sets num_tx_queues to max(offset + count)
from the user's qopt. The only range check is in
mqprio_validate_queue_counts(), and it compares against
dev->real_num_tx_queues, not plat->tx_queues_to_use:

net/sched/sch_mqprio_lib.c:mqprio_validate_queue_counts() {
    ...
	if (qopt->offset[i] >= dev->real_num_tx_queues ||
	    last > dev->real_num_tx_queues) {
    ...
}

While the interface is down, real_num_tx_queues can be larger than
tx_queues_to_use:

  - __stmmac_dvr_probe() allocates the netdev with MTL_MAX_TX_QUEUES and
    doesn't lower real_num_tx_queues before the first open.

  - stmmac_reinit_queues() on a down interface changes tx_queues_to_use
    but leaves real_num_tx_queues as it was.

stmmac_setup_tc() has no netif_running() check. Take tx_queues_to_use = 4
on a down interface and this sequence:

  tc qdisc add ... mqprio num_tc 2 ... queues 4@0 4@4 hw 1
  ip link set ... up

That stores xmit_qdisc.num_tx_queues = 8. On open,
alloc_dma_tx_desc_resources() only allocates rings for
queue < tx_queues_to_use, but stmmac_hw_setup() now publishes 8 real TX
queues.

Traffic mapped to TC1 then goes to queues 4-7, and stmmac_xmit() does:

	tx_q = &priv->dma_conf.tx_queue[queue];
	...
	WARN_ON(tx_q->tx_skbuff[first_entry]);

For those queues, tx_skbuff and the descriptor ring are NULL. Would this
cause a NULL pointer dereference in the TX path?

Before this patch, open always reset real_num_tx_queues to
tx_queues_to_use and netif_setup_tc() dropped the invalid TC map, so
this path couldn't be reached.

Later in the series, "net: stmmac: add tc mqprio hw queue priority
offload" adds a check that offset + count does not exceed
plat->tx_queues_to_use in tc_setup_dwmac510_mqprio(), which closes this.
However, this patch carries a Fixes: tag and the later patch doesn't.
Would a stable backport of this patch alone bring back the oops?

Should the tx_queues_to_use bound be part of this patch instead?

[ ... ]

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 1aa80d51ab33a..d0c3feb45789f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

[ ... ]

> @@ -1321,6 +1324,9 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
>  	if (err)
>  		goto error_reset_num_tx_queues;
>  
> +	priv->xmit_qdisc.num_tx_queues = num_tx_queues;
> +	priv->xmit_qdisc.enabled = true;

[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch. This
patch now restores the software side of the mqprio offload on every
open. Does anything restore the hardware side as well?

On open, stmmac_init_dma_engine() issues a DMA software reset. That
resets the MTL registers, including the FPE_MTL_PREEMPTION_CLASS field.

After that, stmmac_dma_operation_mode()->dwxgmac2_dma_tx_mode() rewrites
Q2TCMAP to the identity map:

	value |= FIELD_PREP(XGMAC_Q2TCMAP, channel);

The only function that programs the preemption class and the mqprio
Q2TCMAP is stmmac_fpe_map_preemption_class(). It is called only from the
taprio and mqprio setup and reset paths in stmmac_tc.c, never from open.

Consider ifdown/ifup, ethtool -L or resume. Afterwards real_num_tx_queues
and the netdev TC map still reflect the mqprio offload, and
xmit_qdisc.enabled stays true. The hardware, though, has no preemptible
queues and, on XGMAC, a 1:1 queue-to-TC map that doesn't match the qopt
offsets and counts.

mqprio->preemptible_tcs is not saved here, so the open path can't replay
it. The final patch in the series restores prio and algo from
xmit_qdisc in stmmac_mtl_configuration(). It still doesn't restore the
preemption class or Q2TCMAP.

Would it make sense to save preemptible_tcs in xmit_qdisc and reprogram
it from the open path?

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v4 3/3] net: stmmac: add tc mqprio hw queue priority offload
  2026-10-01 13:50 ` [PATCH net-next v4 3/3] net: stmmac: add tc mqprio hw queue priority offload Lorenzo Bianconi
@ 2026-10-05 14:14   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-05 14:14 UTC (permalink / raw)
  To: lorenzo.bianconi
  Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32, alexandre.torgue, olteanv, 0x1207, daniel,
	netdev, linux-stm32, linux-arm-kernel, dcaratti

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

Critical: 0 · High: 0 · Medium: 2 (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 patch adds two TX queue bounds checks to
  tc_setup_dwmac510_mqprio()…
- [Medium] In TC_MQPRIO_MODE_CHANNEL, mqprio_enable_offload()
  (net/sched/sch_mqprio.c) passes mqprio->flags to the driver, together…
- [Low] Channel-mode state is set in tc_mqprio_config_queue_prio(): MTL
  scheduler set to SP, PSTQ priorities written, xmit_qdisc.algo = SP,…
- [Low] The 'SP conflicts with AVB queues' rule is checked only once, in
  tc_mqprio_validate_chan_mode(), over queues 0..tx_queues_to_use-1.

Pre-existing issues:
- [Medium] stmmac_setup_tc() takes no runtime-PM reference.

--

net: stmmac: add tc mqprio hw queue priority offload

This patch offloads mqprio channel mode to the stmmac MTL TX scheduler. It
switches the scheduler to strict priority and programs the PSTQX/PSTC
per-queue priority bitmaps from prio_tc_map. Channel mode requires DCB hw
support and a 1:1 TC to TX queue mapping, and is rejected when AVB queues
are configured. DCB mode still offloads only the netdev TC map and the FPE
preemption class mapping.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
> index c889204a7aa5d..b6b5ef7c8fc48 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;
>  		}

[Severity: Low]
Can this check see a stale SP value left over from an earlier channel-mode
offload?

xmit_qdisc.algo is set back to plat->tx_sched_algorithm only in
stmmac_reset_tc_mqprio(), which only runs when num_tc == 0. The DCB branch
of tc_setup_dwmac510_mqprio() never looks at prio_offload, and it never
restores the DT scheduler or the DT queue priorities.

Suppose a channel-mode root mqprio is replaced with a DCB mqprio that uses a
different handle. __tc_modify_qdisc() then takes the create_n_graft path, so
the new qdisc's mqprio_init() calls ndo_setup_tc before the old instance is
torn down:

tc_setup_dwmac510_mqprio()          /* new DCB config */
  stmmac_fpe_map_preemption_class()
    dwmac5_fpe_map_preemption_class()
      priv->xmit_qdisc.algo == MTL_TX_ALGORITHM_SP   /* left over */

In that window, a DCB request with preemptible TCs and a TC with count > 1
fails with ALG_ERR_MSG and -EINVAL, even when the DT scheduler is not SP.

Should the DCB path undo the channel-mode state before the FPE validation
runs? The old instance's teardown resets the netdev TC config afterwards
anyway, but this setup/teardown asymmetry is new in this patch.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index d0c3feb45789f..e697e1c5bd6e4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

[ ... ]

> @@ -1275,6 +1300,83 @@ static int stmmac_reset_tc_mqprio(struct net_device *ndev,
>  	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)
> +{

[ ... ]

> +		priv->xmit_qdisc.prio[i] = prio;
> +		if (i < priv->plat->tx_queues_to_use)
> +			stmmac_tx_queue_prio(priv, priv->hw, prio, i);
> +	}
> +
> +	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 and was not introduced by this patch.
stmmac_setup_tc() takes no runtime PM reference before it dispatches
TC_SETUP_QDISC_MQPRIO.

Once stmmac_release() has called pm_runtime_put() and
stmmac_runtime_suspend() has gated the bus clocks through
stmmac_bus_clks_config(priv, false), could these MTL register writes reach
an unclocked block? The same applies to the writes in the prio_offload
branch of stmmac_reset_tc_mqprio().

Before this patch, the same path already did MMIO through
dwmac5_fpe_map_preemption_class() and dwxgmac3_fpe_map_preemption_class()
on every mqprio setup and reset, so the patch doesn't widen the exposure.

> +
> +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;
> +		}
> +	}

[Severity: Low]
Is this check enough to keep SP and CBS from running at the same time?

It only covers queues 0..tx_queues_to_use-1, and only at offload time.
While xmit_qdisc.enabled is set, stmmac_reinit_queues() only refuses
tx_cnt < xmit_qdisc.num_tx_queues. That means ethtool -L tx N can raise
the TX queue count while prio_offload is active.

On DT platforms, stmmac_mtl_setup() sets mode_to_use only for the queues
described in DT. The remaining tx_queues_cfg[] entries stay 0, which is
MTL_QUEUE_AVB. A DT AVB queue can also come back after the count was
lowered for a while.

On reopen, stmmac_mtl_configuration() programs SP from xmit_qdisc.algo.
stmmac_configure_cbs() then configures CBS on every queue whose
mode_to_use is not MTL_QUEUE_DCB.

Wouldn't that produce the SP plus AVB/CBS combination this function is
meant to reject? It would also bypass the new prio_offload check in
tc_setup_cbs(). The newly enabled queues sit beyond real_num_tx_queues, so
only XDP/AF_XDP traffic would reach them.

> +
> +	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]
What happens to shaper and rate requests in channel mode?

mqprio_enable_offload() passes mqprio->flags to the driver, together with
mqprio->shaper (TC_MQPRIO_F_SHAPER) and min_rate[]/max_rate[]
(TC_MQPRIO_F_MIN_RATE / TC_MQPRIO_F_MAX_RATE). This function checks only
dcben, AVB queues and the 1:1 mapping. tc_setup_dwmac510_mqprio() reads
only TC_MQPRIO_F_MODE from mqprio->flags.

So would a "mode channel shaper bw_rlimit max_rate ..." request be
accepted and marked as hardware offloaded, even though only SP and the
PSTQ/PSTC maps get programmed and no rate limit is applied?

Other drivers such as am65-cpsw-qos, mvneta, cxgb4 and aq_main either
implement these fields or reject them. Should this return -EOPNOTSUPP when
TC_MQPRIO_F_SHAPER, TC_MQPRIO_F_MIN_RATE or TC_MQPRIO_F_MAX_RATE is set?

> +
>  static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
>  				    struct tc_mqprio_qopt_offload *mqprio)
>  {
> @@ -1285,23 +1387,22 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,

[ ... ]

>  	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;
>  

[ ... ]

>  	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;
> +		}
> +

[Severity: Medium]
Both of these bounds checks apply in every mqprio mode, DCB included, and
the commit message doesn't mention them. Are they meant to be a fix on
their own?

Without them, an out-of-range TX queue seems reachable. stmmac sets
caps->validate_queue_counts, so mqprio_validate_queue_counts() only checks
offset + count against dev->real_num_tx_queues. Before the first open,
real_num_tx_queues is still MTL_MAX_TX_QUEUES, as set by
devm_alloc_etherdev_mqs().

Take tx_queues_to_use = 4 with the interface down. "queues 4@0 4@4 hw 1"
passed the old checks and set xmit_qdisc.num_tx_queues to 8. On open,
stmmac_hw_setup() then does:

    netif_set_real_num_tx_queues(dev, priv->xmit_qdisc.num_tx_queues);

That exposes 8 real TX queues with only 4 descriptor rings. The xmit paths
index priv->dma_conf.tx_queue[skb_get_queue_mapping(skb)] with no bound.

The earlier commit in this series, "net: stmmac: align real_num_tx_queues
to mqprio configuration", makes this reachable. It carries
Fixes: 195e4f409a40, so it could be backported to stable without these
checks.

Should the checks be split into their own patch, or folded into that
commit, with a matching Fixes: tag and a note in the commit message?

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v4 0/3] net: stmmac: Introduce hw queue priority offload
  2026-10-01 13:50 [PATCH net-next v4 0/3] net: stmmac: Introduce hw queue priority offload Lorenzo Bianconi
                   ` (2 preceding siblings ...)
  2026-10-01 13:50 ` [PATCH net-next v4 3/3] net: stmmac: add tc mqprio hw queue priority offload Lorenzo Bianconi
@ 2026-10-06 10:11 ` Lorenzo Bianconi
  3 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-06 10:11 UTC (permalink / raw)
  To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Vladimir Oltean, Furong Xu
  Cc: Daniel Thompson, netdev, linux-stm32, linux-arm-kernel,
	Davide Caratti

[-- Attachment #1: Type: text/plain, Size: 2432 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().
> 
> The misbehaviour has been reported by Shashank and the patch has been
> tested on a Qualcomm RB3-Gen2 board.
> 
> ---
> Changes in v4:
> - Move real_num_tx_queues fix before mqprio offload patch in order to
>   allow easy backporting.
> - Add patch 'net: stmmac: set real_num_tx_queues from max(offset +
>   count)' to fix the real_num_tx_queues configuration.
> - Link to v3: https://lore.kernel.org/r/20260928-stmmac-mqprio-prio-offload-v3-0-abbe181f5024@oss.qualcomm.com
> 
> 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 (3):
>       net: stmmac: set real_num_tx_queues to max(offset + count)
>       net: stmmac: align real_num_tx_queues to mqprio configuration
>       net: stmmac: add tc mqprio hw queue priority offload

I will fix sashiko's reported issues in v5.

Regards,
Lorenzo

> 
>  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   | 147 ++++++++++++++++++++--
>  4 files changed, 172 insertions(+), 23 deletions(-)
> ---
> base-commit: eb0c18404c8943356f4aa164e5db9067b79ea93c
> 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-06 10:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-01 13:50 [PATCH net-next v4 0/3] net: stmmac: Introduce hw queue priority offload Lorenzo Bianconi
2026-10-01 13:50 ` [PATCH net-next v4 1/3] net: stmmac: set real_num_tx_queues to max(offset + count) Lorenzo Bianconi
2026-10-05 14:13   ` netdev-bot+sashiko
2026-10-01 13:50 ` [PATCH net-next v4 2/3] net: stmmac: align real_num_tx_queues to mqprio configuration Lorenzo Bianconi
2026-10-05 14:14   ` netdev-bot+sashiko
2026-10-01 13:50 ` [PATCH net-next v4 3/3] net: stmmac: add tc mqprio hw queue priority offload Lorenzo Bianconi
2026-10-05 14:14   ` netdev-bot+sashiko
2026-10-06 10:11 ` [PATCH net-next v4 0/3] net: stmmac: Introduce " Lorenzo Bianconi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox