Group sending path response and reverse path challenge closer on the server side. The advantages are: (1) The outermost ScopedPeerAddressContext can be instantiated after alternative_path_ has been created. This makes it straightforward to find a connection ID for this scoped context. (2) Ideally, path response and reverse path challenge is better sent at the end of packet processing. But that is a more complex change. The current change allows progress made in usage of connection ID without that. Protected by FLAGS_quic_reloadable_flag_quic_group_path_response_and_challenge_sending_closer. PiperOrigin-RevId: 368275656 Change-Id: I1735ff20babd2f190ba27ea7037e2be9317a6f3a
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index a793af8..95a9c3b 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -1703,8 +1703,11 @@ // PATH_RESPONSE. This context needs to be out of scope before returning. // TODO(danzh) inline OnPathChallengeFrameInternal() once // support_reverse_path_validation_ is deprecated. - QuicPacketCreator::ScopedPeerAddressContext context( - &packet_creator_, last_packet_source_address_); + auto context = + group_path_response_and_challenge_sending_closer_ + ? nullptr + : std::make_unique<QuicPacketCreator::ScopedPeerAddressContext>( + &packet_creator_, last_packet_source_address_); if (!OnPathChallengeFrameInternal(frame)) { return false; } @@ -1714,6 +1717,7 @@ bool QuicConnection::OnPathChallengeFrameInternal( const QuicPathChallengeFrame& frame) { + should_proactively_validate_peer_address_on_path_challenge_ = false; // UpdatePacketContent() may start reverse path validation. if (!UpdatePacketContent(PATH_CHALLENGE_FRAME)) { return false; @@ -1722,6 +1726,29 @@ debug_visitor_->OnPathChallengeFrame(frame); } + std::unique_ptr<QuicPacketCreator::ScopedPeerAddressContext> context; + if (group_path_response_and_challenge_sending_closer_) { + context = std::make_unique<QuicPacketCreator::ScopedPeerAddressContext>( + &packet_creator_, last_packet_source_address_); + } + if (should_proactively_validate_peer_address_on_path_challenge_) { + QUIC_RELOADABLE_FLAG_COUNT( + quic_group_path_response_and_challenge_sending_closer); + QuicSocketAddress current_effective_peer_address = + GetEffectivePeerAddressFromCurrentPacket(); + // Conditions to proactively validate peer address: + // The perspective is server + // The PATH_CHALLENGE is received on an unvalidated alternative path. + // The connection isn't validating migrated peer address, which is of + // higher prority. + QUIC_DVLOG(1) << "Proactively validate the effective peer address " + << current_effective_peer_address; + ValidatePath(std::make_unique<ReversePathValidationContext>( + default_path_.self_address, current_effective_peer_address, + current_effective_peer_address, this), + std::make_unique<ReversePathValidationResultDelegate>( + this, peer_address())); + } if (!send_path_response_) { // Save the path challenge's payload, for later use in generating the // response. @@ -5453,19 +5480,23 @@ last_packet_destination_address_, current_effective_peer_address, client_connection_id, last_packet_destination_connection_id_, stateless_reset_token_received, stateless_reset_token); - // Conditions to proactively validate peer address: - // The perspective is server - // The PATH_CHALLENGE is received on an unvalidated alternative path. - // The connection isn't validating migrated peer address, which is of - // higher prority. - QUIC_DVLOG(1) << "Proactively validate the effective peer address " - << current_effective_peer_address; - ValidatePath( - std::make_unique<ReversePathValidationContext>( - default_path_.self_address, current_effective_peer_address, - current_effective_peer_address, this), - std::make_unique<ReversePathValidationResultDelegate>( - this, peer_address())); + if (group_path_response_and_challenge_sending_closer_) { + should_proactively_validate_peer_address_on_path_challenge_ = true; + } else { + // Conditions to proactively validate peer address: + // The perspective is server + // The PATH_CHALLENGE is received on an unvalidated alternative path. + // The connection isn't validating migrated peer address, which is of + // higher prority. + QUIC_DVLOG(1) << "Proactively validate the effective peer address " + << current_effective_peer_address; + ValidatePath( + std::make_unique<ReversePathValidationContext>( + default_path_.self_address, current_effective_peer_address, + current_effective_peer_address, this), + std::make_unique<ReversePathValidationResultDelegate>( + this, peer_address())); + } } } MaybeUpdateBytesReceivedFromAlternativeAddress(last_size_);
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 5513795..b935943 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -2229,6 +2229,14 @@ bool use_connection_id_on_default_path_ = GetQuicReloadableFlag(quic_use_connection_id_on_default_path); + // Indicates whether we should proactively validate peer address on a + // PATH_CHALLENGE received. + bool should_proactively_validate_peer_address_on_path_challenge_ = false; + + const bool group_path_response_and_challenge_sending_closer_ = + GetQuicReloadableFlag( + quic_group_path_response_and_challenge_sending_closer); + const bool quic_deprecate_incoming_connection_ids_ = GetQuicReloadableFlag(quic_deprecate_incoming_connection_ids); };
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 17c34a9..f5fe8bc 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -52,6 +52,7 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_on_stream_reset, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_stateless_reset, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true) +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_group_path_response_and_challenge_sending_closer, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_h3_datagram, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_pass_path_response_to_validator, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_preempt_stream_data_with_handshake_packet, true)