From: Victor Julien <victor@inliniac.net>
Date: Fri, 24 Jan 2014 17:09:46 +0000 (+0100)
Subject: flow-timeout: change error logic
X-Git-Tag: suricata-2.0rc1~159
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F795%2Fhead;p=thirdparty%2Fsuricata.git

flow-timeout: change error logic

If FlowForceReassemblyForFlowV2 can't get packets to inject into the
engine, until now it would bail and retry later. In case of resource
starvation issues, this would cause a lot of lock contention, as the
flow manager would try over and over again.

This patch limits FlowForceReassemblyForFlowV2 to one try per flow,
if it fails... bad luck. It will only fail in serious conditions,
which means we must prefer the health of the engine over the proper
inspection of the flow in question.
---

diff --git a/src/flow-timeout.c b/src/flow-timeout.c
index 78f173b452..16b66fa55c 100644
--- a/src/flow-timeout.c
+++ b/src/flow-timeout.c
@@ -388,7 +388,7 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
     if (client == STREAM_HAS_UNPROCESSED_SEGMENTS_NEED_REASSEMBLY) {
         p1 = FlowForceReassemblyPseudoPacketGet(1, f, ssn, 0);
         if (p1 == NULL) {
-            return 1;
+            goto done;
         }
         PKT_SET_SRC(p1, PKT_SRC_FFR_V2);
 
@@ -397,7 +397,7 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
             if (p2 == NULL) {
                 FlowDeReference(&p1->flow);
                 TmqhOutputPacketpool(NULL, p1);
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p2, PKT_SRC_FFR_V2);
 
@@ -407,7 +407,7 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
                 TmqhOutputPacketpool(NULL, p1);
                 FlowDeReference(&p2->flow);
                 TmqhOutputPacketpool(NULL, p2);
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p3, PKT_SRC_FFR_V2);
         } else {
@@ -415,7 +415,7 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
             if (p2 == NULL) {
                 FlowDeReference(&p1->flow);
                 TmqhOutputPacketpool(NULL, p1);
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p2, PKT_SRC_FFR_V2);
         }
@@ -424,7 +424,7 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
         if (server == STREAM_HAS_UNPROCESSED_SEGMENTS_NEED_REASSEMBLY) {
             p1 = FlowForceReassemblyPseudoPacketGet(0, f, ssn, 0);
             if (p1 == NULL) {
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p1, PKT_SRC_FFR_V2);
 
@@ -432,13 +432,13 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
             if (p2 == NULL) {
                 FlowDeReference(&p1->flow);
                 TmqhOutputPacketpool(NULL, p1);
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p2, PKT_SRC_FFR_V2);
         } else {
             p1 = FlowForceReassemblyPseudoPacketGet(0, f, ssn, 1);
             if (p1 == NULL) {
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p1, PKT_SRC_FFR_V2);
 
@@ -447,7 +447,7 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
                 if (p2 == NULL) {
                     FlowDeReference(&p1->flow);
                     TmqhOutputPacketpool(NULL, p1);
-                    return 1;
+                    goto done;
                 }
                 PKT_SET_SRC(p2, PKT_SRC_FFR_V2);
             }
@@ -457,7 +457,7 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
         if (server == STREAM_HAS_UNPROCESSED_SEGMENTS_NEED_REASSEMBLY) {
             p1 = FlowForceReassemblyPseudoPacketGet(0, f, ssn, 0);
             if (p1 == NULL) {
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p1, PKT_SRC_FFR_V2);
 
@@ -465,13 +465,13 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
             if (p2 == NULL) {
                 FlowDeReference(&p1->flow);
                 TmqhOutputPacketpool(NULL, p1);
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p2, PKT_SRC_FFR_V2);
         } else if (server == STREAM_HAS_UNPROCESSED_SEGMENTS_NEED_ONLY_DETECTION) {
             p1 = FlowForceReassemblyPseudoPacketGet(1, f, ssn, 1);
             if (p1 == NULL) {
-                return 1;
+                goto done;
             }
             PKT_SET_SRC(p1, PKT_SRC_FFR_V2);
         } else {
@@ -480,8 +480,6 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
         }
     }
 
-    f->flags |= FLOW_TIMEOUT_REASSEMBLY_DONE;
-
     SCMutexLock(&stream_pseudo_pkt_decode_tm_slot->slot_post_pq.mutex_q);
     PacketEnqueue(&stream_pseudo_pkt_decode_tm_slot->slot_post_pq, p1);
     if (p2 != NULL)
@@ -493,6 +491,10 @@ int FlowForceReassemblyForFlowV2(Flow *f, int server, int client)
         SCCondSignal(&trans_q[stream_pseudo_pkt_decode_TV->inq->id].cond_q);
     }
 
+    /* done, in case of error (no packet) we still tag flow as complete
+     * as we're probably resource stress if we couldn't get packets */
+done:
+    f->flags |= FLOW_TIMEOUT_REASSEMBLY_DONE;
     return 1;
 }