From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f50.google.com ([209.85.160.50]:43376 "EHLO mail-pl0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935049AbeCGUN7 (ORCPT ); Wed, 7 Mar 2018 15:13:59 -0500 Received: by mail-pl0-f50.google.com with SMTP id f23-v6so1976875plr.10 for ; Wed, 07 Mar 2018 12:13:59 -0800 (PST) Message-ID: <1520453637.109662.53.camel@gmail.com> Subject: Re: [PATCH net 5/5] tcp: send real dupACKs by locking advertized window for non-SACK flows From: Eric Dumazet To: Ilpo =?ISO-8859-1?Q?J=E4rvinen?= Cc: Netdev Date: Wed, 07 Mar 2018 12:13:57 -0800 In-Reply-To: References: <1520427569-14365-1-git-send-email-ilpo.jarvinen@helsinki.fi> <1520427569-14365-6-git-send-email-ilpo.jarvinen@helsinki.fi> <1520438302.109662.42.camel@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: netdev-owner@vger.kernel.org List-ID: 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.