Deprecate --gfe2_reloadable_flag_quic_ignore_gquic_probing.
Also remove code related to handling gQUIC connectivity probing (PING + PADDING) packets.
PiperOrigin-RevId: 823621293
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index 0516c7b..de45c4a 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -42,7 +42,6 @@
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_fail_on_empty_ack, true, true, "If true, closes the connection if there are no packets in the ack frame. This is always the result of a protocol violation by the peer.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_fin_before_completed_http_headers, false, true, "If true, close the connection with error if FIN is received before finish receiving the whole HTTP headers.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_fix_timeouts, true, true, "If true, postpone setting handshake timeout to infinite to handshake complete.")
-QUICHE_FLAG(bool, quiche_reloadable_flag_quic_ignore_gquic_probing, true, true, "If true, QUIC server will not respond to gQUIC probing packet(PING + PADDING) but treat it as a regular packet.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_limit_new_streams_per_loop_2, true, true, "If true, when the peer sends connection options \\\'SLP1\\\', \\\'SLP2\\\' and \\\'SLPF\\\', internet facing GFEs will only allow a limited number of new requests to be processed per event loop, and postpone the rest to the following event loops. Also guard QuicConnection to iterate through all decrypters at each encryption level to get cipher id for a request.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_neuter_packets_on_migration, false, false, "If ture, QuicSentPacketManager::OnConnectionMigration will call old_send_algorithm->OnPacketNeutered on all inflight packets.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_no_path_degrading_before_handshake_confirmed, true, true, "If true, an endpoint does not detect path degrading or blackholing until handshake gets confirmed.")
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index 4591db8..4dcca8f 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -5739,63 +5739,6 @@
::testing::ValuesIn(GetTestParams()),
::testing::PrintToStringParamName());
-TEST_P(EndToEndPacketReorderingTest, ReorderedConnectivityProbing) {
- ASSERT_TRUE(Initialize());
- if (version_.HasIetfQuicFrames() ||
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
- return;
- }
-
- // Finish one request to make sure handshake established.
- EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
-
- // Wait for the connection to become idle, to make sure the packet gets
- // delayed is the connectivity probing packet.
- client_->WaitForDelayedAcks();
-
- QuicSocketAddress old_addr =
- client_->client()->network_helper()->GetLatestClientAddress();
-
- // Migrate socket to the new IP address.
- QuicIpAddress new_host = TestLoopback(2);
- EXPECT_NE(old_addr.host(), new_host);
- ASSERT_TRUE(client_->client()->MigrateSocket(new_host));
-
- // Write a connectivity probing after the next /foo request.
- reorder_writer_->SetDelay(1);
- client_->SendConnectivityProbing();
-
- ASSERT_TRUE(client_->MigrateSocketWithSpecifiedPort(old_addr.host(),
- old_addr.port()));
-
- // The (delayed) connectivity probing will be sent after this request.
- EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
-
- // Send yet another request after the connectivity probing, when this request
- // returns, the probing is guaranteed to have been received by the server, and
- // the server's response to probing is guaranteed to have been received by the
- // client.
- EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
-
- server_thread_->Pause();
- QuicConnection* server_connection = GetServerConnection();
- if (server_connection != nullptr) {
- EXPECT_EQ(1u,
- server_connection->GetStats().num_connectivity_probing_received);
- } else {
- ADD_FAILURE() << "Missing server connection";
- }
- server_thread_->Resume();
-
- // Server definitely responded to the connectivity probing. Sometime it also
- // sends a padded ping that is not a connectivity probing, which is recognized
- // as connectivity probing because client's self address is ANY.
- QuicConnection* client_connection = GetClientConnection();
- ASSERT_TRUE(client_connection);
- EXPECT_LE(1u,
- client_connection->GetStats().num_connectivity_probing_received);
-}
-
// A writer which holds the next packet to be sent till ReleasePacket() is
// called.
class PacketHoldingWriter : public QuicPacketWriterWrapper {
diff --git a/quiche/quic/core/http/quic_spdy_session_test.cc b/quiche/quic/core/http/quic_spdy_session_test.cc
index 996daa2..8a3c3c1 100644
--- a/quiche/quic/core/http/quic_spdy_session_test.cc
+++ b/quiche/quic/core/http/quic_spdy_session_test.cc
@@ -1300,30 +1300,6 @@
session_->OnHttp3GoAway(/* id = */ 1);
}
-// Test that server session will send a connectivity probe in response to a
-// connectivity probe on the same path.
-TEST_P(QuicSpdySessionTestServer, ServerReplyToConnecitivityProbe) {
- Initialize();
- if (VersionHasIetfQuicFrames(transport_version()) ||
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
- return;
- }
- connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
- QuicSocketAddress old_peer_address =
- QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort);
- EXPECT_EQ(old_peer_address, session_->peer_address());
-
- QuicSocketAddress new_peer_address =
- QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort + 1);
-
- EXPECT_CALL(*connection_,
- SendConnectivityProbingPacket(nullptr, new_peer_address));
-
- session_->OnPacketReceived(session_->self_address(), new_peer_address,
- /*is_connectivity_probe=*/true);
- EXPECT_EQ(old_peer_address, session_->peer_address());
-}
-
TEST_P(QuicSpdySessionTestServer, IncreasedTimeoutAfterCryptoHandshake) {
Initialize();
EXPECT_EQ(kInitialIdleTimeoutSecs + 3,
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index 8f1edf6..1507ed9 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -228,7 +228,6 @@
multi_port_probing_interval_(kDefaultMultiPortProbingInterval),
connection_id_generator_(generator),
received_client_addresses_cache_(kMaxReceivedClientAddressSize),
- current_packet_content_(NO_FRAMES_RECEIVED),
perspective_(perspective),
owns_writer_(owns_writer),
can_truncate_connection_ids_(perspective == Perspective::IS_SERVER) {
@@ -1217,8 +1216,6 @@
}
// Initialize the current packet content state.
- current_packet_content_ = NO_FRAMES_RECEIVED;
- is_current_packet_connectivity_probing_ = false;
has_path_challenge_in_current_packet_ = false;
current_effective_peer_migration_type_ = NO_CHANGE;
@@ -2251,11 +2248,6 @@
return;
}
- if (IsCurrentPacketConnectivityProbing()) {
- QUICHE_DCHECK(!version().HasIetfQuicFrames() && !ignore_gquic_probing_);
- ++stats_.num_connectivity_probing_received;
- }
-
QUIC_DVLOG(1) << ENDPOINT << "Got"
<< (SupportsMultiplePacketNumberSpaces()
? (" " +
@@ -2267,14 +2259,6 @@
<< GetServerConnectionIdAsRecipient(
last_received_packet_info_.header, perspective_);
- QUIC_DLOG_IF(INFO, current_packet_content_ == SECOND_FRAME_IS_PADDING)
- << ENDPOINT << "Received a padded PING packet. is_probing: "
- << IsCurrentPacketConnectivityProbing();
-
- if (!version().HasIetfQuicFrames() && !ignore_gquic_probing_) {
- MaybeRespondToConnectivityProbingOrMigration();
- }
-
current_effective_peer_migration_type_ = NO_CHANGE;
// For IETF QUIC, it is guaranteed that TLS will give connection the
@@ -2293,32 +2277,6 @@
CloseIfTooManyOutstandingSentPackets();
}
-void QuicConnection::MaybeRespondToConnectivityProbingOrMigration() {
- 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_received_packet_info_.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;
- }
-}
-
bool QuicConnection::IsValidStatelessResetToken(
const StatelessResetToken& token) const {
QUICHE_DCHECK_EQ(perspective_, Perspective::IS_CLIENT);
@@ -2817,7 +2775,6 @@
<< "Unable to process packet. Last packet processed: "
<< last_received_packet_info_.header.packet_number;
current_packet_data_ = nullptr;
- is_current_packet_connectivity_probing_ = false;
MaybeProcessCoalescedPackets();
return;
@@ -2848,7 +2805,6 @@
SetPingAlarm();
RetirePeerIssuedConnectionIdsNoLongerOnPath();
current_packet_data_ = nullptr;
- is_current_packet_connectivity_probing_ = false;
}
void QuicConnection::OnBlockedWriterCanWrite() {
@@ -5606,10 +5562,6 @@
}
}
-bool QuicConnection::IsCurrentPacketConnectivityProbing() const {
- return is_current_packet_connectivity_probing_;
-}
-
bool QuicConnection::ack_frame_updated() const {
return uber_received_packet_manager_.IsAckFrameUpdated();
}
@@ -5720,68 +5672,6 @@
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 (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 (type == PING_FRAME) {
- if (current_packet_content_ == NO_FRAMES_RECEIVED) {
- current_packet_content_ = FIRST_FRAME_IS_PING;
- 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_N(gquic_padded_ping_received, 1, 2);
- if (perspective_ == Perspective::IS_SERVER) {
- is_current_packet_connectivity_probing_ =
- current_effective_peer_migration_type_ != NO_CHANGE;
- if (is_current_packet_connectivity_probing_) {
- QUIC_CODE_COUNT_N(gquic_padded_ping_received, 2, 2);
- }
- 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);
- }
-
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 6f00f55..689852e 100644
--- a/quiche/quic/core/quic_connection.h
+++ b/quiche/quic/core/quic_connection.h
@@ -177,15 +177,6 @@
virtual void OnSuccessfulVersionNegotiation(
const ParsedQuicVersion& version) = 0;
- // Called when a packet has been received by the connection, after being
- // validated and parsed. Only called when the client receives a valid packet
- // or the server receives a connectivity probing packet.
- // |is_connectivity_probe| is true if the received packet is a connectivity
- // probe.
- virtual void OnPacketReceived(const QuicSocketAddress& self_address,
- const QuicSocketAddress& peer_address,
- bool is_connectivity_probe) = 0;
-
// Called when a blocked socket becomes writable.
virtual void OnCanWrite() = 0;
@@ -1464,8 +1455,6 @@
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,
@@ -1590,11 +1579,6 @@
// migrated.
virtual void OnConnectionMigration();
- // Return whether the packet being processed is a connectivity probing.
- // A packet is a connectivity probing if it is a padded ping packet with self
- // and/or peer address changes.
- bool IsCurrentPacketConnectivityProbing() const;
-
// Return true iff the writer is blocked, if blocked, call
// visitor_->OnWriteBlocked() to add the connection into the write blocked
// list.
@@ -2075,14 +2059,6 @@
// Returns string which contains undecryptable packets information.
std::string UndecryptablePacketsInfo() const;
- // For Google Quic, if the current packet is connectivity probing packet, call
- // session OnPacketReceived() which eventually sends connectivity probing
- // 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) remove it when deprecating ignore_gquic_probing_.
- void MaybeRespondToConnectivityProbingOrMigration();
-
// Called in IETF QUIC. Start peer migration if a non-probing frame is
// received and the current packet number is largest received so far.
void MaybeStartIetfPeerMigration();
@@ -2426,13 +2402,6 @@
RetransmittableOnWireBehavior retransmittable_on_wire_behavior_ = DEFAULT;
- // TODO(danzh) remove `current_packet_content_` and
- // `is_current_packet_connectivity_probing_` 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_;
-
// Caches the current effective peer migration type if a effective peer
// migration might be initiated. As soon as the current packet is confirmed
// not a connectivity probe, effective peer migration will start.
@@ -2467,10 +2436,7 @@
// might be different from the next codepoint in per_packet_options_.
QuicEcnCodepoint last_ecn_codepoint_sent_ = ECN_NOT_ECT;
- // Set to true as soon as the packet currently being processed has been
- // detected as a connectivity probing.
// Always false outside the context of ProcessUdpPacket().
- bool is_current_packet_connectivity_probing_ : 1 = false;
bool has_path_challenge_in_current_packet_ : 1 = false;
bool owns_writer_ : 1;
// On the server, the connection ID is set when receiving the first packet.
@@ -2555,8 +2521,6 @@
bool multi_port_probing_on_rto_ : 1 = false;
// Client side only.
bool active_migration_disabled_ : 1 = false;
- const bool ignore_gquic_probing_ : 1 =
- GetQuicReloadableFlag(quic_ignore_gquic_probing);
// If true, kicks off validation of server_preferred_address_ once it is
// received. Also, send all coalesced packets on both paths until handshake is
// confirmed.
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 9493246..e65f82a 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -531,7 +531,6 @@
SimpleDataProducer* producer() { return &producer_; }
using QuicConnection::active_effective_peer_migration_type;
- using QuicConnection::IsCurrentPacketConnectivityProbing;
using QuicConnection::set_defer_send_in_response_to_packets;
protected:
@@ -698,7 +697,6 @@
EXPECT_CALL(visitor_, ShouldKeepConnectionAlive())
.WillRepeatedly(Return(false));
EXPECT_CALL(visitor_, OnCongestionWindowChange(_)).Times(AnyNumber());
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber());
EXPECT_CALL(visitor_, MaybeBundleOpportunistically()).Times(AnyNumber());
EXPECT_CALL(visitor_, GetFlowControlSendWindowSize(_)).Times(AnyNumber());
@@ -1203,16 +1201,12 @@
std::unique_ptr<SerializedPacket> ConstructProbingPacket() {
peer_creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
- if (VersionHasIetfQuicFrames(version().transport_version)) {
- QuicPathFrameBuffer payload = {
- {0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
- return QuicPacketCreatorPeer::
- SerializePathChallengeConnectivityProbingPacket(&peer_creator_,
- payload);
- }
- QUICHE_DCHECK(!GetQuicReloadableFlag(quic_ignore_gquic_probing));
- return QuicPacketCreatorPeer::SerializeConnectivityProbingPacket(
- &peer_creator_);
+ QUICHE_DCHECK(VersionHasIetfQuicFrames(version().transport_version));
+ QuicPathFrameBuffer payload = {
+ {0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
+ return QuicPacketCreatorPeer::
+ SerializePathChallengeConnectivityProbingPacket(&peer_creator_,
+ payload);
}
std::unique_ptr<QuicPacket> ConstructClosePacket(uint64_t number) {
@@ -2451,17 +2445,14 @@
}
TEST_P(QuicConnectionTest, ReceivePathProbeWithNoAddressChangeAtServer) {
- if (!version().HasIetfQuicFrames() &&
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
+ if (!version().HasIetfQuicFrames()) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, false)).Times(0);
- // Process a padded PING packet with no peer address change on server side
- // will be ignored. But a PATH CHALLENGE packet with no peer address change
+ // Process a PATH CHALLENGE packet with no peer address change
// will be considered as path probing.
std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
@@ -2628,30 +2619,22 @@
// 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)) {
+ if (!version().HasIetfQuicFrames()) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
QuicPathFrameBuffer payload;
- if (!GetParam().version.HasIetfQuicFrames()) {
- EXPECT_CALL(visitor_,
- OnPacketReceived(_, _, /*is_connectivity_probe=*/true))
- .Times(1);
- } else {
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(0);
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
- .Times(AtLeast(1u))
- .WillOnce([&]() {
- EXPECT_EQ(1u, writer_->path_challenge_frames().size());
- EXPECT_EQ(1u, writer_->path_response_frames().size());
- payload = writer_->path_challenge_frames().front().data_buffer;
- })
- .WillRepeatedly(DoDefault());
- }
- // Process a probing packet from a new peer address on server side
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
+ .Times(AtLeast(1u))
+ .WillOnce([&]() {
+ EXPECT_EQ(1u, writer_->path_challenge_frames().size());
+ EXPECT_EQ(1u, writer_->path_response_frames().size());
+ payload = writer_->path_challenge_frames().front().data_buffer;
+ })
+ .WillRepeatedly(DoDefault());
+ // Process a PATH_CHALLENGE from a new peer address on server side
// is effectively receiving a connectivity probing.
const QuicSocketAddress kNewPeerAddress(QuicIpAddress::Loopback4(),
/*port=*/23456);
@@ -2669,51 +2652,48 @@
connection_.GetStats().num_connectivity_probing_received);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
- if (GetParam().version.HasIetfQuicFrames()) {
- QuicByteCount bytes_sent =
- QuicConnectionPeer::BytesSentOnAlternativePath(&connection_);
- EXPECT_LT(0u, bytes_sent);
- EXPECT_EQ(received->length(),
- QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
+ QuicByteCount bytes_sent =
+ QuicConnectionPeer::BytesSentOnAlternativePath(&connection_);
+ EXPECT_LT(0u, bytes_sent);
+ EXPECT_EQ(received->length(),
+ QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
- // Receiving one more probing packet should update the bytes count.
- probing_packet = ConstructProbingPacket();
- received.reset(ConstructReceivedPacket(
- QuicEncryptedPacket(probing_packet->encrypted_buffer,
- probing_packet->encrypted_length),
- clock_.Now()));
- ProcessReceivedPacket(kSelfAddress, kNewPeerAddress, *received);
+ // Receiving one more probing packet should update the bytes count.
+ probing_packet = ConstructProbingPacket();
+ received.reset(ConstructReceivedPacket(
+ QuicEncryptedPacket(probing_packet->encrypted_buffer,
+ probing_packet->encrypted_length),
+ clock_.Now()));
+ ProcessReceivedPacket(kSelfAddress, kNewPeerAddress, *received);
- EXPECT_EQ(num_probing_received + 2,
- connection_.GetStats().num_connectivity_probing_received);
- EXPECT_EQ(2 * bytes_sent,
- QuicConnectionPeer::BytesSentOnAlternativePath(&connection_));
- EXPECT_EQ(2 * received->length(),
- QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
+ EXPECT_EQ(num_probing_received + 2,
+ connection_.GetStats().num_connectivity_probing_received);
+ EXPECT_EQ(2 * bytes_sent,
+ QuicConnectionPeer::BytesSentOnAlternativePath(&connection_));
+ EXPECT_EQ(2 * received->length(),
+ QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
- EXPECT_EQ(2 * bytes_sent,
- QuicConnectionPeer::BytesSentOnAlternativePath(&connection_));
- QuicFrames frames;
- frames.push_back(QuicFrame(QuicPathResponseFrame(99, payload)));
- ProcessFramesPacketWithAddresses(frames, connection_.self_address(),
- kNewPeerAddress,
- ENCRYPTION_FORWARD_SECURE);
- EXPECT_LT(2 * received->length(),
- QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
- EXPECT_TRUE(QuicConnectionPeer::IsAlternativePathValidated(&connection_));
- // Receiving another probing packet from a newer address with a different
- // port shouldn't trigger another reverse path validation.
- QuicSocketAddress kNewerPeerAddress(QuicIpAddress::Loopback4(),
- /*port=*/34567);
- probing_packet = ConstructProbingPacket();
- received.reset(ConstructReceivedPacket(
- QuicEncryptedPacket(probing_packet->encrypted_buffer,
- probing_packet->encrypted_length),
- clock_.Now()));
- ProcessReceivedPacket(kSelfAddress, kNewerPeerAddress, *received);
- EXPECT_FALSE(connection_.HasPendingPathValidation());
- EXPECT_TRUE(QuicConnectionPeer::IsAlternativePathValidated(&connection_));
- }
+ EXPECT_EQ(2 * bytes_sent,
+ QuicConnectionPeer::BytesSentOnAlternativePath(&connection_));
+ QuicFrames frames;
+ frames.push_back(QuicFrame(QuicPathResponseFrame(99, payload)));
+ ProcessFramesPacketWithAddresses(frames, connection_.self_address(),
+ kNewPeerAddress, ENCRYPTION_FORWARD_SECURE);
+ EXPECT_LT(2 * received->length(),
+ QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
+ EXPECT_TRUE(QuicConnectionPeer::IsAlternativePathValidated(&connection_));
+ // Receiving another probing packet from a newer address with a different
+ // port shouldn't trigger another reverse path validation.
+ QuicSocketAddress kNewerPeerAddress(QuicIpAddress::Loopback4(),
+ /*port=*/34567);
+ probing_packet = ConstructProbingPacket();
+ received.reset(ConstructReceivedPacket(
+ QuicEncryptedPacket(probing_packet->encrypted_buffer,
+ probing_packet->encrypted_length),
+ clock_.Now()));
+ ProcessReceivedPacket(kSelfAddress, kNewerPeerAddress, *received);
+ EXPECT_FALSE(connection_.HasPendingPathValidation());
+ EXPECT_TRUE(QuicConnectionPeer::IsAlternativePathValidated(&connection_));
// Process another packet with the old peer address on server side will not
// start peer migration.
@@ -2732,9 +2712,8 @@
ServerHandlePreferredAddressInit();
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(0);
- // Process a probing packet to the server preferred address.
+ // Process a PATH_CHALLENGE to the server preferred address.
std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
QuicEncryptedPacket(probing_packet->encrypted_buffer,
@@ -2816,22 +2795,11 @@
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
- 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())
- .WillRepeatedly(Return(HANDSHAKE_CONFIRMED));
- EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(0);
- } else {
- // In non-IETF version, process a padded PING packet from a new peer
- // address on server side is effectively receiving a connectivity probing.
- EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- EXPECT_CALL(visitor_,
- OnPacketReceived(_, _, /*is_connectivity_probe=*/true))
- .Times(1);
- }
+ // In IETF version, a padded PING packet with port change is not taken as
+ // connectivity probe.
+ EXPECT_CALL(visitor_, GetHandshakeState())
+ .WillRepeatedly(Return(HANDSHAKE_CONFIRMED));
+ EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
const QuicSocketAddress kNewPeerAddress =
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
@@ -2850,25 +2818,13 @@
ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress,
ENCRYPTION_INITIAL);
- 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,
- connection_.GetStats().num_connectivity_probing_received);
- EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
- EXPECT_EQ(kNewPeerAddress, connection_.effective_peer_address());
- } else {
- EXPECT_EQ(num_probing_received + 1,
- connection_.GetStats().num_connectivity_probing_received);
- EXPECT_EQ(kPeerAddress, connection_.peer_address());
- EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
- }
-
- if (GetParam().version.HasIetfQuicFrames() ||
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
- EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
- }
+ // Padded PING with port changen is not considered as connectivity probe but
+ // a PORT CHANGE.
+ EXPECT_EQ(num_probing_received,
+ connection_.GetStats().num_connectivity_probing_received);
+ EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
+ EXPECT_EQ(kNewPeerAddress, connection_.effective_peer_address());
+ EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
// Process another packet with the old peer address on server side.
ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress, kPeerAddress,
ENCRYPTION_INITIAL);
@@ -2877,8 +2833,7 @@
}
TEST_P(QuicConnectionTest, ReceiveReorderedPathProbingAtServer) {
- if (!GetParam().version.HasIetfQuicFrames() &&
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
+ if (!GetParam().version.HasIetfQuicFrames()) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
@@ -2887,15 +2842,8 @@
QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, 4);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- if (!GetParam().version.HasIetfQuicFrames()) {
- EXPECT_CALL(visitor_,
- OnPacketReceived(_, _, /*is_connectivity_probe=*/true))
- .Times(1);
- } else {
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(0);
- }
- // Process a padded PING packet from a new peer address on server side
+ // Process a PATH_CHALLENGE from a new peer address on server side
// is effectively receiving a connectivity probing, even if a newer packet has
// been received before this one.
const QuicSocketAddress kNewPeerAddress =
@@ -2911,39 +2859,19 @@
connection_.GetStats().num_connectivity_probing_received;
ProcessReceivedPacket(kSelfAddress, kNewPeerAddress, *received);
- EXPECT_EQ(num_probing_received +
- (!version().HasIetfQuicFrames() &&
- GetQuicReloadableFlag(quic_ignore_gquic_probing)
- ? 0u
- : 1u),
+ EXPECT_EQ(num_probing_received + 1u,
connection_.GetStats().num_connectivity_probing_received);
- 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());
+ EXPECT_EQ(kPeerAddress, connection_.peer_address());
+ EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
}
TEST_P(QuicConnectionTest, MigrateAfterProbingAtServer) {
- if (!GetParam().version.HasIetfQuicFrames() &&
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
+ if (!GetParam().version.HasIetfQuicFrames()) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- if (!GetParam().version.HasIetfQuicFrames()) {
- EXPECT_CALL(visitor_,
- OnPacketReceived(_, _, /*is_connectivity_probe=*/true))
- .Times(1);
- } else {
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(0);
- }
// Process a padded PING packet from a new peer address on server side
// is effectively receiving a connectivity probing.
@@ -2970,8 +2898,7 @@
}
TEST_P(QuicConnectionTest, ReceiveConnectivityProbingPacketAtClient) {
- if (!version().HasIetfQuicFrames() &&
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
+ if (!version().HasIetfQuicFrames()) {
return;
}
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
@@ -2997,43 +2924,6 @@
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
}
-TEST_P(QuicConnectionTest, ReceiveConnectivityProbingResponseAtClient) {
- if (GetParam().version.HasIetfQuicFrames() ||
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
- return;
- }
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
- PathProbeTestInit(Perspective::IS_CLIENT);
-
- // Process a padded PING packet with a different self address on client side
- // is effectively receiving a connectivity probing.
- EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- if (!GetParam().version.HasIetfQuicFrames()) {
- EXPECT_CALL(visitor_,
- OnPacketReceived(_, _, /*is_connectivity_probe=*/true))
- .Times(1);
- } else {
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(0);
- }
-
- const QuicSocketAddress kNewSelfAddress =
- QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
-
- std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
- std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
- QuicEncryptedPacket(probing_packet->encrypted_buffer,
- probing_packet->encrypted_length),
- clock_.Now()));
- uint64_t num_probing_received =
- connection_.GetStats().num_connectivity_probing_received;
- ProcessReceivedPacket(kNewSelfAddress, kPeerAddress, *received);
-
- EXPECT_EQ(num_probing_received + 1,
- connection_.GetStats().num_connectivity_probing_received);
- EXPECT_EQ(kPeerAddress, connection_.peer_address());
- EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
-}
-
TEST_P(QuicConnectionTest, PeerAddressChangeAtClient) {
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_CLIENT);
@@ -9059,8 +8949,7 @@
TEST_P(QuicConnectionTest,
RestartPathDegradingDetectionAfterMigrationWithProbe) {
- if (!version().HasIetfQuicFrames() &&
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
+ if (!version().HasIetfQuicFrames()) {
return;
}
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
@@ -9085,41 +8974,6 @@
EXPECT_TRUE(connection_.IsPathDegrading());
EXPECT_FALSE(connection_.PathDegradingDetectionInProgress());
- if (!GetParam().version.HasIetfQuicFrames()) {
- // Simulate path degrading handling by sending a probe on an alternet path.
- clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
- TestPacketWriter probing_writer(version(), &clock_, Perspective::IS_CLIENT);
- connection_.SendConnectivityProbingPacket(&probing_writer,
- connection_.peer_address());
- // Verify that path degrading detection is not reset.
- EXPECT_FALSE(connection_.PathDegradingDetectionInProgress());
-
- // Simulate successful path degrading handling by receiving probe response.
- clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(20));
-
- EXPECT_CALL(visitor_,
- OnPacketReceived(_, _, /*is_connectivity_probe=*/true))
- .Times(1);
- const QuicSocketAddress kNewSelfAddress =
- QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
-
- std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
- std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
- QuicEncryptedPacket(probing_packet->encrypted_buffer,
- probing_packet->encrypted_length),
- clock_.Now()));
- uint64_t num_probing_received =
- connection_.GetStats().num_connectivity_probing_received;
- ProcessReceivedPacket(kNewSelfAddress, kPeerAddress, *received);
-
- 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());
- EXPECT_TRUE(connection_.IsPathDegrading());
- }
-
// Verify new path degrading detection is activated.
EXPECT_CALL(visitor_, OnForwardProgressMadeAfterPathDegrading()).Times(1);
connection_.OnSuccessfulMigration(/*is_port_change*/ true);
@@ -9196,22 +9050,6 @@
EXPECT_FALSE(QuicConnectionPeer::SupportsReleaseTime(&connection_));
}
-// Regression test for b/110259444
-// Get a path response without having issued a path challenge...
-TEST_P(QuicConnectionTest, OrphanPathResponse) {
- QuicPathFrameBuffer data = {{0, 1, 2, 3, 4, 5, 6, 7}};
-
- QuicPathResponseFrame frame(99, data);
- EXPECT_TRUE(connection_.OnPathResponseFrame(frame));
- // If PATH_RESPONSE was accepted (payload matches the payload saved
- // in QuicConnection::transmitted_connectivity_probe_payload_) then
- // current_packet_content_ would be set to FIRST_FRAME_IS_PING.
- // Since this PATH_RESPONSE does not match, current_packet_content_
- // must not be FIRST_FRAME_IS_PING.
- EXPECT_NE(QuicConnection::FIRST_FRAME_IS_PING,
- QuicConnectionPeer::GetCurrentPacketContent(&connection_));
-}
-
TEST_P(QuicConnectionTest, AcceptPacketNumberZero) {
if (!VersionHasIetfQuicFrames(version().transport_version)) {
return;
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc
index 0c4c9b0..69e8610 100644
--- a/quiche/quic/core/quic_session.cc
+++ b/quiche/quic/core/quic_session.cc
@@ -647,17 +647,6 @@
void QuicSession::OnSuccessfulVersionNegotiation(
const ParsedQuicVersion& /*version*/) {}
-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.
- connection_->SendConnectivityProbingPacket(nullptr, peer_address);
- }
-}
-
void QuicSession::OnPathDegrading() {
if (visitor_) {
visitor_->OnPathDegrading();
diff --git a/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h
index 2a27f4b..0f76387 100644
--- a/quiche/quic/core/quic_session.h
+++ b/quiche/quic/core/quic_session.h
@@ -149,9 +149,6 @@
void OnWriteBlocked() override;
void OnSuccessfulVersionNegotiation(
const ParsedQuicVersion& version) override;
- void OnPacketReceived(const QuicSocketAddress& self_address,
- const QuicSocketAddress& peer_address,
- bool is_connectivity_probe) override;
void OnCanWrite() override;
void OnCongestionWindowChange(QuicTime /*now*/) override {}
void OnConnectionMigration(AddressChangeType /*type*/) override {}
diff --git a/quiche/quic/core/quic_session_test.cc b/quiche/quic/core/quic_session_test.cc
index ed37f26..42b7a17 100644
--- a/quiche/quic/core/quic_session_test.cc
+++ b/quiche/quic/core/quic_session_test.cc
@@ -1514,35 +1514,6 @@
session_.OnGoAway(go_away);
}
-// 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()) ||
- GetQuicReloadableFlag(quic_ignore_gquic_probing)) {
- return;
- }
- connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
- QuicSocketAddress old_peer_address =
- QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort);
- EXPECT_EQ(old_peer_address, session_.peer_address());
-
- QuicSocketAddress new_peer_address =
- QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort + 1);
-
- MockPacketWriter* writer = static_cast<MockPacketWriter*>(
- QuicConnectionPeer::GetWriter(session_.connection()));
- EXPECT_CALL(*writer, WritePacket(_, _, _, new_peer_address, _, _))
- .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
-
- EXPECT_CALL(*connection_, SendConnectivityProbingPacket(_, _))
- .WillOnce(
- Invoke(connection_,
- &MockQuicConnection::ReallySendConnectivityProbingPacket));
- session_.OnPacketReceived(session_.self_address(), new_peer_address,
- /*is_connectivity_probe=*/true);
- EXPECT_EQ(old_peer_address, session_.peer_address());
-}
-
TEST_P(QuicSessionTestServer, IncreasedTimeoutAfterCryptoHandshake) {
EXPECT_EQ(kInitialIdleTimeoutSecs + 3,
QuicConnectionPeer::GetNetworkTimeout(connection_).ToSeconds());
diff --git a/quiche/quic/test_tools/quic_connection_peer.cc b/quiche/quic/test_tools/quic_connection_peer.cc
index 8e51697..2c93939 100644
--- a/quiche/quic/test_tools/quic_connection_peer.cc
+++ b/quiche/quic/test_tools/quic_connection_peer.cc
@@ -291,12 +291,6 @@
}
// static
-QuicConnection::PacketContent QuicConnectionPeer::GetCurrentPacketContent(
- QuicConnection* connection) {
- return connection->current_packet_content_;
-}
-
-// static
void QuicConnectionPeer::AddBytesReceived(QuicConnection* connection,
size_t length) {
if (connection->EnforceAntiAmplificationLimit()) {
diff --git a/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h
index 64d32a6..1d04449 100644
--- a/quiche/quic/test_tools/quic_test_utils.h
+++ b/quiche/quic/test_tools/quic_test_utils.h
@@ -488,11 +488,6 @@
MOCK_METHOD(std::string, GetStreamsInfoForLogging, (), (const, override));
MOCK_METHOD(void, OnSuccessfulVersionNegotiation,
(const ParsedQuicVersion& version), (override));
- MOCK_METHOD(void, OnPacketReceived,
- (const QuicSocketAddress& self_address,
- const QuicSocketAddress& peer_address,
- bool is_connectivity_probe),
- (override));
MOCK_METHOD(void, OnAckNeedsRetransmittableFrame, (), (override));
MOCK_METHOD(void, SendAckFrequency, (const QuicAckFrequencyFrame& frame),
(override));
diff --git a/quiche/quic/test_tools/simulator/quic_endpoint.h b/quiche/quic/test_tools/simulator/quic_endpoint.h
index 210e4a8..7395848 100644
--- a/quiche/quic/test_tools/simulator/quic_endpoint.h
+++ b/quiche/quic/test_tools/simulator/quic_endpoint.h
@@ -64,9 +64,6 @@
void OnWriteBlocked() override {}
void OnSuccessfulVersionNegotiation(
const ParsedQuicVersion& /*version*/) override {}
- void OnPacketReceived(const QuicSocketAddress& /*self_address*/,
- const QuicSocketAddress& /*peer_address*/,
- bool /*is_connectivity_probe*/) override {}
void OnCongestionWindowChange(QuicTime /*now*/) override {}
void OnConnectionMigration(AddressChangeType /*type*/) override {}
void OnPathDegrading() override {}
