Re: [PATCH net-next] net: avoid unneeded UDP L4 and fraglist GSO resegmentation

[Alexander Lobakin]

On Saturday, 31 October 2020, 2:12, Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote:

Hi Willem,

> On Fri, Oct 30, 2020 at 2:33 PM Alexander Lobakin alobakin@xxxxx wrote:
>
> > Commit 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.") added a support
> > for fraglist UDP L4 and fraglist GSO not only for local traffic, but also
> > for forwarding. This works well on simple setups, but when any logical
> > netdev (e.g. VLAN) is present, kernel stack always performs software
> > resegmentation which actually kills the performance.
> > Despite the fact that no mainline drivers currently supports fraglist GSO,
> > this should and can be easily fixed by adding UDP L4 and fraglist GSO to
> > the list of GSO types that can be passed-through the logical interfaces
> > (NETIF_F_GSO_SOFTWARE). After this change, no resegmentation occurs (if
> > a particular driver supports and advertises this), and the performance
> > goes on par with e.g. 1:1 forwarding.
> > The only logical netdevs that seem to be unaffected to this are bridge
> > interfaces, as their code uses full NETIF_F_GSO_MASK.
> > Tested on MIPS32 R2 router board with a WIP NIC driver in VLAN NAT:
> > 20 Mbps baseline, 1 Gbps / link speed with this patch.
> >
> > Signed-off-by: Alexander Lobakin alobakin@xxxxx
> >
> > ------------------------------------------------
> >
> > include/linux/netdev_features.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> > index 0b17c4322b09..934de56644e7 100644
> > --- a/include/linux/netdev_features.h
> > +++ b/include/linux/netdev_features.h
> > @@ -207,8 +207,8 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
> > NETIF_F_FSO)
> > /* List of features with software fallbacks. */
> > -#define NETIF_F_GSO_SOFTWARE (NETIF_F_ALL_TSO | \
> >
> > -                                  NETIF_F_GSO_SCTP)
> >
> >
> >
> > +#define NETIF_F_GSO_SOFTWARE (NETIF_F_ALL_TSO | NETIF_F_GSO_SCTP | \
> >
> > -                                  NETIF_F_GSO_UDP_L4 | NETIF_F_GSO_FRAGLIST)
> >
> >
>
> What exactly do you mean by resegmenting?

I mean pts 5-6 from the full path:
1. Our NIC driver advertises a support for fraglists, GSO UDP L4, GSO fraglists.
2. User enables fraglisted GRO via Ethtool.
3. GRO subsystem receives UDP frames from driver and merges the packets into
   fraglisted GSO skb(s).
4. Networking stack queues it up for xmitting.
5. Virtual device like VLAN doesn't advertise a support for GSO UDP L4 and
   GSO fraglists, so skb_gso_check() doesn't allow to pass this skb as is to
   the real driver.
6. Kernel then has to form a bunch of regular UDP skbs from that one and pass
   it to the driver instead. This fallback is *extremely* slow for any GSO types,
   but especially for GSO fraglists.
7. All further processing performs with a series of plain UDP skbs, and the
   driver gets it one-by-one, despite that it supports UDP L4 and fraglisted GSO.

That's not OK because:
a) logical/virtual netdevs like VLANs, bridges etc. should pass GSO skbs as is;
b) even if the final driver doesn't support such type of GSO, this software
   resegmenting should be performed right before it, not in the middle of
   processing -- I think I even saw that note somewhere in kernel documentation,
   and it's totally reasonable in terms of performance.

> I think it is fine to reenable this again, now that UDP sockets will
> segment unexpected UDP GSO packets that may have looped. We previously
> added general software support in commit 83aa025f535f ("udp: add gso
> support to virtual devices"). Then reduced its scope to egress only in
> 8eea1ca82be9 ("gso: limit udp gso to egress-only virtual devices") to
> handle that edge case.
>
> If we can enable for all virtual devices again, we could revert those
> device specific options.

Thanks,
Al