libsuricata: add rate_filter callback - v1 #13086
Conversation
Use the BIT_U8 macros for packet alert flags and rename PACKET_ALERT_RATE_FILTER_MODIFIED to PACKET_ALERT_FLAG_RATE_FILTER_MODIFIED for consistency.
src/decode.h
Outdated
| /** user private flag 0 */ | ||
| PACKET_ALERT_FLAG_USER0 = BIT_U8(12), | ||
|
|
||
| /** user private flag 1 */ | ||
| PACKET_ALERT_FLAG_USER1 = BIT_U8(13), | ||
|
|
||
| /** user private flag 2 */ | ||
| PACKET_ALERT_FLAG_USER2 = BIT_U8(14), | ||
|
|
||
| /** user private flag 3 */ | ||
| PACKET_ALERT_FLAG_USER3 = BIT_U8(15), |
There was a problem hiding this comment.
In the POC shown to me, based on an older version of Suricata there were still some bits free user for the user to add. The idea here is to reserve some specifically for users (library users in this scope).
| uint8_t flags; | ||
| uint16_t flags; |
There was a problem hiding this comment.
There was a hole of 3 bytes here according to pahole, now there is a 1 byte hole.
src/detect.h
Outdated
| /* user provided rate filter callbacks. */ | ||
| SCDetectRateFilterFunc rate_filter_callback; | ||
|
|
||
| /* use provided data to be passed to rate_filter_callback. */ | ||
| void *rate_filter_callback_arg; |
There was a problem hiding this comment.
Just allow one callback for now, I'm not sure if a list of callbacks really makes sense here.
There was a problem hiding this comment.
Yes, probably even in non-breaking ways if DetectEngineCtx is treated as an opaque data structure. And of course a library user could just fan it out as needed.
|
Ping @amirabell. |
|
Some other thoughts:
But if there is some other logic/lookups that are not built into Suricata, then a callback makes sense. |
jasonish
force-pushed
the
ratefilter-callback/v1
branch
from
c550e9e to
87a8739
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13086 +/- ##
==========================================
- Coverage 82.78% 82.61% -0.17%
==========================================
Files 985 985
Lines 272547 272554 +7
==========================================
- Hits 225617 225178 -439
- Misses 46930 47376 +446
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
jasonish
force-pushed
the
ratefilter-callback/v1
branch
from
87a8739 to
f4880a8
Compare
|
Information: QA ran without warnings. Pipeline 25767 |
|
Information: QA ran without warnings. Pipeline 25773 |
Expand to a uint16_t as there is a library use case to have some user private flags, so reserve the last 4 flags for library use. We were already at the max for a u8, so this opens up 4 more for our use. As #define's like this can't be logically grouped into an enum, try Doxygen documentation groups, which create a group just of these flag values and documents them together on a page.
As the callback is added to the current detection engine, make sure its copied to the new detection engine on reload. Ticket: OISF#7673
jasonish
force-pushed
the
ratefilter-callback/v1
branch
from
f4880a8 to
6afc16d
Compare
|
Information: QA ran without warnings. Pipeline 25776 |
|
Replaced by #13121 |
Mainly to start some conversation around this requested feature.
From ticket https://redmine.openinfosecfoundation.org/issues/7673:
More comments in-line.