netdev - Re: [PATCH RFC 3/5] net: Add a netdev software feature set that defaults to off.

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]

Date:   Tue, 1 Oct 2019 08:18:16 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC:     Network Development <netdev@...r.kernel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Subject: Re: [PATCH RFC 3/5] net: Add a netdev software feature set that
 defaults to off.

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@...unet.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.