1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
|
From 3c53a1601b6f861f8b7f0cd0984b18e78291fe85 Mon Sep 17 00:00:00 2001
From: Victor Julien <victor@inliniac.net>
Date: Wed, 18 Aug 2021 20:14:48 +0200
Subject: [PATCH] threading: don't pass locked flow between threads
Previously the flow manager would share evicted flows with the workers
while keeping the flows mutex locked. This reduced the number of unlock/
lock cycles while there was guaranteed to be no contention.
This turns out to be undefined behavior. A lock is supposed to be locked
and unlocked from the same thread. It appears that FreeBSD is stricter on
this than Linux.
This patch addresses the issue by unlocking before handing a flow off
to another thread, and locking again from the new thread.
Issue was reported and largely analyzed by Bill Meeks.
Bug: #4478
(cherry picked from commit 9551cd05357925e8bec8e0030d5f98fd07f17839)
---
src/flow-hash.c | 1 +
src/flow-manager.c | 2 +-
src/flow-timeout.c | 1 +
src/flow-worker.c | 1 +
4 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/flow-hash.c b/src/flow-hash.c
index ebbd836e81a..760bc53e0a8 100644
--- src/flow-hash.c
+++ src/flow-hash.c
@@ -669,6 +669,7 @@ static inline void MoveToWorkQueue(ThreadVars *tv, FlowLookupStruct *fls,
f->fb = NULL;
f->next = NULL;
FlowQueuePrivateAppendFlow(&fls->work_queue, f);
+ FLOWLOCK_UNLOCK(f);
} else {
/* implied: TCP but our thread does not own it. So set it
* aside for the Flow Manager to pick it up. */
diff --git a/src/flow-manager.c b/src/flow-manager.c
index d58a49637d6..9228c88490c 100644
--- src/flow-manager.c
+++ src/flow-manager.c
@@ -333,9 +333,9 @@ static uint32_t ProcessAsideQueue(FlowManagerTimeoutThread *td, FlowTimeoutCount
FlowForceReassemblyNeedReassembly(f) == 1)
{
FlowForceReassemblyForFlow(f);
+ FLOWLOCK_UNLOCK(f);
/* flow ownership is passed to the worker thread */
- /* flow remains locked */
counters->flows_aside_needs_work++;
continue;
}
diff --git a/src/flow-timeout.c b/src/flow-timeout.c
index 972b35076bd..d6cca490087 100644
--- src/flow-timeout.c
+++ src/flow-timeout.c
@@ -401,6 +401,7 @@ static inline void FlowForceReassemblyForHash(void)
RemoveFromHash(f, prev_f);
f->flow_end_flags |= FLOW_END_FLAG_SHUTDOWN;
FlowForceReassemblyForFlow(f);
+ FLOWLOCK_UNLOCK(f);
f = next_f;
continue;
}
diff --git a/src/flow-worker.c b/src/flow-worker.c
index 69dbb6ac575..dccf3581dd5 100644
--- src/flow-worker.c
+++ src/flow-worker.c
@@ -168,6 +168,7 @@ static void CheckWorkQueue(ThreadVars *tv, FlowWorkerThreadData *fw,
{
Flow *f;
while ((f = FlowQueuePrivateGetFromTop(fq)) != NULL) {
+ FLOWLOCK_WRLOCK(f);
f->flow_end_flags |= FLOW_END_FLAG_TIMEOUT; //TODO emerg
const FlowStateType state = f->flow_state;
|
