(7.0.x) Backport: Port grouping/v1 #10581
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
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)
inashivb
commented
Mar 5, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main-7.0.x #10581 +/- ##
==============================================
+ Coverage 82.43% 82.48% +0.04%
==============================================
Files 976 978 +2
Lines 275046 275771 +725
==============================================
+ Hits 226726 227457 +731
+ Misses 48320 48314 -6
Flags with carried forward coverage won't be shown. Click here to find out more. |
inashivb
force-pushed
the
backports-port-grouping/v1
branch
from
c6b97c9
to
f363c12
Compare
|
Q1: yes |
|
Information: QA ran without warnings. Pipeline 18979 |
inashivb
changed the title
|
Information: QA ran without warnings. Pipeline 18983 |
|
I've set this to draft due to unclean history, as well as it needing the latest fixes from master. When we're ready for review, please open a new PR. |
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#1685
Note:
mastersuch that they'll be easy to backport and they were for the most part except that given that cleanups todetect-*are missing in7.0.xand6.0.xmaking it more effort than low to do the backports. I was just trying out what backporting would look like.