gfe-relnote: (n/a) In QUIC BBRv1, default enable connection options LRTT and BBQ2, and deprecate unused QUIC connection options BBS1 and BBRS. Protected by --gfe2_reloadable_flag_quic_bbr_default_exit_startup_on_loss. PiperOrigin-RevId: 304406108 Change-Id: I2e89d9474a09e9d9fb5157f69577f55f7a4176b8
diff --git a/quic/core/congestion_control/bbr_sender.cc b/quic/core/congestion_control/bbr_sender.cc index ee74d70..8331611 100644 --- a/quic/core/congestion_control/bbr_sender.cc +++ b/quic/core/congestion_control/bbr_sender.cc
@@ -106,7 +106,8 @@ congestion_window_gain_constant_( static_cast<float>(GetQuicFlag(FLAGS_quic_bbr_cwnd_gain))), num_startup_rtts_(kRoundTripsWithoutGrowthBeforeExitingStartup), - exit_startup_on_loss_(false), + exit_startup_on_loss_( + GetQuicReloadableFlag(quic_bbr_default_exit_startup_on_loss)), cycle_current_offset_(0), last_cycle_start_(QuicTime::Zero()), is_at_full_bandwidth_(false), @@ -137,6 +138,10 @@ stats_->slowstart_duration = QuicTimeAccumulator(); } EnterStartupMode(now); + if (exit_startup_on_loss_) { + QUIC_RELOADABLE_FLAG_COUNT(quic_bbr_default_exit_startup_on_loss); + set_high_cwnd_gain(kDerivedHighCWNDGain); + } } BbrSender::~BbrSender() {} @@ -198,8 +203,14 @@ return ProbeRttCongestionWindow(); } - if (InRecovery() && !(rate_based_startup_ && mode_ == STARTUP)) { - return std::min(congestion_window_, recovery_window_); + if (exit_startup_on_loss_) { + if (InRecovery()) { + return std::min(congestion_window_, recovery_window_); + } + } else { + if (InRecovery() && !(rate_based_startup_ && mode_ == STARTUP)) { + return std::min(congestion_window_, recovery_window_); + } } return congestion_window_; @@ -257,13 +268,15 @@ if (config.HasClientRequestedIndependentOption(k2RTT, perspective)) { num_startup_rtts_ = 2; } - if (config.HasClientRequestedIndependentOption(kBBRS, perspective)) { + if (!exit_startup_on_loss_ && + config.HasClientRequestedIndependentOption(kBBRS, perspective)) { slower_startup_ = true; } if (config.HasClientRequestedIndependentOption(kBBR3, perspective)) { drain_to_target_ = true; } - if (config.HasClientRequestedIndependentOption(kBBS1, perspective)) { + if (!exit_startup_on_loss_ && + config.HasClientRequestedIndependentOption(kBBS1, perspective)) { rate_based_startup_ = true; } if (GetQuicReloadableFlag(quic_bbr_mitigate_overly_large_bandwidth_sample)) { @@ -290,7 +303,8 @@ set_high_cwnd_gain(kDerivedHighGain); set_drain_gain(1.f / kDerivedHighGain); } - if (config.HasClientRequestedIndependentOption(kBBQ2, perspective)) { + if (!exit_startup_on_loss_ && + config.HasClientRequestedIndependentOption(kBBQ2, perspective)) { set_high_cwnd_gain(kDerivedHighCWNDGain); } if (config.HasClientRequestedIndependentOption(kBBQ3, perspective)) { @@ -824,12 +838,14 @@ } } - // Slow the pacing rate in STARTUP once loss has ever been detected. - const bool has_ever_detected_loss = end_recovery_at_.IsInitialized(); - if (slower_startup_ && has_ever_detected_loss && - has_non_app_limited_sample_) { - pacing_rate_ = kStartupAfterLossGain * BandwidthEstimate(); - return; + if (!exit_startup_on_loss_) { + // Slow the pacing rate in STARTUP once loss has ever been detected. + const bool has_ever_detected_loss = end_recovery_at_.IsInitialized(); + if (slower_startup_ && has_ever_detected_loss && + has_non_app_limited_sample_) { + pacing_rate_ = kStartupAfterLossGain * BandwidthEstimate(); + return; + } } // Do not decrease the pacing rate during startup. @@ -877,7 +893,7 @@ void BbrSender::CalculateRecoveryWindow(QuicByteCount bytes_acked, QuicByteCount bytes_lost) { - if (rate_based_startup_ && mode_ == STARTUP) { + if (!exit_startup_on_loss_ && rate_based_startup_ && mode_ == STARTUP) { return; }
diff --git a/quic/core/congestion_control/bbr_sender.h b/quic/core/congestion_control/bbr_sender.h index eb8282a..ab5d819 100644 --- a/quic/core/congestion_control/bbr_sender.h +++ b/quic/core/congestion_control/bbr_sender.h
@@ -324,6 +324,8 @@ const float congestion_window_gain_constant_; // The number of RTTs to stay in STARTUP mode. Defaults to 3. QuicRoundTripCount num_startup_rtts_; + + // Latched value of --quic_bbr_default_exit_startup_on_loss. // If true, exit startup if all of the following conditions are met: // - 1RTT has passed with no bandwidth increase, // - Some number of congestion events happened with loss, in the last round.
diff --git a/quic/core/congestion_control/bbr_sender_test.cc b/quic/core/congestion_control/bbr_sender_test.cc index 6f91153..e07931b 100644 --- a/quic/core/congestion_control/bbr_sender_test.cc +++ b/quic/core/congestion_control/bbr_sender_test.cc
@@ -340,45 +340,6 @@ EXPECT_APPROX_EQ(kTestRtt, sender_->GetMinRtt(), 0.2f); } -TEST_F(BbrSenderTest, SimpleTransferEarlyPacketLoss) { - SetQuicReloadableFlag(quic_bbr_no_bytes_acked_in_startup_recovery, true); - // Enable rate based startup so the recovery window doesn't hide the true - // congestion_window_ in GetCongestionWindow(). - SetConnectionOption(kBBS1); - // Disable Ack Decimation on the receiver, because it can increase srtt. - QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING); - CreateDefaultSetup(); - - // At startup make sure we are at the default. - EXPECT_EQ(kDefaultWindowTCP, sender_->GetCongestionWindow()); - // Verify that Sender is in slow start. - EXPECT_TRUE(sender_->InSlowStart()); - // At startup make sure we can send. - EXPECT_TRUE(sender_->CanSend(0)); - // And that window is un-affected. - EXPECT_EQ(kDefaultWindowTCP, sender_->GetCongestionWindow()); - - // Transfer 12MB. - bbr_sender_.AddBytesToTransfer(12 * 1024 * 1024); - // Drop the first packet. - receiver_.DropNextIncomingPacket(); - bool simulator_result = simulator_.RunUntilOrTimeout( - [this]() { - if (sender_->InRecovery()) { - // Two packets are acked before the first is declared lost. - EXPECT_LE(sender_->GetCongestionWindow(), - (kDefaultWindowTCP + 2 * kDefaultTCPMSS)); - } - return bbr_sender_.bytes_to_transfer() == 0 || !sender_->InSlowStart(); - }, - QuicTime::Delta::FromSeconds(30)); - EXPECT_TRUE(simulator_result) << "Simple transfer failed. Bytes remaining: " - << bbr_sender_.bytes_to_transfer(); - EXPECT_EQ(BbrSender::DRAIN, sender_->ExportDebugState().mode); - EXPECT_EQ(1u, bbr_sender_.connection()->GetStats().packets_lost); - EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); -} - TEST_F(BbrSenderTest, RemoveBytesLostInRecovery) { SetQuicReloadableFlag(quic_bbr_one_mss_conservation, false); // Disable Ack Decimation on the receiver, because it can increase srtt. @@ -706,10 +667,18 @@ ASSERT_EQ(BbrSender::DRAIN, sender_->ExportDebugState().mode); EXPECT_APPROX_EQ(sender_->BandwidthEstimate() * (1 / 2.885f), sender_->PacingRate(0), 0.01f); - // BBR uses CWND gain of 2.88 during STARTUP, hence it will fill the buffer - // with approximately 1.88 BDPs. Here, we use 1.5 to give some margin for - // error. - EXPECT_GE(queue->bytes_queued(), 1.5 * kTestBdp); + + if (!GetQuicReloadableFlag(quic_bbr_default_exit_startup_on_loss)) { + // BBR uses CWND gain of 2.88 during STARTUP, hence it will fill the buffer + // with approximately 1.88 BDPs. Here, we use 1.5 to give some margin for + // error. + EXPECT_GE(queue->bytes_queued(), 1.5 * kTestBdp); + } else { + // BBR uses CWND gain of 2 during STARTUP, hence it will fill the buffer + // with approximately 1 BDP. Here, we use 0.8 to give some margin for + // error. + EXPECT_GE(queue->bytes_queued(), 0.8 * kTestBdp); + } // Observe increased RTT due to bufferbloat. const QuicTime::Delta queueing_delay = @@ -843,12 +812,12 @@ DriveOutOfStartup(); const QuicTime::Delta timeout = QuicTime::Delta::FromSeconds(5); - bool simulator_result; - - // Start a few cycles prior to the high gain one. - simulator_result = simulator_.RunUntilOrTimeout( - [this]() { return sender_->ExportDebugState().gain_cycle_index == 6; }, - timeout); + while (!(sender_->ExportDebugState().gain_cycle_index >= 4 && + bbr_sender_.bytes_to_transfer() == 0)) { + bbr_sender_.AddBytesToTransfer(kTestLinkBandwidth.ToBytesPerSecond()); + ASSERT_TRUE(simulator_.RunUntilOrTimeout( + [this]() { return bbr_sender_.bytes_to_transfer() == 0; }, timeout)); + } // Send at 10% of available rate. Run for 3 seconds, checking in the middle // and at the end. The pacing gain should be high throughout. @@ -867,10 +836,10 @@ // PROBE_BW mode to enter low gain cycle, and exit it earlier than one min_rtt // due to running out of data to send. bbr_sender_.AddBytesToTransfer(1.3 * kTestBdp); - simulator_result = simulator_.RunUntilOrTimeout( + ASSERT_TRUE(simulator_.RunUntilOrTimeout( [this]() { return sender_->ExportDebugState().gain_cycle_index == 1; }, - timeout); - ASSERT_TRUE(simulator_result); + timeout)); + simulator_.RunFor(0.75 * sender_->ExportDebugState().min_rtt); EXPECT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode); EXPECT_EQ(2, sender_->ExportDebugState().gain_cycle_index); @@ -963,11 +932,13 @@ EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); } -// Test exiting STARTUP earlier upon loss due to the LRTT connection option. -TEST_F(BbrSenderTest, SimpleTransferLRTTStartup) { +// Test exiting STARTUP earlier upon loss. +TEST_F(BbrSenderTest, SimpleTransferExitStartupOnLoss) { CreateDefaultSetup(); - SetConnectionOption(kLRTT); + if (!GetQuicReloadableFlag(quic_bbr_default_exit_startup_on_loss)) { + SetConnectionOption(kLRTT); + } EXPECT_EQ(3u, sender_->num_startup_rtts()); // Run until the full bandwidth is reached and check how many rounds it was. @@ -991,11 +962,13 @@ EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); } -// Test exiting STARTUP earlier upon loss due to the LRTT connection option. -TEST_F(BbrSenderTest, SimpleTransferLRTTStartupSmallBuffer) { +// Test exiting STARTUP earlier upon loss with a small buffer. +TEST_F(BbrSenderTest, SimpleTransferExitStartupOnLossSmallBuffer) { CreateSmallBufferSetup(); - SetConnectionOption(kLRTT); + if (!GetQuicReloadableFlag(quic_bbr_default_exit_startup_on_loss)) { + SetConnectionOption(kLRTT); + } EXPECT_EQ(3u, sender_->num_startup_rtts()); // Run until the full bandwidth is reached and check how many rounds it was. @@ -1019,68 +992,6 @@ EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); } -// Test slower pacing after loss in STARTUP due to the BBRS connection option. -TEST_F(BbrSenderTest, SimpleTransferSlowerStartup) { - CreateSmallBufferSetup(); - - SetConnectionOption(kBBRS); - EXPECT_EQ(3u, sender_->num_startup_rtts()); - - // Run until the full bandwidth is reached and check how many rounds it was. - bbr_sender_.AddBytesToTransfer(12 * 1024 * 1024); - QuicRoundTripCount max_bw_round = 0; - QuicBandwidth max_bw(QuicBandwidth::Zero()); - bool simulator_result = simulator_.RunUntilOrTimeout( - [this, &max_bw, &max_bw_round]() { - if (max_bw < sender_->ExportDebugState().max_bandwidth) { - max_bw = sender_->ExportDebugState().max_bandwidth; - max_bw_round = sender_->ExportDebugState().round_trip_count; - } - // Expect the pacing rate in STARTUP to decrease once packet loss - // is observed, but the CWND does not. - if (bbr_sender_.connection()->GetStats().packets_lost > 0 && - !sender_->ExportDebugState().is_at_full_bandwidth && - sender_->has_non_app_limited_sample()) { - EXPECT_EQ(1.5f * max_bw, sender_->PacingRate(0)); - } - return sender_->ExportDebugState().is_at_full_bandwidth; - }, - QuicTime::Delta::FromSeconds(5)); - ASSERT_TRUE(simulator_result); - EXPECT_EQ(BbrSender::DRAIN, sender_->ExportDebugState().mode); - EXPECT_GE(3u, sender_->ExportDebugState().round_trip_count - max_bw_round); - EXPECT_EQ(3u, sender_->ExportDebugState().rounds_without_bandwidth_gain); - EXPECT_NE(0u, bbr_sender_.connection()->GetStats().packets_lost); - EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); -} - -// Ensures no change in congestion window in STARTUP after loss. -TEST_F(BbrSenderTest, SimpleTransferNoConservationInStartup) { - CreateSmallBufferSetup(); - - SetConnectionOption(kBBS1); - - // Run until the full bandwidth is reached and check how many rounds it was. - bbr_sender_.AddBytesToTransfer(12 * 1024 * 1024); - bool used_conservation_cwnd = false; - bool simulator_result = simulator_.RunUntilOrTimeout( - [this, &used_conservation_cwnd]() { - if (!sender_->ExportDebugState().is_at_full_bandwidth && - sender_->GetCongestionWindow() < - sender_->ExportDebugState().congestion_window) { - used_conservation_cwnd = true; - } - return sender_->ExportDebugState().is_at_full_bandwidth; - }, - QuicTime::Delta::FromSeconds(5)); - ASSERT_TRUE(simulator_result); - EXPECT_FALSE(used_conservation_cwnd); - EXPECT_EQ(BbrSender::DRAIN, sender_->ExportDebugState().mode); - EXPECT_EQ(3u, sender_->ExportDebugState().rounds_without_bandwidth_gain); - EXPECT_NE(0u, bbr_sender_.connection()->GetStats().packets_lost); - EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); -} - TEST_F(BbrSenderTest, DerivedPacingGainStartup) { CreateDefaultSetup(); @@ -1111,7 +1022,9 @@ TEST_F(BbrSenderTest, DerivedCWNDGainStartup) { CreateSmallBufferSetup(); - SetConnectionOption(kBBQ2); + if (!GetQuicReloadableFlag(quic_bbr_default_exit_startup_on_loss)) { + SetConnectionOption(kBBQ2); + } EXPECT_EQ(3u, sender_->num_startup_rtts()); // Verify that Sender is in slow start. EXPECT_TRUE(sender_->InSlowStart()); @@ -1128,7 +1041,9 @@ QuicTime::Delta::FromSeconds(5)); ASSERT_TRUE(simulator_result); EXPECT_EQ(BbrSender::DRAIN, sender_->ExportDebugState().mode); - EXPECT_EQ(3u, sender_->ExportDebugState().rounds_without_bandwidth_gain); + if (!bbr_sender_.connection()->GetStats().bbr_exit_startup_due_to_loss) { + EXPECT_EQ(3u, sender_->ExportDebugState().rounds_without_bandwidth_gain); + } EXPECT_APPROX_EQ(kTestLinkBandwidth, sender_->ExportDebugState().max_bandwidth, 0.01f); float loss_rate =
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h index 8e03bb3..8af8a04 100644 --- a/quic/core/crypto/crypto_protocol.h +++ b/quic/core/crypto/crypto_protocol.h
@@ -82,8 +82,7 @@ const QuicTag k1RTT = TAG('1', 'R', 'T', 'T'); // STARTUP in BBR for 1 RTT const QuicTag k2RTT = TAG('2', 'R', 'T', 'T'); // STARTUP in BBR for 2 RTTs const QuicTag kLRTT = TAG('L', 'R', 'T', 'T'); // Exit STARTUP in BBR on loss -const QuicTag kBBS1 = TAG('B', 'B', 'S', '1'); // Rate-based recovery in - // BBR STARTUP +const QuicTag kBBS1 = TAG('B', 'B', 'S', '1'); // DEPRECATED const QuicTag kBBS2 = TAG('B', 'B', 'S', '2'); // More aggressive packet // conservation in BBR STARTUP const QuicTag kBBS3 = TAG('B', 'B', 'S', '3'); // Slowstart packet @@ -99,8 +98,7 @@ const QuicTag kBBR5 = TAG('B', 'B', 'R', '5'); // 40 RTT ack aggregation const QuicTag kBBR9 = TAG('B', 'B', 'R', '9'); // Ignore app-limited calls in // BBR if enough inflight. -const QuicTag kBBRS = TAG('B', 'B', 'R', 'S'); // Use 1.5x pacing in startup - // after a loss has occurred. +const QuicTag kBBRS = TAG('B', 'B', 'R', 'S'); // DEPRECATED const QuicTag kBBQ1 = TAG('B', 'B', 'Q', '1'); // BBR with lower 2.77 STARTUP // pacing and CWND gain. const QuicTag kBBQ2 = TAG('B', 'B', 'Q', '2'); // BBR with lower 2.0 STARTUP