From patchwork Wed Jan 6 20:42:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yuchung Cheng X-Patchwork-Id: 564082 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 0253F1402E2 for ; Thu, 7 Jan 2016 07:43:59 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b=Ty2o1mPN; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751922AbcAFUn0 (ORCPT ); Wed, 6 Jan 2016 15:43:26 -0500 Received: from mail-pa0-f50.google.com ([209.85.220.50]:36484 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751761AbcAFUnY (ORCPT ); Wed, 6 Jan 2016 15:43:24 -0500 Received: by mail-pa0-f50.google.com with SMTP id yy13so146992974pab.3 for ; Wed, 06 Jan 2016 12:43:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=nNuStyYl8gTrIjCKcd51xhzh2pQYSlKJpt5SxzfEUFI=; b=Ty2o1mPN3hnIzJCZGz7gLsP4DmBExkVsi+fvIbDp3tetzxmomBT+ZiRUg0HPZQUgtZ JIStiGt93w0WgFFL/t+0RgbVa9EEPc+OcQquCxvlknJQ0rWM1vPKM5FyWEJMiR/s1jFp vg65G6NlgJQ8hlN4hU+LlyQYQaQZxoT0kbiPeeqD6F/6C3glx8oDxnj5b07O218vpWFE CROA3KXeAO4k1SMaoCJVCU71yrp9CJrGuLqM6qEwTt2/Hcj9mFO/NF8qLcVpj1mOjdmr A3h3Hy+GWoOLFcmfgqxNbMi8jWVS8cNLYnXOylyXu0PpIw6Z34FX+E/lcZUQcvuFVn3u LD9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=nNuStyYl8gTrIjCKcd51xhzh2pQYSlKJpt5SxzfEUFI=; b=RobLSqZ3hkZW3RkYzUfW90RkcQ8vleS7DSlExowBE8kzjMWLxmCsvYQLrJ5c5p9HYv FyCQcTWfjGce9NdneR3oyRcEg7xPdU3xp33SiOyNEtav4eP0tKCAm5bqcTi4lFeic/WK Ox1yP7fFK0dqYA6b0bbxRNvoM9YKX1iLbEfc/+rpTM5iaEW/Vt0+UZCVQCdkNUmLH+e6 r3ItuSu2VwmAhz1gALzGewInGjJ13ScT2QeSh2xJVKbfaR10Lkqj0w9tr8vVtfAYNtUW QrFSjV8vA7virnIPVFF/+kVkLTAYaJMBYv2EO0ao5pumMYmr0fkqZrc8XiVsq+6dIHm5 3a0w== X-Gm-Message-State: ALoCoQmb0EljmrRYtrDRwggJYykoIDtdKjoOUiWlIGa8HPD+8vwQAXYZxkWguQmua7coRIXGdbPFieTDk7CUsNbBFiCmWZqEyA== X-Received: by 10.66.218.225 with SMTP id pj1mr140993319pac.40.1452113003838; Wed, 06 Jan 2016 12:43:23 -0800 (PST) Received: from ycheng.mtv.corp.google.com ([172.17.133.36]) by smtp.gmail.com with ESMTPSA id e14sm142939922pap.24.2016.01.06.12.43.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 06 Jan 2016 12:43:22 -0800 (PST) From: Yuchung Cheng To: davem@davemloft.net Cc: netdev@vger.kernel.org, oleksandr@natalenko.name, Yuchung Cheng , Neal Cardwell , Eric Dumazet Subject: [PATCH net] tcp: fix zero cwnd in tcp_cwnd_reduction Date: Wed, 6 Jan 2016 12:42:38 -0800 Message-Id: <1452112958-1589-1-git-send-email-ycheng@google.com> X-Mailer: git-send-email 2.6.0.rc2.230.g3dd15c0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Patch 3759824da87b ("tcp: PRR uses CRB mode by default and SS mode conditionally") introduced a bug that cwnd may become 0 when both inflight and sndcnt are 0 (cwnd = inflight + sndcnt). This may lead to a div-by-zero if the connection starts another cwnd reduction phase by setting tp->prior_cwnd to the current cwnd (0) in tcp_init_cwnd_reduction(). To prevent this we skip PRR operation when nothing is acked or sacked. Then cwnd must be positive in all cases as long as ssthresh is positive: 1) The proportional reduction mode inflight > ssthresh > 0 2) The reduction bound mode a) inflight == ssthresh > 0 b) inflight < ssthresh sndcnt > 0 since newly_acked_sacked > 0 and inflight < ssthresh Therefore in all cases inflight and sndcnt can not both be 0. We check invalid tp->prior_cwnd to avoid potential div0 bugs. In reality this bug is triggered only with a sequence of less common events. For example, the connection is terminating an ECN-triggered cwnd reduction with an inflight 0, then it receives reordered/old ACKs or DSACKs from prior transmission (which acks nothing). Or the connection is in fast recovery stage that marks everything lost, but fails to retransmit due to local issues, then receives data packets from other end which acks nothing. Fixes: 3759824da87b ("tcp: PRR uses CRB mode by default and SS mode conditionally") Reported-by: Oleksandr Natalenko Signed-off-by: Yuchung Cheng Signed-off-by: Neal Cardwell Signed-off-by: Eric Dumazet --- net/ipv4/tcp_input.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 2d656ee..d4c5115 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -2478,6 +2478,9 @@ static void tcp_cwnd_reduction(struct sock *sk, const int prior_unsacked, int newly_acked_sacked = prior_unsacked - (tp->packets_out - tp->sacked_out); + if (newly_acked_sacked <= 0 || WARN_ON_ONCE(!tp->prior_cwnd)) + return; + tp->prr_delivered += newly_acked_sacked; if (delta < 0) { u64 dividend = (u64)tp->snd_ssthresh * tp->prr_delivered +