Date: Wed, 30 Oct 2013 11:53:31 +0800 From: Julian Elischer <julian@freebsd.org> To: Andre Oppermann <andre@FreeBSD.org>, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: Re: svn commit: r257367 - stable/10/sys/netinet Message-ID: <527082BB.5000300@freebsd.org> In-Reply-To: <201310292100.r9TL0sRR034734@svn.freebsd.org> References: <201310292100.r9TL0sRR034734@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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; >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?527082BB.5000300>