Skip to content

fix(router): stop merging separately advertised prefixes #172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
encodeous wants to merge 1 commit into from
Closed

fix(router): stop merging separately advertised prefixes #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions core/router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package core

import (
"net/netip"
"slices"
"time"

"github.com/gaissmai/bart"
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
37 changes: 35 additions & 2 deletions core/sys_route_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
Loading