Merge pull request #1425 in SNORT/snort3 from ~MASHASAN/snort3:databus_clone to master

Squashed commit of the following:

commit 8ba948e060cad592234fc4b0786a0942fec30dde
Author: Masud Hasan <mashasan@cisco.com>
Date:   Thu Nov 1 00:02:28 2018 -0400

    framework: Cloning databus to new config during module reload

diff --git a/src/file_api/file_log.cc b/src/file_api/file_log.cc
index d91e1daa46fd31f4bc793ee6874997ce84215aa5..b2cc8828114033df2a724d26a5331282be7f7c02 100644 (file)
--- a/src/file_api/file_log.cc
+++ b/src/file_api/file_log.cc
@@ -83,7 +83,7 @@ static void dl_tterm()
 class LogHandler : public DataHandler
 {
 public:
-    LogHandler(FileLogConfig& conf)
+    LogHandler(FileLogConfig& conf) : DataHandler(s_name)
     { config = conf; }
 
     void handle(DataEvent&, Flow*) override;

diff --git a/src/framework/data_bus.cc b/src/framework/data_bus.cc
index 7793e9f3b6ed0464441a87eaac228e7d6e42b158..16b2c317609e213b6892206431944a8422fc93ed 100644 (file)
--- a/src/framework/data_bus.cc
+++ b/src/framework/data_bus.cc
@@ -69,7 +69,33 @@ DataBus::~DataBus()
 {
     for ( auto& p : map )
         for ( auto* h : p.second )
-            delete h;
+        {
+            // If the object is cloned, pass the ownership to the next config.
+            // When the object is no further cloned (e.g., the last config), delete it.
+            if ( h->cloned )
+                h->cloned = false;
+            else
+                delete h;
+        }
+
+    mapped_module.clear();
+}
+
+void DataBus::add_mapped_module(const char* name)
+{
+    if ( name )
+        mapped_module.emplace(name);
+}
+
+void DataBus::clone(DataBus& from)
+{
+    for ( auto& p : from.map )
+        for ( auto* h : p.second )
+            if ( mapped_module.count(h->module_name) == 0 )
+            {
+                h->cloned = true;
+                _subscribe(p.first.c_str(), h);
+            }
 }
 
 // add handler to list of handlers to be notified upon
@@ -137,6 +163,10 @@ void DataBus::_subscribe(const char* key, DataHandler* h)
 {
     DataList& v = map[key];
     v.emplace_back(h);
+
+    // Track fresh subscriptions to distinguish during cloning
+    if ( !h->cloned )
+        add_mapped_module(h->module_name);
 }
 
 void DataBus::_unsubscribe(const char* key, DataHandler* h)

diff --git a/src/framework/data_bus.h b/src/framework/data_bus.h
index c37a36640520864c1a11522bf9d0738e56f07035..204b9a80b450ee051976f35c6459f31e9545c6ba 100644 (file)
--- a/src/framework/data_bus.h
+++ b/src/framework/data_bus.h
@@ -29,6 +29,7 @@
 
 #include <map>
 #include <string>
+#include <unordered_set>
 #include <vector>
 
 #include "main/snort_types.h"
@@ -72,14 +73,18 @@ public:
     virtual ~DataHandler() = default;
 
     virtual void handle(DataEvent&, Flow*) { }
+    const char* module_name;
+    bool cloned;
 
 protected:
-    DataHandler() = default;
+    DataHandler(std::nullptr_t) = delete;
+    DataHandler(const char* mod_name) : module_name(mod_name), cloned(false) { }
 };
 
 // FIXIT-P evaluate perf; focus is on correctness
 typedef std::vector<DataHandler*> DataList;
 typedef std::map<std::string, DataList> DataMap;
+typedef std::unordered_set<const char*> DataModule;
 
 class SO_PUBLIC DataBus
 {
@@ -87,6 +92,9 @@ public:
     DataBus();
     ~DataBus();
 
+    void clone(DataBus& from);
+    void add_mapped_module(const char*);
+
     static void subscribe(const char* key, DataHandler*);
     static void subscribe_default(const char* key, DataHandler*);
     static void unsubscribe(const char* key, DataHandler*);
@@ -105,6 +113,7 @@ private:
 
 private:
     DataMap map;
+    DataModule mapped_module;
 };
 
 class SO_PUBLIC DaqMetaEvent : public DataEvent

diff --git a/src/loggers/log_hext.cc b/src/loggers/log_hext.cc
index 77ea327c00612aa08674ce0d24ef62225e8a686a..8184709288c6f795b9355be503387d7a0e7feec5 100644 (file)
--- a/src/loggers/log_hext.cc
+++ b/src/loggers/log_hext.cc
@@ -43,7 +43,7 @@ static THREAD_LOCAL unsigned s_pkt_num = 0;
 class DaqMetaEventHandler : public DataHandler
 {
 public:
-    DaqMetaEventHandler() = default;
+    DaqMetaEventHandler() : DataHandler(S_NAME) { }
     void handle(DataEvent&, Flow*) override;
 };
 

diff --git a/src/main/policy.cc b/src/main/policy.cc
index 36103c911daf898da6500ee26d765fb424427a00..cb6399fdea9f7199e6e2406a118d4f3dd6d9a124 100644 (file)
--- a/src/main/policy.cc
+++ b/src/main/policy.cc
@@ -60,7 +60,7 @@ NetworkPolicy::NetworkPolicy(PolicyId id)
 class AltPktHandler : public DataHandler
 {
 public:
-    AltPktHandler() = default;
+    AltPktHandler() : DataHandler("detection") { }
 
     void handle(DataEvent& e, Flow*) override
     { DetectionEngine::detect(const_cast<Packet*>(e.get_packet())); }
@@ -93,6 +93,14 @@ void InspectionPolicy::configure()
     dbus.subscribe(PACKET_EVENT, new AltPktHandler);
 }
 
+void InspectionPolicy::clone_dbus(SnortConfig* from, const char* exclude_module)
+{
+    // Clone subscriptions from another config if they are not excluded
+    // (e.g., reloading module) and not already subscribed
+    dbus.add_mapped_module(exclude_module);
+    dbus.clone(from->policy_map->get_inspection_policy()->dbus);
+}
+
 //-------------------------------------------------------------------------
 // detection policy
 //-------------------------------------------------------------------------

diff --git a/src/main/policy.h b/src/main/policy.h
index cc7d9718ce89599ade31b36e85ac8579685fc9fe..97c9c0aff47b09dc7bc6d2e8a66c03b3a67f1650 100644 (file)
--- a/src/main/policy.h
+++ b/src/main/policy.h
@@ -40,6 +40,7 @@ typedef unsigned char uuid_t[16];
 namespace snort
 {
 struct GHash;
+struct SnortConfig;
 }
 
 struct PortTable;
@@ -111,6 +112,7 @@ public:
     ~InspectionPolicy();
 
     void configure();
+    void clone_dbus(snort::SnortConfig*, const char*);
 
 public:
     PolicyId policy_id;
@@ -249,8 +251,6 @@ private:
 // FIXIT-L SO_PUBLIC required because SnortConfig::inline_mode(), etc. uses the function
 namespace snort
 {
-struct SnortConfig;
-
 SO_PUBLIC NetworkPolicy* get_network_policy();
 SO_PUBLIC InspectionPolicy* get_inspection_policy();
 SO_PUBLIC IpsPolicy* get_ips_policy();

diff --git a/src/main/snort.cc b/src/main/snort.cc
index 7f643aebcdde3441ca9a05f21d3a065b2a4600a5..73c6bd6bc5e88555a227ceb6c7b671b9400c8543 100644 (file)
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -674,7 +674,7 @@ SnortConfig* Snort::get_updated_policy(SnortConfig* other_conf, const char* fnam
     }
 
     other_conf->cloned = true;
-
+    sc->policy_map->get_inspection_policy()->clone_dbus(other_conf, iname);
     InspectorManager::update_policy(sc);
     reloading = false;
     return sc;
@@ -688,6 +688,7 @@ SnortConfig* Snort::get_updated_module(SnortConfig* other_conf, const char* name
 
     if ( name )
     {
+        ModuleManager::reset_errors();
         ModuleManager::reload_module(name, sc);
         if ( ModuleManager::get_errors() || !sc->verify() )
         {
@@ -711,7 +712,7 @@ SnortConfig* Snort::get_updated_module(SnortConfig* other_conf, const char* name
     }
 
     other_conf->cloned = true;
-
+    sc->policy_map->get_inspection_policy()->clone_dbus(other_conf, name);
     InspectorManager::update_policy(sc);
     reloading = false;
     return sc;

diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc
index 1e25eb6e545907cd9830d5c4459e859bf6035290..331660062a62dccb37fa118e30a824ef1a731309 100644 (file)
--- a/src/managers/module_manager.cc
+++ b/src/managers/module_manager.cc
@@ -1079,6 +1079,7 @@ void ModuleManager::reload_module(const char* name, snort::SnortConfig* sc)
     {
         cout << "Module " << name <<" doesn't exist";
         cout << endl;
+        ++s_errors;
     }
 }
 

diff --git a/src/network_inspectors/appid/appid_http_event_handler.h b/src/network_inspectors/appid/appid_http_event_handler.h
index 0560f11eb4a5a27581239260da28d0326b9ed0b5..03b964c4a0fe151c653d5e847ed502a3870faa15 100644 (file)
--- a/src/network_inspectors/appid/appid_http_event_handler.h
+++ b/src/network_inspectors/appid/appid_http_event_handler.h
@@ -26,6 +26,8 @@
 
 #include "pub_sub/http_events.h"
 
+#include "appid_module.h"
+
 namespace snort
 {
 class Flow;
@@ -40,7 +42,7 @@ public:
         RESPONSE_EVENT,
     };
 
-    HttpEventHandler(HttpEventType type)
+    HttpEventHandler(HttpEventType type) : DataHandler(MOD_NAME)
     {
         event_type = type;
     }

diff --git a/src/network_inspectors/appid/detector_plugins/detector_sip.h b/src/network_inspectors/appid/detector_plugins/detector_sip.h
index 0a42bccde8b66e512358f2201dee300ee5197bbd..d2edc06365577701192d16643064de386d87b237 100644 (file)
--- a/src/network_inspectors/appid/detector_plugins/detector_sip.h
+++ b/src/network_inspectors/appid/detector_plugins/detector_sip.h
@@ -27,6 +27,8 @@
 #include "framework/data_bus.h"
 #include "pub_sub/sip_events.h"
 
+#include "appid_module.h"
+
 namespace snort
 {
 class Flow;
@@ -105,7 +107,7 @@ public:
     void handle(snort::DataEvent&, snort::Flow*) override;
 
 private:
-    SipEventHandler() = default;
+    SipEventHandler() : DataHandler(MOD_NAME) { }
     void client_handler(SipEvent&, AppIdSession&, AppidChangeBits&);
     void service_handler(SipEvent&, AppIdSession&, AppidChangeBits&);
 

diff --git a/src/network_inspectors/binder/binder.cc b/src/network_inspectors/binder/binder.cc
index d57e2936cbe18c87763082508a1eac1e503c50b0..4c63ae0accd70a9105788ea80085f756e22411c3 100644 (file)
--- a/src/network_inspectors/binder/binder.cc
+++ b/src/network_inspectors/binder/binder.cc
@@ -599,7 +599,7 @@ private:
 class FlowStateSetupHandler : public DataHandler
 {
 public:
-    FlowStateSetupHandler() = default;
+    FlowStateSetupHandler() : DataHandler(BIND_NAME) { }
 
     void handle(DataEvent& event, Flow* flow) override
     {
@@ -613,7 +613,7 @@ public:
 class FlowServiceChangeHandler : public DataHandler
 {
 public:
-    FlowServiceChangeHandler() = default;
+    FlowServiceChangeHandler() : DataHandler(BIND_NAME) { }
 
     void handle(DataEvent&, Flow* flow) override
     {
@@ -626,7 +626,7 @@ public:
 class StreamHANewFlowHandler : public DataHandler
 {
 public:
-    StreamHANewFlowHandler() = default;
+    StreamHANewFlowHandler() : DataHandler(BIND_NAME) { }
 
     void handle(DataEvent&, Flow* flow) override
     {

diff --git a/src/network_inspectors/perf_monitor/perf_monitor.cc b/src/network_inspectors/perf_monitor/perf_monitor.cc
index 5fa927419536a707351930372c5e06aa509fcc74..3c09d13215a0dfac764bbdd0cdaa29c03a565790 100644 (file)
--- a/src/network_inspectors/perf_monitor/perf_monitor.cc
+++ b/src/network_inspectors/perf_monitor/perf_monitor.cc
@@ -86,7 +86,7 @@ private:
 class PerfIdleHandler : public DataHandler
 {
 public:
-    PerfIdleHandler(PerfMonitor& p) : perf_monitor(p)
+    PerfIdleHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
     { DataBus::subscribe_default(THREAD_IDLE_EVENT, this); }
 
     void handle(DataEvent&, Flow*) override
@@ -99,7 +99,7 @@ private:
 class PerfRotateHandler : public DataHandler
 {
 public:
-    PerfRotateHandler(PerfMonitor& p) : perf_monitor(p)
+    PerfRotateHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
     { DataBus::subscribe_default(THREAD_ROTATE_EVENT, this); }
 
     void handle(DataEvent&, Flow*) override
@@ -112,7 +112,7 @@ private:
 class FlowIPDataHandler : public DataHandler
 {
 public:
-    FlowIPDataHandler(PerfMonitor& p) : perf_monitor(p)
+    FlowIPDataHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
     { DataBus::subscribe_default(FLOW_STATE_EVENT, this); }
 
     void handle(DataEvent&, Flow* flow) override