Deprecate gfe2_reloadable_flag_quic_simplify_set_retransmission_alarm. PiperOrigin-RevId: 452313856
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index af9eb4a..a88d7f5 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -1969,29 +1969,6 @@ } } -bool QuicConnection::ShouldSetRetransmissionAlarmOnPacketSent( - bool in_flight, EncryptionLevel level) const { - QUICHE_DCHECK(!sent_packet_manager_.simplify_set_retransmission_alarm()); - if (!retransmission_alarm_->IsSet()) { - return true; - } - if (!in_flight) { - return false; - } - - if (!SupportsMultiplePacketNumberSpaces()) { - return true; - } - // Before handshake gets confirmed, do not re-arm PTO timer on application - // data. Think about this scenario: on the client side, the CHLO gets - // acknowledged and the SHLO is not received yet. The PTO alarm is set when - // the CHLO acknowledge is received (and there is no in flight INITIAL - // packet). Re-arming PTO alarm on 0-RTT packet would keep postponing the PTO - // alarm. - return IsHandshakeConfirmed() || level == ENCRYPTION_INITIAL || - level == ENCRYPTION_HANDSHAKE; -} - bool QuicConnection::OnNewConnectionIdFrameInner( const QuicNewConnectionIdFrame& frame) { if (peer_issued_cid_manager_ == nullptr) { @@ -3679,13 +3656,7 @@ return true; } } - if (sent_packet_manager_.simplify_set_retransmission_alarm()) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_simplify_set_retransmission_alarm, 1, 2); - if (in_flight || !retransmission_alarm_->IsSet()) { - SetRetransmissionAlarm(); - } - } else if (ShouldSetRetransmissionAlarmOnPacketSent( - in_flight, packet->encryption_level)) { + if (in_flight || !retransmission_alarm_->IsSet()) { SetRetransmissionAlarm(); } SetPingAlarm(); @@ -4362,14 +4333,7 @@ undecryptable_packets_.emplace_back(packet, decryption_level, last_received_packet_info_); if (perspective_ == Perspective::IS_CLIENT) { - if (sent_packet_manager_.simplify_set_retransmission_alarm()) { - SetRetransmissionAlarm(); - } else if (!retransmission_alarm_->IsSet() || - GetRetransmissionDeadline() < - retransmission_alarm_->deadline()) { - // Re-arm PTO only if we can make it sooner to speed up recovery. - SetRetransmissionAlarm(); - } + SetRetransmissionAlarm(); } } @@ -4433,18 +4397,7 @@ undecryptable_packets_.clear(); } if (perspective_ == Perspective::IS_CLIENT) { - if (sent_packet_manager_.simplify_set_retransmission_alarm()) { - SetRetransmissionAlarm(); - } else if (!retransmission_alarm_->IsSet() || - undecryptable_packets_.empty() || - GetRetransmissionDeadline() < - retransmission_alarm_->deadline()) { - // 1) If there is still undecryptable packet, only re-arm PTO to make it - // sooner to speed up recovery. - // 2) If all undecryptable packets get processed, re-arm (which may - // postpone) PTO since no immediate recovery is needed. - SetRetransmissionAlarm(); - } + SetRetransmissionAlarm(); } } @@ -4789,15 +4742,13 @@ return; } PacketNumberSpace packet_number_space; - if (sent_packet_manager_.simplify_set_retransmission_alarm() && - SupportsMultiplePacketNumberSpaces() && !IsHandshakeConfirmed() && + if (SupportsMultiplePacketNumberSpaces() && !IsHandshakeConfirmed() && !sent_packet_manager_ .GetEarliestPacketSentTimeForPto(&packet_number_space) .IsInitialized()) { // Before handshake gets confirmed, GetEarliestPacketSentTimeForPto // returning 0 indicates no packets are in flight or only application data // is in flight. - QUIC_RELOADABLE_FLAG_COUNT_N(quic_simplify_set_retransmission_alarm, 2, 2); if (perspective_ == Perspective::IS_SERVER) { // No need to arm PTO on server side. retransmission_alarm_->Cancel();
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 8bb2629..276a960 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1858,11 +1858,6 @@ // when a new client connection ID is received. void OnClientConnectionIdAvailable(); - // Returns true if connection needs to set retransmission alarm after a packet - // gets sent. - bool ShouldSetRetransmissionAlarmOnPacketSent(bool in_flight, - EncryptionLevel level) const; - // Determines encryption level to send ping in `packet_number_space`. EncryptionLevel GetEncryptionLevelToSendPingForSpace( PacketNumberSpace space) const;
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index c4f9f91..62407d4 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -35,8 +35,6 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bounded_crypto_send_buffer, false) // If true, consider original connection ID as active before handshake completes. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_consider_original_connection_id_as_active_pre_handshake, false) -// If true, consolidate more logic into SetRetransmissionAlarm to ensure the logic is applied consistently. -QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_simplify_set_retransmission_alarm, true) // If true, default-enable 5RTO blachole detection. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_default_enable_5rto_blackhole_detection2, true) // If true, deliver INITIAL packets before other types of packets in QuicBufferedPacketStore.
diff --git a/quiche/quic/core/quic_sent_packet_manager.cc b/quiche/quic/core/quic_sent_packet_manager.cc index 41e6ea7..76061c4 100644 --- a/quiche/quic/core/quic_sent_packet_manager.cc +++ b/quiche/quic/core/quic_sent_packet_manager.cc
@@ -996,15 +996,6 @@ // Do not set the timer if there is any credit left. return QuicTime::Zero(); } - PacketNumberSpace packet_number_space; - if (!simplify_set_retransmission_alarm_ && - supports_multiple_packet_number_spaces() && - unacked_packets_.perspective() == Perspective::IS_SERVER && - !GetEarliestPacketSentTimeForPto(&packet_number_space).IsInitialized()) { - // Do not set the timer on the server side if the only in flight packets are - // half RTT data. - return QuicTime::Zero(); - } switch (GetRetransmissionMode()) { case HANDSHAKE_MODE: return unacked_packets_.GetLastCryptoPacketSentTime() +
diff --git a/quiche/quic/core/quic_sent_packet_manager.h b/quiche/quic/core/quic_sent_packet_manager.h index 04ba435..953c11c 100644 --- a/quiche/quic/core/quic_sent_packet_manager.h +++ b/quiche/quic/core/quic_sent_packet_manager.h
@@ -466,10 +466,6 @@ bool use_lower_min_irtt() const { return use_lower_min_irtt_; } - bool simplify_set_retransmission_alarm() const { - return simplify_set_retransmission_alarm_; - } - private: friend class test::QuicConnectionPeer; friend class test::QuicSentPacketManagerPeer; @@ -678,9 +674,6 @@ // Latched value of --quic_use_lower_min_for_trusted_irtt. bool use_lower_min_irtt_ = GetQuicReloadableFlag(quic_use_lower_min_for_trusted_irtt); - - const bool simplify_set_retransmission_alarm_ = - GetQuicReloadableFlag(quic_simplify_set_retransmission_alarm); }; } // namespace quic
diff --git a/quiche/quic/core/quic_sent_packet_manager_test.cc b/quiche/quic/core/quic_sent_packet_manager_test.cc index 9c8db65..6c823a4 100644 --- a/quiche/quic/core/quic_sent_packet_manager_test.cc +++ b/quiche/quic/core/quic_sent_packet_manager_test.cc
@@ -2699,53 +2699,6 @@ EXPECT_EQ(packet2_sent_time + pto_delay, manager_.GetRetransmissionTime()); } -// Regression test for b/157895910. -TEST_F(QuicSentPacketManagerTest, EarliestSentTimeNotInitializedWhenPtoFires) { - if (GetQuicReloadableFlag(quic_simplify_set_retransmission_alarm)) { - return; - } - manager_.EnableMultiplePacketNumberSpacesSupport(); - EXPECT_CALL(*send_algorithm_, PacingRate(_)) - .WillRepeatedly(Return(QuicBandwidth::Zero())); - EXPECT_CALL(*send_algorithm_, GetCongestionWindow()) - .WillRepeatedly(Return(10 * kDefaultTCPMSS)); - - // Send INITIAL 1. - SendDataPacket(1, ENCRYPTION_INITIAL); - - // Send HANDSHAKE packets. - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); - SendDataPacket(2, ENCRYPTION_HANDSHAKE); - SendDataPacket(3, ENCRYPTION_HANDSHAKE); - SendDataPacket(4, ENCRYPTION_HANDSHAKE); - - // Send half RTT packet. - SendDataPacket(5, ENCRYPTION_FORWARD_SECURE); - - // Received ACK for INITIAL packet 1. - ExpectAck(1); - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(90)); - manager_.OnAckFrameStart(QuicPacketNumber(1), QuicTime::Delta::Infinite(), - clock_.Now()); - manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2)); - EXPECT_EQ(PACKETS_NEWLY_ACKED, - manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), - ENCRYPTION_INITIAL)); - - // Received ACK for HANDSHAKE packets. - uint64_t acked[] = {2, 3, 4}; - ExpectAcksAndLosses(true, acked, ABSL_ARRAYSIZE(acked), nullptr, 0); - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(90)); - manager_.OnAckFrameStart(QuicPacketNumber(4), QuicTime::Delta::Infinite(), - clock_.Now()); - manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(5)); - EXPECT_EQ(PACKETS_NEWLY_ACKED, - manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(4), - ENCRYPTION_HANDSHAKE)); - // Verify PTO will not be armed. - EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime()); -} - TEST_F(QuicSentPacketManagerTest, MaybeRetransmitInitialData) { manager_.EnableMultiplePacketNumberSpacesSupport(); EXPECT_CALL(*send_algorithm_, PacingRate(_))