From 8d3acc9235e07a981d206750aa866ce66dc4d306 Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Mon, 3 Apr 2023 16:08:29 -0400 Subject: [PATCH] util/sysresources, magicsock: scale DERP buffer based on system memory This adds the util/sysresources package, which currently only contains a function to return the total memory size of the current system. Then, we modify magicsock to scale the number of buffered DERP messages based on the system's available memory, ensuring that we never use a value lower than the previous constant of 32. Signed-off-by: Andrew Dunham Change-Id: Ib763c877de4d0d4ee88869078e7d512f6a3a148d --- cmd/tailscaled/depaware.txt | 1 + util/sysresources/memory.go | 10 ++++ util/sysresources/memory_bsd.go | 16 +++++++ util/sysresources/memory_darwin.go | 16 +++++++ util/sysresources/memory_linux.go | 19 ++++++++ util/sysresources/memory_unsupported.go | 8 ++++ util/sysresources/sysresources.go | 6 +++ util/sysresources/sysresources_test.go | 25 ++++++++++ wgengine/magicsock/magicsock.go | 62 ++++++++++++++++++++++--- wgengine/magicsock/magicsock_test.go | 8 ++++ 10 files changed, 164 insertions(+), 7 deletions(-) create mode 100644 util/sysresources/memory.go create mode 100644 util/sysresources/memory_bsd.go create mode 100644 util/sysresources/memory_darwin.go create mode 100644 util/sysresources/memory_linux.go create mode 100644 util/sysresources/memory_unsupported.go create mode 100644 util/sysresources/sysresources.go create mode 100644 util/sysresources/sysresources_test.go diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 6e1a0f912..96d64216e 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -312,6 +312,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/util/set from tailscale.com/health+ tailscale.com/util/singleflight from tailscale.com/control/controlclient+ tailscale.com/util/slicesx from tailscale.com/net/dnscache+ + tailscale.com/util/sysresources from tailscale.com/wgengine/magicsock tailscale.com/util/systemd from tailscale.com/control/controlclient+ tailscale.com/util/uniq from tailscale.com/wgengine/magicsock+ tailscale.com/util/vizerror from tailscale.com/tsweb diff --git a/util/sysresources/memory.go b/util/sysresources/memory.go new file mode 100644 index 000000000..7363155cd --- /dev/null +++ b/util/sysresources/memory.go @@ -0,0 +1,10 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package sysresources + +// TotalMemory returns the total accessible system memory, in bytes. If the +// value cannot be determined, then 0 will be returned. +func TotalMemory() uint64 { + return totalMemoryImpl() +} diff --git a/util/sysresources/memory_bsd.go b/util/sysresources/memory_bsd.go new file mode 100644 index 000000000..26850dce6 --- /dev/null +++ b/util/sysresources/memory_bsd.go @@ -0,0 +1,16 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build freebsd || openbsd || dragonfly || netbsd + +package sysresources + +import "golang.org/x/sys/unix" + +func totalMemoryImpl() uint64 { + val, err := unix.SysctlUint64("hw.physmem") + if err != nil { + return 0 + } + return val +} diff --git a/util/sysresources/memory_darwin.go b/util/sysresources/memory_darwin.go new file mode 100644 index 000000000..e07bac0cd --- /dev/null +++ b/util/sysresources/memory_darwin.go @@ -0,0 +1,16 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build darwin + +package sysresources + +import "golang.org/x/sys/unix" + +func totalMemoryImpl() uint64 { + val, err := unix.SysctlUint64("hw.memsize") + if err != nil { + return 0 + } + return val +} diff --git a/util/sysresources/memory_linux.go b/util/sysresources/memory_linux.go new file mode 100644 index 000000000..0239b0e80 --- /dev/null +++ b/util/sysresources/memory_linux.go @@ -0,0 +1,19 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build linux + +package sysresources + +import "golang.org/x/sys/unix" + +func totalMemoryImpl() uint64 { + var info unix.Sysinfo_t + + if err := unix.Sysinfo(&info); err != nil { + return 0 + } + + // uint64 casts are required since these might be uint32s + return uint64(info.Totalram) * uint64(info.Unit) +} diff --git a/util/sysresources/memory_unsupported.go b/util/sysresources/memory_unsupported.go new file mode 100644 index 000000000..0fde256e0 --- /dev/null +++ b/util/sysresources/memory_unsupported.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !(linux || darwin || freebsd || openbsd || dragonfly || netbsd) + +package sysresources + +func totalMemoryImpl() uint64 { return 0 } diff --git a/util/sysresources/sysresources.go b/util/sysresources/sysresources.go new file mode 100644 index 000000000..32d972ab1 --- /dev/null +++ b/util/sysresources/sysresources.go @@ -0,0 +1,6 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Package sysresources provides OS-independent methods of determining the +// resources available to the current system. +package sysresources diff --git a/util/sysresources/sysresources_test.go b/util/sysresources/sysresources_test.go new file mode 100644 index 000000000..331ad913b --- /dev/null +++ b/util/sysresources/sysresources_test.go @@ -0,0 +1,25 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package sysresources + +import ( + "runtime" + "testing" +) + +func TestTotalMemory(t *testing.T) { + switch runtime.GOOS { + case "linux": + case "freebsd", "openbsd", "dragonfly", "netbsd": + case "darwin": + default: + t.Skipf("not supported on runtime.GOOS=%q yet", runtime.GOOS) + } + + mem := TotalMemory() + if mem == 0 { + t.Fatal("wanted TotalMemory > 0") + } + t.Logf("total memory: %v bytes", mem) +} diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index d391ea2c8..d33cbd244 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -63,6 +63,7 @@ "tailscale.com/util/clientmetric" "tailscale.com/util/mak" "tailscale.com/util/ringbuffer" + "tailscale.com/util/sysresources" "tailscale.com/util/uniq" "tailscale.com/version" "tailscale.com/wgengine/capture" @@ -1418,12 +1419,59 @@ func (c *Conn) sendAddr(addr netip.AddrPort, pubKey key.NodePublic, b []byte) (s } } -// bufferedDerpWritesBeforeDrop is how many packets writes can be -// queued up the DERP client to write on the wire before we start -// dropping. -// -// TODO: this is currently arbitrary. Figure out something better? -const bufferedDerpWritesBeforeDrop = 32 +var ( + bufferedDerpWrites int + bufferedDerpWritesOnce sync.Once +) + +// bufferedDerpWritesBeforeDrop returns how many packets writes can be queued +// up the DERP client to write on the wire before we start dropping. +func bufferedDerpWritesBeforeDrop() int { + // For mobile devices, always return the previous minimum value of 32; + // we can do this outside the sync.Once to avoid that overhead. + if runtime.GOOS == "ios" || runtime.GOOS == "android" { + return 32 + } + + bufferedDerpWritesOnce.Do(func() { + // Some rough sizing: for the previous fixed value of 32, the + // total consumed memory can be: + // = numDerpRegions * messages/region * sizeof(message) + // + // For sake of this calculation, assume 100 DERP regions; at + // time of writing (2023-04-03), we have 24. + // + // A reasonable upper bound for the worst-case average size of + // a message is a *disco.CallMeMaybe message with 16 endpoints; + // since sizeof(netip.AddrPort) = 32, that's 512 bytes. Thus: + // = 100 * 32 * 512 + // = 1638400 (1.6MiB) + // + // On a reasonably-small node with 4GiB of memory that's + // connected to each region and handling a lot of load, 1.6MiB + // is about 0.04% of the total system memory. + // + // For sake of this calculation, then, let's double that memory + // usage to 0.08% and scale based on total system memory. + // + // For a 16GiB Linux box, this should buffer just over 256 + // messages. + systemMemory := sysresources.TotalMemory() + memoryUsable := float64(systemMemory) * 0.0008 + + const ( + theoreticalDERPRegions = 100 + messageMaximumSizeBytes = 512 + ) + bufferedDerpWrites = int(memoryUsable / (theoreticalDERPRegions * messageMaximumSizeBytes)) + + // Never drop below the previous minimum value. + if bufferedDerpWrites < 32 { + bufferedDerpWrites = 32 + } + }) + return bufferedDerpWrites +} // derpWriteChanOfAddr returns a DERP client for fake UDP addresses that // represent DERP servers, creating them as necessary. For real UDP @@ -1520,7 +1568,7 @@ func (c *Conn) derpWriteChanOfAddr(addr netip.AddrPort, peer key.NodePublic) cha dc.DNSCache = dnscache.Get() ctx, cancel := context.WithCancel(c.connCtx) - ch := make(chan derpWriteRequest, bufferedDerpWritesBeforeDrop) + ch := make(chan derpWriteRequest, bufferedDerpWritesBeforeDrop()) ad.c = dc ad.writeCh = ch diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 53e8b60d9..18851d28d 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -1853,3 +1853,11 @@ func TestSetNetworkMapWithNoPeers(t *testing.T) { } } } + +func TestBufferedDerpWritesBeforeDrop(t *testing.T) { + vv := bufferedDerpWritesBeforeDrop() + if vv < 32 { + t.Fatalf("got bufferedDerpWritesBeforeDrop=%d, which is < 32", vv) + } + t.Logf("bufferedDerpWritesBeforeDrop = %d", vv) +}