[PATCH net-next] tcp: reset reordering est. selectively on timeout

[PATCH net-next] tcp: reset reordering est. selectively on timeout

[Yuchung Cheng]

On timeout the TCP sender unconditionally reset the estimated
degree of network reordering (tp->reordering). This idea behind
is that the estimate is too large to trigger fast recovery
(e.g., due to a IP path change).

But for example if the sender only had 2 packets outstanding,
then a timeout doesn't tell much about reordering. A sender that
learns about reordering on big writes and lose packets on small
writes will end up falsely retransmit again and again, especially
when reordering is more likely on big writes.

Therefore the sender should only suspects tp->reordering is too
high if it could have gone into fast recovery with the (lower)
default estimate.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_input.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b61274b..e965cc7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1877,8 +1877,13 @@ void tcp_enter_loss(struct sock *sk, int how)
 	}
 	tcp_verify_left_out(tp);
 
-	tp->reordering = min_t(unsigned int, tp->reordering,
-			       sysctl_tcp_reordering);
+	/* Timeout in disordered state after receiving substantial DUPACKs
+	 * suggests that the degree of reordering is over-estimated.
+	 */
+	if (icsk->icsk_ca_state <= TCP_CA_Disorder &&
+	    tp->sacked_out >= sysctl_tcp_reordering)
+		tp->reordering = min_t(unsigned int, tp->reordering,
+				       sysctl_tcp_reordering);
 	tcp_set_ca_state(sk, TCP_CA_Loss);
 	tp->high_seq = tp->snd_nxt;
 	TCP_ECN_queue_cwr(tp);
-- 
1.8.3

Re: [PATCH net-next] tcp: reset reordering est. selectively on timeout

[Neal Cardwell]

On Mon, Aug 12, 2013 at 7:41 PM, Yuchung Cheng <ycheng@google.com> wrote:
> On timeout the TCP sender unconditionally reset the estimated
> degree of network reordering (tp->reordering). This idea behind
> is that the estimate is too large to trigger fast recovery
> (e.g., due to a IP path change).
>
> But for example if the sender only had 2 packets outstanding,
> then a timeout doesn't tell much about reordering. A sender that
> learns about reordering on big writes and lose packets on small
> writes will end up falsely retransmit again and again, especially
> when reordering is more likely on big writes.
>
> Therefore the sender should only suspects tp->reordering is too
> high if it could have gone into fast recovery with the (lower)
> default estimate.
>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> ---
>  net/ipv4/tcp_input.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Acked-by: Neal Cardwell <ncardwell@google.com>

neal

Re: [PATCH net-next] tcp: reset reordering est. selectively on timeout

[David Miller]

From: Neal Cardwell <ncardwell@google.com>
Date: Tue, 13 Aug 2013 00:16:57 -0400

> On Mon, Aug 12, 2013 at 7:41 PM, Yuchung Cheng <ycheng@google.com> wrote:
>> On timeout the TCP sender unconditionally reset the estimated
>> degree of network reordering (tp->reordering). This idea behind
>> is that the estimate is too large to trigger fast recovery
>> (e.g., due to a IP path change).
>>
>> But for example if the sender only had 2 packets outstanding,
>> then a timeout doesn't tell much about reordering. A sender that
>> learns about reordering on big writes and lose packets on small
>> writes will end up falsely retransmit again and again, especially
>> when reordering is more likely on big writes.
>>
>> Therefore the sender should only suspects tp->reordering is too
>> high if it could have gone into fast recovery with the (lower)
>> default estimate.
>>
>> Signed-off-by: Yuchung Cheng <ycheng@google.com>
>> ---
>>  net/ipv4/tcp_input.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> Acked-by: Neal Cardwell <ncardwell@google.com>

Applied, thanks.