Add path validation failure reason codes to QUICHE. Enables analysis of migration failures. See https://chromium-review.googlesource.com/c/chromium/src/+/7920112 for corresponding changes in Cronet. PiperOrigin-RevId: 942562673
diff --git a/quiche/quic/core/http/quic_connection_migration_manager.cc b/quiche/quic/core/http/quic_connection_migration_manager.cc index ea1a7ac..41d15af 100644 --- a/quiche/quic/core/http/quic_connection_migration_manager.cc +++ b/quiche/quic/core/http/quic_connection_migration_manager.cc
@@ -306,7 +306,8 @@ QuicPathValidationContext* context = connection_->GetPathValidationContext(); if (context && context->network() == disconnected_network && context->peer_address() == connection_->peer_address()) { - connection_->CancelPathValidation(); + connection_->CancelPathValidation( + PathValidationFailure{PathValidationFailure::Reason::kNotConnected}); } if (disconnected_network == default_network_) { @@ -396,7 +397,8 @@ QuicPathValidationContext* context = connection_->GetPathValidationContext(); if (context && context->network() == network && context->peer_address() == connection_->peer_address()) { - connection_->CancelPathValidation(); + connection_->CancelPathValidation( + PathValidationFailure{PathValidationFailure::Reason::kNewerValidation}); } pending_migrate_network_immediately_ = true; Migrate(network, connection_->peer_address(), @@ -1002,7 +1004,8 @@ } if (context->network() == network && context->peer_address() == connection_->peer_address()) { - connection_->CancelPathValidation(); + connection_->CancelPathValidation( + PathValidationFailure{PathValidationFailure::Reason::kUnknown}); } if (network != kInvalidNetworkHandle) { // Probing failure can be ignored.
diff --git a/quiche/quic/core/http/quic_connection_migration_manager_test.cc b/quiche/quic/core/http/quic_connection_migration_manager_test.cc index b4fe7d3..224b8a6 100644 --- a/quiche/quic/core/http/quic_connection_migration_manager_test.cc +++ b/quiche/quic/core/http/quic_connection_migration_manager_test.cc
@@ -10,6 +10,7 @@ #include "quiche/quic/core/http/quic_spdy_client_session_with_migration.h" #include "quiche/quic/core/io/socket.h" #include "quiche/quic/core/quic_force_blockable_packet_writer.h" +#include "quiche/quic/core/quic_path_validator.h" #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/test_tools/quic_config_peer.h" #include "quiche/quic/test_tools/quic_connection_peer.h" @@ -967,7 +968,8 @@ EXPECT_TRUE(migrate_back_alarm->IsSet()); // Fail the current path validation. auto* path_validator = QuicConnectionPeer::path_validator(connection_); - path_validator->CancelPathValidation(); + path_validator->CancelPathValidation( + PathValidationFailure::Reason::kUnknown); // Following attempt should be scheduled with expotential delay. QuicTimeDelta next_delay = QuicTimeDelta::FromSeconds(UINT64_C(1) << i); EXPECT_EQ(migrate_back_alarm->deadline(), @@ -1040,7 +1042,8 @@ // Fail the current path validation. auto* path_validator = QuicConnectionPeer::path_validator(connection_); - path_validator->CancelPathValidation(); + path_validator->CancelPathValidation( + PathValidationFailure::Reason::kUnknown); EXPECT_TRUE(migrate_back_alarm->IsSet()); QuicTimeDelta next_delay = QuicTimeDelta::FromSeconds(UINT64_C(1) << i);
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 1aacd74..a1f2592 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -2426,7 +2426,8 @@ QUIC_BUG_IF(quic_bug_12714_18, alternative_path_.validated) << "STATELESS_RESET received on alternate path after it's " "validated."; - path_validator_.CancelPathValidation(); + path_validator_.CancelPathValidation( + PathValidationFailure::Reason::kStatelessReset); ++stats_.num_stateless_resets_on_alternate_path; } else { QUIC_BUG(quic_bug_10511_17) @@ -4945,7 +4946,8 @@ // Cancel the alarms so they don't trigger any action now that the // connection is closed. CancelAllAlarms(); - CancelPathValidation(); + CancelPathValidation( + PathValidationFailure{PathValidationFailure::Reason::kNotConnected}); peer_issued_cid_manager_.reset(); self_issued_cid_manager_.reset(); @@ -5639,7 +5641,8 @@ QUIC_DVLOG(1) << "Cancel validation of previous peer address change to " << previous_default_path.peer_address << " upon peer migration to " << default_path_.peer_address; - path_validator_.CancelPathValidation(); + path_validator_.CancelPathValidation( + PathValidationFailure::Reason::kNewerValidation); ++stats_.num_peer_migration_while_validating_default_path; } @@ -6951,7 +6954,8 @@ "an on-going server preferred address validation."); } // Cancel and fail any earlier validation. - path_validator_.CancelPathValidation(); + path_validator_.CancelPathValidation( + PathValidationFailure::Reason::kNewerValidation); } if (perspective_ == Perspective::IS_CLIENT && !IsDefaultPath(context->self_address(), context->peer_address())) { @@ -7074,7 +7078,11 @@ } void QuicConnection::CancelPathValidation() { - path_validator_.CancelPathValidation(); + path_validator_.CancelPathValidation(PathValidationFailure::Reason::kUnknown); +} + +void QuicConnection::CancelPathValidation(PathValidationFailure failure) { + path_validator_.CancelPathValidation(failure.reason); } bool QuicConnection::UpdateConnectionIdsOnMigration(
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index b1816fc..67aeea9 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1366,7 +1366,10 @@ QuicPathValidationContext* GetPathValidationContext() const; + // TODO(martinduke): Delete this deprecated method once non-QUICHE callers are + // updated to provide a reason. void CancelPathValidation(); + void CancelPathValidation(PathValidationFailure failure); // Returns true if the migration succeeds, otherwise returns false (e.g., no // available CIDs, connection disconnected, etc).
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 1f28529..f0b0b3c 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -2593,15 +2593,17 @@ class TestValidationResultDelegate : public QuicPathValidator::ResultDelegate { public: - TestValidationResultDelegate(QuicConnection* connection, - const QuicSocketAddress& expected_self_address, - const QuicSocketAddress& expected_peer_address, - bool* success) + TestValidationResultDelegate( + QuicConnection* connection, + const QuicSocketAddress& expected_self_address, + const QuicSocketAddress& expected_peer_address, bool* success, + std::optional<PathValidationFailure::Reason>* failure_reason = nullptr) : QuicPathValidator::ResultDelegate(), connection_(connection), expected_self_address_(expected_self_address), expected_peer_address_(expected_peer_address), - success_(success) {} + success_(success), + failure_reason_(failure_reason) {} void OnPathValidationSuccess( std::unique_ptr<QuicPathValidationContext> context, QuicTime /*start_time*/) override { @@ -2614,6 +2616,9 @@ std::unique_ptr<QuicPathValidationContext> context) override { EXPECT_EQ(expected_self_address_, context->self_address()); EXPECT_EQ(expected_peer_address_, context->peer_address()); + if (failure_reason_ != nullptr) { + *failure_reason_ = context->failure_reason(); + } if (connection_->perspective() == Perspective::IS_CLIENT) { connection_->OnPathValidationFailureAtClient(/*is_multi_port=*/false, *context); @@ -2626,6 +2631,7 @@ QuicSocketAddress expected_self_address_; QuicSocketAddress expected_peer_address_; bool* success_; + std::optional<PathValidationFailure::Reason>* failure_reason_; }; // A test implementation which migrates to server preferred address @@ -15365,17 +15371,20 @@ const QuicSocketAddress kNewSelfAddress(QuicIpAddress::Loopback4(), /*port=*/34567); bool success; + std::optional<PathValidationFailure::Reason> failure_reason; connection_.ValidatePath( std::make_unique<TestQuicPathValidationContext>( kNewSelfAddress, connection_.peer_address(), writer_.get()), std::make_unique<TestValidationResultDelegate>( - &connection_, kNewSelfAddress, connection_.peer_address(), &success), + &connection_, kNewSelfAddress, connection_.peer_address(), &success, + &failure_reason), PathValidationReason::kReasonUnknown); auto* path_validator = QuicConnectionPeer::path_validator(&connection_); - path_validator->CancelPathValidation(); + path_validator->CancelPathValidation(PathValidationFailure::Reason::kUnknown); QuicConnectionPeer::RetirePeerIssuedConnectionIdsNoLongerOnPath(&connection_); EXPECT_FALSE(success); + EXPECT_TRUE(failure_reason == PathValidationFailure::Reason::kUnknown); const auto* alternative_path = QuicConnectionPeer::GetAlternativePath(&connection_); EXPECT_TRUE(alternative_path->client_connection_id.IsEmpty());
diff --git a/quiche/quic/core/quic_path_validator.cc b/quiche/quic/core/quic_path_validator.cc index c31366f..75425ef 100644 --- a/quiche/quic/core/quic_path_validator.cc +++ b/quiche/quic/core/quic_path_validator.cc
@@ -101,11 +101,13 @@ reason_ = PathValidationReason::kReasonUnknown; } -void QuicPathValidator::CancelPathValidation() { +void QuicPathValidator::CancelPathValidation( + PathValidationFailure::Reason failure_reason) { if (path_context_ == nullptr) { return; } QUIC_DVLOG(1) << "Cancel validation on path" << *path_context_; + path_context_->set_failure_reason(failure_reason); result_delegate_->OnPathValidationFailure(std::move(path_context_)); ResetPathValidation(); } @@ -134,7 +136,7 @@ void QuicPathValidator::OnRetryTimeout() { ++retry_count_; if (retry_count_ > kMaxRetryTimes) { - CancelPathValidation(); + CancelPathValidation(PathValidationFailure::Reason::kRetryTimeout); return; } QUIC_DVLOG(1) << "Send another PATH_CHALLENGE on path " << *path_context_; @@ -149,7 +151,7 @@ if (!should_continue) { // The delegate doesn't want to continue the path validation. - CancelPathValidation(); + CancelPathValidation(PathValidationFailure::Reason::kNotConnected); return; } retry_timer_->Set(send_delegate_->GetRetryTimeout(
diff --git a/quiche/quic/core/quic_path_validator.h b/quiche/quic/core/quic_path_validator.h index 6b15cd2..edd98ce 100644 --- a/quiche/quic/core/quic_path_validator.h +++ b/quiche/quic/core/quic_path_validator.h
@@ -5,7 +5,10 @@ #ifndef QUICHE_QUIC_CORE_QUIC_PATH_VALIDATOR_H_ #define QUICHE_QUIC_CORE_QUIC_PATH_VALIDATOR_H_ +#include <cstddef> +#include <cstdint> #include <memory> +#include <optional> #include <ostream> #include "absl/container/inlined_vector.h" @@ -19,8 +22,8 @@ #include "quiche/quic/core/quic_packet_writer.h" #include "quiche/quic/core/quic_time.h" #include "quiche/quic/core/quic_types.h" -#include "quiche/quic/platform/api/quic_export.h" #include "quiche/quic/platform/api/quic_socket_address.h" +#include "quiche/common/platform/api/quiche_export.h" namespace quic { @@ -40,6 +43,18 @@ kMaxValue, }; +struct QUICHE_EXPORT PathValidationFailure { + enum class Reason : uint8_t { + kUnknown = 0, + kStatelessReset = 1, // The peer sent a stateless reset on the path. + kNewerValidation = 2, // Starting validation on a new path. + kRetryTimeout = 3, // The validation process hit the retry limit. + kNotConnected = 4, // PATH_CHALLENGE can't be sent because the + // connection is no longer active. + }; + Reason reason; +}; + // Opaque handle for device-wide connection to a particular network. // For example an association with a particular WiFi network with a particular // SSID or a connection to particular cellular network. The meaning of this @@ -91,6 +106,12 @@ const QuicSocketAddress& effective_peer_address() const { return effective_peer_address_; } + void set_failure_reason(PathValidationFailure::Reason failure_reason) { + failure_reason_ = failure_reason; + } + std::optional<PathValidationFailure::Reason> failure_reason() const { + return failure_reason_; + } private: QUICHE_EXPORT friend std::ostream& operator<<( @@ -106,6 +127,8 @@ // `kInvalidNetworkHandle`, if the platform doesn't expose handles for // networks, e.g. iOS and Linux. It shouldn't referred to in such case. QuicNetworkHandle network_handle_; + // nullopt if pending or successful. + std::optional<PathValidationFailure::Reason> failure_reason_; }; // Used to validate a path by sending up to 3 PATH_CHALLENGE frames before @@ -168,7 +191,7 @@ QuicSocketAddress self_address); // Cancel the retry timer and reset the path and result delegate. - void CancelPathValidation(); + void CancelPathValidation(PathValidationFailure::Reason failure_reason); bool HasPendingPathValidation() const;
diff --git a/quiche/quic/core/quic_path_validator_test.cc b/quiche/quic/core/quic_path_validator_test.cc index faca361..d39638c 100644 --- a/quiche/quic/core/quic_path_validator_test.cc +++ b/quiche/quic/core/quic_path_validator_test.cc
@@ -236,6 +236,8 @@ EXPECT_CALL(*result_delegate_, OnPathValidationFailure(_)) .WillOnce([=, this](std::unique_ptr<QuicPathValidationContext> context) { EXPECT_EQ(context_, context.get()); + EXPECT_EQ(context->failure_reason(), + PathValidationFailure::Reason::kRetryTimeout); }); for (size_t i = 0; i <= QuicPathValidator::kMaxRetryTimes; ++i) { clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(3 * kInitialRttMs)); @@ -255,12 +257,18 @@ QuicPacketWriter*) { // Abandon this validation in the call stack shouldn't cause crash and // should cancel the alarm. - path_validator_.CancelPathValidation(); + path_validator_.CancelPathValidation( + PathValidationFailure::Reason::kUnknown); return false; }); EXPECT_CALL(send_delegate_, GetRetryTimeout(peer_address_, &writer_)) .Times(0u); - EXPECT_CALL(*result_delegate_, OnPathValidationFailure(_)); + EXPECT_CALL(*result_delegate_, OnPathValidationFailure(_)) + .WillOnce([=, this](std::unique_ptr<QuicPathValidationContext> context) { + EXPECT_EQ(context_, context.get()); + EXPECT_EQ(context->failure_reason(), + PathValidationFailure::Reason::kUnknown); + }); path_validator_.StartPathValidation( std::unique_ptr<QuicPathValidationContext>(context_), std::unique_ptr<MockQuicPathValidationResultDelegate>(result_delegate_), @@ -271,5 +279,25 @@ path_validator_.GetPathValidationReason()); } +TEST_F(QuicPathValidatorTest, SendPathChallengeReturnsFalse) { + EXPECT_CALL(send_delegate_, + SendPathChallenge(_, self_address_, peer_address_, + effective_peer_address_, &writer_)) + .WillOnce(Return(false)); + EXPECT_CALL(send_delegate_, GetRetryTimeout(peer_address_, &writer_)) + .Times(0u); + EXPECT_CALL(*result_delegate_, OnPathValidationFailure(_)) + .WillOnce([=, this](std::unique_ptr<QuicPathValidationContext> context) { + EXPECT_EQ(context_, context.get()); + EXPECT_EQ(context->failure_reason(), + PathValidationFailure::Reason::kNotConnected); + }); + path_validator_.StartPathValidation( + std::unique_ptr<QuicPathValidationContext>(context_), + std::unique_ptr<MockQuicPathValidationResultDelegate>(result_delegate_), + PathValidationReason::kMultiPort); + EXPECT_FALSE(path_validator_.HasPendingPathValidation()); +} + } // namespace test } // namespace quic