netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Holger Hoffstätte" <holger@applied-asynchrony.com>
To: Eric Dumazet <eric.dumazet@gmail.com>,
	Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>,
	netdev@vger.kernel.org
Cc: hkallweit1@gmail.com
Subject: Re: r8169: Performance regression and latency instability
Date: Fri, 16 Aug 2019 15:59:21 +0200	[thread overview]
Message-ID: <792d3a56-32aa-afee-f2b4-1f867b9cf75f@applied-asynchrony.com> (raw)
In-Reply-To: <217e3fa9-7782-08c7-1f2b-8dabacaa83f9@gmail.com>

On 8/16/19 2:35 PM, Eric Dumazet wrote:
..snip..
> I also see this relevant commit : I have no idea why SG would have any relation with TSO.
> 
> commit a7eb6a4f2560d5ae64bfac98d79d11378ca2de6c
> Author: Holger Hoffstätte <holger@applied-asynchrony.com>
> Date:   Fri Aug 9 00:02:40 2019 +0200
> 
>      r8169: fix performance issue on RTL8168evl
>      
>      Disabling TSO but leaving SG active results is a significant
>      performance drop. Therefore disable also SG on RTL8168evl.
>      This restores the original performance.
>      
>      Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
>      Signed-off-by: Holger Hoffstätte <holger@applied-asynchrony.com>
>      Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>      Signed-off-by: David S. Miller <davem@davemloft.net>

It does not - and admittedly none of this makes sense, but stay with me here.

The commit 93681cd7d94f to net-next enabled rx/tx HW checksumming and TSO
by default, but disabled TSO for one specific chip revision - the most popular
one, of course. Enabling rx/tx checksums by default while leaving SG on turned
out to be the performance issue (~780 MBit max) that I found & fixed in the
quoted commit. SG *can* be enabled when rx/tx checkusmming is *dis*abled
(I just verified again), we just had to sanitize the new default.

An alternative strategy could still be to (again?) disable everything by default
and just let people manually enable whatever settings work for their random
chip revision + BIOS combination. I'll let Heiner chime in here.

Basically these chips are dumpster fires and should not be used for anything
ever, which of course means they are everywhere.

AFAICT none of this has anything to do with Juliana's problem..

-h

  reply	other threads:[~2019-08-16 13:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-16 12:09 r8169: Performance regression and latency instability Juliana Rodrigueiro
2019-08-16 12:35 ` Eric Dumazet
2019-08-16 13:59   ` Holger Hoffstätte [this message]
2019-08-16 19:12     ` Heiner Kallweit
2019-08-19 16:04       ` Juliana Rodrigueiro
2019-09-06 11:25         ` Juliana Rodrigueiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=792d3a56-32aa-afee-f2b4-1f867b9cf75f@applied-asynchrony.com \
    --to=holger@applied-asynchrony.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=juliana.rodrigueiro@intra2net.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).