netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Neal Cardwell <ncardwell@google.com>
To: Oleksandr Natalenko <oleksandr@natalenko.name>
Cc: Yuchung Cheng <ycheng@google.com>,
	netdev <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Patrick McHardy <kaber@trash.net>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	James Morris <jmorris@namei.org>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [REGRESSION] tcp/ipv4: kernel panic because of (possible) division by zero
Date: Sun, 10 Jan 2016 12:29:17 -0500	[thread overview]
Message-ID: <CADVnQykPOVf7PoK8-6Mvbmbcpe1eYY=cfKHgiNGDNSFfCDPb-A@mail.gmail.com> (raw)
In-Reply-To: <1562805.Wa36G0OKaZ@spock>

On Sun, Jan 10, 2016 at 9:57 AM, Oleksandr Natalenko
<oleksandr@natalenko.name> wrote:
> I use YeAH. But YeAH code wasn't touched between 4.2 and 4.3.

Oh, interesting. Looks like tcp_yeah_ssthresh() has a bug where its
intended reduction can be bigger than tp->snd_cwnd, leading to it
return a zero ssthresh (or even an ssthresh that underflows to ~4
billion). If tcp_yeah_ssthresh() returns an ssthresh of 0 then PRR
will try to pull the cwnd down to 0.

Can you please leave ECN and Yeah enabled and run something like the
following patch, to verify this conjecture? If the conjecture is
right, then the tcp_yeah warning should fire but not the new
tcp_cwnd_reduction() warning:

-----------
diff --git a/net/ipv4/tcp_yeah.c b/net/ipv4/tcp_yeah.c
index 17d3566..ef60cba 100644
--- a/net/ipv4/tcp_yeah.c
+++ b/net/ipv4/tcp_yeah.c
@@ -206,6 +206,7 @@ static u32 tcp_yeah_ssthresh(struct sock *sk)
        const struct tcp_sock *tp = tcp_sk(sk);
        struct yeah *yeah = inet_csk_ca(sk);
        u32 reduction;
+       s32 ssthresh;

        if (yeah->doing_reno_now < TCP_YEAH_RHO) {
                reduction = yeah->lastQ;
@@ -219,7 +220,9 @@ static u32 tcp_yeah_ssthresh(struct sock *sk)
        yeah->fast_count = 0;
        yeah->reno_count = max(yeah->reno_count>>1, 2U);

-       return tp->snd_cwnd - reduction;
+       ssthresh = tp->snd_cwnd - reduction;
+       if (WARN_ON_ONCE(ssthresh <= 0))
+               ssthresh = 1;
 }

 static struct tcp_congestion_ops tcp_yeah __read_mostly = {
-----------

If that works, then we may just want a version of this patch without
the warning.

Thanks!
neal

  reply	other threads:[~2016-01-10 17:29 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-21 20:25 [REGRESSION] tcp/ipv4: kernel panic because of (possible) division by zero Oleksandr Natalenko
2015-12-22  2:10 ` Yuchung Cheng
2015-12-22 20:13   ` Oleksandr Natalenko
2016-01-06 16:50   ` Oleksandr Natalenko
2016-01-06 18:19     ` Yuchung Cheng
2016-01-06 18:43       ` Yuchung Cheng
2016-01-06 18:49         ` Oleksandr Natalenko
2016-01-09 17:34         ` Oleksandr Natalenko
2016-01-10 10:23         ` Oleksandr Natalenko
2016-01-10 14:48           ` Neal Cardwell
2016-01-10 14:54             ` Neal Cardwell
2016-01-10 14:57               ` Oleksandr Natalenko
2016-01-10 14:57             ` Oleksandr Natalenko
2016-01-10 17:29               ` Neal Cardwell [this message]
2016-01-10 17:50                 ` Oleksandr Natalenko
2016-01-10 18:00                   ` Neal Cardwell
2016-01-10 21:56                 ` Oleksandr Natalenko
2016-01-11 18:47                   ` Neal Cardwell
2016-01-11 23:26                     ` Oleksandr Natalenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CADVnQykPOVf7PoK8-6Mvbmbcpe1eYY=cfKHgiNGDNSFfCDPb-A@mail.gmail.com' \
    --to=ncardwell@google.com \
    --cc=davem@davemloft.net \
    --cc=jmorris@namei.org \
    --cc=kaber@trash.net \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oleksandr@natalenko.name \
    --cc=ycheng@google.com \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).