From: Russ Combs (rucombs) Date: Tue, 17 Jan 2023 22:34:43 +0000 (+0000) Subject: Pull request #3733: Memory Updates X-Git-Tag: 3.1.52.0~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1c77d7e946ea9af98b86040695585cf3a00b4ce9;p=thirdparty%2Fsnort3.git 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 Date: Sun Jan 15 07:10:01 2023 -0500 memory: add regression test hooks commit fda0e1eb1a540ee8ad2a2256955d7ded488b5f8d Author: Russ Combs 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 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 402e04540..9ee93eb14 100644 --- 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 3201e98d8..ca2305ce0 100644 --- 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 5fc19fc85..48d825a1f 100644 --- 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 bb1bad671..9e818ee64 100644 --- 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 7dcbf7696..726b3b65c 100644 --- a/src/managers/module_manager.h +++ b/src/managers/module_manager.h @@ -28,6 +28,7 @@ #include #include +#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 gids; SO_PUBLIC static std::mutex stats_mutex; }; diff --git a/src/memory/memory_cap.cc b/src/memory/memory_cap.cc index bc2f5dfb4..d3fcf37e8 100644 --- 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 396988fd3..bf4d6f8cb 100644 --- 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 76d5c689b..0adb8e222 100644 --- 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 a1c1fc4b8..65de58f94 100644 --- 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 29a951208..a59ac7890 100644 --- 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();