Rearrange fields in QuicConnection::ReceivedPacketInfo and eliminate duplicate storage of destination_connection_id.
Changed QuicEcnCodepoint to an enum uint8_t, which fixes an earlier oversight.
Reduces size from 288B to 248B when the second field is finally deprecated; 272B for now.
PiperOrigin-RevId: 841855204
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index 372ed84..c94364b 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -51,6 +51,7 @@
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_notify_ack_listener_earlier, true, true, "If true, call QuicAckListenerInterface::OnPacketAcked() before moving the stream to closed stream list.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_notify_stream_soon_to_destroy, true, true, "If true, notify each QUIC stream before it gets destroyed and update ACK listener before that.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_on_packet_header_return_connected, false, true, "If true, QuicConnection::OnPacketHeader will return connected_ at the end of the function.")
+QUICHE_FLAG(bool, quiche_reloadable_flag_quic_one_dcid, false, false, "If true, stores only one copy of the destination connection ID in QuicConnection::ReceivedPacketInfo.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_optimize_qpack_blocking_manager, false, false, "If true, optimize qpack_blocking_manager for CPU efficiency.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_pacing_remove_non_initial_burst, false, false, "If true, remove the non-initial burst in QUIC PacingSender.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_parse_cert_compression_algos_from_chlo, true, true, "If true, parse offered cert compression algorithms from received CHLOs.")
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index 2015e27..57b9181 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -228,7 +228,8 @@
received_client_addresses_cache_(kMaxReceivedClientAddressSize),
perspective_(perspective),
owns_writer_(owns_writer),
- can_truncate_connection_ids_(perspective == Perspective::IS_SERVER) {
+ can_truncate_connection_ids_(perspective == Perspective::IS_SERVER),
+ store_one_dcid_(GetQuicReloadableFlag(quic_one_dcid)) {
QUICHE_DCHECK(perspective_ == Perspective::IS_CLIENT ||
default_path_.self_address.IsInitialized());
@@ -987,17 +988,33 @@
bool QuicConnection::OnUnauthenticatedPublicHeader(
const QuicPacketHeader& header) {
- last_received_packet_info_.destination_connection_id =
- header.destination_connection_id;
- // If last packet destination connection ID is the original server
- // connection ID chosen by client, replaces it with the connection ID chosen
- // by server.
- if (perspective_ == Perspective::IS_SERVER &&
- original_destination_connection_id_.has_value() &&
- last_received_packet_info_.destination_connection_id ==
- *original_destination_connection_id_) {
+ if (store_one_dcid_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_one_dcid, 2, 3);
+ last_received_packet_info_.header.destination_connection_id =
+ header.destination_connection_id;
+ // If last packet destination connection ID is the original server
+ // connection ID chosen by client, replaces it with the connection ID chosen
+ // by server.
+ if (perspective_ == Perspective::IS_SERVER &&
+ original_destination_connection_id_.has_value() &&
+ last_received_packet_info_.header.destination_connection_id ==
+ *original_destination_connection_id_) {
+ last_received_packet_info_.header.destination_connection_id =
+ original_destination_connection_id_replacement_;
+ }
+ } else {
last_received_packet_info_.destination_connection_id =
- original_destination_connection_id_replacement_;
+ header.destination_connection_id;
+ // If last packet destination connection ID is the original server
+ // connection ID chosen by client, replaces it with the connection ID chosen
+ // by server.
+ if (perspective_ == Perspective::IS_SERVER &&
+ original_destination_connection_id_.has_value() &&
+ last_received_packet_info_.destination_connection_id ==
+ *original_destination_connection_id_) {
+ last_received_packet_info_.destination_connection_id =
+ original_destination_connection_id_replacement_;
+ }
}
// As soon as we receive an initial we start ignoring subsequent retries.
@@ -1233,6 +1250,15 @@
}
}
+ if (store_one_dcid_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_one_dcid, 3, 3);
+ // Save the stored destination connection ID, in case it was substituted.
+ QuicConnectionId destination_connection_id =
+ last_received_packet_info_.header.destination_connection_id;
+ last_received_packet_info_.header = header;
+ last_received_packet_info_.header.destination_connection_id =
+ destination_connection_id;
+ }
if (perspective_ == Perspective::IS_CLIENT) {
if (!GetLargestReceivedPacket().IsInitialized() ||
header.packet_number > GetLargestReceivedPacket()) {
@@ -1276,34 +1302,33 @@
// connection ID set on path.
// 1) If client uses 1 unique server connection ID per path and the packet
// is received from an existing path, then
- // last_received_packet_info_.destination_connection_id will always be the
- // same as the server connection ID on path. Server side will maintain the
- // 1-to-1 mapping from server connection ID to path. 2) If client uses
- // multiple server connection IDs on the same path, compared to the
- // server_connection_id on path,
- // last_received_packet_info_.destination_connection_id has the advantage
- // that it is still present in the session map since the packet can be
- // routed here regardless of packet reordering.
+ // last_received_packet_info_.header.destination_connection_id will always
+ // be the same as the server connection ID on path. Server side will
+ // maintain the 1-to-1 mapping from server connection ID to path. 2) If
+ // client uses multiple server connection IDs on the same path, compared
+ // to the server_connection_id on path,
+ // last_received_packet_info_.header.destination_connection_id has the
+ // advantage that it is still present in the session map since the packet
+ // can be routed here regardless of packet reordering.
if (IsDefaultPath(last_received_packet_info_.destination_address,
effective_peer_address)) {
default_path_.server_connection_id =
- last_received_packet_info_.destination_connection_id;
+ GetDestinationConnectionId(last_received_packet_info_);
} else if (IsAlternativePath(
last_received_packet_info_.destination_address,
effective_peer_address)) {
alternative_path_.server_connection_id =
- last_received_packet_info_.destination_connection_id;
+ GetDestinationConnectionId(last_received_packet_info_);
}
}
- if (last_received_packet_info_.destination_connection_id !=
+ if (GetDestinationConnectionId(last_received_packet_info_) !=
default_path_.server_connection_id &&
(!original_destination_connection_id_.has_value() ||
- last_received_packet_info_.destination_connection_id !=
+ GetDestinationConnectionId(last_received_packet_info_) !=
*original_destination_connection_id_)) {
QUIC_CODE_COUNT(quic_connection_id_change);
}
-
QUIC_DLOG_IF(INFO, current_effective_peer_migration_type_ != NO_CHANGE)
<< ENDPOINT << "Effective peer's ip:port changed from "
<< default_path_.peer_address.ToString() << " to "
@@ -1314,7 +1339,19 @@
--stats_.packets_dropped;
QUIC_DVLOG(1) << ENDPOINT << "Received packet header: " << header;
- last_received_packet_info_.header = header;
+ if (store_one_dcid_) {
+ // last_received_packet_info_.header.destination_connection_id will often
+ // be different from header.destination_connection_id, so we can't simply
+ // compare the two headers.
+ QUIC_BUG_IF(
+ quic_bug_header_mismatch,
+ last_received_packet_info_.header.packet_number !=
+ header.packet_number ||
+ last_received_packet_info_.header.type_byte != header.type_byte)
+ << "last_received_packet_info.header not assigned";
+ } else {
+ last_received_packet_info_.header = header;
+ }
if (!stats_.first_decrypted_packet.IsInitialized()) {
stats_.first_decrypted_packet =
last_received_packet_info_.header.packet_number;
@@ -5071,8 +5108,8 @@
os << " { destination_address: " << info.destination_address.ToString()
<< ", source_address: " << info.source_address.ToString()
<< ", received_bytes_counted: " << info.received_bytes_counted
- << ", length: " << info.length
- << ", destination_connection_id: " << info.destination_connection_id;
+ << ", length: " << info.length << ", destination_connection_id: "
+ << info.header.destination_connection_id;
if (!info.decrypted) {
os << " }\n";
return os;
@@ -5433,12 +5470,12 @@
std::optional<StatelessResetToken> stateless_reset_token;
FindMatchingOrNewClientConnectionIdOrToken(
previous_default_path, alternative_path_,
- last_received_packet_info_.destination_connection_id,
+ GetDestinationConnectionId(last_received_packet_info_),
&client_connection_id, &stateless_reset_token);
SetDefaultPathState(
PathState(last_received_packet_info_.destination_address,
current_effective_peer_address, client_connection_id,
- last_received_packet_info_.destination_connection_id,
+ GetDestinationConnectionId(last_received_packet_info_),
stateless_reset_token));
// The path is considered validated if its peer IP address matches any
// validated path's peer IP address.
@@ -5636,15 +5673,15 @@
std::optional<StatelessResetToken> stateless_reset_token;
FindMatchingOrNewClientConnectionIdOrToken(
default_path_, alternative_path_,
- last_received_packet_info_.destination_connection_id,
+ GetDestinationConnectionId(last_received_packet_info_),
&client_connection_id, &stateless_reset_token);
- // Only override alternative path state upon receiving a PATH_CHALLENGE
- // from an unvalidated peer address, and the connection isn't validating
- // a recent peer migration.
+ // Only override alternative path state upon receiving a
+ // PATH_CHALLENGE from an unvalidated peer address, and the connection
+ // isn't validating a recent peer migration.
alternative_path_ =
PathState(last_received_packet_info_.destination_address,
current_effective_peer_address, client_connection_id,
- last_received_packet_info_.destination_connection_id,
+ GetDestinationConnectionId(last_received_packet_info_),
stateless_reset_token);
should_proactively_validate_peer_address_on_path_challenge_ = true;
}
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h
index 2539953..a4d3d15 100644
--- a/quiche/quic/core/quic_connection.h
+++ b/quiche/quic/core/quic_connection.h
@@ -1698,22 +1698,30 @@
QuicSocketAddress destination_address;
QuicSocketAddress source_address;
QuicTime receipt_time = QuicTime::Zero();
- bool received_bytes_counted = false;
QuicByteCount length = 0;
- QuicConnectionId destination_connection_id;
- // Fields below are only populated if packet gets decrypted successfully.
+ // END FIRST CACHELINE
+ // Fields below are only populated if packet gets decrypted successfully,
+ // except received_bytes_counted and header.destination_connection_id.
// TODO(fayang): consider using std::optional for following fields.
+ QuicPacketHeader header; // Placed to fall on the cacheline boundary, as it
+ // fills two full cachelines including padding.
+ // END THIRD CACHELINE
+ bool received_bytes_counted = false;
bool decrypted = false;
- EncryptionLevel decrypted_level = ENCRYPTION_INITIAL;
- QuicPacketHeader header;
- absl::InlinedVector<QuicFrameType, 1> frames;
QuicEcnCodepoint ecn_codepoint = ECN_NOT_ECT;
+ EncryptionLevel decrypted_level = ENCRYPTION_INITIAL;
uint32_t flow_label = 0;
+ absl::InlinedVector<QuicFrameType, 1> frames;
// Stores the actual address this packet is received on when it is received
// on the preferred address. In this case, |destination_address| will
// be overridden to the current default self address.
QuicSocketAddress actual_destination_address;
+ // 8B remaining in the fourth cacheline.
+ // TODO(martinduke): Remove once gfe2_reloadable_flag_quic_one_dcid is
+ // deprecated.
+ QuicConnectionId destination_connection_id;
};
+ static_assert(offsetof(ReceivedPacketInfo, received_bytes_counted) <= 192);
QUICHE_EXPORT friend std::ostream& operator<<(
std::ostream& os, const QuicConnection::ReceivedPacketInfo& info);
@@ -2171,6 +2179,16 @@
return QuicAlarmProxy(&alarms_, QuicAlarmSlot::kMultiPortProbing);
}
+ // Extract destination connection ID from ReceivedPacketInfo.
+ inline QuicConnectionId GetDestinationConnectionId(
+ const ReceivedPacketInfo& packet_info) const {
+ if (store_one_dcid_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_one_dcid, 1, 3);
+ return packet_info.header.destination_connection_id;
+ }
+ return packet_info.destination_connection_id;
+ }
+
QuicConnectionContext context_;
QuicFramer framer_;
@@ -2530,6 +2548,10 @@
// If true then flow labels will be changed when a PTO fires, or when
// a PTO'd packet from a peer is detected.
bool enable_black_hole_avoidance_via_flow_label_ : 1 = false;
+ // If true, stores only one copy of the destination connection ID in
+ // ReceivedPacketInfo.
+ const bool store_one_dcid_ : 1;
+
const bool quic_limit_new_streams_per_loop_2_ : 1 =
GetQuicReloadableFlag(quic_limit_new_streams_per_loop_2);
const bool quic_test_peer_addr_change_after_normalize_ : 1 =
diff --git a/quiche/quic/test_tools/quic_connection_peer.cc b/quiche/quic/test_tools/quic_connection_peer.cc
index 9e94e70..1e549f9 100644
--- a/quiche/quic/test_tools/quic_connection_peer.cc
+++ b/quiche/quic/test_tools/quic_connection_peer.cc
@@ -567,7 +567,7 @@
<< " ecn_codepoint passed: " << (info.ecn_codepoint == ECN_NOT_ECT)
<< " sizeof(ReceivedPacketInfo) passed: "
<< (sizeof(size_t) != 8 ||
- sizeof(QuicConnection::ReceivedPacketInfo) == 288);
+ sizeof(QuicConnection::ReceivedPacketInfo) == 272);
return info.destination_address == QuicSocketAddress() &&
info.source_address == QuicSocketAddress() &&
info.receipt_time == QuicTime::Zero() &&
@@ -581,7 +581,7 @@
// have changed. Please add the relevant conditions and update the
// length below.
(sizeof(size_t) != 8 ||
- sizeof(QuicConnection::ReceivedPacketInfo) == 288);
+ sizeof(QuicConnection::ReceivedPacketInfo) == 272);
}
// static
