diff --git a/wgengine/router/ifconfig_windows.go b/wgengine/router/ifconfig_windows.go index 414a8b708..87ff69b3f 100644 --- a/wgengine/router/ifconfig_windows.go +++ b/wgengine/router/ifconfig_windows.go @@ -331,7 +331,7 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) (retErr error) { } } - var routes []*winipcfg.RouteData + var routes []*routeData foundDefault4 := false foundDefault6 := false for _, route := range cfg.Routes { @@ -361,10 +361,12 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) (retErr error) { } else if route.Addr().Is6() { gateway = firstGateway6 } - r := &winipcfg.RouteData{ - Destination: route, - NextHop: gateway, - Metric: 0, + r := &routeData{ + RouteData: winipcfg.RouteData{ + Destination: route, + NextHop: gateway, + Metric: 0, + }, } if r.Destination.Addr().Unmap() == gateway { // no need to add a route for the interface's @@ -393,9 +395,9 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) (retErr error) { return fmt.Errorf("syncAddresses: %w", err) } - slices.SortFunc(routes, routeDataCompare) + slices.SortFunc(routes, (*routeData).Compare) - deduplicatedRoutes := []*winipcfg.RouteData{} + deduplicatedRoutes := []*routeData{} for i := 0; i < len(routes); i++ { // There's only one way to get to a given IP+Mask, so delete // all matches after the first. @@ -596,18 +598,26 @@ func syncAddresses(ifc *winipcfg.IPAdapterAddresses, want []netip.Prefix) error return erracc } -func routeDataLess(a, b *winipcfg.RouteData) bool { - return routeDataCompare(a, b) < 0 +// routeData wraps winipcfg.RouteData with an additional field that permits +// caching of the associated MibIPForwardRow2; by keeping it around, we can +// avoid unnecessary (and slow) lookups of information that we already have. +type routeData struct { + winipcfg.RouteData + Row *winipcfg.MibIPforwardRow2 } -func routeDataCompare(a, b *winipcfg.RouteData) int { - v := a.Destination.Addr().Compare(b.Destination.Addr()) +func (rd *routeData) Less(other *routeData) bool { + return rd.Compare(other) < 0 +} + +func (rd *routeData) Compare(other *routeData) int { + v := rd.Destination.Addr().Compare(other.Destination.Addr()) if v != 0 { return v } // Narrower masks first - b1, b2 := a.Destination.Bits(), b.Destination.Bits() + b1, b2 := rd.Destination.Bits(), other.Destination.Bits() if b1 != b2 { if b1 > b2 { return -1 @@ -616,31 +626,31 @@ func routeDataCompare(a, b *winipcfg.RouteData) int { } // No nexthop before non-empty nexthop - v = a.NextHop.Compare(b.NextHop) + v = rd.NextHop.Compare(other.NextHop) if v != 0 { return v } // Lower metrics first - if a.Metric < b.Metric { + if rd.Metric < other.Metric { return -1 - } else if a.Metric > b.Metric { + } else if rd.Metric > other.Metric { return 1 } return 0 } -func deltaRouteData(a, b []*winipcfg.RouteData) (add, del []*winipcfg.RouteData) { - add = make([]*winipcfg.RouteData, 0, len(b)) - del = make([]*winipcfg.RouteData, 0, len(a)) - slices.SortFunc(a, routeDataCompare) - slices.SortFunc(b, routeDataCompare) +func deltaRouteData(a, b []*routeData) (add, del []*routeData) { + add = make([]*routeData, 0, len(b)) + del = make([]*routeData, 0, len(a)) + slices.SortFunc(a, (*routeData).Compare) + slices.SortFunc(b, (*routeData).Compare) i := 0 j := 0 for i < len(a) && j < len(b) { - switch routeDataCompare(a[i], b[j]) { + switch a[i].Compare(b[j]) { case -1: // a < b, delete del = append(del, a[i]) @@ -677,7 +687,7 @@ func getInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses, family winipcfg.Addres return } -func getAllInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses) ([]*winipcfg.RouteData, error) { +func getAllInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses) ([]*routeData, error) { routes4, err := getInterfaceRoutes(ifc, windows.AF_INET) if err != nil { return nil, err @@ -688,19 +698,27 @@ func getAllInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses) ([]*winipcfg.RouteD // TODO: what if v6 unavailable? return nil, err } - rd := make([]*winipcfg.RouteData, 0, len(routes4)+len(routes6)) + + rd := make([]*routeData, 0, len(routes4)+len(routes6)) for _, r := range routes4 { - rd = append(rd, &winipcfg.RouteData{ - Destination: r.DestinationPrefix.Prefix(), - NextHop: r.NextHop.Addr(), - Metric: r.Metric, + rd = append(rd, &routeData{ + RouteData: winipcfg.RouteData{ + Destination: r.DestinationPrefix.Prefix(), + NextHop: r.NextHop.Addr(), + Metric: r.Metric, + }, + Row: r, }) } + for _, r := range routes6 { - rd = append(rd, &winipcfg.RouteData{ - Destination: r.DestinationPrefix.Prefix(), - NextHop: r.NextHop.Addr(), - Metric: r.Metric, + rd = append(rd, &routeData{ + RouteData: winipcfg.RouteData{ + Destination: r.DestinationPrefix.Prefix(), + NextHop: r.NextHop.Addr(), + Metric: r.Metric, + }, + Row: r, }) } return rd, nil @@ -708,7 +726,7 @@ func getAllInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses) ([]*winipcfg.RouteD // filterRoutes removes routes that have been added by Windows and should not // be managed by us. -func filterRoutes(routes []*winipcfg.RouteData, dontDelete []netip.Prefix) []*winipcfg.RouteData { +func filterRoutes(routes []*routeData, dontDelete []netip.Prefix) []*routeData { ddm := make(map[netip.Prefix]bool) for _, dd := range dontDelete { // See issue 1448: we don't want to touch the routes added @@ -727,7 +745,7 @@ func filterRoutes(routes []*winipcfg.RouteData, dontDelete []netip.Prefix) []*wi lastIP := netipx.RangeOfPrefix(nr).To() ddm[netip.PrefixFrom(lastIP, lastIP.BitLen())] = true } - filtered := make([]*winipcfg.RouteData, 0, len(routes)) + filtered := make([]*routeData, 0, len(routes)) for _, r := range routes { rr := r.Destination if rr.IsValid() && ddm[rr] { @@ -742,7 +760,7 @@ func filterRoutes(routes []*winipcfg.RouteData, dontDelete []netip.Prefix) []*wi // This avoids a full ifc.FlushRoutes call. // dontDelete is a list of interface address routes that the // synchronization logic should never delete. -func syncRoutes(ifc *winipcfg.IPAdapterAddresses, want []*winipcfg.RouteData, dontDelete []netip.Prefix) error { +func syncRoutes(ifc *winipcfg.IPAdapterAddresses, want []*routeData, dontDelete []netip.Prefix) error { existingRoutes, err := getAllInterfaceRoutes(ifc) if err != nil { return err @@ -753,7 +771,15 @@ func syncRoutes(ifc *winipcfg.IPAdapterAddresses, want []*winipcfg.RouteData, do var errs []error for _, a := range del { - err := ifc.LUID.DeleteRoute(a.Destination, a.NextHop) + var err error + if a.Row == nil { + // DeleteRoute requires a routing table lookup, so only do that if + // a does not already have the row. + err = ifc.LUID.DeleteRoute(a.Destination, a.NextHop) + } else { + // Otherwise, delete the row directly. + err = a.Row.Delete() + } if err != nil { dstStr := a.Destination.String() if dstStr == "169.254.255.255/32" { diff --git a/wgengine/router/ifconfig_windows_test.go b/wgengine/router/ifconfig_windows_test.go index 4679a149d..64c0ff0e9 100644 --- a/wgengine/router/ifconfig_windows_test.go +++ b/wgengine/router/ifconfig_windows_test.go @@ -19,58 +19,62 @@ func randIP() netip.Addr { return netip.AddrFrom4([4]byte{b, b, b, b}) } -func randRouteData() *winipcfg.RouteData { - return &winipcfg.RouteData{ - Destination: netip.PrefixFrom(randIP(), rand.Intn(30)+1), - NextHop: randIP(), - Metric: uint32(rand.Intn(3)), +func randRouteData() *routeData { + return &routeData{ + RouteData: winipcfg.RouteData{ + Destination: netip.PrefixFrom(randIP(), rand.Intn(30)+1), + NextHop: randIP(), + Metric: uint32(rand.Intn(3)), + }, } } +type W = winipcfg.RouteData + func TestRouteLess(t *testing.T) { - type D = winipcfg.RouteData + type D = routeData ipnet := netip.MustParsePrefix tests := []struct { - ri, rj *winipcfg.RouteData + ri, rj *routeData want bool }{ { - ri: &D{Metric: 1}, - rj: &D{Metric: 2}, + ri: &D{RouteData: W{Metric: 1}}, + rj: &D{RouteData: W{Metric: 2}}, want: true, }, { - ri: &D{Destination: ipnet("1.1.0.0/16"), Metric: 2}, - rj: &D{Destination: ipnet("2.2.0.0/16"), Metric: 1}, + ri: &D{RouteData: W{Destination: ipnet("1.1.0.0/16"), Metric: 2}}, + rj: &D{RouteData: W{Destination: ipnet("2.2.0.0/16"), Metric: 1}}, want: true, }, { - ri: &D{Destination: ipnet("1.1.0.0/16"), Metric: 1}, - rj: &D{Destination: ipnet("2.2.0.0/16"), Metric: 1}, + ri: &D{RouteData: W{Destination: ipnet("1.1.0.0/16"), Metric: 1}}, + rj: &D{RouteData: W{Destination: ipnet("2.2.0.0/16"), Metric: 1}}, want: true, }, { - ri: &D{Destination: ipnet("1.1.0.0/32"), Metric: 2}, - rj: &D{Destination: ipnet("1.1.0.0/16"), Metric: 1}, + ri: &D{RouteData: W{Destination: ipnet("1.1.0.0/32"), Metric: 2}}, + rj: &D{RouteData: W{Destination: ipnet("1.1.0.0/16"), Metric: 1}}, want: true, }, { - ri: &D{Destination: ipnet("1.1.0.0/32"), Metric: 1}, - rj: &D{Destination: ipnet("1.1.0.0/16"), Metric: 1}, + ri: &D{RouteData: W{Destination: ipnet("1.1.0.0/32"), Metric: 1}}, + rj: &D{RouteData: W{Destination: ipnet("1.1.0.0/16"), Metric: 1}}, want: true, }, { - ri: &D{Destination: ipnet("1.1.0.0/16"), Metric: 1, NextHop: netip.MustParseAddr("3.3.3.3")}, - rj: &D{Destination: ipnet("1.1.0.0/16"), Metric: 1, NextHop: netip.MustParseAddr("4.4.4.4")}, + ri: &D{RouteData: W{Destination: ipnet("1.1.0.0/16"), Metric: 1, NextHop: netip.MustParseAddr("3.3.3.3")}}, + rj: &D{RouteData: W{Destination: ipnet("1.1.0.0/16"), Metric: 1, NextHop: netip.MustParseAddr("4.4.4.4")}}, want: true, }, } for i, tt := range tests { - got := routeDataLess(tt.ri, tt.rj) + got := tt.ri.Less(tt.rj) if got != tt.want { t.Errorf("%v. less = %v; want %v", i, got, tt.want) } - back := routeDataLess(tt.rj, tt.ri) + back := tt.rj.Less(tt.ri) if back && got { t.Errorf("%v. less both ways", i) } @@ -81,7 +85,7 @@ func TestRouteDataLessConsistent(t *testing.T) { for i := 0; i < 10000; i++ { ri := randRouteData() rj := randRouteData() - if routeDataLess(ri, rj) && routeDataLess(rj, ri) { + if ri.Less(rj) && rj.Less(ri) { t.Fatalf("both compare less to each other:\n\t%#v\nand\n\t%#v", ri, rj) } } @@ -139,7 +143,7 @@ func TestDeltaNets(t *testing.T) { } } -func formatRouteData(rds []*winipcfg.RouteData) string { +func formatRouteData(rds []*routeData) string { var b strings.Builder for _, rd := range rds { b.WriteString(fmt.Sprintf("%+v", rd)) @@ -147,12 +151,12 @@ func formatRouteData(rds []*winipcfg.RouteData) string { return b.String() } -func equalRouteDatas(a, b []*winipcfg.RouteData) bool { +func equalRouteDatas(a, b []*routeData) bool { if len(a) != len(b) { return false } for i := range a { - if routeDataCompare(a[i], b[i]) != 0 { + if a[i].Compare(b[i]) != 0 { return false } } @@ -166,33 +170,33 @@ func ipnet4(ip string, bits int) netip.Prefix { func TestFilterRoutes(t *testing.T) { var h0 netip.Addr - in := []*winipcfg.RouteData{ + in := []*routeData{ // LinkLocal and Loopback routes. - {ipnet4("169.254.0.0", 16), h0, 1}, - {ipnet4("169.254.255.255", 32), h0, 1}, - {ipnet4("127.0.0.0", 8), h0, 1}, - {ipnet4("127.255.255.255", 32), h0, 1}, + {RouteData: W{ipnet4("169.254.0.0", 16), h0, 1}}, + {RouteData: W{ipnet4("169.254.255.255", 32), h0, 1}}, + {RouteData: W{ipnet4("127.0.0.0", 8), h0, 1}}, + {RouteData: W{ipnet4("127.255.255.255", 32), h0, 1}}, // Local LAN routes. - {ipnet4("192.168.0.0", 24), h0, 1}, - {ipnet4("192.168.0.255", 32), h0, 1}, - {ipnet4("192.168.1.0", 25), h0, 1}, - {ipnet4("192.168.1.127", 32), h0, 1}, + {RouteData: W{ipnet4("192.168.0.0", 24), h0, 1}}, + {RouteData: W{ipnet4("192.168.0.255", 32), h0, 1}}, + {RouteData: W{ipnet4("192.168.1.0", 25), h0, 1}}, + {RouteData: W{ipnet4("192.168.1.127", 32), h0, 1}}, // Some random other route. - {ipnet4("192.168.2.23", 32), h0, 1}, + {RouteData: W{ipnet4("192.168.2.23", 32), h0, 1}}, // Our own tailscale address. - {ipnet4("100.100.100.100", 32), h0, 1}, + {RouteData: W{ipnet4("100.100.100.100", 32), h0, 1}}, // Other tailscale addresses. - {ipnet4("100.100.100.101", 32), h0, 1}, - {ipnet4("100.100.100.102", 32), h0, 1}, + {RouteData: W{ipnet4("100.100.100.101", 32), h0, 1}}, + {RouteData: W{ipnet4("100.100.100.102", 32), h0, 1}}, } - want := []*winipcfg.RouteData{ - {ipnet4("169.254.0.0", 16), h0, 1}, - {ipnet4("127.0.0.0", 8), h0, 1}, - {ipnet4("192.168.0.0", 24), h0, 1}, - {ipnet4("192.168.1.0", 25), h0, 1}, - {ipnet4("192.168.2.23", 32), h0, 1}, - {ipnet4("100.100.100.101", 32), h0, 1}, - {ipnet4("100.100.100.102", 32), h0, 1}, + want := []*routeData{ + {RouteData: W{ipnet4("169.254.0.0", 16), h0, 1}}, + {RouteData: W{ipnet4("127.0.0.0", 8), h0, 1}}, + {RouteData: W{ipnet4("192.168.0.0", 24), h0, 1}}, + {RouteData: W{ipnet4("192.168.1.0", 25), h0, 1}}, + {RouteData: W{ipnet4("192.168.2.23", 32), h0, 1}}, + {RouteData: W{ipnet4("100.100.100.101", 32), h0, 1}}, + {RouteData: W{ipnet4("100.100.100.102", 32), h0, 1}}, } got := filterRoutes(in, mustCIDRs("100.100.100.100/32")) @@ -206,25 +210,25 @@ func TestDeltaRouteData(t *testing.T) { h1 := netip.MustParseAddr("99.99.99.99") h2 := netip.MustParseAddr("99.99.9.99") - a := []*winipcfg.RouteData{ - {ipnet4("1.2.3.4", 32), h0, 1}, - {ipnet4("1.2.3.4", 24), h1, 2}, - {ipnet4("1.2.3.4", 24), h2, 1}, - {ipnet4("1.2.3.5", 32), h0, 1}, + a := []*routeData{ + {RouteData: W{ipnet4("1.2.3.4", 32), h0, 1}}, + {RouteData: W{ipnet4("1.2.3.4", 24), h1, 2}}, + {RouteData: W{ipnet4("1.2.3.4", 24), h2, 1}}, + {RouteData: W{ipnet4("1.2.3.5", 32), h0, 1}}, } - b := []*winipcfg.RouteData{ - {ipnet4("1.2.3.5", 32), h0, 1}, - {ipnet4("1.2.3.4", 24), h1, 2}, - {ipnet4("1.2.3.4", 24), h2, 2}, + b := []*routeData{ + {RouteData: W{ipnet4("1.2.3.5", 32), h0, 1}}, + {RouteData: W{ipnet4("1.2.3.4", 24), h1, 2}}, + {RouteData: W{ipnet4("1.2.3.4", 24), h2, 2}}, } add, del := deltaRouteData(a, b) - wantAdd := []*winipcfg.RouteData{ - {ipnet4("1.2.3.4", 24), h2, 2}, + wantAdd := []*routeData{ + {RouteData: W{ipnet4("1.2.3.4", 24), h2, 2}}, } - wantDel := []*winipcfg.RouteData{ - {ipnet4("1.2.3.4", 32), h0, 1}, - {ipnet4("1.2.3.4", 24), h2, 1}, + wantDel := []*routeData{ + {RouteData: W{ipnet4("1.2.3.4", 32), h0, 1}}, + {RouteData: W{ipnet4("1.2.3.4", 24), h2, 1}}, } if !equalRouteDatas(add, wantAdd) {