diff mbox

[net-next,07/17] tcp: remove redundant code from tcp_mark_lost_retrans

Message ID 12358322831147-git-send-email-ilpo.jarvinen@helsinki.fi
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Ilpo Järvinen Feb. 28, 2009, 2:44 p.m. UTC 
From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

Arnd Hannemann <hannemann@nets.rwth-aachen.de> noticed and was
puzzled by the fact that !tcp_is_fack(tp) leads to early return
near the beginning and the later on tcp_is_fack(tp) was still
used in an if condition. The later check was a left-over from
RFC3517 SACK stuff (== !tcp_is_fack(tp) behavior nowadays) as
there wasn't clear way how to handle this particular check
cheaply in the spirit of RFC3517 (using only SACK blocks, not
holes + SACK blocks as with FACK). I sort of left it there as
a reminder but since it's confusing other people just remove
it and comment the missing-feature stuff instead.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Cc: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
---
 net/ipv4/tcp_input.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

Comments

David Miller March 2, 2009, 11:01 a.m. UTC | #1 
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Sat, 28 Feb 2009 16:44:32 +0200

> From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> 
> Arnd Hannemann <hannemann@nets.rwth-aachen.de> noticed and was
> puzzled by the fact that !tcp_is_fack(tp) leads to early return
> near the beginning and the later on tcp_is_fack(tp) was still
> used in an if condition. The later check was a left-over from
> RFC3517 SACK stuff (== !tcp_is_fack(tp) behavior nowadays) as
> there wasn't clear way how to handle this particular check
> cheaply in the spirit of RFC3517 (using only SACK blocks, not
> holes + SACK blocks as with FACK). I sort of left it there as
> a reminder but since it's confusing other people just remove
> it and comment the missing-feature stuff instead.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 3f2f090..125b451 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1178,10 +1178,18 @@  static void tcp_mark_lost_retrans(struct sock *sk)
 		if (!(TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS))
 			continue;
 
-		if (after(received_upto, ack_seq) &&
-		    (tcp_is_fack(tp) ||
-		     !before(received_upto,
-			     ack_seq + tp->reordering * tp->mss_cache))) {
+		/* TODO: We would like to get rid of tcp_is_fack(tp) only
+		 * constraint here (see above) but figuring out that at
+		 * least tp->reordering SACK blocks reside between ack_seq
+		 * and received_upto is not easy task to do cheaply with
+		 * the available datastructures.
+		 *
+		 * Whether FACK should check here for tp->reordering segs
+		 * in-between one could argue for either way (it would be
+		 * rather simple to implement as we could count fack_count
+		 * during the walk and do tp->fackets_out - fack_count).
+		 */
+		if (after(received_upto, ack_seq)) {
 			TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS;
 			tp->retrans_out -= tcp_skb_pcount(skb);