From patchwork Tue Mar 13 10:25:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 885050 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=helsinki.fi Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 400rbV21nxz9sTK for ; Tue, 13 Mar 2018 21:25:42 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932753AbeCMKZf (ORCPT ); Tue, 13 Mar 2018 06:25:35 -0400 Received: from smtp-rs1-vallila2.fe.helsinki.fi ([128.214.173.75]:39262 "EHLO smtp-rs1-vallila2.fe.helsinki.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932639AbeCMKZZ (ORCPT ); Tue, 13 Mar 2018 06:25:25 -0400 Received: from whs-18.cs.helsinki.fi (whs-18.cs.helsinki.fi [128.214.166.46]) by smtp-rs1.it.helsinki.fi (8.14.4/8.14.4) with ESMTP id w2DAPKAr032479; Tue, 13 Mar 2018 12:25:20 +0200 Received: by whs-18.cs.helsinki.fi (Postfix, from userid 1070048) id 06FE0360385; Tue, 13 Mar 2018 12:25:20 +0200 (EET) From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= To: netdev@vger.kernel.org Cc: Yuchung Cheng , Neal Cardwell , Eric Dumazet , Sergei Shtylyov Subject: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery Date: Tue, 13 Mar 2018 12:25:07 +0200 Message-Id: <1520936711-16784-2-git-send-email-ilpo.jarvinen@helsinki.fi> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> References: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org A miscalculation for the number of acknowledged packets occurs during RTO recovery whenever SACK is not enabled and a cumulative ACK covers any non-retransmitted skbs. The reason is that pkts_acked value calculated in tcp_clean_rtx_queue is not correct for slow start after RTO as it may include segments that were not lost and therefore did not need retransmissions in the slow start following the RTO. Then tcp_slow_start will add the excess into cwnd bloating it and triggering a burst. Instead, we want to pass only the number of retransmitted segments that were covered by the cumulative ACK (and potentially newly sent data segments too if the cumulative ACK covers that far). Signed-off-by: Ilpo Järvinen --- net/ipv4/tcp_input.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 9a1b3c1..4a26c09 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -3027,6 +3027,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack, long seq_rtt_us = -1L; long ca_rtt_us = -1L; u32 pkts_acked = 0; + u32 rexmit_acked = 0; + u32 newdata_acked = 0; u32 last_in_flight = 0; bool rtt_update; int flag = 0; @@ -3056,8 +3058,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack, } if (unlikely(sacked & TCPCB_RETRANS)) { - if (sacked & TCPCB_SACKED_RETRANS) + if (sacked & TCPCB_SACKED_RETRANS) { tp->retrans_out -= acked_pcount; + rexmit_acked += acked_pcount; + } flag |= FLAG_RETRANS_DATA_ACKED; } else if (!(sacked & TCPCB_SACKED_ACKED)) { last_ackt = skb->skb_mstamp; @@ -3070,6 +3074,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack, reord = start_seq; if (!after(scb->end_seq, tp->high_seq)) flag |= FLAG_ORIG_SACK_ACKED; + else + newdata_acked += acked_pcount; } if (sacked & TCPCB_SACKED_ACKED) { @@ -3151,6 +3157,14 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack, } if (tcp_is_reno(tp)) { + /* Due to discontinuity on RTO in the artificial + * sacked_out calculations, TCP must restrict + * pkts_acked without SACK to rexmits and new data + * segments + */ + if (icsk->icsk_ca_state == TCP_CA_Loss) + pkts_acked = rexmit_acked + newdata_acked; + tcp_remove_reno_sacks(sk, pkts_acked); } else { int delta; From patchwork Tue Mar 13 10:25:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 885049 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=helsinki.fi Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 400rbQ0W0kz9sTH for ; Tue, 13 Mar 2018 21:25:38 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932760AbeCMKZg (ORCPT ); Tue, 13 Mar 2018 06:25:36 -0400 Received: from smtp-rs1-vallila2.fe.helsinki.fi ([128.214.173.75]:39260 "EHLO smtp-rs1-vallila2.fe.helsinki.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932636AbeCMKZZ (ORCPT ); Tue, 13 Mar 2018 06:25:25 -0400 Received: from whs-18.cs.helsinki.fi (whs-18.cs.helsinki.fi [128.214.166.46]) by smtp-rs1.it.helsinki.fi (8.14.4/8.14.4) with ESMTP id w2DAPKXh032481; Tue, 13 Mar 2018 12:25:20 +0200 Received: by whs-18.cs.helsinki.fi (Postfix, from userid 1070048) id 0F37C360388; Tue, 13 Mar 2018 12:25:20 +0200 (EET) From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= To: netdev@vger.kernel.org Cc: Yuchung Cheng , Neal Cardwell , Eric Dumazet , Sergei Shtylyov Subject: [PATCH v3 net 2/5] tcp: prevent bogus FRTO undos with non-SACK flows Date: Tue, 13 Mar 2018 12:25:08 +0200 Message-Id: <1520936711-16784-3-git-send-email-ilpo.jarvinen@helsinki.fi> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> References: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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. Signed-off-by: Ilpo Järvinen --- net/ipv4/tcp_input.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 4a26c09..c60745c 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -3166,6 +3166,15 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack, pkts_acked = rexmit_acked + newdata_acked; tcp_remove_reno_sacks(sk, pkts_acked); + + /* If any of the cumulatively ACKed segments was + * retransmitted, non-SACK case cannot confirm that + * progress was due to original transmission due to + * lack of TCPCB_SACKED_ACKED bits even if some of + * the packets may have been never retransmitted. + */ + if (flag & FLAG_RETRANS_DATA_ACKED) + flag &= ~FLAG_ORIG_SACK_ACKED; } else { int delta; From patchwork Tue Mar 13 10:25:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 885051 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=helsinki.fi Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 400rbZ132bz9sTH for ; Tue, 13 Mar 2018 21:25:46 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932749AbeCMKZe (ORCPT ); Tue, 13 Mar 2018 06:25:34 -0400 Received: from smtp-rs1-vallila2.fe.helsinki.fi ([128.214.173.75]:39254 "EHLO smtp-rs1-vallila2.fe.helsinki.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932618AbeCMKZZ (ORCPT ); Tue, 13 Mar 2018 06:25:25 -0400 Received: from whs-18.cs.helsinki.fi (whs-18.cs.helsinki.fi [128.214.166.46]) by smtp-rs1.it.helsinki.fi (8.14.4/8.14.4) with ESMTP id w2DAPK4N032483; Tue, 13 Mar 2018 12:25:20 +0200 Received: by whs-18.cs.helsinki.fi (Postfix, from userid 1070048) id 16EFC360389; Tue, 13 Mar 2018 12:25:20 +0200 (EET) From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= To: netdev@vger.kernel.org Cc: Yuchung Cheng , Neal Cardwell , Eric Dumazet , Sergei Shtylyov Subject: [PATCH v3 net 3/5] tcp: move false FR condition into tcp_false_fast_retrans_possible() Date: Tue, 13 Mar 2018 12:25:09 +0200 Message-Id: <1520936711-16784-4-git-send-email-ilpo.jarvinen@helsinki.fi> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> References: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org No functional changes. This change simplifies the next change slightly. Signed-off-by: Ilpo Järvinen --- net/ipv4/tcp_input.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index c60745c..72ecfbb 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -2211,6 +2211,17 @@ static void tcp_update_scoreboard(struct sock *sk, int fast_rexmit) } } +/* False fast retransmits may occur when SACK is not in use under certain + * conditions (RFC6582). The sender MUST hold old state until something + * *above* high_seq is ACKed to prevent triggering such false fast + * retransmits. SACK TCP is safe. + */ +static bool tcp_false_fast_retrans_possible(const struct tcp_sock *tp, + const u32 snd_una) +{ + return tcp_is_reno(tp) && (snd_una == tp->high_seq); +} + static bool tcp_tsopt_ecr_before(const struct tcp_sock *tp, u32 when) { return tp->rx_opt.saw_tstamp && tp->rx_opt.rcv_tsecr && @@ -2350,10 +2361,10 @@ static bool tcp_try_undo_recovery(struct sock *sk) } else if (tp->rack.reo_wnd_persist) { tp->rack.reo_wnd_persist--; } - if (tp->snd_una == tp->high_seq && tcp_is_reno(tp)) { - /* Hold old state until something *above* high_seq - * is ACKed. For Reno it is MUST to prevent false - * fast retransmits (RFC2582). SACK TCP is safe. */ + if (tcp_false_fast_retrans_possible(tp, tp->snd_una)) { + /* Hold old state until something *above* high_seq is ACKed + * if false fast retransmit is possible. + */ if (!tcp_any_retrans_done(sk)) tp->retrans_stamp = 0; return true; From patchwork Tue Mar 13 10:25:10 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 885052 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=helsinki.fi Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 400rbg5S7Sz9sTH for ; Tue, 13 Mar 2018 21:25:51 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932763AbeCMKZt (ORCPT ); Tue, 13 Mar 2018 06:25:49 -0400 Received: from smtp-rs1-vallila2.fe.helsinki.fi ([128.214.173.75]:39256 "EHLO smtp-rs1-vallila2.fe.helsinki.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932612AbeCMKZZ (ORCPT ); Tue, 13 Mar 2018 06:25:25 -0400 Received: from whs-18.cs.helsinki.fi (whs-18.cs.helsinki.fi [128.214.166.46]) by smtp-rs1.it.helsinki.fi (8.14.4/8.14.4) with ESMTP id w2DAPKJ4032484; Tue, 13 Mar 2018 12:25:20 +0200 Received: by whs-18.cs.helsinki.fi (Postfix, from userid 1070048) id 1CCE736038A; Tue, 13 Mar 2018 12:25:20 +0200 (EET) From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= To: netdev@vger.kernel.org Cc: Yuchung Cheng , Neal Cardwell , Eric Dumazet , Sergei Shtylyov Subject: [PATCH v3 net 4/5] tcp: prevent bogus undos when SACK is not enabled Date: Tue, 13 Mar 2018 12:25:10 +0200 Message-Id: <1520936711-16784-5-git-send-email-ilpo.jarvinen@helsinki.fi> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> References: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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. When we enter to this transient state in tcp_try_undo_recovery, tcp_any_retrans_done is often (always?) false resulting in clearing retrans_stamp. On the next cumulative ACK, tcp_try_undo_recovery again executes because CA state still remains in the same recovery state and tcp_may_undo will always return true because tcp_packet_delayed has this condition: return !tp->retrans_stamp || ... Check if the false fast retransmit transient avoidance is in progress in tcp_packet_delayed to avoid bogus undos. Since snd_una has advanced already on this ACK but CA state still remains unchanged (CA state is updated slightly later than undo is checked), prior_snd_una needs to be passed to tcp_packet_delayed (instead of tp->snd_una). Passing prior_snd_una around to the tcp_packet_delayed makes this change look more involved than it really is. The additional checks done in this change only affect non-SACK case, the SACK case remains the same. Signed-off-by: Ilpo Järvinen --- net/ipv4/tcp_input.c | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 72ecfbb..270aa48 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -2241,10 +2241,17 @@ static bool tcp_skb_spurious_retrans(const struct tcp_sock *tp, /* Nothing was retransmitted or returned timestamp is less * than timestamp of the first retransmission. */ -static inline bool tcp_packet_delayed(const struct tcp_sock *tp) +static inline bool tcp_packet_delayed(const struct tcp_sock *tp, + const u32 prior_snd_una) { - return !tp->retrans_stamp || - tcp_tsopt_ecr_before(tp, tp->retrans_stamp); + if (!tp->retrans_stamp) { + /* Sender will be in a transient state with cleared + * retrans_stamp during false fast retransmit prevention + * mechanism + */ + return !tcp_false_fast_retrans_possible(tp, prior_snd_una); + } + return tcp_tsopt_ecr_before(tp, tp->retrans_stamp); } /* Undo procedures. */ @@ -2334,17 +2341,19 @@ static void tcp_undo_cwnd_reduction(struct sock *sk, bool unmark_loss) tp->rack.advanced = 1; /* Force RACK to re-exam losses */ } -static inline bool tcp_may_undo(const struct tcp_sock *tp) +static inline bool tcp_may_undo(const struct tcp_sock *tp, + const u32 prior_snd_una) { - return tp->undo_marker && (!tp->undo_retrans || tcp_packet_delayed(tp)); + return tp->undo_marker && + (!tp->undo_retrans || tcp_packet_delayed(tp, prior_snd_una)); } /* People celebrate: "We love our President!" */ -static bool tcp_try_undo_recovery(struct sock *sk) +static bool tcp_try_undo_recovery(struct sock *sk, const u32 prior_snd_una) { struct tcp_sock *tp = tcp_sk(sk); - if (tcp_may_undo(tp)) { + if (tcp_may_undo(tp, prior_snd_una)) { int mib_idx; /* Happy end! We did not retransmit anything @@ -2391,11 +2400,12 @@ static bool tcp_try_undo_dsack(struct sock *sk) } /* Undo during loss recovery after partial ACK or using F-RTO. */ -static bool tcp_try_undo_loss(struct sock *sk, bool frto_undo) +static bool tcp_try_undo_loss(struct sock *sk, const u32 prior_snd_una, + bool frto_undo) { struct tcp_sock *tp = tcp_sk(sk); - if (frto_undo || tcp_may_undo(tp)) { + if (frto_undo || tcp_may_undo(tp, prior_snd_una)) { tcp_undo_cwnd_reduction(sk, true); DBGUNDO(sk, "partial loss"); @@ -2628,13 +2638,13 @@ void tcp_enter_recovery(struct sock *sk, bool ece_ack) * recovered or spurious. Otherwise retransmits more on partial ACKs. */ static void tcp_process_loss(struct sock *sk, int flag, bool is_dupack, - int *rexmit) + int *rexmit, const u32 prior_snd_una) { struct tcp_sock *tp = tcp_sk(sk); bool recovered = !before(tp->snd_una, tp->high_seq); if ((flag & FLAG_SND_UNA_ADVANCED) && - tcp_try_undo_loss(sk, false)) + tcp_try_undo_loss(sk, prior_snd_una, false)) return; if (tp->frto) { /* F-RTO RFC5682 sec 3.1 (sack enhanced version). */ @@ -2642,7 +2652,7 @@ static void tcp_process_loss(struct sock *sk, int flag, bool is_dupack, * lost, i.e., never-retransmitted data are (s)acked. */ if ((flag & FLAG_ORIG_SACK_ACKED) && - tcp_try_undo_loss(sk, true)) + tcp_try_undo_loss(sk, prior_snd_una, true)) return; if (after(tp->snd_nxt, tp->high_seq)) { @@ -2665,7 +2675,7 @@ static void tcp_process_loss(struct sock *sk, int flag, bool is_dupack, if (recovered) { /* F-RTO RFC5682 sec 3.1 step 2.a and 1st part of step 3.a */ - tcp_try_undo_recovery(sk); + tcp_try_undo_recovery(sk, prior_snd_una); return; } if (tcp_is_reno(tp)) { @@ -2685,7 +2695,7 @@ static bool tcp_try_undo_partial(struct sock *sk, u32 prior_snd_una) { struct tcp_sock *tp = tcp_sk(sk); - if (tp->undo_marker && tcp_packet_delayed(tp)) { + if (tp->undo_marker && tcp_packet_delayed(tp, prior_snd_una)) { /* Plain luck! Hole if filled with delayed * packet, rather than with a retransmit. Check reordering. */ @@ -2788,7 +2798,7 @@ static void tcp_fastretrans_alert(struct sock *sk, const u32 prior_snd_una, case TCP_CA_Recovery: if (tcp_is_reno(tp)) tcp_reset_reno_sack(tp); - if (tcp_try_undo_recovery(sk)) + if (tcp_try_undo_recovery(sk, prior_snd_una)) return; tcp_end_cwnd_reduction(sk); break; @@ -2815,7 +2825,7 @@ static void tcp_fastretrans_alert(struct sock *sk, const u32 prior_snd_una, tcp_rack_identify_loss(sk, ack_flag); break; case TCP_CA_Loss: - tcp_process_loss(sk, flag, is_dupack, rexmit); + tcp_process_loss(sk, flag, is_dupack, rexmit, prior_snd_una); tcp_rack_identify_loss(sk, ack_flag); if (!(icsk->icsk_ca_state == TCP_CA_Open || (*ack_flag & FLAG_LOST_RETRANS))) From patchwork Tue Mar 13 10:25:11 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 885048 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=helsinki.fi Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 400rbK5mBqz9sTL for ; Tue, 13 Mar 2018 21:25:33 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932742AbeCMKZ1 (ORCPT ); Tue, 13 Mar 2018 06:25:27 -0400 Received: from smtp-rs1-vallila2.fe.helsinki.fi ([128.214.173.75]:39264 "EHLO smtp-rs1-vallila2.fe.helsinki.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932620AbeCMKZZ (ORCPT ); Tue, 13 Mar 2018 06:25:25 -0400 Received: from whs-18.cs.helsinki.fi (whs-18.cs.helsinki.fi [128.214.166.46]) by smtp-rs1.it.helsinki.fi (8.14.4/8.14.4) with ESMTP id w2DAPKHs032489; Tue, 13 Mar 2018 12:25:20 +0200 Received: by whs-18.cs.helsinki.fi (Postfix, from userid 1070048) id 209C836038C; Tue, 13 Mar 2018 12:25:20 +0200 (EET) From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= To: netdev@vger.kernel.org Cc: Yuchung Cheng , Neal Cardwell , Eric Dumazet , Sergei Shtylyov Subject: [PATCH v3 net 5/5] tcp: send real dupACKs by locking advertized window for non-SACK flows Date: Tue, 13 Mar 2018 12:25:11 +0200 Message-Id: <1520936711-16784-6-git-send-email-ilpo.jarvinen@helsinki.fi> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> References: <1520936711-16784-1-git-send-email-ilpo.jarvinen@helsinki.fi> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Currently, the TCP code is overly eager to increase window on almost every ACK. It makes those ACKs that the receiver should sent as dupACKs look like they update window that is not considered a real dupACK by the non-SACK sender-side code. Therefore the sender needs to resort to RTO to recover losses as fast retransmit/fast recovery cannot be triggered by such masked duplicate ACKs. This change makes sure that when an ofo segment is received, no change to window is applied if we are going to send a dupACK. Even with this change, the window updates keep being maintained but that occurs "behind the scenes". That is, this change does not interfere with memory management of the flow which could have long-term impact for the progress of the flow but only prevents those updates being seen on the wire on short-term. 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 and also whenever the ack field advances. As ack field typically advances once per RTT as the first hole is retransmitted, the window updates are not delayed entirely during long recoveries. Even before this fix, tcp_select_window did not allow ACK shrinking the window for duplicate ACKs (that was previously even called "treason" but the old charmy message is gone now). The advertized window can only shrink when also ack field changes which will not be blocked by this change as it is not duplicate ACK. Signed-off-by: Ilpo Järvinen --- include/linux/tcp.h | 3 ++- net/ipv4/tcp_input.c | 5 ++++- net/ipv4/tcp_output.c | 43 +++++++++++++++++++++++++------------------ 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/include/linux/tcp.h b/include/linux/tcp.h index 8f4c549..e239662 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -225,7 +225,8 @@ struct tcp_sock { fastopen_connect:1, /* FASTOPEN_CONNECT sockopt */ fastopen_no_cookie:1, /* Allow send/recv SYN+data without a cookie */ is_sack_reneg:1, /* in recovery from loss with SACK reneg? */ - unused:2; + dupack_wnd_lock :1, /* Non-SACK constant rwnd dupacks needed? */ + unused:1; u8 nonagle : 4,/* Disable Nagle algorithm? */ thin_lto : 1,/* Use linear timeouts for thin streams */ unused1 : 1, diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 270aa48..4ff192b 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -4626,6 +4626,7 @@ int tcp_send_rcvq(struct sock *sk, struct msghdr *msg, size_t size) static void tcp_data_queue(struct sock *sk, struct sk_buff *skb) { struct tcp_sock *tp = tcp_sk(sk); + struct inet_connection_sock *icsk = inet_csk(sk); bool fragstolen; int eaten; @@ -4669,7 +4670,7 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb) * gap in queue is filled. */ if (RB_EMPTY_ROOT(&tp->out_of_order_queue)) - inet_csk(sk)->icsk_ack.pingpong = 0; + icsk->icsk_ack.pingpong = 0; } if (tp->rx_opt.num_sacks) @@ -4719,6 +4720,8 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb) goto queue_and_out; } + if (tcp_is_reno(tp) && !(icsk->icsk_ack.pending & ICSK_ACK_TIMER)) + tp->dupack_wnd_lock = 1; tcp_data_queue_ofo(sk, skb); } diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 6818042..45fbe92 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -249,25 +249,32 @@ static u16 tcp_select_window(struct sock *sk) { struct tcp_sock *tp = tcp_sk(sk); u32 old_win = tp->rcv_wnd; - u32 cur_win = tcp_receive_window(tp); - u32 new_win = __tcp_select_window(sk); - - /* Never shrink the offered window */ - if (new_win < cur_win) { - /* Danger Will Robinson! - * Don't update rcv_wup/rcv_wnd here or else - * we will not be able to advertise a zero - * window in time. --DaveM - * - * Relax Will Robinson. - */ - if (new_win == 0) - NET_INC_STATS(sock_net(sk), - LINUX_MIB_TCPWANTZEROWINDOWADV); - new_win = ALIGN(cur_win, 1 << tp->rx_opt.rcv_wscale); + u32 cur_win; + u32 new_win; + + if (tp->dupack_wnd_lock) { + new_win = old_win; + tp->dupack_wnd_lock = 0; + } else { + cur_win = tcp_receive_window(tp); + new_win = __tcp_select_window(sk); + /* Never shrink the offered window */ + if (new_win < cur_win) { + /* Danger Will Robinson! + * Don't update rcv_wup/rcv_wnd here or else + * we will not be able to advertise a zero + * window in time. --DaveM + * + * Relax Will Robinson. + */ + if (new_win == 0) + NET_INC_STATS(sock_net(sk), + LINUX_MIB_TCPWANTZEROWINDOWADV); + new_win = ALIGN(cur_win, 1 << tp->rx_opt.rcv_wscale); + } + tp->rcv_wnd = new_win; + tp->rcv_wup = tp->rcv_nxt; } - tp->rcv_wnd = new_win; - tp->rcv_wup = tp->rcv_nxt; /* Make sure we do not exceed the maximum possible * scaled window.