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.
diff --git a/include/iocore/net/NetHandler.h b/include/iocore/net/NetHandler.h
index 2c84f4e..c55e18b 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 @@
/// 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 @@
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 3fd4b11..7994bac 100644
--- a/src/iocore/net/NetHandler.cc
+++ b/src/iocore/net/NetHandler.cc
@@ -128,15 +128,6 @@
} 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 @@
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 @@
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 f32df6a..7c90eeb 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 @@
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
{
