All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: Neal Cardwell <ncardwell@google.com>, David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, Yuchung Cheng <ycheng@google.com>,
	Kevin Yang <yyd@google.com>
Subject: Re: [PATCH net] tcp_bbr: fix u32 wrap bug in round logic if bbr_init() called after 2B packets
Date: Wed, 11 Aug 2021 09:49:52 +0200	[thread overview]
Message-ID: <9dfee38c-1a4a-8c3c-a9ac-20763a9becc7@gmail.com> (raw)
In-Reply-To: <20210811024056.235161-1-ncardwell@google.com>



On 8/11/21 4:40 AM, Neal Cardwell wrote:
> Currently if BBR congestion control is initialized after more than 2B
> packets have been delivered, depending on the phase of the
> tp->delivered counter the tracking of BBR round trips can get stuck.
> 
> The bug arises because if tp->delivered is between 2^31 and 2^32 at
> the time the BBR congestion control module is initialized, then the
> initialization of bbr->next_rtt_delivered to 0 will cause the logic to
> believe that the end of the round trip is still billions of packets in
> the future. More specifically, the following check will fail
> repeatedly:
> 
>   !before(rs->prior_delivered, bbr->next_rtt_delivered)
> 
> and thus the connection will take up to 2B packets delivered before
> that check will pass and the connection will set:
> 
>   bbr->round_start = 1;
> 
> This could cause many mechanisms in BBR to fail to trigger, for
> example bbr_check_full_bw_reached() would likely never exit STARTUP.
> 
> This bug is 5 years old and has not been observed, and as a practical
> matter this would likely rarely trigger, since it would require
> transferring at least 2B packets, or likely more than 3 terabytes of
> data, before switching congestion control algorithms to BBR.
> 
> This patch is a stable candidate for kernels as far back as v4.9,
> when tcp_bbr.c was added.
> 
> Fixes: 0f8782ea1497 ("tcp_bbr: add BBR congestion control")
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Reviewed-by: Yuchung Cheng <ycheng@google.com>
> Reviewed-by: Kevin Yang <yyd@google.com>

Nice catch :(

Reviewed-by: Eric Dumazet <edumazet@google.com>


  reply	other threads:[~2021-08-11  7:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-11  2:40 [PATCH net] tcp_bbr: fix u32 wrap bug in round logic if bbr_init() called after 2B packets Neal Cardwell
2021-08-11  7:49 ` Eric Dumazet [this message]
2021-08-11 22:10 ` patchwork-bot+netdevbpf

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=9dfee38c-1a4a-8c3c-a9ac-20763a9becc7@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=ycheng@google.com \
    --cc=yyd@google.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.