From ef40fe1f31a0e7644ffe9f0154df07ff027f37f8 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Jan 2014 18:09:46 +0100 Subject: [PATCH] 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. --- src/flow-timeout.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) 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; }