Enable QUIC Retransmittable-On-Wire-PING experiment on server side. Protected by FLAGS_quic_reloadable_flag_quic_enable_server_on_wire_ping. PiperOrigin-RevId: 345216641 Change-Id: I39bc30c6687d4b11637eda7c69f3b1e658194b0f
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h index 853c42a..ddd3dbd 100644 --- a/quic/core/crypto/crypto_protocol.h +++ b/quic/core/crypto/crypto_protocol.h
@@ -402,6 +402,10 @@ // for RTT measure or // congestion control. +const QuicTag kSRWP = TAG('S', 'R', 'W', 'P'); // Enable retransmittable on + // wire PING (ROWP) on the + // server side. + // Rejection tags const QuicTag kRREJ = TAG('R', 'R', 'E', 'J'); // Reasons for server sending
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index e0abf37..f864140 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -656,6 +656,14 @@ anti_amplification_factor_ = 10; } + if (GetQuicReloadableFlag(quic_enable_server_on_wire_ping) && + perspective_ == Perspective::IS_SERVER && + config.HasClientSentConnectionOption(kSRWP, perspective_)) { + QUIC_RELOADABLE_FLAG_COUNT(quic_enable_server_on_wire_ping); + set_initial_retransmittable_on_wire_timeout( + QuicTime::Delta::FromMilliseconds(200)); + } + if (debug_visitor_ != nullptr) { debug_visitor_->OnSetFromConfig(config); } @@ -4083,8 +4091,13 @@ } void QuicConnection::SetPingAlarm() { - if (perspective_ == Perspective::IS_SERVER) { - // Only clients send pings to avoid NATs from timing out. + 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()) { @@ -4097,9 +4110,14 @@ sent_packet_manager_.HasInFlightPackets() || retransmittable_on_wire_ping_count_ > GetQuicFlag(FLAGS_quic_max_retransmittable_on_wire_ping_count)) { - // Extend the ping alarm. - ping_alarm_->Update(clock_->ApproximateNow() + ping_timeout_, - QuicTime::Delta::FromSeconds(1)); + if (perspective_ == Perspective::IS_CLIENT) { + // Clients send 15s PINGs to avoid NATs from timing out. + ping_alarm_->Update(clock_->ApproximateNow() + ping_timeout_, + QuicTime::Delta::FromSeconds(1)); + } else { + // Servers do not send 15s PINGs. + ping_alarm_->Cancel(); + } return; } DCHECK_LT(initial_retransmittable_on_wire_timeout_, ping_timeout_);
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 10ebebb..327a696 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -706,7 +706,6 @@ ping_timeout_ = ping_timeout; } const QuicTime::Delta ping_timeout() const { return ping_timeout_; } - // Used in Chromium, but not internally. // Sets an initial timeout for the ping alarm when there is no retransmittable // data in flight, allowing for a more aggressive ping alarm in that case. void set_initial_retransmittable_on_wire_timeout(
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 783913e..52d4d84 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -7119,6 +7119,45 @@ clock_.ApproximateNow()); } +TEST_P(QuicConnectionTest, ServerRetransmittableOnWire) { + set_perspective(Perspective::IS_SERVER); + QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false); + SetQuicReloadableFlag(quic_enable_server_on_wire_ping, true); + + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + QuicConfig config; + QuicTagVector connection_options; + connection_options.push_back(kSRWP); + config.SetInitialReceivedConnectionOptions(connection_options); + connection_.SetFromConfig(config); + + EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) + .WillRepeatedly(Return(true)); + + ProcessPacket(1); + + ASSERT_TRUE(connection_.GetPingAlarm()->IsSet()); + QuicTime::Delta ping_delay = QuicTime::Delta::FromMilliseconds(200); + EXPECT_EQ(ping_delay, + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); + + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + connection_.SendStreamDataWithString(2, "foo", 0, NO_FIN); + // Verify PING alarm gets cancelled. + EXPECT_FALSE(connection_.GetPingAlarm()->IsSet()); + + // Now receive an ACK of the packet. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(100)); + EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)); + QuicAckFrame frame = + InitAckFrame({{QuicPacketNumber(1), QuicPacketNumber(2)}}); + ProcessAckPacket(2, &frame); + // Verify PING alarm gets scheduled. + ASSERT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(ping_delay, + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); +} + // This test verifies that the connection marks path as degrading and does not // spin timer to detect path degrading when a new packet is sent on the // degraded path.
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 69b6bc3..e21ad08 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -34,6 +34,7 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_donot_reset_ideal_next_packet_send_time, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_aead_limits, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_mtu_discovery_at_server, false) +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_server_on_wire_ping, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_encrypted_control_frames, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_extract_x509_subject_using_certificate_view, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_address_validation, true)