Re: Fix FRTO+NewReno problem (Was: Re: This has a work around)

["Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 595 bytes --]

On Mon, 12 May 2008, Ilpo Järvinen wrote:

> On the bright side, the FRTO problem that was occuring previously is now fixed
> but there seems to be very few ways to communicate with that device sanely
> because it assumes in-order arrival and keeps discarding, as it seems, _all_
> other segments... If you could try with this additional work-around attached
> (keep the fixes there as well). Turn tcp_frto_inorder_workaround sysctl to 1
> before testing with FRTO.

I missed one thing with the last workaround patch, please use this one 
instead if you haven't yet have time to try it.

-- 
 i.

[-- Attachment #2: Type: TEXT/PLAIN, Size: 3378 bytes --]

From 9bdbbca4162ba89f0be27a74d69b39af95cf2c99 Mon Sep 17 00:00:00 2001
From: =?ISO-8859-1?q?Ilpo=20J=E4rvinen?= <ilpo.jarvinen@helsinki.fi>
Date: Mon, 12 May 2008 17:24:02 +0300
Subject: [PATCH] [TCP] FRTO: workaround in-order only receivers
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit

If receiver consumes segments successfully only in-order,
FRTO fallback to conventional recovery produces RTO loop because
FRTO's forward transmissions will always get dropped and need to
be resent, yet by default they're not marked as lost (which are
the only segments we will retransmit in CA_Loss).

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Reported-by: Damon L. Chesser <damon@damtek.com>
---
 include/net/tcp.h          |    1 +
 net/ipv4/sysctl_net_ipv4.c |    8 ++++++++
 net/ipv4/tcp_input.c       |    6 +++++-
 3 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index cb5b033..e6e44b8 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -222,6 +222,7 @@ extern int sysctl_tcp_adv_win_scale;
 extern int sysctl_tcp_tw_reuse;
 extern int sysctl_tcp_frto;
 extern int sysctl_tcp_frto_response;
+extern int sysctl_tcp_frto_inorder_workaround;
 extern int sysctl_tcp_low_latency;
 extern int sysctl_tcp_dma_copybreak;
 extern int sysctl_tcp_nometrics_save;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index bec6fe8..9cbdccf 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -723,6 +723,14 @@ ctl_table ipv4_table[] = {
 		.proc_handler	= &proc_dointvec
 	},
 	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "tcp_frto_inorder_workaround",
+		.data		= &sysctl_tcp_frto_inorder_workaround,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec
+	},	
+	{
 		.ctl_name	= NET_TCP_LOW_LATENCY,
 		.procname	= "tcp_low_latency",
 		.data		= &sysctl_tcp_low_latency,
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b39f0d8..fbab150 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -87,6 +87,7 @@ int sysctl_tcp_rfc1337 __read_mostly;
 int sysctl_tcp_max_orphans __read_mostly = NR_FILE;
 int sysctl_tcp_frto __read_mostly = 2;
 int sysctl_tcp_frto_response __read_mostly;
+int sysctl_tcp_frto_inorder_workaround __read_mostly;
 int sysctl_tcp_nometrics_save __read_mostly;
 
 int sysctl_tcp_moderate_rcvbuf __read_mostly = 1;
@@ -1729,7 +1730,8 @@ static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
 
 		/* Don't lost mark skbs that were fwd transmitted after RTO */
 		if (!(TCP_SKB_CB(skb)->sacked&TCPCB_SACKED_ACKED) &&
-		    !after(TCP_SKB_CB(skb)->end_seq, tp->frto_highmark)) {
+		    (sysctl_tcp_frto_inorder_workaround ||
+		     !after(TCP_SKB_CB(skb)->end_seq, tp->frto_highmark))) {
 			TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
 			tp->lost_out += tcp_skb_pcount(skb);
 		}
@@ -1746,6 +1748,8 @@ static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
 					     sysctl_tcp_reordering);
 	tcp_set_ca_state(sk, TCP_CA_Loss);
 	tp->high_seq = tp->frto_highmark;
+	if (sysctl_tcp_frto_inorder_workaround)
+		tp->high_seq = tp->snd_nxt;
 	TCP_ECN_queue_cwr(tp);
 
 	tcp_clear_retrans_hints_partial(tp);
-- 
1.5.2.2