From 80bcaf97516f5098212026e23c4406ab60de7c0e Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 11 Aug 2026 16:06:21 +0200 Subject: [PATCH 1/3] Remove dead config records from NetHandler Three proxy.config.net.* records have been unregistered since their introduction in 2015 (TS-3313), so their callbacks never fired and the struct fields they wrote were never read. Removing them shifts default_inactivity_timeout to index 2; the bitset selecting per-thread dependent values is unaffected since those members keep indices 0 and 1. Assertions now pin the field offsets and the bitset width, both of which were previously implicit. Fixes: #12933 --- include/iocore/net/NetHandler.h | 17 +++++++++++------ src/iocore/net/NetHandler.cc | 26 ++------------------------ src/iocore/net/UnixNet.cc | 11 ++++++++++- 3 files changed, 23 insertions(+), 31 deletions(-) diff --git a/include/iocore/net/NetHandler.h b/include/iocore/net/NetHandler.h index 2c84f4e865b..c02a57b8107 100644 --- a/include/iocore/net/NetHandler.h +++ b/include/iocore/net/NetHandler.h @@ -24,6 +24,7 @@ #pragma once #include +#include #include "iocore/eventsystem/Continuation.h" #include "iocore/eventsystem/EThread.h" @@ -111,12 +112,9 @@ class NetHandler : public Continuation, public EThread::LoopTailHandler /// configuration settings for managing the active and keep-alive queues struct Config { - uint32_t max_connections_in = 0; - uint32_t max_requests_in = 0; - uint32_t inactive_threshold_in = 0; - uint32_t transaction_no_activity_timeout_in = 0; - uint32_t keep_alive_no_activity_timeout_in = 0; - uint32_t default_inactivity_timeout = 0; + uint32_t max_connections_in = 0; + uint32_t max_requests_in = 0; + uint32_t default_inactivity_timeout = 0; /** Return the address of the first value in this struct. @@ -130,6 +128,13 @@ class NetHandler : public Continuation, public EThread::LoopTailHandler return *(&max_connections_in + n); } }; + // Config is addressed as an array of uint32_t through operator[], and + // config_value_affects_per_thread_value is a bitset indexed by field + // position, so the offset of each member is part of the interface. + static_assert(offsetof(Config, max_connections_in) == 0 * sizeof(uint32_t)); + static_assert(offsetof(Config, max_requests_in) == 1 * sizeof(uint32_t)); + static_assert(offsetof(Config, default_inactivity_timeout) == 2 * sizeof(uint32_t)); + /** Static global config, set and updated per process. This is updated asynchronously and then events are sent to the NetHandler diff --git a/src/iocore/net/NetHandler.cc b/src/iocore/net/NetHandler.cc index 3fd4b11fe32..7994bac83a7 100644 --- a/src/iocore/net/NetHandler.cc +++ b/src/iocore/net/NetHandler.cc @@ -128,15 +128,6 @@ NetHandler::update_nethandler_config(const char *str, RecDataT, RecData data, vo } else if (name == "proxy.config.net.max_requests_in"sv) { updated_member = &NetHandler::global_config.max_requests_in; Dbg(dbg_ctl_net_queue, "proxy.config.net.max_requests_in updated to %" PRId64, data.rec_int); - } else if (name == "proxy.config.net.inactive_threshold_in"sv) { - updated_member = &NetHandler::global_config.inactive_threshold_in; - Dbg(dbg_ctl_net_queue, "proxy.config.net.inactive_threshold_in updated to %" PRId64, data.rec_int); - } else if (name == "proxy.config.net.transaction_no_activity_timeout_in"sv) { - updated_member = &NetHandler::global_config.transaction_no_activity_timeout_in; - Dbg(dbg_ctl_net_queue, "proxy.config.net.transaction_no_activity_timeout_in updated to %" PRId64, data.rec_int); - } else if (name == "proxy.config.net.keep_alive_no_activity_timeout_in"sv) { - updated_member = &NetHandler::global_config.keep_alive_no_activity_timeout_in; - Dbg(dbg_ctl_net_queue, "proxy.config.net.keep_alive_no_activity_timeout_in updated to %" PRId64, data.rec_int); } else if (name == "proxy.config.net.default_inactivity_timeout"sv) { updated_member = &NetHandler::global_config.default_inactivity_timeout; Dbg(dbg_ctl_net_queue, "proxy.config.net.default_inactivity_timeout updated to %" PRId64, data.rec_int); @@ -175,13 +166,8 @@ void NetHandler::init_for_process() { // read configuration values and setup callbacks for when they change - global_config.max_connections_in = RecGetRecordInt("proxy.config.net.max_connections_in").value_or(0); - global_config.max_requests_in = RecGetRecordInt("proxy.config.net.max_requests_in").value_or(0); - global_config.inactive_threshold_in = RecGetRecordInt("proxy.config.net.inactive_threshold_in").value_or(0); - global_config.transaction_no_activity_timeout_in = - RecGetRecordInt("proxy.config.net.transaction_no_activity_timeout_in").value_or(0); - global_config.keep_alive_no_activity_timeout_in = - RecGetRecordInt("proxy.config.net.keep_alive_no_activity_timeout_in").value_or(0); + global_config.max_connections_in = RecGetRecordInt("proxy.config.net.max_connections_in").value_or(0); + global_config.max_requests_in = RecGetRecordInt("proxy.config.net.max_requests_in").value_or(0); global_config.default_inactivity_timeout = RecGetRecordInt("proxy.config.net.default_inactivity_timeout").value_or(0); // Atomic configurations. @@ -198,20 +184,12 @@ NetHandler::init_for_process() RecRegisterConfigUpdateCb("proxy.config.net.max_connections_in", update_nethandler_config, nullptr); RecRegisterConfigUpdateCb("proxy.config.net.max_requests_in", update_nethandler_config, nullptr); - RecRegisterConfigUpdateCb("proxy.config.net.inactive_threshold_in", update_nethandler_config, nullptr); - RecRegisterConfigUpdateCb("proxy.config.net.transaction_no_activity_timeout_in", update_nethandler_config, nullptr); - RecRegisterConfigUpdateCb("proxy.config.net.keep_alive_no_activity_timeout_in", update_nethandler_config, nullptr); RecRegisterConfigUpdateCb("proxy.config.net.default_inactivity_timeout", update_nethandler_config, nullptr); RecRegisterConfigUpdateCb("proxy.config.net.additional_accepts", update_nethandler_config, nullptr); RecRegisterConfigUpdateCb("proxy.config.net.per_client.max_connections_in", update_nethandler_config, nullptr); Dbg(dbg_ctl_net_queue, "proxy.config.net.max_connections_in updated to %d", global_config.max_connections_in); Dbg(dbg_ctl_net_queue, "proxy.config.net.max_requests_in updated to %d", global_config.max_requests_in); - Dbg(dbg_ctl_net_queue, "proxy.config.net.inactive_threshold_in updated to %d", global_config.inactive_threshold_in); - Dbg(dbg_ctl_net_queue, "proxy.config.net.transaction_no_activity_timeout_in updated to %d", - global_config.transaction_no_activity_timeout_in); - Dbg(dbg_ctl_net_queue, "proxy.config.net.keep_alive_no_activity_timeout_in updated to %d", - global_config.keep_alive_no_activity_timeout_in); Dbg(dbg_ctl_net_queue, "proxy.config.net.default_inactivity_timeout updated to %d", global_config.default_inactivity_timeout); Dbg(dbg_ctl_net_queue, "proxy.config.net.additional_accepts updated to %d", additional_accepts.load(std::memory_order_relaxed)); Dbg(dbg_ctl_net_queue, "proxy.config.net.per_client.max_connections_in updated to %d", diff --git a/src/iocore/net/UnixNet.cc b/src/iocore/net/UnixNet.cc index f32df6ad7d0..158d3ceb24c 100644 --- a/src/iocore/net/UnixNet.cc +++ b/src/iocore/net/UnixNet.cc @@ -39,9 +39,18 @@ std::atomic net_memory_throttle = false; int fds_throttle; ink_hrtime last_transient_accept_error; +namespace +{ +/// Config members that @c NetHandler::configure_per_thread_values reads. +constexpr unsigned long long PER_THREAD_DEPENDENT_CONFIG{0x3}; +// std::bitset silently discards bits at or above its width, which would drop a +// member from the set without any diagnostic if Config ever shrinks. +static_assert(PER_THREAD_DEPENDENT_CONFIG < (1ULL << NetHandler::CONFIG_ITEM_COUNT)); +} // end anonymous namespace + NetHandler::Config NetHandler::global_config; std::bitset::digits> NetHandler::active_thread_types; -const std::bitset NetHandler::config_value_affects_per_thread_value{0x3}; +const std::bitset NetHandler::config_value_affects_per_thread_value{PER_THREAD_DEPENDENT_CONFIG}; namespace { From 54b79ae69c73ae17d8f5a5754f6ea1ed14fc5e79 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 11 Aug 2026 16:34:02 +0200 Subject: [PATCH 2/3] Harden NetHandler config layout assertions Address review feedback. Assert the standard layout and alignment that offsetof and the array-like operator[] rely on, and replace the mask fit check with std::bit_width so it does not depend on a shift that would be ill formed if the field count ever reached the width of the shifted type. --- include/iocore/net/NetHandler.h | 3 +++ src/iocore/net/UnixNet.cc | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/include/iocore/net/NetHandler.h b/include/iocore/net/NetHandler.h index c02a57b8107..c55e18be003 100644 --- a/include/iocore/net/NetHandler.h +++ b/include/iocore/net/NetHandler.h @@ -25,6 +25,7 @@ #include #include +#include #include "iocore/eventsystem/Continuation.h" #include "iocore/eventsystem/EThread.h" @@ -131,6 +132,8 @@ class NetHandler : public Continuation, public EThread::LoopTailHandler // Config is addressed as an array of uint32_t through operator[], and // config_value_affects_per_thread_value is a bitset indexed by field // position, so the offset of each member is part of the interface. + static_assert(std::is_standard_layout_v); // required for offsetof below to be well defined + static_assert(alignof(Config) == alignof(uint32_t)); // a member of wider type would break operator[] static_assert(offsetof(Config, max_connections_in) == 0 * sizeof(uint32_t)); static_assert(offsetof(Config, max_requests_in) == 1 * sizeof(uint32_t)); static_assert(offsetof(Config, default_inactivity_timeout) == 2 * sizeof(uint32_t)); diff --git a/src/iocore/net/UnixNet.cc b/src/iocore/net/UnixNet.cc index 158d3ceb24c..671dbc3bb1e 100644 --- a/src/iocore/net/UnixNet.cc +++ b/src/iocore/net/UnixNet.cc @@ -32,6 +32,8 @@ #include "iocore/io_uring/IO_URING.h" #endif +#include + ink_hrtime last_throttle_warning; ink_hrtime last_shedding_warning; int net_connections_throttle; @@ -45,7 +47,7 @@ namespace constexpr unsigned long long PER_THREAD_DEPENDENT_CONFIG{0x3}; // std::bitset silently discards bits at or above its width, which would drop a // member from the set without any diagnostic if Config ever shrinks. -static_assert(PER_THREAD_DEPENDENT_CONFIG < (1ULL << NetHandler::CONFIG_ITEM_COUNT)); +static_assert(std::bit_width(PER_THREAD_DEPENDENT_CONFIG) <= NetHandler::CONFIG_ITEM_COUNT); } // end anonymous namespace NetHandler::Config NetHandler::global_config; From 723cde959c8b77af02c3f4bcc82aa0f0691ea130 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 11 Aug 2026 18:12:31 +0200 Subject: [PATCH 3/3] Avoid std::bit_width in bitmask assertion The Ubuntu CI toolchain (clang 12) does not provide std::bit_width, so the mask-width check broke the build there. Assert the mask fits using a shift instead, guarded by a bit-width check so the shift itself stays well defined. --- src/iocore/net/UnixNet.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/iocore/net/UnixNet.cc b/src/iocore/net/UnixNet.cc index 671dbc3bb1e..7c90eeb3d70 100644 --- a/src/iocore/net/UnixNet.cc +++ b/src/iocore/net/UnixNet.cc @@ -32,7 +32,7 @@ #include "iocore/io_uring/IO_URING.h" #endif -#include +#include ink_hrtime last_throttle_warning; ink_hrtime last_shedding_warning; @@ -46,8 +46,10 @@ namespace /// Config members that @c NetHandler::configure_per_thread_values reads. constexpr unsigned long long PER_THREAD_DEPENDENT_CONFIG{0x3}; // std::bitset silently discards bits at or above its width, which would drop a -// member from the set without any diagnostic if Config ever shrinks. -static_assert(std::bit_width(PER_THREAD_DEPENDENT_CONFIG) <= NetHandler::CONFIG_ITEM_COUNT); +// member from the set without any diagnostic if Config ever shrinks. The first +// assertion keeps the shift in the second one well defined. +static_assert(NetHandler::CONFIG_ITEM_COUNT < std::numeric_limits::digits); +static_assert(PER_THREAD_DEPENDENT_CONFIG < (1ULL << NetHandler::CONFIG_ITEM_COUNT)); } // end anonymous namespace NetHandler::Config NetHandler::global_config;