From: Victor Julien <vjulien@oisf.net>
Date: Fri, 24 May 2024 17:11:41 +0000 (+0200)
Subject: defrag: remove trackers on lookup
X-Git-Tag: suricata-8.0.0-beta1~1217
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1190e426f919d8319cac996b2f214f7930dc3cfc;p=thirdparty%2Fsuricata.git

defrag: remove trackers on lookup

When looking up a tracker, remove any timed out / completed trackers.
---

diff --git a/etc/schema.json b/etc/schema.json
index f72fd050f8..276d2c0e1c 100644
--- a/etc/schema.json
+++ b/etc/schema.json
@@ -5257,6 +5257,15 @@
                                 }
                             },
                             "additionalProperties": false
+                        },
+                        "wrk": {
+                            "type": "object",
+                            "properties": {
+                                "tracker_timeout": {
+                                    "type": "integer"
+                                }
+                            },
+                            "additionalProperties": false
                         }
                     },
                     "additionalProperties": false
diff --git a/src/decode.c b/src/decode.c
index e0deafabc9..4523442052 100644
--- a/src/decode.c
+++ b/src/decode.c
@@ -679,6 +679,7 @@ void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv)
     dtv->counter_defrag_no_frags = StatsRegisterCounter("defrag.max_frags_reached", tv);
     dtv->counter_defrag_tracker_soft_reuse = StatsRegisterCounter("defrag.tracker_soft_reuse", tv);
     dtv->counter_defrag_tracker_hard_reuse = StatsRegisterCounter("defrag.tracker_hard_reuse", tv);
+    dtv->counter_defrag_tracker_timeout = StatsRegisterCounter("defrag.wrk.tracker_timeout", tv);
 
     ExceptionPolicySetStatsCounters(tv, &dtv->counter_defrag_memcap_eps, &defrag_memcap_eps_stats,
             DefragGetMemcapExceptionPolicy(), "defrag.memcap_exception_policy.",
diff --git a/src/decode.h b/src/decode.h
index a2961f43b3..eafdf35f4d 100644
--- a/src/decode.h
+++ b/src/decode.h
@@ -1002,6 +1002,7 @@ typedef struct DecodeThreadVars_
     uint16_t counter_defrag_no_frags;
     uint16_t counter_defrag_tracker_soft_reuse;
     uint16_t counter_defrag_tracker_hard_reuse;
+    uint16_t counter_defrag_tracker_timeout;
     ExceptionPolicyCounters counter_defrag_memcap_eps;
 
     uint16_t counter_flow_memcap;
diff --git a/src/defrag-hash.c b/src/defrag-hash.c
index 2340826dab..b5b21f3778 100644
--- a/src/defrag-hash.c
+++ b/src/defrag-hash.c
@@ -555,27 +555,48 @@ DefragTracker *DefragGetTrackerFromHash(ThreadVars *tv, DecodeThreadVars *dtv, P
     }
 
     /* ok, we have a tracker in the bucket. Let's find out if it is our tracker */
+    DefragTracker *prev_dt = NULL;
     dt = hb->head;
 
     do {
+        DefragTracker *next_dt = NULL;
+
+        SCMutexLock(&dt->lock);
         if (DefragTrackerTimedOut(dt, p->ts)) {
-            dt->remove = 1;
+            next_dt = dt->hnext;
+            dt->hnext = NULL;
+            if (prev_dt) {
+                prev_dt->hnext = next_dt;
+            } else {
+                hb->head = next_dt;
+            }
+            DefragTrackerClearMemory(dt);
+            SCMutexUnlock(&dt->lock);
+
+            DefragTrackerMoveToSpare(dt);
+            StatsIncr(tv, dtv->counter_defrag_tracker_timeout);
+            goto tracker_removed;
         } else if (!dt->remove && DefragTrackerCompare(dt, p)) {
-            /* found our tracker, lock & return */
-            SCMutexLock(&dt->lock);
+            /* found our tracker, keep locked & return */
             (void)DefragTrackerIncrUsecnt(dt);
             DRLOCK_UNLOCK(hb);
             return dt;
         }
+        SCMutexUnlock(&dt->lock);
+        /* unless we removed 'dt', prev_dt needs to point to
+         * current 'dt' when adding a new tracker below. */
+        prev_dt = dt;
+        next_dt = dt->hnext;
 
-        if (dt->hnext == NULL) {
-            DefragTracker *prev = dt;
+    tracker_removed:
+        if (next_dt == NULL) {
             dt = DefragTrackerGetNew(tv, dtv, p);
             if (dt == NULL) {
                 DRLOCK_UNLOCK(hb);
                 return NULL;
             }
-            prev->hnext = dt;
+            dt->hnext = hb->head;
+            hb->head = dt;
 
             /* tracker is locked */
 
@@ -586,7 +607,7 @@ DefragTracker *DefragGetTrackerFromHash(ThreadVars *tv, DecodeThreadVars *dtv, P
             return dt;
         }
 
-        dt = dt->hnext;
+        dt = next_dt;
     } while (dt != NULL);
 
     /* should be unreachable */
diff --git a/src/defrag.c b/src/defrag.c
index 1d55d9a944..6d647d6cc5 100644
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -2571,7 +2571,10 @@ static int DefragTrackerReuseTest(void)
 
     tracker2 = DefragGetTracker(&tv, &dtv, p1);
     FAIL_IF_NULL(tracker2);
-    FAIL_IF(tracker2 == tracker1);
+    /* DefragGetTracker will have returned tracker1 to the stack,
+     * the set up a new tracker. Since it pops the stack, it got
+     * tracker1. */
+    FAIL_IF(tracker2 != tracker1);
     FAIL_IF(tracker2->remove);
 
     SCFree(p1);