Internal change PiperOrigin-RevId: 540935137
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc index 52efb1a..183609a 100644 --- a/quiche/quic/core/http/end_to_end_test.cc +++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -5203,7 +5203,8 @@ TEST_P(EndToEndPacketReorderingTest, ReorderedConnectivityProbing) { ASSERT_TRUE(Initialize()); - if (version_.HasIetfQuicFrames()) { + if (version_.HasIetfQuicFrames() || + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { return; }
diff --git a/quiche/quic/core/http/quic_spdy_session_test.cc b/quiche/quic/core/http/quic_spdy_session_test.cc index 86a22c0..cf95ec7 100644 --- a/quiche/quic/core/http/quic_spdy_session_test.cc +++ b/quiche/quic/core/http/quic_spdy_session_test.cc
@@ -1216,7 +1216,8 @@ // Test that server session will send a connectivity probe in response to a // connectivity probe on the same path. TEST_P(QuicSpdySessionTestServer, ServerReplyToConnecitivityProbe) { - if (VersionHasIetfQuicFrames(transport_version())) { + if (VersionHasIetfQuicFrames(transport_version()) || + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { return; } connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); @@ -1230,12 +1231,6 @@ EXPECT_CALL(*connection_, SendConnectivityProbingPacket(nullptr, new_peer_address)); - if (VersionHasIetfQuicFrames(transport_version())) { - // Need to explicitly do this to emulate the reception of a PathChallenge, - // which stores its payload for use in generating the response. - connection_->OnPathChallengeFrame( - QuicPathChallengeFrame(0, {{0, 1, 2, 3, 4, 5, 6, 7}})); - } session_.OnPacketReceived(session_.self_address(), new_peer_address, /*is_connectivity_probe=*/true); EXPECT_EQ(old_peer_address, session_.peer_address());
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 80945f6..22174b9 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -2138,7 +2138,7 @@ } if (IsCurrentPacketConnectivityProbing()) { - QUICHE_DCHECK(!version().HasIetfQuicFrames()); + QUICHE_DCHECK(!version().HasIetfQuicFrames() && !ignore_gquic_probing_); ++stats_.num_connectivity_probing_received; } @@ -2157,7 +2157,7 @@ << ENDPOINT << "Received a padded PING packet. is_probing: " << IsCurrentPacketConnectivityProbing(); - if (!version().HasIetfQuicFrames()) { + if (!version().HasIetfQuicFrames() && !ignore_gquic_probing_) { MaybeRespondToConnectivityProbingOrMigration(); } @@ -5453,57 +5453,66 @@ last_received_packet_info_.length); return connected_; } - // Packet content is tracked to identify connectivity probe in non-IETF - // version, where a connectivity probe is defined as - // - a padded PING packet with peer address change received by server, - // - a padded PING packet on new path received by client. - if (current_packet_content_ == NOT_PADDED_PING) { - // We have already learned the current packet is not a connectivity - // probing packet. Peer migration should have already been started earlier - // if needed. - return connected_; - } + if (!ignore_gquic_probing_) { + // Packet content is tracked to identify connectivity probe in non-IETF + // version, where a connectivity probe is defined as + // - a padded PING packet with peer address change received by server, + // - a padded PING packet on new path received by client. - if (type == PING_FRAME) { - if (current_packet_content_ == NO_FRAMES_RECEIVED) { - current_packet_content_ = FIRST_FRAME_IS_PING; + if (current_packet_content_ == NOT_PADDED_PING) { + // We have already learned the current packet is not a connectivity + // probing packet. Peer migration should have already been started earlier + // if needed. return connected_; } - } - // In Google QUIC, we look for a packet with just a PING and PADDING. - // If the condition is met, mark things as connectivity-probing, causing - // later processing to generate the correct response. - if (type == PADDING_FRAME && current_packet_content_ == FIRST_FRAME_IS_PING) { - current_packet_content_ = SECOND_FRAME_IS_PADDING; - if (perspective_ == Perspective::IS_SERVER) { - is_current_packet_connectivity_probing_ = - current_effective_peer_migration_type_ != NO_CHANGE; - QUIC_DLOG_IF(INFO, is_current_packet_connectivity_probing_) - << ENDPOINT - << "Detected connectivity probing packet. " - "current_effective_peer_migration_type_:" - << current_effective_peer_migration_type_; - } else { - is_current_packet_connectivity_probing_ = - (last_received_packet_info_.source_address != peer_address()) || - (last_received_packet_info_.destination_address != - default_path_.self_address); - QUIC_DLOG_IF(INFO, is_current_packet_connectivity_probing_) - << ENDPOINT - << "Detected connectivity probing packet. " - "last_packet_source_address:" - << last_received_packet_info_.source_address - << ", peer_address_:" << peer_address() - << ", last_packet_destination_address:" - << last_received_packet_info_.destination_address - << ", default path self_address :" << default_path_.self_address; + if (type == PING_FRAME) { + if (current_packet_content_ == NO_FRAMES_RECEIVED) { + current_packet_content_ = FIRST_FRAME_IS_PING; + return connected_; + } } - return connected_; + + // In Google QUIC, we look for a packet with just a PING and PADDING. + // If the condition is met, mark things as connectivity-probing, causing + // later processing to generate the correct response. + if (type == PADDING_FRAME && + current_packet_content_ == FIRST_FRAME_IS_PING) { + current_packet_content_ = SECOND_FRAME_IS_PADDING; + QUIC_CODE_COUNT(gquic_padded_ping_received); + if (perspective_ == Perspective::IS_SERVER) { + is_current_packet_connectivity_probing_ = + current_effective_peer_migration_type_ != NO_CHANGE; + QUIC_DLOG_IF(INFO, is_current_packet_connectivity_probing_) + << ENDPOINT + << "Detected connectivity probing packet. " + "current_effective_peer_migration_type_:" + << current_effective_peer_migration_type_; + } else { + is_current_packet_connectivity_probing_ = + (last_received_packet_info_.source_address != peer_address()) || + (last_received_packet_info_.destination_address != + default_path_.self_address); + QUIC_DLOG_IF(INFO, is_current_packet_connectivity_probing_) + << ENDPOINT + << "Detected connectivity probing packet. " + "last_packet_source_address:" + << last_received_packet_info_.source_address + << ", peer_address_:" << peer_address() + << ", last_packet_destination_address:" + << last_received_packet_info_.destination_address + << ", default path self_address :" << default_path_.self_address; + } + return connected_; + } + + current_packet_content_ = NOT_PADDED_PING; + } else { + QUIC_RELOADABLE_FLAG_COUNT(quic_ignore_gquic_probing); + QUICHE_DCHECK_EQ(current_packet_content_, NO_FRAMES_RECEIVED); } - current_packet_content_ = NOT_PADDED_PING; if (GetLargestReceivedPacket().IsInitialized() && last_received_packet_info_.header.packet_number == GetLargestReceivedPacket()) {
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 90e5018..9004fe0 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1318,6 +1318,8 @@ return peer_issued_cid_manager_ != nullptr; } + bool ignore_gquic_probing() const { return ignore_gquic_probing_; } + // Sets the ECN marking for all outgoing packets, assuming that the congestion // control supports that codepoint. QuicConnection will revert to sending // ECN_NOT_ECT if there is evidence the path is dropping ECN-marked packets, @@ -1903,8 +1905,7 @@ // response on server side. And no-op on client side. And for both Google Quic // and IETF Quic, start migration if the current packet is a non-probing // packet. - // TODO(danzh) rename to MaybeRespondToPeerMigration() when Google Quic is - // deprecated. + // TODO(danzh) remove it when deprecating ignore_gquic_probing_. void MaybeRespondToConnectivityProbingOrMigration(); // Called in IETF QUIC. Start peer migration if a non-probing frame is @@ -2002,8 +2003,9 @@ QuicFramer framer_; - // Contents received in the current packet, especially used to identify - // whether the current packet is a padded PING packet. + // TODO(danzh) remove below fields once quic_ignore_gquic_probing_ gets + // deprecated. Contents received in the current packet, especially used to + // identify whether the current packet is a padded PING packet. PacketContent current_packet_content_; // Set to true as soon as the packet currently being processed has been // detected as a connectivity probing. @@ -2353,8 +2355,12 @@ std::unique_ptr<MultiPortStats> multi_port_stats_; + // Client side only. bool active_migration_disabled_ = false; + const bool ignore_gquic_probing_ = + GetQuicReloadableFlag(quic_ignore_gquic_probing); + RetransmittableOnWireBehavior retransmittable_on_wire_behavior_ = DEFAULT; // Server addresses that are known to the client.
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index bdfd034..12cea2f 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -1196,6 +1196,7 @@ SerializePathChallengeConnectivityProbingPacket(&peer_creator_, payload); } + QUICHE_DCHECK(!GetQuicReloadableFlag(quic_ignore_gquic_probing)); return QuicPacketCreatorPeer::SerializeConnectivityProbingPacket( &peer_creator_); } @@ -1620,6 +1621,7 @@ ProcessFramePacketWithAddresses(MakeCryptoFrame(), self_address, kPeerAddress, ENCRYPTION_INITIAL); EXPECT_TRUE(connection_.connected()); + EXPECT_NE(connection_.self_address(), self_address); } TEST_P(QuicConnectionTest, SelfAddressChangeAtServer) { @@ -2401,6 +2403,10 @@ } TEST_P(QuicConnectionTest, ReceivePathProbeWithNoAddressChangeAtServer) { + if (!version().HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { + return; + } PathProbeTestInit(Perspective::IS_SERVER); EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0); @@ -2569,11 +2575,13 @@ QuicConnection* connection_; }; -// Receive a path probe request at the server side, i.e., -// in non-IETF version: receive a padded PING packet with a peer addess change; -// in IETF version: receive a packet contains PATH CHALLENGE with peer address -// change. +// Receive a path probe request at the server side, in IETF version: receive a +// packet contains PATH CHALLENGE with peer address change. TEST_P(QuicConnectionTest, ReceivePathProbingFromNewPeerAddressAtServer) { + if (!version().HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { + return; + } PathProbeTestInit(Perspective::IS_SERVER); EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0); @@ -2757,7 +2765,8 @@ EXPECT_EQ(kPeerAddress, connection_.peer_address()); EXPECT_EQ(kPeerAddress, connection_.effective_peer_address()); - if (GetParam().version.HasIetfQuicFrames()) { + if (GetParam().version.HasIetfQuicFrames() || + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { // In IETF version, a padded PING packet with port change is not taken as // connectivity probe. EXPECT_CALL(visitor_, GetHandshakeState()) @@ -2790,7 +2799,8 @@ ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress, ENCRYPTION_INITIAL); - if (GetParam().version.HasIetfQuicFrames()) { + if (GetParam().version.HasIetfQuicFrames() || + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { // Padded PING with port changen is not considered as connectivity probe but // a PORT CHANGE. EXPECT_EQ(num_probing_received, @@ -2804,11 +2814,11 @@ EXPECT_EQ(kPeerAddress, connection_.effective_peer_address()); } - if (GetParam().version.HasIetfQuicFrames()) { + if (GetParam().version.HasIetfQuicFrames() || + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1); } - // Process another packet with the old peer address on server side. gQUIC - // shouldn't regard this as a peer migration. + // Process another packet with the old peer address on server side. ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress, kPeerAddress, ENCRYPTION_INITIAL); EXPECT_EQ(kPeerAddress, connection_.peer_address()); @@ -2816,6 +2826,10 @@ } TEST_P(QuicConnectionTest, ReceiveReorderedPathProbingAtServer) { + if (!GetParam().version.HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { + return; + } PathProbeTestInit(Perspective::IS_SERVER); // Decrease packet number to simulate out-of-order packets. @@ -2846,13 +2860,29 @@ connection_.GetStats().num_connectivity_probing_received; ProcessReceivedPacket(kSelfAddress, kNewPeerAddress, *received); - EXPECT_EQ(num_probing_received + 1, + EXPECT_EQ(num_probing_received + + (!version().HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing) + ? 0u + : 1u), connection_.GetStats().num_connectivity_probing_received); - EXPECT_EQ(kPeerAddress, connection_.peer_address()); - EXPECT_EQ(kPeerAddress, connection_.effective_peer_address()); + EXPECT_EQ((!version().HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing) + ? kNewPeerAddress + : kPeerAddress), + connection_.peer_address()); + EXPECT_EQ((!version().HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing) + ? kNewPeerAddress + : kPeerAddress), + connection_.effective_peer_address()); } TEST_P(QuicConnectionTest, MigrateAfterProbingAtServer) { + if (!GetParam().version.HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { + return; + } PathProbeTestInit(Perspective::IS_SERVER); EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0); @@ -2889,6 +2919,10 @@ } TEST_P(QuicConnectionTest, ReceiveConnectivityProbingPacketAtClient) { + if (!version().HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { + return; + } EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); PathProbeTestInit(Perspective::IS_CLIENT); @@ -2913,10 +2947,8 @@ } TEST_P(QuicConnectionTest, ReceiveConnectivityProbingResponseAtClient) { - // TODO(b/150095484): add test coverage for IETF to verify that client takes - // PATH RESPONSE with peer address change as correct validation on the new - // path. - if (GetParam().version.HasIetfQuicFrames()) { + if (GetParam().version.HasIetfQuicFrames() || + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { return; } EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); @@ -8696,6 +8728,10 @@ TEST_P(QuicConnectionTest, RestartPathDegradingDetectionAfterMigrationWithProbe) { + if (!version().HasIetfQuicFrames() && + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { + return; + } EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); PathProbeTestInit(Perspective::IS_CLIENT); @@ -8745,7 +8781,8 @@ connection_.GetStats().num_connectivity_probing_received; ProcessReceivedPacket(kNewSelfAddress, kPeerAddress, *received); - EXPECT_EQ(num_probing_received + 1, + EXPECT_EQ(num_probing_received + + (GetQuicReloadableFlag(quic_ignore_gquic_probing) ? 0u : 1u), connection_.GetStats().num_connectivity_probing_received); EXPECT_EQ(kPeerAddress, connection_.peer_address()); EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index f9ffb39..d28cb38 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -17,6 +17,8 @@ QUIC_FLAG(quic_restart_flag_quic_testonly_default_true, true) // If trrue, early return before write control frame in OnCanWrite() if the connection is already closed. QUIC_FLAG(quic_reloadable_flag_quic_no_write_control_frame_upon_connection_close, true) +// If true, QUIC server will not respond to gQUIC probing packet(PING + PADDING) but treat it as a regular packet. +QUIC_FLAG(quic_reloadable_flag_quic_ignore_gquic_probing, false) // If true, QUIC will default enable MTU discovery at server, with a target of 1450 bytes. QUIC_FLAG(quic_reloadable_flag_quic_enable_mtu_discovery_at_server, false) // If true, QuicGsoBatchWriter will support release time if it is available and the process has the permission to do so.
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc index 0720fa1..e140bb4 100644 --- a/quiche/quic/core/quic_session.cc +++ b/quiche/quic/core/quic_session.cc
@@ -488,6 +488,7 @@ void QuicSession::OnPacketReceived(const QuicSocketAddress& /*self_address*/, const QuicSocketAddress& peer_address, bool is_connectivity_probe) { + QUICHE_DCHECK(!connection_->ignore_gquic_probing()); if (is_connectivity_probe && perspective() == Perspective::IS_SERVER) { // Server only sends back a connectivity probe after received a // connectivity probe from a new peer address.
diff --git a/quiche/quic/core/quic_session_test.cc b/quiche/quic/core/quic_session_test.cc index fd3ea97..3b90633 100644 --- a/quiche/quic/core/quic_session_test.cc +++ b/quiche/quic/core/quic_session_test.cc
@@ -1458,7 +1458,8 @@ // Test that server session will send a connectivity probe in response to a // connectivity probe on the same path. TEST_P(QuicSessionTestServer, ServerReplyToConnectivityProbe) { - if (VersionHasIetfQuicFrames(transport_version())) { + if (VersionHasIetfQuicFrames(transport_version()) || + GetQuicReloadableFlag(quic_ignore_gquic_probing)) { return; } connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);