From 1b57b0380d5040446a8cc5ca7509f871e5cb3f03 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Tue, 15 Mar 2022 15:33:06 -0700 Subject: [PATCH] wgengine/magicsock: remove final alloc from ReceiveFrom And now that we don't have to play escape analysis and inlining games, simplify the code. Signed-off-by: Josh Bleecher Snyder --- cmd/tailscaled/depaware.txt | 3 ++- wgengine/magicsock/magicsock.go | 35 +++++++++++++--------------- wgengine/magicsock/magicsock_test.go | 10 +++++--- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index dfadcc7c6..9d7b4566a 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -257,6 +257,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/util/groupmember from tailscale.com/ipn/ipnserver tailscale.com/util/lineread from tailscale.com/hostinfo+ tailscale.com/util/multierr from tailscale.com/cmd/tailscaled+ + tailscale.com/util/netconv from tailscale.com/wgengine/magicsock tailscale.com/util/osshare from tailscale.com/cmd/tailscaled+ tailscale.com/util/pidowner from tailscale.com/ipn/ipnserver tailscale.com/util/racebuild from tailscale.com/logpolicy @@ -382,7 +383,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de net/http/httputil from tailscale.com/cmd/tailscaled+ net/http/internal from net/http+ net/http/pprof from tailscale.com/cmd/tailscaled+ - net/netip from net + net/netip from net+ net/textproto from golang.org/x/net/http/httpguts+ net/url from crypto/x509+ os from crypto/rand+ diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 1d621f753..0d64c717d 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -17,6 +17,7 @@ import ( "math" "math/rand" "net" + "net/netip" "reflect" "runtime" "sort" @@ -53,6 +54,7 @@ import ( "tailscale.com/types/netmap" "tailscale.com/types/nettype" "tailscale.com/util/clientmetric" + "tailscale.com/util/netconv" "tailscale.com/util/uniq" "tailscale.com/version" "tailscale.com/wgengine/monitor" @@ -3025,35 +3027,30 @@ func (c *RebindingUDPConn) ReadFromNetaddr(b []byte) (n int, ipp netaddr.IPPort, pconn := c.currentConn() // Optimization: Treat *net.UDPConn specially. - // ReadFromUDP gets partially inlined, avoiding allocating a *net.UDPAddr, - // as long as pAddr itself doesn't escape. + // This lets us avoid allocations by calling ReadFromUDPAddrPort. // The non-*net.UDPConn case works, but it allocates. - var pAddr *net.UDPAddr if udpConn, ok := pconn.(*net.UDPConn); ok { - n, pAddr, err = udpConn.ReadFromUDP(b) + var ap netip.AddrPort + n, ap, err = udpConn.ReadFromUDPAddrPort(b) + ipp = netconv.AsIPPort(ap) } else { var addr net.Addr n, addr, err = pconn.ReadFrom(b) - if addr != nil { - pAddr, ok = addr.(*net.UDPAddr) + pAddr, ok := addr.(*net.UDPAddr) + if addr != nil && !ok { + return 0, netaddr.IPPort{}, fmt.Errorf("RebindingUDPConn.ReadFromNetaddr: underlying connection returned address of type %T, want *netaddr.UDPAddr", addr) + } + if pAddr != nil { + ipp, ok = netaddr.FromStdAddr(pAddr.IP, pAddr.Port, pAddr.Zone) if !ok { - return 0, netaddr.IPPort{}, fmt.Errorf("RebindingUDPConn.ReadFromNetaddr: underlying connection returned address of type %T, want *netaddr.UDPAddr", addr) + return 0, netaddr.IPPort{}, errors.New("netaddr.FromStdAddr failed") } } } - if err != nil { - if pconn != c.currentConn() { - continue - } - } else { - // Convert pAddr to a netaddr.IPPort. - // This prevents pAddr from escaping. - var ok bool - ipp, ok = netaddr.FromStdAddr(pAddr.IP, pAddr.Port, pAddr.Zone) - if !ok { - return 0, netaddr.IPPort{}, errors.New("netaddr.FromStdAddr failed") - } + if err != nil && pconn != c.currentConn() { + // The connection changed underfoot. Try again. + continue } return n, ipp, err } diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index d53cb5eef..07c9d9cbc 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -1345,14 +1345,18 @@ func TestReceiveFromAllocs(t *testing.T) { } // Go 1.16 and before: allow 3 allocs. // Go 1.17: allow 2 allocs. - // Go Tailscale fork, Go 1.18+: allow 1 alloc. + // Go 1.17, Tailscale fork: allow 1 alloc. + // Go 1.18+: allow 0 allocs. + // Go 2.0: allow -1 allocs (projected). major, ts := goMajorVersion(runtime.Version()) maxAllocs := 3 switch { - case major == 17: + case major == 17 && !ts: maxAllocs = 2 - case major >= 18, ts: + case major == 17 && ts: maxAllocs = 1 + case major >= 18: + maxAllocs = 0 } t.Logf("allowing %d allocs for Go version %q", maxAllocs, runtime.Version()) roundTrip := setUpReceiveFrom(t)