From 6e473ace46bbc19d8cda292491b051bd5d9cc436 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Thu, 13 Jun 2024 19:15:54 +0530 Subject: [PATCH 1/2] flow/manager: add fn docs (cherry picked from commit f97b4ec1e80cbf1fb18c6ca34dddbf1b43b03cbb) --- src/flow-manager.c | 54 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/src/flow-manager.c b/src/flow-manager.c index e5e1aa270276..725361204781 100644 --- a/src/flow-manager.c +++ b/src/flow-manager.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2023 Open Information Security Foundation +/* Copyright (C) 2007-2024 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -259,10 +259,23 @@ static inline int FlowBypassedTimeout(Flow *f, SCTime_t ts, FlowTimeoutCounters typedef struct FlowManagerTimeoutThread { /* used to temporarily store flows that have timed out and are - * removed from the hash */ + * removed from the hash to reduce locking contention */ FlowQueuePrivate aside_queue; } FlowManagerTimeoutThread; +/** + * \internal + * + * \brief Process the temporary Aside Queue + * This means that as long as a flow f is not waiting on detection + * engine to finish dealing with it, f will be put in the recycle + * queue for further processing later on. + * + * \param td FM Timeout Thread instance + * \param counters Flow Timeout counters to be updated + * + * \retval Number of flows that were recycled + */ static uint32_t ProcessAsideQueue(FlowManagerTimeoutThread *td, FlowTimeoutCounters *counters) { FlowQueuePrivate recycle = { NULL, NULL, 0 }; @@ -279,7 +292,7 @@ static uint32_t ProcessAsideQueue(FlowManagerTimeoutThread *td, FlowTimeoutCount /* Send the flow to its thread */ FlowForceReassemblyForFlow(f); FLOWLOCK_UNLOCK(f); - /* flow ownership is passed to the worker thread */ + /* flow ownership is already passed to the worker thread */ counters->flows_aside_needs_work++; continue; @@ -364,6 +377,16 @@ static void FlowManagerHashRowTimeout(FlowManagerTimeoutThread *td, Flow *f, SCT counters->rows_maxlen = checked; } +/** + * \internal + * + * \brief Clear evicted list from Flow Manager. + * All the evicted flows are removed from the Flow bucket and added + * to the temporary Aside Queue. + * + * \param td FM timeout thread instance + * \param f head of the evicted list + */ static void FlowManagerHashRowClearEvictedList( FlowManagerTimeoutThread *td, Flow *f, SCTime_t ts, FlowTimeoutCounters *counters) { @@ -440,6 +463,7 @@ static uint32_t FlowTimeoutHash(FlowManagerTimeoutThread *td, SCTime_t ts, const SC_ATOMIC_SET(fb->next_ts, next_ts); } if (fb->evicted == NULL && fb->head == NULL) { + /* row is empty */ SC_ATOMIC_SET(fb->next_ts, UINT_MAX); } } else { @@ -473,8 +497,19 @@ static uint32_t FlowTimeoutHash(FlowManagerTimeoutThread *td, SCTime_t ts, const } /** \internal + * * \brief handle timeout for a slice of hash rows - * If we wrap around we call FlowTimeoutHash twice */ + * If we wrap around we call FlowTimeoutHash twice + * \param td FM timeout thread + * \param ts timeout in seconds + * \param hash_min lower bound of the row slice + * \param hash_max upper bound of the row slice + * \param counters Flow timeout counters to be passed + * \param rows number of rows for this worker unit + * \param pos position of the beginning of row slice in the hash table + * + * \retval number of successfully timed out flows + */ static uint32_t FlowTimeoutHashInChunks(FlowManagerTimeoutThread *td, SCTime_t ts, const uint32_t hash_min, const uint32_t hash_max, FlowTimeoutCounters *counters, const uint32_t rows, uint32_t *pos) @@ -509,8 +544,10 @@ static uint32_t FlowTimeoutHashInChunks(FlowManagerTimeoutThread *td, SCTime_t t * \brief move all flows out of a hash row * * \param f last flow in the hash row + * \param recycle_q Flow recycle queue + * \param mode emergency or not * - * \retval cnt removed out flows + * \retval cnt number of flows removed from the hash and added to the recycle queue */ static uint32_t FlowManagerHashRowCleanup(Flow *f, FlowQueuePrivate *recycle_q, const int mode) { @@ -716,6 +753,13 @@ static TmEcode FlowManagerThreadDeinit(ThreadVars *t, void *data) * a rapid increase of the busy score, which could lead to the flow manager * suddenly scanning a much larger slice of the hash leading to a burst * in scan/eviction work. + * + * \param rows number of rows for the work unit + * \param mp current memcap pressure value + * \param emergency emergency mode is set or not + * \param wu_sleep holds value of sleep time per worker unit + * \param wu_rows holds value of calculated rows to be processed per second + * \param rows_sec same as wu_rows, only used for counter updates */ static void GetWorkUnitSizing(const uint32_t rows, const uint32_t mp, const bool emergency, uint64_t *wu_sleep, uint32_t *wu_rows, uint32_t *rows_sec) From 48916b1f2e5ea54d4f02618ad74998550d7d1e50 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 3 Dec 2024 11:36:27 +0100 Subject: [PATCH 2/2] flow/manager: fix multi instance row tracking In multi instance flow manager setups, each flow manager gets a slice of the hash table to manage. Due to a logic error in the chunked scanning of the hash slice, instances beyond the first would always rescan the same (first) subslice of their slice. The `pos` variable that is used to keep the state of what the starting position for the next scan was supposed to be, was treated as if it held a relative value. Relative to the bounds of the slice. It was however, holding an absolute position. This meant that when doing it's bounds check it was always considered out of bounds. This would reset the sub- slice to be scanned to the first part of the instances slice. This patch addresses the issue by correctly handling the fact that the value is absolute. Bug: #7365. Fixes: e9d2417e0ff3 ("flow/manager: adaptive hash eviction timing") (cherry picked from commit ae072d5c07e0f5e02a8583127c7b3edd97f93d54) --- src/flow-manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/flow-manager.c b/src/flow-manager.c index 725361204781..f58bcf41ff25 100644 --- a/src/flow-manager.c +++ b/src/flow-manager.c @@ -506,7 +506,7 @@ static uint32_t FlowTimeoutHash(FlowManagerTimeoutThread *td, SCTime_t ts, const * \param hash_max upper bound of the row slice * \param counters Flow timeout counters to be passed * \param rows number of rows for this worker unit - * \param pos position of the beginning of row slice in the hash table + * \param pos absolute position of the beginning of row slice in the hash table * * \retval number of successfully timed out flows */ @@ -520,7 +520,7 @@ static uint32_t FlowTimeoutHashInChunks(FlowManagerTimeoutThread *td, SCTime_t t uint32_t rows_left = rows; again: - start = hash_min + (*pos); + start = (*pos); if (start >= hash_max) { start = hash_min; } @@ -800,7 +800,7 @@ static TmEcode FlowManager(ThreadVars *th_v, void *thread_data) uint32_t emerg_over_cnt = 0; uint64_t next_run_ms = 0; - uint32_t pos = 0; + uint32_t pos = ftd->min; uint32_t rows_sec = 0; uint32_t rows_per_wu = 0; uint64_t sleep_per_wu = 0;