On Fri, 2021-07-02 at 17:23 +0200, Matthias Treydte wrote:
Quoting Paolo Abeni pabeni@redhat.com:
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index 54e06b88af69..458c888337a5 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -526,6 +526,8 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb, if ((!sk && (skb->dev->features & NETIF_F_GRO_UDP_FWD)) || (sk && udp_sk(sk)->gro_enabled) || NAPI_GRO_CB(skb)->is_flist) pp = call_gro_receive(udp_gro_receive_segment, head, skb);
elsegoto out; return pp; }Impressive! This patch, applied to 5.13, fixes the problem. What I like even more is that it again confirms my suspicion that an "if" without an "else" is always a code smell. :-)
Thank you for the quick feedback! I'll submit formally soon, after more tests. I'll probably change the code to be something hopefully more readable, as follow:
--- diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index 7a670757f37a..b3aabc886763 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -551,8 +551,10 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
if ((!sk && (skb->dev->features & NETIF_F_GRO_UDP_FWD)) || (sk && udp_sk(sk)->gro_enabled) || NAPI_GRO_CB(skb)->is_flist) - pp = call_gro_receive(udp_gro_receive_segment, head, skb); - return pp; + return call_gro_receive(udp_gro_receive_segment, head, skb); + + /* no GRO, be sure flush the current packet */ + goto out; } ---
With this and the reproducer in my previous mail, is there still value in doing the "perf" stuff?
Not needed, thank you!
Would be great instead if you could have a spin to the proposed variant above - not stritly needed, I'm really asking for a few extra miles here ;)
Cheers,
Paolo