New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 794054 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

BBR protocol algorithm full bandwidth estimation

Reported by shaune.f...@gmail.com, Dec 12 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_1) AppleWebKit/604.3.5 (KHTML, like Gecko) Version/11.0.1 Safari/604.3.5

Example URL:
https://chromium.googlesource.com/chromium/src/net/+/master/quic/core/congestion_control/bbr_sender.cc

Steps to reproduce the problem:
1. 
2. 
3. 

What is the expected behavior?
The variable is_at_full_bandwidth_ is always true.

What went wrong?
There no condistion to assign the is_at_full_bandwidth_ variable to false value.

Did this work before? No 

Chrome version: <Copy from: 'about:version'>  Channel: n/a
OS Version: 
Flash Version: 

if ((rounds_without_bandwidth_gain_ >= num_startup_rtts_) ||
      (exit_startup_on_loss_ && InRecovery())) {
    is_at_full_bandwidth_ = true;
  } else {
    is_at_full_bandwidth_ = false; // added
}
 
Cc: sc00335...@techmahindra.com
Labels: Triaged-ET TE-NeedsTriageHelp Needs-Milestone
This issue seems to be out of scope, hence adding TE-NeedsTriageHelp for further investigation of the issue.        
Cc: jri@chromium.org
Components: -Internals>Network Internals>Network>QUIC
I don't this is a bug. |is_at_full_bandwidth_| is default initialized to be false, so it seems to me that there's no need for an else-statement. 

+ Internals>Network>QUIC 
To do that, |is_at_full_bandwidth_| there no chance to be false, the algorithm always think current usage of the link is full.
But in linux BBR algorithm , the condition is:

static void bbr_check_full_bw_reached(struct sock *sk,
				      const struct rate_sample *rs)
{
	struct bbr *bbr = inet_csk_ca(sk);
	u32 bw_thresh;

	if (bbr_full_bw_reached(sk) || !bbr->round_start || rs->is_app_limited)
		return;

	bw_thresh = (u64)bbr->full_bw * bbr_full_bw_thresh >> BBR_SCALE;
	if (bbr_max_bw(sk) >= bw_thresh) {
		bbr->full_bw = bbr_max_bw(sk);
		bbr->full_bw_cnt = 0;
		return;
	}
	++bbr->full_bw_cnt;
	*** bbr->full_bw_reached = bbr->full_bw_cnt >= bbr_full_bw_cnt; ***
}

Comment 4 by rch@chromium.org, Dec 13 2017

Cc: ianswett@chromium.org
ianswett: FYI

Comment 5 by jri@chromium.org, Dec 14 2017

Cc: ncardwell@google.com
Status: WontFix (was: Unconfirmed)
shuane.feng: Thanks for your careful read the BBR implementations! 

The QUIC BBR code uses is_at_full_bandwidth_ only during STARTUP, to decide whether it is time to exit STARTUP mode. Since we don't re-enter STARTUP, it doesn't need to be set back to false.

The TCP BBR implementation is slightly different, but I don't believe this behavior is different; Cc'ing ncardwell@google.com who should be able to say more.
OK, I see, thank you!
Yes, the Linux TCP BBR implementation is very similar. There is a full_bw_reached bool that is only set during STARTUP, when it estimates that the "pipe" is full and decides to exit STARTUP. In Linux TCP BBR code it is important to *not* set the bool back to false because when exiting the PROBE_RTT state the sender needs this bit to know whether it should transition to STARTUP or PROBE_BW (depending on whether we estimate that we ever filled the pipe).
  See:
  https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/tree/net/ipv4/tcp_bbr.c

Comment 8 by jri@chromium.org, Dec 14 2017

ncardwell: That's exactly what the QUIC code does as well. Thanks for clarifying!

Sign in to add a comment