Deprecate gfe2_reloadable_flag_quic_use_ping_manager2. PiperOrigin-RevId: 491786729
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 91fc27c..93cef1d 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -123,17 +123,6 @@ } }; -class PingAlarmDelegate : public QuicConnectionAlarmDelegate { - public: - using QuicConnectionAlarmDelegate::QuicConnectionAlarmDelegate; - - void OnAlarm() override { - QUICHE_DCHECK(connection_->connected()); - QUICHE_DCHECK(!GetQuicReloadableFlag(quic_use_ping_manager2)); - connection_->OnPingTimeout(); - } -}; - class MtuDiscoveryAlarmDelegate : public QuicConnectionAlarmDelegate { public: using QuicConnectionAlarmDelegate::QuicConnectionAlarmDelegate; @@ -299,10 +288,6 @@ stop_waiting_count_(0), pending_retransmission_alarm_(false), defer_send_in_response_to_packets_(false), - keep_alive_ping_timeout_(QuicTime::Delta::FromSeconds(kPingTimeoutSecs)), - initial_retransmittable_on_wire_timeout_(QuicTime::Delta::Infinite()), - consecutive_retransmittable_on_wire_ping_count_(0), - retransmittable_on_wire_ping_count_(0), arena_(), ack_alarm_(alarm_factory_->CreateAlarm(arena_.New<AckAlarmDelegate>(this), &arena_)), @@ -310,8 +295,6 @@ arena_.New<RetransmissionAlarmDelegate>(this), &arena_)), send_alarm_(alarm_factory_->CreateAlarm( arena_.New<SendAlarmDelegate>(this), &arena_)), - ping_alarm_(alarm_factory_->CreateAlarm( - arena_.New<PingAlarmDelegate>(this), &arena_)), mtu_discovery_alarm_(alarm_factory_->CreateAlarm( arena_.New<MtuDiscoveryAlarmDelegate>(this), &arena_)), process_undecryptable_packets_alarm_(alarm_factory_->CreateAlarm( @@ -1360,11 +1343,7 @@ MaybeUpdateAckTimeout(); visitor_->OnStreamFrame(frame); stats_.stream_bytes_received += frame.data_length; - if (use_ping_manager_) { - ping_manager_.reset_consecutive_retransmittable_on_wire_count(); - } else { - consecutive_retransmittable_on_wire_ping_count_ = 0; - } + ping_manager_.reset_consecutive_retransmittable_on_wire_count(); return connected_; } @@ -3967,15 +3946,6 @@ WritePacket(&packet); } -void QuicConnection::OnPingTimeout() { - QUICHE_DCHECK(!use_ping_manager_); - if (retransmission_alarm_->IsSet() || - !visitor_->ShouldKeepConnectionAlive()) { - return; - } - SendPingAtLevel(framer().GetEncryptionLevelToSendApplicationData()); -} - void QuicConnection::SendAck() { QUICHE_DCHECK(!SupportsMultiplePacketNumberSpaces()); QUIC_DVLOG(1) << ENDPOINT << "Sending an ACK proactively"; @@ -4518,11 +4488,7 @@ QUIC_DVLOG(1) << "Cancelling all QuicConnection alarms."; ack_alarm_->PermanentCancel(); - if (use_ping_manager_) { - ping_manager_.Stop(); - } else { - ping_alarm_->PermanentCancel(); - } + ping_manager_.Stop(); retransmission_alarm_->PermanentCancel(); send_alarm_->PermanentCancel(); mtu_discovery_alarm_->PermanentCancel(); @@ -4566,79 +4532,9 @@ if (!connected_) { return; } - if (use_ping_manager_) { - QUIC_RELOADABLE_FLAG_COUNT(quic_use_ping_manager2); - ping_manager_.SetAlarm(clock_->ApproximateNow(), - visitor_->ShouldKeepConnectionAlive(), - sent_packet_manager_.HasInFlightPackets()); - return; - } - if (perspective_ == Perspective::IS_SERVER && - initial_retransmittable_on_wire_timeout_.IsInfinite()) { - // The PING alarm exists to support two features: - // 1) clients send PINGs every 15s to prevent NAT timeouts, - // 2) both clients and servers can send retransmittable on the wire PINGs - // (ROWP) while ShouldKeepConnectionAlive is true and there is no packets in - // flight. - return; - } - if (!visitor_->ShouldKeepConnectionAlive()) { - ping_alarm_->Cancel(); - // Don't send a ping unless the application (ie: HTTP/3) says to, usually - // because it is expecting a response from the server. - return; - } - if (initial_retransmittable_on_wire_timeout_.IsInfinite() || - sent_packet_manager_.HasInFlightPackets() || - retransmittable_on_wire_ping_count_ > - GetQuicFlag(quic_max_retransmittable_on_wire_ping_count)) { - if (perspective_ == Perspective::IS_CLIENT) { - // Clients send 15s PINGs to avoid NATs from timing out. - ping_alarm_->Update(clock_->ApproximateNow() + keep_alive_ping_timeout_, - QuicTime::Delta::FromSeconds(1)); - } else { - // Servers do not send 15s PINGs. - ping_alarm_->Cancel(); - } - return; - } - QUICHE_DCHECK_LT(initial_retransmittable_on_wire_timeout_, - keep_alive_ping_timeout_); - QuicTime::Delta retransmittable_on_wire_timeout = - initial_retransmittable_on_wire_timeout_; - int max_aggressive_retransmittable_on_wire_ping_count = - GetQuicFlag(quic_max_aggressive_retransmittable_on_wire_ping_count); - QUICHE_DCHECK_LE(0, max_aggressive_retransmittable_on_wire_ping_count); - if (consecutive_retransmittable_on_wire_ping_count_ > - max_aggressive_retransmittable_on_wire_ping_count) { - // Exponentially back off the timeout if the number of consecutive - // retransmittable on wire pings has exceeds the allowance. - int shift = consecutive_retransmittable_on_wire_ping_count_ - - max_aggressive_retransmittable_on_wire_ping_count; - retransmittable_on_wire_timeout = - initial_retransmittable_on_wire_timeout_ * (1 << shift); - } - // If it's already set to an earlier time, then don't update it. - if (ping_alarm_->IsSet() && - ping_alarm_->deadline() < - clock_->ApproximateNow() + retransmittable_on_wire_timeout) { - return; - } - - if (retransmittable_on_wire_timeout < keep_alive_ping_timeout_) { - // Use a shorter timeout if there are open streams, but nothing on the wire. - ping_alarm_->Update( - clock_->ApproximateNow() + retransmittable_on_wire_timeout, - kAlarmGranularity); - if (max_aggressive_retransmittable_on_wire_ping_count != 0) { - consecutive_retransmittable_on_wire_ping_count_++; - } - retransmittable_on_wire_ping_count_++; - return; - } - - ping_alarm_->Update(clock_->ApproximateNow() + keep_alive_ping_timeout_, - kAlarmGranularity); + ping_manager_.SetAlarm(clock_->ApproximateNow(), + visitor_->ShouldKeepConnectionAlive(), + sent_packet_manager_.HasInFlightPackets()); } void QuicConnection::SetRetransmissionAlarm() { @@ -6194,7 +6090,6 @@ } void QuicConnection::OnKeepAliveTimeout() { - QUICHE_DCHECK(use_ping_manager_); if (retransmission_alarm_->IsSet() || !visitor_->ShouldKeepConnectionAlive()) { return; @@ -6203,7 +6098,6 @@ } void QuicConnection::OnRetransmittableOnWireTimeout() { - QUICHE_DCHECK(use_ping_manager_); if (retransmission_alarm_->IsSet() || !visitor_->ShouldKeepConnectionAlive()) { return; @@ -7175,23 +7069,13 @@ void QuicConnection::set_keep_alive_ping_timeout( QuicTime::Delta keep_alive_ping_timeout) { - if (use_ping_manager_) { - ping_manager_.set_keep_alive_timeout(keep_alive_ping_timeout); - return; - } - QUICHE_DCHECK(!ping_alarm_->IsSet()); - keep_alive_ping_timeout_ = keep_alive_ping_timeout; + ping_manager_.set_keep_alive_timeout(keep_alive_ping_timeout); } void QuicConnection::set_initial_retransmittable_on_wire_timeout( QuicTime::Delta retransmittable_on_wire_timeout) { - if (use_ping_manager_) { - ping_manager_.set_initial_retransmittable_on_wire_timeout( - retransmittable_on_wire_timeout); - return; - } - QUICHE_DCHECK(!ping_alarm_->IsSet()); - initial_retransmittable_on_wire_timeout_ = retransmittable_on_wire_timeout; + ping_manager_.set_initial_retransmittable_on_wire_timeout( + retransmittable_on_wire_timeout); } #undef ENDPOINT // undef for jumbo builds
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 869409b..63d4723 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -573,10 +573,6 @@ QuicConnectionStats& mutable_stats() { return stats_; } - int retransmittable_on_wire_ping_count() const { - return retransmittable_on_wire_ping_count_; - } - // Returns statistics tracked for this connection. const QuicConnectionStats& GetStats(); @@ -839,10 +835,6 @@ return multi_port_stats_.get(); } - // Called when the ping alarm fires. Causes a ping frame to be sent only - // if the retransmission alarm is not running. - void OnPingTimeout(); - // Sets up a packet with an QuicAckFrame and sends it out. void SendAck(); @@ -2030,21 +2022,6 @@ // SendAlarm. bool defer_send_in_response_to_packets_; - // TODO(fayang): remove PING related fields below when deprecating - // quic_use_ping_manager2. - // The timeout for keep-alive PING. - QuicTime::Delta keep_alive_ping_timeout_; - - // Initial timeout for how long the wire can have no retransmittable packets. - QuicTime::Delta initial_retransmittable_on_wire_timeout_; - - // Indicates how many retransmittable-on-wire pings have been emitted without - // receiving any new data in between. - int consecutive_retransmittable_on_wire_ping_count_; - - // Indicates how many retransmittable-on-wire pings have been emitted. - int retransmittable_on_wire_ping_count_; - // Arena to store class implementations within the QuicConnection. QuicConnectionArena arena_; @@ -2055,9 +2032,6 @@ // An alarm that is scheduled when the SentPacketManager requires a delay // before sending packets and fires when the packet may be sent. QuicArenaScopedPtr<QuicAlarm> send_alarm_; - // TODO(fayang): remove ping_alarm_ when deprecating quic_use_ping_manager2. - // An alarm that fires when a ping should be sent. - QuicArenaScopedPtr<QuicAlarm> ping_alarm_; // An alarm that fires when an MTU probe should be sent. QuicArenaScopedPtr<QuicAlarm> mtu_discovery_alarm_; // An alarm that fires to process undecryptable packets when new decyrption @@ -2272,8 +2246,6 @@ // If true, send connection close packet on INVALID_VERSION. bool send_connection_close_for_invalid_version_ = false; - const bool use_ping_manager_ = GetQuicReloadableFlag(quic_use_ping_manager2); - QuicPingManager ping_manager_; // Records first serialized 1-RTT packet.
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index e700d88..fdb0a0d 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -7343,8 +7343,7 @@ } TEST_P(QuicConnectionTest, RetransmittableOnWireSendFirstPacket) { - if (!GetQuicReloadableFlag(quic_use_ping_manager2) || - !VersionHasIetfQuicFrames(connection_.version().transport_version)) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) @@ -7389,8 +7388,7 @@ } TEST_P(QuicConnectionTest, RetransmittableOnWireSendRandomBytes) { - if (!GetQuicReloadableFlag(quic_use_ping_manager2) || - !VersionHasIetfQuicFrames(connection_.version().transport_version)) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) @@ -7438,8 +7436,7 @@ TEST_P(QuicConnectionTest, RetransmittableOnWireSendRandomBytesWithWriterBlocked) { - if (!GetQuicReloadableFlag(quic_use_ping_manager2) || - !VersionHasIetfQuicFrames(connection_.version().transport_version)) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } EXPECT_CALL(visitor_, ShouldKeepConnectionAlive())
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index e84b98d..5fcb551 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -77,8 +77,6 @@ QUIC_FLAG(quic_reloadable_flag_quic_conservative_bursts, false) // If true, use BBRv2 as the default congestion controller. Takes precedence over --quic_default_to_bbr. QUIC_FLAG(quic_reloadable_flag_quic_default_to_bbr_v2, false) -// If true, use PING manager to manage the PING alarm. -QUIC_FLAG(quic_reloadable_flag_quic_use_ping_manager2, true) // If true, use new connection ID in connection migration. QUIC_FLAG(quic_reloadable_flag_quic_connection_migration_use_new_cid_v2, true) // If true, use next_connection_id_sequence_number to validate retired cid number.
diff --git a/quiche/quic/test_tools/quic_connection_peer.cc b/quiche/quic/test_tools/quic_connection_peer.cc index f7c6424..b2d1862 100644 --- a/quiche/quic/test_tools/quic_connection_peer.cc +++ b/quiche/quic/test_tools/quic_connection_peer.cc
@@ -146,10 +146,7 @@ // static QuicAlarm* QuicConnectionPeer::GetPingAlarm(QuicConnection* connection) { - if (GetQuicReloadableFlag(quic_use_ping_manager2)) { - return connection->ping_manager_.alarm_.get(); - } - return connection->ping_alarm_.get(); + return connection->ping_manager_.alarm_.get(); } // static