From fe9c17cedcb83ace15e959fe7ec09799e4876cc3 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Tue, 13 Aug 2024 14:19:49 +0200 Subject: [PATCH] Compact BGPFilter to save switch memory (#559) --- .../internal/service/switch-service.go | 32 ++++++- .../internal/service/switch-service_test.go | 94 +++++++++++++++++++ go.mod | 2 +- 3 files changed, 126 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 4e9cd57b..8fea0f96 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/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 diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index 51be9077..8d003d12 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/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) +} diff --git a/go.mod b/go.mod index d6f93369..dfd9688d 100644 --- a/go.mod +++ b/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