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
Optimization/defrag rbtree/v3 #3475
Closed
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
To improve worst case performance turn the segments list into a rbtree. This greatly improves inserts, lookups and removals if the number of segments gets very large. The tree is sorted by the segment sequence number as its primary key. If 2 segments have the same seq, the payload_len (segment length) is used. Then the larger segment will be places after the smaller segment. Exact matches are not added to the tree.
Now that with the RBTREE we have a properly sorted Segment tree, where with exact SEQ matches the tree is sorted by payload_len smallest to largest, we can avoid walking backwards when checking for overlaps. Our direct RB_PREV either overlaps or not and that is a reliable verdict for the rest of the tree.
Don't try to do a 'fast path' by checking RB_MAX. RB_MAX walks the tree which means it can be quite expensive. This cost would be paid for virtually every data segment. The actual insert that follows would walk the tree again. Instead, simply insert it. There is a slight cost of the unnecessary overlap check, but this is much less than the tree walk in a full tree.
Use this in places where we need to use the outer right edge of our sequence space. This way we can avoid walking the tree to find this, which is a potentially expensive operation.
Convert to rbtree from linked list. These ranges, of which there can be multiple per packet, are fully controlled by an attacked. The attacker could craft a stream of packet in such a way that the list would grow very large. This would make inserts/removals very expensive, as well as the list walk that is done and size calculation and pruning operations. The RBTREE makes inserts/removals much cheaper, at a slight overhead for 'normal' operations and slightly higher per record memory use.
Optimize by keeping count during insert/remove instead of walking the tree per check.
Switch StreamBufferBlocks implementation to use RBTREE instead of a list. This makes inserts/removals and lookups a lot cheaper if the number of data gaps is large. Use separate compare functions for inserts and regular lookups. Inserts care about the offset, while lookups care about the blocks right edge as well.
Change the way fields are ordered to reduce TcpSegment structure with 8 bytes.
Instead of just marking fragments that have been completely overlapped and won't be part of the assembled packet, remove them from the fragment tree when detected.
Merged
Merged into #3479, thanks Jason! |
catenacyber
added a commit
to catenacyber/suricata
that referenced
this pull request
Jan 15, 2021
finishes OISF#3475
victorjulien
pushed a commit
to victorjulien/suricata
that referenced
this pull request
Feb 23, 2021
Add AndX support for SMB1. Finishes OISF#3475. [Updated by Victor Julien to split functions]
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:
Changes:
PRscript: