[PATCH net-2.6.24 0/4]: TCP fixes

[PATCH net-2.6.24 0/4]: TCP fixes

[Ilpo Järvinen]

Hi Dave,

This fixes the newreno fackets_out case, which turned out to be
not related to the Cedric's case being under investigation. Two
trivial comment patches, and frto with high-speed seqno
wrap-around protection. Compile tested. Please apply to
net-2.6.24.

-- 
 i.

[PATCH 1/4] [TCP]: No fackets_out/highest_sack tuning when SACK isn't enabled

[Ilpo Järvinen]

This was found due to bug report from Cedric Le Goater though
it turned this turned out to be unrelated bug.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_output.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 94c8011..6199abe 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -660,7 +660,7 @@ static void tcp_set_skb_tso_segs(struct sock *sk, struct sk_buff *skb, unsigned
 static void tcp_adjust_fackets_out(struct tcp_sock *tp, struct sk_buff *skb,
 				   int decr)
 {
-	if (!tp->sacked_out)
+	if (!tp->sacked_out || tcp_is_reno(tp))
 		return;
 
 	if (!before(tp->highest_sack, TCP_SKB_CB(skb)->seq))
@@ -712,7 +712,8 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len, unsigned int mss
 	TCP_SKB_CB(buff)->end_seq = TCP_SKB_CB(skb)->end_seq;
 	TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(buff)->seq;
 
-	if (tp->sacked_out && (TCP_SKB_CB(skb)->seq == tp->highest_sack))
+	if (tcp_is_sack(tp) && tp->sacked_out &&
+	    (TCP_SKB_CB(skb)->seq == tp->highest_sack))
 		tp->highest_sack = TCP_SKB_CB(buff)->seq;
 
 	/* PSH and FIN should only be set in the second packet. */
@@ -1718,7 +1719,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
 		BUG_ON(tcp_skb_pcount(skb) != 1 ||
 		       tcp_skb_pcount(next_skb) != 1);
 
-		if (WARN_ON(tp->sacked_out &&
+		if (WARN_ON(tcp_is_sack(tp) && tp->sacked_out &&
 		    (TCP_SKB_CB(next_skb)->seq == tp->highest_sack)))
 			return;
 
-- 
1.5.0.6

[PATCH 2/4] [TCP]: fix comments that got messed up during code move

[Ilpo Järvinen]

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_input.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2286361..135f046 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1467,8 +1467,9 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
 	return flag;
 }
 
-/* F-RTO can only be used if TCP has never retransmitted anything other than
- * head (SACK enhanced variant from Appendix B of RFC4138 is more robust here)
+/* If we receive more dupacks than we expected counting segments
+ * in assumption of absent reordering, interpret this as reordering.
+ * The only another reason could be bug in receiver TCP.
  */
 static void tcp_check_reno_reordering(struct sock *sk, const int addend)
 {
@@ -1516,6 +1517,9 @@ static inline void tcp_reset_reno_sack(struct tcp_sock *tp)
 	tp->sacked_out = 0;
 }
 
+/* F-RTO can only be used if TCP has never retransmitted anything other than
+ * head (SACK enhanced variant from Appendix B of RFC4138 is more robust here)
+ */
 int tcp_use_frto(struct sock *sk)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
-- 
1.5.0.6

[PATCH 3/4] [TCP]: Update comment of SACK block validator

[Ilpo Järvinen]

Just came across what RFC2018 states about generation of valid
SACK blocks in case of reneging. Alter comment a bit to point
out clearly.

IMHO, there isn't any reason to change code because the
validation is there for a purpose (counters will inform user
about decision TCP made if this case ever surfaces).

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_input.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 135f046..cec2611 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1027,8 +1027,15 @@ static void tcp_update_reordering(struct sock *sk, const int metric,
  * SACK block range validation checks that the received SACK block fits to
  * the expected sequence limits, i.e., it is between SND.UNA and SND.NXT.
  * Note that SND.UNA is not included to the range though being valid because
- * it means that the receiver is rather inconsistent with itself (reports
- * SACK reneging when it should advance SND.UNA).
+ * it means that the receiver is rather inconsistent with itself reporting
+ * SACK reneging when it should advance SND.UNA. Such SACK block this is
+ * perfectly valid, however, in light of RFC2018 which explicitly states
+ * that "SACK block MUST reflect the newest segment.  Even if the newest
+ * segment is going to be discarded ...", not that it looks very clever
+ * in case of head skb. Due to potentional receiver driven attacks, we
+ * choose to avoid immediate execution of a walk in write queue due to
+ * reneging and defer head skb's loss recovery to standard loss recovery
+ * procedure that will eventually trigger (nothing forbids us doing this).
  *
  * Implements also blockage to start_seq wrap-around. Problem lies in the
  * fact that though start_seq (s) is before end_seq (i.e., not reversed),
-- 
1.5.0.6

[PATCH 4/4] [TCP]: Wrap-safed reordering detection FRTO check

[Ilpo Järvinen]

In case somebody has a suggestion about a better place for this
check, which must guarantee execution "early enough" (i.e,
before the wrap can occur), I'm very open to them.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_input.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index cec2611..e22ffe7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3024,6 +3024,9 @@ static int tcp_ack(struct sock *sk, struct sk_buff *skb, int flag)
 	/* See if we can take anything off of the retransmit queue. */
 	flag |= tcp_clean_rtx_queue(sk, &seq_rtt);
 
+	/* Guarantee sacktag reordering detection against wrap-arounds */
+	if (before(tp->frto_highmark, tp->snd_una))
+		tp->frto_highmark = 0;
 	if (tp->frto_counter)
 		frto_cwnd = tcp_process_frto(sk, flag);
 
-- 
1.5.0.6

Re: [PATCH net-2.6.24 0/4]: TCP fixes

[David Miller]

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Mon,  1 Oct 2007 15:29:40 +0300

> This fixes the newreno fackets_out case, which turned out to be
> not related to the Cedric's case being under investigation. Two
> trivial comment patches, and frto with high-speed seqno
> wrap-around protection. Compile tested. Please apply to
> net-2.6.24.

I've applied them all to net-2.6.24, thanks Ilpo!