-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IPS stats/v3 #9285
IPS stats/v3 #9285
Conversation
Since many implementations use the ReleasePacket callback to issue their verdict, no thread ctx is available. To work around this just register the stats in a `thread_local` variable instead.
ReleasePacket based verdicts can happen in several threads, depending on the runmode details. Only register and update if in IPS mode.
This adds support to all capture methods for these counters. Ticket: OISF#4756.
{ "accepted": 296185, "blocked": 162, "rejected": 0, "replaced": 0, "drop_reason": { "decode_error": 0, "defrag_error": 0, "defrag_memcap": 0, "flow_memcap": 0, "flow_drop": 94, "applayer_error": 0, "applayer_memcap": 0, "rules": 3, "threshold_detection_filter": 0, "stream_error": 63, "stream_memcap": 0, "stream_midstream": 2, "nfq_error": 0, "tunnel_packet_drop": 0 } } Ticket: OISF#6230.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #9285 +/- ##
==========================================
- Coverage 82.43% 82.40% -0.03%
==========================================
Files 968 968
Lines 273953 274004 +51
==========================================
- Hits 225840 225806 -34
- Misses 48113 48198 +85
Flags with carried forward coverage won't be shown. Click here to find out more. |
Information: QA ran without warnings. Pipeline 15384 |
@@ -403,6 +403,7 @@ enum PacketDropReason { | |||
PKT_DROP_REASON_STREAM_MIDSTREAM, | |||
PKT_DROP_REASON_NFQ_ERROR, /**< no nfq verdict, must be error */ | |||
PKT_DROP_REASON_INNER_PACKET, /**< drop issued by inner (tunnel) packet */ | |||
PKT_DROP_REASON_MAX, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the idea is to help users figure out what's happening, wouldn't it make sense to have a drop reason specifically for stream reassembly memcap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah agree, but that is outside of the scope of this PR. Can you do a ticket for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to have an SV test showcasing these new stats counters. I could take that up. |
Merged in #9288 |
https://redmine.openinfosecfoundation.org/issues/4756
https://redmine.openinfosecfoundation.org/issues/6230
replaces #9284, fixing counters updates