From owner-svn-src-stable@FreeBSD.ORG Wed Oct 30 03:53:42 2013 Return-Path: Delivered-To: svn-src-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 453301E8; Wed, 30 Oct 2013 03:53:42 +0000 (UTC) (envelope-from julian@freebsd.org) Received: from vps1.elischer.org (vps1.elischer.org [204.109.63.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 089E5201A; Wed, 30 Oct 2013 03:53:41 +0000 (UTC) Received: from Julian-MBP3.local (ppp121-45-253-246.lns20.per2.internode.on.net [121.45.253.246]) (authenticated bits=0) by vps1.elischer.org (8.14.7/8.14.7) with ESMTP id r9U3rbJ9021685 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Tue, 29 Oct 2013 20:53:39 -0700 (PDT) (envelope-from julian@freebsd.org) Message-ID: <527082BB.5000300@freebsd.org> Date: Wed, 30 Oct 2013 11:53:31 +0800 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.0.1 MIME-Version: 1.0 To: Andre Oppermann , svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: Re: svn commit: r257367 - stable/10/sys/netinet References: <201310292100.r9TL0sRR034734@svn.freebsd.org> In-Reply-To: <201310292100.r9TL0sRR034734@svn.freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Oct 2013 03:53:42 -0000 I think 9 needs this too... On 10/30/13, 5:00 AM, Andre Oppermann wrote: > Author: andre > Date: Tue Oct 29 21:00:54 2013 > New Revision: 257367 > URL: http://svnweb.freebsd.org/changeset/base/257367 > > Log: > MFC r256920: > > The TCP delayed ACK logic isn't aware of LRO passing up large aggregated > segments thinking it received only one segment. This causes it to enable > the delay the ACK for 100ms to wait for another segment which may never > come because all the data was received already. > > Doing delayed ACK for LRO segments is bogus for two reasons: a) it pushes > us further away from acking every other packet; b) it introduces additional > delay in responding to the sender. The latter is especially bad because it > is in the nature of LRO to aggregated all segments of a burst with no more > coming until an ACK is sent back. > > Change the delayed ACK logic to detect LRO segments by being larger than > the MSS for this connection and issuing an immediate ACK for them to keep > the ACK clock ticking without interruption. > > Reported by: julian, cperciva > Tested by: cperciva > Reviewed by: lstewart > > Approved by: re (glebius) > > Modified: > stable/10/sys/netinet/tcp_input.c > Directory Properties: > stable/10/sys/ (props changed) > > Modified: stable/10/sys/netinet/tcp_input.c > ============================================================================== > --- stable/10/sys/netinet/tcp_input.c Tue Oct 29 20:53:09 2013 (r257366) > +++ stable/10/sys/netinet/tcp_input.c Tue Oct 29 21:00:54 2013 (r257367) > @@ -508,10 +508,13 @@ do { \ > * the ack that opens up a 0-sized window and > * - delayed acks are enabled or > * - this is a half-synchronized T/TCP connection. > + * - the segment size is not larger than the MSS and LRO wasn't used > + * for this segment. > */ > -#define DELAY_ACK(tp) \ > +#define DELAY_ACK(tp, tlen) \ > ((!tcp_timer_active(tp, TT_DELACK) && \ > (tp->t_flags & TF_RXWIN0SENT) == 0) && \ > + (tlen <= tp->t_maxopd) && \ > (V_tcp_delack_enabled || (tp->t_flags & TF_NEEDSYN))) > > /* > @@ -1863,7 +1866,7 @@ tcp_do_segment(struct mbuf *m, struct tc > } > /* NB: sorwakeup_locked() does an implicit unlock. */ > sorwakeup_locked(so); > - if (DELAY_ACK(tp)) { > + if (DELAY_ACK(tp, tlen)) { > tp->t_flags |= TF_DELACK; > } else { > tp->t_flags |= TF_ACKNOW; > @@ -1954,7 +1957,7 @@ tcp_do_segment(struct mbuf *m, struct tc > * If there's data, delay ACK; if there's also a FIN > * ACKNOW will be turned on later. > */ > - if (DELAY_ACK(tp) && tlen != 0) > + if (DELAY_ACK(tp, tlen) && tlen != 0) > tcp_timer_activate(tp, TT_DELACK, > tcp_delacktime); > else > @@ -2926,7 +2929,7 @@ dodata: /* XXX */ > if (th->th_seq == tp->rcv_nxt && > LIST_EMPTY(&tp->t_segq) && > TCPS_HAVEESTABLISHED(tp->t_state)) { > - if (DELAY_ACK(tp)) > + if (DELAY_ACK(tp, tlen)) > tp->t_flags |= TF_DELACK; > else > tp->t_flags |= TF_ACKNOW; >