Pull request #3803: appid: AppIdPegCounters thread fixes

Merge in SNORT/snort3 from ~OSTEPANO/snort3:appid_pegs_data_ref to master

Squashed commit of the following:

commit 49fedbbdbfb3e6e06a131f51aefec1603a1b3d83
Author: Oleksandr Stepanov <ostepano@cisco.com>
Date:   Fri Apr 7 12:02:51 2023 -0400

    appid: AppIdPegCounters thread data handling refactored to prevent data races

diff --git a/src/network_inspectors/appid/appid_peg_counts.cc b/src/network_inspectors/appid/appid_peg_counts.cc
index 45c3e4b24be5f56333e81fbab37622aa4ac808ef..937eaf65025498808f6f612ff54fd5064070afab 100644 (file)
--- a/src/network_inspectors/appid/appid_peg_counts.cc
+++ b/src/network_inspectors/appid/appid_peg_counts.cc
@@ -34,139 +34,165 @@
 
 using namespace snort;
 
-std::unordered_map<AppId, uint32_t> AppIdPegCounts::appid_detector_pegs_idx;
-std::vector<std::string> AppIdPegCounts::appid_detectors_info;
-THREAD_LOCAL std::vector<AppIdPegCounts::AppIdDynamicPeg>* AppIdPegCounts::appid_peg_counts;
+THREAD_LOCAL AppIdPegCounts::AppIdDynamicPeg* AppIdPegCounts::unknown_appids_peg;
+THREAD_LOCAL std::unordered_map<AppId, AppIdPegCounts::AppIdDynamicPeg>* AppIdPegCounts::appid_peg_counts;
+std::unordered_map<AppId, std::pair<std::string, uint32_t>> AppIdPegCounts::appid_peg_ids;
 AppIdPegCounts::AppIdDynamicPeg AppIdPegCounts::appid_dynamic_sum[SF_APPID_MAX + 1];
 AppIdPegCounts::AppIdDynamicPeg AppIdPegCounts::zeroed_peg;
 PegCount AppIdPegCounts::all_zeroed_peg[DetectorPegs::NUM_APPID_DETECTOR_PEGS] = {};
 
+static std::mutex dynamic_stats_mutex;
+
 void AppIdPegCounts::init_pegs()
 {
     assert(!appid_peg_counts);
-    appid_peg_counts = new std::vector<AppIdPegCounts::AppIdDynamicPeg>(
-        appid_detectors_info.size() + 1, zeroed_peg);
+    appid_peg_counts = new std::unordered_map<AppId, AppIdPegCounts::AppIdDynamicPeg>();
+
+    for (auto& appid : appid_peg_ids)
+    {
+        appid_peg_counts->emplace(appid.first, zeroed_peg);
+    }
+    unknown_appids_peg = new AppIdDynamicPeg(zeroed_peg);
 }
 
 void AppIdPegCounts::cleanup_pegs()
 {
     delete appid_peg_counts;
     appid_peg_counts = nullptr;
+    delete unknown_appids_peg;
+    unknown_appids_peg = nullptr;
 }
 
 void AppIdPegCounts::cleanup_peg_info()
 {
-    appid_detectors_info.clear();
-    appid_detector_pegs_idx.clear();
+    appid_peg_ids.clear();
 }
 
 void AppIdPegCounts::cleanup_dynamic_sum()
 {
-    for (unsigned app_num = 0; app_num < AppIdPegCounts::appid_detectors_info.size(); app_num++)
+    const std::lock_guard<std::mutex> _lock(dynamic_stats_mutex);
+
+    for (unsigned app_num = 0; app_num < SF_APPID_MAX; app_num++)
     {
         memset(appid_dynamic_sum[app_num].stats, 0, sizeof(PegCount) *
             DetectorPegs::NUM_APPID_DETECTOR_PEGS);
-        if ( appid_peg_counts )
-            memset((*appid_peg_counts)[app_num].stats, 0, sizeof(PegCount) *
-                DetectorPegs::NUM_APPID_DETECTOR_PEGS);
+    }
+
+    if (appid_peg_counts)
+    {
+        for (auto& peg : (*appid_peg_counts))
+        {
+            memset(&peg.second.stats, 0, sizeof(PegCount) * DetectorPegs::NUM_APPID_DETECTOR_PEGS);
+        }
     }
 
     // reset unknown_app stats
     memset(appid_dynamic_sum[SF_APPID_MAX].stats, 0, sizeof(PegCount) *
         DetectorPegs::NUM_APPID_DETECTOR_PEGS);
-    if ( appid_peg_counts )
-        memset((*appid_peg_counts)[appid_peg_counts->size() - 1].stats, 0, sizeof(PegCount) *
-            DetectorPegs::NUM_APPID_DETECTOR_PEGS);
+    if (unknown_appids_peg)
+        memset(&unknown_appids_peg->stats, 0, sizeof(PegCount) * DetectorPegs::NUM_APPID_DETECTOR_PEGS);
+        
 }
 
 void AppIdPegCounts::add_app_peg_info(std::string app_name, AppId app_id)
 {
     std::replace(app_name.begin(), app_name.end(), ' ', '_');
 
-    appid_detector_pegs_idx[app_id] = appid_detectors_info.size();
-    appid_detectors_info.emplace_back(app_name);
+    appid_peg_ids.emplace(app_id, std::make_pair(app_name, appid_peg_ids.size()));
 }
 
 void AppIdPegCounts::sum_stats()
 {
-    if (!appid_peg_counts)
+    if (!appid_peg_counts or !unknown_appids_peg)
         return;
 
-    static std::mutex r_mutex;
+    const std::lock_guard<std::mutex> _lock(dynamic_stats_mutex);
 
+    for (auto& peg : (*appid_peg_counts))
     {
-        std::lock_guard<std::mutex> _lock(r_mutex);
-        const unsigned peg_num = appid_peg_counts->size() ? (appid_peg_counts->size() - 1) : 0;
-        AppIdDynamicPeg* ptr = (AppIdDynamicPeg*)appid_peg_counts->data();
+        auto dyn_indx = appid_peg_ids[peg.first].second;
+        for (unsigned j = 0; j < DetectorPegs::NUM_APPID_DETECTOR_PEGS; ++j)
+            appid_dynamic_sum[dyn_indx].stats[j] += peg.second.stats[j];
 
-        for (unsigned i = 0; i < peg_num; ++i)
-        {
-            for (unsigned j = 0; j < DetectorPegs::NUM_APPID_DETECTOR_PEGS; ++j)
-                appid_dynamic_sum[i].stats[j] += ptr[i].stats[j];
+        peg.second.zero_out();
+    }
 
-            ptr[i].zero_out();
-        }
+    // unknown_app stats
+    for (unsigned j = 0; j < DetectorPegs::NUM_APPID_DETECTOR_PEGS; ++j)
+        appid_dynamic_sum[SF_APPID_MAX].stats[j] += unknown_appids_peg->stats[j];
 
-        // unknown_app stats
-        for (unsigned j = 0; j < DetectorPegs::NUM_APPID_DETECTOR_PEGS; ++j)
-            appid_dynamic_sum[SF_APPID_MAX].stats[j] += ptr[peg_num].stats[j];
-        ptr[peg_num].zero_out();
-    }
-    
+    unknown_appids_peg->zero_out();
 }
 
 void AppIdPegCounts::inc_service_count(AppId id)
 {
-    (*appid_peg_counts)[get_stats_index(id)].stats[DetectorPegs::SERVICE_DETECTS]++;
+    auto peg = appid_peg_counts->find(id);
+    if (peg != appid_peg_counts->end())
+        peg->second.stats[DetectorPegs::SERVICE_DETECTS]++;
+    else
+        unknown_appids_peg->stats[DetectorPegs::SERVICE_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_client_count(AppId id)
 {
-    (*appid_peg_counts)[get_stats_index(id)].stats[DetectorPegs::CLIENT_DETECTS]++;
+    auto peg = appid_peg_counts->find(id);
+    if (peg != appid_peg_counts->end())
+        peg->second.stats[DetectorPegs::CLIENT_DETECTS]++;
+    else
+        unknown_appids_peg->stats[DetectorPegs::CLIENT_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_payload_count(AppId id)
 {
-    (*appid_peg_counts)[get_stats_index(id)].stats[DetectorPegs::PAYLOAD_DETECTS]++;
+    auto peg = appid_peg_counts->find(id);
+    if (peg != appid_peg_counts->end())
+        peg->second.stats[DetectorPegs::PAYLOAD_DETECTS]++;
+    else
+        unknown_appids_peg->stats[DetectorPegs::PAYLOAD_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_user_count(AppId id)
 {
-    (*appid_peg_counts)[get_stats_index(id)].stats[DetectorPegs::USER_DETECTS]++;
+    auto peg = appid_peg_counts->find(id);
+    if (peg != appid_peg_counts->end())
+        peg->second.stats[DetectorPegs::USER_DETECTS]++;
+    else
+        unknown_appids_peg->stats[DetectorPegs::USER_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_misc_count(AppId id)
 {
-    (*appid_peg_counts)[get_stats_index(id)].stats[DetectorPegs::MISC_DETECTS]++;
+    auto peg = appid_peg_counts->find(id);
+    if (peg != appid_peg_counts->end())
+        peg->second.stats[DetectorPegs::MISC_DETECTS]++;
+    else
+        unknown_appids_peg->stats[DetectorPegs::MISC_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_referred_count(AppId id)
 {
-    (*appid_peg_counts)[get_stats_index(id)].stats[DetectorPegs::REFERRED_DETECTS]++;
-}
-
-uint32_t AppIdPegCounts::get_stats_index(AppId id)
-{
-    std::unordered_map<AppId, uint32_t>::iterator stats_idx_it = appid_detector_pegs_idx.find(id);
-    if ( stats_idx_it != appid_detector_pegs_idx.end() )
-        return stats_idx_it->second;
+    auto peg = appid_peg_counts->find(id);
+    if (peg != appid_peg_counts->end())
+        peg->second.stats[DetectorPegs::REFERRED_DETECTS]++;
     else
-        return appid_detectors_info.size();
+        unknown_appids_peg->stats[DetectorPegs::REFERRED_DETECTS]++;
 }
 
 void AppIdPegCounts::print()
 {
     bool print = false;
-    unsigned app_num = AppIdPegCounts::appid_detectors_info.size();
+    unsigned app_num = AppIdPegCounts::appid_peg_ids.size();
+
+    std::unordered_map<uint32_t, std::string*> tmp_sorting_map;
 
-    for (unsigned i = 0; i < app_num; i++)
+    for (auto& det_info : AppIdPegCounts::appid_peg_ids)
     {
-        AppIdDynamicPeg* pegs = &appid_dynamic_sum[i];
+        AppIdDynamicPeg* pegs = &appid_dynamic_sum[det_info.second.second];
         if (!pegs->all_zeros())
         {
-            print = true;
-            break;
+            print |= true;
         }
+        tmp_sorting_map.emplace(det_info.second.second, &det_info.second.first);
     }
 
     AppIdDynamicPeg* unknown_pegs = &appid_dynamic_sum[SF_APPID_MAX];
@@ -181,14 +207,13 @@ void AppIdPegCounts::print()
         "Application", "Services", "Clients", "Users", "Payloads", "Misc", "Referred");
     LogText(buff);
 
-    for (unsigned i = 0; i < app_num; i++)
+    for (uint32_t i = 0; i < app_num; i++)
     {
         AppIdDynamicPeg* pegs = &appid_dynamic_sum[i];
         if (pegs->all_zeros())
             continue;
 
-        std::string app_name = AppIdPegCounts::appid_detectors_info[i];
-        pegs->print(app_name.c_str(), buff, sizeof(buff));
+        pegs->print(tmp_sorting_map[i]->c_str(), buff, sizeof(buff));
         LogText(buff);
     }
 

diff --git a/src/network_inspectors/appid/appid_peg_counts.h b/src/network_inspectors/appid/appid_peg_counts.h
index 010ff71755446d6910b0e20d0ebf14661cc01a2b..b74847bd6a236100c727a9051e3b6b1d5b88ae6e 100644 (file)
--- a/src/network_inspectors/appid/appid_peg_counts.h
+++ b/src/network_inspectors/appid/appid_peg_counts.h
@@ -112,11 +112,10 @@ public:
     static void print();
 
 private:
-    static std::unordered_map<AppId, uint32_t> appid_detector_pegs_idx;
-    static std::vector<std::string> appid_detectors_info;
     static AppIdDynamicPeg appid_dynamic_sum[SF_APPID_MAX+1];
-    static THREAD_LOCAL std::vector<AppIdDynamicPeg>* appid_peg_counts;
-    static uint32_t get_stats_index(AppId id);
+    static THREAD_LOCAL AppIdDynamicPeg* unknown_appids_peg;
+    static THREAD_LOCAL std::unordered_map<AppId, AppIdPegCounts::AppIdDynamicPeg>* appid_peg_counts;
+    static std::unordered_map<AppId, std::pair<std::string, uint32_t>> appid_peg_ids;
     static AppIdDynamicPeg zeroed_peg;
     static PegCount all_zeroed_peg[DetectorPegs::NUM_APPID_DETECTOR_PEGS];
 };