Merge pull request #2044 in SNORT/snort3 from ~MASHASAN/snort3:stats_and_data_races...

Squashed commit of the following:

commit 9d4b9171cdde544f26b63f2390e6dafc3fb7f1fb
Author: Masud Hasan <mashasan@cisco.com>
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 9eb9be71a2245fc86827aeaaa643a6035393077d..9a554f51db9f8f4dc25b8a04957e1735b2244e2b 100644 (file)
--- 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 657db6d1c236799c380e1cf1663bcce87a8dffc5..59c44ca17266da70615f44ff6ca9e031542ee1dd 100644 (file)
--- a/src/main/request.h
+++ b/src/main/request.h
@@ -22,8 +22,6 @@
 #ifndef REQUEST_H
 #define REQUEST_H
 
-#include <atomic>
-
 #include "main/snort_types.h"
 
 class Request
@@ -43,6 +41,6 @@ private:
     int fd;
     char read_buf[1024];
     size_t bytes_read;
-    std::atomic<const char*> queued_response;
+    const char* queued_response;
 };
 #endif

diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc
index 1b66efe7ab6f9b035fbee52d6c3ac55611c30d28..33e91d8ed08a65689a1d2a6de8f9725c7e1f3647 100644 (file)
--- a/src/managers/module_manager.cc
+++ b/src/managers/module_manager.cc
@@ -29,7 +29,6 @@
 #include <algorithm>
 #include <cassert>
 #include <iostream>
-#include <mutex>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -75,7 +74,8 @@ static std::unordered_map<std::string, const Parameter*> s_pmap;
 
 static unsigned s_errors = 0;
 
-std::set<uint32_t> ModuleManager::gids;
+set<uint32_t> 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<std::mutex> lock(stats_mutex);
+            lock_guard<mutex> 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<std::mutex> lock(stats_mutex);
+        lock_guard<mutex> 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<std::mutex> lock(stats_mutex);
+        lock_guard<mutex> 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<std::mutex> lock(stats_mutex);
+        lock_guard<mutex> lock(stats_mutex);
         mh->mod->reset_stats();
     }
 }

diff --git a/src/managers/module_manager.h b/src/managers/module_manager.h
index 46facfecddc65477e2902c1ecc1c0891e019ae75..eef450c2d1d0ccdfa2c891a0da90f338855688fe 100644 (file)
--- a/src/managers/module_manager.h
+++ b/src/managers/module_manager.h
@@ -24,8 +24,9 @@
 // Modules are strictly used during parse time.
 
 #include <cstdint>
-#include <set>
 #include <list>
+#include <mutex>
+#include <set>
 
 #include "main/snort_types.h"
 
@@ -90,6 +91,7 @@ public:
     static void reset_stats(SnortConfig*);
 
     static std::set<uint32_t> 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 70fded374e6cebc4d064d1614f03dcdae88ea9d0..a5aa2f20141609b0be3baf872628571f5f293025 100644 (file)
--- 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 <vector>)
 
+#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<mutex> 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 ddaa338859cd8ec8378f7bdc10be9918f7bd425a..5e2a9e9710f35f220ef6f7b013f60110be3e0148 100644 (file)
--- 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 3384ab30aeb9137310aea57b78eb8f61fb225c80..8bded598376c840dc5ca410caf0c3ca2fc893ed0 100644 (file)
--- 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 beea28088653453f8e5d13acd9ce1aaa69f0196d..3de048e305856c860a2141f5be61f14eca1777eb 100644 (file)
--- 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);