linux-kernel - [PATCH] stmmac: disable tx coalescing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161205122711.GA30774@amd>
Date:   Mon, 5 Dec 2016 13:27:11 +0100
From:   Pavel Machek <pavel@....cz>
To:     David Miller <davem@...emloft.net>
Cc:     peppe.cavallaro@...com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, alexandre.torgue@...com,
        LinoSanfilippo@....de
Subject: [PATCH] stmmac: disable tx coalescing

Tx coalescing in stmmac is broken in more than one way, so disable it
for now.

First, low-res timers have resolution down to one per second. It is
not acceptable to delay transmits for 40msec, and certainly not
acceptable to delay them for 1000msec.

Second, the logic is wrong:

    if (likely(priv->tx_coal_frames > priv->tx_count_frames))
     	mod_timer(&priv->txtimer,
     	          STMMAC_COAL_TIMER(priv->tx_coal_timer));
    ...

doing tx_clean() after set number of packets, or set time after the
first packet is transmitted would make sense. But that's not what the
code does. As long as packets are being transmitted, you move the
timer into the future.. so that finally you run out of the place, then
wait for timer (!) and only then you do the cleaning.

Third, tx_cleanup is not safe to call from interrupt (tx_lock is not
irqsave), but that's exactly what coalescing code does.

Signed-off-by: Pavel Machek <pavel@...x.de>

---

This is stable candidate, afaict. It is broken in 4.4, too.

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 3ced2e1..32ce148 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -244,11 +244,11 @@ struct stmmac_extra_stats {
 /* Max/Min RI Watchdog Timer count value */
 #define MAX_DMA_RIWT		0xff
 #define MIN_DMA_RIWT		0x20
-/* Tx coalesce parameters */
+/* Tx coalesce parameters. Set STMMAC_TX_FRAMES to 0 to disable coalescing. */
 #define STMMAC_COAL_TX_TIMER	40000
 #define STMMAC_MAX_COAL_TX_TICK	100000
 #define STMMAC_TX_MAX_FRAMES	256
-#define STMMAC_TX_FRAMES	64
+#define STMMAC_TX_FRAMES	0

 /* Rx IPC status */
 enum rx_frame_status {

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)