From patchwork Tue Nov 10 00:17:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Lobakin X-Patchwork-Id: 1397187 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=pm.me Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; secure) header.d=pm.me header.i=@pm.me header.a=rsa-sha256 header.s=protonmail header.b=QVw7gVxX; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4CVT2q2jy4z9sRK for ; Tue, 10 Nov 2020 11:17:39 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730459AbgKJAR3 (ORCPT ); Mon, 9 Nov 2020 19:17:29 -0500 Received: from mail-40131.protonmail.ch ([185.70.40.131]:38620 "EHLO mail-40131.protonmail.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727311AbgKJAR2 (ORCPT ); Mon, 9 Nov 2020 19:17:28 -0500 X-Greylist: delayed 10916 seconds by postgrey-1.27 at vger.kernel.org; Mon, 09 Nov 2020 19:17:26 EST Date: Tue, 10 Nov 2020 00:17:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pm.me; s=protonmail; t=1604967445; bh=3r5eH4kmQg8GtWwXBLWSLr+/uoVW1i5B4scSDxo7/Fw=; h=Date:To:From:Cc:Reply-To:Subject:From; b=QVw7gVxXrBphejmocuPxl0Fslxe9chWynYwC/lwOmLn4TZYfcqgVIQSMJj7ScX7Yz k18zKmyC5mRSgQj6MU5XkBP/yGUPhXlDOcAhgQ6XnvLiExj7NrG9drVX7AL0I2M3yU osoCI/jeVvyiieDEYqv9FcJJH3kdInHG4MGn4EMqubXXlWuwXjhbTM21PgDNy2dzUO AoWNa7W4LK0pQNZc33TcV3BPdIqMuYHRV2Wk+cxTShDgL1soCRKb6vIhIvQs5iEjgK sbgjvHg5mwB4ti4FFlDLj9n7ynwXiDobV2VoprwF9G4ntd2zGYX/sOEMRGwBM3vzrk bnv7HM102/9Xg== To: "David S. Miller" , Jakub Kicinski From: Alexander Lobakin Cc: Alexey Kuznetsov , Hideaki YOSHIFUJI , Paolo Abeni , Willem de Bruijn , Steffen Klassert , Alexander Lobakin , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Eric Dumazet Reply-To: Alexander Lobakin Subject: [PATCH v4 net] net: udp: fix Fast/frag0 UDP GRO Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-1.2 required=10.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=disabled version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on mailout.protonmail.ch Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org While testing UDP GSO fraglists forwarding through driver that uses Fast GRO (via napi_gro_frags()), I was observing lots of out-of-order iperf packets: [ ID] Interval Transfer Bitrate Jitter [SUM] 0.0-40.0 sec 12106 datagrams received out-of-order Simple switch to napi_gro_receive() or any other method without frag0 shortcut completely resolved them. I've found that UDP GRO uses udp_hdr(skb) in its .gro_receive() callback. While it's probably OK for non-frag0 paths (when all headers or even the entire frame are already in skb->data), this inline points to junk when using Fast GRO (napi_gro_frags() or napi_gro_receive() with only Ethernet header in skb->data and all the rest in shinfo->frags) and breaks GRO packet compilation and the packet flow itself. To support both modes, skb_gro_header_fast() + skb_gro_header_slow() are typically used. UDP even has an inline helper that makes use of them, udp_gro_udphdr(). Use that instead of troublemaking udp_hdr() to get rid of the out-of-order delivers. Present since the introduction of plain UDP GRO in 5.0-rc1. Since v3 [1]: - restore the original {,__}udp{4,6}_lib_lookup_skb() and use private versions of them inside GRO code (Willem). Since v2 [2]: - dropped redundant check introduced in v2 as it's performed right before (thanks to Eric); - udp_hdr() switched to data + off for skbs from list (also Eric); - fixed possible malfunction of {,__}udp{4,6}_lib_lookup_skb() with Fast/frag0 due to ip{,v6}_hdr() usage (Willem). Since v1 [3]: - added a NULL pointer check for "uh" as suggested by Willem. [1] https://lore.kernel.org/netdev/MgZce9htmEtCtHg7pmWxXXfdhmQ6AHrnltXC41zOoo@cp7-web-042.plabs.ch [2] https://lore.kernel.org/netdev/0eaG8xtbtKY1dEKCTKUBubGiC9QawGgB3tVZtNqVdY@cp4-web-030.plabs.ch [3] https://lore.kernel.org/netdev/YazU6GEzBdpyZMDMwJirxDX7B4sualpDG68ADZYvJI@cp4-web-034.plabs.ch Fixes: e20cf8d3f1f7 ("udp: implement GRO for plain UDP sockets.") Cc: Eric Dumazet Cc: Willem de Bruijn Signed-off-by: Alexander Lobakin --- net/ipv4/udp_offload.c | 23 +++++++++++++++++++---- net/ipv6/udp_offload.c | 14 +++++++++++++- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index e67a66fbf27b..6064efe17cdb 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -366,11 +366,11 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb, static struct sk_buff *udp_gro_receive_segment(struct list_head *head, struct sk_buff *skb) { - struct udphdr *uh = udp_hdr(skb); + struct udphdr *uh = udp_gro_udphdr(skb); struct sk_buff *pp = NULL; struct udphdr *uh2; struct sk_buff *p; - unsigned int ulen; + u32 ulen, off; int ret = 0; /* requires non zero csum, for symmetry with GSO */ @@ -385,6 +385,9 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head, NAPI_GRO_CB(skb)->flush = 1; return NULL; } + + off = skb_gro_offset(skb); + /* pull encapsulating udp header */ skb_gro_pull(skb, sizeof(struct udphdr)); @@ -392,7 +395,7 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head, if (!NAPI_GRO_CB(p)->same_flow) continue; - uh2 = udp_hdr(p); + uh2 = (void *)p->data + off; /* Match ports only, as csum is always non zero */ if ((*(u32 *)&uh->source != *(u32 *)&uh2->source)) { @@ -500,6 +503,16 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb, } EXPORT_SYMBOL(udp_gro_receive); +static struct sock *udp4_gro_lookup_skb(struct sk_buff *skb, __be16 sport, + __be16 dport) +{ + const struct iphdr *iph = skb_gro_network_header(skb); + + return __udp4_lib_lookup(dev_net(skb->dev), iph->saddr, sport, + iph->daddr, dport, inet_iif(skb), + inet_sdif(skb), &udp_table, NULL); +} + INDIRECT_CALLABLE_SCOPE struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb) { @@ -523,7 +536,9 @@ struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb) skip: NAPI_GRO_CB(skb)->is_ipv6 = 0; rcu_read_lock(); - sk = static_branch_unlikely(&udp_encap_needed_key) ? udp4_lib_lookup_skb(skb, uh->source, uh->dest) : NULL; + sk = static_branch_unlikely(&udp_encap_needed_key) ? + udp4_gro_lookup_skb(skb, uh->source, uh->dest) : + NULL; pp = udp_gro_receive(head, skb, uh, sk); rcu_read_unlock(); return pp; diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c index 584157a07759..b126ab2120d9 100644 --- a/net/ipv6/udp_offload.c +++ b/net/ipv6/udp_offload.c @@ -111,6 +111,16 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb, return segs; } +static struct sock *udp6_gro_lookup_skb(struct sk_buff *skb, __be16 sport, + __be16 dport) +{ + const struct ipv6hdr *iph = skb_gro_network_header(skb); + + return __udp6_lib_lookup(dev_net(skb->dev), &iph->saddr, sport, + &iph->daddr, dport, inet6_iif(skb), + inet6_sdif(skb), &udp_table, NULL); +} + INDIRECT_CALLABLE_SCOPE struct sk_buff *udp6_gro_receive(struct list_head *head, struct sk_buff *skb) { @@ -135,7 +145,9 @@ struct sk_buff *udp6_gro_receive(struct list_head *head, struct sk_buff *skb) skip: NAPI_GRO_CB(skb)->is_ipv6 = 1; rcu_read_lock(); - sk = static_branch_unlikely(&udpv6_encap_needed_key) ? udp6_lib_lookup_skb(skb, uh->source, uh->dest) : NULL; + sk = static_branch_unlikely(&udpv6_encap_needed_key) ? + udp6_gro_lookup_skb(skb, uh->source, uh->dest) : + NULL; pp = udp_gro_receive(head, skb, uh, sk); rcu_read_unlock(); return pp;