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
