-
Notifications
You must be signed in to change notification settings - Fork 163
Add NetInterval helper #342
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
Conversation
afe2136 to
a183ed5
Compare
Can you explain when/where/how nftables uses such an interval? I would also like to add a reference to the wraparound behavior (ending up with :: or 0.0.0.0), because that sounds a little unexpected…
I haven’t worked much with the new netip type. I would hold off on making this change for now. We can consider updating all net.IP to netip.Addr independently.
No, I would like to keep the dependencies as minimal as possible for this project. |
When you add a CIDR element to an nftables set, nft internally represents that CIDR as two separate interval elements. For example, adding []nftables.SetElement{
{Key: net.ParseIP("10.0.0.0").To4()},
{Key: net.ParseIP("10.0.1.0").To4(), IntervalEnd: true},
}You can see this by running: which prints something like: If the set is initially empty, nft may show an extra unrelated element. That behavior is explained in more detail in #247 and is outside the scope of this PR. What happens if you use the normal range?If you instead create the elements like this: []nftables.SetElement{
{Key: net.ParseIP("10.0.0.0").To4()},
{Key: net.ParseIP("10.0.0.255").To4(), IntervalEnd: true},
}and then inspect the set with This shows that nft normalizes interval boundaries itself so the correct approach should be to follow nft’s internal interval representation. Edge case: ranges that overflowIf the final IP in the interval overflows (e.g., With #343 this would be addressed like this: []nftables.SetElement{
{Key: net.ParseIP("255.255.255.254").To4(), IntervalOpen: true},
}Putting it all togetherGiven an arbitrary CIDR, this is how I would use those two features together: var elements []nftables.SetElement
first, last, err := nftables.NetInterval(unknownCIDR)
if err != nil {
return err
}
// last overflowed - there is no end element
if last.IsUnspecified() { // perhaps last == nil would be better ?
elements = []nftables.SetElement{
{
Key: first.To4(),
IntervalOpen: true,
},
}
} else {
elements = []nftables.SetElement{
{
Key: first.To4(),
},
{
Key: last.To4(),
IntervalEnd: true,
},
}
}Please let me know if you would like me to clarify anything. I am happy to go into more detail.
I think the explanation above covers this, but just to make sure we are aligned: the zero IP is used to represent "no end IP", since in some cases, the last IP would go past the maximum valid address. Would using a pointer be a better approach ?
Sounds good 👍
Also sounds good 👍 |
|
Thanks for the explanation!
So, to be clear, using the zero IP as a sentinel value is our own choice, not required by nftables, yes? And when marshaling, do we convert zero IP to userdata-marker? If so, I think we should clarify the comment of
|
When creating set elements that represent a network, the interval range must be half-open [start, end) rather than inclusive [start, end]. For example, for 10.0.0.0/24, the expected range is 10.0.0.0 to 10.0.1.0 instead of 10.0.0.0 to 10.0.0.255. This change introduces a NetInterval helper that returns the correct range given a CIDR string.
a183ed5 to
434649a
Compare
Yes, it's our own choice and not something required by nftables. However, we don’t attempt to automatically detect the zero IP or add the userdata marker during marshaling. Instead, we rely on the user to supply the appropriate flag on the correct element, as shown in the example above.
👍 |
|
Thank you for this - very useful! |
When creating set elements that represent a network, the interval range must be half-open [start, end) rather than inclusive [start, end]. For example, for 10.0.0.0/24, the expected range is 10.0.0.0 to 10.0.1.0 instead of 10.0.0.0 to 10.0.0.255.
This change introduces a NetInterval helper that returns the correct range given a CIDR string.
Some notes for consideration:
net.IPfor consistency withNetFirstAndLastIP. However,netip.Addrseems to be the new standard. Should I change this PR to convert the returned values ofNetIntervaltonetip.Addr?