From: Victor Julien <victor@inliniac.net>
Date: Sat, 24 Jan 2015 12:46:25 +0000 (+0100)
Subject: flow: modify lastts update logic
X-Git-Tag: suricata-2.1beta4~220
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5587372ce15cf3ead98867e1daf3dd010529b4ad;p=thirdparty%2Fsuricata.git

flow: modify lastts update logic

In the lastts timeval struct field in the flow the timestamp of the
last packet to update is recorded. This allows for tracking the timeout
of the flow. So far, this value was updated under the flow lock and also
read under the flow lock.

This patch moves the updating of this field to the FlowGetFlowFromHash
function, where it updated at the point where both the Flow and the
Flow Hash Row lock are held. This guarantees that the field is only
updated when both locks are held.

This makes reading the field safe when either lock is held, which is the
purpose of this patch.

The flow manager, while holding the flow hash row lock, can now safely
read the lastts value. This allows it to do the flow timeout check
without actually locking the flow.
---

diff --git a/src/flow-hash.c b/src/flow-hash.c
index 03e1df6b70..a2ce5708c9 100644
--- a/src/flow-hash.c
+++ b/src/flow-hash.c
@@ -529,6 +529,9 @@ Flow *FlowGetFlowFromHash(ThreadVars *tv, DecodeThreadVars *dtv, const Packet *p
         FlowInit(f, p);
         f->fb = fb;
 
+        /* update the last seen timestamp of this flow */
+        COPY_TIMESTAMP(&p->ts,&f->lastts);
+
         FBLOCK_UNLOCK(fb);
         FlowHashCountUpdate;
         return f;
@@ -564,6 +567,9 @@ Flow *FlowGetFlowFromHash(ThreadVars *tv, DecodeThreadVars *dtv, const Packet *p
                 FlowInit(f, p);
                 f->fb = fb;
 
+                /* update the last seen timestamp of this flow */
+                COPY_TIMESTAMP(&p->ts,&f->lastts);
+
                 FBLOCK_UNLOCK(fb);
                 FlowHashCountUpdate;
                 return f;
@@ -589,6 +595,9 @@ Flow *FlowGetFlowFromHash(ThreadVars *tv, DecodeThreadVars *dtv, const Packet *p
 
                 /* found our flow, lock & return */
                 FLOWLOCK_WRLOCK(f);
+                /* update the last seen timestamp of this flow */
+                COPY_TIMESTAMP(&p->ts,&f->lastts);
+
                 FBLOCK_UNLOCK(fb);
                 FlowHashCountUpdate;
                 return f;
@@ -598,6 +607,9 @@ Flow *FlowGetFlowFromHash(ThreadVars *tv, DecodeThreadVars *dtv, const Packet *p
 
     /* lock & return */
     FLOWLOCK_WRLOCK(f);
+    /* update the last seen timestamp of this flow */
+    COPY_TIMESTAMP(&p->ts,&f->lastts);
+
     FBLOCK_UNLOCK(fb);
     FlowHashCountUpdate;
     return f;
diff --git a/src/flow.c b/src/flow.c
index 2546e5289e..018e1fe648 100644
--- a/src/flow.c
+++ b/src/flow.c
@@ -246,9 +246,6 @@ void FlowHandlePacket(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p)
     /* Point the Packet at the Flow */
     FlowReference(&p->flow, f);
 
-    /* update the last seen timestamp of this flow */
-    COPY_TIMESTAMP(&p->ts,&f->lastts);
-
     /* update flags and counters */
     if (FlowGetPacketDirection(f, p) == TOSERVER) {
         if (FlowUpdateSeenFlag(p)) {
diff --git a/src/flow.h b/src/flow.h
index 6f95401aa0..fc1ea3d8ef 100644
--- a/src/flow.h
+++ b/src/flow.h
@@ -337,7 +337,9 @@ typedef struct Flow_
 
     uint32_t flags;
 
-    /* time stamp of last update (last packet) */
+    /* time stamp of last update (last packet). Set/updated under the
+     * flow and flow hash row locks, safe to read under either the
+     * flow lock or flow hash row lock. */
     struct timeval lastts;
 
 #ifdef FLOWLOCK_RWLOCK