rule grouping: speed up port based grouping

Create a hash table of unique DetectPort objects before trying to
create a unique list of these objects. This safes a lot of cycles
in the creation of the list.

src/detect-engine-port.c

@@ -1493,6 +1493,139 @@ int DetectPortIsValidRange(char *port)
 }
 /********************** End parsing routines ********************/
 
+/* hash table */
+
+/**
+ * \brief The hash function to be the used by the hash table -
+ *        DetectEngineCtx->dport_hash_table.
+ *
+ * \param ht      Pointer to the hash table.
+ * \param data    Pointer to the DetectPort.
+ * \param datalen Not used in our case.
+ *
+ * \retval hash The generated hash value.
+ */
+static uint32_t DetectPortHashFunc(HashListTable *ht, void *data, uint16_t datalen)
+{
+    DetectPort *p = (DetectPort *)data;
+    uint32_t hash = 0;
+
+    SCLogDebug("hashing sgh %p", p);
+
+    hash = (p->port << 16) | p->port2;
+
+    hash %= ht->array_size;
+    SCLogDebug("hash %"PRIu32, hash);
+    return hash;
+}
+
+/**
+ * \brief The Compare function to be used by the DetectPort hash table -
+ *        DetectEngineCtx->dport_hash_table.
+ *
+ * \param data1 Pointer to the first DetectPort.
+ * \param len1  Not used.
+ * \param data2 Pointer to the second DetectPort.
+ * \param len2  Not used.
+ *
+ * \retval 1 If the 2 DetectPort sent as args match.
+ * \retval 0 If the 2 DetectPort sent as args do not match.
+ */
+static char DetectPortCompareFunc(void *data1, uint16_t len1,
+                                  void *data2, uint16_t len2)
+{
+    DetectPort *dp1 = (DetectPort *)data1;
+    DetectPort *dp2 = (DetectPort *)data2;
+
+    if (data1 == NULL || data2 == NULL)
+        return 0;
+
+    if (dp1->port == dp2->port && dp1->port2 == dp2->port2)
+        return 1;
+
+    return 0;
+}
+
+static void DetectPortHashFreeFunc(void *ptr)
+{
+    DetectPort *p = ptr;
+    DetectPortFree(p);
+}
+
+/**
+ * \brief Initializes the hash table in the detection engine context to hold the
+ *        DetectPort hash.
+ *
+ * \param de_ctx Pointer to the detection engine context.
+ *
+ * \retval  0 On success.
+ * \retval -1 On failure.
+ */
+int DetectPortHashInit(DetectEngineCtx *de_ctx)
+{
+    de_ctx->dport_hash_table = HashListTableInit(4096, DetectPortHashFunc,
+                                                       DetectPortCompareFunc,
+                                                       DetectPortHashFreeFunc);
+    if (de_ctx->dport_hash_table == NULL)
+        goto error;
+
+    return 0;
+
+error:
+    return -1;
+}
+
+/**
+ * \brief Adds a DetectPort to the detection engine context DetectPort
+ *        hash table.
+ *
+ * \param de_ctx Pointer to the detection engine context.
+ * \param dp     Pointer to the DetectPort.
+ *
+ * \retval ret 0 on Successfully adding the DetectPort; -1 on failure.
+ */
+int DetectPortHashAdd(DetectEngineCtx *de_ctx, DetectPort *dp)
+{
+    int ret = HashListTableAdd(de_ctx->dport_hash_table, (void *)dp, 0);
+    return ret;
+}
+
+/**
+ * \brief Used to lookup a DetectPort hash from the detection engine context
+ *        DetectPort hash table.
+ *
+ * \param de_ctx Pointer to the detection engine context.
+ * \param sgh    Pointer to the DetectPort.
+ *
+ * \retval rsgh On success a pointer to the DetectPort if the DetectPort is
+ *              found in the hash table; NULL on failure.
+ */
+DetectPort *DetectPortHashLookup(DetectEngineCtx *de_ctx, DetectPort *dp)
+{
+    SCEnter();
+
+    DetectPort *rdp = HashListTableLookup(de_ctx->dport_hash_table, (void *)dp, 0);
+
+    SCReturnPtr(rdp, "DetectPort");
+}
+
+/**
+ * \brief Frees the hash table - DetectEngineCtx->sgh_hash_table, allocated by
+ *        DetectPortInit() function.
+ *
+ * \param de_ctx Pointer to the detection engine context.
+ */
+void DetectPortHashFree(DetectEngineCtx *de_ctx)
+{
+    if (de_ctx->sgh_hash_table == NULL)
+        return;
+
+    HashListTableFree(de_ctx->dport_hash_table);
+    de_ctx->dport_hash_table = NULL;
+
+    return;
+}
+
 /*---------------------- Unittests -------------------------*/
 
 #ifdef UNITTESTS

src/detect-engine-port.h

@@ -46,6 +46,11 @@ void DetectPortFree(DetectPort *);
 
 int DetectPortTestConfVars(void);
 
+DetectPort *DetectPortHashLookup(DetectEngineCtx *de_ctx, DetectPort *dp);
+void DetectPortHashFree(DetectEngineCtx *de_ctx);
+int DetectPortHashAdd(DetectEngineCtx *de_ctx, DetectPort *dp);
+int DetectPortHashInit(DetectEngineCtx *de_ctx);
+
 void DetectPortTests(void);
 
 #endif /* __DETECT_PORT_H__ */

src/detect.c

@@ -3374,9 +3374,10 @@ int CreateGroupedPortList(DetectEngineCtx *de_ctx, DetectPort *port_list, Detect
 int CreateGroupedPortListCmpCnt(DetectPort *a, DetectPort *b);
 
 static DetectPort *RulesGroupByPorts(DetectEngineCtx *de_ctx, int ipproto, uint32_t direction) {
-    /* step 1: create a list of 'DetectPort' objects based on all the
+    /* step 1: create a hash of 'DetectPort' objects based on all the
      *         rules. Each object will have a SGH with the sigs added
      *         that belong to the SGH. */
+    DetectPortHashInit(de_ctx);
 
     uint32_t max_idx = 0;
     const Signature *s = de_ctx->sig_list;
@@ -3414,21 +3415,20 @@ static DetectPort *RulesGroupByPorts(DetectEngineCtx *de_ctx, int ipproto, uint3
 
         int wl = s->whitelist;
         while (p) {
-            DetectPort *tmp = DetectPortCopySingle(de_ctx, p);
-            BUG_ON(tmp == NULL);
-            SigGroupHeadAppendSig(de_ctx, &tmp->sh, s);
-
-            int pwl = PortIsWhitelisted(tmp, ipproto) ? 111 : 0;
-            tmp->sh->init->whitelist = MAX(wl, pwl);
-            if (tmp->sh->init->whitelist) {
-                SCLogDebug("%s/%s Rule %u whitelisted port group %u:%u",
-                        direction == SIG_FLAG_TOSERVER ? "toserver" : "toclient",
-                        ipproto == 6 ? "TCP" : "UDP",
-                        s->id, p->port, p->port2);
-            }
+            int pwl = PortIsWhitelisted(p, ipproto) ? 111 : 0;
+            pwl = MAX(wl,pwl);
 
-            int r = DetectPortInsert(de_ctx, &list , tmp);
-            BUG_ON(r == -1);
+            DetectPort *lookup = DetectPortHashLookup(de_ctx, p);
+            if (lookup) {
+                SigGroupHeadAppendSig(de_ctx, &lookup->sh, s);
+                lookup->sh->init->whitelist = MAX(lookup->sh->init->whitelist, pwl);
+            } else {
+                DetectPort *tmp2 = DetectPortCopySingle(de_ctx, p);
+                BUG_ON(tmp2 == NULL);
+                SigGroupHeadAppendSig(de_ctx, &tmp2->sh, s);
+                tmp2->sh->init->whitelist = pwl;
+                DetectPortHashAdd(de_ctx, tmp2);
+            }
 
             p = p->next;
         }
@@ -3437,15 +3437,31 @@ static DetectPort *RulesGroupByPorts(DetectEngineCtx *de_ctx, int ipproto, uint3
         s = s->next;
     }
 
+    /* step 2: create a list of DetectPort objects */
+    HashListTableBucket *htb = NULL;
+    for (htb = HashListTableGetListHead(de_ctx->dport_hash_table);
+            htb != NULL;
+            htb = HashListTableGetListNext(htb))
+    {
+        DetectPort *p = HashListTableGetListData(htb);
+        DetectPort *tmp = DetectPortCopySingle(de_ctx, p);
+        BUG_ON(tmp == NULL);
+        int r = DetectPortInsert(de_ctx, &list , tmp);
+        BUG_ON(r == -1);
+    }
+    DetectPortHashFree(de_ctx);
+    de_ctx->dport_hash_table = NULL;
+
     SCLogDebug("rules analyzed");
 
+    /* step 3: group the list and shrink it if necessary */
     DetectPort *newlist = NULL;
     uint16_t groupmax = (direction == SIG_FLAG_TOCLIENT) ? de_ctx->max_uniq_toclient_groups :
                                                            de_ctx->max_uniq_toserver_groups;
     CreateGroupedPortList(de_ctx, list, &newlist, groupmax, CreateGroupedPortListCmpCnt, max_idx);
     list = newlist;
 
-    /* step 2: deduplicate the SGH's */
+    /* step 4: deduplicate the SGH's */
     SigGroupHeadHashFree(de_ctx);
     SigGroupHeadHashInit(de_ctx);
 

src/detect.h

@@ -665,6 +665,9 @@ typedef struct DetectEngineCtx_ {
     /** id of loader thread 'owning' this de_ctx */
     int loader_id;
 
+
+    HashListTable *dport_hash_table;
+
 } DetectEngineCtx;
 
 /* Engine groups profiles (low, medium, high, custom) */