Merge pull request #302 in SNORT/snort3 from ~JOCORNET/snort3:memory to master

Squashed commit of the following:

commit 96b932a314ef5a906bcf6aa144853c36d1a4410f
Author: Joel Cornett <joel.cornett@gmail.com>
Date:   Mon Feb 29 15:21:08 2016 -0500

    prevent allocation reentry

commit 69026ddc973bc19e4ca0f22609f8de2bf0f872f3
Author: Joel Cornett <joel.cornett@gmail.com>
Date:   Mon Feb 29 10:38:15 2016 -0500

    added checks

commit f3a80f12654a4451ab5a061f3249745deb3b13c0
Author: Joel Cornett <joel.cornett@gmail.com>
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 <joel.cornett@gmail.com>
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 6ab2fce5a02838e8166dff87fe8d03810a744dfd..7f7a8a7348cb51e635ce889e87de445f91b4c584 100644 (file)
--- 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 2133b82e59e45eeeef27ef7d41a441f5ddf893c9..70b067d7546edfb7b136e3e6a6dfdee66051b43b 100644 (file)
--- 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 1f3b974e106559681e783d3077b6a3b8e8567ff9..acd062af9a930b55ce79a958233285ed95e83804 100644 (file)
--- 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 18778c0211aaad9d08352058c36a2264e20f3b1f..74fa04a7d54d148c03d0a1b145b49a85ddfae653 100644 (file)
--- 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 ff6bc392f48ad7a06ce005e0fcb23b119b676d98..0c3a39a8041031927019cb3f66756351e2b8dd64 100644 (file)
--- 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 cfd3f46ab19bab1697323e330fc7d58b2a6517e0..bb0413548913012ca7b467edaffa11eb25210b54 100644 (file)
--- a/src/memory/memory_cap.cc
+++ b/src/memory/memory_cap.cc
@@ -27,6 +27,7 @@
 #include <cassert>
 
 #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<typename Tracker, typename Handler>
-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<typename Tracker, typename Handler>
+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 67d70da17b3182fa7907406f4af74fb5a67bffce..ccbf4c9b4dc12eaa4e07912c3a1f4f8ba44decf3 100644 (file)
--- 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 e36f212e85364d1280a0fddf49c8e77b46e605e0..7d29f2e50fc973983b40e3b5b3fb115ba0d5b22e 100644 (file)
--- 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 8609aeef771e6679b3ddf19672df34f1a5529d3b..99584b40f87565c797e4bd72be8807a9cb147b6f 100644 (file)
--- a/src/memory/memory_manager.cc
+++ b/src/memory/memory_manager.cc
@@ -18,13 +18,15 @@
 
 // memory_manager.cc author Joel Cornett <jocornet@cisco.com>
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <new>
 #include <cstdio>
 #include <cassert>
 
-#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<typename Shim> static Metadata* create(size_t);
+
+    template<typename Allocator>
+    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<typename Shim>
+template<typename Allocator>
 Metadata* Metadata::create(size_t n)
 {
     auto meta =
-        static_cast<Metadata*>(Shim::allocate(calculate_total_size(n)));
+        static_cast<Metadata*>(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<typename Allocator = DefaultAllocator, typename Cap = DefaultCap>
+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<typename Allocator = MemoryAllocator, typename Cap = MemoryCap>
 struct Interface
 {
     static void* allocate(size_t);
     static void deallocate(void*);
+
+    static THREAD_LOCAL bool in_allocation_call;
 };
 
 template<typename Allocator, typename Cap>
 void* Interface<Allocator, Cap>::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<Allocator, Cap>::deallocate(void* p)
     Allocator::deallocate(meta);
 }
 
+template<typename Allocator, typename Cap>
+THREAD_LOCAL bool Interface<Allocator, Cap>::in_allocation_call = false;
+
 } //namespace memory
 
 // -----------------------------------------------------------------------------