SACKED_ACKED and LOST are mutually exclusive, thus this
condition is bug with SACK (IMHO). NewReno, however, could get
enough duplicate ACKs which increment sacked_out, so it makes
sense to do this kind of limitting for non-SACK TCP but not for
SACK-enabled one. Perhaps the author had that in mind but did
the logic accidently wrong way around?
Eventually these bugs trigger traps in the tcp_clean_rtx_queue
but it's much more informative to do this here (excludes some
other possible bugs).
Maybe this BUG_TRAP is too expensive to be included everywhere
in the TCP code. Should there be some #if to surround it?
Compile tested. Sadly enough I don't have time for couple of
weeks to test this as it would require some setuping, and besides,
my test machines are occupied currently to other work, but this
might also be net-2.6 (or even stable) material if it really
works (feel free to cut this paragraph or part of it if you
decide to include this :-)).
Signed-off-by: Ilpo Järvinen <[EMAIL PROTECTED]>
---
include/net/tcp.h | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index fe1c4f0..3c8dd13 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -733,9 +733,10 @@ static inline __u32 tcp_current_ssthresh
static inline void tcp_sync_left_out(struct tcp_sock *tp)
{
- if (tp->rx_opt.sack_ok &&
- (tp->sacked_out >= tp->packets_out - tp->lost_out))
+ if (tp->sacked_out + tp->lost_out > tp->packets_out) {
+ BUG_TRAP(!tp->rx_opt.sack_ok);
tp->sacked_out = tp->packets_out - tp->lost_out;
+ }
tp->left_out = tp->sacked_out + tp->lost_out;
}
--
1.4.2
