From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH v3 net-next] tcp: TSO packets automatic sizing Date: Wed, 28 Aug 2013 03:34:44 -0700 Message-ID: <1377686084.8828.175.camel@edumazet-glaptop> References: <1377304192.8828.43.camel@edumazet-glaptop> <1377491162.8828.116.camel@edumazet-glaptop> <1377607592.8828.149.camel@edumazet-glaptop> <521DA8BE.3060709@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: David Miller , netdev , Neal Cardwell , Yuchung Cheng , Van Jacobson , Tom Herbert , "Michael S. Tsirkin" To: Jason Wang Return-path: Received: from mail-pd0-f180.google.com ([209.85.192.180]:39410 "EHLO mail-pd0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752523Ab3H1Keq (ORCPT ); Wed, 28 Aug 2013 06:34:46 -0400 Received: by mail-pd0-f180.google.com with SMTP id y10so6067439pdj.39 for ; Wed, 28 Aug 2013 03:34:46 -0700 (PDT) In-Reply-To: <521DA8BE.3060709@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 2013-08-28 at 15:37 +0800, Jason Wang wrote: > > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c > > index 884efff..e63ae4c 100644 > > --- a/net/ipv4/tcp_output.c > > +++ b/net/ipv4/tcp_output.c > > @@ -1631,7 +1631,7 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb) > > > > /* If a full-sized TSO skb can be sent, do it. */ > > if (limit >= min_t(unsigned int, sk->sk_gso_max_size, > > - sk->sk_gso_max_segs * tp->mss_cache)) > > + tp->xmit_size_goal_segs * tp->mss_cache)) > > goto send_now; > A question is: Does this really guarantee the minimal TSO segments > excluding the case of small available window? The skb->len may be much > smaller and can still be sent here. Maybe we should check skb->len also? tcp_tso_should_defer() is all about hoping the application will 'complete' the last skb in write queue with more payload in the near future. skb->len might therefore change because sendmsg()/sendpage() will add new stuff in the skb. We try hard to not remove tcp_tso_should_defer() and take the best of it. We have not yet decided to add a real timer instead of relying on upcoming ACKS. Neal has an idea/patch to avoid a defer depending on the expected time of following ACKS. By making the TSO sizes smaller for low rates, we avoid these stalls from tcp_tso_should_defer(), because an incoming ACK has normally freed enough window to send the next packet in write queue without the need to split it into two parts. These changes are fundamental to use delay based congestion modules like Vegas/Westwood and experimental new ones, without having to disable TSO.