On Mon, Sep 30, 2019 at 2:24 AM Steffen Klassert
[off-list ref] wrote:

On Mon, Sep 23, 2019 at 08:38:56AM -0400, Willem de Bruijn wrote:

quoted

On Fri, Sep 20, 2019 at 12:49 AM Steffen Klassert
[off-list ref] wrote:

quoted

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.

quoted

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.

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

quoted

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