From mboxrd@z Thu Jan 1 00:00:00 1970 From: Willem de Bruijn Subject: Re: [RFC PATCH v2 02/10] udp: implement GRO for plain UDP sockets. Date: Sun, 21 Oct 2018 16:06:28 -0400 Message-ID: References: <3fa3822651e29b8484d598b10ae61b0efde6b14f.1539957909.git.pabeni@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Network Development , Willem de Bruijn , steffen.klassert@secunet.com To: Paolo Abeni Return-path: Received: from mail-ed1-f68.google.com ([209.85.208.68]:35934 "EHLO mail-ed1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727367AbeJVEWe (ORCPT ); Mon, 22 Oct 2018 00:22:34 -0400 Received: by mail-ed1-f68.google.com with SMTP id x2-v6so1065894eds.3 for ; Sun, 21 Oct 2018 13:07:04 -0700 (PDT) In-Reply-To: <3fa3822651e29b8484d598b10ae61b0efde6b14f.1539957909.git.pabeni@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, Oct 19, 2018 at 10:30 AM Paolo Abeni wrote: > > This is the RX counterpart of commit bec1f6f69736 ("udp: generate gso > with UDP_SEGMENT"). When UDP_GRO is enabled, such socket is also > eligible for GRO in the rx path: UDP segments directed to such socket > are assembled into a larger GSO_UDP_L4 packet. > > The core UDP GRO support is enabled with setsockopt(UDP_GRO). > > Initial benchmark numbers: > > Before: > udp rx: 1079 MB/s 769065 calls/s > > After: > udp rx: 1466 MB/s 24877 calls/s > > > This change introduces a side effect in respect to UDP tunnels: > after a UDP tunnel creation, now the kernel performs a lookup per ingress > UDP packet, while before such lookup happened only if the ingress packet > carried a valid internal header csum. > > v1 -> v2: > - use a new option to enable UDP GRO > - use static keys to protect the UDP GRO socket lookup > > Signed-off-by: Paolo Abeni > --- > include/linux/udp.h | 3 +- > include/uapi/linux/udp.h | 1 + > net/ipv4/udp.c | 7 +++ > net/ipv4/udp_offload.c | 109 +++++++++++++++++++++++++++++++-------- > net/ipv6/udp_offload.c | 6 +-- > 5 files changed, 98 insertions(+), 28 deletions(-) > > diff --git a/include/linux/udp.h b/include/linux/udp.h > index a4dafff407fb..f613b329852e 100644 > --- a/include/linux/udp.h > +++ b/include/linux/udp.h > @@ -50,11 +50,12 @@ struct udp_sock { > __u8 encap_type; /* Is this an Encapsulation socket? */ > unsigned char no_check6_tx:1,/* Send zero UDP6 checksums on TX? */ > no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */ > - encap_enabled:1; /* This socket enabled encap > + encap_enabled:1, /* This socket enabled encap > * processing; UDP tunnels and > * different encapsulation layer set > * this > */ > + gro_enabled:1; /* Can accept GRO packets */ > > /* > * Following member retains the information to create a UDP header > * when the socket is uncorked. > diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h > index 09502de447f5..30baccb6c9c4 100644 > --- a/include/uapi/linux/udp.h > +++ b/include/uapi/linux/udp.h > @@ -33,6 +33,7 @@ struct udphdr { > #define UDP_NO_CHECK6_TX 101 /* Disable sending checksum for UDP6X */ > #define UDP_NO_CHECK6_RX 102 /* Disable accpeting checksum for UDP6 */ > #define UDP_SEGMENT 103 /* Set GSO segmentation size */ > +#define UDP_GRO 104 /* This socket can receive UDP GRO packets */ > > /* UDP encapsulation types */ > #define UDP_ENCAP_ESPINUDP_NON_IKE 1 /* draft-ietf-ipsec-nat-t-ike-00/01 */ > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index 9fcb5374e166..3c277378814f 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -115,6 +115,7 @@ > #include "udp_impl.h" > #include > #include > +#include > > struct udp_table udp_table __read_mostly; > EXPORT_SYMBOL(udp_table); > @@ -2459,6 +2460,12 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname, > up->gso_size = val; > break; > > + case UDP_GRO: > + if (valbool) > + udp_tunnel_encap_enable(sk->sk_socket); > + up->gro_enabled = valbool; The socket lock is not held here, so multiple updates to up->gro_enabled and the up->encap_enabled and the static branch can race. Syzkaller is adept at generating those. > +#define UDO_GRO_CNT_MAX 64 > +static struct sk_buff *udp_gro_receive_segment(struct list_head *head, > + struct sk_buff *skb) > +{ > + struct udphdr *uh = udp_hdr(skb); > + struct sk_buff *pp = NULL; > + struct udphdr *uh2; > + struct sk_buff *p; > + > + /* requires non zero csum, for simmetry with GSO */ symmetry