flow hash: Mask vlan_id if not used for tracking

If vlan.use-for-tracking is disabled, set the vlan_id fields to 0 when
hashing or comparing flows. This is done using a bitmask as suggested by
Victor Julien in IRC, in order to avoid adding more branches to this
code.

Currently, suricata does not fill in vlan_id fields if
vlan.use-for-tracking is disabled and instead leaves them at the default
0 value, so this commit makes no functional change. This change is in
preparation for future commits where the vlan_ids will be always filled
in.

Related to https://redmine.openinfosecfoundation.org/issues/3076

diff --git a/src/flow-hash.c b/src/flow-hash.c
index 5afa154304e4455e5824a193c16f55082a52d182..8492706aff404b74639a2e848c7433aab908e3ef 100644 (file)
--- a/src/flow-hash.c
+++ b/src/flow-hash.c
@@ -135,8 +135,10 @@ static inline uint32_t FlowGetHash(const Packet *p)
 
             fhk.proto = (uint16_t)p->proto;
             fhk.recur = (uint16_t)p->recursion_level;
-            fhk.vlan_id[0] = p->vlan_id[0];
-            fhk.vlan_id[1] = p->vlan_id[1];
+            /* g_vlan_mask sets the vlan_ids to 0 if vlan.use-for-tracking
+             * is disabled. */
+            fhk.vlan_id[0] = p->vlan_id[0] & g_vlan_mask;
+            fhk.vlan_id[1] = p->vlan_id[1] & g_vlan_mask;
 
             hash = hashword(fhk.u32, 5, flow_config.hash_rand);
 
@@ -155,8 +157,8 @@ static inline uint32_t FlowGetHash(const Packet *p)
 
             fhk.proto = (uint16_t)ICMPV4_GET_EMB_PROTO(p);
             fhk.recur = (uint16_t)p->recursion_level;
-            fhk.vlan_id[0] = p->vlan_id[0];
-            fhk.vlan_id[1] = p->vlan_id[1];
+            fhk.vlan_id[0] = p->vlan_id[0] & g_vlan_mask;
+            fhk.vlan_id[1] = p->vlan_id[1] & g_vlan_mask;
 
             hash = hashword(fhk.u32, 5, flow_config.hash_rand);
 
@@ -169,8 +171,8 @@ static inline uint32_t FlowGetHash(const Packet *p)
             fhk.ports[1] = 0xbeef;
             fhk.proto = (uint16_t)p->proto;
             fhk.recur = (uint16_t)p->recursion_level;
-            fhk.vlan_id[0] = p->vlan_id[0];
-            fhk.vlan_id[1] = p->vlan_id[1];
+            fhk.vlan_id[0] = p->vlan_id[0] & g_vlan_mask;
+            fhk.vlan_id[1] = p->vlan_id[1] & g_vlan_mask;
 
             hash = hashword(fhk.u32, 5, flow_config.hash_rand);
         }
@@ -201,8 +203,8 @@ static inline uint32_t FlowGetHash(const Packet *p)
         fhk.ports[pi] = p->dp;
         fhk.proto = (uint16_t)p->proto;
         fhk.recur = (uint16_t)p->recursion_level;
-        fhk.vlan_id[0] = p->vlan_id[0];
-        fhk.vlan_id[1] = p->vlan_id[1];
+        fhk.vlan_id[0] = p->vlan_id[0] & g_vlan_mask;
+        fhk.vlan_id[1] = p->vlan_id[1] & g_vlan_mask;
 
         hash = hashword(fhk.u32, 11, flow_config.hash_rand);
     }
@@ -234,8 +236,8 @@ uint32_t FlowKeyGetHash(FlowKey *fk)
 
         fhk.proto = (uint16_t)fk->proto;
         fhk.recur = (uint16_t)fk->recursion_level;
-        fhk.vlan_id[0] = fk->vlan_id[0];
-        fhk.vlan_id[1] = fk->vlan_id[1];
+        fhk.vlan_id[0] = fk->vlan_id[0] & g_vlan_mask;
+        fhk.vlan_id[1] = fk->vlan_id[1] & g_vlan_mask;
 
         hash = hashword(fhk.u32, 5, flow_config.hash_rand);
     } else {
@@ -266,8 +268,8 @@ uint32_t FlowKeyGetHash(FlowKey *fk)
         fhk.ports[pi] = fk->dp;
         fhk.proto = (uint16_t)fk->proto;
         fhk.recur = (uint16_t)fk->recursion_level;
-        fhk.vlan_id[0] = fk->vlan_id[0];
-        fhk.vlan_id[1] = fk->vlan_id[1];
+        fhk.vlan_id[0] = fk->vlan_id[0] & g_vlan_mask;
+        fhk.vlan_id[1] = fk->vlan_id[1] & g_vlan_mask;
 
         hash = hashword(fhk.u32, 11, flow_config.hash_rand);
     }
@@ -296,7 +298,8 @@ static inline bool CmpAddrsAndPorts(const uint32_t src1[4],
 
 static inline bool CmpVlanIds(const uint16_t vlan_id1[2], const uint16_t vlan_id2[2])
 {
-    return vlan_id1[0] == vlan_id2[0] && vlan_id1[1] == vlan_id2[1];
+    return ((vlan_id1[0] ^ vlan_id2[0]) & g_vlan_mask) == 0 &&
+        ((vlan_id1[1] ^ vlan_id2[1]) & g_vlan_mask) == 0;
 }
 
 /* Since two or more flows can have the same hash key, we need to compare

diff --git a/src/suricata.c b/src/suricata.c
index f970d82463aa064cddae5f120cdd9fe656e40a8d..49eadee5e52181fc60808da3a9651b9727a769a3 100644 (file)
--- a/src/suricata.c
+++ b/src/suricata.c
@@ -230,6 +230,10 @@ int g_disable_randomness = 0;
 int g_disable_randomness = 1;
 #endif
 
+/** determine (without branching) if we include the vlan_ids when hashing or
+  * comparing flows */
+uint16_t g_vlan_mask = 0xffff;
+
 /** Suricata instance */
 SCInstance suricata;
 
@@ -2967,6 +2971,12 @@ int main(int argc, char **argv)
         exit(EXIT_SUCCESS);
     }
 
+    int vlan_tracking = 1;
+    if (ConfGetBool("vlan.use-for-tracking", &vlan_tracking) == 1 && !vlan_tracking) {
+        /* Ignore vlan_ids when comparing flows. */
+        g_vlan_mask = 0x0000;
+    }
+
     SetupUserMode(&suricata);
 
     /* Since our config is now loaded we can finish configurating the

diff --git a/src/suricata.h b/src/suricata.h
index bbd2a3aafa8018707bf63c9de9e3873ee7badd62..1b0e72a99eb953dfb6702da04a2f6632cea8361b 100644 (file)
--- a/src/suricata.h
+++ b/src/suricata.h
@@ -174,6 +174,7 @@ void GlobalsInitPreConfig(void);
 
 extern volatile uint8_t suricata_ctl_flags;
 extern int g_disable_randomness;
+extern uint16_t g_vlan_mask;
 
 #include <ctype.h>
 #define u8_tolower(c) tolower((uint8_t)(c))