From: Mike Stepanek (mstepane) Date: Mon, 2 Mar 2020 17:59:53 +0000 (+0000) Subject: Merge pull request #2044 in SNORT/snort3 from ~MASHASAN/snort3:stats_and_data_races... X-Git-Tag: 3.0.0-269~24 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=27683414c47632d0a957b571d701b716388f55a9;p=thirdparty%2Fsnort3.git Merge pull request #2044 in SNORT/snort3 from ~MASHASAN/snort3:stats_and_data_races to master Squashed commit of the following: commit 9d4b9171cdde544f26b63f2390e6dafc3fb7f1fb Author: Masud Hasan Date: Thu Feb 27 18:27:03 2020 -0500 stream: Addressing inconsistent stream stats and some data races --- diff --git a/src/main/request.cc b/src/main/request.cc index 9eb9be71a..9a554f51d 100644 --- a/src/main/request.cc +++ b/src/main/request.cc @@ -100,10 +100,9 @@ void Request::respond(const char* s, bool queue_response, bool remote_only) #ifdef SHELL void Request::send_queued_response() { - const char* qr = queued_response; - if ( qr ) + if ( queued_response ) { - write_response(qr); + write_response(queued_response); queued_response = nullptr; } } diff --git a/src/main/request.h b/src/main/request.h index 657db6d1c..59c44ca17 100644 --- a/src/main/request.h +++ b/src/main/request.h @@ -22,8 +22,6 @@ #ifndef REQUEST_H #define REQUEST_H -#include - #include "main/snort_types.h" class Request @@ -43,6 +41,6 @@ private: int fd; char read_buf[1024]; size_t bytes_read; - std::atomic queued_response; + const char* queued_response; }; #endif diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc index 1b66efe7a..33e91d8ed 100644 --- a/src/managers/module_manager.cc +++ b/src/managers/module_manager.cc @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -75,7 +74,8 @@ static std::unordered_map s_pmap; static unsigned s_errors = 0; -std::set ModuleManager::gids; +set ModuleManager::gids; +mutex ModuleManager::stats_mutex; static string s_current; static string s_name; @@ -84,8 +84,6 @@ static string s_type; // for callbacks from Lua static SnortConfig* s_config = nullptr; -static std::mutex stats_mutex; - // forward decls extern "C" { @@ -1382,7 +1380,7 @@ void ModuleManager::dump_stats(SnortConfig*, const char* skip, bool dynamic) { if ( !skip || !strstr(skip, mh->mod->get_name()) ) { - std::lock_guard lock(stats_mutex); + lock_guard lock(stats_mutex); if ( dynamic ) mh->mod->show_dynamic_stats(); else @@ -1397,7 +1395,7 @@ void ModuleManager::accumulate(SnortConfig*) for ( auto* mh : mod_hooks ) { - std::lock_guard lock(stats_mutex); + lock_guard lock(stats_mutex); mh->mod->prep_counts(); mh->mod->sum_stats(true); } @@ -1408,7 +1406,7 @@ void ModuleManager::accumulate_offload(const char* name) ModHook* mh = get_hook(name); if ( mh ) { - std::lock_guard lock(stats_mutex); + lock_guard lock(stats_mutex); mh->mod->prep_counts(); mh->mod->sum_stats(true); } @@ -1420,7 +1418,7 @@ void ModuleManager::reset_stats(SnortConfig*) for ( auto* mh : mod_hooks ) { - std::lock_guard lock(stats_mutex); + lock_guard lock(stats_mutex); mh->mod->reset_stats(); } } diff --git a/src/managers/module_manager.h b/src/managers/module_manager.h index 46facfecd..eef450c2d 100644 --- a/src/managers/module_manager.h +++ b/src/managers/module_manager.h @@ -24,8 +24,9 @@ // Modules are strictly used during parse time. #include -#include #include +#include +#include #include "main/snort_types.h" @@ -90,6 +91,7 @@ public: static void reset_stats(SnortConfig*); static std::set gids; + SO_PUBLIC static std::mutex stats_mutex; }; } diff --git a/src/network_inspectors/perf_monitor/base_tracker.cc b/src/network_inspectors/perf_monitor/base_tracker.cc index 70fded374..a5aa2f201 100644 --- a/src/network_inspectors/perf_monitor/base_tracker.cc +++ b/src/network_inspectors/perf_monitor/base_tracker.cc @@ -24,6 +24,8 @@ #include "base_tracker.h" // FIXIT-W Returning null reference (from ) +#include "managers/module_manager.h" + #ifdef UNIT_TEST #include "catch/snort_catch.h" #include "utils/util.h" @@ -52,9 +54,14 @@ void BaseTracker::process(bool summary) write(); - for ( const ModuleConfig& mod : modules ) - if ( !summary ) + if ( !summary ) + { + for ( const ModuleConfig& mod : modules ) + { + lock_guard lock(ModuleManager::stats_mutex); mod.ptr->sum_stats(false); + } + } } #ifdef UNIT_TEST diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc index ddaa33885..5e2a9e971 100644 --- a/src/stream/base/stream_base.cc +++ b/src/stream/base/stream_base.cc @@ -61,6 +61,7 @@ const PegInfo base_pegs[] = { CountType::SUM, "preemptive_prunes", "sessions pruned during preemptive pruning" }, { CountType::SUM, "memcap_prunes", "sessions pruned due to memcap" }, { CountType::SUM, "ha_prunes", "sessions pruned by high availability sync" }, + { CountType::SUM, "stale_prunes", "sessions pruned due to stale connection" }, { CountType::SUM, "expected_flows", "total expected flows created within snort" }, { CountType::SUM, "expected_realized", "number of expected flows realized" }, { CountType::SUM, "expected_pruned", "number of expected flows pruned" }, @@ -90,6 +91,7 @@ void base_sum() stream_base_stats.preemptive_prunes = flow_con->get_prunes(PruneReason::PREEMPTIVE); stream_base_stats.memcap_prunes = flow_con->get_prunes(PruneReason::MEMCAP); stream_base_stats.ha_prunes = flow_con->get_prunes(PruneReason::HA); + stream_base_stats.stale_prunes = flow_con->get_prunes(PruneReason::STALE); stream_base_stats.reload_freelist_flow_deletes = flow_con->get_deletes(FlowDeleteState::FREELIST); stream_base_stats.reload_allowed_flow_deletes = flow_con->get_deletes(FlowDeleteState::ALLOWED); stream_base_stats.reload_offloaded_flow_deletes= flow_con->get_deletes(FlowDeleteState::OFFLOADED); diff --git a/src/stream/base/stream_module.h b/src/stream/base/stream_module.h index 3384ab30a..8bded5983 100644 --- a/src/stream/base/stream_module.h +++ b/src/stream/base/stream_module.h @@ -53,6 +53,7 @@ struct BaseStats PegCount preemptive_prunes; PegCount memcap_prunes; PegCount ha_prunes; + PegCount stale_prunes; PegCount expected_flows; PegCount expected_realized; PegCount expected_pruned; diff --git a/src/utils/test/memcap_allocator_test.cc b/src/utils/test/memcap_allocator_test.cc index beea28088..3de048e30 100644 --- a/src/utils/test/memcap_allocator_test.cc +++ b/src/utils/test/memcap_allocator_test.cc @@ -153,10 +153,13 @@ TEST(memcap_allocator, max_allocations_test) CHECK(mem_blocks[i]); CHECK(mca->get_mem_allocated() == bytes_allocated); if ( i < MAX_ALLOCATIONS_10000 - 1 ) + { CHECK(mca->is_space_available()); + } else + { CHECK(!mca->is_space_available()); - + } CHECK(!mca->is_over_capacity()); } CHECK(mca->get_allocation_requests() == MAX_ALLOCATIONS_10000);