From patchwork Tue Jun 8 14:05:45 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Bader X-Patchwork-Id: 54978 X-Patchwork-Delegate: stefan.bader@canonical.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 27738B7D4A for ; Wed, 9 Jun 2010 00:06:05 +1000 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.69) (envelope-from ) id 1OLzRF-0004Eg-Bx; Tue, 08 Jun 2010 15:05:57 +0100 Received: from adelie.canonical.com ([91.189.90.139]) by chlorine.canonical.com with esmtp (Exim 4.69) (envelope-from ) id 1OLzRD-0004E5-Ga for kernel-team@lists.ubuntu.com; Tue, 08 Jun 2010 15:05:55 +0100 Received: from hutte.canonical.com ([91.189.90.181]) by adelie.canonical.com with esmtp (Exim 4.69 #1 (Debian)) id 1OLzRD-0000dF-F9 for ; Tue, 08 Jun 2010 15:05:55 +0100 Received: from [88.128.83.233] (helo=canonical.com) by hutte.canonical.com with esmtpsa (TLS-1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.69) (envelope-from ) id 1OLzRC-0003V1-RY for kernel-team@lists.ubuntu.com; Tue, 08 Jun 2010 15:05:55 +0100 From: Stefan Bader To: kernel-team@lists.ubuntu.com Subject: [PATCH 3/3] tcp FRTO: work-around inorder receivers Date: Tue, 8 Jun 2010 16:05:45 +0200 Message-Id: <1276005945-4096-4-git-send-email-stefan.bader@canonical.com> X-Mailer: git-send-email 1.7.0.4 In-Reply-To: <1276005945-4096-1-git-send-email-stefan.bader@canonical.com> References: <1276005945-4096-1-git-send-email-stefan.bader@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.9 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com From: Ilpo Järvinen BugLink: http://bugs.launchpad.net/bugs/567394 [ upstream commit: 79d44516b4b178ffb6e2159c75584cfcfc097914 ] 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). Price to pay about this is occassionally unnecessarily retransmitting the forward transmission(s). SACK blocks help a bit to avoid this, so it's mainly a concern for NewReno case though SACK is not fully immune either. This change has a side-effect of fixing SACKFRTO problem where it didn't have snd_nxt of the RTO time available anymore when fallback become necessary (this problem would have only occured when RTO would occur for two or more segments and ECE arrives in step 3; no need to figure out how to fix that unless the TODO item of selective behavior is considered in future). Signed-off-by: Ilpo Järvinen Reported-by: Damon L. Chesser Tested-by: Damon L. Chesser Signed-off-by: David S. Miller Signed-off-by: Chris Wright (cherry-picked from commit 99d737e98d81762332242cc82e5604520842911a 2.6.25.y) Signed-off-by: Stefan Bader --- net/ipv4/tcp_input.c | 15 +++++++++++---- 1 files changed, 11 insertions(+), 4 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 013bce3..b39200c 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -1730,9 +1730,16 @@ static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag) TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS; } - /* 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)) { + /* Marking forward transmissions that were made after RTO lost + * can cause unnecessary retransmissions in some scenarios, + * SACK blocks will mitigate that in some but not in all cases. + * We used to not mark them but it was causing break-ups with + * receivers that do only in-order receival. + * + * TODO: we could detect presence of such receiver and select + * different behavior per flow. + */ + if (!(TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED)) { TCP_SKB_CB(skb)->sacked |= TCPCB_LOST; tp->lost_out += tcp_skb_pcount(skb); } @@ -1748,7 +1755,7 @@ static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag) tp->reordering = min_t(unsigned int, tp->reordering, sysctl_tcp_reordering); tcp_set_ca_state(sk, TCP_CA_Loss); - tp->high_seq = tp->frto_highmark; + tp->high_seq = tp->snd_nxt; TCP_ECN_queue_cwr(tp); tcp_clear_retrans_hints_partial(tp);