Alert queue optimizations - v3 #7297
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Initial work to bring the alert queue processing to DetectEngineThreadCtx. Task OISF#4943
Sort the PacketAlert queue that will be passed to the Packet handling functions, so there's less work to be done with it from there. Task OISF#4943
Do all alert queue processing before actually appending the PacketAlerts to the Packet's alert queue. Task OISF#4943
Since we now only copy the PacketAlerts to the Packet's queue after processing them, we no longer do packet alert appending from detect-engine-alert, nor do we remove PacketAlerts from the queue (if they're discarded by overflow or thresholding, they're not copied to the final alert queue). Task OISF#4943
The maximum of possible alerts triggered by a unique packet was hardcoded to 15. With usage of 'noalert' rules, that limit could be reached somewhat easily. Make that configurable via suricata.yaml. Conf Bug#4941 Task OISF#4207
Add a counter to our stats log with the total of alerts that have been discarded due to packet alert queue overflow. Task OISF#5179
Codecov Report
@@ Coverage Diff @@
## master #7297 +/- ##
==========================================
- Coverage 75.82% 75.77% -0.05%
==========================================
Files 656 656
Lines 190051 190091 +40
==========================================
- Hits 144102 144049 -53
- Misses 45949 46042 +93
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
QA on my end looks good. |
jlucovsky
reviewed
Apr 26, 2022
| det_ctx->alert_queue, (size_t)(sizeof(PacketAlert) * det_ctx->alert_queue_capacity)); | ||
| if (tmp_queue == NULL) { | ||
| SCLogWarning(SC_ERR_MEM_ALLOC, "failed to allocate %" PRIuMAX " bytes", | ||
| (uintmax_t)(PACKET_ALERT_MAX * sizeof(PacketAlert))); |
There was a problem hiding this comment.
Restore alert_queue_capcity on error return.
The size value in the message is kind of correct because you're trying to allocate an additional group of bytes of that size. However, the total allocation is sizeof(PacketAlert)* det_ctx->alert_queue_capacity
| { | ||
| if (det_ctx->alert_queue_size == det_ctx->alert_queue_capacity) { | ||
| /* we must grow the alert queue */ | ||
| AlertQueueExpand(det_ctx); |
There was a problem hiding this comment.
Need to handle AlertQueueExpand failures
| /** \internal | ||
| * \brief sort helper for sorting alerts by priority | ||
| * | ||
| * The id field is set from Signature::num, so we have higher priority (lower ids) first |
There was a problem hiding this comment.
This actually needed some rewording, thanks for making me re-read it :P
|
Note: please make sure all tickets in the commits are mentioned in the PR:
Please update PR body and remove the 'needs ticket' label when done. |
|
Followed by #7314 |
Nancyenos
added a commit
to Nancyenos/suricata
that referenced
this pull request
Oct 19, 2024
2 tasks
Nancyenos
added a commit
to Nancyenos/suricata
that referenced
this pull request
Oct 21, 2024
2 tasks
Nancyenos
added a commit
to Nancyenos/suricata
that referenced
this pull request
Oct 22, 2024
Nancyenos
added a commit
to Nancyenos/suricata
that referenced
this pull request
Oct 23, 2024
2 tasks
Nancyenos
added a commit
to Nancyenos/suricata
that referenced
this pull request
Oct 24, 2024
2 tasks
victorjulien
pushed a commit
to victorjulien/suricata
that referenced
this pull request
Oct 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previous PR: #7290
Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/4943
Describe changes:
suricata-verify-pr: 808
OISF/suricata-verify#808