In quic, bundle server initial crypto data with initial ack. protected by gfe2_reloadable_flag_quic_bundle_crypto_data_with_initial_ack. This is used to speed up handshake since INITIAL ACK will be padded to full anyway. PiperOrigin-RevId: 317176505 Change-Id: I5444372466c64cdc662792333916d74b08e8b594
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 03f6922..b69edfe 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -4237,13 +4237,39 @@ return ENCRYPTION_INITIAL; } +void QuicConnection::MaybeBundleCryptoDataWithInitialAck() { + DCHECK(SupportsMultiplePacketNumberSpaces()); + const QuicTime initial_ack_timeout = + uber_received_packet_manager_.GetAckTimeout(INITIAL_DATA); + if (!initial_ack_timeout.IsInitialized() || + (initial_ack_timeout > clock_->ApproximateNow() && + initial_ack_timeout > + uber_received_packet_manager_.GetEarliestAckTimeout())) { + // Not going to send initial ACK. + return; + } + // Initial ACK will be padded to full anyway, so try to bundle INITIAL crypto + // data. + sent_packet_manager_.RetransmitInitialDataIfAny(); +} + void QuicConnection::SendAllPendingAcks() { DCHECK(SupportsMultiplePacketNumberSpaces()); QUIC_DVLOG(1) << ENDPOINT << "Trying to send all pending ACKs"; ack_alarm_->Cancel(); - const QuicTime earliest_ack_timeout = + QuicTime earliest_ack_timeout = uber_received_packet_manager_.GetEarliestAckTimeout(); QUIC_BUG_IF(!earliest_ack_timeout.IsInitialized()); + if (GetQuicReloadableFlag(quic_bundle_crypto_data_with_initial_ack) && + perspective() == Perspective::IS_SERVER) { + MaybeBundleCryptoDataWithInitialAck(); + earliest_ack_timeout = + uber_received_packet_manager_.GetEarliestAckTimeout(); + if (!earliest_ack_timeout.IsInitialized()) { + QUIC_RELOADABLE_FLAG_COUNT(quic_bundle_crypto_data_with_initial_ack); + return; + } + } // Latches current encryption level. const EncryptionLevel current_encryption_level = encryption_level_; for (int8_t i = INITIAL_DATA; i <= APPLICATION_DATA; ++i) { @@ -4342,13 +4368,15 @@ if (coalesced_packet_.length() == 0) { return true; } - QUIC_DVLOG(1) << ENDPOINT << "Sending coalesced packet"; + char buffer[kMaxOutgoingPacketSize]; const size_t length = packet_creator_.SerializeCoalescedPacket( coalesced_packet_, buffer, coalesced_packet_.max_packet_length()); if (length == 0) { return false; } + QUIC_DVLOG(1) << ENDPOINT << "Sending coalesced packet " + << coalesced_packet_.ToString(length); if (!buffered_packets_.empty() || HandleWriteBlocked()) { QUIC_DVLOG(1) << ENDPOINT
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 48d16a2..6cf01f0 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -1318,6 +1318,9 @@ bool ValidateConfigConnectionIds(const QuicConfig& config); bool ValidateConfigConnectionIdsOld(const QuicConfig& config); + // Called when ACK alarm goes off. Try to bundle INITIAL data with the ACK. + void MaybeBundleCryptoDataWithInitialAck(); + // Returns true if an undecryptable packet of |decryption_level| should be // buffered (such that connection can try to decrypt it later). bool ShouldEnqueueUnDecryptablePacket(EncryptionLevel decryption_level,
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index d273bbe..41e1974 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -11259,6 +11259,57 @@ EXPECT_EQ(0u, QuicConnectionPeer::NumUndecryptablePackets(&connection_)); } +TEST_P(QuicConnectionTest, BundleInitialDataWithInitialAck) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + set_perspective(Perspective::IS_SERVER); + 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_.HasPendingAcks()); + + connection_.SetEncrypter(ENCRYPTION_INITIAL, + std::make_unique<TaggingEncrypter>(0x01)); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_INITIAL); + connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_INITIAL); + QuicTime expected_pto_time = + connection_.sent_packet_manager().GetRetransmissionTime(); + + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5)); + connection_.SetEncrypter(ENCRYPTION_HANDSHAKE, + std::make_unique<TaggingEncrypter>(0x02)); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE); + EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); + connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE); + // Verify PTO time does not change. + EXPECT_EQ(expected_pto_time, + connection_.sent_packet_manager().GetRetransmissionTime()); + + // Receives packet 1001 in initial data. + ProcessCryptoPacketAtLevel(1001, ENCRYPTION_INITIAL); + EXPECT_TRUE(connection_.HasPendingAcks()); + // Receives packet 1002 in initial data. + ProcessCryptoPacketAtLevel(1002, ENCRYPTION_INITIAL); + EXPECT_FALSE(writer_->ack_frames().empty()); + if (GetQuicReloadableFlag(quic_bundle_crypto_data_with_initial_ack)) { + // Verify CRYPTO frame is bundled with INITIAL ACK. + EXPECT_FALSE(writer_->crypto_frames().empty()); + // Verify PTO time changes. + EXPECT_NE(expected_pto_time, + connection_.sent_packet_manager().GetRetransmissionTime()); + } else { + EXPECT_TRUE(writer_->crypto_frames().empty()); + // Verify PTO time does not change. + EXPECT_EQ(expected_pto_time, + connection_.sent_packet_manager().GetRetransmissionTime()); + } +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index 31ac454..d74a487 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -912,6 +912,27 @@ pto_exponential_backoff_start_point_ = exponential_backoff_start_point; } +void QuicSentPacketManager::RetransmitInitialDataIfAny() { + DCHECK(supports_multiple_packet_number_spaces()); + if (!unacked_packets_.GetLastInFlightPacketSentTime(INITIAL_DATA) + .IsInitialized()) { + // No in flight initial data. + return; + } + QuicPacketNumber packet_number = unacked_packets_.GetLeastUnacked(); + for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); + it != unacked_packets_.end(); ++it, ++packet_number) { + if (it->state == OUTSTANDING && + unacked_packets_.HasRetransmittableFrames(*it) && + unacked_packets_.GetPacketNumberSpace(it->encryption_level) == + INITIAL_DATA) { + DCHECK(it->in_flight); + MarkForRetransmission(packet_number, PTO_RETRANSMISSION); + return; + } + } +} + QuicSentPacketManager::RetransmissionTimeoutMode QuicSentPacketManager::GetRetransmissionMode() const { DCHECK(unacked_packets_.HasInFlightPackets() ||
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index b0a8352..af32eba 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -394,6 +394,9 @@ void StartExponentialBackoffAfterNthPto( size_t exponential_backoff_start_point); + // Called to retransmit in flight INITIAL packet if any. + void RetransmitInitialDataIfAny(); + bool supports_multiple_packet_number_spaces() const { return unacked_packets_.supports_multiple_packet_number_spaces(); }
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index cdec2d0..90c7e15 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -4044,6 +4044,58 @@ "retransmissions"); } +TEST_F(QuicSentPacketManagerTest, MaybeRetransmitInitialData) { + manager_.EnableMultiplePacketNumberSpacesSupport(); + EXPECT_CALL(*send_algorithm_, PacingRate(_)) + .WillRepeatedly(Return(QuicBandwidth::Zero())); + EXPECT_CALL(*send_algorithm_, GetCongestionWindow()) + .WillRepeatedly(Return(10 * kDefaultTCPMSS)); + RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); + rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(100), + QuicTime::Delta::Zero(), QuicTime::Zero()); + QuicTime::Delta srtt = rtt_stats->smoothed_rtt(); + + // Send packet 1. + SendDataPacket(1, ENCRYPTION_INITIAL); + QuicTime packet1_sent_time = clock_.Now(); + + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + // Send packets 2 and 3. + SendDataPacket(2, ENCRYPTION_HANDSHAKE); + QuicTime packet2_sent_time = clock_.Now(); + SendDataPacket(3, ENCRYPTION_HANDSHAKE); + // Verify PTO is correctly set based on packet 1. + int pto_rttvar_multiplier = + GetQuicReloadableFlag(quic_default_on_pto) ? 2 : 4; + QuicTime::Delta expected_pto_delay = + srtt + pto_rttvar_multiplier * rtt_stats->mean_deviation() + + QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs); + EXPECT_EQ(packet1_sent_time + expected_pto_delay, + manager_.GetRetransmissionTime()); + + // Assume connection is going to send INITIAL ACK. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { + RetransmitDataPacket(4, type, ENCRYPTION_INITIAL); + }))); + manager_.RetransmitInitialDataIfAny(); + // Verify PTO is re-armed based on packet 2. + EXPECT_EQ(packet2_sent_time + expected_pto_delay, + manager_.GetRetransmissionTime()); + + // Connection is going to send another INITIAL ACK. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { + RetransmitDataPacket(5, type, ENCRYPTION_INITIAL); + }))); + manager_.RetransmitInitialDataIfAny(); + // Verify PTO does not change. + EXPECT_EQ(packet2_sent_time + expected_pto_delay, + manager_.GetRetransmissionTime()); +} + } // namespace } // namespace test } // namespace quic