When server tries to send a packet from original address to a client address (say A), override the packet's self address to server preferred address if server has not received packet from A on the original address.
Protected by FLAGS_quic_reloadable_flag_quic_use_received_client_addresses_cache.
PiperOrigin-RevId: 518029299
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index 75872f4..11d0820 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -441,6 +441,7 @@
StartServer();
if (use_preferred_address_) {
+ SetQuicReloadableFlag(quic_use_received_client_addresses_cache, true);
// At this point, the server has an ephemeral port to listen on. Restart
// the server with the preferred address.
StopServer();
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index 659821a..9994e09 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -64,6 +64,9 @@
// The minimum release time into future in ms.
const int kMinReleaseTimeIntoFutureMs = 1;
+// The maximum number of recorded client addresses.
+const size_t kMaxReceivedClientAddressSize = 20;
+
// Base class of all alarms owned by a QuicConnection.
class QuicConnectionAlarmDelegate : public QuicAlarm::Delegate {
public:
@@ -338,7 +341,8 @@
&context_),
ping_manager_(perspective, this, &arena_, alarm_factory_, &context_),
multi_port_probing_interval_(kDefaultMultiPortProbingInterval),
- connection_id_generator_(generator) {
+ connection_id_generator_(generator),
+ received_client_addresses_cache_(kMaxReceivedClientAddressSize) {
QUICHE_DCHECK(perspective_ == Perspective::IS_CLIENT ||
default_path_.self_address.IsInitialized());
@@ -3017,6 +3021,17 @@
default_path_.self_address = last_received_packet_info_.destination_address;
}
+ if (GetQuicReloadableFlag(quic_use_received_client_addresses_cache) &&
+ perspective_ == Perspective::IS_SERVER &&
+ !last_received_packet_info_.actual_destination_address.IsInitialized() &&
+ last_received_packet_info_.source_address.IsInitialized()) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_use_received_client_addresses_cache);
+ // Record client address of packets received on server original address.
+ received_client_addresses_cache_.Insert(
+ last_received_packet_info_.source_address,
+ std::make_unique<bool>(true));
+ }
+
if (perspective_ == Perspective::IS_SERVER &&
last_received_packet_info_.actual_destination_address.IsInitialized() &&
!IsHandshakeConfirmed() &&
@@ -3399,6 +3414,19 @@
QuicTime packet_send_time = CalculatePacketSentTime();
WriteResult result(WRITE_STATUS_OK, encrypted_length);
QuicSocketAddress send_to_address = packet->peer_address;
+ QuicSocketAddress send_from_address = self_address();
+ if (perspective_ == Perspective::IS_SERVER &&
+ sent_server_preferred_address_.IsInitialized() &&
+ received_client_addresses_cache_.Lookup(send_to_address) ==
+ received_client_addresses_cache_.end()) {
+ // Given server has not received packets from send_to_address to
+ // self_address(), most NATs do not allow packets from self_address() to
+ // send_to_address to go through. Override packet's self address to
+ // sent_server_preferred_address_.
+ // TODO(b/262386897): server should validate reverse path before changing
+ // self address of packets to send.
+ send_from_address = sent_server_preferred_address_;
+ }
// Self address is always the default self address on this code path.
const bool send_on_current_path = send_to_address == peer_address();
if (!send_on_current_path) {
@@ -3422,7 +3450,7 @@
QUIC_BUG_IF(quic_bug_12714_24,
!version().CanSendCoalescedPackets() || coalescing_done_);
if (!coalesced_packet_.MaybeCoalescePacket(
- *packet, self_address(), send_to_address,
+ *packet, send_from_address, send_to_address,
helper_->GetStreamSendBufferAllocator(),
packet_creator_.max_packet_length())) {
// Failed to coalesce packet, flush current coalesced packet.
@@ -3435,7 +3463,7 @@
return false;
}
if (!coalesced_packet_.MaybeCoalescePacket(
- *packet, self_address(), send_to_address,
+ *packet, send_from_address, send_to_address,
helper_->GetStreamSendBufferAllocator(),
packet_creator_.max_packet_length())) {
// Failed to coalesce packet even it is the only packet, raise a write
@@ -3456,7 +3484,8 @@
case BUFFER:
QUIC_DVLOG(1) << ENDPOINT << "Adding packet: " << packet->packet_number
<< " to buffered packets";
- buffered_packets_.emplace_back(*packet, self_address(), send_to_address);
+ buffered_packets_.emplace_back(*packet, send_from_address,
+ send_to_address);
break;
case SEND_TO_WRITER:
// Stop using coalescer from now on.
@@ -3470,7 +3499,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(), send_to_address,
+ send_from_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
@@ -3505,7 +3534,8 @@
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(), send_to_address);
+ buffered_packets_.emplace_back(*packet, send_from_address,
+ send_to_address);
}
}
@@ -3533,9 +3563,9 @@
if (IsWriteError(result.status)) {
QUIC_LOG_FIRST_N(ERROR, 10)
<< ENDPOINT << "Failed writing packet " << packet_number << " of "
- << encrypted_length << " bytes from " << self_address().host() << " to "
- << send_to_address << ", with error code " << result.error_code
- << ". long_term_mtu_:" << long_term_mtu_
+ << encrypted_length << " bytes from " << send_from_address.host()
+ << " to " << 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()
<< ", is_mtu_discovery:" << is_mtu_discovery;
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h
index bb6de27..d0d1655 100644
--- a/quiche/quic/core/quic_connection.h
+++ b/quiche/quic/core/quic_connection.h
@@ -45,6 +45,7 @@
#include "quiche/quic/core/quic_constants.h"
#include "quiche/quic/core/quic_framer.h"
#include "quiche/quic/core/quic_idle_network_detector.h"
+#include "quiche/quic/core/quic_lru_cache.h"
#include "quiche/quic/core/quic_mtu_discovery.h"
#include "quiche/quic/core/quic_network_blackhole_detector.h"
#include "quiche/quic/core/quic_one_block_arena.h"
@@ -2356,6 +2357,11 @@
// peer. For now, this is only stored for tests.
QuicEcnCounts
peer_ack_ecn_counts_[PacketNumberSpace::NUM_PACKET_NUMBER_SPACES];
+
+ // This LRU cache records source addresses of packets received on server's
+ // original address.
+ QuicLRUCache<QuicSocketAddress, bool, QuicSocketAddressHash>
+ received_client_addresses_cache_;
};
} // namespace quic
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index e0cd6e3..d3ff120 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -1497,6 +1497,7 @@
connection_.CreateConnectionIdManager();
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
SetQuicReloadableFlag(quic_connection_migration_use_new_cid_v2, true);
+ SetQuicReloadableFlag(quic_use_received_client_addresses_cache, true);
EXPECT_CALL(visitor_, AllowSelfAddressChange())
.WillRepeatedly(Return(true));
@@ -2791,8 +2792,11 @@
.WillOnce(Invoke([&]() {
EXPECT_EQ(1u, writer_->path_response_frames().size());
EXPECT_EQ(1u, writer_->path_challenge_frames().size());
- EXPECT_EQ(kSelfAddress.host(), writer_->last_write_source_address());
- // The responses should still be sent from the original address.
+ EXPECT_EQ(kServerPreferredAddress.host(),
+ writer_->last_write_source_address());
+ // The responses should be sent from preferred address given server
+ // has not received packet on original address from the new client
+ // address.
EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
}));
ProcessReceivedPacket(kServerPreferredAddress, kNewPeerAddress, *received);
@@ -17413,8 +17417,11 @@
.WillOnce(Invoke([&]() {
EXPECT_EQ(1u, writer_->path_response_frames().size());
EXPECT_EQ(1u, writer_->path_challenge_frames().size());
- // The responses should still be sent from the original address.
- EXPECT_EQ(kSelfAddress.host(), writer_->last_write_source_address());
+ // The responses should be sent from preferred address given server
+ // has not received packet on original address from the new client
+ // address.
+ EXPECT_EQ(kServerPreferredAddress.host(),
+ writer_->last_write_source_address());
EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
}));
ProcessReceivedPacket(kServerPreferredAddress, kNewPeerAddress, *received);
@@ -17451,7 +17458,8 @@
ProcessFramePacketWithAddresses(MakeCryptoFrame(), kServerPreferredAddress,
kNewPeerAddress, ENCRYPTION_FORWARD_SECURE);
EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
- EXPECT_EQ(kSelfAddress.host(), writer_->last_write_source_address());
+ EXPECT_EQ(kServerPreferredAddress.host(),
+ writer_->last_write_source_address());
EXPECT_EQ(kSelfAddress, connection_.self_address());
}
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index 08b6902..399b2ec 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -75,6 +75,8 @@
QUIC_FLAG(quic_reloadable_flag_quic_conservative_bursts, false)
// If true, use BBRv2 as the default congestion controller. Takes precedence over --quic_default_to_bbr.
QUIC_FLAG(quic_reloadable_flag_quic_default_to_bbr_v2, false)
+// If true, use a LRU cache to record client addresses of packets received on server\'s original address.
+QUIC_FLAG(quic_reloadable_flag_quic_use_received_client_addresses_cache, true)
// If true, use new connection ID in connection migration.
QUIC_FLAG(quic_reloadable_flag_quic_connection_migration_use_new_cid_v2, true)
// If true, use next_connection_id_sequence_number to validate retired cid number.
