Always send the earliest ack in quicconnection::sendallpendingacks. protected by gfe2_reloadable_flag_quic_always_send_earliest_ack.
PiperOrigin-RevId: 313445668
Change-Id: Ibb17ff261f40d9da52f5e5e043e496daac6829ea
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index d39aa1e..36dea57 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -4001,21 +4001,27 @@
DCHECK(SupportsMultiplePacketNumberSpaces());
QUIC_DVLOG(1) << ENDPOINT << "Trying to send all pending ACKs";
ack_alarm_->Cancel();
+ const QuicTime earliest_ack_timeout =
+ uber_received_packet_manager_.GetEarliestAckTimeout();
+ QUIC_BUG_IF(!earliest_ack_timeout.IsInitialized());
// Latches current encryption level.
const EncryptionLevel current_encryption_level = encryption_level_;
for (int8_t i = INITIAL_DATA; i <= APPLICATION_DATA; ++i) {
const QuicTime ack_timeout = uber_received_packet_manager_.GetAckTimeout(
static_cast<PacketNumberSpace>(i));
- if (!ack_timeout.IsInitialized() ||
- ack_timeout > clock_->ApproximateNow()) {
+ if (!ack_timeout.IsInitialized()) {
continue;
}
- if (!framer_.HasEncrypterOfEncryptionLevel(
- QuicUtils::GetEncryptionLevel(static_cast<PacketNumberSpace>(i)))) {
- QUIC_BUG << ENDPOINT << "Cannot send ACKs for packet number space "
- << PacketNumberSpaceToString(static_cast<PacketNumberSpace>(i))
- << " without corresponding encrypter";
- continue;
+ if (ack_timeout > clock_->ApproximateNow()) {
+ if (!GetQuicReloadableFlag(quic_always_send_earliest_ack)) {
+ continue;
+ }
+ QUIC_RELOADABLE_FLAG_COUNT(quic_always_send_earliest_ack);
+ if (ack_timeout > earliest_ack_timeout) {
+ // Always send the earliest ACK to make forward progress in case alarm
+ // fires early.
+ continue;
+ }
}
QUIC_DVLOG(1) << ENDPOINT << "Sending ACK of packet number space "
<< PacketNumberSpaceToString(
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 714db95..a9e25f0 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -10715,6 +10715,52 @@
}
}
+TEST_P(QuicConnectionTest, AckAlarmFiresEarly) {
+ if (!connection_.SupportsMultiplePacketNumberSpaces()) {
+ return;
+ }
+ EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+ if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+ EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
+ }
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
+ use_tagging_decrypter();
+ // Receives packet 1000 in initial data.
+ ProcessCryptoPacketAtLevel(1000, ENCRYPTION_INITIAL);
+ EXPECT_TRUE(connection_.GetAckAlarm()->IsSet());
+
+ peer_framer_.SetEncrypter(ENCRYPTION_ZERO_RTT,
+ std::make_unique<TaggingEncrypter>(0x02));
+ SetDecrypter(ENCRYPTION_ZERO_RTT,
+ std::make_unique<StrictTaggingDecrypter>(0x02));
+ connection_.SetEncrypter(ENCRYPTION_INITIAL,
+ std::make_unique<TaggingEncrypter>(0x02));
+ // Receives packet 1000 in application data.
+ ProcessDataPacketAtLevel(1000, false, ENCRYPTION_ZERO_RTT);
+ EXPECT_TRUE(connection_.GetAckAlarm()->IsSet());
+ // Verify ACK deadline does not change.
+ EXPECT_EQ(clock_.ApproximateNow() + kAlarmGranularity,
+ connection_.GetAckAlarm()->deadline());
+
+ // Ack alarm fires early.
+ if (GetQuicReloadableFlag(quic_always_send_earliest_ack)) {
+ // Verify the earliest ACK is flushed.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ } else {
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
+ }
+ connection_.GetAckAlarm()->Fire();
+ EXPECT_TRUE(connection_.GetAckAlarm()->IsSet());
+ if (GetQuicReloadableFlag(quic_always_send_earliest_ack)) {
+ EXPECT_EQ(clock_.ApproximateNow() + DefaultDelayedAckTime(),
+ connection_.GetAckAlarm()->deadline());
+ } else {
+ // No forward progress has been made.
+ EXPECT_EQ(clock_.ApproximateNow() + kAlarmGranularity,
+ connection_.GetAckAlarm()->deadline());
+ }
+}
+
} // namespace
} // namespace test
} // namespace quic
