Merge pull request #1739 in SNORT/snort3 from ~BBANTWAL/snort3:mpse_stash_fix to...

Squashed commit of the following:

commit 94b58403014d34f2160aceb3c4d6ab6bfbb89ae0
Author: Bhagya Tholpady <bbantwal@cisco.com>
Date:   Mon Sep 9 12:40:23 2019 -0400

    detection: change the hardcoded stash max to configurable one, convert the stash queue to vector, and add new pegcounts for stash overruns

diff --git a/src/detection/fp_config.cc b/src/detection/fp_config.cc
index 22b1d8530a1348183c46372572f3441b82c2c1ee..ac763605bddfc644ce2d25e39ed8c4fa841fe7a1 100644 (file)
--- a/src/detection/fp_config.cc
+++ b/src/detection/fp_config.cc
@@ -104,3 +104,7 @@ unsigned FastPatternConfig::set_max(unsigned bytes)
     return bytes;
 }
 
+void FastPatternConfig::set_queue_limit(unsigned int limit)
+{
+    queue_limit = limit;
+}

diff --git a/src/detection/fp_config.h b/src/detection/fp_config.h
index ca998ea69e727266c2b39def23b76b77b6bfe508..a0eb4426caa3283f4a779c273ab96c8ca606d414 100644 (file)
--- a/src/detection/fp_config.h
+++ b/src/detection/fp_config.h
@@ -130,6 +130,11 @@ public:
 
     void set_max_pattern_len(unsigned);
 
+    void set_queue_limit(unsigned);
+
+    unsigned get_queue_limit()
+    { return queue_limit; }
+
     const snort::MpseApi* get_search_api()
     { return search_api; }
 
@@ -165,6 +170,8 @@ private:
     unsigned bleedover_port_limit = 1024;
     unsigned max_pattern_len = 0;
 
+    unsigned queue_limit = 0;
+
     int portlists_flags = 0;
     int num_patterns_truncated = 0;  // due to max_pattern_len
     int num_patterns_trimmed = 0;    // due to zero byte prefix

diff --git a/src/detection/fp_detect.cc b/src/detection/fp_detect.cc
index de460dcb36ded08a776ecc99365248a7c22a7a7d..f754a7ded753b9e31090d737c020a6f703946635 100644 (file)
--- a/src/detection/fp_detect.cc
+++ b/src/detection/fp_detect.cc
@@ -720,20 +720,28 @@ static inline int fpFinalSelectEvent(OtnxMatchData* omd, Packet* p)
     return 0;
 }
 
+struct Node
+{
+    void* user;
+    void* tree;
+    void* list;
+    int index;
+};
+
+
 class MpseStash
 {
 public:
-    // FIXIT-H use max = n * k, at most k per group
-    // need n >= 4 for src+dst+gen+svc
-    static const unsigned max = 32;
-
-    MpseStash()
-    { enable = false; }
+    MpseStash(unsigned limit)
+    {
+        enable = false;
+        max = limit;
+    }
 
     void init()
     {
         if ( enable )
-            count = flushed = 0;
+            count = 0;
     }
 
     // this is done in the offload thread
@@ -751,42 +759,40 @@ public:
 private:
     bool enable;
     unsigned count;
-    unsigned flushed;
-
-    struct Node
-    {
-        void* user;
-        void* tree;
-        void* list;
-        int index;
-    } queue[max];
+    unsigned max;
+    std::vector<Node> queue;
 };
 
 // uniquely insert into q, should splay elements for performance
 // return true if maxed out to trigger a flush
 bool MpseStash::push(void* user, void* tree, int index, void* list)
 {
-    pmqs.tot_inq_inserts++;
 
-    for ( int i = (int)(count) - 1; i >= 0; --i )
+    for ( auto it = queue.rbegin(); it != queue.rend(); it++ )
     {
-        if ( tree == queue[i].tree )
+        if ( tree == (*it).tree )
+        {
+            pmqs.tot_inq_inserts++;
             return false;
+        }
     }
 
-    if ( count < max )
+    if ( !max or ( count < max ) )
     {
-        Node& node = queue[count++];
+        Node node;
         node.user = user;
         node.tree = tree;
         node.index = index;
         node.list = list;
+        queue.push_back(node);
         pmqs.tot_inq_uinserts++;
+        pmqs.tot_inq_inserts++;
+        count++;
     }
 
-    if ( count == max )
+    if ( max and ( count == max ) )
     {
-        flushed++;
+        pmqs.tot_inq_overruns++;
         return true;
     }
 
@@ -801,35 +807,39 @@ bool MpseStash::process(MpseMatch match, void* context)
     if ( count > pmqs.max_inq )
         pmqs.max_inq = count;
 
-    pmqs.tot_inq_flush += flushed;
 
 #ifdef DEBUG_MSGS
     if (count == 0)
         trace_log(detection, TRACE_RULE_EVAL, "Fast pattern processing - no matches found\n");
 #endif
-
-    for ( unsigned i = 0; i < count; ++i )
+    unsigned i = 0;
+    for ( auto it : queue )
     {
-        Node& node = queue[i];
-
+        Node& node = it;
+        i++;
         // process a pattern - case is handled by otn processing
-        trace_logf(detection, TRACE_RULE_EVAL,"Processing pattern match #%d\n", i+1);
+        trace_logf(detection, TRACE_RULE_EVAL,"Processing pattern match #%d\n", i);
         int res = match(node.user, node.tree, node.index, context, node.list);
 
         if ( res > 0 )
         {
             /* terminate matching */
+            pmqs.tot_inq_flush += count;
             count = 0;
+            queue.clear();
             return true;
         }
     }
+    pmqs.tot_inq_flush += count;
     count = 0;
+    queue.clear();
     return false;
 }
 
 void fp_set_context(IpsContext& c)
 {
-    c.stash = new MpseStash;
+    FastPatternConfig* fp = SnortConfig::get_conf()->fast_pattern_config;
+    c.stash = new MpseStash(fp->get_queue_limit());
     c.otnx = (OtnxMatchData*)snort_calloc(sizeof(OtnxMatchData));
     c.otnx->matchInfo = (MatchInfo*)snort_calloc(MAX_NUM_RULE_TYPES, sizeof(MatchInfo));
     c.context_num = 0;
@@ -848,11 +858,7 @@ static int rule_tree_queue(
 {
     MpseStash* stash = ((IpsContext*)context)->stash;
 
-    if ( stash->push(user, tree, index, list) )
-    {
-        if ( stash->process(rule_tree_match, context) )
-            return 1;
-    }
+    stash->push(user, tree, index, list);
     return 0;
 }
 

diff --git a/src/main/modules.cc b/src/main/modules.cc
index 15db0e0436fbca4b937a933f7a6775f47bc2dd07..5f59de895493f1270245ce75a9fd16e00b53d18f 100644 (file)
--- a/src/main/modules.cc
+++ b/src/main/modules.cc
@@ -319,6 +319,9 @@ static const Parameter search_engine_params[] =
     { "split_any_any", Parameter::PT_BOOL, nullptr, "true",
       "evaluate any-any rules separately to save memory" },
 
+    { "queue_limit", Parameter::PT_INT, "0:max32", "128",
+      "maximum number of fast pattern matches to queue per packet (0 means no maximum)" },
+
     { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr }
 };
 
@@ -333,8 +336,9 @@ THREAD_LOCAL PatMatQStat pmqs;
 const PegInfo mpse_pegs[] =
 {
     { CountType::SUM, "max_queued", "maximum fast pattern matches queued for further evaluation" },
-    { CountType::SUM, "total_flushed", "fast pattern matches discarded due to overflow" },
+    { CountType::SUM, "total_flushed", "total fast pattern matches processed" },
     { CountType::SUM, "total_inserts", "total fast pattern hits" },
+    { CountType::SUM, "total_overruns", "fast pattern matches discarded due to overflow" },
     { CountType::SUM, "total_unique", "total unique fast pattern hits" },
     { CountType::SUM, "non_qualified_events", "total non-qualified events" },
     { CountType::SUM, "qualified_events", "total qualified events" },
@@ -428,6 +432,9 @@ bool SearchEngineModule::set(const char*, Value& v, SnortConfig* sc)
     else if ( v.is("split_any_any") )
         fp->set_split_any_any(v.get_bool());
 
+    else if ( v.is("queue_limit") )
+        fp->set_queue_limit(v.get_uint32());
+
     else
         return false;
 

diff --git a/src/search_engines/pat_stats.h b/src/search_engines/pat_stats.h
index 21ba09320db8a64e6873be9d3de11d73b4593eb1..5a63101e8dcc9c56b8ab6934a72842c3f62cd071 100644 (file)
--- a/src/search_engines/pat_stats.h
+++ b/src/search_engines/pat_stats.h
@@ -31,6 +31,7 @@ struct PatMatQStat
     PegCount max_inq;
     PegCount tot_inq_flush;
     PegCount tot_inq_inserts;
+    PegCount tot_inq_overruns;
     PegCount tot_inq_uinserts;
     PegCount non_qualified_events;
     PegCount qualified_events;