Re: [PATCH RFC 3/3] udp: Support UDP fraglist GRO/GSO.
From: Paolo Abeni <pabeni@redhat.com>
Date: 2019-01-08 15:00:04
Subsystem:
networking [general], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
On Fri, 2018-12-21 at 08:53 +0100, Steffen Klassert wrote:
quoted hunk ↗ jump to hunk
@@ -403,10 +428,17 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb, rcu_read_lock(); sk = (*lookup)(skb, uh->source, uh->dest); - if (!sk) - goto out_unlock; + if (!sk) { + NAPI_GRO_CB(skb)->is_flist = 1; + pp = call_gro_receive(udp_gro_receive_segment, head, skb); + rcu_read_unlock(); + return pp; + } + + if (!udp_sk(sk)->gro_receive) { + if (!udp_sk(sk)->gro_enabled) + NAPI_GRO_CB(skb)->is_flist = 1; - if (udp_sk(sk)->gro_enabled) { pp = call_gro_receive(udp_gro_receive_segment, head, skb); rcu_read_unlock(); return pp;
I think we could still avoid the lookup when no vxlan/GRO sockets are
present moving the lookup into udp{4,6}_gro_receive. Very roughly
something alike:
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index f79f1b5b2f9e..b0c0983eac6b 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c@@ -420,20 +420,16 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head, INDIRECT_CALLABLE_DECLARE(struct sock *udp6_lib_lookup_skb(struct sk_buff *skb, __be16 sport, __be16 dport)); struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb, - struct udphdr *uh, udp_lookup_t lookup) + struct udphdr *uh, struct sock *sk) { struct sk_buff *pp = NULL; struct sk_buff *p; struct udphdr *uh2; unsigned int off = skb_gro_offset(skb); int flush = 1; - struct sock *sk; - rcu_read_lock(); - sk = INDIRECT_CALL_INET(lookup, udp6_lib_lookup_skb, - udp4_lib_lookup_skb, skb, uh->source, uh->dest); - if (!sk) { - NAPI_GRO_CB(skb)->is_flist = 1; + if (!sk || !udp_sk(sk)->gro_receive) { + NAPI_GRO_CB(skb)->is_flist = sk ? !udp_sk(sk)->gro_enabled: 1; pp = call_gro_receive(udp_gro_receive_segment, head, skb); rcu_read_unlock(); return pp;
@@ -506,7 +502,12 @@ struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb) inet_gro_compute_pseudo); skip: NAPI_GRO_CB(skb)->is_ipv6 = 0; - return udp_gro_receive(head, skb, uh, udp4_lib_lookup_skb); + rcu_read_lock(); + sk = static_branch_unlikely(&udp_encap_needed_key) ? + udp4_lib_lookup_skb(skb, uh->source, uh->dest) : NULL; + pp = udp_gro_receive(head, skb, uh, sk); + rcu_read_unlock(); + return pp; flush: NAPI_GRO_CB(skb)->flush = 1; ---
Regardless of the above, I think we should drop the later check for gro_receive:
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c@@ -450,8 +450,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb, if (NAPI_GRO_CB(skb)->encap_mark || (skb->ip_summed != CHECKSUM_PARTIAL && NAPI_GRO_CB(skb)->csum_cnt == 0 && - !NAPI_GRO_CB(skb)->csum_valid) || - !udp_sk(sk)->gro_receive) + !NAPI_GRO_CB(skb)->csum_valid)) goto out_unlock; /* mark that this skb passed once through the tunnel gro layer */ ---
Finally this will cause GRO/GSO for local UDP packets delivery to non GSO_SEGMENT sockets. That could be possibly a win or a regression: we save on netfilter/IP stack traversal, but we add additional work, some performances figures would probably help. Cheers, Paolo