On Wed, 28 Mar 2018, Yuchung Cheng wrote: > On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen > <ilpo.jarvi...@helsinki.fi> wrote: > > When a cumulative ACK lands to high_seq at the end of loss > > recovery and SACK is not enabled, the sender needs to avoid > > false fast retransmits (RFC6582). The avoidance mechanisms is > > implemented by remaining in the loss recovery CA state until > > one additional cumulative ACK arrives. During the operation of > > this avoidance mechanism, there is internal transient in the > > use of state variables which will always trigger a bogus undo. > > Do we have to make undo in non-sack perfect? can we consider a much > simpler but imperfect fix of > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > index 8d480542aa07..95225d9de0af 100644 > --- a/net/ipv4/tcp_input.c > +++ b/net/ipv4/tcp_input.c > @@ -2356,6 +2356,7 @@ static bool tcp_try_undo_recovery(struct sock *sk) > * fast retransmits (RFC2582). SACK TCP is safe. */ > if (!tcp_any_retrans_done(sk)) > tp->retrans_stamp = 0; > + tp->undo_marker = 0; > return true; > }
Yes, that's of course a possible and would workaround the issue too. In fact, I initially did that kind of fix for myself (I put it into a block with tp->retrans_stamp = 0 though). But then I realized that it is not that complicated to make the fix locally into tcp_packet_delayed() (except the annoyance of passing all the necessary state parameters through the deep static call-chain but that should pose no big challenge for the compiler to handle I guess). BTW, do you know under what circumstances that tcp_any_retrans_done(sk) would return non-zero here (snd_una == high_seq so those rexmit would need to be above high_seq)? -- i.
