google / google/nftables

Inconsistent ordering of interval set elements in GetSetElements

Open
#320 0 comments 1 reaction 0 assignees View on GitHub
question
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.