Deprecate flag --gfe2_reloadable_flag_quic_send_path_response2
PiperOrigin-RevId: 430477270
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 4550621..67f2e44 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1345,8 +1345,7 @@
// 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()) &&
- connection_->send_path_response()) {
+ if (VersionHasIetfQuicFrames(transport_version())) {
return;
}
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
@@ -1357,13 +1356,8 @@
QuicSocketAddress new_peer_address =
QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort + 1);
- if (connection_->send_path_response()) {
EXPECT_CALL(*connection_,
SendConnectivityProbingPacket(nullptr, new_peer_address));
- } else {
- EXPECT_CALL(*connection_,
- SendConnectivityProbingResponsePacket(new_peer_address));
- }
if (VersionHasIetfQuicFrames(transport_version())) {
// Need to explicitly do this to emulate the reception of a PathChallenge,
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 09fb156..9db7965 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -1671,8 +1671,6 @@
"is closed. Last frame: "
<< most_recent_frame_type_;
if (has_path_challenge_in_current_packet_) {
- QUICHE_DCHECK(send_path_response_);
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_send_path_response2, 2, 5);
// Only respond to the 1st PATH_CHALLENGE in the packet.
return true;
}
@@ -1726,15 +1724,6 @@
std::make_unique<ReversePathValidationResultDelegate>(
this, peer_address()));
}
- if (!send_path_response_) {
- // Save the path challenge's payload, for later use in generating the
- // response.
- received_path_challenge_payloads_.push_back(frame.data_buffer);
-
- MaybeUpdateAckTimeout();
- return true;
- }
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_send_path_response2, 3, 5);
has_path_challenge_in_current_packet_ = true;
MaybeUpdateAckTimeout();
// Queue or send PATH_RESPONSE. Send PATH_RESPONSE to the source address of
@@ -2201,7 +2190,9 @@
<< ENDPOINT << "Received a padded PING packet. is_probing: "
<< IsCurrentPacketConnectivityProbing();
- MaybeRespondToConnectivityProbingOrMigration();
+ if (!version().HasIetfQuicFrames()) {
+ MaybeRespondToConnectivityProbingOrMigration();
+ }
current_effective_peer_migration_type_ = NO_CHANGE;
@@ -2220,55 +2211,28 @@
}
void QuicConnection::MaybeRespondToConnectivityProbingOrMigration() {
- if (version().HasIetfQuicFrames()) {
- if (send_path_response_) {
- return;
- }
- if (perspective_ == Perspective::IS_CLIENT) {
- // This node is a client, notify that a speculative connectivity probing
- // packet has been received anyway.
- visitor_->OnPacketReceived(last_received_packet_info_.destination_address,
- last_received_packet_info_.source_address,
- /*is_connectivity_probe=*/false);
- return;
- }
- if (!received_path_challenge_payloads_.empty()) {
- if (current_effective_peer_migration_type_ != NO_CHANGE) {
- // TODO(b/150095588): change the stats to
- // num_valid_path_challenge_received.
- ++stats_.num_connectivity_probing_received;
- }
- // If the packet contains PATH CHALLENGE, send appropriate RESPONSE.
- // There was at least one PATH CHALLENGE in the received packet,
- // Generate the required PATH RESPONSE.
- SendGenericPathProbePacket(nullptr,
- last_received_packet_info_.source_address,
- /* is_response=*/true);
- return;
- }
- } else {
- if (IsCurrentPacketConnectivityProbing()) {
- visitor_->OnPacketReceived(last_received_packet_info_.destination_address,
- last_received_packet_info_.source_address,
- /*is_connectivity_probe=*/true);
- return;
- }
- if (perspective_ == Perspective::IS_CLIENT) {
- // This node is a client, notify that a speculative connectivity probing
- // packet has been received anyway.
- QUIC_DVLOG(1)
- << ENDPOINT
- << "Received a speculative connectivity probing packet for "
- << GetServerConnectionIdAsRecipient(last_header_, perspective_)
- << " from ip:port: "
- << last_received_packet_info_.source_address.ToString()
- << " to ip:port: "
- << last_received_packet_info_.destination_address.ToString();
- visitor_->OnPacketReceived(last_received_packet_info_.destination_address,
- last_received_packet_info_.source_address,
- /*is_connectivity_probe=*/false);
- return;
- }
+ QUICHE_DCHECK(!version().HasIetfQuicFrames());
+ if (IsCurrentPacketConnectivityProbing()) {
+ visitor_->OnPacketReceived(last_received_packet_info_.destination_address,
+ last_received_packet_info_.source_address,
+ /*is_connectivity_probe=*/true);
+ return;
+ }
+ if (perspective_ == Perspective::IS_CLIENT) {
+ // This node is a client, notify that a speculative connectivity probing
+ // packet has been received anyway.
+ QUIC_DVLOG(1) << ENDPOINT
+ << "Received a speculative connectivity probing packet for "
+ << GetServerConnectionIdAsRecipient(last_header_,
+ perspective_)
+ << " from ip:port: "
+ << last_received_packet_info_.source_address.ToString()
+ << " to ip:port: "
+ << last_received_packet_info_.destination_address.ToString();
+ visitor_->OnPacketReceived(last_received_packet_info_.destination_address,
+ last_received_packet_info_.source_address,
+ /*is_connectivity_probe=*/false);
+ return;
}
}
@@ -3434,8 +3398,7 @@
// during the WritePacket below.
QuicTime packet_send_time = CalculatePacketSentTime();
WriteResult result(WRITE_STATUS_OK, encrypted_length);
- QuicSocketAddress send_to_address =
- (send_path_response_) ? packet->peer_address : peer_address();
+ QuicSocketAddress send_to_address = packet->peer_address;
// Self address is always the default self address on this code path.
bool send_on_current_path = send_to_address == peer_address();
switch (fate) {
@@ -5027,20 +4990,6 @@
bool QuicConnection::SendConnectivityProbingPacket(
QuicPacketWriter* probing_writer,
const QuicSocketAddress& peer_address) {
- return SendGenericPathProbePacket(probing_writer, peer_address,
- /* is_response= */ false);
-}
-
-void QuicConnection::SendConnectivityProbingResponsePacket(
- const QuicSocketAddress& peer_address) {
- SendGenericPathProbePacket(nullptr, peer_address,
- /* is_response= */ true);
-}
-
-bool QuicConnection::SendGenericPathProbePacket(
- QuicPacketWriter* probing_writer,
- const QuicSocketAddress& peer_address,
- bool is_response) {
QUICHE_DCHECK(peer_address.IsInitialized());
if (!connected_) {
QUIC_BUG(quic_bug_10511_31)
@@ -5075,15 +5024,6 @@
// Non-IETF QUIC, generate a padded ping regardless of whether this is a
// request or a response.
probing_packet = packet_creator_.SerializeConnectivityProbingPacket();
- } else if (is_response) {
- QUICHE_DCHECK(!send_path_response_);
- // IETF QUIC path response.
- // Respond to path probe request using IETF QUIC PATH_RESPONSE frame.
- probing_packet =
- packet_creator_.SerializePathResponseConnectivityProbingPacket(
- received_path_challenge_payloads_,
- /*is_padded=*/false);
- received_path_challenge_payloads_.clear();
} else {
// IETF QUIC path challenge.
// Send a path probe request using IETF QUIC PATH_CHALLENGE frame.
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index ba71d31..129d8bf 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -966,14 +966,6 @@
QuicPacketWriter* probing_writer,
const QuicSocketAddress& peer_address);
- // Sends response to a connectivity probe. Sends either a Padded Ping
- // or an IETF PATH_RESPONSE based on the version of the connection.
- // Is the counterpart to SendConnectivityProbingPacket().
- // TODO(danzh): remove this method after deprecating
- // --gfe2_reloadable_flag_quic_send_path_response.
- virtual void SendConnectivityProbingResponsePacket(
- const QuicSocketAddress& peer_address);
-
// Disable MTU discovery on this connection.
void DisableMtuDiscovery();
@@ -1160,8 +1152,6 @@
// Can only be set if this is a client connection.
void EnableLegacyVersionEncapsulation(const std::string& server_name);
- bool send_path_response() const { return send_path_response_; }
-
bool use_path_validator() const { return use_path_validator_; }
// If now is close to idle timeout, returns true and sends a connectivity
@@ -1679,11 +1669,8 @@
// Sends generic path probe packet to the peer. If we are not IETF QUIC, will
// always send a padded ping, regardless of whether this is a request or not.
- // TODO(danzh): remove |is_response| after deprecating
- // --gfe2_reloadable_flag_quic_send_path_response.
bool SendGenericPathProbePacket(QuicPacketWriter* probing_writer,
- const QuicSocketAddress& peer_address,
- bool is_response);
+ const QuicSocketAddress& peer_address);
// Called when an ACK is about to send. Resets ACK related internal states,
// e.g., cancels ack_alarm_, resets
@@ -2221,11 +2208,7 @@
size_t anti_amplification_factor_ =
GetQuicFlag(FLAGS_quic_anti_amplification_factor);
- // latch --gfe2_reloadable_flag_quic_send_path_response.
- bool send_path_response_ = GetQuicReloadableFlag(quic_send_path_response2);
-
bool use_path_validator_ =
- send_path_response_ &&
GetQuicReloadableFlag(quic_pass_path_response_to_validator);
// True if AckFrequencyFrame is supported.
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 8727d83..5845769 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -2323,11 +2323,9 @@
connection_.GetStats().num_connectivity_probing_received;
ProcessReceivedPacket(kSelfAddress, kPeerAddress, *received);
- EXPECT_EQ(num_probing_received + (GetParam().version.HasIetfQuicFrames() &&
- connection_.send_path_response()
- ? 1u
- : 0u),
- connection_.GetStats().num_connectivity_probing_received);
+ EXPECT_EQ(
+ num_probing_received + (GetParam().version.HasIetfQuicFrames() ? 1u : 0u),
+ connection_.GetStats().num_connectivity_probing_received);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
}
@@ -2730,9 +2728,6 @@
// Client takes all padded PING packet as speculative connectivity
// probing packet, and reports to visitor.
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- if (!connection_.send_path_response()) {
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, false)).Times(1);
- }
std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
@@ -2743,11 +2738,9 @@
connection_.GetStats().num_connectivity_probing_received;
ProcessReceivedPacket(kSelfAddress, kPeerAddress, *received);
- EXPECT_EQ(num_probing_received + (GetParam().version.HasIetfQuicFrames() &&
- connection_.send_path_response()
- ? 1u
- : 0u),
- connection_.GetStats().num_connectivity_probing_received);
+ EXPECT_EQ(
+ num_probing_received + (GetParam().version.HasIetfQuicFrames() ? 1u : 0u),
+ connection_.GetStats().num_connectivity_probing_received);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
}
@@ -6894,8 +6887,7 @@
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(1), _, _))
.Times(1);
- if (connection_.send_path_response() &&
- VersionHasIetfQuicFrames(GetParam().version.transport_version)) {
+ if (VersionHasIetfQuicFrames(GetParam().version.transport_version)) {
QuicPathFrameBuffer payload{
{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
QuicConnection::ScopedPacketFlusher flusher(&connection_);
@@ -8861,10 +8853,6 @@
EXPECT_TRUE(connection_.OnPathChallengeFrame(
writer_->path_challenge_frames().front()));
EXPECT_TRUE(connection_.OnPaddingFrame(writer_->padding_frames().front()));
- if (!connection_.send_path_response()) {
- connection_.SendConnectivityProbingResponsePacket(
- connection_.peer_address());
- }
creator_->FlushCurrentPacket();
// The final check is to ensure that the random data in the response matches
@@ -8876,8 +8864,7 @@
}
TEST_P(QuicConnectionTest, ClientResponseToPathChallengeOnDefaulSocket) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
- !connection_.send_path_response()) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -12296,22 +12283,14 @@
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
.Times(2)
.WillOnce(Invoke([=]() {
- EXPECT_EQ((connection_.send_path_response() ? 1u : 2u),
- writer_->path_response_frames().size());
+ EXPECT_EQ(1u, writer_->path_response_frames().size());
// The final check is to ensure that the random data in the response
// matches the random data from the challenge.
EXPECT_EQ(0,
memcmp(path_frame_buffer1.data(),
&(writer_->path_response_frames().front().data_buffer),
sizeof(path_frame_buffer1)));
- if (!connection_.send_path_response()) {
- EXPECT_EQ(
- 0, memcmp(path_frame_buffer2.data(),
- &(writer_->path_response_frames().back().data_buffer),
- sizeof(path_frame_buffer2)));
- } else {
- EXPECT_EQ(1u, writer_->padding_frames().size());
- }
+ EXPECT_EQ(1u, writer_->padding_frames().size());
EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
}))
.WillOnce(Invoke([=]() {
@@ -12324,8 +12303,7 @@
}
TEST_P(QuicConnectionTest, ReceiveStreamFrameBeforePathChallenge) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
- !connection_.send_path_response()) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
@@ -12378,8 +12356,7 @@
}
TEST_P(QuicConnectionTest, ReceiveStreamFrameFollowingPathChallenge) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
- !connection_.send_path_response()) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
@@ -12443,8 +12420,7 @@
// Tests that a PATH_CHALLENGE is received in between other frames in an out of
// order packet.
TEST_P(QuicConnectionTest, PathChallengeWithDataInOutOfOrderPacket) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
- !connection_.send_path_response()) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
@@ -12510,8 +12486,7 @@
// Tests that a PATH_CHALLENGE is cached if its PATH_RESPONSE can't be sent.
TEST_P(QuicConnectionTest, FailToWritePathResponse) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
- !connection_.send_path_response()) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 5244028..c5c31c5 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -91,8 +91,6 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_require_handshake_confirmation, false)
// If true, reset per packet state before processing undecryptable packets.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_reset_per_packet_state_for_undecryptable_packets, true)
-// If true, send PATH_RESPONSE upon receiving PATH_CHALLENGE regardless of perspective. --gfe2_reloadable_flag_quic_start_peer_migration_earlier has to be true before turn on this flag.
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_path_response2, true)
// If true, server proactively retires client issued connection ID on reverse path validation failure.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_retire_cid_on_reverse_path_validation_failure, false)
// If true, set burst token to 2 in cwnd bootstrapping experiment.
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 388aada..5042d66 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -2220,7 +2220,6 @@
}
QUIC_DVLOG(1) << ENDPOINT << "Can't send PATH_RESPONSE now";
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_send_path_response2, 5, 5);
delete path_response;
return false;
}
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index faec9d5..fe2955b 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -491,14 +491,7 @@
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.
- if (connection_->send_path_response()) {
- // SendConnectivityProbingResponsePacket() will be deprecated.
- // SendConnectivityProbingPacket() will be used to send both probing
- // request and response as both of them are padded PING.
connection_->SendConnectivityProbingPacket(nullptr, peer_address);
- } else {
- connection_->SendConnectivityProbingResponsePacket(peer_address);
- }
}
}
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index 256865d..b62f2a9 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -1411,8 +1411,7 @@
// Test that server session will send a connectivity probe in response to a
// connectivity probe on the same path.
TEST_P(QuicSessionTestServer, ServerReplyToConnectivityProbe) {
- if (connection_->send_path_response() &&
- VersionHasIetfQuicFrames(transport_version())) {
+ if (VersionHasIetfQuicFrames(transport_version())) {
return;
}
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
@@ -1427,64 +1426,16 @@
QuicConnectionPeer::GetWriter(session_.connection()));
EXPECT_CALL(*writer, WritePacket(_, _, _, new_peer_address, _))
.WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- if (connection_->send_path_response()) {
+
EXPECT_CALL(*connection_, SendConnectivityProbingPacket(_, _))
.WillOnce(
Invoke(connection_,
&MockQuicConnection::ReallySendConnectivityProbingPacket));
- } else {
- EXPECT_CALL(*connection_, SendConnectivityProbingResponsePacket(_))
- .WillOnce(Invoke(
- connection_,
- &MockQuicConnection::ReallySendConnectivityProbingResponsePacket));
- }
- 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, path_frame_buffer1_));
- }
session_.OnPacketReceived(session_.self_address(), new_peer_address,
/*is_connectivity_probe=*/true);
EXPECT_EQ(old_peer_address, session_.peer_address());
}
-// Same as above, but check that if there are two PATH_CHALLENGE frames in the
-// packet, the response has both of them AND we do not do migration. This for
-// IETF QUIC only.
-TEST_P(QuicSessionTestServer, ServerReplyToConnectivityProbes) {
- if (connection_->send_path_response() ||
- !VersionHasIetfQuicFrames(transport_version())) {
- return;
- }
- connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
- QuicSocketAddress old_peer_address =
- QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort);
- EXPECT_EQ(old_peer_address, session_.peer_address());
-
- MockPacketWriter* writer = static_cast<MockPacketWriter*>(
- QuicConnectionPeer::GetWriter(session_.connection()));
- // CheckMultiPathResponse validates that the written packet
- // contains both path responses.
- EXPECT_CALL(*writer, WritePacket(_, _, _, old_peer_address, _))
- .WillOnce(Invoke(this, &QuicSessionTestServer::CheckMultiPathResponse));
-
- EXPECT_CALL(*connection_, SendConnectivityProbingResponsePacket(_))
- .WillOnce(Invoke(
- connection_,
- &MockQuicConnection::ReallySendConnectivityProbingResponsePacket));
- QuicConnectionPeer::SetLastHeaderFormat(connection_,
- IETF_QUIC_SHORT_HEADER_PACKET);
- // 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, path_frame_buffer1_));
- connection_->OnPathChallengeFrame(
- QuicPathChallengeFrame(0, path_frame_buffer2_));
- session_.OnPacketReceived(session_.self_address(), old_peer_address,
- /*is_connectivity_probe=*/true);
-}
-
TEST_P(QuicSessionTestServer, IncreasedTimeoutAfterCryptoHandshake) {
EXPECT_EQ(kInitialIdleTimeoutSecs + 3,
QuicConnectionPeer::GetNetworkTimeout(connection_).ToSeconds());
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index d8d0979..8d12d16 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -631,8 +631,6 @@
const std::string& details),
(override));
MOCK_METHOD(void, OnCanWrite, (), (override));
- MOCK_METHOD(void, SendConnectivityProbingResponsePacket,
- (const QuicSocketAddress& peer_address), (override));
MOCK_METHOD(bool, SendConnectivityProbingPacket,
(QuicPacketWriter*, const QuicSocketAddress& peer_address),
(override));
@@ -703,11 +701,6 @@
peer_address);
}
- void ReallySendConnectivityProbingResponsePacket(
- const QuicSocketAddress& peer_address) {
- QuicConnection::SendConnectivityProbingResponsePacket(peer_address);
- }
-
bool ReallyOnPathResponseFrame(const QuicPathResponseFrame& frame) {
return QuicConnection::OnPathResponseFrame(frame);
}
