Skip to content

Commit 5861c8c

Browse files
committed
Merge branch 'net-udp-fix-fast-frag0-udp-gro'
Alexander Lobakin says: ==================== net: udp: fix Fast/frag0 UDP GRO 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 two incorrect header accesses in GRO receive callback(s): - udp_hdr() (instead of udp_gro_udphdr()) that always points to junk in "fast" mode and could probably do this in "regular". This was the actual bug that caused all out-of-order delivers; - udp{4,6}_lib_lookup_skb() -> ip{,v6}_hdr() (instead of skb_gro_network_header()) that potentionally might return odd pointers in both modes. Each patch addresses one of these two issues. This doesn't cover a support for nested tunnels as it's out of the subject and requires more invasive changes. It will be handled separately in net-next series. Credits: Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Willem de Bruijn <willemb@google.com> Since v4 [0]: - split the fix into two logical ones (Willem); - replace ternaries with plain ifs to beautify the code (Jakub); - drop p->data part to reintroduce it later in abovementioned set. 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. [0] https://lore.kernel.org/netdev/Ha2hou5eJPcblo4abjAqxZRzIl1RaLs2Hy0oOAgFs@cp4-web-036.plabs.ch [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 ==================== Link: https://lore.kernel.org/r/hjGOh0iCOYyo1FPiZh6TMXcx3YCgNs1T1eGKLrDz8@cp4-web-037.plabs.ch Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 8a5c290 + 55e7298 commit 5861c8c

2 files changed

Lines changed: 31 additions & 5 deletions

File tree

net/ipv4/udp_offload.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
366366
static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
367367
struct sk_buff *skb)
368368
{
369-
struct udphdr *uh = udp_hdr(skb);
369+
struct udphdr *uh = udp_gro_udphdr(skb);
370370
struct sk_buff *pp = NULL;
371371
struct udphdr *uh2;
372372
struct sk_buff *p;
@@ -500,12 +500,22 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
500500
}
501501
EXPORT_SYMBOL(udp_gro_receive);
502502

503+
static struct sock *udp4_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
504+
__be16 dport)
505+
{
506+
const struct iphdr *iph = skb_gro_network_header(skb);
507+
508+
return __udp4_lib_lookup(dev_net(skb->dev), iph->saddr, sport,
509+
iph->daddr, dport, inet_iif(skb),
510+
inet_sdif(skb), &udp_table, NULL);
511+
}
512+
503513
INDIRECT_CALLABLE_SCOPE
504514
struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb)
505515
{
506516
struct udphdr *uh = udp_gro_udphdr(skb);
517+
struct sock *sk = NULL;
507518
struct sk_buff *pp;
508-
struct sock *sk;
509519

510520
if (unlikely(!uh))
511521
goto flush;
@@ -523,7 +533,10 @@ struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb)
523533
skip:
524534
NAPI_GRO_CB(skb)->is_ipv6 = 0;
525535
rcu_read_lock();
526-
sk = static_branch_unlikely(&udp_encap_needed_key) ? udp4_lib_lookup_skb(skb, uh->source, uh->dest) : NULL;
536+
537+
if (static_branch_unlikely(&udp_encap_needed_key))
538+
sk = udp4_gro_lookup_skb(skb, uh->source, uh->dest);
539+
527540
pp = udp_gro_receive(head, skb, uh, sk);
528541
rcu_read_unlock();
529542
return pp;

net/ipv6/udp_offload.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,22 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
111111
return segs;
112112
}
113113

114+
static struct sock *udp6_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
115+
__be16 dport)
116+
{
117+
const struct ipv6hdr *iph = skb_gro_network_header(skb);
118+
119+
return __udp6_lib_lookup(dev_net(skb->dev), &iph->saddr, sport,
120+
&iph->daddr, dport, inet6_iif(skb),
121+
inet6_sdif(skb), &udp_table, NULL);
122+
}
123+
114124
INDIRECT_CALLABLE_SCOPE
115125
struct sk_buff *udp6_gro_receive(struct list_head *head, struct sk_buff *skb)
116126
{
117127
struct udphdr *uh = udp_gro_udphdr(skb);
128+
struct sock *sk = NULL;
118129
struct sk_buff *pp;
119-
struct sock *sk;
120130

121131
if (unlikely(!uh))
122132
goto flush;
@@ -135,7 +145,10 @@ struct sk_buff *udp6_gro_receive(struct list_head *head, struct sk_buff *skb)
135145
skip:
136146
NAPI_GRO_CB(skb)->is_ipv6 = 1;
137147
rcu_read_lock();
138-
sk = static_branch_unlikely(&udpv6_encap_needed_key) ? udp6_lib_lookup_skb(skb, uh->source, uh->dest) : NULL;
148+
149+
if (static_branch_unlikely(&udpv6_encap_needed_key))
150+
sk = udp6_gro_lookup_skb(skb, uh->source, uh->dest);
151+
139152
pp = udp_gro_receive(head, skb, uh, sk);
140153
rcu_read_unlock();
141154
return pp;

0 commit comments

Comments
 (0)