This is an automated email from the ASF dual-hosted git repository.
brbzull0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 66d890830d Remove dead config records from NetHandler (#13533)
66d890830d is described below
commit 66d890830d36ac782498140fb2b2b71b5cadebbd
Author: Damian Meden <[email protected]>
AuthorDate: Thu Aug 13 08:55:16 2026 +0200
Remove dead config records from NetHandler (#13533)
* 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
* 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.
* 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.
---
include/iocore/net/NetHandler.h | 20 ++++++++++++++------
src/iocore/net/NetHandler.cc | 26 ++------------------------
src/iocore/net/UnixNet.cc | 15 ++++++++++++++-
3 files changed, 30 insertions(+), 31 deletions(-)
diff --git a/include/iocore/net/NetHandler.h b/include/iocore/net/NetHandler.h
index 2c84f4e865..c55e18be00 100644
--- a/include/iocore/net/NetHandler.h
+++ b/include/iocore/net/NetHandler.h
@@ -24,6 +24,8 @@
#pragma once
#include <atomic>
+#include <cstddef>
+#include <type_traits>
#include "iocore/eventsystem/Continuation.h"
#include "iocore/eventsystem/EThread.h"
@@ -111,12 +113,9 @@ public:
/// 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 +129,15 @@ public:
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(std::is_standard_layout_v<Config>); // 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));
+
/** 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 3fd4b11fe3..7994bac83a 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 f32df6ad7d..7c90eeb3d7 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 <limits>
+
ink_hrtime last_throttle_warning;
ink_hrtime last_shedding_warning;
int net_connections_throttle;
@@ -39,9 +41,20 @@ std::atomic<bool> 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. The first
+// assertion keeps the shift in the second one well defined.
+static_assert(NetHandler::CONFIG_ITEM_COUNT < std::numeric_limits<unsigned
long long>::digits);
+static_assert(PER_THREAD_DEPENDENT_CONFIG < (1ULL <<
NetHandler::CONFIG_ITEM_COUNT));
+} // end anonymous namespace
+
NetHandler::Config
NetHandler::global_config;
std::bitset<std::numeric_limits<unsigned int>::digits>
NetHandler::active_thread_types;
-const std::bitset<NetHandler::CONFIG_ITEM_COUNT>
NetHandler::config_value_affects_per_thread_value{0x3};
+const std::bitset<NetHandler::CONFIG_ITEM_COUNT>
NetHandler::config_value_affects_per_thread_value{PER_THREAD_DEPENDENT_CONFIG};
namespace
{
