From: Ron Dempster (rdempste) <rdempste@cisco.com>
Date: Tue, 6 Oct 2020 22:00:14 +0000 (+0000)
Subject: Merge pull request #2530 in SNORT/snort3 from ~RDEMPSTE/snort3:rrt to master
X-Git-Tag: 3.0.3-2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=79d9bf66c7291d995613926a8d4f6eedae23592d;p=thirdparty%2Fsnort3.git

Merge pull request #2530 in SNORT/snort3 from ~RDEMPSTE/snort3:rrt to master

Squashed commit of the following:

commit 9b53cbafd6645a86e1665d53f58cd614e0773d74
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Mon Oct 5 19:06:46 2020 -0400

    rna: Update rna to use instance based reload tuner

commit c217dfd4694b63b5e2ff5967afc6d817b720c964
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Mon Oct 5 18:57:22 2020 -0400

    stream: Update stream to use instance based reload tuner

commit bb8a5b14ba4c136adfbe629a8a877c7c4260f6b5
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Mon Oct 5 18:39:37 2020 -0400

    port_scan: Update port scan to use instance based reload tuner

commit 23d389d7f2b10e63d1107672e49ec2ce569055f4
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Mon Oct 5 18:39:14 2020 -0400

    perf_monitor: Update perf monitor to use instance based reload tuner

commit 32e23d8e8731580f396924a000e0ccdcccbcdea9
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Mon Oct 5 18:38:50 2020 -0400

    appid: Update appid to use instance based reload tuner

commit 395f4974a4f0613546dc03002e7b270add17b48b
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Tue Oct 6 12:54:04 2020 -0400

    host_tracker: Update host tracker to use instance based reload tuner

commit a5d808b82e0e68d8b2979fa765e608ded4e397c5
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Mon Oct 5 18:37:44 2020 -0400

    main: Update host attribute class to use instance based reload tuner

commit 2746eb3fdf7e2e0125770237bb53af94f5ec3324
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Mon Oct 5 18:36:26 2020 -0400

    main: Change reload memcap framework to use object instances
---

diff --git a/src/host_tracker/host_cache_module.cc b/src/host_tracker/host_cache_module.cc
index ce962e47f..e9279b786 100644
--- a/src/host_tracker/host_cache_module.cc
+++ b/src/host_tracker/host_cache_module.cc
@@ -88,27 +88,21 @@ bool HostCacheModule::set(const char*, Value& v, SnortConfig*)
         dump_file = snort_strdup(v.get_string());
     }
     else if ( v.is("memcap") )
-        hc_rrt.memcap = v.get_size();
+        memcap = v.get_size();
     else
         return false;
 
     return true;
 }
 
-bool HostCacheModule::begin(const char*, int, SnortConfig*)
-{
-    hc_rrt.memcap = 0;
-    return true;
-}
-
 bool HostCacheModule::end(const char* fqn, int, SnortConfig* sc)
 {
-    if ( hc_rrt.memcap && !strcmp(fqn, HOST_CACHE_NAME) )
+    if ( memcap && !strcmp(fqn, HOST_CACHE_NAME) )
     {
         if ( Snort::is_reloading() )
-            sc->register_reload_resource_tuner(hc_rrt);
+            sc->register_reload_resource_tuner(new HostCacheReloadTuner(memcap));
         else
-            host_cache.set_max_size(hc_rrt.memcap);
+            host_cache.set_max_size(memcap);
     }
 
     return true;
diff --git a/src/host_tracker/host_cache_module.h b/src/host_tracker/host_cache_module.h
index e0f1bc2b3..1585c5d5e 100644
--- a/src/host_tracker/host_cache_module.h
+++ b/src/host_tracker/host_cache_module.h
@@ -35,6 +35,7 @@
 class HostCacheReloadTuner : public snort::ReloadResourceTuner
 {
 public:
+    explicit HostCacheReloadTuner(size_t memcap) : memcap(memcap) { }
     bool tinit() override
     { return host_cache.reload_resize(memcap); }
 
@@ -44,6 +45,7 @@ public:
     bool tune_packet_context() override
     { return host_cache.reload_prune(memcap, max_work); }
 
+private:
     size_t memcap;
 };
 
@@ -53,7 +55,6 @@ public:
     HostCacheModule();
     ~HostCacheModule() override;
 
-    bool begin(const char*, int, snort::SnortConfig*) override;
     bool end(const char*, int, snort::SnortConfig*) override;
     bool set(const char*, snort::Value&, snort::SnortConfig*) override;
 
@@ -69,7 +70,7 @@ public:
 
 private:
     const char* dump_file = nullptr;
-    HostCacheReloadTuner hc_rrt;
+    size_t memcap = 0;
 };
 
 #endif
diff --git a/src/host_tracker/test/host_cache_module_test.cc b/src/host_tracker/test/host_cache_module_test.cc
index b7b8391e5..261e67df4 100644
--- a/src/host_tracker/test/host_cache_module_test.cc
+++ b/src/host_tracker/test/host_cache_module_test.cc
@@ -65,6 +65,7 @@ void LogMessage(const char* format,...)
 }
 time_t packet_time() { return 0; }
 bool Snort::is_reloading() { return false; }
+void SnortConfig::register_reload_resource_tuner(ReloadResourceTuner* rrt) { delete rrt; }
 } // end of namespace snort
 
 extern "C"
diff --git a/src/main/modules.cc b/src/main/modules.cc
index 76428b53b..62be9831f 100644
--- a/src/main/modules.cc
+++ b/src/main/modules.cc
@@ -1899,7 +1899,6 @@ public:
 private:
     HostServiceDescriptor service;
     HostAttributesEntry host;
-    HostAttributesReloadTuner hart;
 };
 
 bool HostsModule::set(const char*, Value& v, SnortConfig* sc)
@@ -1961,7 +1960,7 @@ bool HostsModule::end(const char* fqn, int idx, SnortConfig* sc)
         if ( HostAttributesManager::activate() )
         {
             if ( Snort::is_reloading() )
-                sc->register_reload_resource_tuner(hart);
+                sc->register_reload_resource_tuner(new HostAttributesReloadTuner);
         }
     }
 
diff --git a/src/main/snort_config.cc b/src/main/snort_config.cc
index 1cf328ac1..d585bf189 100644
--- a/src/main/snort_config.cc
+++ b/src/main/snort_config.cc
@@ -60,6 +60,7 @@
 #include "profiler/profiler.h"
 #include "protocols/packet.h"
 #include "sfip/sf_ip.h"
+#include "main/snort.h"
 #include "target_based/host_attributes.h"
 #include "target_based/snort_protocols.h"
 #include "trace/trace_config.h"
@@ -275,7 +276,7 @@ SnortConfig::~SnortConfig()
     delete so_rules;
     if ( plugins )
         delete plugins;
-    reload_tuners.clear();
+    clear_reload_resource_tuner_list();
 
     trim_heap();
 }
@@ -1009,6 +1010,21 @@ void SnortConfig::set_conf(const SnortConfig* sc)
     }
 }
 
+void SnortConfig::register_reload_resource_tuner(ReloadResourceTuner* rrt)
+{
+    if (Snort::is_reloading())
+        reload_tuners.push_back(rrt);
+    else
+        delete rrt;
+}
+
+void SnortConfig::clear_reload_resource_tuner_list()
+{
+    for (ReloadResourceTuner* rrt : reload_tuners)
+        delete rrt;
+    reload_tuners.clear();
+}
+
 void SnortConfig::cleanup_fatal_error()
 {
     // FIXIT-L need a generic way to manage type other threads
diff --git a/src/main/snort_config.h b/src/main/snort_config.h
index 40f8ac6e9..3c8a85a78 100644
--- a/src/main/snort_config.h
+++ b/src/main/snort_config.h
@@ -703,8 +703,7 @@ public:
     const std::list<ReloadResourceTuner*>& get_reload_resource_tuners() const
     { return reload_tuners; }
 
-    void clear_reload_resource_tuner_list()
-    { reload_tuners.clear(); }
+    void clear_reload_resource_tuner_list();
 
     bool get_default_rule_state() const;
 
@@ -730,8 +729,7 @@ public:
 
     static void set_conf(const SnortConfig*);
 
-    SO_PUBLIC void register_reload_resource_tuner(ReloadResourceTuner& rrt)
-    { reload_tuners.push_back(&rrt); }
+    SO_PUBLIC void register_reload_resource_tuner(ReloadResourceTuner*);
 
     static void cleanup_fatal_error();
 };
diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc
index 9a661019a..b551686db 100644
--- a/src/network_inspectors/appid/appid_module.cc
+++ b/src/network_inspectors/appid/appid_module.cc
@@ -367,7 +367,7 @@ static int reload_detectors(lua_State* L)
     assert(odp_thread_local_ctxt);
     delete odp_thread_local_ctxt;
     odp_thread_local_ctxt = new OdpThreadContext(true);
- 
+
     OdpContext& odp_ctxt = ctxt.get_odp_ctxt();
     odp_ctxt.get_client_disco_mgr().initialize();
     odp_ctxt.get_service_disco_mgr().initialize();
@@ -493,12 +493,8 @@ bool AppIdModule::end(const char* fqn, int, SnortConfig* sc)
 {
     assert(config);
 
-    if ( strcmp(fqn, "appid") == 0 )
-    {
-        appid_rrt.memcap = config->memcap;
-        if ( Snort::is_reloading() )
-            sc->register_reload_resource_tuner(appid_rrt);
-    }
+    if ( Snort::is_reloading() && strcmp(fqn, "appid") == 0 )
+        sc->register_reload_resource_tuner(new AppIdReloadTuner(config->memcap));
 
     if ( !config->app_detector_dir )
     {
diff --git a/src/network_inspectors/appid/appid_module.h b/src/network_inspectors/appid/appid_module.h
index d2197f8ad..df3afa24b 100644
--- a/src/network_inspectors/appid/appid_module.h
+++ b/src/network_inspectors/appid/appid_module.h
@@ -46,7 +46,8 @@ extern THREAD_LOCAL const snort::Trace* appid_trace;
 class AppIdReloadTuner : public snort::ReloadResourceTuner
 {
 public:
-    AppIdReloadTuner() = default;
+    explicit AppIdReloadTuner(size_t memcap) : memcap(memcap) { }
+    ~AppIdReloadTuner() override = default;
 
     bool tinit() override;
     bool tune_packet_context() override
@@ -61,7 +62,7 @@ public:
     friend class AppIdModule;
 
 private:
-    size_t memcap = 0;
+    size_t memcap;
 
     bool tune_resources(unsigned work_limit);
 };
@@ -95,7 +96,6 @@ public:
 
 private:
     AppIdConfig* config;
-    AppIdReloadTuner appid_rrt;
 };
 
 #endif
diff --git a/src/network_inspectors/perf_monitor/perf_module.cc b/src/network_inspectors/perf_monitor/perf_module.cc
index 60aeda899..adb1f82d5 100644
--- a/src/network_inspectors/perf_monitor/perf_module.cc
+++ b/src/network_inspectors/perf_monitor/perf_module.cc
@@ -333,15 +333,8 @@ bool PerfMonModule::begin(const char* fqn, int idx, SnortConfig*)
 bool PerfMonModule::end(const char* fqn, int idx, SnortConfig* sc)
 {
 
-    if ( strcmp(fqn, "perf_monitor") == 0 )
-    {
-        perfmon_rrt.set_memcap(config->flowip_memcap);
-
-        if ( Snort::is_reloading() )
-        {
-            sc->register_reload_resource_tuner(perfmon_rrt);
-        }
-    }
+    if ( Snort::is_reloading() && strcmp(fqn, "perf_monitor") == 0 )
+        sc->register_reload_resource_tuner(new PerfMonReloadTuner(config->flowip_memcap));
 
     if ( idx != 0 && strcmp(fqn, "perf_monitor.modules") == 0 )
         return config->modules.back().confirm_parse();
diff --git a/src/network_inspectors/perf_monitor/perf_module.h b/src/network_inspectors/perf_monitor/perf_module.h
index 5782b0873..85dac19b6 100644
--- a/src/network_inspectors/perf_monitor/perf_module.h
+++ b/src/network_inspectors/perf_monitor/perf_module.h
@@ -131,7 +131,6 @@ public:
 
 private:
     PerfConfig* config = nullptr;
-    PerfMonReloadTuner perfmon_rrt;
 };
 
 extern THREAD_LOCAL PerfPegStats pmstats;
diff --git a/src/network_inspectors/perf_monitor/perf_reload_tuner.h b/src/network_inspectors/perf_monitor/perf_reload_tuner.h
index d6ae7e247..9e737bed5 100644
--- a/src/network_inspectors/perf_monitor/perf_reload_tuner.h
+++ b/src/network_inspectors/perf_monitor/perf_reload_tuner.h
@@ -26,7 +26,8 @@
 class PerfMonReloadTuner : public snort::ReloadResourceTuner
 {
 public:
-    PerfMonReloadTuner() = default;
+    explicit PerfMonReloadTuner(size_t memcap) : memcap(memcap) { }
+    ~PerfMonReloadTuner() override = default;
 
     bool tinit() override;
 
@@ -38,14 +39,8 @@ public:
 
     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;
+    size_t memcap;
 
 };
 
diff --git a/src/network_inspectors/port_scan/ps_module.cc b/src/network_inspectors/port_scan/ps_module.cc
index f8db426a5..64f8d8316 100644
--- a/src/network_inspectors/port_scan/ps_module.cc
+++ b/src/network_inspectors/port_scan/ps_module.cc
@@ -328,12 +328,8 @@ bool PortScanModule::set(const char* fqn, Value& v, SnortConfig*)
 
 bool PortScanModule::end(const char* fqn, int, SnortConfig* sc)
 {
-    if ( strcmp(fqn, "port_scan") == 0 )
-    {
-        ps_rrt.memcap = config->memcap;
-        if ( Snort::is_reloading() )
-            sc->register_reload_resource_tuner(ps_rrt);
-    }
+    if ( Snort::is_reloading() && strcmp(fqn, "port_scan") == 0 )
+        sc->register_reload_resource_tuner(new PortScanReloadTuner(config->memcap));
     return true;
 }
 
diff --git a/src/network_inspectors/port_scan/ps_module.h b/src/network_inspectors/port_scan/ps_module.h
index 0ba6e2c2b..5345defd3 100644
--- a/src/network_inspectors/port_scan/ps_module.h
+++ b/src/network_inspectors/port_scan/ps_module.h
@@ -137,6 +137,9 @@
 class PortScanReloadTuner : public snort::ReloadResourceTuner
 {
 public:
+    explicit PortScanReloadTuner(size_t memcap) : memcap(memcap) { }
+    ~PortScanReloadTuner() override = default;
+
     bool tinit() override
     { return ps_init_hash(memcap); }
 
@@ -146,7 +149,8 @@ public:
     bool tune_packet_context() override
     { return ps_prune_hash(max_work); }
 
-    size_t memcap = 0;
+private:
+    size_t memcap;
 };
 
 //-------------------------------------------------------------------------
@@ -184,7 +188,6 @@ public:
 private:
     PS_ALERT_CONF* get_alert_conf(const char* fqn);
     PortscanConfig* config;
-    PortScanReloadTuner ps_rrt;
 };
 
 #endif
diff --git a/src/network_inspectors/rna/rna_module.cc b/src/network_inspectors/rna/rna_module.cc
index fef5a6eac..6a9e7ef9b 100644
--- a/src/network_inspectors/rna/rna_module.cc
+++ b/src/network_inspectors/rna/rna_module.cc
@@ -78,8 +78,8 @@ static inline string format_dump_mac(const uint8_t mac[MAC_SIZE])
 
 bool FpProcReloadTuner::tinit()
 {
-    set_tcp_fp_processor(mod_conf->tcp_processor);
-    set_ua_fp_processor(mod_conf->ua_processor);
+    set_tcp_fp_processor(mod_conf.tcp_processor);
+    set_ua_fp_processor(mod_conf.ua_processor);
     return false;  // no work to do after this
 }
 
@@ -312,11 +312,7 @@ bool RnaModule::end(const char* fqn, int index, SnortConfig* sc)
             mod_conf->ua_processor->make_mpse(sc);
 
         if ( (mod_conf->tcp_processor || mod_conf->ua_processor) && Snort::is_reloading() )
-        {
-            fprt.mod_conf = mod_conf;
-            sc->register_reload_resource_tuner(fprt);
-        }
-
+            sc->register_reload_resource_tuner(new FpProcReloadTuner(*mod_conf));
     }
 
     if ( index > 0 and mod_conf->tcp_processor and !strcmp(fqn, "rna.tcp_fingerprints") )
diff --git a/src/network_inspectors/rna/rna_module.h b/src/network_inspectors/rna/rna_module.h
index fe4747da0..4fbcd0bae 100644
--- a/src/network_inspectors/rna/rna_module.h
+++ b/src/network_inspectors/rna/rna_module.h
@@ -55,7 +55,8 @@ extern THREAD_LOCAL const snort::Trace* rna_trace;
 class FpProcReloadTuner : public snort::ReloadResourceTuner
 {
 public:
-    FpProcReloadTuner() = default;
+    explicit FpProcReloadTuner(RnaModuleConfig& mod_conf) : mod_conf(mod_conf) { }
+    ~FpProcReloadTuner() override = default;
 
     bool tinit() override;
 
@@ -65,7 +66,8 @@ public:
     bool tune_idle_context() override
     { return true; }
 
-    RnaModuleConfig* mod_conf = nullptr;
+private:
+    RnaModuleConfig& mod_conf;
 };
 
 class RnaModule : public snort::Module
@@ -97,8 +99,6 @@ private:
 
     RawFingerprint fingerprint;
 
-    FpProcReloadTuner fprt;
-
     bool is_valid_fqn(const char* fqn) const;
 };
 
diff --git a/src/network_inspectors/rna/test/rna_module_test.cc b/src/network_inspectors/rna/test/rna_module_test.cc
index d088c402e..e0d653d61 100644
--- a/src/network_inspectors/rna/test/rna_module_test.cc
+++ b/src/network_inspectors/rna/test/rna_module_test.cc
@@ -42,6 +42,7 @@ void set_ua_fp_processor(UaFpProcessor*) { }
 namespace snort
 {
     bool Snort::is_reloading() { return false; }
+    void SnortConfig::register_reload_resource_tuner(ReloadResourceTuner* rrt) { delete rrt; }
 }
 
 TEST_GROUP(rna_module_test)
diff --git a/src/stream/base/stream_module.cc b/src/stream/base/stream_module.cc
index 0a8450ec9..3157338f8 100644
--- a/src/stream/base/stream_module.cc
+++ b/src/stream/base/stream_module.cc
@@ -201,13 +201,18 @@ bool StreamModule::set(const char* fqn, Value& v, SnortConfig* c)
     return true;
 }
 
-bool StreamModule::end(const char*, int, SnortConfig* sc)
+bool StreamModule::end(const char* fqn, int, SnortConfig* sc)
 {
-    if ( reload_resource_manager.initialize(config) )
-        sc->register_reload_resource_tuner(reload_resource_manager);
+    if ( Snort::is_reloading() && strcmp(fqn, MOD_NAME) == 0 )
+    {
+        StreamReloadResourceManager* reload_resource_manager = new StreamReloadResourceManager;
+        if (reload_resource_manager->initialize(config))
+            sc->register_reload_resource_tuner(reload_resource_manager);
+        else
+            delete reload_resource_manager;
 
-    if ( hpq_rrt.initialize(config.held_packet_timeout) )
-        sc->register_reload_resource_tuner(hpq_rrt);
+        sc->register_reload_resource_tuner(new HPQReloadTuner(config.held_packet_timeout));
+    }
 
     return true;
 }
@@ -229,12 +234,6 @@ bool StreamReloadResourceManager::initialize(const StreamModuleConfig& config_)
 {
     // saving a copy of the config only works here because there is only
     // one stream inspector per packet thread...
-    if ( !Snort::is_reloading() )
-    {
-        config = config_;
-        return false;
-    }
-
 #ifdef REG_TEST
     if ( config.footprint != config_.footprint )
     {
@@ -309,12 +308,6 @@ void StreamModuleConfig::show() const
     }
 }
 
-bool HPQReloadTuner::initialize(uint32_t new_timeout_ms)
-{
-    held_packet_timeout = new_timeout_ms;
-    return Snort::is_reloading();
-}
-
 bool HPQReloadTuner::tinit()
 {
     packet_gettimeofday(&reload_time);
diff --git a/src/stream/base/stream_module.h b/src/stream/base/stream_module.h
index 7b494ef60..fcf54c1a4 100644
--- a/src/stream/base/stream_module.h
+++ b/src/stream/base/stream_module.h
@@ -105,16 +105,16 @@ private:
 class HPQReloadTuner : public snort::ReloadResourceTuner
 {
 public:
-    HPQReloadTuner() = default;
+    explicit HPQReloadTuner(uint32_t packet_timeout) : held_packet_timeout(packet_timeout) { }
+    ~HPQReloadTuner() override = default;
 
     bool tinit() override;
     bool tune_packet_context() override;
     bool tune_idle_context() override;
-    bool initialize(uint32_t new_timeout_ms);
 
 private:
     uint32_t held_packet_timeout;
-    timeval reload_time;
+    timeval reload_time{};
 };
 
 class StreamModule : public snort::Module
@@ -150,8 +150,6 @@ public:
 
 private:
     StreamModuleConfig config;
-    StreamReloadResourceManager reload_resource_manager;
-    HPQReloadTuner hpq_rrt;
 };
 
 extern void base_prep();