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

[Steffen Klassert <steffen.klassert@secunet.com>]

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@secunet.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).