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)
