Merge set elements

[tacerus]

Hi,

nft will print intervals in CIDR representation - for example:

inet filter @testset6
        element 000080fe 00000000 00000000 02000000  : 1 [end]
        element 000080fe 00000000 00000000 01000000  : 0 [end]
        element 0000072a 01000000 00000000 00000000  : 1 [end]
        element 0000072a 00000000 00000000 00000000  : 0 [end]
        element 00000000 00000000 00000000 00000000  : 1 [end]
inet filter @testset4
        element 0001a8c0  : 1 [end]
        element 0000a8c0  : 0 [end]
        element 0200007f  : 1 [end]
        element 0100007f  : 0 [end]
        element 0200000a  : 1 [end]
        element 0100000a  : 0 [end]
        element 00000000  : 1 [end]

gets turned into:

table inet filter {
        set testset6 {
                type ipv6_addr
                flags interval
                elements = { 2a07::/64,
                             fe80::1 }
        }
        set testset4 {
                type ipv4_addr
                flags interval
                elements = { 10.0.0.1, 127.0.0.1,
                             192.168.0.0/24 }
        }
}

Thanks to https://github.com/google/nftables/pull/342 it is now easy to add new elements in this format.

I would appreciate some advice in regards to retrieving elements in the same representation.

One issue is https://github.com/google/nftables/issues/320, but it can be worked around by iterating over the returned elements in reverse order (hoping the order is stable).

So far I came up with the following (omitted set discovery and error handling for brevity):

        elements, _ := nft.GetSetElements(set)

        var (
                out []string  // will contain the resulting nft-style elements
                last nftables.SetElement
        )

        NullIPv4 := []byte{0, 0, 0, 0}
        NullIPv6 := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}

        for i := len(elements)-1; i >= 0; i-- {
                e := elements[i]
                switch set.KeyType.Name {
                case "ipv4_addr", "ipv6_addr":
                        if bytes.Compare(e.Key, NullIPv4) == 0 || bytes.Compare(e.Key, NullIPv6) == 0 {
                                continue
                        }

                        ipCurrent, _ := netip.AddrFromSlice(e.Key)
                        ipLast, _ := netip.AddrFromSlice(last.Key)
                        ipLastNext := ipLast.Next()

                        if e.IntervalEnd && ipCurrent == ipLastNext {
                                out = append(out, ipLast.String())
                                continue
                        }

                        if e.IntervalEnd {
                                maxLen := 32
                                if ipLastNext.Is6() {
                                        if !ipCurrent.Is6() {
                                                continue
                                        }
                                        maxLen = 128
                                }
                                for l := maxLen; l >= 0; l-- {
                                        mask := net.CIDRMask(l, maxLen)
                                        na := net.IP(ipLastNext.AsSlice()).Mask(mask)
                                        n := net.IPNet{IP: na, Mask: mask}
                                        if n.Contains(net.IP(ipCurrent.AsSlice())) {
                                                out = append(out, fmt.Sprintf("%s/%d", na, l+1))
                                                break
                                        }
                                }
                                continue
                        }

                        last = e
                }
        }

Returns (in out):

["10.0.0.1","127.0.0.1","192.168.0.0/24"]  // "testset4"
["2a07::/64","fe80::1"] // "testset6"

I would be interested if anyone has feedback and/or a better approach for this - the above feels rather clunky, and I'm not sure if I am not missing any cases (for example, the skipping of zero elements and the need for appending 1 to the mask feels wrong).

If there is a cleaner solution, possibly we could integrate it into the library (either directly into GetSetElements or into a helper function)? It would be useful to reduce the differences between nft and Go, particularly when using both for management.

[nickgarlis]

Having this would definitely be useful. However, I am not sure how much of it can be cleanly abstracted.

Perhaps, a few helpers like the following would be a good start.

• NetFromFirstAndLastIP, for getting a CIDR out of a range e.g (10.0.0.0 - 10.0.0.255)
• previousIP
• NetFromNetInterval, for combining NetFromFirstAndLastIP and previousIP

With only these helpers, the user would still need to know which pairs should be combined (and which should not) after calling GetSetElements. The usage would end up looking something like this:

els, _ := conn.GetSetElements(set)
slices.Reverse(els)

var cidrs []net.IPNet
var first []byte

for i, el := range els {
    if i == 0 && el.IntervalEnd {
        continue // skip first segment if present
    }

    if !el.IntervalEnd {
        // Start of interval
        first = el.Key
        continue
    }

    if el.IntervalEnd {
        // Normal closed interval
        cidrs = append(cidrs, NetFromNetInterval(first, el.Key))
    }

    if el.IntervalOpen {
        // no corresponding end element
        cidrs = append(cidrs, NetFromNetInterval(first, net.IPv4zero))
    }
}

Abstracting this logic inside the library would likely require significant changes to existing types and creation/retrieval code.

[tacerus]

Thanks for the input! The suggested helper functions sound good indeed, I will start with those.

Your example is much cleaner, however I have not been able to fully fit it. While the helpers work well, I still need to track the last address in the iteration, as only working with IntervalEnd/IntervalOpen would return bogus networks - will need to investigate some more.

[tacerus]

I think I figured it out. I made https://github.com/google/nftables/pull/347 (I named the functions slightly differently to better indicate they're related).