IMMEDIATE_ACK frame processing. Protected by FLAGS_quic_reloadable_flag_quic_receive_ack_frequency. PiperOrigin-RevId: 718051494
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 6a12e48..11e859c 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -2121,7 +2121,16 @@ QUIC_LOG_EVERY_N_SEC(ERROR, 120) << "Got unexpected ImmediateAck Frame."; return false; } - QUIC_RELOADABLE_FLAG_COUNT_N(quic_receive_ack_frequency, 1, 1); + QUIC_RELOADABLE_FLAG_COUNT_N(quic_receive_ack_frequency, 1, 2); + if (last_received_packet_info_.decrypted_level == ENCRYPTION_FORWARD_SECURE) { + uber_received_packet_manager_.OnImmediateAckFrame(); + } else { + QUIC_LOG_EVERY_N_SEC(ERROR, 120) + << "Got ImmediateAckFrame at EncryptionLevel " + << EncryptionLevelToString(last_received_packet_info_.decrypted_level); + return false; + } + should_last_packet_instigate_acks_ = false; MaybeUpdateAckTimeout(); return true; }
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 9fa3a16..e2d4fcb 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -15924,16 +15924,37 @@ ProcessFramesPacketAtLevel(packet_number++, frames, ENCRYPTION_FORWARD_SECURE); if (QuicUtils::IsAckElicitingFrame(frame_type)) { - ASSERT_TRUE(connection_.HasPendingAcks()) << frame; - // Flush ACK. - clock_.AdvanceTime(DefaultDelayedAckTime()); - connection_.GetAckAlarm()->Fire(); + if (frame_type != IMMEDIATE_ACK_FRAME) { + ASSERT_TRUE(connection_.HasPendingAcks()) << frame; + // Flush ACK. + clock_.AdvanceTime(DefaultDelayedAckTime()); + connection_.GetAckAlarm()->Fire(); + } + EXPECT_FALSE(writer_->ack_frames().empty()); + writer_->framer()->Reset(); // Clear the visitor. } EXPECT_FALSE(connection_.HasPendingAcks()); ASSERT_TRUE(connection_.connected()); } } +TEST_P(QuicConnectionTest, ImmediateAckOverridesOtherFrame) { + if (!version().HasIetfQuicFrames()) { + return; + } + QuicFrames frames; + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.set_can_receive_ack_frequency_immediate_ack(true); + // A PING frame will set the ack timer. IMMEDIATE_ACK should override it to + // send an ACK immediately. + frames.push_back(QuicFrame(QuicPingFrame())); + frames.push_back(QuicFrame(QuicImmediateAckFrame())); + writer_->framer()->Reset(); // Clear the visitor. + EXPECT_TRUE(writer_->ack_frames().empty()); + ProcessFramesPacketAtLevel(1, frames, ENCRYPTION_FORWARD_SECURE); + EXPECT_FALSE(writer_->ack_frames().empty()); +} + TEST_P(QuicConnectionTest, ReceivedChloAndAck) { if (!version().HasIetfQuicFrames()) { return;
diff --git a/quiche/quic/core/quic_received_packet_manager.cc b/quiche/quic/core/quic_received_packet_manager.cc index fa0f64b..56c97cb 100644 --- a/quiche/quic/core/quic_received_packet_manager.cc +++ b/quiche/quic/core/quic_received_packet_manager.cc
@@ -83,6 +83,7 @@ ack_frame_.received_packet_times.clear(); } ack_frame_updated_ = true; + ack_now_ = false; // Whether |packet_number| is received out of order. bool packet_reordered = false; @@ -289,6 +290,13 @@ return; } + if (ack_now_) { + // An IMMEDIATE_ACK frame arrived. Send an ack immediately. + QUIC_RELOADABLE_FLAG_COUNT_N(quic_receive_ack_frequency, 2, 2); + ack_timeout_ = now; + return; + } + if (!ignore_order_ && was_last_packet_missing_ && last_sent_largest_acked_.IsInitialized() && last_received_packet_number < last_sent_largest_acked_) {
diff --git a/quiche/quic/core/quic_received_packet_manager.h b/quiche/quic/core/quic_received_packet_manager.h index c08bb70..d1feccc 100644 --- a/quiche/quic/core/quic_received_packet_manager.h +++ b/quiche/quic/core/quic_received_packet_manager.h
@@ -61,6 +61,10 @@ // received after this call. void DontWaitForPacketsBefore(QuicPacketNumber least_unacked); + // An IMMEDIATE_ACK frame arrived, so update the ack_timeout_ to now the next + // time it's set. + void OnImmediateAckFrame() { ack_now_ = true; } + // Called to update ack_timeout_ to the time when an ACK needs to be sent. A // caller can decide whether and when to send an ACK by retrieving // ack_timeout_. If ack_timeout_ is not initialized, no ACK needs to be sent. @@ -211,6 +215,9 @@ // The current packet is CE-marked, and the previous packet was not. This // condition should trigger an immediate ACK. bool changed_to_ce_marked_ = false; + // Because of an IMMEDIATE_ACK frame, the next call to MaybeUpdateAckTimeout + // should set the ack timeout to now. + bool ack_now_ = false; // Last sent largest acked, which gets updated when ACK was successfully sent. QuicPacketNumber last_sent_largest_acked_;
diff --git a/quiche/quic/core/uber_received_packet_manager.cc b/quiche/quic/core/uber_received_packet_manager.cc index b82ab9c..2f64754 100644 --- a/quiche/quic/core/uber_received_packet_manager.cc +++ b/quiche/quic/core/uber_received_packet_manager.cc
@@ -72,6 +72,16 @@ .DontWaitForPacketsBefore(least_unacked); } +void UberReceivedPacketManager::OnImmediateAckFrame() { + if (!supports_multiple_packet_number_spaces_) { + QUIC_BUG(quic_bug_10495_4) + << "Received ImmediateAckFrame when multiple packet number spaces " + "is not supported"; + return; + } + received_packet_managers_[APPLICATION_DATA].OnImmediateAckFrame(); +} + void UberReceivedPacketManager::MaybeUpdateAckTimeout( bool should_last_packet_instigate_acks, EncryptionLevel decrypted_packet_level,
diff --git a/quiche/quic/core/uber_received_packet_manager.h b/quiche/quic/core/uber_received_packet_manager.h index 7f4c7cc..4d60ce0 100644 --- a/quiche/quic/core/uber_received_packet_manager.h +++ b/quiche/quic/core/uber_received_packet_manager.h
@@ -43,6 +43,9 @@ void DontWaitForPacketsBefore(EncryptionLevel decrypted_packet_level, QuicPacketNumber least_unacked); + // Trigger an immediate ACK. + void OnImmediateAckFrame(); + // Called after header of last received packet has been successfully processed // to update ACK timeout. void MaybeUpdateAckTimeout(bool should_last_packet_instigate_acks,
diff --git a/quiche/quic/core/uber_received_packet_manager_test.cc b/quiche/quic/core/uber_received_packet_manager_test.cc index 083d312..4cab508 100644 --- a/quiche/quic/core/uber_received_packet_manager_test.cc +++ b/quiche/quic/core/uber_received_packet_manager_test.cc
@@ -6,12 +6,12 @@ #include <algorithm> #include <memory> -#include <utility> #include "quiche/quic/core/congestion_control/rtt_stats.h" #include "quiche/quic/core/crypto/crypto_protocol.h" #include "quiche/quic/core/quic_connection_stats.h" -#include "quiche/quic/core/quic_utils.h" +#include "quiche/quic/core/quic_packet_number.h" +#include "quiche/quic/core/quic_types.h" #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/test_tools/mock_clock.h" @@ -565,6 +565,22 @@ QuicTime::Delta::FromMilliseconds(11) + kDelayedAckTime); } +TEST_F(UberReceivedPacketManagerTest, ImmediateAckFrameTriggersAck) { + manager_->EnableMultiplePacketNumberSpacesSupport(Perspective::IS_CLIENT); + EXPECT_FALSE(HasPendingAck()); + RecordPacketReceipt(1, clock_.ApproximateNow()); + manager_->OnImmediateAckFrame(); + manager_->MaybeUpdateAckTimeout(kInstigateAck, ENCRYPTION_FORWARD_SECURE, + QuicPacketNumber(1), clock_.ApproximateNow(), + clock_.ApproximateNow(), &rtt_stats_); + CheckAckTimeout(clock_.ApproximateNow()); + RecordPacketReceipt(2, clock_.ApproximateNow()); + manager_->MaybeUpdateAckTimeout(kInstigateAck, ENCRYPTION_FORWARD_SECURE, + QuicPacketNumber(2), clock_.ApproximateNow(), + clock_.ApproximateNow(), &rtt_stats_); + CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime); +} + } // namespace } // namespace test } // namespace quic