Pull request #3798: thread_config: add preemptive watchdog kick for flow deletion

Merge in SNORT/snort3 from ~ADMAMOLE/snort3:wdt_flow_deletes to master

Squashed commit of the following:

commit da7e7eeddf30ca011d46949ec76df28480ec331d
Author: Adrian Mamolea <admamole@cisco.com>
Date:   Tue Apr 4 15:35:17 2023 -0400

    thread_config: add preemptive watchdog kick for flow deletion

diff --git a/src/flow/flow_cache.cc b/src/flow/flow_cache.cc
index 39fbce6417a781325dfc857da84b94d5fce2938c..22460ebb78627ce76e9aa892faef5db3a5ccf2d9 100644 (file)
--- a/src/flow/flow_cache.cc
+++ b/src/flow/flow_cache.cc
@@ -28,6 +28,7 @@
 #include "hash/hash_defs.h"
 #include "hash/zhash.h"
 #include "helpers/flag_context.h"
+#include "main/thread_config.h"
 #include "packet_io/active.h"
 #include "packet_tracer/packet_tracer.h"
 #include "stream/base/stream_module.h"
@@ -48,6 +49,7 @@ using namespace snort;
 static const unsigned ALLOWED_FLOWS_ONLY = 1;
 static const unsigned OFFLOADED_FLOWS_TOO = 2;
 static const unsigned ALL_FLOWS = 3;
+static const unsigned WDT_MASK = 7; // kick watchdog once for every 8 flows deleted
 
 //-------------------------------------------------------------------------
 // FlowCache stuff
@@ -454,7 +456,9 @@ unsigned FlowCache::delete_active_flows(unsigned mode, unsigned num_to_delete, u
             continue;
         }
 
-        // we have a winner...
+        if ( (deleted & WDT_MASK) == 0 )
+            ThreadConfig::preemptive_kick();
+
         unlink_uni(flow);
 
         if ( flow->was_blocked() )
@@ -488,6 +492,9 @@ unsigned FlowCache::delete_flows(unsigned num_to_delete)
         // delete from the free list first...
         while ( num_to_delete )
         {
+            if ( (deleted & WDT_MASK) == 0 )
+                ThreadConfig::preemptive_kick();
+
             Flow* flow = (Flow*)hash_table->pop();
             if ( !flow )
                 break;

diff --git a/src/flow/test/flow_cache_test.cc b/src/flow/test/flow_cache_test.cc
index 1ceca8f4312e5df5089e527bc28a89424f370d0c..6fc0d0e2a7b0559334d3320192b990f7c8544cb0 100644 (file)
--- a/src/flow/test/flow_cache_test.cc
+++ b/src/flow/test/flow_cache_test.cc
@@ -33,6 +33,7 @@
 #include "flow/session.h"
 #include "main/policy.h"
 #include "main/snort_config.h"
+#include "main/thread_config.h"
 #include "managers/inspector_manager.h"
 #include "packet_io/active.h"
 #include "packet_tracer/packet_tracer.h"
@@ -105,6 +106,7 @@ SfIpRet SfIp::set(void const*, int) { return SFIP_SUCCESS; }
 void snort::trace_vprintf(const char*, TraceLevel, const char*, const Packet*, const char*, va_list) {}
 uint8_t snort::TraceApi::get_constraints_generation() { return 0; }
 void snort::TraceApi::filter(const Packet&) {}
+void ThreadConfig::preemptive_kick() {}
 
 namespace snort
 {

diff --git a/src/main/thread_config.cc b/src/main/thread_config.cc
index 98286047b2a993f3d1cec550b73f61f8d3b91c54..1119c8a998a1f188f82d1bfa696a5278a5f42558 100644 (file)
--- a/src/main/thread_config.cc
+++ b/src/main/thread_config.cc
@@ -275,7 +275,7 @@ struct Watchdog
 class WatchdogKick : public AnalyzerCommand
 {
 public:
-    WatchdogKick(Watchdog* d) : dog(d) { dog->waiting = true; }
+    WatchdogKick(Watchdog* d) : dog(d) { }
     bool execute(Analyzer&, void**) override
     {
         dog->resp[get_instance_id()] = true;
@@ -283,7 +283,7 @@ public:
     }
     const char* stringify() override { return "WATCHDOG_KICK"; }
 
-    ~WatchdogKick() override { dog->waiting = false; }
+    ~WatchdogKick() override { }
 private:
     Watchdog* dog;
 };
@@ -294,13 +294,16 @@ void Watchdog::kick()
     if ( waiting )
     {
         uint16_t thread_count = 0;
-        WarningMessage("Packet processing threads are unresponsive\n");
-        WarningMessage("Unresponsive thread ID: ");
         for ( unsigned i = 0; i < max; ++i )
         {
             if ( !resp[i] )
             {
                 ++thread_count;
+                if (thread_count == 1)
+                {
+                    WarningMessage("Packet processing threads are unresponsive\n");
+                    WarningMessage("Unresponsive thread ID: ");
+                }
                 const int tid = SnortConfig::get_conf()->thread_config->get_instance_tid(i);
                 if ( tid != -1 )
                     WarningMessage("%d (TID: %d)", i, tid);
@@ -308,7 +311,10 @@ void Watchdog::kick()
                     WarningMessage("%d ", i);
             }
         }
-        WarningMessage("\n");
+
+        if ( thread_count )
+            WarningMessage("\n");
+
         if ( thread_count >= SnortConfig::get_conf()->watchdog_min_thread_count )
         {
             WarningMessage("Aborting Snort\n");
@@ -320,19 +326,26 @@ void Watchdog::kick()
         resp[i] = false;
 
     main_broadcast_command(new WatchdogKick(this), nullptr);
+    waiting = true;
 }
 
-static void s_watchdog_handler(void*)
+static Watchdog& get_watchdog()
 {
     static Watchdog s_dog(SnortConfig::get_conf()->watchdog_timer);
+    return s_dog;
+}
+
+static void s_watchdog_handler(void*)
+{
+    Watchdog& dog = get_watchdog();
     if ( SnortConfig::get_conf()->watchdog_timer > 0 )
     {
-        if ( s_dog.seconds_count > 0 )
-            s_dog.seconds_count--;
+        if ( dog.seconds_count > 0 )
+            dog.seconds_count--;
         else
         {
-            s_dog.kick();
-            s_dog.seconds_count = SnortConfig::get_conf()->watchdog_timer;
+            dog.kick();
+            dog.seconds_count = SnortConfig::get_conf()->watchdog_timer;
         }
     }
 }
@@ -342,6 +355,15 @@ void ThreadConfig::start_watchdog()
     Periodic::register_handler(s_watchdog_handler, nullptr, 0, 1000);
 }
 
+void ThreadConfig::preemptive_kick()
+{
+    if (SnortConfig::get_conf()->watchdog_timer)
+    {
+        Watchdog& dog = get_watchdog();
+        dog.resp[get_instance_id()] = true;
+    }
+}
+
 
 // -----------------------------------------------------------------------------
 // unit tests

diff --git a/src/main/thread_config.h b/src/main/thread_config.h
index c59c7d792aafe7242075c9c4477f063c008e4cac..ba409694179ede8dcd7842dea24e2522ac7fd42c 100644 (file)
--- a/src/main/thread_config.h
+++ b/src/main/thread_config.h
@@ -39,6 +39,7 @@ public:
     static unsigned get_instance_max();
     static void term();
     static void start_watchdog();
+    static void preemptive_kick();
     static void set_instance_tid(int);
     static int get_instance_tid(int);