Handle PINGs in QUIC connection. Currently, sending and retransmission of PINGs are handled by control frame manager (and session). Protected by FLAGS_quic_reloadable_flag_quic_let_connection_handle_pings. PiperOrigin-RevId: 335072367 Change-Id: I79f20404656beee3d9404c1ccaa8ef2f00af567c
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index da0cdaf..a2f32b6 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -3233,7 +3233,11 @@ !visitor_->ShouldKeepConnectionAlive()) { return; } - visitor_->SendPing(); + if (packet_creator_.let_connection_handle_pings()) { + SendPingAtLevel(encryption_level_); + } else { + visitor_->SendPing(); + } } void QuicConnection::SendAck() { @@ -3329,7 +3333,19 @@ << "No packet gets sent when timer fires in mode " << retransmission_mode << ", send PING"; DCHECK_LT(0u, sent_packet_manager_.pending_timer_transmission_count()); - visitor_->SendPing(); + if (packet_creator_.let_connection_handle_pings()) { + EncryptionLevel level = encryption_level_; + PacketNumberSpace packet_number_space = NUM_PACKET_NUMBER_SPACES; + if (SupportsMultiplePacketNumberSpaces() && + sent_packet_manager_ + .GetEarliestPacketSentTimeForPto(&packet_number_space) + .IsInitialized()) { + level = QuicUtils::GetEncryptionLevel(packet_number_space); + } + SendPingAtLevel(level); + } else { + visitor_->SendPing(); + } } if (retransmission_mode == QuicSentPacketManager::PTO_MODE) { sent_packet_manager_.AdjustPendingTimerTransmissions(); @@ -3935,6 +3951,24 @@ !connection_->packet_creator_.PacketFlusherAttached()); } +QuicConnection::ScopedEncryptionLevelContext::ScopedEncryptionLevelContext( + QuicConnection* connection, + EncryptionLevel encryption_level) + : connection_(connection), latched_encryption_level_(ENCRYPTION_INITIAL) { + if (connection_ == nullptr) { + return; + } + latched_encryption_level_ = connection_->encryption_level_; + connection_->SetDefaultEncryptionLevel(encryption_level); +} + +QuicConnection::ScopedEncryptionLevelContext::~ScopedEncryptionLevelContext() { + if (connection_ == nullptr || !connection_->connected_) { + return; + } + connection_->SetDefaultEncryptionLevel(latched_encryption_level_); +} + QuicConnection::BufferedPacket::BufferedPacket( const SerializedPacket& packet, const QuicSocketAddress& self_address, @@ -5078,5 +5112,11 @@ packet_creator_.SetDefaultPeerAddress(peer_address); } +void QuicConnection::SendPingAtLevel(EncryptionLevel level) { + DCHECK(packet_creator_.let_connection_handle_pings()); + ScopedEncryptionLevelContext context(this, level); + SendControlFrame(QuicFrame(QuicPingFrame())); +} + #undef ENDPOINT // undef for jumbo builds } // namespace quic
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 0617478..1abb1f7 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -831,6 +831,18 @@ const bool handshake_packet_sent_; }; + class QUIC_EXPORT_PRIVATE ScopedEncryptionLevelContext { + public: + ScopedEncryptionLevelContext(QuicConnection* connection, + EncryptionLevel level); + ~ScopedEncryptionLevelContext(); + + private: + QuicConnection* connection_; + // Latched current write encryption level on creation of this context. + EncryptionLevel latched_encryption_level_; + }; + QuicPacketWriter* writer() { return writer_; } const QuicPacketWriter* writer() const { return writer_; } @@ -1422,6 +1434,9 @@ // Update both connection's and packet creator's peer address. void UpdatePeerAddress(QuicSocketAddress peer_address); + // Send PING at encryption level. + void SendPingAtLevel(EncryptionLevel level); + QuicFramer framer_; // Contents received in the current packet, especially used to identify
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index b18b367..06488e5 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -4024,7 +4024,11 @@ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); // Simulate firing of the retransmittable on wire and send a PING. - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } clock_.AdvanceTime(retransmittable_on_wire_timeout); connection_.GetPingAlarm()->Fire(); @@ -4038,13 +4042,21 @@ QuicTime::Delta::FromMilliseconds(kMinRetransmissionTimeMs); srtt = manager_->GetRttStats()->SmoothedOrInitialRtt(); - // First TLP without unacked stream data will no longer use TLPR. - expected_delay = std::max(2 * srtt, 1.5 * srtt + 0.5 * min_rto_timeout); + if (GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + // Arm RTO mode since there is only PING in flight. + expected_delay = manager_->GetPtoDelay(); + } else { + // First TLP without unacked stream data will no longer use TLPR. + expected_delay = std::max(2 * srtt, 1.5 * srtt + 0.5 * min_rto_timeout); + } EXPECT_EQ(expected_delay, connection_.GetRetransmissionAlarm()->deadline() - clock_.Now()); // Verify the path degrading delay = TLP delay + 1st RTO + 2nd RTO. // Add 1st RTO. + if (GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + expected_delay = std::max(2 * srtt, 1.5 * srtt + 0.5 * min_rto_timeout); + } retransmission_delay = std::max(manager_->GetRttStats()->smoothed_rtt() + 4 * manager_->GetRttStats()->mean_deviation(), @@ -4071,7 +4083,12 @@ // Verify the retransmission delay. // First TLP without unacked stream data will no longer use TLPR. - expected_delay = std::max(2 * srtt, 1.5 * srtt + 0.5 * min_rto_timeout); + if (GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + // Arm RTO mode since there is only PING in flight. + expected_delay = manager_->GetPtoDelay(); + } else { + expected_delay = std::max(2 * srtt, 1.5 * srtt + 0.5 * min_rto_timeout); + } expected_delay = expected_delay - QuicTime::Delta::FromMilliseconds(5); EXPECT_EQ(expected_delay, connection_.GetRetransmissionAlarm()->deadline() - clock_.Now()); @@ -4597,7 +4614,11 @@ writer_->Reset(); clock_.AdvanceTime(QuicTime::Delta::FromSeconds(15)); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } connection_.GetPingAlarm()->Fire(); size_t padding_frame_count = writer_->padding_frames().size(); EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); @@ -4651,9 +4672,11 @@ writer_->Reset(); clock_.AdvanceTime(QuicTime::Delta::FromSeconds(10)); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { - connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); - })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); + })); + } connection_.GetPingAlarm()->Fire(); size_t padding_frame_count = writer_->padding_frames().size(); EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); @@ -7159,9 +7182,11 @@ // Simulate firing the ping alarm and sending a PING. clock_.AdvanceTime(retransmittable_on_wire_timeout); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { - connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); - })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); + })); + } connection_.GetPingAlarm()->Fire(); // Now there's a retransmittable packet (PING) on the wire, so the path @@ -7862,9 +7887,11 @@ EXPECT_EQ(prev_deadline, connection_.GetPingAlarm()->deadline()); // Simulate the alarm firing and check that a PING is sent. - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { - connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); - })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); + })); + } connection_.GetPingAlarm()->Fire(); size_t padding_frame_count = writer_->padding_frames().size(); if (GetParam().no_stop_waiting) { @@ -7935,9 +7962,11 @@ // Simulate the alarm firing and check that a PING is sent. writer_->Reset(); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { - connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); - })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); + })); + } connection_.GetPingAlarm()->Fire(); size_t padding_frame_count = writer_->padding_frames().size(); if (GetParam().no_stop_waiting) { @@ -7997,9 +8026,11 @@ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); // Simulate the alarm firing and check that a PING is sent. writer_->Reset(); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { - SendPing(); - })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } clock_.AdvanceTime(initial_retransmittable_on_wire_timeout); connection_.GetPingAlarm()->Fire(); } @@ -8024,9 +8055,11 @@ // Simulate the alarm firing and check that a PING is sent. writer_->Reset(); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { - SendPing(); - })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } clock_.AdvanceTime(retransmittable_on_wire_timeout); connection_.GetPingAlarm()->Fire(); } @@ -8092,7 +8125,11 @@ // Simulate the alarm firing and check that a PING is sent. writer_->Reset(); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } clock_.AdvanceTime(initial_retransmittable_on_wire_timeout); connection_.GetPingAlarm()->Fire(); @@ -8128,9 +8165,11 @@ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); // Simulate the alarm firing and check that a PING is sent. writer_->Reset(); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { - SendPing(); - })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } clock_.AdvanceTime(initial_retransmittable_on_wire_timeout); connection_.GetPingAlarm()->Fire(); // Advance 5ms to receive next packet. @@ -8148,7 +8187,11 @@ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); writer_->Reset(); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } clock_.AdvanceTime(2 * initial_retransmittable_on_wire_timeout); connection_.GetPingAlarm()->Fire(); @@ -9110,7 +9153,11 @@ // RTO fires, verify a PING packet gets sent because there is no data to send. EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(3), _, _)); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } clock_.AdvanceTime(retransmission_time - clock_.Now()); connection_.GetRetransmissionAlarm()->Fire(); EXPECT_EQ(1u, connection_.GetStats().tlp_count); @@ -9330,7 +9377,11 @@ ? QuicPacketNumber(2) : QuicPacketNumber(3), _, _)); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } connection_.GetRetransmissionAlarm()->Fire(); EXPECT_EQ(1u, connection_.GetStats().pto_count); EXPECT_EQ(1u, connection_.GetStats().crypto_retransmit_count); @@ -9627,7 +9678,11 @@ // Fires TLP, verify a PING gets sent because packet 3 is marked // RTO_RETRANSMITTED. EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(70), _, _)); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } connection_.GetRetransmissionAlarm()->Fire(); } @@ -10218,7 +10273,11 @@ // PTO fires, verify a PING packet gets sent because there is no data to // send. EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(3), _, _)); - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } connection_.GetRetransmissionAlarm()->Fire(); EXPECT_EQ(1u, connection_.GetStats().pto_count); EXPECT_EQ(0u, connection_.GetStats().crypto_retransmit_count); @@ -11003,7 +11062,11 @@ EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) .WillRepeatedly(Return(false)); // Verify PING does not get sent. - EXPECT_CALL(visitor_, SendPing()).Times(0); + if (GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0); + } else { + EXPECT_CALL(visitor_, SendPing()).Times(0); + } connection_.GetPingAlarm()->Fire(); } @@ -11581,6 +11644,10 @@ if (!GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); } + } else if (GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + if (!GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { + EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); + } } else { EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(0); EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { @@ -11592,10 +11659,15 @@ // Verify a handshake packet gets PTOed and 1-RTT packet gets coalesced. EXPECT_EQ(0x03030303u, writer_->final_bytes_of_last_packet()); } else { - // Verify an 1-RTT PING gets sent because there is nothing to PTO, bummer, - // since this 1-RTT PING cannot be processed by peer and there is a - // deadlock. - EXPECT_EQ(0x03030303u, writer_->final_bytes_of_last_packet()); + if (GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + // Verify PING is sent in the right encryption level. + EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet()); + } else { + // Verify an 1-RTT PING gets sent because there is nothing to PTO, bummer, + // since this 1-RTT PING cannot be processed by peer and there is a + // deadlock. + EXPECT_EQ(0x03030303u, writer_->final_bytes_of_last_packet()); + } EXPECT_FALSE(writer_->ping_frames().empty()); } } @@ -11684,7 +11756,11 @@ EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet()); // Fire retransmission alarm. - EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); })); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } connection_.GetRetransmissionAlarm()->Fire(); QuicFrames frames1;
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc index 30b911f..af9d079 100644 --- a/quic/core/quic_packet_creator.cc +++ b/quic/core/quic_packet_creator.cc
@@ -137,6 +137,9 @@ if (close_connection_on_serialization_failure_) { QUIC_RELOADABLE_FLAG_COUNT(quic_close_connection_on_serialization_failure); } + if (let_connection_handle_pings_) { + QUIC_RELOADABLE_FLAG_COUNT(quic_let_connection_handle_pings); + } SetMaxPacketLength(kDefaultMaxPacketSize); if (!framer_->version().UsesTls()) { // QUIC+TLS negotiates the maximum datagram frame size via the @@ -1303,7 +1306,8 @@ bool QuicPacketCreator::ConsumeRetransmittableControlFrame( const QuicFrame& frame) { - QUIC_BUG_IF(IsControlFrame(frame.type) && !GetControlFrameId(frame)) + QUIC_BUG_IF(IsControlFrame(frame.type) && !GetControlFrameId(frame) && + (!let_connection_handle_pings_ || frame.type != PING_FRAME)) << "Adding a control frame with no control frame id: " << frame; DCHECK(QuicUtils::IsRetransmittableFrame(frame.type)) << frame; MaybeBundleAckOpportunistically();
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h index b025d06..b65bd27 100644 --- a/quic/core/quic_packet_creator.h +++ b/quic/core/quic_packet_creator.h
@@ -469,6 +469,10 @@ // different from the current one, flush all the queue frames first. void SetDefaultPeerAddress(QuicSocketAddress address); + bool let_connection_handle_pings() const { + return let_connection_handle_pings_; + } + private: friend class test::QuicPacketCreatorPeer; @@ -665,6 +669,9 @@ const bool close_connection_on_serialization_failure_ = GetQuicReloadableFlag(quic_close_connection_on_serialization_failure); + + const bool let_connection_handle_pings_ = + GetQuicReloadableFlag(quic_let_connection_handle_pings); }; } // namespace quic
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index 6e36da9..b332034 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -442,6 +442,11 @@ void OnUserAgentIdKnown() { loss_algorithm_->OnUserAgentIdKnown(); } + // 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; + bool give_sent_packet_to_debug_visitor_after_sent() const { return give_sent_packet_to_debug_visitor_after_sent_; } @@ -540,11 +545,6 @@ // timeout. bool ShouldAddMaxAckDelay(PacketNumberSpace space) 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; - // Returns true if application data should be used to arm PTO. Only used when // multiple packet number space is enabled. bool ShouldArmPtoForApplicationData() const;