diff --git a/core/router.go b/core/router.go index aebf19cc..58d9c66d 100644 --- a/core/router.go +++ b/core/router.go @@ -2,6 +2,7 @@ package core import ( "net/netip" + "slices" "time" "github.com/gaissmai/bart" @@ -209,7 +210,12 @@ func (n *Nylon) InitRouter() error { return nil } -// ComputeSysRouteTable computes: computed = prefixes - (((n.CentralCfg.ExcludeIPs U selected self prefixes) - n.LocalCfg.UnexcludeIPs) U n.LocalCfg.ExcludeIPs) +// ComputeSysRouteTable computes, for each prefix p: p - (((n.CentralCfg.ExcludeIPs U selected self prefixes) - n.LocalCfg.UnexcludeIPs) U n.LocalCfg.ExcludeIPs) +// +// Each prefix is computed on its own, so separately advertised prefixes are +// never merged. Windows treats the last address of an on-link prefix as its +// broadcast address, so merging 10.0.0.2/32 and 10.0.0.3/32 into 10.0.0.2/31 +// would make 10.0.0.3 unreachable. func (n *Nylon) ComputeSysRouteTable() []netip.Prefix { prefixes := make([]netip.Prefix, 0) selectedSelf := make([]netip.Prefix, 0) @@ -225,14 +231,21 @@ func (n *Nylon) ComputeSysRouteTable() []netip.Prefix { excludes.AddSet(state.MakeSet(selectedSelf)) excludes.RemoveSet(state.MakeSet(n.LocalCfg.UnexcludeIPs)) excludes.AddSet(state.MakeSet(n.LocalCfg.ExcludeIPs)) - - final := netipx.IPSetBuilder{} - final.AddSet(state.MakeSet(prefixes)) - res, _ := excludes.IPSet() - final.RemoveSet(res) - - res, _ = final.IPSet() - return res.Prefixes() + excluded, _ := excludes.IPSet() + + res := make([]netip.Prefix, 0, len(prefixes)) + for _, prefix := range prefixes { + remaining := netipx.IPSetBuilder{} + remaining.AddPrefix(prefix) + remaining.RemoveSet(excluded) + set, _ := remaining.IPSet() + res = append(res, set.Prefixes()...) + } + // overlapping prefixes can leave the same remainder + slices.SortFunc(res, func(a, b netip.Prefix) int { + return a.Compare(b) + }) + return slices.Compact(res) } func (n *Nylon) updatePassiveClient(prefix state.PrefixHealthWrapper, node state.NodeId, passiveHold bool) { diff --git a/core/sys_route_test.go b/core/sys_route_test.go index c3b5b13c..84be5e93 100644 --- a/core/sys_route_test.go +++ b/core/sys_route_test.go @@ -75,7 +75,7 @@ func TestComputeSysRouteTableDoesNotMutateCentralExcludes(t *testing.T) { assert.Equal(t, []netip.Prefix{pfx("10.0.0.0/25")}, n.CentralCfg.ExcludeIPs) } -func TestComputeSysRouteTableCoalescesAdjacentResults(t *testing.T) { +func TestComputeSysRouteTableKeepsAdjacentPrefixesSeparate(t *testing.T) { n := sysRouteTestNylon( "a", nil, @@ -84,10 +84,43 @@ func TestComputeSysRouteTableCoalescesAdjacentResults(t *testing.T) { map[netip.Prefix]state.SelRoute{ pfx("10.0.0.0/25"): {Nh: "b"}, pfx("10.0.0.128/25"): {Nh: "b"}, + pfx("10.0.1.2/32"): {Nh: "b"}, + pfx("10.0.1.3/32"): {Nh: "c"}, }, ) - assert.Equal(t, []netip.Prefix{pfx("10.0.0.0/24")}, sortedPrefixes(n.ComputeSysRouteTable())) + assert.Equal(t, []netip.Prefix{ + pfx("10.0.0.0/25"), + pfx("10.0.0.128/25"), + pfx("10.0.1.2/32"), + pfx("10.0.1.3/32"), + }, n.ComputeSysRouteTable()) +} + +func TestComputeSysRouteTableKeepsOverlappingPrefixes(t *testing.T) { + n := sysRouteTestNylon( + "a", + nil, + nil, + nil, + map[netip.Prefix]state.SelRoute{ + pfx("10.0.0.0/24"): {Nh: "b"}, + pfx("10.0.0.0/25"): {Nh: "c"}, + pfx("10.0.0.5/32"): {Nh: "a"}, + }, + ) + + // the remainders of the /24 and /25 share everything below 10.0.0.128, which is listed once + assert.ElementsMatch(t, []netip.Prefix{ + pfx("10.0.0.0/30"), + pfx("10.0.0.4/32"), + pfx("10.0.0.6/31"), + pfx("10.0.0.8/29"), + pfx("10.0.0.16/28"), + pfx("10.0.0.32/27"), + pfx("10.0.0.64/26"), + pfx("10.0.0.128/25"), + }, n.ComputeSysRouteTable()) } func sysRouteTestNylon(local state.NodeId, centralExcludes, localUnexcludes, localExcludes []netip.Prefix, routes map[netip.Prefix]state.SelRoute) *Nylon {