From 536f5e83ff5518ff5f710bf3be25db8cde3a785f Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Fri, 14 Aug 2026 18:54:50 +0200 Subject: [PATCH] Rank Windows route candidates by combined route and interface metric --- .../systemops/routeselection_windows_test.go | 82 +++++++++++++++++++ .../systemops/systemops_windows.go | 27 ++++-- 2 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 client/internal/routemanager/systemops/routeselection_windows_test.go diff --git a/client/internal/routemanager/systemops/routeselection_windows_test.go b/client/internal/routemanager/systemops/routeselection_windows_test.go new file mode 100644 index 000000000..108338dd9 --- /dev/null +++ b/client/internal/routemanager/systemops/routeselection_windows_test.go @@ -0,0 +1,82 @@ +//go:build windows + +package systemops + +import ( + "math" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSortRouteCandidates(t *testing.T) { + tests := []struct { + name string + candidates []candidateRoute + wantOrder []uint32 + }{ + { + name: "longest prefix wins over metrics", + candidates: []candidateRoute{ + {interfaceIndex: 1, prefixLength: 0, routeMetric: 0, interfaceMetric: 5}, + {interfaceIndex: 2, prefixLength: 24, routeMetric: 100, interfaceMetric: 50}, + }, + wantOrder: []uint32{2, 1}, + }, + { + // Windows ranks equal-length prefixes by route metric + interface metric, + // so a higher route metric on a low metric interface can still win. + name: "combined metric beats route metric alone", + candidates: []candidateRoute{ + {interfaceIndex: 8, prefixLength: 0, routeMetric: 0, interfaceMetric: 100}, + {interfaceIndex: 5, prefixLength: 0, routeMetric: 10, interfaceMetric: 5}, + }, + wantOrder: []uint32{5, 8}, + }, + { + name: "lower combined metric wins", + candidates: []candidateRoute{ + {interfaceIndex: 5, prefixLength: 0, routeMetric: 300, interfaceMetric: 5}, + {interfaceIndex: 8, prefixLength: 0, routeMetric: 0, interfaceMetric: 100}, + }, + wantOrder: []uint32{8, 5}, + }, + { + name: "equal combined metric falls back to route metric", + candidates: []candidateRoute{ + {interfaceIndex: 1, prefixLength: 0, routeMetric: 20, interfaceMetric: 10}, + {interfaceIndex: 2, prefixLength: 0, routeMetric: 5, interfaceMetric: 25}, + }, + wantOrder: []uint32{2, 1}, + }, + { + // The metrics are uint32 on the Windows side, so the sum must not wrap. + name: "combined metric beyond the uint32 range", + candidates: []candidateRoute{ + {interfaceIndex: 1, prefixLength: 0, routeMetric: math.MaxUint32, interfaceMetric: 5}, + {interfaceIndex: 2, prefixLength: 0, routeMetric: math.MaxUint32 - 10, interfaceMetric: 5}, + }, + wantOrder: []uint32{2, 1}, + }, + { + name: "unknown interface metric ranks on route metric only", + candidates: []candidateRoute{ + {interfaceIndex: 1, prefixLength: 0, routeMetric: 30, interfaceMetric: -1}, + {interfaceIndex: 2, prefixLength: 0, routeMetric: 5, interfaceMetric: 10}, + }, + wantOrder: []uint32{2, 1}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sortRouteCandidates(tt.candidates) + + got := make([]uint32, 0, len(tt.candidates)) + for _, c := range tt.candidates { + got = append(got, c.interfaceIndex) + } + assert.Equal(t, tt.wantOrder, got) + }) + } +} diff --git a/client/internal/routemanager/systemops/systemops_windows.go b/client/internal/routemanager/systemops/systemops_windows.go index 7bce6af80..47d556cf6 100644 --- a/client/internal/routemanager/systemops/systemops_windows.go +++ b/client/internal/routemanager/systemops/systemops_windows.go @@ -882,26 +882,40 @@ func getInterfaceMetric(interfaceIndex uint32, family int16) int { return int(ipInterfaceRow.Metric) } -// sortRouteCandidates sorts route candidates by priority: prefix length -> route metric -> interface metric +// sortRouteCandidates sorts route candidates by priority: prefix length -> combined metric -> route metric. +// Windows prefers the longest matching prefix and, among prefixes of the same length, the lowest metric, see +// https://learn.microsoft.com/en-us/windows-hardware/customize/desktop/unattend/microsoft-windows-tcpip-interfaces-interface-routes-route-metric func sortRouteCandidates(candidates []candidateRoute) { sort.Slice(candidates, func(i, j int) bool { if candidates[i].prefixLength != candidates[j].prefixLength { return candidates[i].prefixLength > candidates[j].prefixLength } - if candidates[i].routeMetric != candidates[j].routeMetric { - return candidates[i].routeMetric < candidates[j].routeMetric + mi, mj := combinedMetric(candidates[i]), combinedMetric(candidates[j]) + if mi != mj { + return mi < mj } - return candidates[i].interfaceMetric < candidates[j].interfaceMetric + return candidates[i].routeMetric < candidates[j].routeMetric }) } +// combinedMetric returns the effective metric Windows uses to rank routes with an equal prefix length: +// the sum of the route metric and the metric of the interface the route is on, see +// https://learn.microsoft.com/en-us/windows-server/networking/technologies/network-subsystem/net-sub-interface-metric +// An unknown interface metric contributes nothing. +func combinedMetric(candidate candidateRoute) uint64 { + if candidate.interfaceMetric < 0 { + return uint64(candidate.routeMetric) + } + return uint64(candidate.routeMetric) + uint64(candidate.interfaceMetric) +} + // GetBestInterface finds the best interface for reaching a destination, // excluding the VPN interface to avoid routing loops. // // Route selection priority: // 1. Longest prefix match (most specific route) -// 2. Lowest route metric -// 3. Lowest interface metric +// 2. Lowest combined metric (route metric + interface metric) +// 3. Lowest route metric. func GetBestInterface(dest netip.Addr, vpnIntf string) (*net.Interface, error) { var skipInterfaceIndex int if vpnIntf != "" { @@ -925,7 +939,6 @@ func GetBestInterface(dest netip.Addr, vpnIntf string) (*net.Interface, error) { return nil, fmt.Errorf("no route to %s", dest) } - // Sort routes: prefix length -> route metric -> interface metric sortRouteCandidates(candidates) for _, candidate := range candidates {