diff mbox

[net-next,03/17] tcp: fix lost_cnt_hint miscounts

Message ID 12358322823783-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>

It is possible that lost_cnt_hint gets underflow in
tcp_clean_rtx_queue because the cumulative ACK can cover
the segment where lost_skb_hint points to only partially,
which means that the hint is not cleared, opposite to what
my (earlier) comment claimed.

Also I don't agree what I ended up writing about non-trivial
case there to be what I intented to say. It was not supposed
to happen that the hint won't get cleared and we underflow
in any scenario.

In general, this is quite hard to trigger in practice.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_input.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 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:28 +0200

> From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> 
> It is possible that lost_cnt_hint gets underflow in
> tcp_clean_rtx_queue because the cumulative ACK can cover
> the segment where lost_skb_hint points to only partially,
> which means that the hint is not cleared, opposite to what
> my (earlier) comment claimed.
> 
> Also I don't agree what I ended up writing about non-trivial
> case there to be what I intented to say. It was not supposed
> to happen that the hint won't get cleared and we underflow
> in any scenario.
> 
> In general, this is quite hard to trigger in practice.
> 
> 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 c28976a..3f2f090 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3273,18 +3273,15 @@  static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
 		if (tcp_is_reno(tp)) {
 			tcp_remove_reno_sacks(sk, pkts_acked);
 		} else {
+			int delta;
+
 			/* Non-retransmitted hole got filled? That's reordering */
 			if (reord < prior_fackets)
 				tcp_update_reordering(sk, tp->fackets_out - reord, 0);
 
-			/* No need to care for underflows here because
-			 * the lost_skb_hint gets NULLed if we're past it
-			 * (or something non-trivial happened)
-			 */
-			if (tcp_is_fack(tp))
-				tp->lost_cnt_hint -= pkts_acked;
-			else
-				tp->lost_cnt_hint -= prior_sacked - tp->sacked_out;
+			delta = tcp_is_fack(tp) ? pkts_acked :
+						  prior_sacked - tp->sacked_out;
+			tp->lost_cnt_hint -= min(tp->lost_cnt_hint, delta);
 		}
 
 		tp->fackets_out -= min(pkts_acked, tp->fackets_out);