-
Notifications
You must be signed in to change notification settings - Fork 75
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
implement CIDR matching #42
Conversation
64af4b7
to
552a203
Compare
mapping map[netip.Prefix]V | ||
} | ||
|
||
func (b *bruteForceMapper[V]) GetIP(addr netip.Addr) (value V, matched bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: named return values seem to be the easiest way to return a default generic type cleanly while also documenting the return values
otherwise we would have a local type like var value V
to achieve the same thing, you cannot do say return nil, false
👁️ /cc |
83149f9
to
2982e29
Compare
preliminary benchmarking has brute force taking 150x time, even with the current relatively simplistic implementation. it does take longer than i'd like to initialize the trie, we may want to pre-initialize it (expected, but didn't want to do that before measuring...) |
a9db5d0
to
9b640a4
Compare
NOTE: most of the lines in the diff are just |
9b640a4
to
3ccc71f
Compare
With the real AWS data loaded including IPv6 (unless I've somehow misread these results, I don't find myself benchmarking go all that often...) ~ 0.0015 s to initialize the TrieMap and ~ 0.87 MB memory overhead ( We have room to improve these if need be, but they should be plenty acceptable already. |
007a5e7
to
5545b69
Compare
Also a conscious decision: we could fairly easily minify The file is not included at runtime anyhow, only in the repo. The raw data from AWS is pre-processed by ranges2go into pre-parsed CIDRs in a more useful format for our purposes ( |
I think the
does not fail locally, and by trivial inspection these errors are not correct |
/retest |
5b2f483
to
d2c8b12
Compare
d2c8b12
to
14eeea7
Compare
/approve thanks @BenTheElder looking forward to hooking up this to the main code base next! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, dims The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is inspired by how the kernel does routing (with a LPC Trie), thanks @aojea for the pointer to https://vincent.bernat.ch/en/blog/2017-ipv4-route-lookup-linux
There is one existing go library for this at https://github.com/yl2chen/cidranger but it implements more of a set type (we need a map to AWS regions), doesn't use the newer
net/netip
types which are more efficient, etc.We implement a reasonably efficient generic map type in relatively little code here (~260 lines TrieMap including comments, whitespace, license header etc.).
While the current implementation here lacks level and path compression, cidranger only implements path compression.
For AWS data memory usage is acceptable without compression (ballpark less than 1MB), while lookup performance is with the AWS IPv4 data is ~40ns / op for this implementation vs ~400ns / op for cidranger on my 16" intel macbook laptop in microbenchmarking (and both drastically better than brute force looping over CIDRs).
See: #42 (comment) with the full dataset ipv4 + ipv6 matching, performance is ~fine. We can continue optimizing in follow-ups as needed.
See: #39
/cc @dims @ameukam