On Wed, 2018-03-07 at 22:09 +0200, Ilpo Järvinen wrote: > Thanks for taking a look. > > On Wed, 7 Mar 2018, Eric Dumazet wrote: > > On Wed, 2018-03-07 at 14:59 +0200, Ilpo Järvinen wrote: > > > Currently, the TCP code is overly eager to update window on > > > every ACK. It makes some of the ACKs that the receiver should > > > sent as dupACKs look like they update window update that are > > > not considered real dupACKs by the non-SACK sender-side code. > > > > > > Make sure that when an ofo segment is received, no change to > > > window is applied if we are going to send a dupACK. It's ok > > > to change the window for non-dupACKs (such as the first ACK > > > after ofo arrivals start if that ACK was using delayed ACKs). > > > > This looks dangerous to me. > > > > We certainly want to lower the window at some point, or risk > > expensive > > pruning and/or drops. > > I see you're conspiring for "treason" (if you recall those charmy > times) ;-). > > > It is not clear by reading your changelog/patch, but it looks like > > some > > malicious peers could hurt us. > > The malicious peers can certainly send out-of-window data already at > will > (with all of such packets being dropped regardless of that being > expensive or not) so I don't see there's a big change for malicious > case > really. The window is only locked for what we've already promised for > in > an earlier ACK! ...Previously, reneging that promise (advertising > smaller > than the previous value) was called "treason" by us (unfortunately, > the > message is no longer as charmy). > > Even with this change, the window is free to change when the ack > field is > updated which for legimate senders occurs typically once per RTT. > > > By current standards, non TCP sack flows are not worth any > > potential > > issues. > > Currently non-SACK senders cannot identify almost any duplicate ACKs > because the window keeps updating for almost all ACKs. As a result, > non-SACK senders end up doing loss recovery only with RTO. RTO > recovery > without SACK is quite annoying because it potentially sends > large number of unnecessary rexmits.
I get that, but switching from "always" to "never" sounds dangerous. May I suggest you refine your patch, to maybe allow win reductions, in a logarithmic fashion maybe ? This way, when you send 1000 DUPACK, only few of them would actually have to lower the window, and 99% of them would be considered as DUPACK by these prehistoric TCP stacks.
