[PATCH net v3 1/2] net: stmmac: ethtool: ignore inapplicable per-queue coalesce fields

[PATCH net v3 1/2] net: stmmac: ethtool: ignore inapplicable per-queue coalesce fields

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

rx_queues_to_use and tx_queues_to_use are independent: dwmac-intel uses
6 RX and 4 TX channels on TGL and EHL, and snps,rx-queues-to-use and
snps,tx-queues-to-use are parsed separately, so a queue index can exist
in one direction only. include/linux/ethtool.h documents the parameters
of the missing side as inapplicable and asks set_per_queue_coalesce() to
ignore them, but __stmmac_set_coalesce() validates both directions for
every index while __stmmac_get_coalesce() reports the inapplicable ones
as zero.

ethtool implements --per-queue by reading the current settings of every
queue in the mask and sending them back with only the requested fields
overwritten, so the zeroed fields are fed straight back into the setter:

  - on an RX-only index the test for both TX fields being zero rejects
    the request, so per-queue RX coalescing cannot be changed at all;
  - on a TX-only index rx_coalesce_usecs is zero, stmmac_usec2riwt()
    returns zero and the MIN_DMA_RIWT test rejects the request, so
    per-queue TX coalescing cannot be changed at all.

On an RX-only index the TX test rejects the request after the RX block
has already called stmmac_rx_watchdog() and stored rx_riwt[] and
rx_coal_frames[], so the driver reports -EINVAL with the hardware half
reprogrammed.  The roll-back that ethtool_set_per_queue_coalesce() runs
for the queues it has already changed then trips over the same test and
cannot restore them.  On a TX-only index the rx_riwt range check fails
before anything is written, but the request is still rejected and the
TX settings are never stored.

Validate and apply only the side that the queue index actually has, and
move the TX checks in front of the RX block so that a request is either
applied completely or rejected without touching the device.

Fixes: db2f2842e6f5 ("net: stmmac: add per-queue TX & RX coalesce ethtool support")
Cc: stable@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
Changes in v2:
- Trim the comment in front of the TX checks to one line; the reasoning
  belongs in the changelog. (Andrew Lunn)
- Link: https://lore.kernel.org/all/20260920015647.1783574-1-xiaolinkui@126.com/

Changes in v3:
- Only run the TX checks when the index carries a TX ring, and the rx_riwt
  range check only when it carries an RX ring. Moving the checks alone
  rejected per-queue RX coalescing on an RX-only index and broke the
  ethtool core rollback for it. (Sashiko review)
- Reword the subject and changelog to describe the direction-aware
  validation.

 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  | 35 +++++++++++++------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 154cc0c7623d..fed648a9f784 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -837,6 +837,8 @@ static int __stmmac_set_coalesce(struct net_device *dev,
 	struct stmmac_priv *priv = netdev_priv(dev);
 	bool all_queues = false;
 	unsigned int rx_riwt;
+	bool has_rx;
+	bool has_tx;
 	u32 max_cnt;
 	u32 rx_cnt;
 	u32 tx_cnt;
@@ -850,7 +852,20 @@ static int __stmmac_set_coalesce(struct net_device *dev,
 	else if (queue >= max_cnt)
 		return -EINVAL;
 
-	if (priv->use_riwt) {
+	has_rx = all_queues || queue < rx_cnt;
+	has_tx = all_queues || queue < tx_cnt;
+
+	/* An index can be RX-only or TX-only; ignore the missing side. */
+	if (has_tx &&
+	    ec->tx_coalesce_usecs == 0 && ec->tx_max_coalesced_frames == 0)
+		return -EINVAL;
+
+	if (has_tx &&
+	    (ec->tx_coalesce_usecs > STMMAC_MAX_COAL_TX_TICK ||
+	     ec->tx_max_coalesced_frames > STMMAC_TX_MAX_FRAMES))
+		return -EINVAL;
+
+	if (has_rx && priv->use_riwt) {
 		rx_riwt = stmmac_usec2riwt(ec->rx_coalesce_usecs, priv);
 
 		if ((rx_riwt > MAX_DMA_RIWT) || (rx_riwt < MIN_DMA_RIWT))
@@ -866,7 +881,7 @@ static int __stmmac_set_coalesce(struct net_device *dev,
 				priv->rx_coal_frames[i] =
 					ec->rx_max_coalesced_frames;
 			}
-		} else if (queue < rx_cnt) {
+		} else {
 			priv->rx_riwt[queue] = rx_riwt;
 			stmmac_rx_watchdog(priv, priv->ioaddr,
 					   rx_riwt, queue);
@@ -875,14 +890,6 @@ static int __stmmac_set_coalesce(struct net_device *dev,
 		}
 	}
 
-	if ((ec->tx_coalesce_usecs == 0) &&
-	    (ec->tx_max_coalesced_frames == 0))
-		return -EINVAL;
-
-	if ((ec->tx_coalesce_usecs > STMMAC_MAX_COAL_TX_TICK) ||
-	    (ec->tx_max_coalesced_frames > STMMAC_TX_MAX_FRAMES))
-		return -EINVAL;
-
 	if (all_queues) {
 		int i;
 
@@ -892,7 +899,7 @@ static int __stmmac_set_coalesce(struct net_device *dev,
 			priv->tx_coal_timer[i] =
 				ec->tx_coalesce_usecs;
 		}
-	} else if (queue < tx_cnt) {
+	} else if (has_tx) {
 		priv->tx_coal_frames[queue] =
 			ec->tx_max_coalesced_frames;
 		priv->tx_coal_timer[queue] =
-- 
2.25.1

[PATCH net v3 2/2] net: stmmac: fix a divide by zero in stmmac_xdp_xmit_xdpf()

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

tx-frames 0 is a valid coalescing request: it only stops the frame count
from raising TX completion interrupts, while the coalescing timer armed
by stmmac_tx_timer_arm() keeps reclaiming the descriptors. That is why
stmmac_xmit(), stmmac_tso_xmit() and stmmac_xdp_xmit_zc() all test
priv->tx_coal_frames[queue] before taking the modulo, and why
__stmmac_set_coalesce() rejects the request only when tx-usecs is zero
as well, since then nothing would complete the transmissions.

stmmac_xdp_xmit_xdpf() is the one transmit path that takes the modulo
without the test, so

	ethtool -C eth0 tx-usecs 10 tx-frames 0

followed by an XDP_TX or an ndo_xdp_xmit frame divides by zero, which
oopses in softirq context on the architectures that trap on a zero
divisor.

Add the missing test, which leaves set_ic false exactly like the other
transmit paths do.

Fixes: be8b38a722e6 ("net: stmmac: Add support for XDP_TX action")
Cc: stable@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
v3:
- New patch. stmmac_xdp_xmit_xdpf() is the only transmit path that divides
  by tx_coal_frames[queue] without a zero test; add the missing test the
  way stmmac_xdp_xmit_zc() has it. (Sashiko review)

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e2e680dd980c..276187f50ee3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5256,7 +5256,9 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 
 	tx_q->tx_count_frames++;
 
-	if (tx_q->tx_count_frames % priv->tx_coal_frames[queue] == 0)
+	if (!priv->tx_coal_frames[queue])
+		set_ic = false;
+	else if (tx_q->tx_count_frames % priv->tx_coal_frames[queue] == 0)
 		set_ic = true;
 	else
 		set_ic = false;
-- 
2.25.1