Inconsistent ordering of interval set elements in GetSetElements
- Dominant language
- Go
- Stars
- 1.4k
- Forks
- 184
- PR merge metrics
- No merged PRs in 30d
Description
When retrieving set elements that include intervals via `GetSetElements`, I've observed inconsistent ordering of elements with the IntervalEnd flag. Sometimes the IntervalEnd element appears before its corresponding interval start, and sometimes it appears after, which is unexpected and inconsistent.
## Steps to reproduce
1. Given the following set elements:
```go
elements := []nftables.SetElement{
// 10.0.0.0/24
{
Key: net.IPv4(10, 0, 0, 0).To4(),
},
{
Key: net.IPv4(10, 0, 1, 0).To4(),
IntervalEnd: true,
},
// 1.1.1.1/32
{
Key: net.IPv4(1, 1, 1, 1).To4(),
},
{
Key: net.IPv4(1, 1, 1, 1).To4(),
IntervalEnd: true,
},
}
```
2. Call `GetSetElements` to retrieve the set elements.
## Observed behavior
The returned elements are ordered inconsistently:
```go
elements := []nftables.SetElement{
// 10.0.0.0/24
{
Key: net.IPv4(10, 0, 1, 0).To4(),
IntervalEnd: true, // IntervalEnd appears before the interval start
},
{
Key: net.IPv4(10, 0, 0, 0).To4(),
},
// 1.1.1.1/32
{
Key: net.IPv4(1, 1, 1, 1).To4(),
},
{
Key: net.IPv4(1, 1, 1, 1).To4(),
IntervalEnd: true, // IntervalEnd is in the expected position
},
}
```
I've created a test case that demonstrates this in detail: https://github.com/nickgarlis/nftables/blob/test-set-elements-order/nftables_test.go#L7433
## Question
Is this ordering behavior expected, or could it be a bug in the `GetSetElements` implementation? Could it be that the elements are simply returned in the order they are received from netlink ? I briefly looked through the source code but didn’t see any obvious logic handling element sorting.
## Notes
- I noticed this behavior because I was trying to parse some existing set elements which is a little tricky now.
- `nft` appears to sort set elements when displaying them in CIDR notation. By contrast, this library returns elements in their order of creation. Should this library consider adopting nft’s approach ?
## Edit:
After opening this issue, I realized I made a mistake in the 1.1.1.1/32 example. I had used two elements with the same key, which likely caused the unexpected behavior with interval boundaries.
The correct elements for that interval should have been:
```go
{
Key: net.IPv4(1, 1, 1, 1).To4(),
},
{
Key: net.IPv4(1, 1, 1, 2).To4(),
IntervalEnd: true,
},
```
That said, the elements are still being returned in reverse order which might be worth discussing. I am leaving the issue open in case others have insights or opinions about this.
Contributor guide
Assessment
This issue has not been assessed yet.