Delay the first Initial ACK sent by the server so it can be bundled with the server's first Initial packet. Protected by FLAGS_quic_reloadable_flag_quic_delay_initial_ack. PiperOrigin-RevId: 338518843 Change-Id: Ib8cb7375f212ac5f028bd2c7f6a191278ac01540
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 7f7089b..5b9ab32 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -5113,7 +5113,8 @@ << " supports multiple packet number spaces"; framer_.EnableMultiplePacketNumberSpacesSupport(); sent_packet_manager_.EnableMultiplePacketNumberSpacesSupport(); - uber_received_packet_manager_.EnableMultiplePacketNumberSpacesSupport(); + uber_received_packet_manager_.EnableMultiplePacketNumberSpacesSupport( + perspective_); } bool QuicConnection::SupportsMultiplePacketNumberSpaces() const {
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc index 0a91709..98f450c 100644 --- a/quic/core/quic_received_packet_manager.cc +++ b/quic/core/quic_received_packet_manager.cc
@@ -264,8 +264,11 @@ return; } - MaybeUpdateAckTimeoutTo( - now + GetMaxAckDelay(last_received_packet_number, *rtt_stats)); + QuicTime updated_ack_time = + now + GetMaxAckDelay(last_received_packet_number, *rtt_stats); + if (!ack_timeout_.IsInitialized() || ack_timeout_ > updated_ack_time) { + ack_timeout_ = updated_ack_time; + } } void QuicReceivedPacketManager::ResetAckStates() { @@ -275,12 +278,6 @@ last_sent_largest_acked_ = LargestAcked(ack_frame_); } -void QuicReceivedPacketManager::MaybeUpdateAckTimeoutTo(QuicTime time) { - if (!ack_timeout_.IsInitialized() || ack_timeout_ > time) { - ack_timeout_ = time; - } -} - bool QuicReceivedPacketManager::HasMissingPackets() const { if (ack_frame_.packets.Empty()) { return false;
diff --git a/quic/core/uber_received_packet_manager.cc b/quic/core/uber_received_packet_manager.cc index 7b2b32f..025da64 100644 --- a/quic/core/uber_received_packet_manager.cc +++ b/quic/core/uber_received_packet_manager.cc
@@ -98,9 +98,15 @@ } received_packet_managers_[QuicUtils::GetPacketNumberSpace(encryption_level)] .ResetAckStates(); + if (encryption_level == ENCRYPTION_INITIAL) { + // After one Initial ACK is sent, the others should be sent 'immediately'. + received_packet_managers_[INITIAL_DATA].set_local_max_ack_delay( + kAlarmGranularity); + } } -void UberReceivedPacketManager::EnableMultiplePacketNumberSpacesSupport() { +void UberReceivedPacketManager::EnableMultiplePacketNumberSpacesSupport( + Perspective perspective) { if (supports_multiple_packet_number_spaces_) { QUIC_BUG << "Multiple packet number spaces has already been enabled"; return; @@ -112,8 +118,15 @@ } // In IETF QUIC, the peer is expected to acknowledge packets in Initial and // Handshake packets with minimal delay. - received_packet_managers_[INITIAL_DATA].set_local_max_ack_delay( - kAlarmGranularity); + if (!GetQuicReloadableFlag(quic_delay_initial_ack) || + perspective == Perspective::IS_CLIENT) { + // Delay the first server ACK, because server ACKs are padded to + // full size and count towards the amplification limit. + received_packet_managers_[INITIAL_DATA].set_local_max_ack_delay( + kAlarmGranularity); + } else { + QUIC_RELOADABLE_FLAG_COUNT(quic_delay_initial_ack); + } received_packet_managers_[HANDSHAKE_DATA].set_local_max_ack_delay( kAlarmGranularity);
diff --git a/quic/core/uber_received_packet_manager.h b/quic/core/uber_received_packet_manager.h index 17b5e26..a9f1775 100644 --- a/quic/core/uber_received_packet_manager.h +++ b/quic/core/uber_received_packet_manager.h
@@ -54,7 +54,7 @@ void ResetAckStates(EncryptionLevel encryption_level); // Called to enable multiple packet number support. - void EnableMultiplePacketNumberSpacesSupport(); + void EnableMultiplePacketNumberSpacesSupport(Perspective perspective); // Returns true if ACK frame has been updated since GetUpdatedAckFrame was // last called.
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc index b7063d2..3a1531b 100644 --- a/quic/core/uber_received_packet_manager_test.cc +++ b/quic/core/uber_received_packet_manager_test.cc
@@ -439,7 +439,7 @@ TEST_F(UberReceivedPacketManagerTest, DontWaitForPacketsBeforeMultiplePacketNumberSpaces) { - manager_->EnableMultiplePacketNumberSpacesSupport(); + manager_->EnableMultiplePacketNumberSpacesSupport(Perspective::IS_CLIENT); EXPECT_FALSE( manager_->GetLargestObserved(ENCRYPTION_HANDSHAKE).IsInitialized()); EXPECT_FALSE( @@ -469,10 +469,42 @@ } TEST_F(UberReceivedPacketManagerTest, AckSendingDifferentPacketNumberSpaces) { - manager_->EnableMultiplePacketNumberSpacesSupport(); + manager_->EnableMultiplePacketNumberSpacesSupport(Perspective::IS_SERVER); EXPECT_FALSE(HasPendingAck()); EXPECT_FALSE(manager_->IsAckFrameUpdated()); + RecordPacketReceipt(ENCRYPTION_INITIAL, 3); + EXPECT_TRUE(manager_->IsAckFrameUpdated()); + MaybeUpdateAckTimeout(kInstigateAck, ENCRYPTION_INITIAL, 3); + EXPECT_TRUE(HasPendingAck()); + // Delayed ack is scheduled. + if (GetQuicReloadableFlag(quic_delay_initial_ack)) { + CheckAckTimeout(clock_.ApproximateNow() + + QuicTime::Delta::FromMilliseconds(25)); + // Send delayed handshake data ACK. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(25)); + } else { + CheckAckTimeout(clock_.ApproximateNow() + + QuicTime::Delta::FromMilliseconds(1)); + // Send delayed handshake data ACK. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1)); + } + CheckAckTimeout(clock_.ApproximateNow()); + EXPECT_FALSE(HasPendingAck()); + + // Second delayed ack should have a shorter delay. + RecordPacketReceipt(ENCRYPTION_INITIAL, 4); + EXPECT_TRUE(manager_->IsAckFrameUpdated()); + MaybeUpdateAckTimeout(kInstigateAck, ENCRYPTION_INITIAL, 4); + EXPECT_TRUE(HasPendingAck()); + // Delayed ack is scheduled. + CheckAckTimeout(clock_.ApproximateNow() + + QuicTime::Delta::FromMilliseconds(1)); + // Send delayed handshake data ACK. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1)); + CheckAckTimeout(clock_.ApproximateNow()); + EXPECT_FALSE(HasPendingAck()); + RecordPacketReceipt(ENCRYPTION_HANDSHAKE, 3); EXPECT_TRUE(manager_->IsAckFrameUpdated()); MaybeUpdateAckTimeout(kInstigateAck, ENCRYPTION_HANDSHAKE, 3);