Consult self_issued_connection_id_manager instead of default_path_ for connection ID to look up session in session map.
As a bug fix, gfe2_reloadable_flag_quic_use_active_cid_for_session_lookup is default enabled.
Protected by FLAGS_quic_reloadable_flag_quic_use_active_cid_for_session_lookup.
PiperOrigin-RevId: 389761333
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 12bf186..0e309e2 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -47,6 +47,7 @@
#include "quic/platform/api/quic_logging.h"
#include "quic/platform/api/quic_server_stats.h"
#include "quic/platform/api/quic_socket_address.h"
+#include "common/platform/api/quiche_flag_utils.h"
#include "common/quiche_text_utils.h"
namespace quic {
@@ -6872,6 +6873,25 @@
RetirePeerIssuedConnectionIdsNoLongerOnPath();
}
+QuicConnectionId QuicConnection::GetOneActiveServerConnectionId() const {
+ if (perspective_ == Perspective::IS_CLIENT ||
+ self_issued_cid_manager_ == nullptr ||
+ !use_active_cid_for_session_lookup_) {
+ return connection_id();
+ }
+ auto active_connection_ids = GetActiveServerConnectionIds();
+ QUIC_BUG_IF(quic_bug_6944, active_connection_ids.empty());
+ if (active_connection_ids.empty() ||
+ std::find(active_connection_ids.begin(), active_connection_ids.end(),
+ connection_id()) != active_connection_ids.end()) {
+ return connection_id();
+ }
+ QUICHE_CODE_COUNT(connection_id_on_default_path_has_been_retired);
+ auto active_connection_id =
+ self_issued_cid_manager_->GetOneActiveConnectionId();
+ return active_connection_id;
+}
+
std::vector<QuicConnectionId> QuicConnection::GetActiveServerConnectionIds()
const {
if (!support_multiple_connection_ids_ ||
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index dea513a..6ed3f5a 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -778,9 +778,12 @@
const QuicSocketAddress& effective_peer_address() const {
return default_path_.peer_address;
}
+
+ // Returns the server connection ID used on the default path.
const QuicConnectionId& connection_id() const {
return default_path_.server_connection_id;
}
+
const QuicConnectionId& client_connection_id() const {
return default_path_.client_connection_id;
}
@@ -1225,6 +1228,12 @@
SendPingAtLevel(framer().GetEncryptionLevelToSendApplicationData());
}
+ // Returns one server connection ID that associates the current session in the
+ // session map.
+ virtual QuicConnectionId GetOneActiveServerConnectionId() const;
+
+ // Returns all server connection IDs that have not been removed from the
+ // session map.
virtual std::vector<QuicConnectionId> GetActiveServerConnectionIds() const;
bool validate_client_address() const { return validate_client_addresses_; }
@@ -1237,6 +1246,10 @@
return connection_migration_use_new_cid_;
}
+ bool use_active_cid_for_session_lookup() const {
+ return use_active_cid_for_session_lookup_;
+ }
+
bool count_bytes_on_alternative_path_separately() const {
return count_bytes_on_alternative_path_separately_;
}
@@ -2279,6 +2292,9 @@
GetQuicReloadableFlag(quic_add_missing_update_ack_timeout);
const bool ack_cid_frames_ = GetQuicReloadableFlag(quic_ack_cid_frames);
+
+ const bool use_active_cid_for_session_lookup_ =
+ GetQuicReloadableFlag(quic_use_active_cid_for_session_lookup);
};
} // namespace quic
diff --git a/quic/core/quic_connection_id_manager.cc b/quic/core/quic_connection_id_manager.cc
index f7089b7..c7e8972 100644
--- a/quic/core/quic_connection_id_manager.cc
+++ b/quic/core/quic_connection_id_manager.cc
@@ -3,12 +3,14 @@
// found in the LICENSE file.
#include "quic/core/quic_connection_id_manager.h"
+
#include <cstdio>
#include "quic/core/quic_clock.h"
#include "quic/core/quic_connection_id.h"
#include "quic/core/quic_error_codes.h"
#include "quic/core/quic_utils.h"
+#include "common/platform/api/quiche_logging.h"
namespace quic {
@@ -372,6 +374,12 @@
return unretired_ids;
}
+QuicConnectionId QuicSelfIssuedConnectionIdManager::GetOneActiveConnectionId()
+ const {
+ QUICHE_DCHECK(!active_connection_ids_.empty());
+ return active_connection_ids_.front().first;
+}
+
void QuicSelfIssuedConnectionIdManager::RetireConnectionId() {
if (to_be_retired_connection_ids_.empty()) {
QUIC_BUG(quic_bug_12420_1)
diff --git a/quic/core/quic_connection_id_manager.h b/quic/core/quic_connection_id_manager.h
index 5e7fc8c..775c016 100644
--- a/quic/core/quic_connection_id_manager.h
+++ b/quic/core/quic_connection_id_manager.h
@@ -139,6 +139,8 @@
std::vector<QuicConnectionId> GetUnretiredConnectionIds() const;
+ QuicConnectionId GetOneActiveConnectionId() const;
+
// Called when the retire_connection_id alarm_ fires. Removes the to be
// retired connection ID locally.
void RetireConnectionId();
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 6966630..8466ab0 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -826,10 +826,8 @@
level);
}
- void ProcessFramesPacketWithAddresses(QuicFrames frames,
- QuicSocketAddress self_address,
- QuicSocketAddress peer_address,
- EncryptionLevel level) {
+ QuicReceivedPacket ConstructPacket(QuicFrames frames, EncryptionLevel level,
+ char* buffer, size_t buffer_len) {
QUICHE_DCHECK(peer_framer_.HasEncrypterOfEncryptionLevel(level));
peer_creator_.set_encryption_level(level);
QuicPacketCreatorPeer::SetSendVersionInPacket(
@@ -837,14 +835,22 @@
level < ENCRYPTION_FORWARD_SECURE &&
connection_.perspective() == Perspective::IS_SERVER);
- char buffer[kMaxOutgoingPacketSize];
SerializedPacket serialized_packet =
- QuicPacketCreatorPeer::SerializeAllFrames(
- &peer_creator_, frames, buffer, kMaxOutgoingPacketSize);
+ QuicPacketCreatorPeer::SerializeAllFrames(&peer_creator_, frames,
+ buffer, buffer_len);
+ return QuicReceivedPacket(serialized_packet.encrypted_buffer,
+ serialized_packet.encrypted_length, clock_.Now());
+ }
+
+ void ProcessFramesPacketWithAddresses(QuicFrames frames,
+ QuicSocketAddress self_address,
+ QuicSocketAddress peer_address,
+ EncryptionLevel level) {
+ char buffer[kMaxOutgoingPacketSize];
connection_.ProcessUdpPacket(
self_address, peer_address,
- QuicReceivedPacket(serialized_packet.encrypted_buffer,
- serialized_packet.encrypted_length, clock_.Now()));
+ ConstructPacket(std::move(frames), level, buffer,
+ kMaxOutgoingPacketSize));
if (connection_.GetSendAlarm()->IsSet()) {
connection_.GetSendAlarm()->Fire();
}
@@ -14793,11 +14799,13 @@
}
TEST_P(QuicConnectionTest, ServerRetireSelfIssuedConnectionId) {
- if (!version().HasIetfQuicFrames() ||
- !connection_.connection_migration_use_new_cid()) {
+ QuicConfig config;
+ config.SetConnectionOptionsToSend({kRVCM});
+ EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+ connection_.SetFromConfig(config);
+ if (!connection_.connection_migration_use_new_cid()) {
return;
}
- QuicConnectionPeer::EnableMultipleConnectionIdSupport(&connection_);
set_perspective(Perspective::IS_SERVER);
connection_.CreateConnectionIdManager();
QuicConnectionId recorded_cid;
@@ -14807,7 +14815,10 @@
QuicConnectionId cid0 = connection_id_;
QuicConnectionId cid1;
QuicConnectionId cid2;
+ EXPECT_EQ(connection_.connection_id(), cid0);
+ EXPECT_EQ(connection_.GetOneActiveServerConnectionId(), cid0);
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
.WillOnce(Invoke(cid_recorder));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
@@ -14818,23 +14829,71 @@
connection_.GetRetireSelfIssuedConnectionIdAlarm();
ASSERT_FALSE(retire_self_issued_cid_alarm->IsSet());
- QuicRetireConnectionIdFrame frame;
- frame.sequence_number = 0u;
+ // Generate three packets with different connection IDs that will arrive out
+ // of order (2, 1, 3) later.
+ char buffers[3][kMaxOutgoingPacketSize];
+ // Destination connection ID of packet1 is cid0.
+ auto packet1 =
+ ConstructPacket({QuicFrame(QuicPingFrame())}, ENCRYPTION_FORWARD_SECURE,
+ buffers[0], kMaxOutgoingPacketSize);
+ peer_creator_.SetServerConnectionId(cid1);
+ auto retire_cid_frame = std::make_unique<QuicRetireConnectionIdFrame>();
+ retire_cid_frame->sequence_number = 0u;
+ // Destination connection ID of packet2 is cid1.
+ auto packet2 = ConstructPacket({QuicFrame(retire_cid_frame.release())},
+ ENCRYPTION_FORWARD_SECURE, buffers[1],
+ kMaxOutgoingPacketSize);
+ // Destination connection ID of packet3 is cid1.
+ auto packet3 =
+ ConstructPacket({QuicFrame(QuicPingFrame())}, ENCRYPTION_FORWARD_SECURE,
+ buffers[2], kMaxOutgoingPacketSize);
+
+ // Packet2 with RetireConnectionId frame trigers sending NewConnectionId
+ // immediately.
EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
.WillOnce(Invoke(cid_recorder));
- // RetireConnectionId trigers sending NewConnectionId immediately.
EXPECT_CALL(visitor_, SendNewConnectionId(_));
- EXPECT_TRUE(connection_.OnRetireConnectionIdFrame(frame));
+ peer_creator_.SetServerConnectionId(cid1);
+ connection_.ProcessUdpPacket(kSelfAddress, kPeerAddress, packet2);
cid2 = recorded_cid;
// cid0 is not retired immediately.
EXPECT_THAT(connection_.GetActiveServerConnectionIds(),
ElementsAre(cid0, cid1, cid2));
ASSERT_TRUE(retire_self_issued_cid_alarm->IsSet());
+ EXPECT_EQ(connection_.connection_id(), cid1);
+ EXPECT_TRUE(connection_.GetOneActiveServerConnectionId() == cid0 ||
+ connection_.GetOneActiveServerConnectionId() == cid1 ||
+ connection_.GetOneActiveServerConnectionId() == cid2);
+
+ // Packet1 updates the connection ID on the default path but not the active
+ // connection ID.
+ connection_.ProcessUdpPacket(kSelfAddress, kPeerAddress, packet1);
+ EXPECT_EQ(connection_.connection_id(), cid0);
+ if (connection_.use_active_cid_for_session_lookup()) {
+ EXPECT_TRUE(connection_.GetOneActiveServerConnectionId() == cid0 ||
+ connection_.GetOneActiveServerConnectionId() == cid1 ||
+ connection_.GetOneActiveServerConnectionId() == cid2);
+ } else {
+ EXPECT_EQ(connection_.GetOneActiveServerConnectionId(), cid0);
+ }
+
// cid0 is retired when the retire CID alarm fires.
EXPECT_CALL(visitor_, OnServerConnectionIdRetired(cid0));
retire_self_issued_cid_alarm->Fire();
EXPECT_THAT(connection_.GetActiveServerConnectionIds(),
ElementsAre(cid1, cid2));
+ if (connection_.use_active_cid_for_session_lookup()) {
+ EXPECT_TRUE(connection_.GetOneActiveServerConnectionId() == cid1 ||
+ connection_.GetOneActiveServerConnectionId() == cid2);
+ } else {
+ EXPECT_EQ(connection_.GetOneActiveServerConnectionId(), cid0);
+ }
+
+ // Packet3 updates the connection ID on the default path.
+ connection_.ProcessUdpPacket(kSelfAddress, kPeerAddress, packet3);
+ EXPECT_EQ(connection_.connection_id(), cid1);
+ EXPECT_TRUE(connection_.GetOneActiveServerConnectionId() == cid1 ||
+ connection_.GetOneActiveServerConnectionId() == cid2);
}
TEST_P(QuicConnectionTest, PatchMissingClientConnectionIdOntoAlternativePath) {
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 509b14d..b57fad2 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -77,6 +77,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_add_stream_info_to_idle_close_detail, true)
// If true, increase the size of stream sequencer buffer block container on demand.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_allocate_stream_sequencer_buffer_blocks_on_demand, false)
+// If true, looks up session to close via connection ID managed by self_issued_connection_id_manager instead of the one on default path.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_active_cid_for_session_lookup, true)
// If true, pass the received PATH_RESPONSE payload to path validator to move forward the path validation.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_pass_path_response_to_validator, true)
// If true, quic connection sends/recieives NewConnectionId & RetireConnectionId frames.
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index aef3fc2..193783a 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -444,7 +444,7 @@
closed_streams_clean_up_alarm_->Cancel();
if (visitor_) {
- visitor_->OnConnectionClosed(connection_->connection_id(),
+ visitor_->OnConnectionClosed(connection_->GetOneActiveServerConnectionId(),
frame.quic_error_code, frame.error_details,
source);
}
@@ -2089,7 +2089,7 @@
void QuicSession::OnServerConnectionIdIssued(
const QuicConnectionId& server_connection_id) {
- visitor_->OnNewConnectionIdSent(connection_->connection_id(),
+ visitor_->OnNewConnectionIdSent(connection_->GetOneActiveServerConnectionId(),
server_connection_id);
}
