From: Russ Combs (rucombs) Date: Thu, 3 Mar 2016 19:24:46 +0000 (-0500) Subject: Merge pull request #302 in SNORT/snort3 from ~JOCORNET/snort3:memory to master X-Git-Tag: 3.0.0-233~563 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=842ece22ced9ae6d52fcfe2467f381d5fbe4315d;p=thirdparty%2Fsnort3.git Merge pull request #302 in SNORT/snort3 from ~JOCORNET/snort3:memory to master Squashed commit of the following: commit 96b932a314ef5a906bcf6aa144853c36d1a4410f Author: Joel Cornett Date: Mon Feb 29 15:21:08 2016 -0500 prevent allocation reentry commit 69026ddc973bc19e4ca0f22609f8de2bf0f872f3 Author: Joel Cornett Date: Mon Feb 29 10:38:15 2016 -0500 added checks commit f3a80f12654a4451ab5a061f3249745deb3b13c0 Author: Joel Cornett Date: Mon Feb 29 10:15:44 2016 -0500 fixed memory tracking in main thread, added debug message for memcap calc commit 95d233fc038cbb0a96832fb3137de13822141669 Author: Joel Cornett Date: Fri Feb 26 15:14:20 2016 -0500 Added per-thread memcap calculation --- diff --git a/src/main.cc b/src/main.cc index 6ab2fce5a..7f7a8a734 100644 --- a/src/main.cc +++ b/src/main.cc @@ -50,6 +50,7 @@ using namespace std; #include "managers/module_manager.h" #include "managers/plugin_manager.h" #include "managers/inspector_manager.h" +#include "memory/memory_cap.h" #include "utils/util.h" #include "parser/parser.h" #include "packet_io/trough.h" @@ -841,6 +842,8 @@ static void snort_main() pigs = new Pig[max_pigs]; + memory::MemoryCap::calculate(max_pigs); + main_loop(); for ( unsigned idx = 0; idx < max_pigs; ++idx ) diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc index 2133b82e5..70b067d75 100644 --- a/src/main/analyzer.cc +++ b/src/main/analyzer.cc @@ -27,6 +27,7 @@ using namespace std; #include "snort.h" #include "thread.h" #include "helpers/swapper.h" +#include "memory/memory_cap.h" #include "packet_io/sfdaq.h" typedef DAQ_Verdict @@ -52,6 +53,8 @@ Analyzer::Analyzer(const char* s) void Analyzer::operator()(unsigned id, Swapper* ps) { set_packet_thread(true); + // needs to happen before any heap allocations + memory::MemoryCap::tinit(); set_instance_id(id); ps->apply(); diff --git a/src/main/snort_debug.h b/src/main/snort_debug.h index 1f3b974e1..acd062af9 100644 --- a/src/main/snort_debug.h +++ b/src/main/snort_debug.h @@ -50,8 +50,7 @@ #define DEBUG_LOG 0x0000000000000200LL #define DEBUG_FLOWBITS 0x0000000000000400LL #define DEBUG_FILE 0x0000000000000800LL -// FIXIT-L J latency doesn't use any debug messages -#define DEBUG_LATENCY 0x0000000000001000LL +#define DEBUG_MEMORY 0x0000000000001000LL // this env var uses the upper 32 bits of the flags: #define DEBUG_PLUGIN "SNORT_PP_DEBUG" diff --git a/src/memory/memory_allocator.cc b/src/memory/memory_allocator.cc index 18778c021..74fa04a7d 100644 --- a/src/memory/memory_allocator.cc +++ b/src/memory/memory_allocator.cc @@ -25,10 +25,11 @@ namespace memory { -void* DefaultAllocator::allocate(size_t n) +// FIXIT-L J these could be made inlineable by defining in memory_manager.cc +void* MemoryAllocator::allocate(size_t n) { return malloc(n); } -void DefaultAllocator::deallocate(void* p) +void MemoryAllocator::deallocate(void* p) { free(p); } } // namespace memory diff --git a/src/memory/memory_allocator.h b/src/memory/memory_allocator.h index ff6bc392f..0c3a39a80 100644 --- a/src/memory/memory_allocator.h +++ b/src/memory/memory_allocator.h @@ -26,7 +26,7 @@ namespace memory { -struct DefaultAllocator +struct MemoryAllocator { static void* allocate(size_t); static void deallocate(void*); diff --git a/src/memory/memory_cap.cc b/src/memory/memory_cap.cc index cfd3f46ab..bb0413548 100644 --- a/src/memory/memory_cap.cc +++ b/src/memory/memory_cap.cc @@ -27,6 +27,7 @@ #include #include "main/snort_config.h" +#include "main/snort_debug.h" #include "main/thread.h" #include "profiler/memory_profiler_active_context.h" #include "memory_config.h" @@ -36,24 +37,11 @@ #include "catch/catch.hpp" #endif -template -static inline bool free_space(size_t requested, size_t cap, Tracker& trk, Handler& handler) -{ - assert(requested <= cap); - const auto required = cap - requested; - - if ( trk.used() > required ) - handler(); - - return trk.used() <= required; -} - namespace memory { -// ----------------------------------------------------------------------------- -// helpers -// ----------------------------------------------------------------------------- +namespace +{ struct Tracker { @@ -69,45 +57,88 @@ struct Tracker constexpr Tracker() = default; }; +THREAD_LOCAL MemoryConfig s_config; +THREAD_LOCAL Tracker s_tracker; +const MemoryConfig* s_main_config = nullptr; + // ----------------------------------------------------------------------------- -// static variables +// helpers // ----------------------------------------------------------------------------- -static THREAD_LOCAL Tracker s_tracker; +template +inline bool free_space(size_t requested, size_t cap, Tracker& trk, Handler& handler) +{ + assert(requested <= cap); + const auto required = cap - requested; + + if ( trk.used() > required ) + handler(); + + return trk.used() <= required; +} + +} // namespace // ----------------------------------------------------------------------------- // public interface // ----------------------------------------------------------------------------- -bool DefaultCap::free_space(size_t n) +bool MemoryCap::free_space(size_t n) { if ( !is_packet_thread() ) return true; - const auto& config = *snort_conf->memory; + const auto& config = s_config; - if ( !config.enable || !config.cap ) + if ( !config.enable ) return true; - return ::free_space(n, config.cap, s_tracker, prune_handler); + return memory::free_space(n, config.cap, s_tracker, prune_handler); } -void DefaultCap::update_allocations(size_t n) +void MemoryCap::update_allocations(size_t n) { - if ( is_packet_thread() ) - s_tracker.allocated += n; - + s_tracker.allocated += n; mp_active_context.update_allocs(n); } -void DefaultCap::update_deallocations(size_t n) +void MemoryCap::update_deallocations(size_t n) { - if ( is_packet_thread() ) - s_tracker.deallocated += n; - + s_tracker.deallocated += n; mp_active_context.update_deallocs(n); } +// FIXIT-H J need to validate and print out warnings for configurations +// that result in very low values for per-thread memory +void MemoryCap::calculate(unsigned num_threads) +{ + if ( !snort_conf->memory->enable ) + return; + + s_config.enable = snort_conf->memory->enable; + assert(snort_conf->memory->cap >= s_tracker.used()); + + auto remaining = snort_conf->memory->cap - s_tracker.used(); + auto per_thread_cap = remaining / num_threads; + + assert(per_thread_cap > 0); + + s_config.cap = per_thread_cap; + + s_main_config = &s_config; + + DebugFormat(DEBUG_MEMORY, + ("local memcap set: %zu startup cost, " + "%zu available (%zu per-thread)\n"), + s_tracker.used(), remaining, per_thread_cap, num_threads); +} + +void MemoryCap::tinit() +{ + if ( s_main_config ) + s_config = *s_main_config; +} + } // namespace memory #ifdef UNIT_TEST @@ -150,7 +181,7 @@ TEST_CASE( "memory cap free space", "[memory]" ) MockTracker tracker { 0 }; HandlerSpy handler { 1, tracker }; - CHECK( ::free_space(1, 1024, tracker, handler) ); + CHECK( memory::free_space(1, 1024, tracker, handler) ); CHECK_FALSE( handler.called ); } @@ -159,7 +190,7 @@ TEST_CASE( "memory cap free space", "[memory]" ) MockTracker tracker { 1024 }; HandlerSpy handler { 1023, tracker }; - CHECK( ::free_space(1, 1024, tracker, handler) ); + CHECK( memory::free_space(1, 1024, tracker, handler) ); CHECK( handler.called ); CHECK( tracker.result == handler.modify_tracker ); } @@ -169,7 +200,7 @@ TEST_CASE( "memory cap free space", "[memory]" ) MockTracker tracker { 1024 }; HandlerSpy handler { 0, tracker }; - CHECK_FALSE( ::free_space(1, 1024, tracker, handler) ); + CHECK_FALSE( memory::free_space(1, 1024, tracker, handler) ); CHECK( handler.called ); CHECK( tracker.result == 1024 ); } diff --git a/src/memory/memory_cap.h b/src/memory/memory_cap.h index 67d70da17..ccbf4c9b4 100644 --- a/src/memory/memory_cap.h +++ b/src/memory/memory_cap.h @@ -26,11 +26,17 @@ namespace memory { -struct DefaultCap +struct MemoryCap { static bool free_space(size_t); static void update_allocations(size_t); static void update_deallocations(size_t); + + // call from main thread before thread spawn + static void calculate(unsigned num_threads); + + // call from threads + static void tinit(); }; } // namespace memory diff --git a/src/memory/memory_config.h b/src/memory/memory_config.h index e36f212e8..7d29f2e50 100644 --- a/src/memory/memory_config.h +++ b/src/memory/memory_config.h @@ -27,6 +27,8 @@ struct MemoryConfig { bool enable = false; size_t cap = 0; + + constexpr MemoryConfig() = default; }; #endif diff --git a/src/memory/memory_manager.cc b/src/memory/memory_manager.cc index 8609aeef7..99584b40f 100644 --- a/src/memory/memory_manager.cc +++ b/src/memory/memory_manager.cc @@ -18,13 +18,15 @@ // memory_manager.cc author Joel Cornett +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + #include #include #include -#ifdef HAVE_CONFIG_H -#include "config.h" -#endif +#include "main/thread.h" #include "memory_allocator.h" #include "memory_cap.h" @@ -53,11 +55,14 @@ struct Metadata Metadata(size_t = 0); - static size_t SANITY_CHECK_VALUE; - static size_t calculate_total_size(size_t); - template static Metadata* create(size_t); + + template + static Metadata* create(size_t); + static Metadata* extract(void*); + + static size_t SANITY_CHECK_VALUE; }; inline size_t Metadata::total_size() const @@ -73,16 +78,14 @@ inline Metadata::Metadata(size_t n) : sanity(SANITY_CHECK_VALUE), payload_size(n) { } -size_t Metadata::SANITY_CHECK_VALUE = 0xabcdef; - inline size_t Metadata::calculate_total_size(size_t n) { return sizeof(Metadata) + n; } -template +template Metadata* Metadata::create(size_t n) { auto meta = - static_cast(Shim::allocate(calculate_total_size(n))); + static_cast(Allocator::allocate(calculate_total_size(n))); if ( !meta ) return nullptr; @@ -105,20 +108,46 @@ Metadata* Metadata::extract(void* p) return meta; } +size_t Metadata::SANITY_CHECK_VALUE = 0xabcdef; + // ----------------------------------------------------------------------------- // the meat // ----------------------------------------------------------------------------- -template +class ReentryContext +{ +public: + ReentryContext(bool& flag) : + already_entered(flag), flag(flag) + { flag = true; } + + ~ReentryContext() + { flag = false; } + + bool is_reentry() const + { return already_entered; } + +private: + const bool already_entered; + bool& flag; +}; + +template struct Interface { static void* allocate(size_t); static void deallocate(void*); + + static THREAD_LOCAL bool in_allocation_call; }; template void* Interface::allocate(size_t n) { + // prevent allocation reentry + ReentryContext reentry_context(in_allocation_call); + assert(!reentry_context.is_reentry()); + if ( !Cap::free_space(Metadata::calculate_total_size(n)) ) return nullptr; @@ -143,6 +172,9 @@ void Interface::deallocate(void* p) Allocator::deallocate(meta); } +template +THREAD_LOCAL bool Interface::in_allocation_call = false; + } //namespace memory // -----------------------------------------------------------------------------