From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH net] tcp: prevent bogus FRTO undos with non-SACK flows Date: Sun, 01 Jul 2018 19:24:35 +0900 (KST) Message-ID: <20180701.192435.353563332676493794.davem@davemloft.net> References: Mime-Version: 1.0 Content-Type: Text/Plain; charset=iso-8859-1 Content-Transfer-Encoding: 8BIT Cc: netdev@vger.kernel.org, ycheng@google.com, edumazet@google.com, ncardwell@google.com, mkubecek@suse.cz To: ilpo.jarvinen@helsinki.fi Return-path: Received: from shards.monkeyblade.net ([23.128.96.9]:53218 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751628AbeGAKkO (ORCPT ); Sun, 1 Jul 2018 06:40:14 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: From: Ilpo Järvinen Date: Fri, 29 Jun 2018 13:07:53 +0300 (EEST) > If SACK is not enabled and the first cumulative ACK after the RTO > retransmission covers more than the retransmitted skb, a spurious > FRTO undo will trigger (assuming FRTO is enabled for that RTO). > The reason is that any non-retransmitted segment acknowledged will > set FLAG_ORIG_SACK_ACKED in tcp_clean_rtx_queue even if there is > no indication that it would have been delivered for real (the > scoreboard is not kept with TCPCB_SACKED_ACKED bits in the non-SACK > case so the check for that bit won't help like it does with SACK). > Having FLAG_ORIG_SACK_ACKED set results in the spurious FRTO undo > in tcp_process_loss. > > We need to use more strict condition for non-SACK case and check > that none of the cumulatively ACKed segments were retransmitted > to prove that progress is due to original transmissions. Only then > keep FLAG_ORIG_SACK_ACKED set, allowing FRTO undo to proceed in > non-SACK case. > > (FLAG_ORIG_SACK_ACKED is planned to be renamed to FLAG_ORIG_PROGRESS > to better indicate its purpose but to keep this change minimal, it > will be done in another patch). > > Besides burstiness and congestion control violations, this problem > can result in RTO loop: When the loss recovery is prematurely > undoed, only new data will be transmitted (if available) and > the next retransmission can occur only after a new RTO which in case > of multiple losses (that are not for consecutive packets) requires > one RTO per loss to recover. > > Signed-off-by: Ilpo Järvinen Applied and queued up for -stable, thanks!