Compact BGPFilter to save switch memory (#559)

Author: majst01

cmd/metal-api/internal/service/switch-service.go

@@ -14,6 +14,7 @@ import (
 	"github.com/avast/retry-go/v4"
 	restfulspec "github.com/emicklei/go-restful-openapi/v2"
 	restful "github.com/emicklei/go-restful/v3"
+	"go4.org/netipx"
 
 	"github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore"
 	"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
@@ -835,6 +836,9 @@ func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) (v1.BGPFilter, erro
 		if underlay != nil && underlay.ContainsIP(i.IPAddress) {
 			continue
 		}
+
+		// TODO machine BGPFilter must not contain firewall private network IPs
+
 		// Allow all other ip addresses allocated for the project.
 		ipwithMask, err := ipWithMask(i.IPAddress)
 		if err != nil {
@@ -843,7 +847,11 @@ func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) (v1.BGPFilter, erro
 		cidrs = append(cidrs, ipwithMask)
 	}
 
-	return v1.NewBGPFilter(vnis, cidrs), nil
+	compactedCidrs, err := compactCidrs(cidrs)
+	if err != nil {
+		return v1.BGPFilter{}, err
+	}
+	return v1.NewBGPFilter(vnis, compactedCidrs), nil
 }
 
 func ipWithMask(ip string) (string, error) {
@@ -854,6 +862,28 @@ func ipWithMask(ip string) (string, error) {
 	return fmt.Sprintf("%s/%d", ip, parsed.BitLen()), nil
 }
 
+func compactCidrs(cidrs []string) ([]string, error) {
+	// compact all cidrs which are used to be added to the route map
+	// to find the smallest sorted set of prefixes which covers all cidrs which need to be added.
+	var ipsetBuilder netipx.IPSetBuilder
+	for _, cidr := range cidrs {
+		parsed, err := netip.ParsePrefix(cidr)
+		if err != nil {
+			return nil, err
+		}
+		ipsetBuilder.AddPrefix(parsed)
+	}
+	set, err := ipsetBuilder.IPSet()
+	if err != nil {
+		return nil, fmt.Errorf("unable to create ipset:%w", err)
+	}
+	var compactedCidrs []string
+	for _, pfx := range set.Prefixes() {
+		compactedCidrs = append(compactedCidrs, pfx.String())
+	}
+	return compactedCidrs, nil
+}
+
 func makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap) (v1.BGPFilter, error) {
 	var (
 		filter v1.BGPFilter

cmd/metal-api/internal/service/switch-service_test.go

@@ -1561,3 +1561,97 @@ func Test_SwitchDelete(t *testing.T) {
 		})
 	}
 }
+
+func TestCompactCidrs(t *testing.T) {
+	// sample cidrs from a production cluster passed in to be added to the route map
+	cidrs := []string{
+		"10.4.0.31/32",
+		"10.6.0.25/32",
+		"10.64.28.0/22",
+		"10.67.36.138/32",
+		"10.76.20.12/32",
+		"10.76.20.14/32",
+		"10.76.20.15/32",
+		"10.76.20.16/32",
+		"10.76.20.17/32",
+		"10.76.20.2/32",
+		"10.76.20.3/32",
+		"10.76.20.4/32",
+		"10.76.20.5/32",
+		"10.76.20.6/32",
+		"10.76.20.7/32",
+		"10.76.20.8/32",
+		"10.76.20.9/32",
+		"10.78.248.134/32",
+		"2001:db8::7/128",
+		"2001:db8::8/128",
+		"2001:db8::20/128",
+		"2001:db8::db/128",
+		"100.127.130.178/32",
+		"100.127.130.179/32",
+		"100.127.130.180/32",
+		"100.127.130.181/32",
+		"100.127.130.182/32",
+		"100.127.130.183/32",
+		"100.153.67.112/32",
+		"100.153.67.113/32",
+		"100.153.67.114/32",
+		"100.153.67.115/32",
+		"100.153.67.116/32",
+		"100.34.85.136/32",
+		"100.34.85.17/32",
+		"100.34.89.209/32",
+		"2001:db8::9/128",
+		"2001:db8::10/128",
+		"100.34.89.210/32",
+		"100.90.30.12/32",
+		"100.90.30.13/32",
+		"100.90.30.14/32",
+		"100.90.30.15/32",
+		"100.90.30.16/32",
+		"100.90.30.32/32",
+		"100.90.30.4/32",
+		"100.90.30.7/32",
+	}
+
+	compactedCidrs := []string{
+		"10.4.0.31/32",
+		"10.6.0.25/32",
+		"10.64.28.0/22",
+		"10.67.36.138/32",
+		"10.76.20.2/31",
+		"10.76.20.4/30",
+		"10.76.20.8/31",
+		"10.76.20.12/32",
+		"10.76.20.14/31",
+		"10.76.20.16/31",
+		"10.78.248.134/32",
+		"100.90.30.4/32",
+		"100.90.30.7/32",
+		"100.90.30.12/30",
+		"100.90.30.16/32",
+		"100.90.30.32/32",
+		"100.127.130.178/31",
+		"100.127.130.180/30",
+		"100.153.67.112/30",
+		"100.153.67.116/32",
+		"100.34.85.17/32",
+		"100.34.85.136/32",
+		"100.34.89.209/32",
+		"100.34.89.210/32",
+		"2001:db8::7/128",
+		"2001:db8::8/127",
+		"2001:db8::10/128",
+		"2001:db8::20/128",
+		"2001:db8::db/128",
+	}
+
+	compacted, err := compactCidrs(cidrs)
+	require.NoError(t, err)
+	require.Less(t, len(compacted), len(cidrs))
+	require.Len(t, compacted, 29)
+
+	t.Logf("aggregated cidrs:%s old count:%d new count:%d", compacted, len(cidrs), len(compacted))
+
+	require.ElementsMatch(t, compactedCidrs, compacted)
+}

go.mod

@@ -30,6 +30,7 @@ require (
 	github.com/spf13/viper v1.19.0
 	github.com/stretchr/testify v1.9.0
 	github.com/testcontainers/testcontainers-go v0.32.0
+	go4.org/netipx v0.0.0-20231129151722-fdeea329fbba
 	golang.org/x/crypto v0.26.0
 	golang.org/x/sync v0.8.0
 	google.golang.org/grpc v1.65.0
@@ -199,7 +200,6 @@ require (
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.27.0 // indirect
 	go4.org/mem v0.0.0-20240501181205-ae6ca9944745 // indirect
-	go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect
 	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
 	golang.org/x/net v0.28.0 // indirect
 	golang.org/x/oauth2 v0.22.0 // indirect