If connection option 'B201' is set, when attempting to grow inflight_hi in PROBE_UP, check whether Bbr2Sender is cwnd limited before the current aggregation epoch, instead of before the current ack event. Protected by FLAGS_quic_reloadable_flag_quic_bbr2_check_cwnd_limited_before_aggregation_epoch. PiperOrigin-RevId: 398259930
diff --git a/quic/core/congestion_control/bbr2_misc.cc b/quic/core/congestion_control/bbr2_misc.cc index 6eb9b2e..2a772ca 100644 --- a/quic/core/congestion_control/bbr2_misc.cc +++ b/quic/core/congestion_control/bbr2_misc.cc
@@ -102,6 +102,11 @@ lost_packets, MaxBandwidth(), bandwidth_lo(), RoundTripCount()); + if (sample.extra_acked == 0) { + cwnd_limited_before_aggregation_epoch_ = + congestion_event->prior_bytes_in_flight >= congestion_event->prior_cwnd; + } + if (sample.last_packet_send_state.is_valid) { congestion_event->last_packet_send_state = sample.last_packet_send_state; congestion_event->last_sample_is_app_limited =
diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h index d28036d..8234999 100644 --- a/quic/core/congestion_control/bbr2_misc.h +++ b/quic/core/congestion_control/bbr2_misc.h
@@ -130,6 +130,11 @@ // Multiplier to get target inflight (as multiple of BDP) for PROBE_UP phase. float probe_bw_probe_inflight_gain = 1.25; + // When attempting to grow inflight_hi in PROBE_UP, check whether we are cwnd + // limited before the current aggregation epoch, instead of before the current + // ack event. + bool probe_bw_check_cwnd_limited_before_aggregation_epoch = false; + // Pacing gains. float probe_bw_probe_up_pacing_gain = 1.25; float probe_bw_probe_down_pacing_gain = 0.75; @@ -386,6 +391,10 @@ return bandwidth_sampler_.max_ack_height(); } + bool cwnd_limited_before_aggregation_epoch() const { + return cwnd_limited_before_aggregation_epoch_; + } + void EnableOverestimateAvoidance() { bandwidth_sampler_.EnableOverestimateAvoidance(); } @@ -546,6 +555,10 @@ float cwnd_gain_; float pacing_gain_; + // Whether we are cwnd limited prior to the start of the current aggregation + // epoch. + bool cwnd_limited_before_aggregation_epoch_ = false; + // STARTUP-centric fields which experimentally used by PROBE_UP. bool full_bandwidth_reached_ = false; QuicBandwidth full_bandwidth_baseline_ = QuicBandwidth::Zero();
diff --git a/quic/core/congestion_control/bbr2_probe_bw.cc b/quic/core/congestion_control/bbr2_probe_bw.cc index ce9443f..0ebe4d0 100644 --- a/quic/core/congestion_control/bbr2_probe_bw.cc +++ b/quic/core/congestion_control/bbr2_probe_bw.cc
@@ -339,11 +339,21 @@ void Bbr2ProbeBwMode::ProbeInflightHighUpward( const Bbr2CongestionEvent& congestion_event) { QUICHE_DCHECK_EQ(cycle_.phase, CyclePhase::PROBE_UP); - if (!model_->IsCongestionWindowLimited(congestion_event)) { - QUIC_DVLOG(3) << sender_ - << " Raising inflight_hi early return: Not cwnd limited."; - // Not fully utilizing cwnd, so can't safely grow. - return; + if (Params().probe_bw_check_cwnd_limited_before_aggregation_epoch) { + if (!model_->cwnd_limited_before_aggregation_epoch()) { + QUIC_DVLOG(3) << sender_ + << " Raising inflight_hi early return: Not cwnd limited " + "before aggregation epoch."; + // Not fully utilizing cwnd, so can't safely grow. + return; + } + } else { + if (!model_->IsCongestionWindowLimited(congestion_event)) { + QUIC_DVLOG(3) << sender_ + << " Raising inflight_hi early return: Not cwnd limited."; + // Not fully utilizing cwnd, so can't safely grow. + return; + } } if (congestion_event.prior_cwnd < model_->inflight_hi()) {
diff --git a/quic/core/congestion_control/bbr2_sender.cc b/quic/core/congestion_control/bbr2_sender.cc index 09b920e..4ff7a6a 100644 --- a/quic/core/congestion_control/bbr2_sender.cc +++ b/quic/core/congestion_control/bbr2_sender.cc
@@ -158,6 +158,13 @@ if (ContainsQuicTag(connection_options, kBBQ9)) { params_.bw_lo_mode_ = Bbr2Params::QuicBandwidthLoMode::CWND_REDUCTION; } + if (GetQuicReloadableFlag( + quic_bbr2_check_cwnd_limited_before_aggregation_epoch) && + ContainsQuicTag(connection_options, kB201)) { + QUIC_RELOADABLE_FLAG_COUNT( + quic_bbr2_check_cwnd_limited_before_aggregation_epoch); + params_.probe_bw_check_cwnd_limited_before_aggregation_epoch = true; + } } Limits<QuicByteCount> Bbr2Sender::GetCwndLimitsByMode() const {
diff --git a/quic/core/congestion_control/bbr2_simulator_test.cc b/quic/core/congestion_control/bbr2_simulator_test.cc index ae6a6a6..9157e1f 100644 --- a/quic/core/congestion_control/bbr2_simulator_test.cc +++ b/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -201,6 +201,12 @@ GetUnackedMap(endpoint->connection()), kDefaultInitialCwndPackets, GetQuicFlag(FLAGS_quic_max_congestion_window), &random_, QuicConnectionPeer::GetStats(endpoint->connection()), old_sender); + if (GetQuicReloadableFlag( + quic_bbr2_check_cwnd_limited_before_aggregation_epoch)) { + // TODO(wub): Remove this option when deprecating the flag. There are + // dedicated tests for this option. + SetConnectionOption(kB201, sender); + } QuicConnectionPeer::SetSendAlgorithm(endpoint->connection(), sender); endpoint->RecordTrace(); return sender; @@ -308,11 +314,15 @@ } void SetConnectionOption(QuicTag option) { + SetConnectionOption(std::move(option), sender_); + } + + void SetConnectionOption(QuicTag option, Bbr2Sender* sender) { QuicConfig config; QuicTagVector options; options.push_back(option); QuicConfigPeer::SetReceivedConnectionOptions(&config, options); - sender_->SetFromConfig(config, Perspective::IS_SERVER); + sender->SetFromConfig(config, Perspective::IS_SERVER); } bool Bbr2ModeIsOneOf(const std::vector<Bbr2Mode>& expected_modes) const { @@ -436,6 +446,25 @@ EXPECT_APPROX_EQ(params.RTT(), rtt_stats()->min_rtt(), 0.2f); } +TEST_F(Bbr2DefaultTopologyTest, SimpleTransferB201) { + SetConnectionOption(kB201); + DefaultTopologyParams params; + CreateNetwork(params); + + // Transfer 12MB. + DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35)); + EXPECT_TRUE(Bbr2ModeIsOneOf({Bbr2Mode::PROBE_BW, Bbr2Mode::PROBE_RTT})); + + EXPECT_APPROX_EQ(params.BottleneckBandwidth(), + sender_->ExportDebugState().bandwidth_hi, 0.01f); + + EXPECT_LE(sender_loss_rate_in_packets(), 0.05); + // The margin here is high, because the aggregation greatly increases + // smoothed rtt. + EXPECT_GE(params.RTT() * 4, rtt_stats()->smoothed_rtt()); + EXPECT_APPROX_EQ(params.RTT(), rtt_stats()->min_rtt(), 0.2f); +} + TEST_F(Bbr2DefaultTopologyTest, SimpleTransferSmallBuffer) { DefaultTopologyParams params; params.switch_queue_capacity_in_bdp = 0.5; @@ -488,6 +517,32 @@ EXPECT_APPROX_EQ(params.RTT(), rtt_stats()->min_rtt(), 0.2f); } +TEST_F(Bbr2DefaultTopologyTest, SimpleTransfer2RTTAggregationBytesB201) { + SetConnectionOption(kB201); + DefaultTopologyParams params; + CreateNetwork(params); + // 2 RTTs of aggregation, with a max of 10kb. + EnableAggregation(10 * 1024, 2 * params.RTT()); + + // Transfer 12MB. + DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35)); + EXPECT_TRUE(Bbr2ModeIsOneOf({Bbr2Mode::PROBE_BW, Bbr2Mode::PROBE_RTT})); + + // TODO(wub): Tighten the error bound once BSAO is default enabled. + EXPECT_APPROX_EQ(params.BottleneckBandwidth(), + sender_->ExportDebugState().bandwidth_hi, 0.5f); + + if (GetQuicReloadableFlag(quic_fix_pacing_sender_bursts)) { + EXPECT_EQ(sender_loss_rate_in_packets(), 0); + } else { + EXPECT_LE(sender_loss_rate_in_packets(), 0.05); + } + // The margin here is high, because both link level aggregation and ack + // decimation can greatly increase smoothed rtt. + EXPECT_GE(params.RTT() * 5, rtt_stats()->smoothed_rtt()); + EXPECT_APPROX_EQ(params.RTT(), rtt_stats()->min_rtt(), 0.2f); +} + TEST_F(Bbr2DefaultTopologyTest, SimpleTransferAckDecimation) { SetConnectionOption(kBSAO); DefaultTopologyParams params;
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h index 26d26d5..15f66e2 100644 --- a/quic/core/crypto/crypto_protocol.h +++ b/quic/core/crypto/crypto_protocol.h
@@ -151,6 +151,9 @@ // aggregation const QuicTag kB2DL = TAG('B', '2', 'D', 'L'); // Increase inflight_hi based // on delievered, not inflight. +const QuicTag kB201 = TAG('B', '2', '0', '1'); // In PROBE_UP, check if cwnd + // limited before aggregation + // epoch, instead of ack event. const QuicTag kNTLP = TAG('N', 'T', 'L', 'P'); // No tail loss probe const QuicTag k1TLP = TAG('1', 'T', 'L', 'P'); // 1 tail loss probe const QuicTag k1RTO = TAG('1', 'R', 'T', 'O'); // Send 1 packet upon RTO
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index c4af7b4..db08117 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -17,6 +17,8 @@ QUIC_FLAG(FLAGS_quic_restart_flag_quic_testonly_default_true, true) // If true and a QUIC connection is traced, add ssl events to the trace. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_trace_ssl_events, true) +// If true and connection option B201 is used, check if cwnd limited before aggregation epoch, instead of ack event, in PROBE_UP phase. +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_check_cwnd_limited_before_aggregation_epoch, true) // If true, GFE will explicitly configure its signature algorithm preference. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_set_signature_algorithm_prefs, true) // If true, QUIC will default enable MTU discovery at server, with a target of 1450 bytes.