gfe-relnote: In QUIC, make PTO per packet number space. This only affects QUIC versions with TLS handshake. Protected by disabled gfe2_reloadable_flag_quic_enable_version_t* flags. PiperOrigin-RevId: 291947359 Change-Id: I18bff16ecbacf2f12e1a1442ccf52a8ebdd292fe
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 035f457..7c20394 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -383,6 +383,8 @@ framer_.framer()->InstallDecrypter( ENCRYPTION_INITIAL, std::make_unique<TaggingDecrypter>()); framer_.framer()->InstallDecrypter( + ENCRYPTION_HANDSHAKE, std::make_unique<TaggingDecrypter>()); + framer_.framer()->InstallDecrypter( ENCRYPTION_ZERO_RTT, std::make_unique<TaggingDecrypter>()); framer_.framer()->InstallDecrypter( ENCRYPTION_FORWARD_SECURE, std::make_unique<TaggingDecrypter>()); @@ -763,18 +765,24 @@ QuicConsumedData SendCryptoDataWithString(quiche::QuicheStringPiece data, QuicStreamOffset offset) { + return SendCryptoDataWithString(data, offset, ENCRYPTION_INITIAL); + } + + QuicConsumedData SendCryptoDataWithString(quiche::QuicheStringPiece data, + QuicStreamOffset offset, + EncryptionLevel encryption_level) { if (!QuicVersionUsesCryptoFrames(transport_version())) { return SendStreamDataWithString( QuicUtils::GetCryptoStreamId(transport_version()), data, offset, NO_FIN); } - producer_.SaveCryptoData(ENCRYPTION_INITIAL, offset, data); + producer_.SaveCryptoData(encryption_level, offset, data); size_t bytes_written; if (notifier_) { bytes_written = - notifier_->WriteCryptoData(ENCRYPTION_INITIAL, data.length(), offset); + notifier_->WriteCryptoData(encryption_level, data.length(), offset); } else { - bytes_written = QuicConnection::SendCryptoData(ENCRYPTION_INITIAL, + bytes_written = QuicConnection::SendCryptoData(encryption_level, data.length(), offset); } return QuicConsumedData(bytes_written, /*fin_consumed*/ false); @@ -872,7 +880,7 @@ if (QuicConnectionPeer::GetSentPacketManager(this)->pto_enabled()) { // PTO mode is default enabled for T099. And TLP/RTO related tests are // stale. - DCHECK_EQ(ParsedQuicVersion(PROTOCOL_TLS1_3, QUIC_VERSION_99), version()); + DCHECK_EQ(PROTOCOL_TLS1_3, version().handshake_protocol); return true; } return false; @@ -9736,6 +9744,57 @@ IsError(IETF_QUIC_PROTOCOL_VIOLATION)); } +TEST_P(QuicConnectionTest, MultiplePacketNumberSpacePto) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + use_tagging_decrypter(); + // Send handshake packet. + connection_.SetEncrypter(ENCRYPTION_HANDSHAKE, + std::make_unique<TaggingEncrypter>(0x02)); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE); + connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE); + EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet()); + + // Send application data. + connection_.SendApplicationDataAtLevel(ENCRYPTION_FORWARD_SECURE, 5, "data", + 0, NO_FIN); + EXPECT_EQ(0x01010101u, writer_->final_bytes_of_last_packet()); + QuicTime retransmission_time = + connection_.GetRetransmissionAlarm()->deadline(); + EXPECT_NE(QuicTime::Zero(), retransmission_time); + + // Retransmit handshake data. + clock_.AdvanceTime(retransmission_time - clock_.Now()); + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(3), _, _)); + connection_.GetRetransmissionAlarm()->Fire(); + EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet()); + + // Send application data. + connection_.SendApplicationDataAtLevel(ENCRYPTION_FORWARD_SECURE, 5, "data", + 4, NO_FIN); + EXPECT_EQ(0x01010101u, writer_->final_bytes_of_last_packet()); + retransmission_time = connection_.GetRetransmissionAlarm()->deadline(); + EXPECT_NE(QuicTime::Zero(), retransmission_time); + + // Retransmit handshake data again. + clock_.AdvanceTime(retransmission_time - clock_.Now()); + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(5), _, _)); + connection_.GetRetransmissionAlarm()->Fire(); + EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet()); + + // Discard handshake key. + connection_.OnHandshakeComplete(); + retransmission_time = connection_.GetRetransmissionAlarm()->deadline(); + EXPECT_NE(QuicTime::Zero(), retransmission_time); + + // Retransmit application data. + clock_.AdvanceTime(retransmission_time - clock_.Now()); + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(6), _, _)); + connection_.GetRetransmissionAlarm()->Fire(); + EXPECT_EQ(0x01010101u, writer_->final_bytes_of_last_packet()); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index 767566c..9246499 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -493,6 +493,31 @@ return true; } +QuicTime QuicSentPacketManager::GetEarliestPacketSentTimeForPto( + PacketNumberSpace* packet_number_space) const { + DCHECK(supports_multiple_packet_number_spaces()); + QuicTime earliest_sent_time = QuicTime::Zero(); + for (int8_t i = 0; i < NUM_PACKET_NUMBER_SPACES; ++i) { + const QuicTime sent_time = unacked_packets_.GetLastInFlightPacketSentTime( + static_cast<PacketNumberSpace>(i)); + if (!earliest_sent_time.IsInitialized()) { + earliest_sent_time = sent_time; + *packet_number_space = static_cast<PacketNumberSpace>(i); + continue; + } + if (i == APPLICATION_DATA) { + // Not arming PTO for application data to prioritize the completion of + // handshake. + break; + } + if (sent_time.IsInitialized()) { + earliest_sent_time = std::min(earliest_sent_time, sent_time); + *packet_number_space = static_cast<PacketNumberSpace>(i); + } + } + return earliest_sent_time; +} + void QuicSentPacketManager::MarkForRetransmission( QuicPacketNumber packet_number, TransmissionType transmission_type) { @@ -776,12 +801,25 @@ if (pending_timer_transmission_count_ == 0) { return; } + PacketNumberSpace packet_number_space; + if (supports_multiple_packet_number_spaces()) { + // Find out the packet number space to send probe packets. + if (!GetEarliestPacketSentTimeForPto(&packet_number_space) + .IsInitialized()) { + QUIC_BUG << "earlist_sent_time not initialized when trying to send PTO " + "retransmissions"; + return; + } + } QuicPacketNumber packet_number = unacked_packets_.GetLeastUnacked(); std::vector<QuicPacketNumber> probing_packets; 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_.HasRetransmittableFrames(*it) && + (!supports_multiple_packet_number_spaces() || + unacked_packets_.GetPacketNumberSpace(it->encryption_level) == + packet_number_space)) { DCHECK(it->in_flight); probing_packets.push_back(packet_number); if (probing_packets.size() == pending_timer_transmission_count_) { @@ -948,9 +986,16 @@ return std::max(tlp_time, rto_time); } case PTO_MODE: { - // Ensure PTO never gets set to a time in the past. + if (!supports_multiple_packet_number_spaces()) { + // Ensure PTO never gets set to a time in the past. + return std::max(clock_->ApproximateNow(), + unacked_packets_.GetLastInFlightPacketSentTime() + + GetProbeTimeoutDelay()); + } + + PacketNumberSpace packet_number_space; return std::max(clock_->ApproximateNow(), - unacked_packets_.GetLastInFlightPacketSentTime() + + GetEarliestPacketSentTimeForPto(&packet_number_space) + GetProbeTimeoutDelay()); } } @@ -1267,6 +1312,7 @@ } void QuicSentPacketManager::EnableMultiplePacketNumberSpacesSupport() { + EnableIetfPtoAndLossDetection(); unacked_packets_.EnableMultiplePacketNumberSpacesSupport(); }
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index ab53066..f70b525 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -511,6 +511,11 @@ // timeout. bool ShouldAddMaxAckDelay() const; + // Gets the earliest in flight packet sent time to calculate PTO. Also + // updates |packet_number_space| if a PTO timer should be armed. + QuicTime GetEarliestPacketSentTimeForPto( + PacketNumberSpace* packet_number_space) const; + // Newly serialized retransmittable packets are added to this map, which // contains owning pointers to any contained frames. If a packet is // retransmitted, this map will contain entries for both the old and the new
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index 49d0b1d..247c03f 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -3294,6 +3294,119 @@ EXPECT_EQ(10u, manager_.initial_congestion_window()); } +TEST_F(QuicSentPacketManagerTest, ClientMultiplePacketNumberSpacePtoTimeout) { + manager_.EnableMultiplePacketNumberSpacesSupport(); + EnablePto(k1PTO); + 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); + // Verify PTO is correctly set. + QuicTime::Delta expected_pto_delay = + srtt + 4 * rtt_stats->mean_deviation() + + QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs); + EXPECT_EQ(clock_.Now() + expected_pto_delay, + manager_.GetRetransmissionTime()); + + // Discard initial key and send packet 2 in handshake. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); + manager_.NeuterUnencryptedPackets(); + + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(true)); + SendDataPacket(2, ENCRYPTION_HANDSHAKE); + // Verify PTO is correctly set based on sent time of packet 2. + EXPECT_EQ(clock_.Now() + expected_pto_delay, + manager_.GetRetransmissionTime()); + // Invoke PTO. + clock_.AdvanceTime(expected_pto_delay); + manager_.OnRetransmissionTimeout(); + EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); + EXPECT_EQ(1u, stats_.pto_count); + + // Verify probe packet gets sent. + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { + RetransmitDataPacket(3, type, ENCRYPTION_HANDSHAKE); + }))); + manager_.MaybeSendProbePackets(); + // Verify PTO period gets set to twice the current value. + const QuicTime packet3_sent_time = clock_.Now(); + EXPECT_EQ(packet3_sent_time + expected_pto_delay * 2, + manager_.GetRetransmissionTime()); + + // Send packet 4 in application data. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + SendDataPacket(4, ENCRYPTION_FORWARD_SECURE); + const QuicTime packet4_sent_time = clock_.Now(); + // Verify PTO timeout is still based on packet 3. + EXPECT_EQ(packet3_sent_time + expected_pto_delay * 2, + manager_.GetRetransmissionTime()); + + // Send packet 5 in handshake. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + SendDataPacket(5, ENCRYPTION_HANDSHAKE); + // Verify PTO timeout is now based on packet 5. + EXPECT_EQ(clock_.Now() + expected_pto_delay * 2, + manager_.GetRetransmissionTime()); + + // Neuter handshake key. + manager_.SetHandshakeConfirmed(); + // Verify PTO timeout is now based on packet 4. + EXPECT_EQ(packet4_sent_time + expected_pto_delay * 2, + manager_.GetRetransmissionTime()); +} + +TEST_F(QuicSentPacketManagerTest, ServerMultiplePacketNumberSpacePtoTimeout) { + manager_.EnableMultiplePacketNumberSpacesSupport(); + EnablePto(k1PTO); + 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); + const QuicTime packet1_sent_time = clock_.Now(); + // Verify PTO is correctly set. + QuicTime::Delta expected_pto_delay = + srtt + 4 * rtt_stats->mean_deviation() + + QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs); + EXPECT_EQ(packet1_sent_time + expected_pto_delay, + manager_.GetRetransmissionTime()); + + // Send packet 2 in handshake. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + SendDataPacket(2, ENCRYPTION_HANDSHAKE); + // Verify PTO timeout is still based on packet 1. + EXPECT_EQ(packet1_sent_time + expected_pto_delay, + manager_.GetRetransmissionTime()); + + // Discard both initial and handshake keys. + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); + manager_.NeuterUnencryptedPackets(); + manager_.SetHandshakeConfirmed(); + EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime()); + + // Send packet 3 in application data. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + SendDataPacket(3, ENCRYPTION_FORWARD_SECURE); + // Verify PTO timeout is correctly set. + EXPECT_EQ(clock_.Now() + expected_pto_delay, + manager_.GetRetransmissionTime()); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_unacked_packet_map.cc b/quic/core/quic_unacked_packet_map.cc index 50f5c85..ef08535 100644 --- a/quic/core/quic_unacked_packet_map.cc +++ b/quic/core/quic_unacked_packet_map.cc
@@ -30,6 +30,9 @@ bytes_in_flight_(0), packets_in_flight_(0), last_inflight_packet_sent_time_(QuicTime::Zero()), + last_inflight_packets_sent_time_{{QuicTime::Zero()}, + {QuicTime::Zero()}, + {QuicTime::Zero()}}, last_crypto_packet_sent_time_(QuicTime::Zero()), session_notifier_(nullptr), supports_multiple_packet_number_spaces_(false) {} @@ -66,14 +69,14 @@ largest_sent_packet_ = packet_number; if (set_in_flight) { + const PacketNumberSpace packet_number_space = + GetPacketNumberSpace(info.encryption_level); bytes_in_flight_ += bytes_sent; ++packets_in_flight_; info.in_flight = true; - largest_sent_retransmittable_packets_[GetPacketNumberSpace( - info.encryption_level)] = packet_number; - // TODO(ianswett): Should this field be per packet number space or should - // GetInFlightPacketSentTime() use largest_sent_retransmittable_packets_? + largest_sent_retransmittable_packets_[packet_number_space] = packet_number; last_inflight_packet_sent_time_ = sent_time; + last_inflight_packets_sent_time_[packet_number_space] = sent_time; } unacked_packets_.push_back(info); // Swap the retransmittable frames to avoid allocations. @@ -218,6 +221,9 @@ DCHECK(!HasRetransmittableFrames(*it)); } } + if (supports_multiple_packet_number_spaces_) { + last_inflight_packets_sent_time_[INITIAL_DATA] = QuicTime::Zero(); + } } void QuicUnackedPacketMap::NeuterHandshakePackets() { @@ -233,6 +239,9 @@ NotifyFramesAcked(*it, QuicTime::Delta::Zero(), QuicTime::Zero()); } } + if (supports_multiple_packet_number_spaces()) { + last_inflight_packets_sent_time_[HANDSHAKE_DATA] = QuicTime::Zero(); + } } bool QuicUnackedPacketMap::HasInFlightPackets() const { @@ -428,6 +437,15 @@ return largest_acked_packets_[packet_number_space]; } +QuicTime QuicUnackedPacketMap::GetLastInFlightPacketSentTime( + PacketNumberSpace packet_number_space) const { + if (packet_number_space >= NUM_PACKET_NUMBER_SPACES) { + QUIC_BUG << "Invalid packet number space: " << packet_number_space; + return QuicTime::Zero(); + } + return last_inflight_packets_sent_time_[packet_number_space]; +} + QuicPacketNumber QuicUnackedPacketMap::GetLargestSentRetransmittableOfPacketNumberSpace( PacketNumberSpace packet_number_space) const {
diff --git a/quic/core/quic_unacked_packet_map.h b/quic/core/quic_unacked_packet_map.h index 61261a5..1cad5eb 100644 --- a/quic/core/quic_unacked_packet_map.h +++ b/quic/core/quic_unacked_packet_map.h
@@ -212,6 +212,10 @@ QuicPacketNumber GetLargestSentPacketOfPacketNumberSpace( EncryptionLevel encryption_level) const; + // Returns last in flight packet sent time of |packet_number_space|. + QuicTime GetLastInFlightPacketSentTime( + PacketNumberSpace packet_number_space) const; + void SetSessionNotifier(SessionNotifierInterface* session_notifier); void EnableMultiplePacketNumberSpacesSupport(); @@ -273,6 +277,8 @@ // Time that the last inflight packet was sent. QuicTime last_inflight_packet_sent_time_; + // Time that the last in flight packet was sent per packet number space. + QuicTime last_inflight_packets_sent_time_[NUM_PACKET_NUMBER_SPACES]; // Time that the last unacked crypto packet was sent. QuicTime last_crypto_packet_sent_time_;