All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
To: netdev@vger.kernel.org
Cc: David Miller <davem@davemloft.net>,
	Pasi Sarolahti <pasi.sarolahti@nokia.com>
Subject: [PATCH 13/18] [TCP] FRTO: Entry is allowed only during (New)Reno like recovery
Date: Mon, 19 Feb 2007 13:38:07 +0200	[thread overview]
Message-ID: <11718850921409-git-send-email-ilpo.jarvinen@helsinki.fi> (raw)
In-Reply-To: <11718850921574-git-send-email-ilpo.jarvinen@helsinki.fi>

This interpretation comes from RFC4138:
    "If the sender implements some loss recovery algorithm other
     than Reno or NewReno [FHG04], the F-RTO algorithm SHOULD
     NOT be entered when earlier fast recovery is underway."

I think the RFC means to say (especially in the light of
Appendix B) that ...recovery is underway (not just fast recovery)
or was underway when it was interrupted by an earlier (F-)RTO
that hasn't yet been resolved (snd_una has not advanced enough).
Thus, my interpretation is that whenever TCP has ever
retransmitted other than head, basic version cannot be used
because then the order assumptions which are used as FRTO basis
do not hold.

NewReno has only the head segment retransmitted at a time.
Therefore, walk up to the segment that has not been SACKed, if
that segment is not retransmitted nor anything before it, we know
for sure, that nothing after the non-SACKed segment should be
either. This assumption is valid because TCPCB_EVER_RETRANS does
not leave holes but each non-SACKed segment is rexmitted
in-order.

Check for retrans_out > 1 avoids more expensive walk through the
skb list, as we can know the result beforehand: F-RTO will not be
allowed.

SACKed skb can turn into non-SACked only in the extremely rare
case of SACK reneging, in this case we might fail to detect
retransmissions if there were them for any other than head. To
get rid of that feature, whole rexmit queue would have to be
walked (always) or FRTO should be prevented when SACK reneging
happens. Of course RTO should still trigger after reneging which
makes this issue even less likely to show up. And as long as the
response is as conservative as it's now, nothing bad happens even
then.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 include/net/tcp.h    |    2 +-
 net/ipv4/tcp_input.c |   25 +++++++++++++++++++++----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 572a77b..7fd6b77 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -341,7 +341,7 @@ extern struct sock *		tcp_check_req(stru
 extern int			tcp_child_process(struct sock *parent,
 						  struct sock *child,
 						  struct sk_buff *skb);
-extern int			tcp_use_frto(const struct sock *sk);
+extern int			tcp_use_frto(struct sock *sk);
 extern void			tcp_enter_frto(struct sock *sk);
 extern void			tcp_enter_loss(struct sock *sk, int how);
 extern void			tcp_clear_retrans(struct tcp_sock *tp);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 5e952f0..8f0aa9d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1239,14 +1239,31 @@ #endif
 /* F-RTO can only be used if these conditions are satisfied:
  *  - there must be some unsent new data
  *  - the advertised window should allow sending it
+ *  - TCP has never retransmitted anything other than head
  */
-int tcp_use_frto(const struct sock *sk)
+int tcp_use_frto(struct sock *sk)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
+	struct sk_buff *skb;
 	
-	return (sysctl_tcp_frto && sk->sk_send_head &&
-		!after(TCP_SKB_CB(sk->sk_send_head)->end_seq,
-		       tp->snd_una + tp->snd_wnd));
+	if (!sysctl_tcp_frto || !sk->sk_send_head ||
+		after(TCP_SKB_CB(sk->sk_send_head)->end_seq,
+		      tp->snd_una + tp->snd_wnd))
+		return 0;
+
+	/* Avoid expensive walking of rexmit queue if possible */
+	if (tp->retrans_out > 1)
+		return 0;
+
+	skb = skb_peek(&sk->sk_write_queue)->next;	/* Skips head */
+	sk_stream_for_retrans_queue_from(skb, sk) {
+		if (TCP_SKB_CB(skb)->sacked&TCPCB_RETRANS)
+			return 0;
+		/* Short-circuit when first non-SACKed skb has been checked */
+		if (!(TCP_SKB_CB(skb)->sacked&TCPCB_SACKED_ACKED))
+			break;
+	}
+	return 1;
 }
 
 /* RTO occurred, but do not yet enter Loss state. Instead, defer RTO
-- 
1.4.2


  reply	other threads:[~2007-02-19 11:38 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-19 11:37 [PATCHSET 0/18] FRTO: fixes and small changes + SACK enhanced version Ilpo Järvinen
2007-02-19 11:37 ` [PATCH 1/18] [TCP] FRTO: Incorrectly clears TCPCB_EVER_RETRANS bit Ilpo Järvinen
2007-02-19 11:37   ` [PATCH 2/18] [TCP] FRTO: Separated response from FRTO detection algorithm Ilpo Järvinen
2007-02-19 11:37     ` [PATCH 3/18] [TCP] FRTO: Moved tcp_use_frto from tcp.h to tcp_input.c Ilpo Järvinen
2007-02-19 11:37       ` [PATCH 4/18] [TCP] FRTO: Comment cleanup & improvement Ilpo Järvinen
2007-02-19 11:37         ` [PATCH 5/18] [TCP] FRTO: Consecutive RTOs keep prior_ssthresh and ssthresh Ilpo Järvinen
2007-02-19 11:38           ` [PATCH 6/18] [TCP] FRTO: Use Disorder state during operation instead of Open Ilpo Järvinen
2007-02-19 11:38             ` [PATCH 7/18] [TCP] FRTO: Ignore some uninteresting ACKs Ilpo Järvinen
2007-02-19 11:38               ` [PATCH 8/18] [TCP] FRTO: fixes fallback to conventional recovery Ilpo Järvinen
2007-02-19 11:38                 ` [PATCH 9/18] [TCP] FRTO: Response should reset also snd_cwnd_cnt Ilpo Järvinen
2007-02-19 11:38                   ` [PATCH 10/18] [TCP]: Don't enter to fast recovery while using FRTO Ilpo Järvinen
2007-02-19 11:38                     ` [PATCH 11/18] [TCP] FRTO: frto_counter modulo-op converted to two assignments Ilpo Järvinen
2007-02-19 11:38                       ` [PATCH 12/18] [TCP]: Prevent unrelated cwnd adjustment while using FRTO Ilpo Järvinen
2007-02-19 11:38                         ` Ilpo Järvinen [this message]
2007-02-19 11:38                           ` [PATCH 14/18] [TCP] FRTO: Reverse RETRANS bit clearing logic Ilpo Järvinen
     [not found]                             ` <1171885092244-git- send-email-ilpo.jarvinen@helsinki.fi>
2007-02-19 11:38                             ` [PATCH 15/18] [TCP] FRTO: Fake cwnd for ssthresh callback Ilpo Järvinen
2007-02-19 11:38                               ` [PATCH 16/18] [TCP]: Prevent reordering adjustments during FRTO Ilpo Järvinen
2007-02-19 11:38                                 ` [PATCH 17/18] [TCP]: SACK enhanced FRTO Ilpo Järvinen
2007-02-19 11:38                                   ` [PATCH 18/18] [TCP] FRTO: Sysctl documentation for SACK enhanced version Ilpo Järvinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=11718850921409-git-send-email-ilpo.jarvinen@helsinki.fi \
    --to=ilpo.jarvinen@helsinki.fi \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=pasi.sarolahti@nokia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.