Hello Andrey,
I've found and fixed two problems with new TCP features.
Cool, I love bug reports with a patch attached to it ;-)
The first problem concerns rtt evaluation using timestamps.
The symptoms of the problem is that the response time on ssh connections
even on a local network could rises to few dozen seconds.
The problem is explained by an incorrect rtt calculation leading
to an unreasonable large retransmit timeout.
...
The origin of the problem is
a missing case in the rtt evaluation code: interpacket delays because of absence
of a data shouldn't be accounted for rtt calculations.
Correct, good spotting.
The second problem is a bug clearing SACK queue as shown by tcpdump:
Hmmm, let's look at this a bit more (some things removed for clarity):
193:1069 > castle:22: . ack 4106046548 win 31856 <sack 4106046568..4106046588>
193:1069 ACK's up to 4106046548, reports a SACK from 4106046568-->4106046588
193:1069 > castle:22: P 4000765003:4000765023(20) ack 4106046548 win 31856
<sack 4106046568..4106046588>
193:1069 ACK's up to 4106046548, reports (again) SACK from 4106046568-->4106046588
castle:22 > 193:1069: P 4106046548:4106046588(40) ack 4000765023 win 31856
Castle:22 ACK's 4000765023, sends data to fill the hole reported by 193:1069
193:1069 > castle:22: . ack 4106046588 win 31856 <sack 4106046568..4106046588>
(buggy SACKs in the last line).
Yep, certainly, the ACK was advanced but the SACK was not zapped.
diff -ru linux.orig/net/ipv4/tcp_input.c linux/net/ipv4/tcp_input.c
--- linux.orig/net/ipv4/tcp_input.c Wed Apr 1 12:28:26 1998
+++ linux/net/ipv4/tcp_input.c Fri Apr 10 18:06:29 1998
@@ -667,7 +667,21 @@
static void tcp_ack_saw_tstamp(struct sock *sk, struct tcp_opt *tp,
u32 seq, u32 ack, int flag)
{
...
Ok, the RTT in the presence of timestamp fix was correct, and in my
tree. But as for the SACK fix...
@@ -1224,7 +1237,8 @@
* from the front of a SACK.
*/
for(this_sack = 0; this_sack < num_sacks; this_sack++, sp++) {
- if(!after(sp->start_seq, TCP_SKB_CB(skb)->seq) &&
+ /* check if the start of the sack is covered by skb */
+ if(!before(sp->start_seq, TCP_SKB_CB(skb)->seq) &&
before(sp->start_seq, TCP_SKB_CB(skb)->end_seq))
break;
}
Oh yes, you're right. I had seen this bug before from a report done
by Finn Gangstad, and the line you changed was my "fix" to his buggy
SACK report, and my fix was backwards ;-)
Thanks a lot for the bugfixes, they are all in my tree now.
Later,
David S. Miller
davem@dm.cobaltmicro.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu