[PATCH net] net: stmmac: ethtool: validate TX coalesce before reprogramming RX

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@xxxxxxxxxx>

__stmmac_set_coalesce() applies the RX part of the request first and
only afterwards checks the TX parameters. The RX path already calls
stmmac_rx_watchdog() and stores rx_riwt[] and rx_coal_frames[], so when
the TX check rejects the request the driver returns -EINVAL after having
silently changed the hardware. A following ethtool -c then reports the
new RX values even though the command failed.

This became easy to hit once the per-queue interface was added.
__stmmac_get_coalesce() reports tx-usecs and tx-frames as 0 for a queue
index that is RX-only, and ethtool applies per-queue coalesce by reading
the current values first and sending them straight back. The next set is
therefore guaranteed to trip the test for both TX fields being zero,
right after the RX watchdog has been reprogrammed.

Move both TX checks in front of the RX block so 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@xxxxxxxxxxxxxxx
Signed-off-by: Linkui Xiao <xiaolinkui@xxxxxxxxxx>
---
 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  | 21 ++++++++++++-------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 154cc0c7623d..325db062f72a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -850,6 +850,19 @@ static int __stmmac_set_coalesce(struct net_device *dev,
 	else if (queue >= max_cnt)
 		return -EINVAL;
 
+	/* Check the TX parameters before anything is applied: the RX part
+	 * below already writes to the hardware, so rejecting the request
+	 * afterwards would leave the device with only half of the settings
+	 * the caller asked for while reporting a failure.
+	 */
+	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 (priv->use_riwt) {
 		rx_riwt = stmmac_usec2riwt(ec->rx_coalesce_usecs, priv);
 
@@ -875,14 +888,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;
 
-- 
2.25.1