netdev - Re: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses.

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: <20161205115653.GB16126@amd>
Date:   Mon, 5 Dec 2016 12:56:53 +0100
From:   Pavel Machek <pavel@....cz>
To:     Giuseppe CAVALLARO <peppe.cavallaro@...com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses.

Hi!

> the idea behind the TX mitigation is to mix the interrupt and
> timer and this approach gave us real benefit in terms
> of performances and CPU usage (especially on SH4-200/SH4-300 platforms
> based).
> In the ring, some descriptors can raise the irq (according to a
> threshold) and set the IC bit. In this path, the NAPI  poll will be
> scheduled.
> But there is a timer that can run (and we experimented that no high
> resolution is needed) to clear the tx resources.

I'm sorry, but it is just broken. It could have never worked. If it
appered it did, you did not test it right.

First, low-res timers have resolution down to one per second (see
David's email). 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));
	                else {
 				priv->tx_count_frames = 0;
 				priv->hw->desc->set_tx_ic(desc);
 				priv->xstats.tx_set_ic_bit++;
			}

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, times are wrong by order of magnitude. AFAICT cleaning should
be at around 5msec at 100mbit speeds, and at around .5msec at
gigabit. You have 40msec there. (Perhaps that is not too important if
the logic is fixed, as described above).

Best regards,
								Pavel
-- 
(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)