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: 1254 bytes --]

On Mon, 12 May 2008, Damon L. Chesser wrote:

> I applied the patches in order, no errors on that.  I compiled a stock
> 2.4.24-1 kernel with the patches, I saw no errors there.
> 
> booted into new kernel, printed with tcp_frto=0.  set tcp_frto =2, restarted
> the network (is this required, or is this a dynamic setting?), printed from OO
> document.  No joy.  tcpdump log attached (almost 15 min. worth of data)
> 
> If you want, I can re-compile and double check for any compilation errors,
> however, if there were any, it was not sever enough to stop the compilation.

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.

...Can you please send a dump about working case too, this seems rather 
nasty device to work with (tcp_frto = 0 is enough to attain it, no 
need to have another kernel booted for that) and I'm interested to see 
what are the loss rates without FRTO...


-- 
 i.

[-- Attachment #2: Type: text/plain, Size: 3017 bytes --]

From 1fbcd93fe81a2276e9df12fe575f33df4dbfb312 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       |    4 +++-
 3 files changed, 12 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..0292070 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);
 		}
-- 
1.5.2.2