-
Notifications
You must be signed in to change notification settings - Fork 556
feat: switch parser whitelist and WAF allowlist to BART lite trie #4142
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
base: master
Are you sure you want to change the base?
feat: switch parser whitelist and WAF allowlist to BART lite trie #4142
Conversation
Replace linear search through IP/CIDR slices with BART lite trie for O(log n) lookups. All IPs are converted to /32 (IPv4) or /128 (IPv6) CIDR format for consistent storage. - Parser whitelist: Use bart.Lite instead of []netip.Addr and []netip.Prefix - WAF allowlist: Use bart.Lite with separate metadata map for descriptions - Add benchmarks showing consistent performance across allowlist sizes - All tests pass with updated expectations Performance improvements: - Parser whitelist: ~1158-1417 ns/op regardless of allowlist size - WAF allowlist: ~353-434 ns/op with 0 allocs for negative lookups - Negative lookups: ~41-735 ns/op (very fast) These changes optimize the hot path in parser and WAF pipelines.
|
@LaurenceJJones: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
|
@LaurenceJJones: There are no area labels on this PR. You can add as many areas as you see fit.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4142 +/- ##
===========================================
- Coverage 62.89% 46.15% -16.75%
===========================================
Files 467 444 -23
Lines 33317 32198 -1119
===========================================
- Hits 20956 14861 -6095
- Misses 10238 15543 +5305
+ Partials 2123 1794 -329
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fix golangci-lint intrange warnings by replacing traditional for loops with Go 1.22+ integer range syntax (for i := range n instead of for i := 0; i < n; i++).
212eec1 to
6f7a825
Compare
Replace for range b.N loops with Go 1.23+ b.Loop() API for better benchmark timer management and more accurate measurements.
Build new trie and metadata map outside lock to minimize reader contention. Only swap pointers if data has actually changed using trie.Equal() comparison. This ensures: - Readers are only blocked during brief pointer swap operation - No unnecessary memory churn when data is unchanged - Thread-safe atomic replacement of both trie and metadata together
Replace linear search through IP/CIDR slices with BART lite trie for O(log n) lookups. All IPs are converted to /32 (IPv4) or /128 (IPv6) CIDR format for consistent storage.
These changes optimize the hot path in parser and WAF pipelines.
Important notes
Small lists (< 10 entries) do see a degradation in performance
3.1 ns/opto21 ns/opbut as allow list in WAF and whitelists in parsers are O(n) implementation their is a tradeoff once entries get to a certain size. During testing the tipping point starts around 50+ entries and from their hits and misses are substantially better for users using the new implementation.We could be smart about it and default to maps for small sets and convert once we hit this but in my opinion users wont feel the
~18nsdegradation in perf for the larger users benefit.