From: ARUNKUMAR KAYAMBU -X (akayambu - XORIANT CORPORATION at Cisco) Date: Thu, 6 Jul 2023 18:27:43 +0000 (+0000) Subject: Pull request #3859: perf_mon: fix dump_stats collision with perf mon X-Git-Tag: 3.1.66.0~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ee7f6600936a82e8e7cdf481d61e39e6ec67e311;p=thirdparty%2Fsnort3.git Pull request #3859: perf_mon: fix dump_stats collision with perf mon Merge in SNORT/snort3 from ~AKAYAMBU/snort3:dump_stats_fix to master Squashed commit of the following: commit 78bdb137f619179005aebbadf9548e1121f90fce Author: Arunkumar Kayambu Date: Tue May 23 10:56:21 2023 -0400 perf_mon: fix dump_stats collision with perf mon --- diff --git a/src/file_api/file_module.cc b/src/file_api/file_module.cc index f9e4612a4..2d8f201ef 100644 --- a/src/file_api/file_module.cc +++ b/src/file_api/file_module.cc @@ -149,10 +149,10 @@ const RuleMap* FileIdModule::get_rules() const return file_id_rules; } -void FileIdModule::sum_stats(bool accumulate_now_stats) +void FileIdModule::sum_stats(bool dump_stats) { file_stats_sum(); - Module::sum_stats(accumulate_now_stats); + Module::sum_stats(dump_stats); } bool FileIdModule::set(const char*, Value& v, SnortConfig*) diff --git a/src/framework/module.cc b/src/framework/module.cc index 0a9afa24d..d2dac5e47 100644 --- a/src/framework/module.cc +++ b/src/framework/module.cc @@ -85,7 +85,7 @@ void Module::clear_global_active_counters() } } -void Module::sum_stats(bool accumulate_now_stats) +void Module::sum_stats(bool dump_stats) { if ( num_counts < 0 ) reset_stats(); @@ -97,11 +97,17 @@ void Module::sum_stats(bool accumulate_now_stats) return; assert(q); + if(dump_stats && !dump_stats_initialized) + { + for (unsigned long i=0; i 0 ) - ::show_stats(&counts[0], get_pegs(), num_counts, get_name()); + { + ::show_stats(&dump_stats_counts[0], get_pegs(), num_counts, get_name()); + dump_stats_initialized = false; + } } void Module::reset_stats() @@ -162,11 +172,13 @@ void Module::reset_stats() ++num_counts; counts.resize(num_counts); + dump_stats_counts.resize(num_counts); } for ( int i = 0; i < num_counts; i++ ) { counts[i] = 0; + dump_stats_counts[i] = 0; if ( pegs[i].type != CountType::NOW ) p[i] = 0; diff --git a/src/framework/module.h b/src/framework/module.h index 87d8200e5..c48e16811 100644 --- a/src/framework/module.h +++ b/src/framework/module.h @@ -144,7 +144,7 @@ public: virtual bool counts_need_prep() const { return false; } - virtual void prep_counts() { } + virtual void prep_counts(bool) { } // counts and profile are thread local virtual PegCount* get_counts() const @@ -169,7 +169,7 @@ public: virtual bool global_stats() const { return false; } - virtual void sum_stats(bool accumulate_now_stats); + virtual void sum_stats(bool dump_stats); virtual void show_interval_stats(IndexVec&, FILE*); virtual void show_stats(); virtual void reset_stats(); @@ -202,11 +202,14 @@ protected: void set_params(const Parameter* p) { params = p; } + bool dump_stats_initialized = false; + private: friend ModuleManager; void init(const char*, const char* = nullptr); std::vector counts; + std::vector dump_stats_counts; int num_counts = -1; const char* name; @@ -216,23 +219,38 @@ private: bool list; int table_level = 0; - void set_peg_count(int index, PegCount value) + void set_peg_count(int index, PegCount value, bool dump_stats = false) { assert(index < num_counts); - counts[index] = value; + if(dump_stats) + dump_stats_counts[index] = value; + else + counts[index] = value; } - void set_max_peg_count(int index, PegCount value) + void set_max_peg_count(int index, PegCount value, bool dump_stats = false) { assert(index < num_counts); - if(value > counts[index]) - counts[index] = value; + if(dump_stats) + { + if(value > dump_stats_counts[index]) + dump_stats_counts[index] = value; + } + else + { + if(value > counts[index]) + counts[index] = value; + } } - void add_peg_count(int index, PegCount value) + void add_peg_count(int index, PegCount value, bool dump_stats = false) { assert(index < num_counts); - counts[index] += value; + if(dump_stats) + dump_stats_counts[index] += value; + else + counts[index] += value; + } }; } diff --git a/src/host_tracker/host_cache_module.cc b/src/host_tracker/host_cache_module.cc index 2ea6c0c49..ff9c5db61 100644 --- a/src/host_tracker/host_cache_module.cc +++ b/src/host_tracker/host_cache_module.cc @@ -476,7 +476,7 @@ const PegInfo* HostCacheModule::get_pegs() const PegCount* HostCacheModule::get_counts() const { return (PegCount*)host_cache.get_counts(); } -void HostCacheModule::sum_stats(bool accumulate_now_stats) +void HostCacheModule::sum_stats(bool dump_stats) { host_cache.lock(); // These could be set in prep_counts but we set them here @@ -484,7 +484,7 @@ void HostCacheModule::sum_stats(bool accumulate_now_stats) host_cache.stats.bytes_in_use = host_cache.current_size; host_cache.stats.items_in_use = host_cache.list.size(); - Module::sum_stats(accumulate_now_stats); + Module::sum_stats(dump_stats); host_cache.unlock(); } diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc index eb73ac079..0f0baff71 100644 --- a/src/managers/module_manager.cc +++ b/src/managers/module_manager.cc @@ -1431,7 +1431,7 @@ PegCount* ModuleManager::get_stats(const char* name) ModHook* mh = get_hook(name); if ( mh ) - pc = &mh->mod->counts[0]; + pc = &mh->mod->dump_stats_counts[0]; return pc; } @@ -1464,7 +1464,7 @@ void ModuleManager::accumulate(const char* except) continue; lock_guard lock(stats_mutex); - mh->mod->prep_counts(); + mh->mod->prep_counts(true); mh->mod->sum_stats(true); } } @@ -1475,7 +1475,7 @@ void ModuleManager::accumulate_module(const char* name) if ( mh ) { lock_guard lock(stats_mutex); - mh->mod->prep_counts(); + mh->mod->prep_counts(true); mh->mod->sum_stats(true); } } diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc index 728ac4762..f95d01f94 100644 --- a/src/network_inspectors/appid/appid_module.cc +++ b/src/network_inspectors/appid/appid_module.cc @@ -588,10 +588,10 @@ PegCount* AppIdModule::get_counts() const return (PegCount*)&appid_stats; } -void AppIdModule::sum_stats(bool accumulate_now_stats) +void AppIdModule::sum_stats(bool dump_stats) { AppIdPegCounts::sum_stats(); - Module::sum_stats(accumulate_now_stats); + Module::sum_stats(dump_stats); } void AppIdModule::show_dynamic_stats() diff --git a/src/network_inspectors/perf_monitor/base_tracker.cc b/src/network_inspectors/perf_monitor/base_tracker.cc index bbd2c069d..ae8de643b 100644 --- a/src/network_inspectors/perf_monitor/base_tracker.cc +++ b/src/network_inspectors/perf_monitor/base_tracker.cc @@ -50,7 +50,7 @@ BaseTracker::BaseTracker(PerfConfig* perf) : PerfTracker(perf, PERF_NAME "_base" void BaseTracker::process(bool summary) { for ( Module* mod : mods_to_prep ) - mod->prep_counts(); + mod->prep_counts(false); write(); diff --git a/src/packet_io/sfdaq_module.cc b/src/packet_io/sfdaq_module.cc index f8f724e70..c0399e1fa 100644 --- a/src/packet_io/sfdaq_module.cc +++ b/src/packet_io/sfdaq_module.cc @@ -209,6 +209,7 @@ const PegInfo daq_names[] = }; THREAD_LOCAL DAQStats daq_stats; +static THREAD_LOCAL DAQ_Stats_t prev_daq_stats; const PegInfo* SFDAQModule::get_pegs() const { @@ -236,9 +237,8 @@ static DAQ_Stats_t operator-(const DAQ_Stats_t& left, const DAQ_Stats_t& right) return ret; } -void SFDAQModule::prep_counts() +void SFDAQModule::prep_counts(bool dump_stats) { - static THREAD_LOCAL DAQ_Stats_t prev_daq_stats; if ( SFDAQ::get_local_instance() == nullptr ) return; @@ -269,11 +269,17 @@ void SFDAQModule::prep_counts() else daq_stats.outstanding = 0; - prev_daq_stats = new_daq_stats; + if(!dump_stats) + prev_daq_stats = new_daq_stats; } void SFDAQModule::reset_stats() { + if ( SFDAQ::get_local_instance() != nullptr ) + { + DAQ_Stats_t new_daq_stats = *SFDAQ::get_stats(); + prev_daq_stats = new_daq_stats; + } Trough::clear_file_count(); Module::reset_stats(); } diff --git a/src/packet_io/sfdaq_module.h b/src/packet_io/sfdaq_module.h index 3693a94d3..770d54696 100644 --- a/src/packet_io/sfdaq_module.h +++ b/src/packet_io/sfdaq_module.h @@ -41,7 +41,7 @@ public: const PegInfo* get_pegs() const override; PegCount* get_counts() const override; - void prep_counts() override; + void prep_counts(bool dump_stats) override; void reset_stats() override; bool counts_need_prep() const override diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc index 7d8afd2ff..3b3c11a94 100644 --- a/src/stream/base/stream_base.cc +++ b/src/stream/base/stream_base.cc @@ -55,11 +55,8 @@ THREAD_LOCAL FlowControl* flow_con = nullptr; std::vector crash_dump_flow_control; static std::mutex crash_dump_flow_control_mutex; -static BaseStats g_stats; THREAD_LOCAL BaseStats stream_base_stats; -THREAD_LOCAL PegCount current_flows_prev; -THREAD_LOCAL PegCount uni_flows_prev; -THREAD_LOCAL PegCount uni_ip_flows_prev; + // FIXIT-L dependency on stats define in another file const PegInfo base_pegs[] = @@ -130,29 +127,8 @@ void base_prep() } } -void base_sum() -{ - sum_stats((PegCount*)&g_stats, (PegCount*)&stream_base_stats, - array_size(base_pegs) - 1 - NOW_PEGS_NUM); - - g_stats.current_flows += (int64_t)stream_base_stats.current_flows - (int64_t)current_flows_prev; - g_stats.uni_flows += (int64_t)stream_base_stats.uni_flows - (int64_t)uni_flows_prev; - g_stats.uni_ip_flows += (int64_t)stream_base_stats.uni_ip_flows - (int64_t)uni_ip_flows_prev; - - base_reset(false); -} - -void base_stats() +void base_reset() { - show_stats((PegCount*)&g_stats, base_pegs, array_size(base_pegs) - 1, MOD_NAME); -} - -void base_reset(bool reset_all) -{ - current_flows_prev = stream_base_stats.current_flows; - uni_flows_prev = stream_base_stats.uni_flows; - uni_ip_flows_prev = stream_base_stats.uni_ip_flows; - memset(&stream_base_stats, 0, sizeof(stream_base_stats)); if ( flow_con ) @@ -162,9 +138,6 @@ void base_reset(bool reset_all) if ( exp_cache ) exp_cache->reset_stats(); } - - if ( reset_all ) - memset(&g_stats, 0, sizeof(g_stats)); } //------------------------------------------------------------------------- diff --git a/src/stream/base/stream_module.cc b/src/stream/base/stream_module.cc index ddd85b5d4..d6b52622b 100644 --- a/src/stream/base/stream_module.cc +++ b/src/stream/base/stream_module.cc @@ -233,17 +233,21 @@ bool StreamModule::end(const char* fqn, int, SnortConfig* sc) return true; } -void StreamModule::prep_counts() +void StreamModule::prep_counts(bool) { base_prep(); } -void StreamModule::sum_stats(bool) -{ base_sum(); } - -void StreamModule::show_stats() -{ base_stats(); } +void StreamModule::sum_stats(bool dump_stats) +{ + Module::sum_stats(dump_stats); + if(!dump_stats) + base_reset(); +} void StreamModule::reset_stats() -{ base_reset(); } +{ + base_reset(); + Module::reset_stats(); +} // Stream handler to adjust allocated resources as needed on a config reload bool StreamReloadResourceManager::initialize(const StreamModuleConfig& config_) diff --git a/src/stream/base/stream_module.h b/src/stream/base/stream_module.h index f8db8fd49..f21718cf6 100644 --- a/src/stream/base/stream_module.h +++ b/src/stream/base/stream_module.h @@ -160,9 +160,8 @@ public: unsigned get_gid() const override; const snort::RuleMap* get_rules() const override; - void prep_counts() override; + void prep_counts(bool dump_stats) override; void sum_stats(bool) override; - void show_stats() override; void reset_stats() override; bool counts_need_prep() const override @@ -179,8 +178,6 @@ private: }; extern void base_prep(); -extern void base_sum(); -extern void base_stats(); -extern void base_reset(bool reset_all=true); +extern void base_reset(); #endif diff --git a/src/trace/trace.cc b/src/trace/trace.cc index 3170ba3dd..3bcbb958a 100644 --- a/src/trace/trace.cc +++ b/src/trace/trace.cc @@ -97,7 +97,7 @@ Module::Module(const char*, const char*, const Parameter*, bool) PegCount Module::get_global_count(char const*) const { return 0; } void Module::show_interval_stats(std::vector >&, FILE*) {} void Module::show_stats(){} -void Module::sum_stats(bool ){} +void Module::sum_stats(bool){} void Module::reset_stats() {} class TraceTestModule : public Module diff --git a/src/utils/stats.cc b/src/utils/stats.cc index 965501c1a..2673ff23a 100644 --- a/src/utils/stats.cc +++ b/src/utils/stats.cc @@ -303,12 +303,13 @@ void PrintStatistics() //------------------------------------------------------------------------- void sum_stats( - PegCount* gpegs, PegCount* tpegs, unsigned n) + PegCount* gpegs, PegCount* tpegs, unsigned n, bool dump_stats) { for ( unsigned i = 0; i < n; ++i ) { gpegs[i] += tpegs[i]; - tpegs[i] = 0; + if(!dump_stats) + tpegs[i] = 0; } } diff --git a/src/utils/stats.h b/src/utils/stats.h index a4a682159..0f34d9a1f 100644 --- a/src/utils/stats.h +++ b/src/utils/stats.h @@ -109,7 +109,7 @@ SO_PUBLIC void LogStat(const char*, uint64_t n, uint64_t tot, FILE* = stdout); SO_PUBLIC void LogStat(const char*, double, FILE* = stdout); } -void sum_stats(PegCount* sums, PegCount* counts, unsigned n); +void sum_stats(PegCount* sums, PegCount* counts, unsigned n, bool dump_stats = false); void show_stats(PegCount*, const PegInfo*, const char* module_name = nullptr); void show_stats(PegCount*, const PegInfo*, unsigned n, const char* module_name = nullptr); void show_stats(PegCount*, const PegInfo*, const IndexVec&, const char* module_name, FILE*);