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);
