Message ID | 1382747177.4032.21.camel@edumazet-glaptop.roam.corp.google.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, Oct 25, 2013 at 5:26 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > From: Eric Dumazet <edumazet@google.com> > > commit 6ff50cd55545 ("tcp: gso: do not generate out of order packets") > had an heuristic that can trigger a warning in skb_try_coalesce(), > because skb->truesize of the gso segments were exactly set to mss. > > This breaks the requirement that > > skb->truesize >= skb->len + truesizeof(struct sk_buff); > > It can trivially be reproduced by : > > ifconfig lo mtu 1500 > ethtool -K lo tso off > netperf > > As the skbs are looped into the TCP networking stack, skb_try_coalesce() > warns us of these skb under-estimating their truesize. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Alexei Starovoitov <ast@plumgrid.com> > --- it fixed it in my setup. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Fri, 25 Oct 2013 17:26:17 -0700 > From: Eric Dumazet <edumazet@google.com> > > commit 6ff50cd55545 ("tcp: gso: do not generate out of order packets") > had an heuristic that can trigger a warning in skb_try_coalesce(), > because skb->truesize of the gso segments were exactly set to mss. > > This breaks the requirement that > > skb->truesize >= skb->len + truesizeof(struct sk_buff); > > It can trivially be reproduced by : > > ifconfig lo mtu 1500 > ethtool -K lo tso off > netperf > > As the skbs are looped into the TCP networking stack, skb_try_coalesce() > warns us of these skb under-estimating their truesize. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Alexei Starovoitov <ast@plumgrid.com> > --- > Based on net-next but applies as well on net tree with some fuzz. > > David, we might backport this one to 3.12 and stable later, I let you > decide. I'm still thinking about what to do with this one. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Fri, 25 Oct 2013 17:26:17 -0700 > From: Eric Dumazet <edumazet@google.com> > > commit 6ff50cd55545 ("tcp: gso: do not generate out of order packets") > had an heuristic that can trigger a warning in skb_try_coalesce(), > because skb->truesize of the gso segments were exactly set to mss. > > This breaks the requirement that > > skb->truesize >= skb->len + truesizeof(struct sk_buff); > > It can trivially be reproduced by : > > ifconfig lo mtu 1500 > ethtool -K lo tso off > netperf > > As the skbs are looped into the TCP networking stack, skb_try_coalesce() > warns us of these skb under-estimating their truesize. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Alexei Starovoitov <ast@plumgrid.com> I decided to apply this to 'net' and queue it up for -stable, thanks Eric! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index a7a5583e..a2b68a1 100644 --- a/net/ipv4/tcp_offload.c +++ b/net/ipv4/tcp_offload.c @@ -18,6 +18,7 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, netdev_features_t features) { struct sk_buff *segs = ERR_PTR(-EINVAL); + unsigned int sum_truesize = 0; struct tcphdr *th; unsigned int thlen; unsigned int seq; @@ -104,13 +105,7 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, if (copy_destructor) { skb->destructor = gso_skb->destructor; skb->sk = gso_skb->sk; - /* {tcp|sock}_wfree() use exact truesize accounting : - * sum(skb->truesize) MUST be exactly be gso_skb->truesize - * So we account mss bytes of 'true size' for each segment. - * The last segment will contain the remaining. - */ - skb->truesize = mss; - gso_skb->truesize -= mss; + sum_truesize += skb->truesize; } skb = skb->next; th = tcp_hdr(skb); @@ -127,7 +122,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb, if (copy_destructor) { swap(gso_skb->sk, skb->sk); swap(gso_skb->destructor, skb->destructor); - swap(gso_skb->truesize, skb->truesize); + sum_truesize += skb->truesize; + atomic_add(sum_truesize - gso_skb->truesize, + &skb->sk->sk_wmem_alloc); } delta = htonl(oldlen + (skb_tail_pointer(skb) -