netdev - Re: [PATCH] r8169: set TxConfig register after TX / RX is enabled, just like RxConfig

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] [day] [month] [year] [list]

Date:   Fri, 07 Sep 2018 14:53:01 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     mail@...iej.szmigiero.name
Cc:     nic_swsd@...ltek.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, a3at.mail@...il.com,
        hkallweit1@...il.com
Subject: Re: [PATCH] r8169: set TxConfig register after TX / RX is enabled,
 just like RxConfig

From: "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>
Date: Fri, 7 Sep 2018 20:15:22 +0200

> Commit 3559d81e76bf ("r8169: simplify rtl_hw_start_8169") changed order of
> two register writes:
> 1) Caused RxConfig to be written before TX / RX is enabled,
> 2) Caused TxConfig to be written before TX / RX is enabled.
> 
> At least on XIDs 10000000 ("RTL8169sb/8110sb") and
> 18000000 ("RTL8169sc/8110sc") such writes are ignored by the chip, leaving
> values in these registers intact.
> 
> Change 1) was reverted by
> commit 05212ba8132b42 ("r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices"),
> however change 2) wasn't.
> 
> In practice, this caused TxConfig's "InterFrameGap time" and "Max DMA Burst
> Size per Tx DMA Burst" bits to be zero dramatically reducing TX performance
> (in my tests it dropped from around 500Mbps to around 50Mbps).
> 
> This patch fixes the issue by moving TxConfig register write a bit later in
> the code so it happens after TX / RX is already enabled.
> 
> Signed-off-by: Maciej S. Szmigiero <mail@...iej.szmigiero.name>
> Fixes: 05212ba8132b42 ("r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices")
> ---
> "Fixes" tag points to the RxConfig fix instead of the actual commit that
> introduced this regression to maintain patch ordering since the RxConfig fix
> partially affects the same code lines as this fix.

Applied, thanks.

Please, in the future, always put your Fixes: tag first in the set of tags.

Thank you.