Capture bypass stats gather v3#15219
Conversation
This commit forces timeout check of all flows in the flow table at the shutdown stage of Suricata. Gathering of capture-bypassed flow statistics was left to the bypass capture method via BypassUpdate callback. Until now, capture-bypassed flows that did not timeout had their statistics unchecked in the period between last check and shutdown. This commit forces gathering of statistics from these flows. Ticket: 8440
Forbid worker to timeout capture-bypassed flows, as it does not have the necessary components (BypassUpdate callback) to gather the necessary statistics from the flows to make this decisions. Leave the processing of bypass-capture flows only to FlowManager. Ticket: 8442
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15219 +/- ##
==========================================
- Coverage 82.68% 82.65% -0.04%
==========================================
Files 993 993
Lines 271880 271899 +19
==========================================
- Hits 224807 224733 -74
- Misses 47073 47166 +93
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
lukashino
left a comment
lukashino
left a comment
There was a problem hiding this comment.
I approve the changes.
SV test is an extra verification -- not required to be merged. Generally, I would be inclined to accept Phillipe's work (OISF/suricata-verify#2969) into SV tests and later on port these SV tests -- it would save some test time (shuts down immediately, after all packets were transmitted and are not waiting, e.g. 5 seconds).
| * flows for new flows and/or it's memcap limit it reached. In this state the | ||
| * flow engine with evaluate flows with lower timeout settings. */ | ||
| #define FLOW_EMERGENCY 0x01 | ||
| #define FLOW_SHUTDOWN 0x02 |
There was a problem hiding this comment.
I think this needs a comment explaining what it is used for.
There was a problem hiding this comment.
Okay, I will add a description containing how it affects FlowManager in the shutdown stage.
| FLOWLOCK_WRLOCK(f); | ||
| const bool timedout = (timeout_check && FlowIsTimedOut(tv_id, f, p->ts, emerg)); | ||
| bool timedout = (timeout_check && FlowIsTimedOut(tv_id, f, p->ts, emerg)); | ||
| #ifdef CAPTURE_OFFLOAD |
There was a problem hiding this comment.
can this move into FlowIsTimedOut?
|
Continues in #15289 |
3 participants
Changes
Links to tickets: 8440, 8442
Previous PR: #15189
SV_BRANCH=OISF/suricata-verify#3027