Merge pull request #1396 in SNORT/snort3 from appid_profile_race to master

Squashed commit of the following:

commit eacad4b72cda229ee7807f171850f18de3d49c83
Author: Masud Hasan <mashasan@cisco.com>
Date:   Thu Oct 18 09:46:22 2018 -0400

    appid: Fixing profiler data race and registration issues

diff --git a/src/network_inspectors/appid/appid_config.cc b/src/network_inspectors/appid/appid_config.cc
index aa2318ae662a2de73277bb51a177af7a6c85e431..06a92dbd2f19dd6cc5c5f00e96dd64107d53aa76 100644 (file)
--- a/src/network_inspectors/appid/appid_config.cc
+++ b/src/network_inspectors/appid/appid_config.cc
@@ -31,6 +31,7 @@
 #include "app_forecast.h"
 #include "app_info_table.h"
 #include "appid_discovery.h"
+#include "appid_http_session.h"
 #include "appid_session.h"
 #ifdef USE_RNA_CONFIG
 #include "appid_utils/network_set.h"
@@ -46,6 +47,7 @@
 #include "detector_plugins/detector_dns.h"
 #include "target_based/snort_protocols.h"
 #ifdef ENABLE_APPID_THIRD_PARTY
+#include "tp_appid_utils.h"
 #include "tp_lib_handler.h"
 #endif
 
@@ -765,10 +767,14 @@ bool AppIdConfig::init_appid(SnortConfig* sc, AppIdInspector *ins)
         PatternClientDetector::finalize_client_port_patterns();
         AppIdDiscovery::finalize_plugins();
         http_matchers->finalize_patterns();
-           ssl_detector_process_patterns();
+        ssl_detector_process_patterns();
         dns_host_detector_process_patterns();
         read_port_detectors(ODP_PORT_DETECTORS);
         read_port_detectors(CUSTOM_PORT_DETECTORS);
+        appid_http_profiler_init();
+#ifdef ENABLE_APPID_THIRD_PARTY
+        tp_appid_profiler_init();
+#endif
         once = true;
     }
 #ifdef USE_RNA_CONFIG

diff --git a/src/network_inspectors/appid/appid_http_session.cc b/src/network_inspectors/appid/appid_http_session.cc
index 553ac32030cc7a37f067b28c17b5f936481ebb87..b0cb62fe249ae17cc55fb4cac0d76336ebfd36ed 100644 (file)
--- a/src/network_inspectors/appid/appid_http_session.cc
+++ b/src/network_inspectors/appid/appid_http_session.cc
@@ -52,7 +52,19 @@ static const char* httpFieldName[ NUM_HTTP_FIELDS ] = // for use in debug messag
     "body",
 };
 
-snort::ProfileStats httpPerfStats;
+#ifdef APPID_DEEP_PERF_PROFILING
+static THREAD_LOCAL snort::ProfileStats process_http_perf_stats;
+static ProfileStats* get_profile(const char*)
+{
+    return &process_http_perf_stats;
+}
+void appid_http_profiler_init()
+{
+    Profiler::register_module("http_process", "appid", get_profile);
+}
+#else
+void appid_http_profiler_init() { return; }
+#endif
 
 AppIdHttpSession::AppIdHttpSession(AppIdSession& asd)
     : asd(asd)
@@ -377,7 +389,9 @@ void AppIdHttpSession::process_chp_buffers(AppidChangeBits& change_bits)
 int AppIdHttpSession::process_http_packet(AppidSessionDirection direction,
     AppidChangeBits& change_bits)
 {
-    snort::Profile http_profile_context(httpPerfStats);
+#ifdef APPID_DEEP_PERF_PROFILING
+    snort::Profile profile(process_http_perf_stats);
+#endif
     AppId service_id = APP_ID_NONE;
     AppId client_id = APP_ID_NONE;
     AppId payload_id = APP_ID_NONE;

diff --git a/src/network_inspectors/appid/appid_http_session.h b/src/network_inspectors/appid/appid_http_session.h
index 2b592c83dfcd213ea846e732aa64681f574368cd..1f2caba46c039802ceb4c59578c60239f3f38fb3 100644 (file)
--- a/src/network_inspectors/appid/appid_http_session.h
+++ b/src/network_inspectors/appid/appid_http_session.h
@@ -232,5 +232,7 @@ protected:
 #endif
 };
 
+void appid_http_profiler_init();
+
 #endif
 

diff --git a/src/network_inspectors/appid/appid_inspector.cc b/src/network_inspectors/appid/appid_inspector.cc
index 28c067bfcd43b468fad9fab3ac5cdbcfb255ef2d..3170b34c46b9c1c6ec234d2e802cad55a3881c83 100644 (file)
--- a/src/network_inspectors/appid/appid_inspector.cc
+++ b/src/network_inspectors/appid/appid_inspector.cc
@@ -181,7 +181,7 @@ void AppIdInspector::tterm()
 
 void AppIdInspector::eval(Packet* p)
 {
-    Profile profile(appidPerfStats);
+    Profile profile(appid_perf_stats);
     appid_stats.packets++;
     
     if (p->flow)

diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc
index 4f6d079b9d6d4bb617b7b16c7bf8e265984f214b..d3a4526570ba120c35c10dd0cb59234b84ff07d1 100644 (file)
--- a/src/network_inspectors/appid/appid_module.cc
+++ b/src/network_inspectors/appid/appid_module.cc
@@ -46,7 +46,7 @@ Trace TRACE_NAME(appid_module);
 // appid module
 //-------------------------------------------------------------------------
 
-THREAD_LOCAL ProfileStats appidPerfStats;
+THREAD_LOCAL ProfileStats appid_perf_stats;
 THREAD_LOCAL AppIdStats appid_stats;
 
 static const Parameter s_params[] =
@@ -207,7 +207,7 @@ AppIdModule::~AppIdModule()
 
 ProfileStats* AppIdModule::get_profile() const
 {
-    return &appidPerfStats;
+    return &appid_perf_stats;
 }
 
 const AppIdModuleConfig* AppIdModule::get_data()

diff --git a/src/network_inspectors/appid/appid_module.h b/src/network_inspectors/appid/appid_module.h
index c47b0344927b34dab684b463ec45f969e6e66033..9595fd11c37b6c3f0a148bcb6442def41e0eb6b2 100644 (file)
--- a/src/network_inspectors/appid/appid_module.h
+++ b/src/network_inspectors/appid/appid_module.h
@@ -28,7 +28,7 @@
 #include "appid_config.h"
 #include "framework/module.h"
 
-extern THREAD_LOCAL snort::ProfileStats appidPerfStats;
+extern THREAD_LOCAL snort::ProfileStats appid_perf_stats;
 
 extern Trace TRACE_NAME(appid_module);
 

diff --git a/src/network_inspectors/appid/client_plugins/client_discovery.cc b/src/network_inspectors/appid/client_plugins/client_discovery.cc
index fcd48468b445c1ec791fe0a3ae4d339e5506a1a0..b63c31a2a3830fe4b01a4fea01b9df934da9eeb5 100644 (file)
--- a/src/network_inspectors/appid/client_plugins/client_discovery.cc
+++ b/src/network_inspectors/appid/client_plugins/client_discovery.cc
@@ -52,8 +52,15 @@ using namespace snort;
 
 #define MAX_CANDIDATE_CLIENTS 10
 
+#ifdef APPID_DEEP_PERF_PROFILING
+static THREAD_LOCAL ProfileStats client_disco_perf_stats;
+static ProfileStats* get_profile(const char*)
+{
+    return &client_disco_perf_stats;
+}
+#endif
+
 ClientDiscovery* ClientDiscovery::discovery_manager = nullptr;
-ProfileStats clientMatchPerfStats;
 THREAD_LOCAL ClientAppMatch* match_free_list = nullptr;
 
 ClientDiscovery::ClientDiscovery(AppIdInspector& ins)
@@ -120,6 +127,10 @@ void ClientDiscovery::initialize()
 
     for ( auto kv : udp_detectors )
         kv.second->initialize();
+
+#ifdef APPID_DEEP_PERF_PROFILING
+    Profiler::register_module("client_discovery", "appid", get_profile);
+#endif
 }
 
 void ClientDiscovery::finalize_client_plugins()
@@ -340,10 +351,11 @@ int ClientDiscovery::exec_client_detectors(AppIdSession& asd, Packet* p,
 bool ClientDiscovery::do_client_discovery(AppIdSession& asd, Packet* p,
     AppidSessionDirection direction, AppidChangeBits& change_bits)
 {
+#ifdef APPID_DEEP_PERF_PROFILING
+    Profile profile(client_disco_perf_stats);
+#endif
     bool isTpAppidDiscoveryDone = false;
     AppInfoTableEntry* entry;
-
-    Profile clientMatchPerfStats_profile_context(clientMatchPerfStats);
     uint32_t prevRnaClientState = asd.client_disco_state;
     bool was_http2 = asd.is_http2;
     bool was_service = asd.is_service_detected();

diff --git a/src/network_inspectors/appid/ips_appid_option.cc b/src/network_inspectors/appid/ips_appid_option.cc
index 3d5594fc6ad9077f84f9abcf199bae693ffea2aa..6687d1d04a43c29b08406c7d03ce8da1bda0b148 100644 (file)
--- a/src/network_inspectors/appid/ips_appid_option.cc
+++ b/src/network_inspectors/appid/ips_appid_option.cc
@@ -56,7 +56,7 @@ using namespace snort;
 #define SP_SERVICE 2
 #define NUM_ID_TYPES 4
 
-static THREAD_LOCAL ProfileStats appidRuleOptionPerfStats;
+static THREAD_LOCAL ProfileStats ips_appid_perf_stats;
 
 class AppIdIpsOption : public IpsOption
 {
@@ -119,17 +119,19 @@ bool AppIdIpsOption::match_id_against_rule(int32_t id)
 // first match wins...
 IpsOption::EvalStatus AppIdIpsOption::eval(Cursor&, Packet* p)
 {
-    AppId app_ids[NUM_ID_TYPES];
-
     if ( !p->flow )
         return NO_MATCH;
 
-    Profile profile(appidRuleOptionPerfStats);
+#ifdef APPID_DEEP_PERF_PROFILING
+    Profile profile(ips_appid_perf_stats);
+#endif
 
     AppIdSession* session = appid_api.get_appid_session(*(p->flow));
     if ( !session )
         return NO_MATCH;
 
+    AppId app_ids[NUM_ID_TYPES];
+
     // id order on stream api call is: service, client, payload, misc
     if ( (p->packet_flags & PKT_FROM_CLIENT) )
         session->get_application_ids(app_ids[CP_SERVICE], app_ids[CP_CLIENT],
@@ -165,7 +167,7 @@ public:
     bool end(const char*, int, SnortConfig*) override;
 
     ProfileStats* get_profile() const override
-    { return &appidRuleOptionPerfStats; }
+    { return &ips_appid_perf_stats; }
 
     Usage get_usage() const override
     { return DETECT; }

diff --git a/src/network_inspectors/appid/lua_detector_api.cc b/src/network_inspectors/appid/lua_detector_api.cc
index 488f404072d7eaf7dab2129183bd146ec9fec344..40817036c70a77f83d953e247ae7d9ea60e2dc83 100644 (file)
--- a/src/network_inspectors/appid/lua_detector_api.cc
+++ b/src/network_inspectors/appid/lua_detector_api.cc
@@ -64,9 +64,20 @@ enum LuaLogLevels
     LUA_LOG_TRACE = 5,
 };
 
-ProfileStats luaDetectorsPerfStats;
-ProfileStats luaCiscoPerfStats;
-ProfileStats luaCustomPerfStats;
+#ifdef APPID_DEEP_PERF_PROFILING
+// FIXIT-L: Bring snort2's luaCiscoPerfStats and luaCustomPerfStats if more granularity is desired
+static THREAD_LOCAL ProfileStats lua_validate_perf_stats;
+static ProfileStats* get_profile(const char*)
+{
+    return &lua_validate_perf_stats;
+}
+void lua_detector_profiler_init()
+{
+    Profiler::register_module("lua_validate", "appid", get_profile);
+}
+#else
+void lua_detector_profiler_init() { return; }
+#endif
 
 static std::unordered_map<AppId, CHPApp*>* CHP_glossary = nullptr; // tracks http multipatterns
 
@@ -2486,8 +2497,6 @@ int register_detector(lua_State* L)
 
 int LuaStateDescriptor::lua_validate(AppIdDiscoveryArgs& args)
 {
-    Profile lua_detector_context(luaCustomPerfStats);
-
     auto my_lua_state = lua_detector_mgr? lua_detector_mgr->L : nullptr;
     if (!my_lua_state)
     {
@@ -2628,6 +2637,9 @@ LuaServiceObject::LuaServiceObject(AppIdDiscovery* sdm, const std::string& detec
 
 int LuaServiceDetector::validate(AppIdDiscoveryArgs& args)
 {
+#ifdef APPID_DEEP_PERF_PROFILING
+    Profile profile(lua_validate_perf_stats);
+#endif
     //FIXIT-M: RELOAD - use lua references to get user data object from stack
     auto my_lua_state = lua_detector_mgr? lua_detector_mgr->L : nullptr;
     lua_settop(my_lua_state,0);
@@ -2705,6 +2717,9 @@ LuaStateDescriptor* LuaObject::validate_lua_state(bool packet_context)
 
 int LuaClientDetector::validate(AppIdDiscoveryArgs& args)
 {
+#ifdef APPID_DEEP_PERF_PROFILING
+    Profile profile(lua_validate_perf_stats);
+#endif
     //FIXIT-M: RELOAD - use lua references to get user data object from stack
     auto my_lua_state = lua_detector_mgr? lua_detector_mgr->L : nullptr;
     std::string name = this->name + "_";

diff --git a/src/network_inspectors/appid/lua_detector_api.h b/src/network_inspectors/appid/lua_detector_api.h
index 3f126b5dff78adcc05710d9d513187c0e8f634dd..add9814044b1e56f32185911651fdc4b2bf93d60 100644 (file)
--- a/src/network_inspectors/appid/lua_detector_api.h
+++ b/src/network_inspectors/appid/lua_detector_api.h
@@ -138,6 +138,7 @@ public:
     { return cd; }
 };
 
+void lua_detector_profiler_init();
 int register_detector(lua_State*);
 void init_chp_glossary();
 int init(lua_State*, int result=0);

diff --git a/src/network_inspectors/appid/lua_detector_module.cc b/src/network_inspectors/appid/lua_detector_module.cc
index 5cee62ffef9a241cffc20c1c8f9b2b3938d6b79e..b6a9ef6cac17cd67340fb8990aa3fbef51ec4e44 100644 (file)
--- a/src/network_inspectors/appid/lua_detector_module.cc
+++ b/src/network_inspectors/appid/lua_detector_module.cc
@@ -196,6 +196,9 @@ LuaDetectorManager::~LuaDetectorManager()
 
 void LuaDetectorManager::initialize(AppIdConfig& config, int is_control)
 {
+    if (is_control)
+        lua_detector_profiler_init();
+
     // FIXIT-M: RELOAD - When reload is supported, remove this line which prevents re-initialize
     if (lua_detector_mgr)
         return;

diff --git a/src/network_inspectors/appid/service_plugins/service_discovery.cc b/src/network_inspectors/appid/service_plugins/service_discovery.cc
index 51f812568cca4609d64f043fe994a16a69e7b82e..ae86d265ffdfdd0dfdbd3df181ce8cc2979121c9 100644 (file)
--- a/src/network_inspectors/appid/service_plugins/service_discovery.cc
+++ b/src/network_inspectors/appid/service_plugins/service_discovery.cc
@@ -86,7 +86,14 @@
 
 using namespace snort;
 
-ProfileStats serviceMatchPerfStats;
+#ifdef APPID_DEEP_PERF_PROFILING
+static THREAD_LOCAL ProfileStats service_disco_perf_stats;
+static ProfileStats* get_profile(const char*)
+{
+    return &service_disco_perf_stats;
+}
+#endif
+
 static ServiceDetector* ftp_service;
 ServiceDiscovery* ServiceDiscovery::discovery_manager = nullptr;
 
@@ -174,6 +181,10 @@ void ServiceDiscovery::initialize()
         kv.second->initialize();
         service_detector_list.push_back(kv.second);    
     }
+
+#ifdef APPID_DEEP_PERF_PROFILING
+    Profiler::register_module("service_discovery", "appid", get_profile);
+#endif
 }
 
 void ServiceDiscovery::finalize_service_patterns()
@@ -584,9 +595,10 @@ int ServiceDiscovery::add_ftp_service_state(AppIdSession& asd)
 bool ServiceDiscovery::do_service_discovery(AppIdSession& asd, Packet* p,
     AppidSessionDirection direction, AppidChangeBits& change_bits)
 {
+#ifdef APPID_DEEP_PERF_PROFILING
+    Profile profile(service_disco_perf_stats);
+#endif
     bool isTpAppidDiscoveryDone = false;
-
-    Profile serviceMatchPerfStats_profile_context(serviceMatchPerfStats);
     uint32_t prevRnaServiceState = asd.service_disco_state;
     AppId tp_app_id = asd.get_tp_app_id();
 

diff --git a/src/network_inspectors/appid/tp_appid_utils.cc b/src/network_inspectors/appid/tp_appid_utils.cc
index 8177ae0e6b97efc9674b682e6a837a8dce1dc7f4..24dbda0e07b26e9b1c56959cc267f4c5df689886 100644 (file)
--- a/src/network_inspectors/appid/tp_appid_utils.cc
+++ b/src/network_inspectors/appid/tp_appid_utils.cc
@@ -53,8 +53,32 @@ using namespace snort;
 
 typedef AppIdHttpSession::pair_t pair_t;
 
-THREAD_LOCAL ProfileStats tpLibPerfStats;
-THREAD_LOCAL ProfileStats tpPerfStats;
+static THREAD_LOCAL ProfileStats tp_lib_perf_stats;
+#ifdef APPID_DEEP_PERF_PROFILING
+static THREAD_LOCAL ProfileStats tp_disco_perf_stats;
+static ProfileStats* get_profile(const char* key)
+{
+    if ( !strcmp(key, "tp_discovery") )
+        return &tp_disco_perf_stats;
+    if ( !strcmp(key, "tp_library") )
+        return &tp_lib_perf_stats;
+    return nullptr;
+}
+void tp_appid_profiler_init()
+{
+    Profiler::register_module("tp_discovery", "appid", get_profile);
+    Profiler::register_module("tp_library", "tp_discovery", get_profile);
+}
+#else
+static ProfileStats* get_profile(const char*)
+{
+    return &tp_lib_perf_stats;
+}
+void tp_appid_profiler_init()
+{
+    Profiler::register_module("tp_library", "appid", get_profile);
+}
+#endif
 
 // std::vector does not have a convenient find() function.
 // There is a generic std::find() in <algorithm>, but this might be faster.
@@ -625,8 +649,9 @@ static inline void check_terminate_tp_module(AppIdSession& asd, uint16_t tpPktCo
 bool do_tp_discovery(AppIdSession& asd, IpProtocol protocol,
     Packet* p, AppidSessionDirection& direction, AppidChangeBits& change_bits)
 {
-    if ( !TPLibHandler::have_tp() )
-        return true;
+#ifdef APPID_DEEP_PERF_PROFILING
+    Profile tp_disco_profile(tp_disco_perf_stats);
+#endif
 
     AppId tp_app_id = asd.get_tp_app_id();
 
@@ -647,8 +672,6 @@ bool do_tp_discovery(AppIdSession& asd, IpProtocol protocol,
 
     if (p->dsize || asd.config->mod_config->tp_allow_probes)
     {
-        Profile tpPerfStats_profile_context(tpPerfStats);
-
         //restart inspection by 3rd party
         if (!asd.tp_reinspect_by_initiator && (direction == APP_ID_FROM_INITIATOR) &&
             check_reinspect(p, asd))
@@ -666,7 +689,7 @@ bool do_tp_discovery(AppIdSession& asd, IpProtocol protocol,
             if (protocol != IpProtocol::TCP || (p->packet_flags & PKT_STREAM_ORDER_OK)
                 || asd.config->mod_config->tp_allow_probes)
             {
-                Profile tpLibPerfStats_profile_context(tpLibPerfStats);
+                Profile tp_lib_profile(tp_lib_perf_stats);
                 int tp_confidence;
                 ThirdPartyAppIDAttributeData tp_attribute_data;
                 vector<AppId> tp_proto_list;

diff --git a/src/network_inspectors/appid/tp_appid_utils.h b/src/network_inspectors/appid/tp_appid_utils.h
index 089ea1d743dccaeb6fc4384f9912aff644d7fd5e..72f970da6e7c78be45fb749f887cf38baed247a5 100644 (file)
--- a/src/network_inspectors/appid/tp_appid_utils.h
+++ b/src/network_inspectors/appid/tp_appid_utils.h
@@ -26,6 +26,7 @@
 
 class AppIdSession;
 
+void tp_appid_profiler_init();
 bool do_tp_discovery(AppIdSession&, IpProtocol, snort::Packet*, AppidSessionDirection&,
     AppidChangeBits&);