Skip to content

Commit

Permalink
pkg/fqdn: use LRU in FQDN policy calculation
Browse files Browse the repository at this point in the history
Using an LRU for the memory-intensive operations such as regex.Compile
bring some benefits as presented in the following benchmarks [1]. These
benchmarks assumed there were 2 CNPs that shared 100 FQDN
`matchPattern` on a node with 20 endpoints.

[1]
```
name                             old time/op    new time/op    delta
_perEPAllow_setPortRulesForID-8    13.9ms ± 6%     1.2ms ±63%  -91.10%  (p=0.008 n=5+5)

name                             old alloc/op   new alloc/op   delta
_perEPAllow_setPortRulesForID-8    17.4MB ± 0%     0.6MB ± 0%  -96.56%  (p=0.008 n=5+5)

name                             old allocs/op  new allocs/op  delta
_perEPAllow_setPortRulesForID-8     42.8k ± 0%      8.1k ± 0%  -81.13%  (p=0.008 n=5+5)
```

Signed-off-by: André Martins <andre@cilium.io>
  • Loading branch information
aanm authored and aditighag committed Sep 2, 2021
1 parent 30f3c2a commit 42417c7
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 8 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ require (
github.com/go-openapi/swag v0.19.15
github.com/go-openapi/validate v0.20.2
github.com/gogo/protobuf v1.3.2
github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e
github.com/golang/protobuf v1.5.2
github.com/google/go-cmp v0.5.6
github.com/google/gopacket v1.1.19
Expand Down Expand Up @@ -152,7 +153,6 @@ require (
github.com/go-openapi/jsonreference v0.19.5 // indirect
github.com/go-stack/stack v1.8.0 // indirect
github.com/gobuffalo/flect v0.2.0 // indirect
github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect
github.com/golang/snappy v0.0.1 // indirect
github.com/google/gofuzz v1.1.0 // indirect
github.com/googleapis/gnostic v0.4.1 // indirect
Expand Down
27 changes: 21 additions & 6 deletions pkg/fqdn/dnsproxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/cilium/cilium/pkg/policy/api"
"github.com/cilium/cilium/pkg/spanstat"

"github.com/golang/groupcache/lru"
"github.com/miekg/dns"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -137,6 +138,11 @@ type DNSProxy struct {
// rejectReply is the OPCode send from the DNS-proxy to the endpoint if the
// DNS request is invalid
rejectReply int32

// regexCompileLRU contains an LRU cache for the regex.Compile of rules.
// Keeping an LRU cache avoids excessive memory allocations when compiling
// regex strings via regex.Compile.
regexCompileLRU *lru.Cache
}

// perEPAllow maps EndpointIDs to ports + selectors + rules
Expand Down Expand Up @@ -259,7 +265,7 @@ func (p *DNSProxy) RemoveRestoredRules(endpointID uint16) {
// setPortRulesForID sets the matching rules for endpointID and destPort for
// later lookups. It converts newRules into a unified regexp that can be reused
// later.
func (allow perEPAllow) setPortRulesForID(endpointID uint64, destPort uint16, newRules policy.L7DataMap) error {
func (allow perEPAllow) setPortRulesForID(lru *lru.Cache, endpointID uint64, destPort uint16, newRules policy.L7DataMap) error {
// This is the delete case
if len(newRules) == 0 {
epPorts := allow[endpointID]
Expand Down Expand Up @@ -288,11 +294,19 @@ func (allow perEPAllow) setPortRulesForID(endpointID uint64, destPort uint16, ne
reStrings = append(reStrings, "("+dnsPatternAsRE+")")
}
}
re, err := regexp.Compile(strings.Join(reStrings, "|"))
if err != nil {
return err
mp := strings.Join(reStrings, "|")
rei, ok := lru.Get(mp)
if ok {
re := rei.(*regexp.Regexp)
newRE[selector] = re
} else {
re, err := regexp.Compile(mp)
if err != nil {
return err
}
lru.Add(mp, re)
newRE[selector] = re
}
newRE[selector] = re
}

epPorts, exist := allow[endpointID]
Expand Down Expand Up @@ -380,6 +394,7 @@ func StartDNSProxy(address string, port uint16, enableDNSCompression bool, maxRe
restoredEPs: make(restoredEPs),
EnableDNSCompression: enableDNSCompression,
maxIPsPerRestoredDNSRule: maxRestoreDNSIPs,
regexCompileLRU: lru.New(128),
}
atomic.StoreInt32(&p.rejectReply, dns.RcodeRefused)

Expand Down Expand Up @@ -455,7 +470,7 @@ func (p *DNSProxy) UpdateAllowed(endpointID uint64, destPort uint16, newRules po
p.Lock()
defer p.Unlock()

err := p.allowed.setPortRulesForID(endpointID, destPort, newRules)
err := p.allowed.setPortRulesForID(p.regexCompileLRU, endpointID, destPort, newRules)
if err == nil {
// Rules were updated based on policy, remove restored rules
p.removeRestoredRulesLocked(endpointID)
Expand Down
63 changes: 62 additions & 1 deletion pkg/fqdn/dnsproxy/proxy_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2018-2020 Authors of Cilium
// Copyright 2018-2021 Authors of Cilium

//go:build privileged_tests
// +build privileged_tests
Expand Down Expand Up @@ -36,6 +36,7 @@ import (
"github.com/cilium/cilium/pkg/source"
"github.com/cilium/cilium/pkg/testutils/allocator"

"github.com/golang/groupcache/lru"
"github.com/miekg/dns"
. "gopkg.in/check.v1"
)
Expand Down Expand Up @@ -972,3 +973,63 @@ func (s *DNSProxyTestSuite) TestRestoredEndpoint(c *C) {

s.restoring = false
}

type selectorMock struct {
}

func (t selectorMock) GetSelections() []identity.NumericIdentity {
panic("implement me")
}

func (t selectorMock) Selects(nid identity.NumericIdentity) bool {
panic("implement me")
}

func (t selectorMock) IsWildcard() bool {
panic("implement me")
}

func (t selectorMock) IsNone() bool {
panic("implement me")
}

func (t selectorMock) String() string {
panic("implement me")
}

func Benchmark_perEPAllow_setPortRulesForID(b *testing.B) {
const (
nMatchPatterns = 100
)

var selectorA, selectorB *selectorMock
newRules := policy.L7DataMap{
selectorA: nil,
selectorB: nil,
}

var portRuleDNS []api.PortRuleDNS
for i := 0; i < nMatchPatterns; i++ {
portRuleDNS = append(portRuleDNS, api.PortRuleDNS{
MatchPattern: "kubernetes.default.svc.cluster.local",
})
}

for selector := range newRules {
newRules[selector] = &policy.PerSelectorPolicy{
L7Rules: api.L7Rules{
DNS: portRuleDNS,
},
}
}
pea := perEPAllow{}

lru := lru.New(128)
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
for epID := uint64(0); epID < 20; epID++ {
pea.setPortRulesForID(lru, epID, 8053, newRules)
}
}
}

0 comments on commit 42417c7

Please sign in to comment.