From: Mike Stepanek (mstepane) Date: Tue, 23 Oct 2018 13:17:23 +0000 (-0400) Subject: Merge pull request #1396 in SNORT/snort3 from appid_profile_race to master X-Git-Tag: 3.0.0-249~23 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6c137644366afc2a8b02e8b83d286b892ee53b5e;p=thirdparty%2Fsnort3.git Merge pull request #1396 in SNORT/snort3 from appid_profile_race to master Squashed commit of the following: commit eacad4b72cda229ee7807f171850f18de3d49c83 Author: Masud Hasan 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 aa2318ae6..06a92dbd2 100644 --- 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 553ac3203..b0cb62fe2 100644 --- 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 2b592c83d..1f2caba46 100644 --- 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 28c067bfc..3170b34c4 100644 --- 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 4f6d079b9..d3a452657 100644 --- 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 c47b03449..9595fd11c 100644 --- 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 fcd48468b..b63c31a2a 100644 --- 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 3d5594fc6..6687d1d04 100644 --- 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 488f40407..40817036c 100644 --- 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* 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 3f126b5df..add981404 100644 --- 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 5cee62ffe..b6a9ef6ca 100644 --- 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 51f812568..ae86d265f 100644 --- 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 8177ae0e6..24dbda0e0 100644 --- 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 , 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 tp_proto_list; diff --git a/src/network_inspectors/appid/tp_appid_utils.h b/src/network_inspectors/appid/tp_appid_utils.h index 089ea1d74..72f970da6 100644 --- 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&);