net/packet: remove the custom IP4/IP6 types in favor of netaddr.IP.

Upstream netaddr has a change that makes it alloc-free, so it's safe to use in hot codepaths. This gets rid of one of the many IP types in our codebase. Performance is currently worse across the board. This is likely due in part to netaddr.IP being a larger value type (4b -> 24b for IPv4, 16b -> 24b for IPv6), and in other part due to missing low-hanging fruit optimizations in netaddr. However, the regression is less bad than it looks at first glance, because we'd micro-optimized packet.IP* in the past few weeks. This change drops us back to roughly where we were at the 1.2 release, but with the benefit of a significant code and architectural simplification. name old time/op new time/op delta pkg:tailscale.com/net/packet goos:linux goarch:amd64 Decode/tcp4-8 12.2ns ± 5% 29.7ns ± 2% +142.32% (p=0.008 n=5+5) Decode/tcp6-8 12.6ns ± 3% 65.1ns ± 2% +418.47% (p=0.008 n=5+5) Decode/udp4-8 11.8ns ± 3% 30.5ns ± 2% +157.94% (p=0.008 n=5+5) Decode/udp6-8 27.1ns ± 1% 65.7ns ± 2% +142.36% (p=0.016 n=4+5) Decode/icmp4-8 24.6ns ± 2% 30.5ns ± 2% +23.65% (p=0.016 n=4+5) Decode/icmp6-8 22.9ns ±51% 65.5ns ± 2% +186.19% (p=0.008 n=5+5) Decode/igmp-8 18.1ns ±44% 30.2ns ± 1% +66.89% (p=0.008 n=5+5) Decode/unknown-8 20.8ns ± 1% 10.6ns ± 9% -49.11% (p=0.016 n=4+5) pkg:tailscale.com/wgengine/filter goos:linux goarch:amd64 Filter/icmp4-8 30.5ns ± 1% 77.9ns ± 3% +155.01% (p=0.008 n=5+5) Filter/tcp4_syn_in-8 43.7ns ± 3% 123.0ns ± 3% +181.72% (p=0.008 n=5+5) Filter/tcp4_syn_out-8 24.5ns ± 2% 45.7ns ± 6% +86.22% (p=0.008 n=5+5) Filter/udp4_in-8 64.8ns ± 1% 210.0ns ± 2% +223.87% (p=0.008 n=5+5) Filter/udp4_out-8 119ns ± 0% 278ns ± 0% +133.78% (p=0.016 n=4+5) Filter/icmp6-8 40.3ns ± 2% 204.4ns ± 4% +407.70% (p=0.008 n=5+5) Filter/tcp6_syn_in-8 35.3ns ± 3% 199.2ns ± 2% +464.95% (p=0.008 n=5+5) Filter/tcp6_syn_out-8 32.8ns ± 2% 81.0ns ± 2% +147.10% (p=0.008 n=5+5) Filter/udp6_in-8 106ns ± 2% 290ns ± 2% +174.48% (p=0.008 n=5+5) Filter/udp6_out-8 184ns ± 2% 314ns ± 3% +70.43% (p=0.016 n=4+5) pkg:tailscale.com/wgengine/tstun goos:linux goarch:amd64 Write-8 9.02ns ± 3% 8.92ns ± 1% ~ (p=0.421 n=5+5) name old alloc/op new alloc/op delta pkg:tailscale.com/net/packet goos:linux goarch:amd64 Decode/tcp4-8 0.00B 0.00B ~ (all equal) Decode/tcp6-8 0.00B 0.00B ~ (all equal) Decode/udp4-8 0.00B 0.00B ~ (all equal) Decode/udp6-8 0.00B 0.00B ~ (all equal) Decode/icmp4-8 0.00B 0.00B ~ (all equal) Decode/icmp6-8 0.00B 0.00B ~ (all equal) Decode/igmp-8 0.00B 0.00B ~ (all equal) Decode/unknown-8 0.00B 0.00B ~ (all equal) pkg:tailscale.com/wgengine/filter goos:linux goarch:amd64 Filter/icmp4-8 0.00B 0.00B ~ (all equal) Filter/tcp4_syn_in-8 0.00B 0.00B ~ (all equal) Filter/tcp4_syn_out-8 0.00B 0.00B ~ (all equal) Filter/udp4_in-8 0.00B 0.00B ~ (all equal) Filter/udp4_out-8 16.0B ± 0% 64.0B ± 0% +300.00% (p=0.008 n=5+5) Filter/icmp6-8 0.00B 0.00B ~ (all equal) Filter/tcp6_syn_in-8 0.00B 0.00B ~ (all equal) Filter/tcp6_syn_out-8 0.00B 0.00B ~ (all equal) Filter/udp6_in-8 0.00B 0.00B ~ (all equal) Filter/udp6_out-8 48.0B ± 0% 64.0B ± 0% +33.33% (p=0.008 n=5+5) name old allocs/op new allocs/op delta pkg:tailscale.com/net/packet goos:linux goarch:amd64 Decode/tcp4-8 0.00 0.00 ~ (all equal) Decode/tcp6-8 0.00 0.00 ~ (all equal) Decode/udp4-8 0.00 0.00 ~ (all equal) Decode/udp6-8 0.00 0.00 ~ (all equal) Decode/icmp4-8 0.00 0.00 ~ (all equal) Decode/icmp6-8 0.00 0.00 ~ (all equal) Decode/igmp-8 0.00 0.00 ~ (all equal) Decode/unknown-8 0.00 0.00 ~ (all equal) pkg:tailscale.com/wgengine/filter goos:linux goarch:amd64 Filter/icmp4-8 0.00 0.00 ~ (all equal) Filter/tcp4_syn_in-8 0.00 0.00 ~ (all equal) Filter/tcp4_syn_out-8 0.00 0.00 ~ (all equal) Filter/udp4_in-8 0.00 0.00 ~ (all equal) Filter/udp4_out-8 1.00 ± 0% 1.00 ± 0% ~ (all equal) Filter/icmp6-8 0.00 0.00 ~ (all equal) Filter/tcp6_syn_in-8 0.00 0.00 ~ (all equal) Filter/tcp6_syn_out-8 0.00 0.00 ~ (all equal) Filter/udp6_in-8 0.00 0.00 ~ (all equal) Filter/udp6_out-8 1.00 ± 0% 1.00 ± 0% ~ (all equal) Signed-off-by: David Anderson <danderson@tailscale.com>
2025-10-23 17:18:42 +00:00 · 2020-12-19 16:43:25 -08:00
parent d0baece5fa
commit cb96b14bf4
13 changed files with 323 additions and 827 deletions
--- a/wgengine/filter/filter.go
+++ b/wgengine/filter/filter.go
@@ -25,45 +25,33 @@ type Filter struct {
 	// tailscale must have a destination within local4 or local6,
 	// regardless of the policy filter below. Zero values reject
 	// all incoming traffic.
-	local4 []net4
-	local6 []net6
+	local4 []netaddr.IPPrefix
+	local6 []netaddr.IPPrefix
 	// matches4 and matches6 are lists of match->action rules
 	// applied to all packets arriving over tailscale
 	// tunnels. Matches are checked in order, and processing stops
 	// at the first matching rule. The default policy if no rules
 	// match is to drop the packet.
-	matches4 matches4
-	matches6 matches6
+	matches4 matches
+	matches6 matches
 	// state is the connection tracking state attached to this
 	// filter. It is used to allow incoming traffic that is a response
 	// to an outbound connection that this node made, even if those
 	// incoming packets don't get accepted by matches above.
-	state4 *filterState
-	state6 *filterState
+	state *filterState
 }

-// tuple4 is a 4-tuple of source and destination IPv4 and port. It's
-// used as a lookup key in filterState.
-type tuple4 struct {
-	SrcIP   packet.IP4
-	DstIP   packet.IP4
-	SrcPort uint16
-	DstPort uint16
-}
-
-// tuple6 is a 4-tuple of source and destination IPv6 and port. It's
-// used as a lookup key in filterState.
-type tuple6 struct {
-	SrcIP   packet.IP6
-	DstIP   packet.IP6
-	SrcPort uint16
-	DstPort uint16
+// tuple is a 4-tuple of source and destination IP and port. It's used
+// as a lookup key in filterState.
+type tuple struct {
+	Src netaddr.IPPort
+	Dst netaddr.IPPort
 }

 // filterState is a state cache of past seen packets.
 type filterState struct {
 	mu  sync.Mutex
-	lru *lru.Cache // of tuple4 or tuple6
+	lru *lru.Cache // of tuple
 }

 // lruMax is the size of the LRU cache in filterState.
@@ -148,30 +136,58 @@ func NewAllowNone(logf logger.Logf) *Filter {
 // shares state with the previous one, to enable changing rules at
 // runtime without breaking existing stateful flows.
 func New(matches []Match, localNets []netaddr.IPPrefix, shareStateWith *Filter, logf logger.Logf) *Filter {
-	var state4, state6 *filterState
+	var state *filterState
 	if shareStateWith != nil {
-		state4 = shareStateWith.state4
-		state6 = shareStateWith.state6
+		state = shareStateWith.state
 	} else {
-		state4 = &filterState{
-			lru: lru.New(lruMax),
-		}
-		state6 = &filterState{
+		state = &filterState{
 			lru: lru.New(lruMax),
 		}
 	}
 	f := &Filter{
 		logf:     logf,
-		matches4: newMatches4(matches),
-		matches6: newMatches6(matches),
-		local4:   nets4FromIPPrefixes(localNets),
-		local6:   nets6FromIPPrefixes(localNets),
-		state4:   state4,
-		state6:   state6,
+		matches4: matchesFamily(matches, netaddr.IP.Is4),
+		matches6: matchesFamily(matches, netaddr.IP.Is6),
+		local4:   netsFamily(localNets, netaddr.IP.Is4),
+		local6:   netsFamily(localNets, netaddr.IP.Is6),
+		state:    state,
 	}
 	return f
 }

+func netsFamily(nets []netaddr.IPPrefix, keep func(netaddr.IP) bool) []netaddr.IPPrefix {
+	var ret []netaddr.IPPrefix
+	for _, net := range nets {
+		if keep(net.IP) {
+			ret = append(ret, net)
+		}
+	}
+	return ret
+}
+
+// matchesFamily returns the subset of ms for which keep(srcNet.IP)
+// and keep(dstNet.IP) are both true.
+func matchesFamily(ms matches, keep func(netaddr.IP) bool) matches {
+	var ret matches
+	for _, m := range ms {
+		var retm Match
+		for _, src := range m.Srcs {
+			if keep(src.IP) {
+				retm.Srcs = append(retm.Srcs, src)
+			}
+		}
+		for _, dst := range m.Dsts {
+			if keep(dst.Net.IP) {
+				retm.Dsts = append(retm.Dsts, dst)
+			}
+		}
+		if len(retm.Srcs) > 0 && len(retm.Dsts) > 0 {
+			ret = append(ret, retm)
+		}
+	}
+	return ret
+}
+
 func maybeHexdump(flag RunFlags, b []byte) string {
 	if flag == 0 {
 		return ""
@@ -229,19 +245,17 @@ func (f *Filter) CheckTCP(srcIP, dstIP netaddr.IP, dstPort uint16) Response {
 		return Drop
 	case srcIP.Is4():
 		pkt.IPVersion = 4
-		pkt.SrcIP4 = packet.IP4FromNetaddr(srcIP)
-		pkt.DstIP4 = packet.IP4FromNetaddr(dstIP)
 	case srcIP.Is6():
 		pkt.IPVersion = 6
-		pkt.SrcIP6 = packet.IP6FromNetaddr(srcIP)
-		pkt.DstIP6 = packet.IP6FromNetaddr(dstIP)
 	default:
 		panic("unreachable")
 	}
+	pkt.Src.IP = srcIP
+	pkt.Dst.IP = dstIP
 	pkt.IPProto = packet.TCP
 	pkt.TCPFlags = packet.TCPSyn
-	pkt.SrcPort = 0
-	pkt.DstPort = dstPort
+	pkt.Src.Port = 0
+	pkt.Dst.Port = dstPort

 	return f.RunIn(pkt, 0)
 }
@@ -287,7 +301,7 @@ func (f *Filter) runIn4(q *packet.Parsed) (r Response, why string) {
 	// A compromised peer could try to send us packets for
 	// destinations we didn't explicitly advertise. This check is to
 	// prevent that.
-	if !ip4InList(q.DstIP4, f.local4) {
+	if !ipInList(q.Dst.IP, f.local4) {
 		return Drop, "destination not allowed"
 	}

@@ -320,11 +334,11 @@ func (f *Filter) runIn4(q *packet.Parsed) (r Response, why string) {
 			return Accept, "tcp ok"
 		}
 	case packet.UDP:
-		t := tuple4{q.SrcIP4, q.DstIP4, q.SrcPort, q.DstPort}
+		t := tuple{q.Src, q.Dst}

-		f.state4.mu.Lock()
-		_, ok := f.state4.lru.Get(t)
-		f.state4.mu.Unlock()
+		f.state.mu.Lock()
+		_, ok := f.state.lru.Get(t)
+		f.state.mu.Unlock()

 		if ok {
 			return Accept, "udp cached"
@@ -342,7 +356,7 @@ func (f *Filter) runIn6(q *packet.Parsed) (r Response, why string) {
 	// A compromised peer could try to send us packets for
 	// destinations we didn't explicitly advertise. This check is to
 	// prevent that.
-	if !ip6InList(q.DstIP6, f.local6) {
+	if !ipInList(q.Dst.IP, f.local6) {
 		return Drop, "destination not allowed"
 	}

@@ -375,11 +389,11 @@ func (f *Filter) runIn6(q *packet.Parsed) (r Response, why string) {
 			return Accept, "tcp ok"
 		}
 	case packet.UDP:
-		t := tuple6{q.SrcIP6, q.DstIP6, q.SrcPort, q.DstPort}
+		t := tuple{q.Src, q.Dst}

-		f.state6.mu.Lock()
-		_, ok := f.state6.lru.Get(t)
-		f.state6.mu.Unlock()
+		f.state.mu.Lock()
+		_, ok := f.state.lru.Get(t)
+		f.state.mu.Unlock()

 		if ok {
 			return Accept, "udp cached"
@@ -399,20 +413,11 @@ func (f *Filter) runOut(q *packet.Parsed) (r Response, why string) {
 		return Accept, "ok out"
 	}

-	switch q.IPVersion {
-	case 4:
-		t := tuple4{q.DstIP4, q.SrcIP4, q.DstPort, q.SrcPort}
-		var ti interface{} = t // allocate once, rather than twice inside mutex
-		f.state4.mu.Lock()
-		f.state4.lru.Add(ti, ti)
-		f.state4.mu.Unlock()
-	case 6:
-		t := tuple6{q.DstIP6, q.SrcIP6, q.DstPort, q.SrcPort}
-		var ti interface{} = t // allocate once, rather than twice inside mutex
-		f.state6.mu.Lock()
-		f.state6.lru.Add(ti, ti)
-		f.state6.mu.Unlock()
-	}
+	t := tuple{q.Dst, q.Src}
+	var ti interface{} = t // allocate once, rather than twice inside mutex
+	f.state.mu.Lock()
+	f.state.lru.Add(ti, ti)
+	f.state.mu.Unlock()
 	return Accept, "ok out"
 }

@@ -436,6 +441,8 @@ func (d direction) String() string {
 	}
 }

+var gcpDNSAddr = netaddr.IPv4(169, 254, 169, 254)
+
 // pre runs the direction-agnostic filter logic. dir is only used for
 // logging.
 func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) Response {
@@ -448,25 +455,13 @@ func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) Response {
 		return Drop
 	}

-	switch q.IPVersion {
-	case 4:
-		if q.DstIP4.IsMulticast() {
-			f.logRateLimit(rf, q, dir, Drop, "multicast")
-			return Drop
-		}
-		if q.DstIP4.IsMostLinkLocalUnicast() {
-			f.logRateLimit(rf, q, dir, Drop, "link-local-unicast")
-			return Drop
-		}
-	case 6:
-		if q.DstIP6.IsMulticast() {
-			f.logRateLimit(rf, q, dir, Drop, "multicast")
-			return Drop
-		}
-		if q.DstIP6.IsLinkLocalUnicast() {
-			f.logRateLimit(rf, q, dir, Drop, "link-local-unicast")
-			return Drop
-		}
+	if q.Dst.IP.IsMulticast() {
+		f.logRateLimit(rf, q, dir, Drop, "multicast")
+		return Drop
+	}
+	if q.Dst.IP.IsLinkLocalUnicast() && q.Dst.IP != gcpDNSAddr {
+		f.logRateLimit(rf, q, dir, Drop, "link-local-unicast")
+		return Drop
 	}

 	switch q.IPProto {
@@ -493,12 +488,5 @@ func omitDropLogging(p *packet.Parsed, dir direction) bool {
 		return false
 	}

-	switch p.IPVersion {
-	case 4:
-		return p.DstIP4.IsMulticast() || p.DstIP4.IsMostLinkLocalUnicast() || p.IPProto == packet.IGMP
-	case 6:
-		return p.DstIP6.IsMulticast() || p.DstIP6.IsLinkLocalUnicast()
-	default:
-		return false
-	}
+	return p.Dst.IP.IsMulticast() || (p.Dst.IP.IsLinkLocalUnicast() && p.Dst.IP != gcpDNSAddr) || p.IPProto == packet.IGMP
 }