diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 89f722e8b..74d3e9e1e 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -48,7 +48,6 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/types/opt from tailscale.com/net/netcheck+ tailscale.com/types/persist from tailscale.com/ipn tailscale.com/types/preftype from tailscale.com/cmd/tailscale/cli+ - tailscale.com/types/strbuilder from tailscale.com/net/packet tailscale.com/types/structs from tailscale.com/ipn+ tailscale.com/types/wgkey from tailscale.com/types/netmap+ tailscale.com/util/dnsname from tailscale.com/cmd/tailscale/cli+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 47e94f4ab..7e934e198 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -128,7 +128,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/types/opt from tailscale.com/control/controlclient+ tailscale.com/types/persist from tailscale.com/control/controlclient+ tailscale.com/types/preftype from tailscale.com/ipn+ - tailscale.com/types/strbuilder from tailscale.com/net/packet tailscale.com/types/structs from tailscale.com/control/controlclient+ tailscale.com/types/wgkey from tailscale.com/control/controlclient+ L tailscale.com/util/cmpver from tailscale.com/net/dns diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index c4ae286c0..08b0a9269 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1697,7 +1697,10 @@ func (b *LocalBackend) authReconfig() { rcfg := b.routerConfig(cfg, uc) - var dcfg dns.Config + dcfg := dns.Config{ + Routes: map[dnsname.FQDN][]netaddr.IPPort{}, + Hosts: map[dnsname.FQDN][]netaddr.IP{}, + } // Populate MagicDNS records. We do this unconditionally so that // quad-100 can always respond to MagicDNS queries, even if the OS @@ -1728,7 +1731,6 @@ func (b *LocalBackend) authReconfig() { } dcfg.Hosts[fqdn] = ips } - dcfg.Hosts = map[dnsname.FQDN][]netaddr.IP{} set(nm.Name, nm.Addresses) for _, peer := range nm.Peers { set(peer.Name, peer.Addresses) @@ -1747,9 +1749,6 @@ func (b *LocalBackend) authReconfig() { } addDefault(nm.DNS.Resolvers) - if len(nm.DNS.Routes) > 0 { - dcfg.Routes = map[dnsname.FQDN][]netaddr.IPPort{} - } for suffix, resolvers := range nm.DNS.Routes { fqdn, err := dnsname.ToFQDN(suffix) if err != nil { diff --git a/net/packet/packet.go b/net/packet/packet.go index 7bb31aa5f..5c27b6cc4 100644 --- a/net/packet/packet.go +++ b/net/packet/packet.go @@ -12,7 +12,6 @@ import ( "inet.af/netaddr" "tailscale.com/types/ipproto" - "tailscale.com/types/strbuilder" ) const unknown = ipproto.Unknown @@ -62,36 +61,17 @@ func (p *Parsed) String() string { return "Unknown{???}" } - sb := strbuilder.Get() - sb.WriteString(p.IPProto.String()) - sb.WriteByte('{') - writeIPPort(sb, p.Src) - sb.WriteString(" > ") - writeIPPort(sb, p.Dst) - sb.WriteByte('}') - return sb.String() -} - -// writeIPPort writes ipp.String() into sb, with fewer allocations. -// -// TODO: make netaddr more efficient in this area, and retire this func. -func writeIPPort(sb *strbuilder.Builder, ipp netaddr.IPPort) { - if ipp.IP().Is4() { - raw := ipp.IP().As4() - sb.WriteUint(uint64(raw[0])) - sb.WriteByte('.') - sb.WriteUint(uint64(raw[1])) - sb.WriteByte('.') - sb.WriteUint(uint64(raw[2])) - sb.WriteByte('.') - sb.WriteUint(uint64(raw[3])) - sb.WriteByte(':') - } else { - sb.WriteByte('[') - sb.WriteString(ipp.IP().String()) // TODO: faster? - sb.WriteString("]:") - } - sb.WriteUint(uint64(ipp.Port())) + // max is the maximum reasonable length of the string we are constructing. + // It's OK to overshoot, as the temp buffer is allocated on the stack. + const max = len("ICMPv6{[ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff%enp5s0]:65535 > [ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff%enp5s0]:65535}") + b := make([]byte, 0, max) + b = append(b, p.IPProto.String()...) + b = append(b, '{') + b = p.Src.AppendTo(b) + b = append(b, ' ', '>', ' ') + b = p.Dst.AppendTo(b) + b = append(b, '}') + return string(b) } // Decode extracts data from the packet in b into q. diff --git a/net/packet/packet_test.go b/net/packet/packet_test.go index ac4fa33f3..a88ab9675 100644 --- a/net/packet/packet_test.go +++ b/net/packet/packet_test.go @@ -378,11 +378,9 @@ func TestParsedString(t *testing.T) { }) } - var sink string allocs := testing.AllocsPerRun(1000, func() { - sink = tests[0].qdecode.String() + sinkString = tests[0].qdecode.String() }) - _ = sink if allocs != 1 { t.Errorf("allocs = %v; want 1", allocs) } @@ -532,3 +530,33 @@ func TestMarshalResponse(t *testing.T) { }) } } + +var sinkString string + +func BenchmarkString(b *testing.B) { + benches := []struct { + name string + buf []byte + }{ + {"tcp4", tcp4PacketBuffer}, + {"tcp6", tcp6RequestBuffer}, + {"udp4", udp4RequestBuffer}, + {"udp6", udp6RequestBuffer}, + {"icmp4", icmp4RequestBuffer}, + {"icmp6", icmp6PacketBuffer}, + {"igmp", igmpPacketBuffer}, + {"unknown", unknownPacketBuffer}, + } + + for _, bench := range benches { + b.Run(bench.name, func(b *testing.B) { + b.ReportAllocs() + var p Parsed + p.Decode(bench.buf) + b.ResetTimer() + for i := 0; i < b.N; i++ { + sinkString = p.String() + } + }) + } +} diff --git a/types/strbuilder/strbuilder.go b/types/strbuilder/strbuilder.go deleted file mode 100644 index 071c03178..000000000 --- a/types/strbuilder/strbuilder.go +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Package strbuilder defines a string builder type that allocates -// less than the standard library's strings.Builder by using a -// sync.Pool, so it doesn't matter if the compiler can't prove that -// the builder doesn't escape into the fmt package, etc. -package strbuilder - -import ( - "bytes" - "strconv" - "sync" -) - -var pool = sync.Pool{ - New: func() interface{} { return new(Builder) }, -} - -type Builder struct { - bb bytes.Buffer - scratch [20]byte // long enough for MinInt64, MaxUint64 - locked bool // in pool, not for use -} - -// Get returns a new or reused string Builder. -func Get() *Builder { - b := pool.Get().(*Builder) - b.bb.Reset() - b.locked = false - return b -} - -// String both returns the Builder's string, and returns the builder -// to the pool. -func (b *Builder) String() string { - if b.locked { - panic("String called twiced on Builder") - } - s := b.bb.String() - b.locked = true - pool.Put(b) - return s -} - -func (b *Builder) WriteByte(v byte) error { - return b.bb.WriteByte(v) -} - -func (b *Builder) WriteString(s string) (int, error) { - return b.bb.WriteString(s) -} - -func (b *Builder) Write(p []byte) (int, error) { - return b.bb.Write(p) -} - -func (b *Builder) WriteInt(v int64) { - b.Write(strconv.AppendInt(b.scratch[:0], v, 10)) -} - -func (b *Builder) WriteUint(v uint64) { - b.Write(strconv.AppendUint(b.scratch[:0], v, 10)) -} - -// Grow grows the buffer's capacity, if necessary, to guarantee space -// for another n bytes. After Grow(n), at least n bytes can be written -// to the buffer without another allocation. If n is negative, Grow -// will panic. If the buffer can't grow it will panic with -// ErrTooLarge. -func (b *Builder) Grow(n int) { - b.bb.Grow(n) -} diff --git a/types/strbuilder/strbuilder_test.go b/types/strbuilder/strbuilder_test.go deleted file mode 100644 index f21d8afac..000000000 --- a/types/strbuilder/strbuilder_test.go +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package strbuilder - -import ( - "math" - "testing" -) - -func TestBuilder(t *testing.T) { - const want = "Hello, world 123 -456!" - bang := []byte("!") - var got string - allocs := testing.AllocsPerRun(1000, func() { - sb := Get() - sb.WriteString("Hello, world ") - sb.WriteUint(123) - sb.WriteByte(' ') - sb.WriteInt(-456) - sb.Write(bang) - got = sb.String() - }) - if got != want { - t.Errorf("got %q; want %q", got, want) - } - if allocs != 1 { - t.Errorf("allocs = %v; want 1", allocs) - } -} - -// Verifies scratch buf is large enough. -func TestIntBounds(t *testing.T) { - const want = "-9223372036854775808 9223372036854775807 18446744073709551615" - var got string - allocs := testing.AllocsPerRun(1000, func() { - sb := Get() - sb.WriteInt(math.MinInt64) - sb.WriteByte(' ') - sb.WriteInt(math.MaxInt64) - sb.WriteByte(' ') - sb.WriteUint(math.MaxUint64) - got = sb.String() - }) - if got != want { - t.Errorf("got %q; want %q", got, want) - } - if allocs != 1 { - t.Errorf("allocs = %v; want 1", allocs) - } -}