Mark flow elephant/v4 #12325
Mark flow elephant/v4 #12325
Conversation
Add a parser for different units of time. Acceptable values for defining time are: us: microseconds ms: milliseconds s: seconds m: minutes h: hours This is in order to have a more fine grained control over time which may be required for certain keywords and operations.
1. Add user defined elephant flow definition based on rate of bytes easily configurable in suricata.yaml. 2. Add an elephant flow counter. Feature 5647
This brings us down from 2 holes of 2 bytes and 4 bytes to one hole of 5 bytes. There may be more room for improvement.
| if (ef_interval.usecs > 0) { | ||
| ef_bytes *= 1000000; | ||
| } | ||
| flow_config.elephant_flow_rate = |
There was a problem hiding this comment.
Thinking this value should be floating point for a more accurate calculation
There was a problem hiding this comment.
I think you should avoid floats if possible
if flow bytes > conf bytes && flow age < conf interval should be good for now, right ?
|
|
||
| if (ef_interval.usecs > 0) { | ||
| ef_bytes *= 1000000; | ||
| } |
There was a problem hiding this comment.
else if secs == 0 && usecs ==0, FatalError
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12325 +/- ##
==========================================
- Coverage 83.26% 83.19% -0.07%
==========================================
Files 912 912
Lines 257643 257844 +201
==========================================
- Hits 214521 214520 -1
- Misses 43122 43324 +202
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 24057 |
| return; /* Flow is a known elephant flow, no need to calculate again */ | ||
| uint32_t age = SCTIME_SECS(f->lastts) - SCTIME_SECS(f->startts); | ||
| if (age == 0) | ||
| return; |
There was a problem hiding this comment.
should we use milliseconds, not just seconds ?
There was a problem hiding this comment.
micro actually as that's the smallest supported unit.
| # the time a flow has been alive. | ||
| #elephant-flow-rate: | ||
| # bytes: 10KiB | ||
| # interval: 10ms # allowed units: us, ms, s, m, h |
There was a problem hiding this comment.
I understand this parameter should be used in FlowIsElephant
as if flow bytes > conf bytes && flow age < conf interval then flow is elephant
There was a problem hiding this comment.
and that is better than calculating the rate and matching against it?
Well, it does save some floating point calculations indeed. ok. I'll take this route instead. Thank you! 🙇🏽♀️
There was a problem hiding this comment.
JFYI, a rate param was calculated as of now when reading these values and that rate was matched against the flow.bytes/flow.age result.
There was a problem hiding this comment.
JFYI, a rate param was calculated as of now when reading these values and that rate was matched against the flow.bytes/flow.age result.
Indeed, I saw that.
But I think it is better to keep the 2 params coming from config instead of merging them in a single floating point value
Link to ticket: https://redmine.openinfosecfoundation.org/issues/5647
Previous PR: #12237
Changes since v3:
TODO: Tests