From: Mike Stepanek (mstepane) <mstepane@cisco.com>
Date: Mon, 16 Dec 2019 20:39:58 +0000 (+0000)
Subject: Merge pull request #1873 in SNORT/snort3 from ~MMATIRKO/snort3:perfmon_reload_tune... 
X-Git-Tag: 3.0.0-267~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=40399cafe47489abf004b9b64b0ea982186b0027;p=thirdparty%2Fsnort3.git

Merge pull request #1873 in SNORT/snort3 from ~MMATIRKO/snort3:perfmon_reload_tune to master

Squashed commit of the following:

commit 1330041efbc4410e003415c46d96dfff98929d60
Author: Michael Matirko <mmatirko@cisco.com>
Date:   Mon Nov 25 15:01:39 2019 -0500

    perf_monitor: tuning for flow_ip_memcap on reload
---

diff --git a/src/hash/xhash.cc b/src/hash/xhash.cc
index 6c001e593..72338cbb7 100644
--- a/src/hash/xhash.cc
+++ b/src/hash/xhash.cc
@@ -261,7 +261,7 @@ static void xhash_delete_free_list(XHash* t)
 }
 
 /*!
- * Try to change the memcap
+ *  Try to change the memcap
  *  Behavior is undefined when t->usrfree is set
  *
  * t             SFXHASH table pointer
@@ -309,11 +309,10 @@ int xhash_change_memcap(XHash *t, unsigned long new_memcap, unsigned *max_work)
     return XHASH_OK;
 }
 
-
 /*!
  *  Delete the hash Table
  *
- *  free key's, free node's, and free the users data.
+ *  free keys, free nodes, and free the users data.
  *
  * h XHash table pointer
  *
@@ -548,7 +547,7 @@ static void movetofront(XHash* t, XHashNode* n)
 }
 
 /*
- * Allocat a new hash node, uses Auto Node Recovery if needed and enabled.
+ * Allocate a new hash node, uses Auto Node Recovery if needed and enabled.
  *
  * The oldest node is the one with the longest time since it was last touched,
  * and does not have any direct indication of how long the node has been around.
@@ -747,10 +746,8 @@ int xhash_add(XHash* t, void* key, void* data)
  * t XHash table pointer
  * key  users key pointer
  *
- * return integer
- * retval XHASH_OK      success
- * retval XHASH_INTABLE already in the table, t->cnode points to the node
- * retval XHASH_NOMEM   not enough memory
+ * return XHashNode*
+ * retval nullptr      failed to add node
  */
 XHashNode* xhash_get_node(XHash* t, const void* key)
 {
@@ -806,6 +803,38 @@ XHashNode* xhash_get_node(XHash* t, const void* key)
     return hnode;
 }
 
+/*!
+ * Add a key to the hash table, return the hash node
+ * If space is not available, it will remove a single
+ * node (the LRU) and replace it with a new node.
+ *
+ * This function handles the scenario that we don't
+ * have enough room to allocate a new node - if this is the
+ * case, additional pruning should be done
+ *
+ * t XHash table pointer
+ * key  users key pointer
+ * prune_performed  bool pointer, 1 = successful prune, 0 = no/failed prune
+ *
+ * return XHashNode*
+ * retval XhashNode*    successfully allocated
+ * retval nullptr       failed to allocate
+ */
+XHashNode* xhash_get_node_with_prune(XHash* t, const void* key, bool* prune_performed)
+{
+    size_t mem_after_alloc = t->mc.memused + xhash_required_mem(t);
+    bool over_capacity = (t->mc.memcap < mem_after_alloc);
+    XHashNode* hnode = nullptr;
+
+    if (over_capacity)
+        *prune_performed = (xhash_free_anr_lru(t) == XHASH_OK);
+
+    if (*prune_performed or !over_capacity)
+        hnode = xhash_get_node(t, key);
+
+    return hnode;
+}
+
 /*!
  * Find a Node based on the key
  *
@@ -1065,6 +1094,33 @@ int xhash_free_anr_lru(XHash *t)
     return XHASH_ERR;
 }
 
+/*!
+ * Unlink and free nodes if the xhash is exceeding the
+ * specified memcap
+ *
+ * t            XHash table pointer
+ * num_freed    set to the number of nodes freed
+ * work_limit   the max amount of work to do
+ *
+ * returns
+ * XHASH_ERR if error occurs
+ * XHASH_OK  if node(s) freed and memused is within memcap
+ * XHASH_PENDING if additional pending is required
+ */
+int xhash_free_overallocations(XHash* t, unsigned work_limit, unsigned* num_freed)
+{
+
+    while (t->mc.memcap < t->mc.memused and work_limit--)
+    {
+        if (xhash_free_anr_lru(t) != XHASH_OK)
+            return XHASH_ERR;
+
+        ++*num_freed;
+    }
+
+    return (t->mc.memcap > t->mc.memused) ? XHASH_OK : XHASH_PENDING;
+}
+
 /*!
  * Find and return the first hash table node
  *
diff --git a/src/hash/xhash.h b/src/hash/xhash.h
index 977f2f5ea..ff530e383 100644
--- a/src/hash/xhash.h
+++ b/src/hash/xhash.h
@@ -101,11 +101,13 @@ SO_PUBLIC XHash* xhash_new(int nrows, int keysize, int datasize, unsigned long m
 
 SO_PUBLIC void xhash_set_max_nodes(XHash* h, int max_nodes);
 SO_PUBLIC int xhash_change_memcap(XHash *t, unsigned long new_memcap, unsigned *max_work);
+SO_PUBLIC int xhash_free_overallocations(XHash* t, unsigned work_limit, unsigned* num_freed);
 SO_PUBLIC void xhash_delete(XHash* h);
 SO_PUBLIC int xhash_make_empty(XHash*);
 
 SO_PUBLIC int xhash_add(XHash* h, void* key, void* data);
 SO_PUBLIC XHashNode* xhash_get_node(XHash* t, const void* key);
+SO_PUBLIC XHashNode* xhash_get_node_with_prune(XHash* t, const void* key, bool* prune_performed);
 SO_PUBLIC int xhash_remove(XHash* h, void* key);
 
 //  Get the # of Nodes in HASH the table
@@ -136,6 +138,10 @@ inline unsigned xhash_overhead_bytes(XHash* t)
 inline unsigned xhash_overhead_blocks(XHash* t)
 { return t->overhead_blocks; }
 
+// Get the amount of space required to allocate a new node in the xhash t.
+inline size_t xhash_required_mem(XHash *t)
+{ return sizeof(XHashNode) + t->pad + t->keysize + t->datasize; }
+
 SO_PUBLIC int xhash_free_anr_lru(XHash* t);
 SO_PUBLIC void* xhash_mru(XHash* t);
 SO_PUBLIC void* xhash_lru(XHash* t);
diff --git a/src/main/snort.h b/src/main/snort.h
index 0d51b1686..1ca9725d4 100644
--- a/src/main/snort.h
+++ b/src/main/snort.h
@@ -47,8 +47,8 @@ public:
     static void cleanup();
 
     static bool is_starting();
-    static bool is_reloading();
     static bool has_dropped_privileges();
+    SO_PUBLIC static bool is_reloading();
 
 private:
     static void init(int, char**);
diff --git a/src/network_inspectors/perf_monitor/CMakeLists.txt b/src/network_inspectors/perf_monitor/CMakeLists.txt
index 0c8c38e9d..f13548656 100644
--- a/src/network_inspectors/perf_monitor/CMakeLists.txt
+++ b/src/network_inspectors/perf_monitor/CMakeLists.txt
@@ -21,6 +21,8 @@ set ( FILE_LIST
     perf_module.cc
     perf_module.h
     perf_monitor.cc
+    perf_pegs.h
+    perf_reload_tuner.h
     perf_tracker.cc
     perf_tracker.h
     text_formatter.cc
diff --git a/src/network_inspectors/perf_monitor/flow_ip_tracker.cc b/src/network_inspectors/perf_monitor/flow_ip_tracker.cc
index 8f074a209..d64798d3b 100644
--- a/src/network_inspectors/perf_monitor/flow_ip_tracker.cc
+++ b/src/network_inspectors/perf_monitor/flow_ip_tracker.cc
@@ -27,8 +27,12 @@
 #include "log/messages.h"
 #include "protocols/packet.h"
 
+#include "perf_pegs.h"
+
 using namespace snort;
 
+// The default number of rows used for the xhash ip_map
+#define DEFAULT_XHASH_NROWS 1021
 #define TRACKER_NAME PERF_NAME "_flow_ip"
 
 struct FlowStateKey
@@ -42,6 +46,7 @@ FlowStateValue* FlowIPTracker::find_stats(const SfIp* src_addr, const SfIp* dst_
 {
     FlowStateKey key;
     FlowStateValue* value = nullptr;
+    bool prune_required = false;
 
     if (src_addr->less_than(*dst_addr))
     {
@@ -59,12 +64,19 @@ FlowStateValue* FlowIPTracker::find_stats(const SfIp* src_addr, const SfIp* dst_
     value = (FlowStateValue*)xhash_find(ip_map, &key);
     if (!value)
     {
-        XHashNode* node = xhash_get_node(ip_map, &key);
+        XHashNode* node = xhash_get_node_with_prune(ip_map, &key, &prune_required);
 
         if (!node)
         {
             return nullptr;
         }
+
+        if (prune_required)
+        {
+            ++pmstats.total_frees;
+            ++pmstats.alloc_prunes;
+        }
+
         memset(node->data, 0, sizeof(FlowStateValue));
         value = (FlowStateValue*)node->data;
     }
@@ -72,8 +84,27 @@ FlowStateValue* FlowIPTracker::find_stats(const SfIp* src_addr, const SfIp* dst_
     return value;
 }
 
+bool FlowIPTracker::initialize(size_t new_memcap)
+{
+    bool need_pruning = false;
+
+    if (!ip_map)
+    {
+        ip_map = xhash_new(DEFAULT_XHASH_NROWS, sizeof(FlowStateKey), sizeof(FlowStateValue),
+            new_memcap, 1, nullptr, nullptr, 1);
+    }
+    else
+    {
+        need_pruning = (new_memcap < memcap);
+        memcap = new_memcap;
+        ip_map->mc.memcap = memcap;
+    }
+
+    return need_pruning;
+}
+
 FlowIPTracker::FlowIPTracker(PerfConfig* perf) : PerfTracker(perf, TRACKER_NAME),
-    perf_flags(perf->perf_flags)
+    perf_flags(perf->perf_flags), perf_conf(perf)
 {
     formatter->register_section("flow_ip");
     formatter->register_field("ip_a", ip_a);
@@ -110,8 +141,10 @@ FlowIPTracker::FlowIPTracker(PerfConfig* perf) : PerfTracker(perf, TRACKER_NAME)
         &stats.state_changes[SFS_STATE_UDP_CREATED]);
     formatter->finalize_fields();
 
-    ip_map = xhash_new(1021, sizeof(FlowStateKey), sizeof(FlowStateValue),
-        perf->flowip_memcap, 1, nullptr, nullptr, 1);
+    memcap = perf->flowip_memcap;
+
+    ip_map = xhash_new(DEFAULT_XHASH_NROWS, sizeof(FlowStateKey), sizeof(FlowStateValue),
+        memcap, 1, nullptr, nullptr, 1);
 
     if (!ip_map)
         FatalError("Unable to allocate memory for FlowIP stats\n");
diff --git a/src/network_inspectors/perf_monitor/flow_ip_tracker.h b/src/network_inspectors/perf_monitor/flow_ip_tracker.h
index 0ee16f0f4..ca5a86963 100644
--- a/src/network_inspectors/perf_monitor/flow_ip_tracker.h
+++ b/src/network_inspectors/perf_monitor/flow_ip_tracker.h
@@ -21,10 +21,10 @@
 #ifndef FLOW_IP_TRACKER_H
 #define FLOW_IP_TRACKER_H
 
-#include "perf_tracker.h"
-
 #include "hash/xhash.h"
 
+#include "perf_tracker.h"
+
 enum FlowState
 {
     SFS_STATE_TCP_ESTABLISHED = 0,
@@ -63,21 +63,25 @@ public:
     FlowIPTracker(PerfConfig* perf);
     ~FlowIPTracker() override;
 
+    bool initialize(size_t new_memcap);
     void reset() override;
     void update(snort::Packet*) override;
     void process(bool) override;
-
     int update_state(const snort::SfIp* src_addr, const snort::SfIp* dst_addr, FlowState);
+    snort::XHash* get_ip_map()
+        { return ip_map; }
 
 private:
     FlowStateValue stats;
     snort::XHash* ip_map;
     char ip_a[41], ip_b[41];
     int perf_flags;
-
+    PerfConfig* perf_conf;
+    size_t memcap;
     FlowStateValue* find_stats(const snort::SfIp* src_addr, const snort::SfIp* dst_addr, int* swapped);
     void write_stats();
     void display_stats();
+
 };
 #endif
 
diff --git a/src/network_inspectors/perf_monitor/perf_module.cc b/src/network_inspectors/perf_monitor/perf_module.cc
index b9de75f84..31eec38ec 100644
--- a/src/network_inspectors/perf_monitor/perf_module.cc
+++ b/src/network_inspectors/perf_monitor/perf_module.cc
@@ -31,8 +31,12 @@
 #include "perf_module.h"
 
 #include "log/messages.h"
+#include "main/snort.h"
 #include "managers/module_manager.h"
 
+#include "perf_pegs.h"
+#include "perf_reload_tuner.h"
+
 using namespace snort;
 
 //-------------------------------------------------------------------------
@@ -201,8 +205,16 @@ bool PerfMonModule::begin(const char* fqn, int idx, SnortConfig*)
     return true;
 }
 
-bool PerfMonModule::end(const char* fqn, int idx, SnortConfig*)
+bool PerfMonModule::end(const char* fqn, int idx, SnortConfig* sc)
 {
+
+    perfmon_rrt.set_memcap(config->flowip_memcap);
+
+    if ( Snort::is_reloading() )
+    {
+        sc->register_reload_resource_tuner(perfmon_rrt);
+    }
+
     if ( idx != 0 && strcmp(fqn, "perf_monitor.modules") == 0 )
         return config->modules.back().confirm_parse();
 
@@ -210,14 +222,14 @@ bool PerfMonModule::end(const char* fqn, int idx, SnortConfig*)
 }
 
 PerfConfig* PerfMonModule::get_config()
-{ 
+{
     PerfConfig* tmp = config;
     config = nullptr;
-    return tmp; 
+    return tmp;
 }
 
 const PegInfo* PerfMonModule::get_pegs() const
-{ return simple_pegs; }
+{ return perf_module_pegs; }
 
 PegCount* PerfMonModule::get_counts() const
 { return (PegCount*)&pmstats; }
@@ -291,7 +303,7 @@ bool PerfConfig::resolve()
 {
     if ( modules.empty() )
     {
-        auto all_modules = ModuleManager::get_all_modules();       
+        auto all_modules = ModuleManager::get_all_modules();
         for ( auto& mod : all_modules )
         {
             ModuleConfig cfg;
diff --git a/src/network_inspectors/perf_monitor/perf_module.h b/src/network_inspectors/perf_monitor/perf_module.h
index 14353deef..6091ccf1f 100644
--- a/src/network_inspectors/perf_monitor/perf_module.h
+++ b/src/network_inspectors/perf_monitor/perf_module.h
@@ -25,6 +25,9 @@
 
 #include "framework/module.h"
 
+#include "perf_pegs.h"
+#include "perf_reload_tuner.h"
+
 #define PERF_NAME "perf_monitor"
 #define PERF_HELP "performance monitoring and flow statistics collection"
 
@@ -114,9 +117,10 @@ public:
 
 private:
     PerfConfig* config = nullptr;
+    PerfMonReloadTuner perfmon_rrt;
 };
 
-extern THREAD_LOCAL SimpleStats pmstats;
+extern THREAD_LOCAL PerfPegStats pmstats;
 extern THREAD_LOCAL snort::ProfileStats perfmonStats;
 
 #endif
diff --git a/src/network_inspectors/perf_monitor/perf_monitor.cc b/src/network_inspectors/perf_monitor/perf_monitor.cc
index c7a7f0396..0b5e913d0 100644
--- a/src/network_inspectors/perf_monitor/perf_monitor.cc
+++ b/src/network_inspectors/perf_monitor/perf_monitor.cc
@@ -29,6 +29,7 @@
 #endif
 
 #include "framework/data_bus.h"
+#include "hash/xhash.h"
 #include "log/messages.h"
 #include "managers/inspector_manager.h"
 #include "profiler/profiler.h"
@@ -40,17 +41,19 @@
 #include "flow_tracker.h"
 #include "perf_module.h"
 
+
 #ifdef UNIT_TEST
 #include "catch/snort_catch.h"
 #endif
 
 using namespace snort;
 
-THREAD_LOCAL SimpleStats pmstats;
+THREAD_LOCAL PerfPegStats pmstats;
 THREAD_LOCAL ProfileStats perfmonStats;
 
 static THREAD_LOCAL std::vector<PerfTracker*>* trackers;
 static THREAD_LOCAL FlowIPTracker* flow_ip_tracker = nullptr;
+
 //-------------------------------------------------------------------------
 // class stuff
 //-------------------------------------------------------------------------
@@ -77,7 +80,6 @@ public:
 
 private:
     PerfConfig* const config;
-
     void disable_tracker(size_t);
 };
 
@@ -256,6 +258,30 @@ void PerfMonitor::tinit()
         tracker->reset();
 }
 
+bool PerfMonReloadTuner::tinit()
+{
+    if (flow_ip_tracker)
+        return flow_ip_tracker->initialize(memcap);
+    else
+        return false;
+}
+
+bool PerfMonReloadTuner::tune_resources(unsigned work_limit)
+{
+    if (flow_ip_tracker)
+    {
+        unsigned num_freed = 0;
+        int result = xhash_free_overallocations(flow_ip_tracker->get_ip_map(), work_limit, &num_freed);
+        pmstats.total_frees += num_freed;
+        pmstats.reload_frees += num_freed;
+        return (result == XHASH_OK);
+    }
+    else
+    {
+        return false;
+    }
+}
+
 void PerfMonitor::tterm()
 {
     if (trackers)
@@ -408,7 +434,7 @@ const BaseApi* nin_perf_monitor[] =
 #ifdef UNIT_TEST
 TEST_CASE("Process timing logic", "[perfmon]")
 {
-    PerfMonModule mod; 
+    PerfMonModule mod;
     PerfConfig* config = new PerfConfig;
     mod.set_config(config);
     PerfMonitor perfmon(mod.get_config());
diff --git a/src/network_inspectors/perf_monitor/perf_pegs.h b/src/network_inspectors/perf_monitor/perf_pegs.h
new file mode 100644
index 000000000..3c8a5c4c6
--- /dev/null
+++ b/src/network_inspectors/perf_monitor/perf_pegs.h
@@ -0,0 +1,47 @@
+//--------------------------------------------------------------------------
+// Copyright (C) 2019-2019 Cisco and/or its affiliates. All rights reserved.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License Version 2 as published
+// by the Free Software Foundation.  You may not use, modify or distribute
+// this program under any other version of the GNU General Public License.
+//
+// This program is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//--------------------------------------------------------------------------
+
+// perf_pegs.h author Michael Matirko <mmatirko@cisco.com>
+
+
+#ifndef PERF_PEGS_H
+#define PERF_PEGS_H
+
+#include "framework/counts.h"
+#include "main/snort_types.h"
+
+static const PegInfo perf_module_pegs[] =
+{
+
+    { CountType::SUM, "packets", "total packets processed by performance monitor" },
+    { CountType::SUM, "total_frees", "total flows pruned or freed by performance monitor" },
+    { CountType::SUM, "reload_frees", "flows freed on reload with changed memcap" },
+    { CountType::SUM, "alloc_prunes", "flows pruned on allocation of IP flows" },
+    { CountType::END, nullptr, nullptr },
+};
+
+struct PerfPegStats
+{
+    PegCount total_packets;
+    PegCount total_frees;
+    PegCount reload_frees;
+    PegCount alloc_prunes;
+};
+
+#endif
+
diff --git a/src/network_inspectors/perf_monitor/perf_reload_tuner.h b/src/network_inspectors/perf_monitor/perf_reload_tuner.h
new file mode 100644
index 000000000..9cd8eaa55
--- /dev/null
+++ b/src/network_inspectors/perf_monitor/perf_reload_tuner.h
@@ -0,0 +1,53 @@
+//--------------------------------------------------------------------------
+// Copyright (C) 2019-2019 Cisco and/or its affiliates. All rights reserved.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License Version 2 as published
+// by the Free Software Foundation.  You may not use, modify or distribute
+// this program under any other version of the GNU General Public License.
+//
+// This program is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//--------------------------------------------------------------------------
+
+// perf_reload_tuner.h author Michael Matirko <mmatirko@cisco.com>
+
+#ifndef PERF_RELOAD_TUNER_H
+#define PERF_RELOAD_TUNER_H
+
+#include "main/snort_config.h"
+
+class PerfMonReloadTuner : public snort::ReloadResourceTuner
+{
+public:
+    PerfMonReloadTuner() = default;
+
+    bool tinit() override;
+
+    bool tune_idle_context() override
+        { return tune_resources(max_work_idle); }
+
+    bool tune_packet_context() override
+        { return tune_resources(max_work); }
+
+    bool tune_resources(unsigned work_limit);
+
+    void set_memcap(size_t new_memcap)
+        { memcap = new_memcap; }
+
+    size_t get_memcap()
+        { return memcap; }
+
+private:
+    size_t memcap = 0;
+
+};
+
+#endif
+