Make QUIC connections send PATH_RESPONSE upon receiving PATH_CHALLENGE according to IETF QUIC draft v29.
Switch to use peer_address in the packet creator to write packet instead of the connection's peer address for both gQUIC and iQUIC.
In iQUIC, change the peer_address in packet creator temporarily to send PATH_RESPONSE to the source address of the current incoming packet.
Diff with old IETF impl:
The old behavior is that only server responds to PATH_CHALLENGE. V29 path validation section says both sides should be able to send and respond to PATH_CHALLENGE.
The old behavior sends PATH_RESPONSE after parsing the whole packet via a callback to QuicSession::OnPacketReceived() which is the same behavior as gQUIC response padded PING. IETF path validation doesn't need to notify session about receiving PATH_CHALLENGE. Connection itself is able to respond right away at OnPathChallenge().
The old behavior sends PATH_RESPONSE in a different code path which doesn't retry if socket is blocked. This CL changes to send PATH_RESPONSE on normal packet writing logic and buffer it if the write attempt fails.
The old behavior sample RTT from probing packets, but as they are sent on different path, they shouldn't contribute to RTT measurement on current path.
Code cleanup:
Since QuicConnection::SendConnectivityProbingResponsePacket() no longer sends PATH_RESPONSE, but only padded PING. Deprecate it with calling SendConnectivityProbingPacket() at the call sites. And update GFE stats accordingly.
Protected by FLAGS_quic_reloadable_flag_quic_send_path_response and existing flag --quic_reloadable_flag_quic_start_peer_migration_earlier.
PiperOrigin-RevId: 329738854
Change-Id: I45345611ff31011f76c72c406a9431cde031db96
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index fa6aec9..32fc3f1 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1138,6 +1138,7 @@
// Test that server session will send a connectivity probe in response to a
// connectivity probe on the same path.
TEST_P(QuicSpdySessionTestServer, ServerReplyToConnecitivityProbe) {
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
QuicSocketAddress old_peer_address =
QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort);
EXPECT_EQ(old_peer_address, session_.peer_address());
@@ -1145,8 +1146,14 @@
QuicSocketAddress new_peer_address =
QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort + 1);
- EXPECT_CALL(*connection_,
- SendConnectivityProbingResponsePacket(new_peer_address));
+ 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,
// which stores its payload for use in generating the response.
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index d89639f..0a7430b 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -26,6 +26,7 @@
#include "net/third_party/quiche/src/quic/core/quic_constants.h"
#include "net/third_party/quiche/src/quic/core/quic_error_codes.h"
#include "net/third_party/quiche/src/quic/core/quic_legacy_version_encapsulator.h"
+#include "net/third_party/quiche/src/quic/core/quic_packet_creator.h"
#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/core/quic_utils.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h"
@@ -37,6 +38,7 @@
#include "net/third_party/quiche/src/quic/platform/api/quic_hostname_utils.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_map_util.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_socket_address.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_string_utils.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_str_cat.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_string_piece.h"
@@ -222,6 +224,7 @@
server_connection_id.length()),
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),
helper_(helper),
alarm_factory_(alarm_factory),
@@ -318,6 +321,7 @@
&arena_,
alarm_factory_),
support_handshake_done_(version().HasHandshakeDone()) {
+ QUIC_BUG_IF(!start_peer_migration_earlier_ && send_path_response_);
if (fix_missing_connected_checks_) {
QUIC_RELOADABLE_FLAG_COUNT(quic_add_missing_connected_checks);
}
@@ -364,6 +368,7 @@
blackhole_detection_disabled_ = true;
}
}
+ packet_creator_.SetDefaultPeerAddress(initial_peer_address);
}
void QuicConnection::InstallInitialCrypters(QuicConnectionId connection_id) {
@@ -1123,6 +1128,7 @@
// 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;
if (perspective_ == Perspective::IS_CLIENT) {
@@ -1132,7 +1138,7 @@
// client connections.
// TODO(fayang): only change peer addresses in application data packet
// number space.
- direct_peer_address_ = last_packet_source_address_;
+ UpdatePeerAddress(last_packet_source_address_);
effective_peer_address_ = GetEffectivePeerAddressFromCurrentPacket();
}
} else {
@@ -1478,16 +1484,42 @@
}
bool QuicConnection::OnPathChallengeFrame(const QuicPathChallengeFrame& frame) {
+ if (has_path_challenge_in_current_packet_) {
+ DCHECK(send_path_response_);
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_send_path_response, 2, 5);
+ // Only respond to the 1st PATH_CHALLENGE.
+ return true;
+ }
UpdatePacketContent(PATH_CHALLENGE_FRAME);
if (debug_visitor_ != nullptr) {
debug_visitor_->OnPathChallengeFrame(frame);
}
- // Save the path challenge's payload, for later use in generating the
- // response.
- received_path_challenge_payloads_.push_back(frame.data_buffer);
+ 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_response, 3, 5);
+ has_path_challenge_in_current_packet_ = true;
MaybeUpdateAckTimeout();
- return true;
+ // Queue or send PATH_RESPONSE. No matter where the pending data are supposed
+ // to sent, PATH_RESPONSE should always be sent to the source address of the
+ // current incoming packet.
+ if (!SendPathResponse(frame.data_buffer, last_packet_source_address_)) {
+ // Queue the payloads to re-try later.
+ pending_path_challenge_payloads_.push_back(
+ {frame.data_buffer, last_packet_source_address_});
+ }
+ // TODO(b/150095588): change the stats to
+ // num_valid_path_challenge_received.
+ ++stats_.num_connectivity_probing_received;
+
+ // SendPathResponse() might cause connection to be closed.
+ return connected_;
}
bool QuicConnection::OnPathResponseFrame(const QuicPathResponseFrame& frame) {
@@ -1746,6 +1778,9 @@
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.
@@ -2191,7 +2226,7 @@
}
if (!direct_peer_address_.IsInitialized()) {
- direct_peer_address_ = last_packet_source_address_;
+ UpdatePeerAddress(last_packet_source_address_);
}
if (!effective_peer_address_.IsInitialized()) {
@@ -2291,6 +2326,17 @@
}
}
+ // TODO(danzh) PATH_RESPONSE is of more interest to the peer than ACK,
+ // evaluate if it's worth to send them before sending ACKs.
+ while (!pending_path_challenge_payloads_.empty()) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_send_path_response, 4, 5);
+ std::pair<QuicPathFrameBuffer, QuicSocketAddress> pair =
+ pending_path_challenge_payloads_.front();
+ if (!SendPathResponse(pair.first, pair.second)) {
+ break;
+ }
+ pending_path_challenge_payloads_.pop_front();
+ }
WriteNewData();
}
@@ -2684,6 +2730,8 @@
// 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();
switch (fate) {
case DISCARD:
++stats_.packets_discarded;
@@ -2691,7 +2739,7 @@
case COALESCE:
QUIC_BUG_IF(!version().CanSendCoalescedPackets());
if (!coalesced_packet_.MaybeCoalescePacket(
- *packet, self_address(), peer_address(),
+ *packet, self_address(), send_to_address,
helper_->GetStreamSendBufferAllocator(),
packet_creator_.max_packet_length())) {
// Failed to coalesce packet, flush current coalesced packet.
@@ -2700,7 +2748,7 @@
return false;
}
if (!coalesced_packet_.MaybeCoalescePacket(
- *packet, self_address(), peer_address(),
+ *packet, self_address(), send_to_address,
helper_->GetStreamSendBufferAllocator(),
packet_creator_.max_packet_length())) {
// Failed to coalesce packet even it is the only packet, raise a write
@@ -2721,7 +2769,7 @@
case BUFFER:
QUIC_DVLOG(1) << ENDPOINT << "Adding packet: " << packet->packet_number
<< " to buffered packets";
- buffered_packets_.emplace_back(*packet, self_address(), peer_address());
+ buffered_packets_.emplace_back(*packet, self_address(), send_to_address);
break;
case SEND_TO_WRITER:
// At this point, packet->release_encrypted_buffer is either nullptr,
@@ -2733,7 +2781,7 @@
// writer_->WritePacket transfers buffer ownership back to the writer.
packet->release_encrypted_buffer = nullptr;
result = writer_->WritePacket(packet->encrypted_buffer, encrypted_length,
- self_address().host(), peer_address(),
+ self_address().host(), send_to_address,
per_packet_options_);
// This is a work around for an issue with linux UDP GSO batch writers.
// When sending a GSO packet with 2 segments, if the first segment is
@@ -2779,13 +2827,14 @@
}
if (!buffered_packets_.empty() || HandleWriteBlocked()) {
// Buffer the packet.
- buffered_packets_.emplace_back(*packet, self_address(), peer_address());
+ buffered_packets_.emplace_back(*packet, self_address(),
+ send_to_address);
} else { // Send the packet to the writer.
// writer_->WritePacket transfers buffer ownership back to the writer.
packet->release_encrypted_buffer = nullptr;
result = writer_->WritePacket(packet->encrypted_buffer,
encrypted_length, self_address().host(),
- peer_address(), per_packet_options_);
+ send_to_address, per_packet_options_);
}
} break;
default:
@@ -2810,7 +2859,7 @@
if (result.status != WRITE_STATUS_BLOCKED_DATA_BUFFERED) {
QUIC_DVLOG(1) << ENDPOINT << "Adding packet: " << packet->packet_number
<< " to buffered packets";
- buffered_packets_.emplace_back(*packet, self_address(), peer_address());
+ buffered_packets_.emplace_back(*packet, self_address(), send_to_address);
}
}
@@ -2833,7 +2882,7 @@
QUIC_LOG_FIRST_N(ERROR, 10)
<< ENDPOINT << "Failed writing packet " << packet_number << " of "
<< encrypted_length << " bytes from " << self_address().host() << " to "
- << peer_address() << ", with error code " << result.error_code
+ << send_to_address << ", with error code " << result.error_code
<< ". long_term_mtu_:" << long_term_mtu_
<< ", previous_validated_mtu_:" << previous_validated_mtu_
<< ", max_packet_length():" << max_packet_length()
@@ -2885,9 +2934,15 @@
bytes_sent_before_address_validation_ += encrypted_length;
}
+ // Do not measure rtt of this packet if it's not sent on current path.
+ const bool measure_rtt = send_to_address == peer_address();
+ QUIC_DLOG_IF(INFO, !measure_rtt)
+ << ENDPOINT << " Sent packet " << packet->packet_number
+ << " on a different path with remote address " << send_to_address
+ << " while current path has peer address " << peer_address();
const bool in_flight = sent_packet_manager_.OnPacketSent(
packet, packet_send_time, packet->transmission_type,
- IsRetransmittable(*packet), /*measure_rtt=*/true);
+ IsRetransmittable(*packet), measure_rtt);
QUIC_BUG_IF(default_enable_5rto_blackhole_detection_ &&
blackhole_detector_.IsDetectionInProgress() &&
!sent_packet_manager_.HasInFlightPackets())
@@ -3487,6 +3542,9 @@
void QuicConnection::SendConnectionClosePacket(QuicErrorCode error,
const std::string& details) {
+ // Always use the current path to send CONNECTION_CLOSE.
+ QuicPacketCreator::ScopedPeerAddressContext context(&packet_creator_,
+ peer_address());
if (!SupportsMultiplePacketNumberSpaces()) {
QUIC_DLOG(INFO) << ENDPOINT << "Sending connection close packet.";
SetDefaultEncryptionLevel(GetConnectionCloseEncryptionLevel());
@@ -3949,6 +4007,7 @@
// request or a response.
probing_packet = packet_creator_.SerializeConnectivityProbingPacket();
} else if (is_response) {
+ DCHECK(!send_path_response_);
// IETF QUIC path response.
// Respond to path probe request using IETF QUIC PATH_RESPONSE frame.
probing_packet =
@@ -4001,7 +4060,7 @@
*probing_packet, probing_packet->transmission_type, packet_send_time);
}
- // Call OnPacketSent regardless of the write result.
+ // Send in currrent path. Call OnPacketSent regardless of the write result.
sent_packet_manager_.OnPacketSent(
probing_packet.get(), packet_send_time, probing_packet->transmission_type,
NO_RETRANSMITTABLE_DATA, /*measure_rtt=*/true);
@@ -4213,7 +4272,7 @@
current_packet_content_ = NOT_PADDED_PING;
if (GetLargestReceivedPacket().IsInitialized() &&
last_header_.packet_number == GetLargestReceivedPacket()) {
- direct_peer_address_ = last_packet_source_address_;
+ UpdatePeerAddress(last_packet_source_address_);
if (current_effective_peer_migration_type_ != NO_CHANGE) {
// Start effective peer migration immediately when the current packet is
// confirmed not a connectivity probing packet.
@@ -4231,7 +4290,7 @@
QUIC_CODE_COUNT(quic_start_peer_migration_earlier);
if (GetLargestReceivedPacket().IsInitialized() &&
last_header_.packet_number == GetLargestReceivedPacket()) {
- direct_peer_address_ = last_packet_source_address_;
+ UpdatePeerAddress(last_packet_source_address_);
if (current_effective_peer_migration_type_ != NO_CHANGE) {
// Start effective peer migration when the current packet contains a
// non-probing frame.
@@ -4873,5 +4932,21 @@
return num_rtos_for_blackhole_detection_ > 0;
}
+bool QuicConnection::SendPathResponse(const QuicPathFrameBuffer& data_buffer,
+ QuicSocketAddress peer_address_to_send) {
+ // Send PATH_RESPONSE using the provided peer address. If the creator has been
+ // using a different peer address, it will flush before and after serializing
+ // the current PATH_RESPONSE.
+ QUIC_DVLOG(1) << ENDPOINT << "Send PATH_RESPONSE to " << peer_address_to_send;
+ QuicPacketCreator::ScopedPeerAddressContext context(&packet_creator_,
+ peer_address_to_send);
+ return packet_creator_.AddPathResponseFrame(data_buffer);
+}
+
+void QuicConnection::UpdatePeerAddress(QuicSocketAddress peer_address) {
+ direct_peer_address_ = peer_address;
+ packet_creator_.SetDefaultPeerAddress(peer_address);
+}
+
#undef ENDPOINT // undef for jumbo builds
} // namespace quic
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 4d9be1a..f9ff508 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -823,6 +823,8 @@
// 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);
@@ -1007,6 +1009,8 @@
// 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_; }
+
// If now is close to idle timeout, returns true and sends a connectivity
// probing packet to test the connection for liveness. Otherwise, returns
// false.
@@ -1247,9 +1251,9 @@
void UpdateReleaseTimeIntoFuture();
// 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
- // response. If version 99/ietf quic, will send a PATH_RESPONSE if
- // |is_response| is true, a PATH_CHALLENGE if not.
+ // 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);
@@ -1372,6 +1376,13 @@
// received and the current packet number is largest received so far.
void MaybeStartIetfPeerMigration(QuicFrameType type);
+ // Send PATH_RESPONSE to the given peer address.
+ bool SendPathResponse(const QuicPathFrameBuffer& data_buffer,
+ QuicSocketAddress peer_address_to_send);
+
+ // Update both connection's and packet creator's peer address.
+ void UpdatePeerAddress(QuicSocketAddress peer_address);
+
QuicFramer framer_;
// Contents received in the current packet, especially used to identify
@@ -1382,6 +1393,8 @@
// Always false outside the context of ProcessUdpPacket().
bool is_current_packet_connectivity_probing_;
+ bool has_path_challenge_in_current_packet_;
+
// 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.
@@ -1407,6 +1420,8 @@
QuicSocketAddress self_address_;
QuicSocketAddress peer_address_;
+ // Other than initialization, do not modify it directly, use
+ // UpdatePeerAddress() instead.
QuicSocketAddress direct_peer_address_;
// Address of the endpoint behind the proxy if the connection is proxied.
// Otherwise it is the same as |peer_address_|.
@@ -1655,8 +1670,16 @@
// Deque because the peer might no be using this implementation, and others
// might send a packet with more than one PATH_CHALLENGE, so all need to be
// saved and responded to.
+ // TODO(danzh) deprecate this field when deprecating
+ // --quic_send_path_response.
QuicCircularDeque<QuicPathFrameBuffer> received_path_challenge_payloads_;
+ // Buffer outstanding PATH_CHALLENGEs if socket write is blocked, future
+ // OnCanWrite will attempt to respond with PATH_RESPONSEs using the retained
+ // payload and peer addresses.
+ QuicCircularDeque<std::pair<QuicPathFrameBuffer, QuicSocketAddress>>
+ pending_path_challenge_payloads_;
+
// Set of connection IDs that should be accepted as destination on
// received packets. This is conceptually a set but is implemented as a
// vector to improve performance since it is expected to be very small.
@@ -1733,6 +1756,11 @@
bool fix_missing_connected_checks_ =
GetQuicReloadableFlag(quic_add_missing_connected_checks);
+
+ // latch --gfe2_reloadable_flag_quic_send_path_response and
+ // --gfe2_reloadable_flag_quic_start_peer_migration_earlier.
+ bool send_path_response_ = start_peer_migration_earlier_ &&
+ GetQuicReloadableFlag(quic_send_path_response);
};
} // namespace quic
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 1cc2329..280094f 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -622,7 +622,7 @@
SimpleQuicFramer* framer() { return &framer_; }
- QuicSocketAddress last_write_peer_address() const {
+ const QuicSocketAddress& last_write_peer_address() const {
return last_write_peer_address_;
}
@@ -731,6 +731,7 @@
SerializedPacket serialized_packet(
QuicPacketNumber(packet_number), PACKET_4BYTE_PACKET_NUMBER, buffer,
encrypted_length, has_ack, has_pending_frames);
+ serialized_packet.peer_address = kPeerAddress;
if (retransmittable == HAS_RETRANSMITTABLE_DATA) {
serialized_packet.retransmittable_frames.push_back(
QuicFrame(QuicPingFrame()));
@@ -1177,6 +1178,7 @@
ENCRYPTION_FORWARD_SECURE,
std::make_unique<NullDecrypter>(Perspective::IS_CLIENT));
}
+ peer_creator_.SetDefaultPeerAddress(kSelfAddress);
}
QuicConnectionTest(const QuicConnectionTest&) = delete;
@@ -1766,6 +1768,9 @@
}
connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
peer_creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
+ // Prevent packets from being coalesced.
+ EXPECT_CALL(visitor_, GetHandshakeState())
+ .WillRepeatedly(Return(HANDSHAKE_CONFIRMED));
}
void TestClientRetryHandling(bool invalid_retry_tag,
@@ -2118,8 +2123,9 @@
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
EXPECT_CALL(visitor_, OnPacketReceived(_, _, false)).Times(0);
- // Process a padded PING or PATH CHALLENGE packet with no peer address change
- // on server side will be ignored.
+ // 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
+ // will be considered as path probing.
std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
@@ -2131,7 +2137,10 @@
connection_.GetStats().num_connectivity_probing_received;
ProcessReceivedPacket(kSelfAddress, kPeerAddress, *received);
- EXPECT_EQ(num_probing_received,
+ EXPECT_EQ(num_probing_received + (GetParam().version.HasIetfQuicFrames() &&
+ connection_.send_path_response()
+ ? 1u
+ : 0u),
connection_.GetStats().num_connectivity_probing_received);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2453,7 +2462,7 @@
EXPECT_EQ(kNewPeerAddress, connection_.effective_peer_address());
}
-TEST_P(QuicConnectionTest, ReceivePaddedPingAtClient) {
+TEST_P(QuicConnectionTest, ReceiveConnectivityProbingPacketAtClient) {
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -2478,7 +2487,9 @@
// Client takes all padded PING packet as speculative connectivity
// probing packet, and reports to visitor.
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- EXPECT_CALL(visitor_, OnPacketReceived(_, _, false)).Times(1);
+ 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(
@@ -2489,7 +2500,10 @@
connection_.GetStats().num_connectivity_probing_received;
ProcessReceivedPacket(kSelfAddress, kPeerAddress, *received);
- EXPECT_EQ(num_probing_received,
+ EXPECT_EQ(num_probing_received + (GetParam().version.HasIetfQuicFrames() &&
+ connection_.send_path_response()
+ ? 1u
+ : 0u),
connection_.GetStats().num_connectivity_probing_received);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -9170,12 +9184,13 @@
// Test to check that the path challenge/path response logic works
// correctly. This test is only for version-99
-TEST_P(QuicConnectionTest, PathChallengeResponse) {
+TEST_P(QuicConnectionTest, ServerResponseToPathChallenge) {
if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
- // First check if we can probe from server to client and back
- set_perspective(Perspective::IS_SERVER);
+ PathProbeTestInit(Perspective::IS_SERVER);
+ QuicConnectionPeer::SetAddressValidated(&connection_);
+ // First check if the server can send probing packet.
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
// Create and send the probe request (PATH_CHALLENGE frame).
@@ -9195,21 +9210,61 @@
// Normally, QuicConnection::OnPathChallengeFrame and OnPaddingFrame would be
// called and it will perform actions to ensure that the rest of the protocol
- // is performed (specifically, call UpdatePacketContent to say that this is a
- // path challenge so that when QuicConnection::OnPacketComplete is called
- // (again, out of the framer), the response is generated). Simulate those
- // calls so that the right internal state is set up for generating
- // the response.
+ // is performed.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
EXPECT_TRUE(connection_.OnPathChallengeFrame(
writer_->path_challenge_frames().front()));
EXPECT_TRUE(connection_.OnPaddingFrame(writer_->padding_frames().front()));
- // Cause the response to be created and sent. Result is that the response
- // should be stashed in writer's path_response_frames.
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- connection_.SendConnectivityProbingResponsePacket(connection_.peer_address());
+ 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
// the random data from the challenge.
+ EXPECT_EQ(1u, writer_->path_response_frames().size());
+ EXPECT_EQ(0, memcmp(&challenge_data,
+ &(writer_->path_response_frames().front().data_buffer),
+ sizeof(challenge_data)));
+}
+
+TEST_P(QuicConnectionTest, ClientResponseToPathChallenge) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.send_path_response()) {
+ return;
+ }
+ PathProbeTestInit(Perspective::IS_CLIENT);
+ // First check if the client can send probing packet.
+ QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
+
+ // Create and send the probe request (PATH_CHALLENGE frame).
+ // SendConnectivityProbingPacket ends up calling
+ // TestPacketWriter::WritePacket() which in turns receives and parses the
+ // packet by calling framer_.ProcessPacket() -- which in turn calls
+ // SimpleQuicFramer::OnPathChallengeFrame(). SimpleQuicFramer saves
+ // the packet in writer_->path_challenge_frames()
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ connection_.SendConnectivityProbingPacket(writer_.get(),
+ connection_.peer_address());
+ // Save the random contents of the challenge for later validation against the
+ // response.
+ ASSERT_GE(writer_->path_challenge_frames().size(), 1u);
+ QuicPathFrameBuffer challenge_data =
+ writer_->path_challenge_frames().front().data_buffer;
+
+ // Normally, QuicConnection::OnPathChallengeFrame would be
+ // called and it will perform actions to ensure that the rest of the protocol
+ // is performed.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ EXPECT_TRUE(connection_.OnPathChallengeFrame(
+ writer_->path_challenge_frames().front()));
+ EXPECT_TRUE(connection_.OnPaddingFrame(writer_->padding_frames().front()));
+ creator_->FlushCurrentPacket();
+
+ // The final check is to ensure that the random data in the response matches
+ // the random data from the challenge.
+ EXPECT_EQ(1u, writer_->path_response_frames().size());
EXPECT_EQ(0, memcmp(&challenge_data,
&(writer_->path_response_frames().front().data_buffer),
sizeof(challenge_data)));
@@ -11914,6 +11969,353 @@
ProcessDataPacketAtLevel(1000, false, ENCRYPTION_FORWARD_SECURE);
}
+// Check that if there are two PATH_CHALLENGE frames in the packet, the latter
+// one is ignored.
+TEST_P(QuicConnectionTest, ReceiveMultiplePathChallenge) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ return;
+ }
+ PathProbeTestInit(Perspective::IS_SERVER);
+
+ // Clear direct_peer_address.
+ QuicConnectionPeer::SetDirectPeerAddress(&connection_, QuicSocketAddress());
+ // Clear effective_peer_address, it is the same as direct_peer_address for
+ // this test.
+ QuicConnectionPeer::SetEffectivePeerAddress(&connection_,
+ QuicSocketAddress());
+ EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
+
+ if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+ EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
+ } else {
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
+ }
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
+ EXPECT_EQ(kPeerAddress, connection_.peer_address());
+ EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
+
+ QuicPathFrameBuffer path_frame_buffer1{0, 1, 2, 3, 4, 5, 6, 7};
+ QuicPathFrameBuffer path_frame_buffer2{8, 9, 10, 11, 12, 13, 14, 15};
+ QuicFrames frames;
+ frames.push_back(
+ QuicFrame(new QuicPathChallengeFrame(0, path_frame_buffer1)));
+ frames.push_back(
+ QuicFrame(new QuicPathChallengeFrame(0, path_frame_buffer2)));
+ const QuicSocketAddress kNewPeerAddress(QuicIpAddress::Loopback6(),
+ /*port=*/23456);
+
+ EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
+
+ // Expect 2 packets to be sent: the first are padded PATH_RESPONSE(s) to the
+ // alternative peer address. The 2nd is a ACK-only packet to the original
+ // peer address.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
+ .Times(2)
+ .WillOnce(Invoke([=]() {
+ EXPECT_EQ((connection_.send_path_response() ? 1u : 2u),
+ 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(kNewPeerAddress, writer_->last_write_peer_address());
+ }))
+ .WillOnce(Invoke([=]() {
+ // The last write of ACK-only packet should still use the old peer
+ // address.
+ EXPECT_EQ(kPeerAddress, writer_->last_write_peer_address());
+ }));
+ ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress);
+}
+
+TEST_P(QuicConnectionTest, ReceiveStreamFrameBeforePathChallenge) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.send_path_response()) {
+ return;
+ }
+ PathProbeTestInit(Perspective::IS_SERVER);
+
+ // Clear direct_peer_address.
+ QuicConnectionPeer::SetDirectPeerAddress(&connection_, QuicSocketAddress());
+ // Clear effective_peer_address, it is the same as direct_peer_address for
+ // this test.
+ QuicConnectionPeer::SetEffectivePeerAddress(&connection_,
+ QuicSocketAddress());
+ EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
+
+ if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+ EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
+ } else {
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
+ }
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
+ EXPECT_EQ(kPeerAddress, connection_.peer_address());
+ EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
+
+ QuicFrames frames;
+ frames.push_back(QuicFrame(frame1_));
+ QuicPathFrameBuffer path_frame_buffer{0, 1, 2, 3, 4, 5, 6, 7};
+ frames.push_back(QuicFrame(new QuicPathChallengeFrame(0, path_frame_buffer)));
+ const QuicSocketAddress kNewPeerAddress(QuicIpAddress::Loopback6(),
+ /*port=*/23456);
+
+ EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE));
+ EXPECT_CALL(visitor_, OnStreamFrame(_))
+ .WillOnce(Invoke([=](const QuicStreamFrame& frame) {
+ // Send some data on the stream. The STREAM_FRAME should be built into
+ // one packet together with the latter PATH_RESPONSE.
+ std::string data{"response body"};
+ struct iovec iov;
+ MakeIOVector(data, &iov);
+ connection_.producer()->SaveStreamData(frame.stream_id, &iov, 1, 0u,
+ data.length());
+ return notifier_.WriteOrBufferData(frame.stream_id, data.length(),
+ NO_FIN);
+ }));
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _));
+ ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress);
+
+ // Verify that this packet contains a STREAM_FRAME and a
+ // PATH_RESPONSE_FRAME.
+ EXPECT_EQ(1u, writer_->stream_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_buffer.data(),
+ &(writer_->path_response_frames().front().data_buffer),
+ sizeof(path_frame_buffer)));
+ EXPECT_EQ(1u, writer_->padding_frames().size());
+ EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
+}
+
+TEST_P(QuicConnectionTest, ReceiveStreamFrameFollowingPathChallenge) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.send_path_response()) {
+ return;
+ }
+ PathProbeTestInit(Perspective::IS_SERVER);
+
+ // Clear direct_peer_address.
+ QuicConnectionPeer::SetDirectPeerAddress(&connection_, QuicSocketAddress());
+ // Clear effective_peer_address, it is the same as direct_peer_address for
+ // this test.
+ QuicConnectionPeer::SetEffectivePeerAddress(&connection_,
+ QuicSocketAddress());
+ EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
+
+ if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+ EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
+ } else {
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
+ }
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
+ EXPECT_EQ(kPeerAddress, connection_.peer_address());
+ EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
+
+ QuicFrames frames;
+ QuicPathFrameBuffer path_frame_buffer{0, 1, 2, 3, 4, 5, 6, 7};
+ frames.push_back(QuicFrame(new QuicPathChallengeFrame(0, path_frame_buffer)));
+ // PATH_RESPONSE should be flushed out before the rest packet is parsed.
+ frames.push_back(QuicFrame(frame1_));
+ const QuicSocketAddress kNewPeerAddress(QuicIpAddress::Loopback6(),
+ /*port=*/23456);
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
+ .WillOnce(Invoke([=]() {
+ // Verify that this packet contains a PATH_RESPONSE_FRAME.
+ EXPECT_EQ(0u, writer_->stream_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_buffer.data(),
+ &(writer_->path_response_frames().front().data_buffer),
+ sizeof(path_frame_buffer)));
+ EXPECT_EQ(1u, writer_->padding_frames().size());
+ EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
+ }))
+ .WillOnce(Invoke([=]() {
+ // Verify that this packet contains a STREAM_FRAME.
+ EXPECT_EQ(1u, writer_->stream_frames().size());
+ EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
+ }));
+ EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE));
+ EXPECT_CALL(visitor_, OnStreamFrame(_))
+ .WillOnce(Invoke([=](const QuicStreamFrame& frame) {
+ // Send some data on the stream. The STREAM_FRAME should be built into
+ // one packet together with the latter PATH_RESPONSE.
+ std::string data{"response body"};
+ struct iovec iov;
+ MakeIOVector(data, &iov);
+ connection_.producer()->SaveStreamData(frame.stream_id, &iov, 1, 0u,
+ data.length());
+ return notifier_.WriteOrBufferData(frame.stream_id, data.length(),
+ NO_FIN);
+ }));
+
+ ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress);
+}
+
+// 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()) {
+ return;
+ }
+ PathProbeTestInit(Perspective::IS_SERVER);
+
+ // Clear direct_peer_address.
+ QuicConnectionPeer::SetDirectPeerAddress(&connection_, QuicSocketAddress());
+ // Clear effective_peer_address, it is the same as direct_peer_address for
+ // this test.
+ QuicConnectionPeer::SetEffectivePeerAddress(&connection_,
+ QuicSocketAddress());
+ EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
+
+ if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+ EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
+ } else {
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
+ }
+ QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, 2);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
+ EXPECT_EQ(kPeerAddress, connection_.peer_address());
+ EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
+
+ QuicFrames frames;
+ frames.push_back(QuicFrame(frame1_));
+ QuicPathFrameBuffer path_frame_buffer{0, 1, 2, 3, 4, 5, 6, 7};
+ frames.push_back(QuicFrame(new QuicPathChallengeFrame(0, path_frame_buffer)));
+ frames.push_back(QuicFrame(frame2_));
+ const QuicSocketAddress kNewPeerAddress(QuicIpAddress::Loopback6(),
+ /*port=*/23456);
+
+ EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0u);
+ EXPECT_CALL(visitor_, OnStreamFrame(_))
+ .Times(2)
+ .WillRepeatedly(Invoke([=](const QuicStreamFrame& frame) {
+ // Send some data on the stream. The STREAM_FRAME should be built into
+ // one packet together with the latter PATH_RESPONSE.
+ std::string data{"response body"};
+ struct iovec iov;
+ MakeIOVector(data, &iov);
+ connection_.producer()->SaveStreamData(frame.stream_id, &iov, 1, 0u,
+ data.length());
+ return notifier_.WriteOrBufferData(frame.stream_id, data.length(),
+ NO_FIN);
+ }));
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
+ .WillOnce(Invoke([=]() {
+ // Verify that this packet contains a STREAM_FRAME and is sent to the
+ // original peer address.
+ EXPECT_EQ(1u, writer_->stream_frames().size());
+ // No connection migration should happen because the packet is received
+ // out of order.
+ EXPECT_EQ(kPeerAddress, writer_->last_write_peer_address());
+ }))
+ .WillOnce(Invoke([=]() {
+ 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_buffer.data(),
+ &(writer_->path_response_frames().front().data_buffer),
+ sizeof(path_frame_buffer)));
+ EXPECT_EQ(1u, writer_->padding_frames().size());
+ // PATH_RESPONSE should be sent in another packet to a different peer
+ // address.
+ EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
+ }))
+ .WillOnce(Invoke([=]() {
+ // Verify that this packet contains a STREAM_FRAME and is sent to the
+ // original peer address.
+ EXPECT_EQ(1u, writer_->stream_frames().size());
+ // No connection migration should happen because the packet is received
+ // out of order.
+ EXPECT_EQ(kPeerAddress, writer_->last_write_peer_address());
+ }));
+ // Lower the packet number so that receiving this packet shouldn't trigger
+ // peer migration.
+ QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, 1);
+ ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress);
+}
+
+// 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()) {
+ return;
+ }
+ PathProbeTestInit(Perspective::IS_SERVER);
+
+ // Clear direct_peer_address.
+ QuicConnectionPeer::SetDirectPeerAddress(&connection_, QuicSocketAddress());
+ // Clear effective_peer_address, it is the same as direct_peer_address for
+ // this test.
+ QuicConnectionPeer::SetEffectivePeerAddress(&connection_,
+ QuicSocketAddress());
+ EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
+
+ if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+ EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
+ } else {
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
+ }
+ QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, 2);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
+ EXPECT_EQ(kPeerAddress, connection_.peer_address());
+ EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
+
+ QuicFrames frames;
+ QuicPathFrameBuffer path_frame_buffer{0, 1, 2, 3, 4, 5, 6, 7};
+ frames.push_back(QuicFrame(new QuicPathChallengeFrame(0, path_frame_buffer)));
+ const QuicSocketAddress kNewPeerAddress(QuicIpAddress::Loopback6(),
+ /*port=*/23456);
+
+ EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0u);
+ // Lower the packet number so that receiving this packet shouldn't trigger
+ // peer migration.
+ QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, 1);
+ EXPECT_CALL(visitor_, OnWriteBlocked()).Times(AtLeast(1));
+ writer_->SetWriteBlocked();
+ ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress);
+
+ EXPECT_EQ(
+ 1u,
+ QuicConnectionPeer::pending_path_challenge_payloads(&connection_).size());
+
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(AtLeast(1));
+ writer_->SetWritable();
+ connection_.OnCanWrite();
+ 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_buffer.data(),
+ &(writer_->path_response_frames().front().data_buffer),
+ sizeof(path_frame_buffer)));
+ EXPECT_EQ(1u, writer_->padding_frames().size());
+ // PATH_RESPONSE should be sent in another packet to a different peer
+ // address.
+ EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
+ EXPECT_TRUE(QuicConnectionPeer::pending_path_challenge_payloads(&connection_)
+ .empty());
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index c06d167..74c96ac 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -2037,5 +2037,29 @@
packet_.encryption_level = level;
}
+bool QuicPacketCreator::AddPathResponseFrame(
+ const QuicPathFrameBuffer& data_buffer) {
+ auto path_response =
+ new QuicPathResponseFrame(kInvalidControlFrameId, data_buffer);
+ QuicFrame frame(path_response);
+ if (HasPendingFrames()) {
+ if (AddPaddedSavedFrame(frame, NOT_RETRANSMISSION)) {
+ // Frame is queued.
+ return true;
+ }
+ }
+ // Frame was not queued but queued frames were flushed.
+ DCHECK(!HasPendingFrames());
+ if (!delegate_->ShouldGeneratePacket(NO_RETRANSMITTABLE_DATA,
+ NOT_HANDSHAKE)) {
+ QUIC_DVLOG(1) << ENDPOINT << "Can't send PATH_RESPONSE now";
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_send_path_response, 5, 5);
+ delete path_response;
+ return false;
+ }
+ bool success = AddPaddedSavedFrame(frame, NOT_RETRANSMISSION);
+ QUIC_BUG_IF(!success);
+ return true;
+}
#undef ENDPOINT // undef for jumbo builds
} // namespace quic
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h
index a82cec4..edb428b 100644
--- a/quic/core/quic_packet_creator.h
+++ b/quic/core/quic_packet_creator.h
@@ -255,6 +255,9 @@
const QuicCircularDeque<QuicPathFrameBuffer>& payloads,
const bool is_padded);
+ // Add PATH_RESPONSE to current packet, flush before or afterwards if needed.
+ bool AddPathResponseFrame(const QuicPathFrameBuffer& data_buffer);
+
// Returns a dummy packet that is valid but contains no useful information.
static SerializedPacket NoPacket();
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index aa7c491..cce8fa5 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -464,7 +464,14 @@
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_->SendConnectivityProbingResponsePacket(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 62c012c..38457d6 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -1460,6 +1460,10 @@
// 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())) {
+ return;
+ }
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
QuicSocketAddress old_peer_address =
QuicSocketAddress(QuicIpAddress::Loopback4(), kTestPort);
@@ -1472,10 +1476,17 @@
QuicConnectionPeer::GetWriter(session_.connection()));
EXPECT_CALL(*writer, WritePacket(_, _, _, new_peer_address, _))
.WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- EXPECT_CALL(*connection_, SendConnectivityProbingResponsePacket(_))
- .WillOnce(Invoke(
- connection_,
- &MockQuicConnection::ReallySendConnectivityProbingResponsePacket));
+ 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.
@@ -1491,7 +1502,8 @@
// packet, the response has both of them AND we do not do migration. This for
// IETF QUIC only.
TEST_P(QuicSessionTestServer, ServerReplyToConnectivityProbes) {
- if (!VersionHasIetfQuicFrames(transport_version())) {
+ if (connection_->send_path_response() ||
+ !VersionHasIetfQuicFrames(transport_version())) {
return;
}
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
diff --git a/quic/core/quic_utils.cc b/quic/core/quic_utils.cc
index 13f6a2a..b6619d4 100644
--- a/quic/core/quic_utils.cc
+++ b/quic/core/quic_utils.cc
@@ -305,6 +305,9 @@
case PADDING_FRAME:
case STOP_WAITING_FRAME:
case MTU_DISCOVERY_FRAME:
+ case PATH_CHALLENGE_FRAME:
+ case PATH_RESPONSE_FRAME:
+ case NEW_CONNECTION_ID_FRAME:
return false;
default:
return true;
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc
index 6f9c8c6..2e8ca59 100644
--- a/quic/test_tools/quic_connection_peer.cc
+++ b/quic/test_tools/quic_connection_peer.cc
@@ -392,5 +392,12 @@
return connection->undecryptable_packets_.size();
}
+// static
+const QuicCircularDeque<std::pair<QuicPathFrameBuffer, QuicSocketAddress>>&
+QuicConnectionPeer::pending_path_challenge_payloads(
+ QuicConnection* connection) {
+ return connection->pending_path_challenge_payloads_;
+}
+
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/quic_connection_peer.h b/quic/test_tools/quic_connection_peer.h
index 01d16f2..8a285e9 100644
--- a/quic/test_tools/quic_connection_peer.h
+++ b/quic/test_tools/quic_connection_peer.h
@@ -156,6 +156,10 @@
const QuicConnectionId& server_connection_id);
static size_t NumUndecryptablePackets(QuicConnection* connection);
+
+ static const QuicCircularDeque<
+ std::pair<QuicPathFrameBuffer, QuicSocketAddress>>&
+ pending_path_challenge_payloads(QuicConnection* connection);
};
} // namespace test
