Put firewall benchmarks back in

slackhq · Sep 7, 2024 · 93c5a8c · 93c5a8c
1 parent afc41a0
commit 93c5a8c
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 118 deletions.
diff --git a/control_test.go b/control_test.go
@@ -13,6 +13,9 @@ import (
 )
 
 func TestControl_GetHostInfoByVpnIp(t *testing.T) {
+	//TODO: with multiple certificate versions we have a problem with this test
+	// Some certs versions have different characteristics and each version implements their own Copy() func
+	// which means this is not a good place to test for exposing memory
 	l := test.NewLogger()
 	// Special care must be taken to re-use all objects provided to the hostmap and certificate in the expectedInfo object
 	// To properly ensure we are not exposing core memory to the caller
@@ -39,9 +42,7 @@ func TestControl_GetHostInfoByVpnIp(t *testing.T) {
 	vpnIp, ok := netip.AddrFromSlice(ipNet.IP)
 	assert.True(t, ok)
 
-	crt := &dummyCert{
-		//TODO: we need to populate this with fields if we ever enable the memory sharing test again
-	}
+	crt := &dummyCert{}
 	hm.unlockedAddHostInfo(&HostInfo{
 		remote:  remote1,
 		remotes: remotes,
@@ -101,8 +102,7 @@ func TestControl_GetHostInfoByVpnIp(t *testing.T) {
 	// Make sure we don't have any unexpected fields
 	assertFields(t, []string{"VpnIp", "LocalIndex", "RemoteIndex", "RemoteAddrs", "Cert", "MessageCounter", "CurrentRemote", "CurrentRelaysToMe", "CurrentRelaysThroughMe"}, thi)
 	assert.EqualValues(t, &expectedInfo, thi)
-	//TODO: netip.Addr reuses global memory for zone identifiers which breaks our "no reused memory check" here
-	//test.AssertDeepCopyEqual(t, &expectedInfo, thi)
+	test.AssertDeepCopyEqual(t, &expectedInfo, thi)
 
 	// Make sure we don't panic if the host info doesn't have a cert yet
 	assert.NotPanics(t, func() {

diff --git a/e2e/handshakes_test.go b/e2e/handshakes_test.go
@@ -6,6 +6,7 @@ package e2e
 import (
 	"fmt"
 	"net/netip"
+	"slices"
 	"testing"
 	"time"
 
@@ -15,7 +16,6 @@ import (
 	"github.com/slackhq/nebula/header"
 	"github.com/slackhq/nebula/udp"
 	"github.com/stretchr/testify/assert"
-	"golang.org/x/exp/slices"
 	"gopkg.in/yaml.v2"
 )
 

diff --git a/firewall_test.go b/firewall_test.go
@@ -211,150 +211,95 @@ func BenchmarkFirewallTable_match(b *testing.B) {
 
 	b.Run("fail on proto", func(b *testing.B) {
 		// This benchmark is showing us the cost of failing to match the protocol
-		c := &cert.CachedCertificate{}
+		c := &cert.CachedCertificate{
+			Certificate: &dummyCert{},
+		}
 		for n := 0; n < b.N; n++ {
 			assert.False(b, ft.match(firewall.Packet{Protocol: firewall.ProtoUDP}, true, c, cp))
 		}
 	})
 
 	b.Run("pass proto, fail on port", func(b *testing.B) {
 		// This benchmark is showing us the cost of matching a specific protocol but failing to match the port
-		c := &cert.CachedCertificate{}
+		c := &cert.CachedCertificate{
+			Certificate: &dummyCert{},
+		}
 		for n := 0; n < b.N; n++ {
 			assert.False(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 1}, true, c, cp))
 		}
 	})
 
 	b.Run("pass proto, port, fail on local CIDR", func(b *testing.B) {
-		c := &cert.CachedCertificate{}
+		c := &cert.CachedCertificate{
+			Certificate: &dummyCert{},
+		}
 		ip := netip.MustParsePrefix("9.254.254.254/32")
 		for n := 0; n < b.N; n++ {
 			assert.False(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 100, LocalIP: ip.Addr()}, true, c, cp))
 		}
 	})
 
 	b.Run("pass proto, port, any local CIDR, fail all group, name, and cidr", func(b *testing.B) {
-		//TODO:
-		//_, ip, _ := net.ParseCIDR("9.254.254.254/32")
-		//c := &cert.NebulaCertificate{
-		//	Details: cert.NebulaCertificateDetails{
-		//		InvertedGroups: map[string]struct{}{"nope": {}},
-		//		Name:           "nope",
-		//		Ips:            []*net.IPNet{ip},
-		//	},
-		//}
-		//for n := 0; n < b.N; n++ {
-		//	assert.False(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 10}, true, c, cp))
-		//}
+		c := &cert.CachedCertificate{
+			Certificate: &dummyCert{
+				name:     "nope",
+				networks: []netip.Prefix{netip.MustParsePrefix("9.254.254.245/32")},
+			},
+			InvertedGroups: map[string]struct{}{"nope": {}},
+		}
+		for n := 0; n < b.N; n++ {
+			assert.False(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 10}, true, c, cp))
+		}
 	})
 
 	b.Run("pass proto, port, specific local CIDR, fail all group, name, and cidr", func(b *testing.B) {
-		//TODO:
-		//_, ip, _ := net.ParseCIDR("9.254.254.254/32")
-		//c := &cert.NebulaCertificate{
-		//	Details: cert.NebulaCertificateDetails{
-		//		InvertedGroups: map[string]struct{}{"nope": {}},
-		//		Name:           "nope",
-		//		Ips:            []*net.IPNet{ip},
-		//	},
-		//}
-		//for n := 0; n < b.N; n++ {
-		//	assert.False(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 100, LocalIP: pfix.Addr()}, true, c, cp))
-		//}
+		c := &cert.CachedCertificate{
+			Certificate: &dummyCert{
+				name:     "nope",
+				networks: []netip.Prefix{netip.MustParsePrefix("9.254.254.245/32")},
+			},
+			InvertedGroups: map[string]struct{}{"nope": {}},
+		}
+		for n := 0; n < b.N; n++ {
+			assert.False(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 100, LocalIP: pfix.Addr()}, true, c, cp))
+		}
 	})
 
 	b.Run("pass on group on any local cidr", func(b *testing.B) {
-		//TODO:
-		//c := &cert.NebulaCertificate{
-		//	Details: cert.NebulaCertificateDetails{
-		//		InvertedGroups: map[string]struct{}{"good-group": {}},
-		//		Name:           "nope",
-		//	},
-		//}
-		//for n := 0; n < b.N; n++ {
-		//	assert.True(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 10}, true, c, cp))
-		//}
+		c := &cert.CachedCertificate{
+			Certificate: &dummyCert{
+				name: "nope",
+			},
+			InvertedGroups: map[string]struct{}{"good-group": {}},
+		}
+		for n := 0; n < b.N; n++ {
+			assert.True(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 10}, true, c, cp))
+		}
 	})
 
 	b.Run("pass on group on specific local cidr", func(b *testing.B) {
-		//TODO:
-		//c := &cert.NebulaCertificate{
-		//	Details: cert.NebulaCertificateDetails{
-		//		InvertedGroups: map[string]struct{}{"good-group": {}},
-		//		Name:           "nope",
-		//	},
-		//}
-		//for n := 0; n < b.N; n++ {
-		//	assert.True(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 100, LocalIP: pfix.Addr()}, true, c, cp))
-		//}
+		c := &cert.CachedCertificate{
+			Certificate: &dummyCert{
+				name: "nope",
+			},
+			InvertedGroups: map[string]struct{}{"good-group": {}},
+		}
+		for n := 0; n < b.N; n++ {
+			assert.True(b, ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 100, LocalIP: pfix.Addr()}, true, c, cp))
+		}
 	})
 
 	b.Run("pass on name", func(b *testing.B) {
-		//TODO:
-		//c := &cert.NebulaCertificate{
-		//	Details: cert.NebulaCertificateDetails{
-		//		InvertedGroups: map[string]struct{}{"nope": {}},
-		//		Name:           "good-host",
-		//	},
-		//}
-		//for n := 0; n < b.N; n++ {
-		//	ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 10}, true, c, cp)
-		//}
+		c := &cert.CachedCertificate{
+			Certificate: &dummyCert{
+				name: "good-host",
+			},
+			InvertedGroups: map[string]struct{}{"nope": {}},
+		}
+		for n := 0; n < b.N; n++ {
+			ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 10}, true, c, cp)
+		}
 	})
-	//
-	//b.Run("pass on ip", func(b *testing.B) {
-	//	ip := iputil.Ip2VpnIp(net.IPv4(172, 1, 1, 1))
-	//	c := &cert.NebulaCertificate{
-	//		Details: cert.NebulaCertificateDetails{
-	//			InvertedGroups: map[string]struct{}{"nope": {}},
-	//			Name:           "good-host",
-	//		},
-	//	}
-	//	for n := 0; n < b.N; n++ {
-	//		ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 10, RemoteIP: ip}, true, c, cp)
-	//	}
-	//})
-	//
-	//b.Run("pass on local ip", func(b *testing.B) {
-	//	ip := iputil.Ip2VpnIp(net.IPv4(172, 1, 1, 1))
-	//	c := &cert.NebulaCertificate{
-	//		Details: cert.NebulaCertificateDetails{
-	//			InvertedGroups: map[string]struct{}{"nope": {}},
-	//			Name:           "good-host",
-	//		},
-	//	}
-	//	for n := 0; n < b.N; n++ {
-	//		ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 10, LocalIP: ip}, true, c, cp)
-	//	}
-	//})
-	//
-	//_ = ft.TCP.addRule(0, 0, []string{"good-group"}, "good-host", n, n, "", "")
-	//
-	//b.Run("pass on ip with any port", func(b *testing.B) {
-	//	ip := iputil.Ip2VpnIp(net.IPv4(172, 1, 1, 1))
-	//	c := &cert.NebulaCertificate{
-	//		Details: cert.NebulaCertificateDetails{
-	//			InvertedGroups: map[string]struct{}{"nope": {}},
-	//			Name:           "good-host",
-	//		},
-	//	}
-	//	for n := 0; n < b.N; n++ {
-	//		ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 100, RemoteIP: ip}, true, c, cp)
-	//	}
-	//})
-	//
-	//b.Run("pass on local ip with any port", func(b *testing.B) {
-	//	ip := iputil.Ip2VpnIp(net.IPv4(172, 1, 1, 1))
-	//	c := &cert.NebulaCertificate{
-	//		Details: cert.NebulaCertificateDetails{
-	//			InvertedGroups: map[string]struct{}{"nope": {}},
-	//			Name:           "good-host",
-	//		},
-	//	}
-	//	for n := 0; n < b.N; n++ {
-	//		ft.match(firewall.Packet{Protocol: firewall.ProtoTCP, LocalPort: 100, LocalIP: ip}, true, c, cp)
-	//	}
-	//})
 }
 
 func TestFirewall_Drop2(t *testing.T) {

diff --git a/handshake_manager.go b/handshake_manager.go
@@ -7,14 +7,14 @@ import (
 	"encoding/binary"
 	"errors"
 	"net/netip"
+	"slices"
 	"sync"
 	"time"
 
 	"github.com/rcrowley/go-metrics"
 	"github.com/sirupsen/logrus"
 	"github.com/slackhq/nebula/header"
 	"github.com/slackhq/nebula/udp"
-	"golang.org/x/exp/slices"
 )
 
 const (