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.