wgengine/router: consolidate routes before reconfiguring router for mobile clients

This helps reduce memory pressure on tailnets with large numbers
of routes.

Updates tailscale/corp#19332

Signed-off-by: Percy Wegmann <percy@tailscale.com>
This commit is contained in:
Percy Wegmann 2024-04-23 10:56:36 -05:00 committed by Percy Wegmann
parent add62af7c6
commit c8e912896e
3 changed files with 146 additions and 1 deletions

View File

@ -0,0 +1,59 @@
// Copyright (c) Tailscale Inc & AUTHORS
// SPDX-License-Identifier: BSD-3-Clause
package router
import (
"go4.org/netipx"
"tailscale.com/types/logger"
)
// ConsolidatingRoutes wraps a Router with logic that consolidates Routes
// whenever Set is called. It attempts to consolidate cfg.Routes into the
// smallest possible set.
func ConsolidatingRoutes(logf logger.Logf, router Router) Router {
return &consolidatingRouter{Router: router, logf: logger.WithPrefix(logf, "router: ")}
}
type consolidatingRouter struct {
Router
logf logger.Logf
}
// Set implements Router and attempts to consolidate cfg.Routes into the
// smallest possible set.
func (cr *consolidatingRouter) Set(cfg *Config) error {
return cr.Router.Set(cr.consolidateRoutes(cfg))
}
func (cr *consolidatingRouter) consolidateRoutes(cfg *Config) *Config {
if cfg == nil {
return nil
}
if len(cfg.Routes) < 2 {
return cfg
}
if len(cfg.Routes) == 2 && cfg.Routes[0].Addr().Is4() != cfg.Routes[1].Addr().Is4() {
return cfg
}
var builder netipx.IPSetBuilder
for _, route := range cfg.Routes {
builder.AddPrefix(route)
}
set, err := builder.IPSet()
if err != nil {
cr.logf("consolidateRoutes failed, keeping existing routes: %s", err)
return cfg
}
newRoutes := set.Prefixes()
oldLength := len(cfg.Routes)
newLength := len(newRoutes)
if oldLength == newLength {
// Nothing consolidated, return as-is.
return cfg
}
cr.logf("consolidated %d routes down to %d", oldLength, newLength)
newCfg := *cfg
newCfg.Routes = newRoutes
return &newCfg
}

View File

@ -0,0 +1,68 @@
// Copyright (c) Tailscale Inc & AUTHORS
// SPDX-License-Identifier: BSD-3-Clause
package router
import (
"log"
"net/netip"
"testing"
"github.com/google/go-cmp/cmp"
)
func TestConsolidateRoutes(t *testing.T) {
parseRoutes := func(routes ...string) []netip.Prefix {
parsed := make([]netip.Prefix, 0, len(routes))
for _, routeString := range routes {
route, err := netip.ParsePrefix(routeString)
if err != nil {
t.Fatal(err)
}
parsed = append(parsed, route)
}
return parsed
}
tests := []struct {
name string
cfg *Config
want *Config
}{
{
"nil cfg",
nil,
nil,
},
{
"single route",
&Config{Routes: parseRoutes("10.0.0.0/32")},
&Config{Routes: parseRoutes("10.0.0.0/32")},
},
{
"two routes from different families",
&Config{Routes: parseRoutes("10.0.0.0/32", "2603:1030:c02::/47")},
&Config{Routes: parseRoutes("10.0.0.0/32", "2603:1030:c02::/47")},
},
{
"two disjoint routes",
&Config{Routes: parseRoutes("10.0.0.0/32", "10.0.2.0/32")},
&Config{Routes: parseRoutes("10.0.0.0/32", "10.0.2.0/32")},
},
{
"two overlapping routes",
&Config{Routes: parseRoutes("10.0.0.0/32", "10.0.0.0/31")},
&Config{Routes: parseRoutes("10.0.0.0/31")},
},
}
cr := &consolidatingRouter{logf: log.Printf}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got := cr.consolidateRoutes(test.cfg)
if diff := cmp.Diff(got, test.want); diff != "" {
t.Errorf("wrong result; (-got+want):%v", diff)
}
})
}
}

View File

@ -47,6 +47,7 @@
"tailscale.com/util/deephash" "tailscale.com/util/deephash"
"tailscale.com/util/mak" "tailscale.com/util/mak"
"tailscale.com/util/set" "tailscale.com/util/set"
"tailscale.com/version"
"tailscale.com/wgengine/capture" "tailscale.com/wgengine/capture"
"tailscale.com/wgengine/filter" "tailscale.com/wgengine/filter"
"tailscale.com/wgengine/magicsock" "tailscale.com/wgengine/magicsock"
@ -281,13 +282,30 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
} }
closePool.add(tsTUNDev) closePool.add(tsTUNDev)
rtr := conf.Router
if version.IsMobile() {
// Android and iOS don't handle large numbers of routes well, so we
// wrap the Router with one that consolidates routes down to the
// smallest number possible.
//
// On Android, too many routes at VPN configuration time result in an
// android.os.TransactionTooLargeException because Android's VPNBuilder
// tries to send the entire set of routes to the VPNService as a single
// Bundle, which is typically limited to 1 MB. The number of routes
// that's too much seems to be very roughly around 4000.
//
// On iOS, the VPNExtension is limited to only 50 MB of memory, so
// keeping the number of routes down helps with memory consumption.
rtr = router.ConsolidatingRoutes(logf, rtr)
}
e := &userspaceEngine{ e := &userspaceEngine{
timeNow: mono.Now, timeNow: mono.Now,
logf: logf, logf: logf,
reqCh: make(chan struct{}, 1), reqCh: make(chan struct{}, 1),
waitCh: make(chan struct{}), waitCh: make(chan struct{}),
tundev: tsTUNDev, tundev: tsTUNDev,
router: conf.Router, router: rtr,
confListenPort: conf.ListenPort, confListenPort: conf.ListenPort,
birdClient: conf.BIRDClient, birdClient: conf.BIRDClient,
controlKnobs: conf.ControlKnobs, controlKnobs: conf.ControlKnobs,