From 36137a4d6a27627df3f13a2371520fa2a538c848 Mon Sep 17 00:00:00 2001 From: Jason Barnett Date: Mon, 15 Jan 2024 14:33:21 -0700 Subject: [PATCH] wgengine/router: dramatically simplify udm-pro support Signed-off-by: Jason Barnett --- version/distro/distro.go | 5 +++ wgengine/router/router_linux.go | 56 +++++++++++++--------------- wgengine/router/router_linux_test.go | 36 ------------------ 3 files changed, 31 insertions(+), 66 deletions(-) diff --git a/version/distro/distro.go b/version/distro/distro.go index 449581222..ccadba7dc 100644 --- a/version/distro/distro.go +++ b/version/distro/distro.go @@ -153,6 +153,8 @@ func DSMVersion() int { }) } +// isUDMPro checks a couple of files known to exist on a UDM-Pro and returns +// true if the expected content exists in the files. func isUDMPro() bool { if exists, err := fileContainsString("/etc/board.info", "UDMPRO"); err == nil && exists { return true @@ -166,6 +168,9 @@ func isUDMPro() bool { return false } +// fileContainsString is used to determine if a string exists in a file. This is +// not efficient for larger files. If you want to use this function to parse +// large files, please refactor to use `io.LimitedReader`. func fileContainsString(filePath, searchString string) (bool, error) { data, err := os.ReadFile(filePath) if err != nil { diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index 67e7fce36..a471db674 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -1031,10 +1031,6 @@ func mustRouteTable(num int) RouteTable { var ( mainRouteTable = newRouteTable("main", 254) defaultRouteTable = newRouteTable("default", 253) - // Port 9 - WAN - udmProRouteTable1 = newRouteTable("201", 201) - // Port 10 - SFP+ WAN 2 - udmProRouteTable2 = newRouteTable("202", 202) // tailscaleRouteTable is the routing table number for Tailscale // network routes. See addIPRules for the detailed policy routing @@ -1055,7 +1051,9 @@ var ( tailscaleRouteTable = newRouteTable("tailscale", 52) ) -// _ipRules are the policy routing rules that Tailscale uses. +// baseIPRules are the policy routing rules that Tailscale uses, when not +// running on a UDM-Pro. +// // The priority is the value represented here added to r.ipPolicyPrefBase, // which is usually 5200. // @@ -1070,7 +1068,7 @@ var ( // and 'ip rule' implementations (including busybox), don't support // checking for the lack of a fwmark, only the presence. The technique // below works even on very old kernels. -var _ipRules = []netlink.Rule{ +var baseIPRules = []netlink.Rule{ // Packets from us, tagged with our fwmark, first try the kernel's // main routing table. { @@ -1106,15 +1104,32 @@ var _ipRules = []netlink.Rule{ // usual rules (pref 32766 and 32767, ie. main and default). } -var addedUDMProIPRules = false +// udmProIPRules are the policy routing rules that Tailscale uses, when running +// on a UDM-Pro. +// +// The priority is the value represented here added to +// r.ipPolicyPrefBase, which is usually 5200. +// +// This represents an experiment that will be used to gather more information. +// If this goes well, Tailscale may opt to use this for all of Linux. +var udmProIPRules = []netlink.Rule{ + // non-fwmark packets fall through to the usual rules (pref 32766 and 32767, + // ie. main and default). + { + Priority: 70, + Invert: true, + Mark: linuxfw.TailscaleBypassMarkNum, + Table: tailscaleRouteTable.Num, + }, +} +// ipRules returns the appropriate list of ip rules to be used by Tailscale. See +// comments on baseIPRules and udmProIPRules for more details. func ipRules() []netlink.Rule { - // lazy add UDM Pro rules if distro.Get() == distro.UDMPro { - addUDMProIpRules() + return udmProIPRules } - - return _ipRules + return baseIPRules } // justAddIPRules adds policy routing rule without deleting any first. @@ -1410,22 +1425,3 @@ func nlAddrOfPrefix(p netip.Prefix) *netlink.Addr { IPNet: netipx.PrefixIPNet(p), } } - -// Adds necessary ip rules for UDM-Pro. See issue 4038. -func addUDMProIpRules() { - if addedUDMProIPRules { - return - } - - _ipRules = append(_ipRules, netlink.Rule{ - Priority: 21, - Mark: linuxfw.TailscaleBypassMarkNum, - Table: udmProRouteTable1.Num, - }, netlink.Rule{ - Priority: 22, - Mark: linuxfw.TailscaleBypassMarkNum, - Table: udmProRouteTable2.Num, - }) - - addedUDMProIPRules = true -} diff --git a/wgengine/router/router_linux_test.go b/wgengine/router/router_linux_test.go index a20e3f565..b86469f6d 100644 --- a/wgengine/router/router_linux_test.go +++ b/wgengine/router/router_linux_test.go @@ -19,7 +19,6 @@ import ( "testing" "github.com/google/go-cmp/cmp" - ts_netlink "github.com/tailscale/netlink" "github.com/tailscale/wireguard-go/tun" "github.com/vishvananda/netlink" "go4.org/netipx" @@ -1133,38 +1132,3 @@ func adjustFwmask(t *testing.T, s string) string { return fwmaskAdjustRe.ReplaceAllString(s, "$1") } - -func TestAddUDMProIpRules(t *testing.T) { - originalIpRules := make([]ts_netlink.Rule, len(_ipRules)) - copy(originalIpRules, _ipRules) - - addUDMProIpRules() - - expectedRules := []ts_netlink.Rule{ - { - Priority: 21, - Mark: linuxfw.TailscaleBypassMarkNum, - Table: udmProRouteTable1.Num, - }, - { - Priority: 22, - Mark: linuxfw.TailscaleBypassMarkNum, - Table: udmProRouteTable2.Num, - }, - } - - for _, expected := range expectedRules { - found := false - for _, actual := range ipRules() { - if reflect.DeepEqual(actual, expected) { - found = true - break - } - } - if !found { - t.Errorf("Expected rule %+v; not found in ipRules", expected) - } - } - - _ipRules = originalIpRules -}