Pull request #3733: Memory Updates

Merge in SNORT/snort3 from ~RUCOMBS/snort3:memory_init to master

Squashed commit of the following:

commit e5194f6de9eb80ce8f47ad114ed13edd440690f1
Author: Russ Combs <rucombs@cisco.com>
Date:   Sun Jan 15 07:10:01 2023 -0500

    memory: add regression test hooks

commit fda0e1eb1a540ee8ad2a2256955d7ded488b5f8d
Author: Russ Combs <rucombs@cisco.com>
Date:   Fri Jan 13 18:13:01 2023 -0500

    memory: add final epoch to capture stats

    Also rename bookend methods for clarity.

commit d036355f926eacbde336039bd8eb9c023d836e00
Author: Russ Combs <rucombs@cisco.com>
Date:   Fri Jan 13 08:29:45 2023 -0500

    memory: fix init sequence

    Thanks to amishmm and Xiche for reporting and debugging the problem.

diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc
index 402e04540089268745ffb49d567728a523b26914..9ee93eb14a5e3dd30742cbcee9f6cd0b604724bf 100644 (file)
--- a/src/main/analyzer.cc
+++ b/src/main/analyzer.cc
@@ -673,7 +673,8 @@ void Analyzer::term()
     DetectionEngine::idle();
     InspectorManager::thread_stop(sc);
     InspectorManager::thread_term();
-    ModuleManager::accumulate("memory");
+    memory::MemoryCap::thread_term();
+    ModuleManager::accumulate();
     ActionManager::thread_term();
 
     IpsManager::clear_options(sc);
@@ -706,8 +707,6 @@ void Analyzer::term()
     RateFilter_Cleanup();
 
     TraceApi::thread_term();
-
-    ModuleManager::accumulate_module("memory");
 }
 
 Analyzer::Analyzer(SFDAQInstance* instance, unsigned i, const char* s, uint64_t msg_cnt)

diff --git a/src/main/snort.cc b/src/main/snort.cc
index 3201e98d82de414b49799c91c28df3678f4b7f91..ca2305ce0966899cd9d0db72402e38b84f7edb6d 100644 (file)
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -168,6 +168,7 @@ void Snort::init(int argc, char** argv)
         EventManager::instantiate(sc->output.c_str(), sc);
 
     HighAvailabilityManager::configure(sc->ha_config);
+    memory::MemoryCap::init(sc->thread_config->get_instance_max());
 
     ModuleManager::reset_stats(sc);
 
@@ -352,7 +353,7 @@ void Snort::term()
     ModuleManager::term();
     PluginManager::release_plugins();
     ScriptManager::release_scripts();
-    memory::MemoryCap::cleanup();
+    memory::MemoryCap::term();
     detection_filter_term();
 
     term_signals();
@@ -399,7 +400,7 @@ void Snort::setup(int argc, char* argv[])
 
     set_quick_exit(false);
 
-    memory::MemoryCap::setup(*sc->memory, sc->thread_config->get_instance_max(), Stream::prune_flows);
+    memory::MemoryCap::start(*sc->memory, Stream::prune_flows);
     memory::MemoryCap::print(SnortConfig::log_verbose(), true);
 
     host_cache.print_config();
@@ -413,6 +414,7 @@ void Snort::cleanup()
 
     SFDAQ::term();
     FileService::close();
+    memory::MemoryCap::stop();
 
     if ( !SnortConfig::get_conf()->test_mode() )  // FIXIT-M ideally the check is in one place
         PrintStatistics();

diff --git a/src/main/test/distill_verdict_stubs.h b/src/main/test/distill_verdict_stubs.h
index 5fc19fc851b17d5793de7edc8a2a0c39e93a8113..48d825a1f592ed4adc5f64030b58a1890a777028 100644 (file)
--- a/src/main/test/distill_verdict_stubs.h
+++ b/src/main/test/distill_verdict_stubs.h
@@ -228,5 +228,6 @@ void ThreadConfig::set_instance_tid(const int, const int) { }
 }
 
 void memory::MemoryCap::thread_init() { }
+void memory::MemoryCap::thread_term() { }
 void memory::MemoryCap::free_space() { }
 

diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc
index bb1bad6715dbf4730b83ff78761fe135fa26e2e6..9e818ee64dd5e1dc09bee4618db64ba75c690577 100644 (file)
--- a/src/managers/module_manager.cc
+++ b/src/managers/module_manager.cc
@@ -1425,6 +1425,17 @@ void ModuleManager::load_rules(SnortConfig* sc)
     }
 }
 
+PegCount* ModuleManager::get_stats(const char* name)
+{
+    PegCount* pc = nullptr;
+    ModHook* mh = get_hook(name);
+
+    if ( mh )
+        pc = &mh->mod->counts[0];
+
+    return pc;
+}
+
 void ModuleManager::dump_stats(const char* skip, bool dynamic)
 {
     auto mod_hooks = get_all_modhooks();

diff --git a/src/managers/module_manager.h b/src/managers/module_manager.h
index 7dcbf76962ef0accb5dcb69097a9231fbe3d9909..726b3b65c8abc549fe8ea656cb0eabda83dd2840 100644 (file)
--- a/src/managers/module_manager.h
+++ b/src/managers/module_manager.h
@@ -28,6 +28,7 @@
 #include <mutex>
 #include <set>
 
+#include "framework/counts.h"
 #include "main/analyzer_command.h"
 #include "main/snort_types.h"
 
@@ -83,15 +84,18 @@ public:
     static void reset_errors();
     static unsigned get_errors();
 
+    static PegCount* get_stats(const char* name);
     static void dump_stats(const char* skip = nullptr, bool dynamic = false);
 
     static void accumulate(const char* except = nullptr);
     static void accumulate_module(const char* name);
+
     static void reset_stats(SnortConfig*);
     static void reset_stats(clear_counter_type_t);
 
     static void clear_global_active_counters();
 
+
     static std::set<uint32_t> gids;
     SO_PUBLIC static std::mutex stats_mutex;
 };

diff --git a/src/memory/memory_cap.cc b/src/memory/memory_cap.cc
index bc2f5dfb40af17b40e01399f2938ca36d3656d9e..d3fcf37e80d1a0175ec3e717b35b44f924d33f4a 100644 (file)
--- a/src/memory/memory_cap.cc
+++ b/src/memory/memory_cap.cc
@@ -80,7 +80,7 @@ static void epoch_check(void*)
     if ( prior != over_limit )
         trace_logf(memory_trace, nullptr, "Epoch=%lu, memory=%lu (%s)\n", epoch, total, over_limit?"over":"under");
 
-    MemoryCounts& mc = memory::MemoryCap::get_mem_stats();
+    MemoryCounts& mc = MemoryCap::get_mem_stats();
 
     if ( total > mc.max_in_use )
         mc.max_in_use = total;
@@ -89,26 +89,79 @@ static void epoch_check(void*)
     mc.epochs++;
 }
 
+// -----------------------------------------------------------------------------
+// test access
+// -----------------------------------------------------------------------------
+
+#ifdef REG_TEST
+static unsigned count_down = 1;
+
+static void test_pkt_check()
+{
+    assert(is_packet_thread());
+
+    if ( !config.enabled )
+        return;
+
+    if ( get_instance_id() != 0 )
+        return;
+
+    if ( !config.interval )
+        return;
+
+    if ( --count_down )
+        return;
+    else
+        count_down = config.interval;
+
+    epoch_check(nullptr);
+}
+
+void MemoryCap::test_main_check()
+{
+    assert(in_main_thread());
+    epoch_check(nullptr);
+}
+#endif
+
 // -----------------------------------------------------------------------------
 // public
 // -----------------------------------------------------------------------------
 
+void MemoryCap::init(unsigned n)
+{
+    assert(in_main_thread());
+    pkt_mem_stats.resize(n);
+
+#ifdef UNIT_TEST
+    pkt_mem_stats[0] = { };
+#endif
+}
+
+void MemoryCap::term()
+{
+    pkt_mem_stats.resize(0);
+    delete heap;
+    heap = nullptr;
+}
+
 void MemoryCap::set_heap_interface(HeapInterface* h)
 { heap = h; }
 
 void MemoryCap::set_pruner(PruneHandler p)
 { pruner = p; }
 
-void MemoryCap::setup(const MemoryConfig& c, unsigned n, PruneHandler ph)
+void MemoryCap::start(const MemoryConfig& c, PruneHandler ph)
 {
-    assert(!is_packet_thread());
+    assert(in_main_thread());
 
-    pkt_mem_stats.resize(n);
     config = c;
 
     if ( !heap )
         heap = HeapInterface::get_instance();
 
+    heap->main_init();
+
     if ( !config.enabled )
         return;
 
@@ -119,38 +172,36 @@ void MemoryCap::setup(const MemoryConfig& c, unsigned n, PruneHandler ph)
     over_limit = false;
     current_epoch = 0;
 
+#ifndef REG_TEST
     Periodic::register_handler(epoch_check, nullptr, 0, config.interval);
-    heap->main_init();
-
-    MemoryCounts& mc = memory::MemoryCap::get_mem_stats();
-#ifdef UNIT_TEST
-    mc = { };
 #endif
 
     epoch_check(nullptr);
+
+    MemoryCounts& mc = get_mem_stats();
     mc.start_up_use = mc.cur_in_use;
 }
 
-void MemoryCap::cleanup()
-{
-    pkt_mem_stats.resize(0);
-    delete heap;
-    heap = nullptr;
-}
+void MemoryCap::stop()
+{ epoch_check(nullptr); }
 
 void MemoryCap::thread_init()
 {
-    if ( config.enabled )
-        heap->thread_init();
-
+    heap->thread_init();
     start_dealloc = 0;
     start_epoch = 0;
 }
 
+void MemoryCap::thread_term()
+{
+    MemoryCounts& mc = get_mem_stats();
+    heap->get_thread_allocs(mc.allocated, mc.deallocated);
+}
+
 MemoryCounts& MemoryCap::get_mem_stats()
 {
     // main thread stats do not overlap with packet threads
-    if ( !is_packet_thread() )
+    if ( in_main_thread() )
         return pkt_mem_stats[0];
 
     auto id = get_instance_id();
@@ -161,7 +212,11 @@ void MemoryCap::free_space()
 {
     assert(is_packet_thread());
 
-    MemoryCounts& mc = memory::MemoryCap::get_mem_stats();
+#ifdef REG_TEST
+    test_pkt_check();
+#endif
+
+    MemoryCounts& mc = get_mem_stats();
     heap->get_thread_allocs(mc.allocated, mc.deallocated);
 
     if ( !over_limit and !start_dealloc )
@@ -194,6 +249,25 @@ void MemoryCap::free_space()
     ++mc.reap_failures;
 }
 
+// required to capture any update in final epoch
+// which happens after packet threads have stopped
+void MemoryCap::update_pegs(PegCount* pc)
+{
+    MemoryCounts* mp = (MemoryCounts*)pc;
+    const MemoryCounts& mc = get_mem_stats();
+
+    if ( config.enabled )
+    {
+        mp->epochs = mc.epochs;
+        mp->cur_in_use = mc.cur_in_use;
+    }
+    else
+    {
+        mp->allocated = 0;
+        mp->deallocated = 0;
+    }
+}
+
 // called at startup and shutdown
 void MemoryCap::print(bool verbose, bool init)
 {

diff --git a/src/memory/memory_cap.h b/src/memory/memory_cap.h
index 396988fd3ddb07f232dae261e48f3a25617b893c..bf4d6f8cb310062a9e667760cecbcb86d9735ffa 100644 (file)
--- a/src/memory/memory_cap.h
+++ b/src/memory/memory_cap.h
@@ -52,20 +52,33 @@ typedef bool (*PruneHandler)();
 class SO_PUBLIC MemoryCap
 {
 public:
+    // main thread
+    static void init(unsigned num_threads);
+    static void term();
+
     // main thread - in configure
     static void set_heap_interface(HeapInterface*);
     static void set_pruner(PruneHandler);
 
     // main thread - after configure
-    static void setup(const MemoryConfig&, unsigned num_threads, PruneHandler);
-    static void cleanup();
+    static void start(const MemoryConfig&, PruneHandler);
+    static void stop();
     static void print(bool verbose, bool init = false);
 
     // packet threads
     static void thread_init();
+    static void thread_term();
     static void free_space();
 
+    // main and packet threads
     static MemoryCounts& get_mem_stats();
+
+    // main thread - shutdown
+    static void update_pegs(PegCount*);
+
+#ifdef REG_TEST
+    static void test_main_check();
+#endif
 };
 
 }

diff --git a/src/memory/memory_module.cc b/src/memory/memory_module.cc
index 76d5c689bf68e94b1539c597aa21bee2735c6739..0adb8e222028edf488662a27c59bf399dea1949c 100644 (file)
--- a/src/memory/memory_module.cc
+++ b/src/memory/memory_module.cc
@@ -47,8 +47,8 @@ static const Parameter s_params[] =
     { "cap", Parameter::PT_INT, "0:maxSZ", "0",
         "set the process cap on memory in bytes (0 to disable)" },
 
-    { "interval", Parameter::PT_INT, "1:max32", "50",
-        "approximate ms between memory epochs" },
+    { "interval", Parameter::PT_INT, "0:max32", "50",
+        "approximate ms between memory epochs (0 to disable)" },
 
     { "prune_target", Parameter::PT_INT, "1:max32", "1048576",
         "bytes to prune per packet thread prune cycle" },

diff --git a/src/memory/test/memory_cap_test.cc b/src/memory/test/memory_cap_test.cc
index a1c1fc4b8ec3d7b9c87f758ee01c6ae2e0af7279..65de58f9470e99ae0483ac597dd9ce540e0105d9 100644 (file)
--- a/src/memory/test/memory_cap_test.cc
+++ b/src/memory/test/memory_cap_test.cc
@@ -87,17 +87,8 @@ public:
     unsigned thread_init_calls = 0;
 };
 
-static PeriodicHook phook = nullptr;
-static void* parg = nullptr;
-
-void Periodic::register_handler(PeriodicHook f, void* v, uint16_t, uint32_t)
-{ phook = f; parg = v; }
-
 static void periodic_check()
-{
-    if ( phook )
-        phook(parg);
-}
+{ MemoryCap::test_main_check(); }
 
 static int flows;
 
@@ -128,20 +119,25 @@ static void free_space()
 TEST_GROUP(memory_off)
 {
     void setup() override
-    { MemoryCap::set_pruner(pruner); }
+    {
+        MemoryCap::init(1);
+        MemoryCap::set_pruner(pruner);
+    }
 
     void teardown() override
-    { flows = 0; }
+    {
+        MemoryCap::term();
+        flows = 0;
+    }
 };
 
 TEST(memory_off, disabled)
 {
     MemoryConfig config { 0, 100, 0, 1, false };
-    MemoryCap::setup(config, 1, pruner);
+    MemoryCap::start(config, pruner);
 
     free_space();
 
-    CHECK(phook == nullptr);
     CHECK(flows == 0);
 
     const MemoryCounts& mc = MemoryCap::get_mem_stats();
@@ -149,17 +145,16 @@ TEST(memory_off, disabled)
     CHECK(mc.epochs == 0);
     CHECK(mc.reap_cycles == 0);
 
-    MemoryCap::cleanup();
+    MemoryCap::stop();
 }
 
 TEST(memory_off, nerfed)
 {
     MemoryConfig config { 100, 100, 0, 1, false };
-    MemoryCap::setup(config, 1, pruner);
+    MemoryCap::start(config, pruner);
 
     free_space();
 
-    CHECK(phook == nullptr);
     CHECK(flows == 0);
 
     const MemoryCounts& mc = MemoryCap::get_mem_stats();
@@ -167,7 +162,7 @@ TEST(memory_off, nerfed)
     CHECK(mc.epochs == 0);
     CHECK(mc.reap_cycles == 0);
 
-    MemoryCap::cleanup();
+    MemoryCap::stop();
 }
 
 //--------------------------------------------------------------------------
@@ -180,15 +175,15 @@ TEST_GROUP(memory)
 
     void setup() override
     {
+        MemoryCap::init(1);
         heap = new MockHeap;
         MemoryCap::set_heap_interface(heap);
     }
 
     void teardown() override
     {
+        MemoryCap::term();
         heap = nullptr;
-        phook = nullptr;
-        parg = nullptr;
         flows = 0;
     }
 };
@@ -196,9 +191,8 @@ TEST_GROUP(memory)
 TEST(memory, default_enabled)
 {
     MemoryConfig config { 0, 100, 0, 1, true };
-    MemoryCap::setup(config, 1, pruner);
+    MemoryCap::start(config, pruner);
 
-    CHECK(phook != nullptr);
     CHECK(heap->epoch == 1);
     CHECK(heap->main_init_calls == 1);
 
@@ -211,7 +205,8 @@ TEST(memory, default_enabled)
     CHECK(heap->epoch == 2);
     CHECK(flows == 0);
 
-    MemoryCap::cleanup();
+    MemoryCap::stop();
+    CHECK(heap->epoch == 3);
 }
 
 TEST(memory, prune1)
@@ -221,7 +216,7 @@ TEST(memory, prune1)
     heap->total = start;
 
     MemoryConfig config { cap, 100, 0, 1, true };
-    MemoryCap::setup(config, 1, pruner);
+    MemoryCap::start(config, pruner);
     MemoryCap::thread_init();
 
     const MemoryCounts& mc = MemoryCap::get_mem_stats();
@@ -262,14 +257,17 @@ TEST(memory, prune1)
     CHECK(mc.reap_failures == 0);
     CHECK(mc.pruned == 1);
 
-    MemoryCap::cleanup();
+    heap->total = start;
+    MemoryCap::stop();
+    CHECK(mc.epochs == heap->epoch);
+    CHECK(mc.cur_in_use == start);
 }
 
 TEST(memory, prune3)
 {
     uint64_t cap = 100;
     MemoryConfig config { cap, 100, 0, 1, true };
-    MemoryCap::setup(config, 1, pruner);
+    MemoryCap::start(config, pruner);
     MemoryCap::thread_init();
 
     flows = 3;
@@ -302,14 +300,14 @@ TEST(memory, prune3)
     CHECK(mc.reap_failures == 0);
     CHECK(mc.pruned == 1);
 
-    MemoryCap::cleanup();
+    MemoryCap::stop();
 }
 
 TEST(memory, two_cycles)
 {
     uint64_t cap = 100;
     MemoryConfig config { cap, 100, 0, 1, true };
-    MemoryCap::setup(config, 1, pruner);
+    MemoryCap::start(config, pruner);
     MemoryCap::thread_init();
 
     flows = 3;
@@ -350,7 +348,7 @@ TEST(memory, two_cycles)
     CHECK(mc.reap_failures == 0);
     CHECK(mc.pruned == 11);
 
-    MemoryCap::cleanup();
+    MemoryCap::stop();
 }
 
 TEST(memory, reap_failure)
@@ -360,7 +358,7 @@ TEST(memory, reap_failure)
     heap->total = start;
 
     MemoryConfig config { cap, 100, 0, 2, true };
-    MemoryCap::setup(config, 1, pruner);
+    MemoryCap::start(config, pruner);
     MemoryCap::thread_init();
 
     flows = 1;
@@ -382,7 +380,7 @@ TEST(memory, reap_failure)
     CHECK(mc.reap_failures == 1);
     CHECK(mc.pruned == 1);
 
-    MemoryCap::cleanup();
+    MemoryCap::stop();
 }
 
 //-------------------------------------------------------------------------

diff --git a/src/utils/stats.cc b/src/utils/stats.cc
index 29a951208891e8021172edf44752d60ee7548d10..a59ac7890f2688f3068e445f91bbdad2cf2c33fd 100644 (file)
--- a/src/utils/stats.cc
+++ b/src/utils/stats.cc
@@ -265,6 +265,9 @@ void DropStats(ControlConn* ctrlcon)
 
 void PrintStatistics()
 {
+    if ( PegCount* pc = ModuleManager::get_stats("memory") )
+        memory::MemoryCap::update_pegs(pc);
+
     DropStats();
     timing_stats();