From c388eba70874f0aa4a9096b6a7b851d663731bab Mon Sep 17 00:00:00 2001 From: "Steven Baigal (sbaigal)" Date: Tue, 19 Aug 2025 22:01:51 +0000 Subject: [PATCH] Pull request #4730: watchdog: replace watchdog command with atomic kcking from packet threads Merge in SNORT/snort3 from ~SBAIGAL/snort3:watchdog_fix to master Squashed commit of the following: commit 2d7d9b64fdd00ab2f5961c8e5168453eaa3e5e82 Author: Steven Baigal Date: Thu May 1 10:25:56 2025 -0400 watchdog: replace watchdog command with atomic kcking from packet threads --- src/main/analyzer.cc | 4 +++ src/main/test/distill_verdict_test.cc | 3 ++ src/main/thread_config.cc | 52 +++++++++++++++------------ src/main/thread_config.h | 2 ++ 4 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc index 3b2961f8f..39e7e1af6 100644 --- a/src/main/analyzer.cc +++ b/src/main/analyzer.cc @@ -555,6 +555,7 @@ void Analyzer::set_state(State s) { state = s; main_poke(id); + ThreadConfig::update_thread_status(s == State::RUNNING); } const char* Analyzer::get_state_string() @@ -608,6 +609,7 @@ void Analyzer::idle() handle_uncompleted_commands(); idling = false; + ThreadConfig::kick_watchdog(); } /* @@ -877,6 +879,7 @@ void Analyzer::handle_uncompleted_commands() } else ++it; + ThreadConfig::kick_watchdog(); } } @@ -918,6 +921,7 @@ DAQ_RecvStatus Analyzer::process_messages() num_recv++; // IMPORTANT: process_daq_msg() is responsible for finalizing the messages. process_daq_msg(msg, false); + ThreadConfig::kick_watchdog(); DetectionEngine::onload(); process_retry_queue(); handle_uncompleted_commands(); diff --git a/src/main/test/distill_verdict_test.cc b/src/main/test/distill_verdict_test.cc index 65f0fa685..aec78f220 100644 --- a/src/main/test/distill_verdict_test.cc +++ b/src/main/test/distill_verdict_test.cc @@ -27,6 +27,7 @@ #include "framework/data_bus.h" #include "main/analyzer.h" +#include "main/thread_config.h" #include "memory/memory_cap.h" #include "packet_io/sfdaq_instance.h" #include "packet_io/sfdaq.h" @@ -56,6 +57,8 @@ unsigned int get_random_seed() { return 3193; } unsigned DataBus::get_id(const PubKey&) { return 0; } +void ThreadConfig::update_thread_status(bool) {} +void ThreadConfig::kick_watchdog() {} } const FlowCacheConfig& FlowControl::get_flow_cache_config() const diff --git a/src/main/thread_config.cc b/src/main/thread_config.cc index 87f676e34..b04f53495 100644 --- a/src/main/thread_config.cc +++ b/src/main/thread_config.cc @@ -394,28 +394,20 @@ struct Watchdog Watchdog(uint16_t tm) : seconds_count(tm) { resp = new std::atomic_bool[ThreadConfig::get_instance_max()]; + thread_is_active = new std::atomic_bool[ThreadConfig::get_instance_max()]; + for ( unsigned i = 0; i < ThreadConfig::get_instance_max(); ++i ) + thread_is_active[i] = false; + } + ~Watchdog() + { + delete[] resp; + delete[] thread_is_active; } - ~Watchdog() { delete[] resp; } void kick(); bool waiting = false; std::atomic_bool* resp; uint16_t seconds_count; -}; - -class WatchdogKick : public AnalyzerCommand -{ -public: - WatchdogKick(Watchdog* d) : dog(d) { } - bool execute(Analyzer&, void**) override - { - dog->resp[get_instance_id()] = true; - return true; - } - const char* stringify() override { return "WATCHDOG_KICK"; } - - ~WatchdogKick() override { } -private: - Watchdog* dog; + std::atomic_bool* thread_is_active; }; void Watchdog::kick() @@ -426,7 +418,7 @@ void Watchdog::kick() uint16_t thread_count = 0; for ( unsigned i = 0; i < max; ++i ) { - if ( !resp[i] ) + if ( !resp[i] && thread_is_active[i] ) { ++thread_count; if (thread_count == 1) @@ -455,7 +447,6 @@ void Watchdog::kick() for ( unsigned i = 0; i < max; ++i ) resp[i] = false; - main_broadcast_command(new WatchdogKick(this), nullptr); waiting = true; } @@ -485,15 +476,30 @@ void ThreadConfig::start_watchdog() Periodic::register_handler(s_watchdog_handler, nullptr, 0, 1000); } -void ThreadConfig::preemptive_kick() +void ThreadConfig::kick_watchdog() { - if (SnortConfig::get_conf()->watchdog_timer) + static const bool watchdog_is_enabled = (SnortConfig::get_conf()->watchdog_timer > 0); + if (watchdog_is_enabled) { - Watchdog& dog = get_watchdog(); - dog.resp[get_instance_id()] = true; + static THREAD_LOCAL std::atomic_bool* tl_resp_ptr = nullptr; + if (!tl_resp_ptr) + tl_resp_ptr = &get_watchdog().resp[get_instance_id()]; + + (*tl_resp_ptr).store(true, std::memory_order_relaxed); } } +void ThreadConfig::preemptive_kick() +{ + kick_watchdog(); +} + +void ThreadConfig::update_thread_status(bool status) +{ + static const bool watchdog_is_enabled = (SnortConfig::get_conf()->watchdog_timer > 0); + if (watchdog_is_enabled) + get_watchdog().thread_is_active[get_instance_id()] = status; +} // ----------------------------------------------------------------------------- // unit tests diff --git a/src/main/thread_config.h b/src/main/thread_config.h index 5300e0dad..112bb152c 100644 --- a/src/main/thread_config.h +++ b/src/main/thread_config.h @@ -44,7 +44,9 @@ public: static unsigned get_instance_max(); static void term(); static void start_watchdog(); + static void kick_watchdog(); static void preemptive_kick(); + static void update_thread_status(bool status); static void set_instance_tid(int); static int get_instance_tid(int); -- 2.47.3