Pull request #4730: watchdog: replace watchdog command with atomic kcking from packet...

Merge in SNORT/snort3 from ~SBAIGAL/snort3:watchdog_fix to master

Squashed commit of the following:

commit 2d7d9b64fdd00ab2f5961c8e5168453eaa3e5e82
Author: Steven Baigal <sbaigal@cisco.com>
Date:   Thu May 1 10:25:56 2025 -0400

    watchdog: replace watchdog command with atomic kcking from packet threads

diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc
index 3b2961f8fae083f71ce93434b62d69a5a09d8326..39e7e1af65b390f0560c69d4cba9a1b7a1870db7 100644 (file)
--- 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 65f0fa6854751c47c1a2082772f77b37469f72d2..aec78f220c2259e5d8c06cabc0b3f7354d853884 100644 (file)
--- 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 87f676e3403dc99818ce4146c5b53026cfac130a..b04f5349533066a7e9f2b955ff3ba1113e4b68a6 100644 (file)
--- 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 5300e0dad70291f11889fdc67ba92ac304a3b413..112bb152c137e250464e746108b3f576805bb068 100644 (file)
--- 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);