Index: net/quic/core/congestion_control/bbr_sender_test.cc |
diff --git a/net/quic/core/congestion_control/bbr_sender_test.cc b/net/quic/core/congestion_control/bbr_sender_test.cc |
index b0813e059dfd7afedf6713c8ac289d3c6bac4b33..f89c8ad8944470dafd3e9403ec6a632ad95693ec 100644 |
--- a/net/quic/core/congestion_control/bbr_sender_test.cc |
+++ b/net/quic/core/congestion_control/bbr_sender_test.cc |
@@ -90,6 +90,8 @@ class BbrSenderTest : public ::testing::Test { |
/*connection_id=*/GetPeerInMemoryConnectionId(43)), |
receiver_multiplexer_("Receiver multiplexer", |
{&receiver_, &competing_receiver_}) { |
+ // TODO(ianswett): Determine why tests become flaky with CWND based on SRTT. |
+ FLAGS_quic_reloadable_flag_quic_bbr_base_cwnd_on_srtt = false; |
rtt_stats_ = bbr_sender_.connection()->sent_packet_manager().GetRttStats(); |
sender_ = SetupBbrSender(&bbr_sender_); |
@@ -289,9 +291,8 @@ TEST_F(BbrSenderTest, SimpleTransferSmallBuffer) { |
} |
// Test a simple long data transfer with 2 rtts of aggregation. |
-TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregation) { |
- QuicFlagSaver flags; |
- FLAGS_quic_reloadable_flag_quic_bbr_ack_spacing2 = true; |
+TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes) { |
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = true; |
FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = false; |
CreateDefaultSetup(); |
// 2 RTTs of aggregation, with a max of 10kb. |
@@ -309,17 +310,17 @@ TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregation) { |
sender_->ExportDebugState().max_bandwidth); |
// TODO(ianswett): Expect 0 packets are lost once BBR no longer measures |
// bandwidth higher than the link rate. |
+ EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); |
// The margin here is high, because the aggregation greatly increases |
// smoothed rtt. |
EXPECT_GE(kTestRtt * 4, rtt_stats_->smoothed_rtt()); |
ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.1f); |
} |
-// Test a simple long data transfer with 2 rtts of aggregation. |
-TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes) { |
- FLAGS_quic_reloadable_flag_quic_bbr_ack_spacing2 = false; |
- FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = true; |
+TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationKeepSending) { |
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = false; |
FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = false; |
+ FLAGS_quic_reloadable_flag_quic_bbr_keep_sending_at_recent_rate = true; |
CreateDefaultSetup(); |
// 2 RTTs of aggregation, with a max of 10kb. |
EnableAggregation(10 * 1024, 2 * kTestRtt); |
@@ -328,7 +329,7 @@ TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes) { |
DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35)); |
EXPECT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode); |
// It's possible to read a bandwidth as much as 50% too high with aggregation. |
- EXPECT_LE(kTestLinkBandwidth * 0.99f, |
+ EXPECT_LE(kTestLinkBandwidth * 0.95f, |
sender_->ExportDebugState().max_bandwidth); |
// TODO(ianswett): Tighten this bound once we understand why BBR is |
// overestimating bandwidth with aggregation. b/36022633 |
@@ -339,16 +340,15 @@ TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes) { |
EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); |
// The margin here is high, because the aggregation greatly increases |
// smoothed rtt. |
- EXPECT_GE(kTestRtt * 4, rtt_stats_->smoothed_rtt()); |
+ EXPECT_GE(kTestRtt * 4.5, rtt_stats_->smoothed_rtt()); |
ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.1f); |
} |
// Test a simple long data transfer with 2 rtts of aggregation. |
TEST_F(BbrSenderTest, SimpleTransferAckDecimation) { |
- FLAGS_quic_reloadable_flag_quic_bbr_ack_spacing2 = false; |
FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = true; |
// Decrease the CWND gain so extra CWND is required with stretch acks. |
- base::SetFlag(&FLAGS_quic_bbr_cwnd_gain, 1.0); |
+ SetQuicFlag(&FLAGS_quic_bbr_cwnd_gain, 1.0); |
sender_ = new BbrSender( |
rtt_stats_, |
QuicSentPacketManagerPeer::GetUnackedPacketMap( |
@@ -380,6 +380,43 @@ TEST_F(BbrSenderTest, SimpleTransferAckDecimation) { |
ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.1f); |
} |
+TEST_F(BbrSenderTest, SimpleTransferAckDecimationKeepSending) { |
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = false; |
+ FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = true; |
+ FLAGS_quic_reloadable_flag_quic_bbr_keep_sending_at_recent_rate = true; |
+ // Decrease the CWND gain so extra CWND is required with stretch acks. |
+ SetQuicFlag(&FLAGS_quic_bbr_cwnd_gain, 1.0); |
+ sender_ = new BbrSender( |
+ rtt_stats_, |
+ QuicSentPacketManagerPeer::GetUnackedPacketMap( |
+ QuicConnectionPeer::GetSentPacketManager(bbr_sender_.connection())), |
+ kInitialCongestionWindowPackets, kDefaultMaxCongestionWindowPackets, |
+ &random_); |
+ QuicConnectionPeer::SetSendAlgorithm(bbr_sender_.connection(), sender_); |
+ // Enable Ack Decimation on the receiver. |
+ QuicConnectionPeer::SetAckMode(receiver_.connection(), |
+ QuicConnection::AckMode::ACK_DECIMATION); |
+ CreateDefaultSetup(); |
+ |
+ // Transfer 12MB. |
+ DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35)); |
+ EXPECT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode); |
+ // It's possible to read a bandwidth as much as 50% too high with aggregation. |
+ EXPECT_LE(kTestLinkBandwidth * 0.95f, |
+ sender_->ExportDebugState().max_bandwidth); |
+ // TODO(ianswett): Tighten this bound once we understand why BBR is |
+ // overestimating bandwidth with aggregation. b/36022633 |
+ EXPECT_GE(kTestLinkBandwidth * 1.5f, |
+ sender_->ExportDebugState().max_bandwidth); |
+ // TODO(ianswett): Expect 0 packets are lost once BBR no longer measures |
+ // bandwidth higher than the link rate. |
+ EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited); |
+ // The margin here is high, because the aggregation greatly increases |
+ // smoothed rtt. |
+ EXPECT_GE(kTestRtt * 2, rtt_stats_->smoothed_rtt()); |
+ ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.1f); |
+} |
+ |
// Test the number of losses incurred by the startup phase in a situation when |
// the buffer is less than BDP. |
TEST_F(BbrSenderTest, PacketLossOnSmallBufferStartup) { |