netdev - Re: [PATCH] tcp: fixes for DSACK-based undo of cwnd reduction during fast recovery

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 8 Nov 2011 17:18:34 -0800
From:	Yuchung Cheng <ycheng@...gle.com>
To:	Neal Cardwell <ncardwell@...gle.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	ilpo.jarvinen@...sinki.fi, Nandita Dukkipati <nanditad@...gle.com>,
	Tom Herbert <therbert@...gle.com>
Subject: Re: [PATCH] tcp: fixes for DSACK-based undo of cwnd reduction during
 fast recovery

On Tue, Nov 8, 2011 at 10:07 AM, Neal Cardwell <ncardwell@...gle.com> wrote:
> Fixes for some issues that prevent DSACKs from allowing TCP senders to
> undo cwnd reductions made during fast recovery.
>
> There were a few related bugs/issues:
>
> 1) Senders ignored DSACKs after recovery when there were no
> outstanding packets (a common scenario for HTTP servers).
>
> 2) When the ACK field is below snd_una (which can happen when ACKs are
> reordered), senders ignored DSACKs (preventing undo) and passed up
> chances to send out more packets based on any newly-SACKed packets.
>
> 3) Senders were overriding cwnd values picked during an undo by
> calling tcp_moderate_cwnd() in tcp_try_to_open().
>
> The fixes:
>
> (1) When there are no outstanding packets (the "no_queue" goto label),
> use DSACKs to undo congestion window reductions.
>
> (2) When the ACK field is below snd_una (the "old_ack" goto label),
> process any DSACKs and try to send out more packets based on
> newly-SACKed packets.
>
> (3) Don't moderate cwnd in tcp_try_to_open() if we're in TCP_CA_Open,
> since doing so is generally unnecessary and specifically would
> override a DSACK-based undo of a cwnd reduction made in fast recovery.
>
> (4) Simplify the congestion avoidance state machine by removing the
> behavior where SACK-enabled connections hung around in the
> TCP_CA_Disorder state just waiting for DSACKs. Instead, when snd_una
> advances to high_seq or beyond we typically move to TCP_CA_Open
> immediately and allow an undo in either TCP_CA_Open or TCP_CA_Disorder
> if we later receive enough DSACKs. Previously, SACK-enabled
> connections hung around in TCP_CA_Disorder state while
> snd_una==high_seq, just waiting to accumulate DSACKs and hopefully
> undo a cwnd reduction. This could and did lead to the following
> unfortunate scenario: if some incoming ACKs advance snd_una beyond
> high_seq then we were setting undo_marker to 0 and moving to
> TCP_CA_Open, so if (due to reordering in the ACK return path) we
> shortly thereafter received a DSACK then we were no longer able to
> undo the cwnd reduction.
>
> Signed-off-by: Neal Cardwell <ncardwell@...gle.com>
Acked-by: Yuchung Cheng <ycheng@...gle.com>


FWIW. This undo-fix patch on Google Web servers increased the undos in
loss by 46% and in disorder by 17%. It also corrects the SNMP stats
TCPTimeouts, TCPRenoFailures, TCPSackFailures by moving state into
open, instead of disorder, after recovery.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html