TheArchive/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2024-11-25 19:15:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

wgengine/router: make the Windows ifconfig implementation reuse existing MibIPforwardRow2 when possible

Browse Source 
Looking at profiles, we spend a lot of time in winipcfg.LUID.DeleteRoute
looking up the routing table entry for the provided RouteData.

But we already have the row! We previously obtained that data via the full
table dump we did in getInterfaceRoutes. We can make this a lot faster by
hanging onto a reference to the wipipcfg.MibIPforwardRow2 and executing
the delete operation directly on that.

Fixes #11123

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
This commit is contained in:
Aaron Klotz 2024-02-13 10:32:48 -07:00
parent 30c9189ed3
commit f7acbefbbb
2 changed files with 125 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

96
wgengine/router/ifconfig_windows.go
View File

@ -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" {

124
wgengine/router/ifconfig_windows_test.go
View File

@ -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) {