Backports port grouping/v2 #10612
Closed
Backports port grouping/v2 #10612
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
Ticket 6792 Bug 6414 (cherry picked from commit fde4ca5)
Ticket 6792 Bug 6414 (cherry picked from commit 30b6e4d)
An interval tree uses red-black tree as its base data structure and follows all the properties of a usual red-black tree. The additional params are: 1. An interval such as [low, high] per node. 2. A max attribute per node. This attribute stores the maximum high value of any subtree rooted at this node. At any point in time, an inorder traversal of an interval tree should give the port ranges sorted by the low key in ascending order. This commit modifies the IRB_AUGMENT macro and it's call sites to make sure that on every insertion, the max attribute of the tree is properly updated. Ticket 6792 Bug 6414 (cherry picked from commit d36d03a)
as this fn will be called upon and further used by other files later on. Ticket 6792 Bug 6414
Add new utility files to deal with the interval trees. These cover the basic ops: 1. Creation/Destruction of the tree 2. Creation/Destruction of the nodes It also adds the support for finding overlaps for a given set of ports. This function is used by the detection engine is the Stage 2 of signature preparation. Ticket 6792 Bug 6414 Co-authored-by: Victor Julien <vjulien@oisf.net> (cherry picked from commit 54558f1)
Warning was: src/util-port-interval-tree.c:50:1: warning: Either the condition 'tmp!=NULL' is redundant or there is possible null pointer dereference: tmp. [nullPointerRedundantCheck] IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: note: Assuming that condition 'tmp!=NULL' is not redundant IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: note: Null pointer dereference IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: warning: Either the condition 'oleft!=NULL' is redundant or there is possible null pointer dereference: oleft. [nullPointerRedundantCheck] IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: note: Assuming that condition 'oleft!=NULL' is not redundant IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: note: Null pointer dereference IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: warning: Either the condition 'oright!=NULL' is redundant or there is possible null pointer dereference: oright. [nullPointerRedundantCheck] IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: note: Assuming that condition 'oright!=NULL' is not redundant IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: note: Null pointer dereference IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: warning: Either the condition 'left!=NULL' is redundant or there is possible null pointer dereference: left. [nullPointerRedundantCheck] IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: note: Assuming that condition 'left!=NULL' is not redundant IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ src/util-port-interval-tree.c:50:1: note: Null pointer dereference IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate); ^ (cherry picked from commit 86f89e0)
In order to create the smallest possible port ranges, it is convenient to first have a list of unique ports. Then, the work becomes simple. See below: Given, a port range P1 = [1, 8]; SGH1 and another, P2 = [3, 94]; SGH2 right now, the code will follow a logic of recursively cutting port ranges until we create the small ranges. But, with the help of unique port points, we get, unique_port_points = [1, 3, 8, 94] So, now, in a later stage, we can create the ranges as [1, 2], [3, 7], [8, 8], [9, 94] and copy the designated SGHs where they belong. Note that the intervals are closed which means that the range is inclusive of both the points. The final result becomes: 1. [1, 2]; SGH1 2. [3, 7]; SGH1 + SGH2 3. [8, 8]; SGH1 + SGH2 4. [9, 94]; SGH2 There would be 3 unique rule groups made for the case above. Group 1: [1, 2] Group 2: [3, 7], [8, 8] Group 3: [9, 94] Ticket 6792 Bug 6414 (cherry picked from commit c9a911b)
After all the SGHs have been appropriately copied to the designated ports, create an interval tree out of it for a faster lookup when later a search for overlaps is made. Ticket 6792 Bug 6414 (cherry picked from commit a02c44a)
Using the unique port points, create a list of small port ranges which contain the DetectPort objects and the designated SGHs found by finding the overlaps with the existing ports and copying the SGHs accordingly. Ticket 6792 Bug 6414 (cherry picked from commit 4ac2382)
As this is already taken care of and a list of ports is available for use by the next stage. Ticket 6792 Bug 6414 (cherry picked from commit 83aba93)
To avoid getting multiple entries in the final port list and to also make the next step more efficient by reducing the size of the items to traverse over. Ticket 6792 Bug 6414 (cherry picked from commit 643ae85)
Make rule group head bitarray 16 bytes aligned and padded to 16 bytes boundaries to assist SIMD operations in follow up commits. (cherry picked from commit 4ba1f44)
During startup large rulesets use a lot of large bitarrays, that are frequently merged (OR'd). Optimize this using SSE2 _mm_or_si128. (cherry picked from commit 94b4619)
Utilize _popcnt64 where available. (cherry picked from commit c4ac6cd)
Fix Coverity warning
** CID 1592992: Incorrect expression (COPY_PASTE_ERROR)
/src/util-port-interval-tree.c: 255 in SCPortIntervalFindOverlaps()
________________________________________________________________________________________________________
*** CID 1592992: Incorrect expression (COPY_PASTE_ERROR)
/src/util-port-interval-tree.c: 255 in SCPortIntervalFindOverlaps()
249 * will be sorted, insert any new ports to the end of the list
250 * and avoid walking the entire list */
251 if (*list == NULL) {
252 *list = new_port;
253 (*list)->last = new_port;
254 } else if (((*list)->last->port != new_port->port) &&
>>> CID 1592992: Incorrect expression (COPY_PASTE_ERROR)
>>> "port" in "(*list)->last->port2 != new_port->port" looks like a copy-paste error.
255 ((*list)->last->port2 != new_port->port)) {
256 DEBUG_VALIDATE_BUG_ON(new_port->port < (*list)->last->port);
257 (*list)->last->next = new_port;
258 new_port->prev = (*list)->last;
259 (*list)->last = new_port;
260 } else {
The code does not generate two port ranges that are same other than the
cases where port == port2 which is why it worked so far. Fix it.
Bug 6839
(cherry picked from commit 2d6708f)
If a port point is single but later on also a part of a range, it ends up only creating the port groups for single points and not the range. Fix it by adding the port next to current single one to unique points and marking it a range port. Bug 6843 (cherry picked from commit 632ca75)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main-7.0.x #10612 +/- ##
==============================================
+ Coverage 82.47% 82.49% +0.02%
==============================================
Files 976 978 +2
Lines 275050 275638 +588
==============================================
+ Hits 226835 227392 +557
- Misses 48215 48246 +31
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 19191 |
|
Why is this a draft ? |
We're not merging this before the upcoming release so to avoid confusion..
No idea. Q: What do you think should be done? Apply the |
|
Is this the clang-format version ? |
I checked that locally the clang-format that fails for me is also v14 and that git clang-format is also using v14 |
|
Looks like CI is still using clang-format-9 for main-7.0.x cf commit 9307150 |
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.
Link to redmine tickets:
SV_BRANCH=OISF/suricata-verify#1696
Previous PR: #10581
Changes since v1: