On Fri, Jun 29, 2018 at 3:52 PM Ilpo Järvinen <ilpo.jarvi...@helsinki.fi> wrote:
> > +.005 < . 1:1(0) ack 2001 win 257
>
> Why did the receiver send a cumulative ACK only for 2001?
Sorry, you are right Ilpo. Upon further reflection, the packetdrill
scenario I posted is not a realistic one, and I agree we should not
worry about it.
As I mentioned, I ran your patch through all our team's TCP
packetdrill tests, and it passes all of the tests. One of our tests
needed updating, because if there is a non-SACK connection with a
spurious RTO due to a delayed flight of ACKs then the FRTO undo now
happens one ACK later (when we get an ACK that doesn't cover a
retransmit). But that seems fine to me.
I also cooked the new packetdrill test below to explicitly cover this
case you are addressing (please let me know if you have an alternate
suggestion).
Tested-by: Neal Cardwell <ncardw...@google.com>
Acked-by: Neal Cardwell <ncardw...@google.com>
Thanks!
neal
---
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 32792 <mss 1000,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,wscale 8>
+.02 < . 1:1(0) ack 1 win 257
+0 accept(3, ..., ...) = 4
// Send 3 packets. First is really lost. And the dupacks
// for the data packets that arrived at the reciver are slow in arriving.
+0 write(4, ..., 3000) = 3000
+0 > P. 1:3001(3000) ack 1
// RTO and retransmit head. This fills a real loss.
+.22 > . 1:1001(1000) ack 1
// Dupacks for packets 2 and 3 arrive.
+.02 < . 1:1(0) ack 1 win 257
+0 < . 1:1(0) ack 1 win 257
// The cumulative ACK for all the data arrives. We do not undo, because
// this is a non-SACK connection, and retransmitted data was ACKed.
// It's good that there's no FRTO undo, since a packet was really lost.
// Because this is non-SACK, tcp_try_undo_recovery() holds CA_Loss
// until something beyond high_seq is ACKed.
+.005 < . 1:1(0) ack 3001 win 257
+0 %{ assert tcpi_ca_state == TCP_CA_Loss, tcpi_ca_state }%
+0 %{ assert tcpi_snd_cwnd == 4, tcpi_snd_cwnd }%
+0 %{ assert tcpi_snd_ssthresh == 7, tcpi_snd_ssthresh }%
