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]

Message-ID: <20190930062427.GF2879@gauss3.secunet.de>
Date:   Mon, 30 Sep 2019 08:24:28 +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 23, 2019 at 08:38:56AM -0400, Willem de Bruijn wrote:
> On Fri, Sep 20, 2019 at 12:49 AM Steffen Klassert
> <steffen.klassert@...unet.com> wrote:
> >
> > diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> > index b239507da2a0..34d050bb1ae6 100644
> > --- a/include/linux/netdev_features.h
> > +++ b/include/linux/netdev_features.h
> > @@ -230,6 +230,9 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
> >  /* changeable features with no special hardware requirements */
> >  #define NETIF_F_SOFT_FEATURES  (NETIF_F_GSO | NETIF_F_GRO)
> >
> > +/* Changeable features with no special hardware requirements that defaults to off. */
> > +#define NETIF_F_SOFT_FEATURES_OFF      NETIF_F_GRO_FRAGLIST
> > +
> 
> NETIF_F_GRO_FRAGLIST is not really a device feature, but a way to
> configure which form of UDP GRO to apply.

NETIF_F_GRO is also not really a device feature. It is a feature with
no special hardware requirements, as NETIF_F_GRO_FRAGLIST is.
Fraglist GRO is a special way to do GRO and should be configured in the
same way we configure standard GRO.

> 
> 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?

> 
> If so, how about a ternary per-netns sysctl {off, on without gro-list,
> on with gro-list} instead of configuring through ethtool?

I'd not like to have a global knob to configure this.
On some devices it might make sense to enable fraglist
GRO, but on others not. Also it would be nice if we can
configure both vatiants with the same tool (ethtool).