Skip to content

Commit

Permalink
fqdn: Fix notifyOnDNSMsg benchmark
Browse files Browse the repository at this point in the history 
The design of BenchmarkNotifyOnDNSMsg was flawed due to the usage of b.N
as benchmark input size. b.N is used by the Go benchmarking framework to
find a reliable timing for the code under test. Changing the amount of
work done at each benchmark call leads to unreliable numbers.  For more
info refer to the Go documentation:

	The benchmark function must run the target code b.N times. During
	benchmark execution, b.N is adjusted until the benchmark function
	lasts long enough to be timed reliably.

And the section `Traps for young players` in the blog post
https://dave.cheney.net/2013/06/30/how-to-write-benchmarks-in-go

To fix it, the benchmark has been updated to setup a reasonable fixed
number of endpoints.

Also, since the code was behind the latest development in the policy and
fqdn subsystems, it has been updated to run correctly. Specifically:

- the global LocalNodeStore is set once at startup
- the FQDN selectors are added to the selector cache with a dummy user
- the daemon context is properly initialized
- the FQDN related global opts are set to their defaults to avoid noisy
  logs

Example run:

goos: linux
goarch: amd64
pkg: github.com/cilium/cilium/daemon/cmd
cpu: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
BenchmarkNotifyOnDNSMsg-8   	      21	  47992143 ns/op	30757230 B/op	  357646 allocs/op
BenchmarkNotifyOnDNSMsg-8   	      24	  48773334 ns/op	30590877 B/op	  355639 allocs/op
BenchmarkNotifyOnDNSMsg-8   	      22	  47940489 ns/op	30719252 B/op	  358088 allocs/op
BenchmarkNotifyOnDNSMsg-8   	      24	  48019454 ns/op	30364074 B/op	  353470 allocs/op
BenchmarkNotifyOnDNSMsg-8   	      24	  66607283 ns/op	30634838 B/op	  356352 allocs/op
PASS

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
  • Loading branch information
@pippolo84 @julianwiedmann
pippolo84 authored and julianwiedmann committed May 21, 2024
1 parent a259c3d commit db5fcdc
Showing 1 changed file with 70 additions and 46 deletions.
116 changes: 70 additions & 46 deletions daemon/cmd/fqdn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"github.com/cilium/cilium/pkg/identity"
"github.com/cilium/cilium/pkg/ipcache"
"github.com/cilium/cilium/pkg/lock"
"github.com/cilium/cilium/pkg/node"
"github.com/cilium/cilium/pkg/option"
"github.com/cilium/cilium/pkg/policy"
"github.com/cilium/cilium/pkg/policy/api"
"github.com/cilium/cilium/pkg/proxy/accesslog"
Expand All @@ -37,13 +39,32 @@ type DaemonFQDNSuite struct {
d *Daemon
}

var notifyOnDNSMsgBenchSetup sync.Once

func setupDaemonFQDNSuite(tb testing.TB) *DaemonFQDNSuite {
testutils.IntegrationTest(tb)

re.InitRegexCompileLRU(defaults.FQDNRegexCompileLRUSize)
// We rely on a sync.Once to complete the benchmark setup that initialize
// read-only global status.
// Also, node.SetTestLocalNodeStore() panics if it called more than once.
notifyOnDNSMsgBenchSetup.Do(func() {
// set FQDN related options to defaults in order to avoid a flood of warnings
option.Config.DNSProxyLockCount = defaults.DNSProxyLockCount
option.Config.DNSProxyLockTimeout = defaults.DNSProxyLockTimeout
option.Config.FQDNProxyResponseMaxDelay = defaults.FQDNProxyResponseMaxDelay

// Set local node store as it is accessed by NewLogRecord to get node IPv4
node.SetTestLocalNodeStore()

re.InitRegexCompileLRU(defaults.FQDNRegexCompileLRUSize)
logger.SetEndpointInfoRegistry(&dummyInfoRegistry{})
})

ds := &DaemonFQDNSuite{}
d := &Daemon{}
d.ctx = context.Background()
d.policy = policy.NewPolicyRepository(d.identityAllocator, nil, nil, nil)
d.endpointManager = endpointmanager.New(&dummyEpSyncher{}, nil, nil)
d.ipcache = ipcache.NewIPCache(&ipcache.Configuration{
Context: context.TODO(),
IdentityAllocator: testidentity.NewMockIdentityAllocator(nil),
Expand All @@ -56,13 +77,10 @@ func setupDaemonFQDNSuite(tb testing.TB) *DaemonFQDNSuite {
UpdateSelectors: d.updateSelectors,
IPCache: d.ipcache,
})
d.endpointManager = endpointmanager.New(&dummyEpSyncher{}, nil, nil)
d.policy.GetSelectorCache().SetLocalIdentityNotifier(d.dnsNameManager)

ds.d = d

logger.SetEndpointInfoRegistry(&dummyInfoRegistry{})

return ds
}

Expand All @@ -71,13 +89,17 @@ type dummyInfoRegistry struct{}
func (*dummyInfoRegistry) FillEndpointInfo(info *accesslog.EndpointInfo, addr netip.Addr, id identity.NumericIdentity) {
}

type dummySelectorCacheUser struct{}

func (d *dummySelectorCacheUser) IdentitySelectionUpdated(selector policy.CachedSelector, added, deleted []identity.NumericIdentity) {
}

// BenchmarkNotifyOnDNSMsg stresses the main callback function for the DNS
// proxy path, which is called on every DNS request and response.
func BenchmarkNotifyOnDNSMsg(b *testing.B) {
ds := setupDaemonFQDNSuite(b)

var (
nameManager = ds.d.dnsNameManager
ciliumIOSel = api.FQDNSelector{MatchName: "cilium.io"}
ciliumIOSelMatchPattern = api.FQDNSelector{MatchPattern: "*cilium.io."}
ebpfIOSel = api.FQDNSelector{MatchName: "ebpf.io"}
Expand All @@ -92,21 +114,22 @@ func BenchmarkNotifyOnDNSMsg(b *testing.B) {
)

// Register rules (simulates applied policies).
dscu := &dummySelectorCacheUser{}
selectorsToAdd := api.FQDNSelectorSlice{ciliumIOSel, ciliumIOSelMatchPattern, ebpfIOSel}
nameManager.Lock()
for _, sel := range selectorsToAdd {
nameManager.RegisterForIPUpdatesLocked(sel)
ds.d.policy.GetSelectorCache().AddFQDNSelector(dscu, nil, sel)
}
nameManager.Unlock()

const nEndpoints int = 1024

// Initialize the endpoints.
endpoints := make([]*endpoint.Endpoint, b.N)
endpoints := make([]*endpoint.Endpoint, nEndpoints)
for i := range endpoints {
endpoints[i] = &endpoint.Endpoint{
ID: uint16(b.N % 65000),
IPv4: netip.MustParseAddr(fmt.Sprintf("10.96.%d.%d", (b.N>>16)%8, b.N%256)),
ID: uint16(i),
IPv4: netip.MustParseAddr(fmt.Sprintf("10.96.%d.%d", i/256, i%256)),
SecurityIdentity: &identity.Identity{
ID: identity.NumericIdentity(b.N % int(identity.GetMaximumAllocationIdentity())),
ID: identity.NumericIdentity(i % int(identity.GetMaximumAllocationIdentity())),
},
DNSZombies: &fqdn.DNSZombieMappings{
Mutex: lock.Mutex{},
Expand All @@ -121,39 +144,40 @@ func BenchmarkNotifyOnDNSMsg(b *testing.B) {
// Simulate parallel DNS responses from the upstream DNS for cilium.io and
// ebpf.io, done by every endpoint.
for i := 0; i < b.N; i++ {
wg.Add(1)
go func(ep *endpoint.Endpoint) {
defer wg.Done()
// Using a hardcoded string representing endpoint IP:port as this
// parameter is only used in logging. Not using the endpoint's IP
// so we don't spend any time in the benchmark on converting from
// net.IP to string.
require.Nil(b, ds.d.notifyOnDNSMsg(time.Now(), ep, "10.96.64.8:12345", 0, "10.96.64.1:53", &ciliumdns.Msg{
MsgHdr: ciliumdns.MsgHdr{
Response: true,
},
Question: []ciliumdns.Question{{
Name: dns.FQDN("cilium.io"),
}},
Answer: []ciliumdns.RR{&ciliumdns.A{
Hdr: ciliumdns.RR_Header{Name: dns.FQDN("cilium.io")},
A: ciliumDNSRecord[dns.FQDN("cilium.io")].IPs[0],
}}}, "udp", true, &dnsproxy.ProxyRequestContext{}))

require.Nil(b, ds.d.notifyOnDNSMsg(time.Now(), ep, "10.96.64.4:54321", 0, "10.96.64.1:53", &ciliumdns.Msg{
MsgHdr: ciliumdns.MsgHdr{
Response: true,
},
Compress: false,
Question: []ciliumdns.Question{{
Name: dns.FQDN("ebpf.io"),
}},
Answer: []ciliumdns.RR{&ciliumdns.A{
Hdr: ciliumdns.RR_Header{Name: dns.FQDN("ebpf.io")},
A: ebpfDNSRecord[dns.FQDN("ebpf.io")].IPs[0],
}}}, "udp", true, &dnsproxy.ProxyRequestContext{}))
}(endpoints[i%len(endpoints)])
for _, ep := range endpoints {
wg.Add(1)
go func() {
defer wg.Done()
// Using a hardcoded string representing endpoint IP:port as this
// parameter is only used in logging. Not using the endpoint's IP
// so we don't spend any time in the benchmark on converting from
// net.IP to string.
require.Nil(b, ds.d.notifyOnDNSMsg(time.Now(), ep, "10.96.64.8:12345", 0, "10.96.64.1:53", &ciliumdns.Msg{
MsgHdr: ciliumdns.MsgHdr{
Response: true,
},
Question: []ciliumdns.Question{{
Name: dns.FQDN("cilium.io"),
}},
Answer: []ciliumdns.RR{&ciliumdns.A{
Hdr: ciliumdns.RR_Header{Name: dns.FQDN("cilium.io")},
A: ciliumDNSRecord[dns.FQDN("cilium.io")].IPs[0],
}}}, "udp", true, &dnsproxy.ProxyRequestContext{}))

require.Nil(b, ds.d.notifyOnDNSMsg(time.Now(), ep, "10.96.64.4:54321", 0, "10.96.64.1:53", &ciliumdns.Msg{
MsgHdr: ciliumdns.MsgHdr{
Response: true,
},
Compress: false,
Question: []ciliumdns.Question{{
Name: dns.FQDN("ebpf.io"),
}},
Answer: []ciliumdns.RR{&ciliumdns.A{
Hdr: ciliumdns.RR_Header{Name: dns.FQDN("ebpf.io")},
A: ebpfDNSRecord[dns.FQDN("ebpf.io")].IPs[0],
}}}, "udp", true, &dnsproxy.ProxyRequestContext{}))
}()
}
wg.Wait()
}

wg.Wait()
}

0 comments on commit db5fcdc

Please sign in to comment.