Deprecate gfe2_reloadable_flag_quic_simplify_received_packet_manager_ack & gfe2_reloadable_flag_quic_remove_unused_ack_options. PiperOrigin-RevId: 330594392 Change-Id: Id628288b713fef6af670363523941d504c2f12ed
diff --git a/quic/core/congestion_control/bbr2_simulator_test.cc b/quic/core/congestion_control/bbr2_simulator_test.cc index 1ebe60c..0dcf427 100644 --- a/quic/core/congestion_control/bbr2_simulator_test.cc +++ b/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -449,9 +449,6 @@ TEST_F(Bbr2DefaultTopologyTest, SimpleTransferAckDecimation) { SetConnectionOption(kBSAO); - // Enable Ack Decimation on the receiver. - QuicConnectionPeer::SetAckMode(receiver_endpoint_.connection(), - AckMode::ACK_DECIMATION); DefaultTopologyParams params; CreateNetwork(params);
diff --git a/quic/core/congestion_control/bbr_sender_test.cc b/quic/core/congestion_control/bbr_sender_test.cc index 0e94b6b..e0ec278 100644 --- a/quic/core/congestion_control/bbr_sender_test.cc +++ b/quic/core/congestion_control/bbr_sender_test.cc
@@ -292,8 +292,6 @@ // Test a simple long data transfer in the default setup. TEST_F(BbrSenderTest, SimpleTransfer) { - // 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. @@ -341,8 +339,6 @@ } TEST_F(BbrSenderTest, RemoveBytesLostInRecovery) { - // Disable Ack Decimation on the receiver, because it can increase srtt. - QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING); CreateDefaultSetup(); DriveOutOfStartup(); @@ -421,9 +417,6 @@ GetQuicFlag(FLAGS_quic_max_congestion_window), &random_, QuicConnectionPeer::GetStats(bbr_sender_.connection())); QuicConnectionPeer::SetSendAlgorithm(bbr_sender_.connection(), sender_); - // Enable Ack Decimation on the receiver. - QuicConnectionPeer::SetAckMode(receiver_.connection(), - AckMode::ACK_DECIMATION); CreateDefaultSetup(); // Transfer 12MB. @@ -445,8 +438,6 @@ // Test a simple long data transfer with 2 rtts of aggregation. TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes20RTTWindow) { SetConnectionOption(kBSAO); - // Disable Ack Decimation on the receiver, because it can increase srtt. - QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING); CreateDefaultSetup(); SetConnectionOption(kBBR4); // 2 RTTs of aggregation, with a max of 10kb. @@ -471,8 +462,6 @@ // Test a simple long data transfer with 2 rtts of aggregation. TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes40RTTWindow) { SetConnectionOption(kBSAO); - // Disable Ack Decimation on the receiver, because it can increase srtt. - QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING); CreateDefaultSetup(); SetConnectionOption(kBBR5); // 2 RTTs of aggregation, with a max of 10kb. @@ -590,8 +579,6 @@ // Verify that the DRAIN phase works correctly. TEST_F(BbrSenderTest, Drain) { - // Disable Ack Decimation on the receiver, because it can increase srtt. - QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING); CreateDefaultSetup(); const QuicTime::Delta timeout = QuicTime::Delta::FromSeconds(10); // Get the queue at the bottleneck, which is the outgoing queue at the port to @@ -654,10 +641,6 @@ // TODO(wub): Re-enable this test once default drain_gain changed to 0.75. // Verify that the DRAIN phase works correctly. TEST_F(BbrSenderTest, DISABLED_ShallowDrain) { - // TODO(haoyuewang) Remove this when TCP_ACKING is deprecated. - // Disable Ack Decimation on the receiver, because it can increase srtt. - QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING); - CreateDefaultSetup(); const QuicTime::Delta timeout = QuicTime::Delta::FromSeconds(10); // Get the queue at the bottleneck, which is the outgoing queue at the port to @@ -746,8 +729,6 @@ // bandwidth will not exit high gain phase, and similarly ensure that the // connection will exit low gain early if the number of bytes in flight is low. TEST_F(BbrSenderTest, InFlightAwareGainCycling) { - // Disable Ack Decimation on the receiver, because it can increase srtt. - QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING); CreateDefaultSetup(); DriveOutOfStartup(); @@ -987,9 +968,6 @@ } TEST_F(BbrSenderTest, AckAggregationInStartup) { - // Disable Ack Decimation on the receiver to avoid loss and make results - // consistent. - QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING); CreateDefaultSetup(); SetConnectionOption(kBBQ3);
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 49ccd97..90b802d 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -1767,7 +1767,6 @@ uber_received_packet_manager_.MaybeUpdateAckTimeout( should_last_packet_instigate_acks_, last_decrypted_packet_level_, last_header_.packet_number, - idle_network_detector_.time_of_last_received_packet(), clock_->ApproximateNow(), sent_packet_manager_.GetRttStats()); } @@ -4847,7 +4846,6 @@ uber_received_packet_manager_.MaybeUpdateAckTimeout( /*should_last_packet_instigate_acks=*/true, last_decrypted_packet_level_, last_header_.packet_number, - idle_network_detector_.time_of_last_received_packet(), clock_->ApproximateNow(), sent_packet_manager_.GetRttStats()); }
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 8136ccb..2662ab7 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -2993,7 +2993,6 @@ QuicTime::Delta::Zero(), QuicTime::Zero()); EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); - QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); // Start ack decimation from 10th packet. connection_.set_min_received_before_ack_decimation(10); @@ -6287,85 +6286,8 @@ EXPECT_FALSE(connection_.HasPendingAcks()); } -TEST_P(QuicConnectionTest, SendDelayedAfterQuiescence) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - QuicConnectionPeer::SetFastAckAfterQuiescence(&connection_, true); - - // The beginning of the connection counts as quiescence. - QuicTime ack_time = clock_.ApproximateNow() + kAlarmGranularity; - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - EXPECT_FALSE(connection_.HasPendingAcks()); - const uint8_t tag = 0x07; - SetDecrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<StrictTaggingDecrypter>(tag)); - peer_framer_.SetEncrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<TaggingEncrypter>(tag)); - // Process a packet from the non-crypto stream. - frame1_.stream_id = 3; - - // The same as ProcessPacket(1) except that ENCRYPTION_ZERO_RTT is used - // instead of ENCRYPTION_INITIAL. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(1, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(DefaultDelayedAckTime()); - connection_.GetAckAlarm()->Fire(); - // Check that ack is sent and that delayed ack alarm is reset. - size_t padding_frame_count = writer_->padding_frames().size(); - if (GetParam().no_stop_waiting) { - EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); - - // Process another packet immedately after sending the ack and expect the - // ack alarm to be set delayed ack time in the future. - ack_time = clock_.ApproximateNow() + DefaultDelayedAckTime(); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(2, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(DefaultDelayedAckTime()); - connection_.GetAckAlarm()->Fire(); - // Check that ack is sent and that delayed ack alarm is reset. - padding_frame_count = writer_->padding_frames().size(); - if (GetParam().no_stop_waiting) { - EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); - - // Wait 1 second and ensure the ack alarm is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(3, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); -} - TEST_P(QuicConnectionTest, SendDelayedAckDecimation) { EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); - QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION); const size_t kMinRttMs = 40; RttStats* rtt_stats = const_cast<RttStats*>(manager_->GetRttStats()); @@ -6422,141 +6344,6 @@ EXPECT_FALSE(connection_.HasPendingAcks()); } -TEST_P(QuicConnectionTest, SendDelayedAckAckDecimationAfterQuiescence) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); - QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION); - QuicConnectionPeer::SetFastAckAfterQuiescence(&connection_, true); - - const size_t kMinRttMs = 40; - RttStats* rtt_stats = const_cast<RttStats*>(manager_->GetRttStats()); - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(kMinRttMs), - QuicTime::Delta::Zero(), QuicTime::Zero()); - - // The beginning of the connection counts as quiescence. - QuicTime ack_time = - clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - EXPECT_FALSE(connection_.HasPendingAcks()); - const uint8_t tag = 0x07; - SetDecrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<StrictTaggingDecrypter>(tag)); - peer_framer_.SetEncrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<TaggingEncrypter>(tag)); - // Process a packet from the non-crypto stream. - frame1_.stream_id = 3; - - // The same as ProcessPacket(1) except that ENCRYPTION_ZERO_RTT is used - // instead of ENCRYPTION_INITIAL. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(1, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(DefaultDelayedAckTime()); - connection_.GetAckAlarm()->Fire(); - // Check that ack is sent and that delayed ack alarm is reset. - size_t padding_frame_count = writer_->padding_frames().size(); - if (GetParam().no_stop_waiting) { - EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); - - // Process another packet immedately after sending the ack and expect the - // ack alarm to be set delayed ack time in the future. - ack_time = clock_.ApproximateNow() + DefaultDelayedAckTime(); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(2, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(DefaultDelayedAckTime()); - connection_.GetAckAlarm()->Fire(); - // Check that ack is sent and that delayed ack alarm is reset. - padding_frame_count = writer_->padding_frames().size(); - if (GetParam().no_stop_waiting) { - EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); - - // Wait 1 second and enesure the ack alarm is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(3, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // Process enough packets to get into ack decimation behavior. - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - ack_time = clock_.ApproximateNow() + - QuicTime::Delta::FromMilliseconds(kMinRttMs / 4); - uint64_t kFirstDecimatedPacket = 101; - for (unsigned int i = 0; i < kFirstDecimatedPacket - 4; ++i) { - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(4 + i, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - } - EXPECT_FALSE(connection_.HasPendingAcks()); - // The same as ProcessPacket(1) except that ENCRYPTION_ZERO_RTT is used - // instead of ENCRYPTION_INITIAL. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // The 10th received packet causes an ack to be sent. - for (int i = 0; i < 9; ++i) { - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 1 + i, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - } - // Check that ack is sent and that delayed ack alarm is reset. - padding_frame_count = writer_->padding_frames().size(); - if (GetParam().no_stop_waiting) { - EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); - - // Wait 1 second and enesure the ack alarm is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 10, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); -} - TEST_P(QuicConnectionTest, SendDelayedAckDecimationUnlimitedAggregation) { EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); @@ -6618,7 +6405,6 @@ TEST_P(QuicConnectionTest, SendDelayedAckDecimationEighthRtt) { EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); - QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION); QuicConnectionPeer::SetAckDecimationDelay(&connection_, 0.125); const size_t kMinRttMs = 40; @@ -6676,319 +6462,6 @@ EXPECT_FALSE(connection_.HasPendingAcks()); } -TEST_P(QuicConnectionTest, SendDelayedAckDecimationWithReordering) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); - QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); - - const size_t kMinRttMs = 40; - RttStats* rtt_stats = const_cast<RttStats*>(manager_->GetRttStats()); - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(kMinRttMs), - QuicTime::Delta::Zero(), QuicTime::Zero()); - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + - QuicTime::Delta::FromMilliseconds(kMinRttMs / 4); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - EXPECT_FALSE(connection_.HasPendingAcks()); - const uint8_t tag = 0x07; - SetDecrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<StrictTaggingDecrypter>(tag)); - peer_framer_.SetEncrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<TaggingEncrypter>(tag)); - // Process a packet from the non-crypto stream. - frame1_.stream_id = 3; - - // Process all the initial packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (unsigned int i = 0; i < kFirstDecimatedPacket - 1; ++i) { - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(1 + i, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - } - EXPECT_FALSE(connection_.HasPendingAcks()); - - // Receive one packet out of order and then the rest in order. - // The loop leaves a one packet gap between acks sent to simulate some loss. - for (int j = 0; j < 3; ++j) { - // Process packet 10 first and ensure the alarm is one eighth min_rtt. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 9 + (j * 11), - !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(5); - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // The 10th received packet causes an ack to be sent. - writer_->Reset(); - for (int i = 0; i < 9; ++i) { - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - // The ACK shouldn't be sent until the 10th packet is processed. - EXPECT_TRUE(writer_->ack_frames().empty()); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + i + (j * 11), - !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - } - // Check that ack is sent and that delayed ack alarm is reset. - size_t padding_frame_count = writer_->padding_frames().size(); - if (GetParam().no_stop_waiting) { - EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); - } -} - -TEST_P(QuicConnectionTest, SendDelayedAckDecimationWithLargeReordering) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); - QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); - - const size_t kMinRttMs = 40; - RttStats* rtt_stats = const_cast<RttStats*>(manager_->GetRttStats()); - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(kMinRttMs), - QuicTime::Delta::Zero(), QuicTime::Zero()); - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + - QuicTime::Delta::FromMilliseconds(kMinRttMs / 4); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - EXPECT_FALSE(connection_.HasPendingAcks()); - const uint8_t tag = 0x07; - SetDecrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<StrictTaggingDecrypter>(tag)); - peer_framer_.SetEncrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<TaggingEncrypter>(tag)); - // Process a packet from the non-crypto stream. - frame1_.stream_id = 3; - - // Process all the initial packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (unsigned int i = 0; i < kFirstDecimatedPacket - 1; ++i) { - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(1 + i, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - } - EXPECT_FALSE(connection_.HasPendingAcks()); - // The same as ProcessPacket(1) except that ENCRYPTION_ZERO_RTT is used - // instead of ENCRYPTION_INITIAL. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // Process packet 10 first and ensure the alarm is one eighth min_rtt. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 19, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(5); - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // The 10th received packet causes an ack to be sent. - for (int i = 0; i < 8; ++i) { - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 1 + i, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - } - // Check that ack is sent and that delayed ack alarm is reset. - if (GetParam().no_stop_waiting) { - EXPECT_EQ(writer_->padding_frames().size() + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); - - // The next packet received in order will cause an immediate ack, - // because it fills a hole. - EXPECT_FALSE(connection_.HasPendingAcks()); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 10, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - // Check that ack is sent and that delayed ack alarm is reset. - if (GetParam().no_stop_waiting) { - EXPECT_EQ(writer_->padding_frames().size() + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); -} - -TEST_P(QuicConnectionTest, SendDelayedAckDecimationWithReorderingEighthRtt) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); - QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); - QuicConnectionPeer::SetAckDecimationDelay(&connection_, 0.125); - - const size_t kMinRttMs = 40; - RttStats* rtt_stats = const_cast<RttStats*>(manager_->GetRttStats()); - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(kMinRttMs), - QuicTime::Delta::Zero(), QuicTime::Zero()); - // The ack time should be based on min_rtt/8, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + - QuicTime::Delta::FromMilliseconds(kMinRttMs / 8); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - EXPECT_FALSE(connection_.HasPendingAcks()); - const uint8_t tag = 0x07; - SetDecrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<StrictTaggingDecrypter>(tag)); - peer_framer_.SetEncrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<TaggingEncrypter>(tag)); - // Process a packet from the non-crypto stream. - frame1_.stream_id = 3; - - // Process all the initial packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (unsigned int i = 0; i < kFirstDecimatedPacket - 1; ++i) { - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(1 + i, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - } - EXPECT_FALSE(connection_.HasPendingAcks()); - // The same as ProcessPacket(1) except that ENCRYPTION_ZERO_RTT is used - // instead of ENCRYPTION_INITIAL. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // Process packet 10 first and ensure the alarm is one eighth min_rtt. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 9, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(5); - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // The 10th received packet causes an ack to be sent. - for (int i = 0; i < 8; ++i) { - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 1 + i, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - } - // Check that ack is sent and that delayed ack alarm is reset. - size_t padding_frame_count = writer_->padding_frames().size(); - if (GetParam().no_stop_waiting) { - EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); -} - -TEST_P(QuicConnectionTest, - SendDelayedAckDecimationWithLargeReorderingEighthRtt) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); - QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); - QuicConnectionPeer::SetAckDecimationDelay(&connection_, 0.125); - - const size_t kMinRttMs = 40; - RttStats* rtt_stats = const_cast<RttStats*>(manager_->GetRttStats()); - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(kMinRttMs), - QuicTime::Delta::Zero(), QuicTime::Zero()); - // The ack time should be based on min_rtt/8, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + - QuicTime::Delta::FromMilliseconds(kMinRttMs / 8); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - EXPECT_FALSE(connection_.HasPendingAcks()); - const uint8_t tag = 0x07; - SetDecrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<StrictTaggingDecrypter>(tag)); - peer_framer_.SetEncrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<TaggingEncrypter>(tag)); - // Process a packet from the non-crypto stream. - frame1_.stream_id = 3; - - // Process all the initial packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (unsigned int i = 0; i < kFirstDecimatedPacket - 1; ++i) { - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(1 + i, !kHasStopWaiting, ENCRYPTION_ZERO_RTT); - } - EXPECT_FALSE(connection_.HasPendingAcks()); - // The same as ProcessPacket(1) except that ENCRYPTION_ZERO_RTT is used - // instead of ENCRYPTION_INITIAL. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - - // Check if delayed ack timer is running for the expected interval. - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // Process packet 10 first and ensure the alarm is one eighth min_rtt. - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 19, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(5); - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); - - // The 10th received packet causes an ack to be sent. - for (int i = 0; i < 8; ++i) { - EXPECT_TRUE(connection_.HasPendingAcks()); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 1 + i, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - } - // Check that ack is sent and that delayed ack alarm is reset. - if (GetParam().no_stop_waiting) { - EXPECT_EQ(writer_->padding_frames().size() + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); - - // The next packet received in order will cause an immediate ack, - // because it fills a hole. - EXPECT_FALSE(connection_.HasPendingAcks()); - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); - ProcessDataPacketAtLevel(kFirstDecimatedPacket + 10, !kHasStopWaiting, - ENCRYPTION_ZERO_RTT); - // Check that ack is sent and that delayed ack alarm is reset. - if (GetParam().no_stop_waiting) { - EXPECT_EQ(writer_->padding_frames().size() + 1u, writer_->frame_count()); - EXPECT_TRUE(writer_->stop_waiting_frames().empty()); - } else { - EXPECT_EQ(2u, writer_->frame_count()); - EXPECT_FALSE(writer_->stop_waiting_frames().empty()); - } - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(connection_.HasPendingAcks()); -} - TEST_P(QuicConnectionTest, SendDelayedAckOnHandshakeConfirmed) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); ProcessPacket(1);
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc index 539cdce..1c9218a 100644 --- a/quic/core/quic_received_packet_manager.cc +++ b/quic/core/quic_received_packet_manager.cc
@@ -41,13 +41,11 @@ time_largest_observed_(QuicTime::Zero()), save_timestamps_(false), stats_(stats), - ack_mode_(ACK_DECIMATION), num_retransmittable_packets_received_since_last_ack_sent_(0), min_received_before_ack_decimation_(kMinReceivedBeforeAckDecimation), ack_frequency_(kDefaultRetransmittablePacketsBeforeAck), ack_decimation_delay_(kAckDecimationDelay), unlimited_ack_decimation_(false), - fast_ack_after_quiescence_(false), one_immediate_ack_(false), local_max_ack_delay_( QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs)), @@ -59,40 +57,12 @@ void QuicReceivedPacketManager::SetFromConfig(const QuicConfig& config, Perspective perspective) { - if (remove_unused_ack_options_) { - QUIC_RELOADABLE_FLAG_COUNT(quic_remove_unused_ack_options); - } - if (!remove_unused_ack_options_) { - if (config.HasClientSentConnectionOption(kACD0, perspective)) { - ack_mode_ = TCP_ACKING; - } - if (config.HasClientSentConnectionOption(kACKD, perspective)) { - ack_mode_ = ACK_DECIMATION; - } - if (config.HasClientSentConnectionOption(kAKD2, perspective)) { - ack_mode_ = ACK_DECIMATION_WITH_REORDERING; - } - } if (config.HasClientSentConnectionOption(kAKD3, perspective)) { - if (!remove_unused_ack_options_) { - ack_mode_ = ACK_DECIMATION; - } ack_decimation_delay_ = kShortAckDecimationDelay; } - if (!remove_unused_ack_options_) { - if (config.HasClientSentConnectionOption(kAKD4, perspective)) { - ack_mode_ = ACK_DECIMATION_WITH_REORDERING; - ack_decimation_delay_ = kShortAckDecimationDelay; - } - } if (config.HasClientSentConnectionOption(kAKDU, perspective)) { unlimited_ack_decimation_ = true; } - if (!remove_unused_ack_options_) { - if (config.HasClientSentConnectionOption(kACKQ, perspective)) { - fast_ack_after_quiescence_ = true; - } - } if (config.HasClientSentConnectionOption(k1ACK, perspective)) { one_immediate_ack_ = true; } @@ -224,7 +194,6 @@ QuicTime::Delta QuicReceivedPacketManager::GetMaxAckDelay( QuicPacketNumber last_received_packet_number, const RttStats& rtt_stats) const { - DCHECK(simplify_received_packet_manager_ack_); if (last_received_packet_number < PeerFirstSendingPacketNumber() + min_received_before_ack_decimation_) { return local_max_ack_delay_; @@ -243,7 +212,6 @@ void QuicReceivedPacketManager::MaybeUpdateAckFrequency( QuicPacketNumber last_received_packet_number) { - DCHECK(simplify_received_packet_manager_ack_); if (last_received_packet_number < PeerFirstSendingPacketNumber() + min_received_before_ack_decimation_) { return; @@ -256,7 +224,6 @@ void QuicReceivedPacketManager::MaybeUpdateAckTimeout( bool should_last_packet_instigate_acks, QuicPacketNumber last_received_packet_number, - QuicTime time_of_last_received_packet, QuicTime now, const RttStats* rtt_stats) { if (!ack_frame_updated_) { @@ -278,85 +245,20 @@ ++num_retransmittable_packets_received_since_last_ack_sent_; - if (simplify_received_packet_manager_ack_) { - QUIC_RELOADABLE_FLAG_COUNT(quic_simplify_received_packet_manager_ack); - MaybeUpdateAckFrequency(last_received_packet_number); - if (num_retransmittable_packets_received_since_last_ack_sent_ >= - ack_frequency_) { - ack_timeout_ = now; - return; - } - - if (HasNewMissingPackets()) { - ack_timeout_ = now; - return; - } - - MaybeUpdateAckTimeoutTo( - now + GetMaxAckDelay(last_received_packet_number, *rtt_stats)); + MaybeUpdateAckFrequency(last_received_packet_number); + if (num_retransmittable_packets_received_since_last_ack_sent_ >= + ack_frequency_) { + ack_timeout_ = now; return; } - if (ack_mode_ != TCP_ACKING && - last_received_packet_number >= PeerFirstSendingPacketNumber() + - min_received_before_ack_decimation_) { - // Ack up to 10 packets at once unless ack decimation is unlimited. - if (!unlimited_ack_decimation_ && - num_retransmittable_packets_received_since_last_ack_sent_ >= - kMaxRetransmittablePacketsBeforeAck) { - ack_timeout_ = now; - return; - } - // Wait for the minimum of the ack decimation delay or the delayed ack time - // before sending an ack. - QuicTime::Delta ack_delay = std::min( - local_max_ack_delay_, rtt_stats->min_rtt() * ack_decimation_delay_); - if (GetQuicReloadableFlag(quic_ack_delay_alarm_granularity)) { - QUIC_RELOADABLE_FLAG_COUNT(quic_ack_delay_alarm_granularity); - ack_delay = std::max(ack_delay, kAlarmGranularity); - } - if (fast_ack_after_quiescence_ && now - time_of_previous_received_packet_ > - rtt_stats->SmoothedOrInitialRtt()) { - // Ack the first packet out of queiscence faster, because QUIC does - // not pace the first few packets and commonly these may be handshake - // or TLP packets, which we'd like to acknowledge quickly. - ack_delay = kAlarmGranularity; - } - MaybeUpdateAckTimeoutTo(now + ack_delay); - } else { - // Ack with a timer or every 2 packets by default. - if (num_retransmittable_packets_received_since_last_ack_sent_ >= - ack_frequency_) { - ack_timeout_ = now; - } else if (fast_ack_after_quiescence_ && - (now - time_of_previous_received_packet_) > - rtt_stats->SmoothedOrInitialRtt()) { - // Ack the first packet out of queiscence faster, because QUIC does - // not pace the first few packets and commonly these may be handshake - // or TLP packets, which we'd like to acknowledge quickly. - MaybeUpdateAckTimeoutTo(now + kAlarmGranularity); - } else { - MaybeUpdateAckTimeoutTo(now + local_max_ack_delay_); - } - } - - // If there are new missing packets to report, send an ack immediately. if (HasNewMissingPackets()) { - // TODO(haoyuewang) Remove ACK_DECIMATION_WITH_REORDERING after - // quic_remove_unused_ack_options is deprecated. - if (ack_mode_ == ACK_DECIMATION_WITH_REORDERING) { - DCHECK(!remove_unused_ack_options_); - // Wait the minimum of an eighth min_rtt and the existing ack time. - QuicTime ack_time = now + kShortAckDecimationDelay * rtt_stats->min_rtt(); - MaybeUpdateAckTimeoutTo(ack_time); - } else { - ack_timeout_ = now; - } + ack_timeout_ = now; + return; } - if (fast_ack_after_quiescence_) { - time_of_previous_received_packet_ = time_of_last_received_packet; - } + MaybeUpdateAckTimeoutTo( + now + GetMaxAckDelay(last_received_packet_number, *rtt_stats)); } void QuicReceivedPacketManager::ResetAckStates() {
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h index 5f5525a..c8b55c4 100644 --- a/quic/core/quic_received_packet_manager.h +++ b/quic/core/quic_received_packet_manager.h
@@ -62,7 +62,6 @@ // Otherwise, ACK needs to be sent by the specified time. void MaybeUpdateAckTimeout(bool should_last_packet_instigate_acks, QuicPacketNumber last_received_packet_number, - QuicTime time_of_last_received_packet, QuicTime now, const RttStats* rtt_stats); @@ -165,7 +164,6 @@ QuicConnectionStats* stats_; - AckMode ack_mode_; // How many retransmittable packets have arrived without sending an ack. QuicPacketCount num_retransmittable_packets_received_since_last_ack_sent_; // Ack decimation will start happening after this many packets are received. @@ -177,11 +175,6 @@ // When true, removes ack decimation's max number of packets(10) before // sending an ack. bool unlimited_ack_decimation_; - // When true, use a 1ms delayed ack timer if it's been an SRTT since a packet - // was received. - // TODO(haoyuewang) Remove fast_ack_after_quiescence_ when - // quic_remove_unused_ack_options flag is deprecated. - bool fast_ack_after_quiescence_; // When true, only send 1 immediate ACK when reordering is detected. bool one_immediate_ack_; @@ -197,15 +190,6 @@ // Whether the most recent packet was missing before it was received. bool was_last_packet_missing_; - // TODO(haoyuewang) Remove TCP_ACKING when - // fast_ack_after_quiescence_ when this flag is deprecated. - const bool remove_unused_ack_options_ = - GetQuicReloadableFlag(quic_remove_unused_ack_options); - - const bool simplify_received_packet_manager_ack_ = - remove_unused_ack_options_ && - GetQuicReloadableFlag(quic_simplify_received_packet_manager_ack); - // Last sent largest acked, which gets updated when ACK was successfully sent. QuicPacketNumber last_sent_largest_acked_; };
diff --git a/quic/core/quic_received_packet_manager_test.cc b/quic/core/quic_received_packet_manager_test.cc index ca56662..2cf8227 100644 --- a/quic/core/quic_received_packet_manager_test.cc +++ b/quic/core/quic_received_packet_manager_test.cc
@@ -22,15 +22,6 @@ class QuicReceivedPacketManagerPeer { public: - static void SetAckMode(QuicReceivedPacketManager* manager, AckMode ack_mode) { - manager->ack_mode_ = ack_mode; - } - - static void SetFastAckAfterQuiescence(QuicReceivedPacketManager* manager, - bool fast_ack_after_quiescence) { - manager->fast_ack_after_quiescence_ = fast_ack_after_quiescence; - } - static void SetOneImmediateAck(QuicReceivedPacketManager* manager, bool one_immediate_ack) { manager->one_immediate_ack_ = one_immediate_ack; @@ -97,7 +88,7 @@ received_manager_.MaybeUpdateAckTimeout( should_last_packet_instigate_acks, QuicPacketNumber(last_received_packet_number), clock_.ApproximateNow(), - clock_.ApproximateNow(), &rtt_stats_); + &rtt_stats_); } void CheckAckTimeout(QuicTime time) { @@ -368,8 +359,6 @@ TEST_P(QuicReceivedPacketManagerTest, AckDecimationReducesAcks) { EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, - ACK_DECIMATION_WITH_REORDERING); // Start ack decimation from 10th packet. received_manager_.set_min_received_before_ack_decimation(10); @@ -401,45 +390,8 @@ CheckAckTimeout(clock_.ApproximateNow()); } -TEST_P(QuicReceivedPacketManagerTest, SendDelayedAfterQuiescence) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetFastAckAfterQuiescence(&received_manager_, - true); - // The beginning of the connection counts as quiescence. - QuicTime ack_time = - clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - - RecordPacketReceipt(1, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 1); - CheckAckTimeout(ack_time); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1)); - CheckAckTimeout(clock_.ApproximateNow()); - - // Process another packet immediately after sending the ack and expect the - // ack timeout to be set delayed ack time in the future. - ack_time = clock_.ApproximateNow() + kDelayedAckTime; - RecordPacketReceipt(2, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 2); - CheckAckTimeout(ack_time); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(kDelayedAckTime); - CheckAckTimeout(clock_.ApproximateNow()); - - // Wait 1 second and enesure the ack timeout is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - RecordPacketReceipt(3, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 3); - CheckAckTimeout(ack_time); -} - TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimation) { EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION); // The ack time should be based on min_rtt * 1/4, since it's less than the // default delayed ack time. QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.25; @@ -474,7 +426,6 @@ return; } EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION); // Seed the min_rtt with a kAlarmGranularity signal. rtt_stats_.UpdateRtt(kAlarmGranularity, QuicTime::Delta::Zero(), clock_.ApproximateNow()); @@ -507,76 +458,6 @@ } TEST_P(QuicReceivedPacketManagerTest, - SendDelayedAckAckDecimationAfterQuiescence) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION); - QuicReceivedPacketManagerPeer::SetFastAckAfterQuiescence(&received_manager_, - true); - // The beginning of the connection counts as quiescence. - QuicTime ack_time = - clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - RecordPacketReceipt(1, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 1); - CheckAckTimeout(ack_time); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1)); - CheckAckTimeout(clock_.ApproximateNow()); - - // Process another packet immedately after sending the ack and expect the - // ack timeout to be set delayed ack time in the future. - ack_time = clock_.ApproximateNow() + kDelayedAckTime; - RecordPacketReceipt(2, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 2); - CheckAckTimeout(ack_time); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(kDelayedAckTime); - CheckAckTimeout(clock_.ApproximateNow()); - - // Wait 1 second and enesure the ack timeout is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - RecordPacketReceipt(3, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 3); - CheckAckTimeout(ack_time); - // Process enough packets to get into ack decimation behavior. - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - ack_time = clock_.ApproximateNow() + kMinRttMs * 0.25; - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 4; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - EXPECT_FALSE(HasPendingAck()); - RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket); - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (uint64_t i = 1; i < 10; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + i); - } - CheckAckTimeout(clock_.ApproximateNow()); - - // Wait 1 second and enesure the ack timeout is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - RecordPacketReceipt(kFirstDecimatedPacket + 10, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 10); - CheckAckTimeout(ack_time); -} - -TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationUnlimitedAggregation) { EXPECT_FALSE(HasPendingAck()); QuicConfig config; @@ -619,7 +500,6 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationEighthRtt) { EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION); QuicReceivedPacketManagerPeer::SetAckDecimationDelay(&received_manager_, 0.125); @@ -652,192 +532,6 @@ CheckAckTimeout(clock_.ApproximateNow()); } -TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithReordering) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, - ACK_DECIMATION_WITH_REORDERING); - - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.25; - // Process all the packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - - // Receive one packet out of order and then the rest in order. - // The loop leaves a one packet gap between acks sent to simulate some loss. - for (int j = 0; j < 3; ++j) { - // Process packet 10 first and ensure the timeout is one eighth min_rtt. - RecordPacketReceipt(kFirstDecimatedPacket + 9 + (j * 11), - clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 9 + (j * 11)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(5); - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (int i = 0; i < 9; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i + (j * 11), - clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, - kFirstDecimatedPacket + i + (j * 11)); - } - CheckAckTimeout(clock_.ApproximateNow()); - } -} - -TEST_P(QuicReceivedPacketManagerTest, - SendDelayedAckDecimationWithLargeReordering) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, - ACK_DECIMATION_WITH_REORDERING); - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.25; - - // Process all the packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - - RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket); - CheckAckTimeout(ack_time); - - RecordPacketReceipt(kFirstDecimatedPacket + 19, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 19); - ack_time = clock_.ApproximateNow() + kMinRttMs * 0.125; - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (int i = 1; i < 9; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + i); - } - CheckAckTimeout(clock_.ApproximateNow()); - - // The next packet received in order will cause an immediate ack, because it - // fills a hole. - RecordPacketReceipt(kFirstDecimatedPacket + 10, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 10); - CheckAckTimeout(clock_.ApproximateNow()); -} - -TEST_P(QuicReceivedPacketManagerTest, - SendDelayedAckDecimationWithReorderingEighthRtt) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, - ACK_DECIMATION_WITH_REORDERING); - QuicReceivedPacketManagerPeer::SetAckDecimationDelay(&received_manager_, - 0.125); - // The ack time should be based on min_rtt/8, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.125; - - // Process all the packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - - RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket); - CheckAckTimeout(ack_time); - - // Process packet 10 first and ensure the timeout is one eighth min_rtt. - RecordPacketReceipt(kFirstDecimatedPacket + 9, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 9); - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (int i = 1; i < 9; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck + i, kFirstDecimatedPacket); - } - CheckAckTimeout(clock_.ApproximateNow()); -} - -TEST_P(QuicReceivedPacketManagerTest, - SendDelayedAckDecimationWithLargeReorderingEighthRtt) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, - ACK_DECIMATION_WITH_REORDERING); - QuicReceivedPacketManagerPeer::SetAckDecimationDelay(&received_manager_, - 0.125); - - // The ack time should be based on min_rtt/8, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.125; - // Process all the packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - - RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket); - CheckAckTimeout(ack_time); - - RecordPacketReceipt(kFirstDecimatedPacket + 19, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 19); - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (int i = 1; i < 9; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + i); - } - CheckAckTimeout(clock_.ApproximateNow()); - - // The next packet received in order will cause an immediate ack, because it - // fills a hole. - RecordPacketReceipt(kFirstDecimatedPacket + 10, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 10); - CheckAckTimeout(clock_.ApproximateNow()); -} - } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h index f39ebdf..13307bb 100644 --- a/quic/core/quic_types.h +++ b/quic/core/quic_types.h
@@ -687,8 +687,6 @@ QUIC_EXPORT_PRIVATE std::string PacketNumberSpaceToString( PacketNumberSpace packet_number_space); -enum AckMode { TCP_ACKING, ACK_DECIMATION, ACK_DECIMATION_WITH_REORDERING }; - // Used to return the result of processing a received ACK frame. enum AckResult { PACKETS_NEWLY_ACKED,
diff --git a/quic/core/uber_received_packet_manager.cc b/quic/core/uber_received_packet_manager.cc index b017a84..d672fb8 100644 --- a/quic/core/uber_received_packet_manager.cc +++ b/quic/core/uber_received_packet_manager.cc
@@ -76,20 +76,18 @@ bool should_last_packet_instigate_acks, EncryptionLevel decrypted_packet_level, QuicPacketNumber last_received_packet_number, - QuicTime time_of_last_received_packet, QuicTime now, const RttStats* rtt_stats) { if (!supports_multiple_packet_number_spaces_) { received_packet_managers_[0].MaybeUpdateAckTimeout( - should_last_packet_instigate_acks, last_received_packet_number, - time_of_last_received_packet, now, rtt_stats); + should_last_packet_instigate_acks, last_received_packet_number, now, + rtt_stats); return; } received_packet_managers_[QuicUtils::GetPacketNumberSpace( decrypted_packet_level)] .MaybeUpdateAckTimeout(should_last_packet_instigate_acks, - last_received_packet_number, - time_of_last_received_packet, now, rtt_stats); + last_received_packet_number, now, rtt_stats); } void UberReceivedPacketManager::ResetAckStates(
diff --git a/quic/core/uber_received_packet_manager.h b/quic/core/uber_received_packet_manager.h index 9cfa247..a3c0877 100644 --- a/quic/core/uber_received_packet_manager.h +++ b/quic/core/uber_received_packet_manager.h
@@ -46,7 +46,6 @@ void MaybeUpdateAckTimeout(bool should_last_packet_instigate_acks, EncryptionLevel decrypted_packet_level, QuicPacketNumber last_received_packet_number, - QuicTime time_of_last_received_packet, QuicTime now, const RttStats* rtt_stats);
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc index 1b5a196..d4815a2 100644 --- a/quic/core/uber_received_packet_manager_test.cc +++ b/quic/core/uber_received_packet_manager_test.cc
@@ -18,20 +18,6 @@ class UberReceivedPacketManagerPeer { public: - static void SetAckMode(UberReceivedPacketManager* manager, AckMode ack_mode) { - for (auto& received_packet_manager : manager->received_packet_managers_) { - received_packet_manager.ack_mode_ = ack_mode; - } - } - - static void SetFastAckAfterQuiescence(UberReceivedPacketManager* manager, - bool fast_ack_after_quiescence) { - for (auto& received_packet_manager : manager->received_packet_managers_) { - received_packet_manager.fast_ack_after_quiescence_ = - fast_ack_after_quiescence; - } - } - static void SetAckDecimationDelay(UberReceivedPacketManager* manager, float ack_decimation_delay) { for (auto& received_packet_manager : manager->received_packet_managers_) { @@ -99,7 +85,7 @@ manager_->MaybeUpdateAckTimeout( should_last_packet_instigate_acks, decrypted_packet_level, QuicPacketNumber(last_received_packet_number), clock_.ApproximateNow(), - clock_.ApproximateNow(), &rtt_stats_); + &rtt_stats_); } void CheckAckTimeout(QuicTime time) { @@ -315,8 +301,6 @@ TEST_F(UberReceivedPacketManagerTest, AckDecimationReducesAcks) { EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), - ACK_DECIMATION_WITH_REORDERING); // Start ack decimation from 10th packet. manager_->set_min_received_before_ack_decimation(10); @@ -348,42 +332,6 @@ CheckAckTimeout(clock_.ApproximateNow()); } -TEST_F(UberReceivedPacketManagerTest, SendDelayedAfterQuiescence) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetFastAckAfterQuiescence(manager_.get(), - true); - // The beginning of the connection counts as quiescence. - QuicTime ack_time = - clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - - RecordPacketReceipt(1, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 1); - CheckAckTimeout(ack_time); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1)); - CheckAckTimeout(clock_.ApproximateNow()); - - // Process another packet immediately after sending the ack and expect the - // ack timeout to be set delayed ack time in the future. - ack_time = clock_.ApproximateNow() + kDelayedAckTime; - RecordPacketReceipt(2, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 2); - CheckAckTimeout(ack_time); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(kDelayedAckTime); - CheckAckTimeout(clock_.ApproximateNow()); - - // Wait 1 second and enesure the ack timeout is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - RecordPacketReceipt(3, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 3); - CheckAckTimeout(ack_time); -} - TEST_F(UberReceivedPacketManagerTest, SendDelayedMaxAckDelay) { EXPECT_FALSE(HasPendingAck()); QuicTime::Delta max_ack_delay = QuicTime::Delta::FromMilliseconds(100); @@ -400,7 +348,6 @@ TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimation) { EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION); // The ack time should be based on min_rtt * 1/4, since it's less than the // default delayed ack time. QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.25; @@ -431,76 +378,6 @@ } TEST_F(UberReceivedPacketManagerTest, - SendDelayedAckAckDecimationAfterQuiescence) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION); - UberReceivedPacketManagerPeer::SetFastAckAfterQuiescence(manager_.get(), - true); - // The beginning of the connection counts as quiescence. - QuicTime ack_time = - clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - RecordPacketReceipt(1, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 1); - CheckAckTimeout(ack_time); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1)); - CheckAckTimeout(clock_.ApproximateNow()); - - // Process another packet immedately after sending the ack and expect the - // ack timeout to be set delayed ack time in the future. - ack_time = clock_.ApproximateNow() + kDelayedAckTime; - RecordPacketReceipt(2, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 2); - CheckAckTimeout(ack_time); - // Simulate delayed ack alarm firing. - clock_.AdvanceTime(kDelayedAckTime); - CheckAckTimeout(clock_.ApproximateNow()); - - // Wait 1 second and enesure the ack timeout is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - RecordPacketReceipt(3, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, 3); - CheckAckTimeout(ack_time); - // Process enough packets to get into ack decimation behavior. - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - ack_time = clock_.ApproximateNow() + kMinRttMs * 0.25; - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 4; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - EXPECT_FALSE(HasPendingAck()); - RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket); - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (uint64_t i = 1; i < 10; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + i); - } - CheckAckTimeout(clock_.ApproximateNow()); - - // Wait 1 second and enesure the ack timeout is set to 1ms in the future. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(1); - RecordPacketReceipt(kFirstDecimatedPacket + 10, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 10); - CheckAckTimeout(ack_time); -} - -TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimationUnlimitedAggregation) { EXPECT_FALSE(HasPendingAck()); QuicConfig config; @@ -543,7 +420,6 @@ TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimationEighthRtt) { EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION); UberReceivedPacketManagerPeer::SetAckDecimationDelay(manager_.get(), 0.125); // The ack time should be based on min_rtt/8, since it's less than the @@ -575,190 +451,6 @@ CheckAckTimeout(clock_.ApproximateNow()); } -TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimationWithReordering) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), - ACK_DECIMATION_WITH_REORDERING); - - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.25; - // Process all the packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - - // Receive one packet out of order and then the rest in order. - // The loop leaves a one packet gap between acks sent to simulate some loss. - for (int j = 0; j < 3; ++j) { - // Process packet 10 first and ensure the timeout is one eighth min_rtt. - RecordPacketReceipt(kFirstDecimatedPacket + 9 + (j * 11), - clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 9 + (j * 11)); - ack_time = clock_.ApproximateNow() + QuicTime::Delta::FromMilliseconds(5); - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (int i = 0; i < 9; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i + (j * 11), - clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, - kFirstDecimatedPacket + i + (j * 11)); - } - CheckAckTimeout(clock_.ApproximateNow()); - } -} - -TEST_F(UberReceivedPacketManagerTest, - SendDelayedAckDecimationWithLargeReordering) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), - ACK_DECIMATION_WITH_REORDERING); - // The ack time should be based on min_rtt/4, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.25; - - // Process all the packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - - RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket); - CheckAckTimeout(ack_time); - - RecordPacketReceipt(kFirstDecimatedPacket + 19, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 19); - ack_time = clock_.ApproximateNow() + kMinRttMs * 0.125; - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (int i = 1; i < 9; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + i); - } - CheckAckTimeout(clock_.ApproximateNow()); - - // The next packet received in order will cause an immediate ack, because it - // fills a hole. - RecordPacketReceipt(kFirstDecimatedPacket + 10, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 10); - CheckAckTimeout(clock_.ApproximateNow()); -} - -TEST_F(UberReceivedPacketManagerTest, - SendDelayedAckDecimationWithReorderingEighthRtt) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), - ACK_DECIMATION_WITH_REORDERING); - UberReceivedPacketManagerPeer::SetAckDecimationDelay(manager_.get(), 0.125); - // The ack time should be based on min_rtt/8, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.125; - - // Process all the packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - - RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket); - CheckAckTimeout(ack_time); - - // Process packet 10 first and ensure the timeout is one eighth min_rtt. - RecordPacketReceipt(kFirstDecimatedPacket + 9, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 9); - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (int i = 1; i < 9; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck + i, kFirstDecimatedPacket); - } - CheckAckTimeout(clock_.ApproximateNow()); -} - -TEST_F(UberReceivedPacketManagerTest, - SendDelayedAckDecimationWithLargeReorderingEighthRtt) { - if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { - return; - } - EXPECT_FALSE(HasPendingAck()); - UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), - ACK_DECIMATION_WITH_REORDERING); - UberReceivedPacketManagerPeer::SetAckDecimationDelay(manager_.get(), 0.125); - - // The ack time should be based on min_rtt/8, since it's less than the - // default delayed ack time. - QuicTime ack_time = clock_.ApproximateNow() + kMinRttMs * 0.125; - // Process all the packets in order so there aren't missing packets. - uint64_t kFirstDecimatedPacket = 101; - for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) { - RecordPacketReceipt(i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, i); - if (i % 2 == 0) { - // Ack every 2 packets by default. - CheckAckTimeout(clock_.ApproximateNow()); - } else { - CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); - } - } - - RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket); - CheckAckTimeout(ack_time); - - RecordPacketReceipt(kFirstDecimatedPacket + 19, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 19); - CheckAckTimeout(ack_time); - - // The 10th received packet causes an ack to be sent. - for (int i = 1; i < 9; ++i) { - RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + i); - } - CheckAckTimeout(clock_.ApproximateNow()); - - // The next packet received in order will cause an immediate ack, because it - // fills a hole. - RecordPacketReceipt(kFirstDecimatedPacket + 10, clock_.ApproximateNow()); - MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + 10); - CheckAckTimeout(clock_.ApproximateNow()); -} - TEST_F(UberReceivedPacketManagerTest, DontWaitForPacketsBeforeMultiplePacketNumberSpaces) { manager_->EnableMultiplePacketNumberSpacesSupport();
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc index 2e8ca59..1949df1 100644 --- a/quic/test_tools/quic_connection_peer.cc +++ b/quic/test_tools/quic_connection_peer.cc
@@ -210,26 +210,6 @@ } // static -void QuicConnectionPeer::SetAckMode(QuicConnection* connection, - AckMode ack_mode) { - for (auto& received_packet_manager : - connection->uber_received_packet_manager_.received_packet_managers_) { - received_packet_manager.ack_mode_ = ack_mode; - } -} - -// static -void QuicConnectionPeer::SetFastAckAfterQuiescence( - QuicConnection* connection, - bool fast_ack_after_quiescence) { - for (auto& received_packet_manager : - connection->uber_received_packet_manager_.received_packet_managers_) { - received_packet_manager.fast_ack_after_quiescence_ = - fast_ack_after_quiescence; - } -} - -// static void QuicConnectionPeer::SetAckDecimationDelay(QuicConnection* connection, float ack_decimation_delay) { for (auto& received_packet_manager :
diff --git a/quic/test_tools/quic_connection_peer.h b/quic/test_tools/quic_connection_peer.h index 8a285e9..b470653 100644 --- a/quic/test_tools/quic_connection_peer.h +++ b/quic/test_tools/quic_connection_peer.h
@@ -102,9 +102,6 @@ QuicConnection* connection, QuicPacketCount packets_between_probes_base, QuicPacketNumber next_probe_at); - static void SetAckMode(QuicConnection* connection, AckMode ack_mode); - static void SetFastAckAfterQuiescence(QuicConnection* connection, - bool fast_ack_after_quiescence); static void SetAckDecimationDelay(QuicConnection* connection, float ack_decimation_delay); static bool HasRetransmittableFrames(QuicConnection* connection,