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.