netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steffen Klassert <steffen.klassert@secunet.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Network Development <netdev@vger.kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Subject: Re: [PATCH RFC 3/5] net: Add a netdev software feature set that defaults to off.
Date: Tue, 1 Oct 2019 08:18:16 +0200	[thread overview]
Message-ID: <20191001061816.GP2879@gauss3.secunet.de> (raw)
In-Reply-To: <CA+FuTScxNZKdb0FqAXjxPXY4XEhFFh+_COy0QjCfvw4phSQF3g@mail.gmail.com>

On Mon, Sep 30, 2019 at 11:26:55AM -0400, Willem de Bruijn wrote:
> On Mon, Sep 30, 2019 at 2:24 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > On Mon, Sep 23, 2019 at 08:38:56AM -0400, Willem de Bruijn wrote:
> > >
> > > The UDP GRO benchmarks were largely positive, but not a strict win if
> > > I read Paolo's previous results correctly. Even if enabling to by
> > > default, it probably should come with a sysctl to disable for specific
> > > workloads.
> >
> > Maybe we can just keep the default for the local input path
> > as is and enable GRO as this:
> >
> > For standard UDP GRO on local input, do GRO only if a GRO enabled
> > socket is found.
> >
> > If there is no local socket found and forwarding is enabled,
> > assume forwarding and do standard GRO.
> >
> > If fraglist GRO is enabled, do it as default on local input and
> > forwarding because it is explicitly configured.
> >
> > Would such a policy make semse?
> 
> Making the choice between fraglist or non-fraglist GRO explicitly
> configurable sounds great. Per device through ethtool over global
> sysctl, too.
> 
> My main concern is not this patch, but 1/5 that enables UDP GRO by
> default. There should be a way to disable it, at least.
> 
> I guess your suggestion is to only enable it with forwarding, which is
> unlikely to see a cycle regression. And if there is a latency
> regression, disable all GRO to disable UDP GRO.

Yes, do GRO only for forwarding or if there is a GRO capable socket.

In this case it can be disabled only by disable all GRO.
It might be a disadvantage, but that's how it is with other
protocols too.

> 
> Instead, how about adding a UDP GRO ethtool feature independent of
> forwarding, analogous to fraglist GRO? Then both are explicitly under
> admin control. And can be enabled by default (either now, or after
> getting more data).

We could add a protocol specific feature, but what would it mean
if UDP GRO is enabled?

Would it be enabled for forwarding, and for local input only if there
is a GRO capable socket? Or would it be enabled even if there
is no GRO capable socket? Same question when UDP GRO is disabled.

Also, what means enabling GRO then? Enable GRO for all protocols
but UDP? Either UDP becomes something special then, or we need
to create protocol specific features for the other protocols
too. Same would apply for fraglist GRO.


  reply	other threads:[~2019-10-01  6:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-20  4:49 [PATCH RFC 0/5] Support fraglist GRO/GSO Steffen Klassert
2019-09-20  4:49 ` [PATCH RFC 1/5] UDP: enable GRO by default Steffen Klassert
2019-09-23 12:53   ` Willem de Bruijn
2019-09-23 12:55     ` Willem de Bruijn
2019-09-20  4:49 ` [PATCH RFC 2/5] net: Add fraglist GRO/GSO feature flags Steffen Klassert
2019-09-20  4:49 ` [PATCH RFC 3/5] net: Add a netdev software feature set that defaults to off Steffen Klassert
2019-09-23 12:38   ` Willem de Bruijn
2019-09-30  6:24     ` Steffen Klassert
2019-09-30 15:26       ` Willem de Bruijn
2019-10-01  6:18         ` Steffen Klassert [this message]
2019-10-01 12:43           ` Willem de Bruijn
2019-10-02  8:27             ` Steffen Klassert
2019-10-02 12:32               ` Willem de Bruijn
2019-09-20  4:49 ` [PATCH RFC 4/5] net: Support GRO/GSO fraglist chaining Steffen Klassert
2019-09-20  4:49 ` [PATCH RFC 5/5] udp: Support UDP fraglist GRO/GSO Steffen Klassert
2019-09-23 13:01   ` Willem de Bruijn
2019-09-30  6:30     ` Steffen Klassert
2019-09-30 15:32       ` Willem de Bruijn

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=20191001061816.GP2879@gauss3.secunet.de \
    --to=steffen.klassert@secunet.com \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=subashab@codeaurora.org \
    --cc=willemdebruijn.kernel@gmail.com \
    /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).