Repository: cilium/cilium
Pull request #24843 (2023-04-12 16:37:45)

v1.13 Backports 2023-04-12
 * [x] #24788 (@jrajahalme)
   * :warning: @jrajahalme Had to fix a conflict in the unit tests around `RulesOrigin` vs `DerivedFromRules`
 * [x] #24815 (@aditighag)

Once this PR is merged, you can update the PR labels via:
```upstream-prs
for pr in 24788 24815; do contrib/backporting/set-labels.py $pr done 1.13; done
```
or with
```
make add-labels BRANCH=v1.13 ISSUES=24788,24815
```


gandro: /test-backport-1.13

Patch:
diff --git a/pkg/cgroups/manager/manager.go b/pkg/cgroups/manager/manager.go
index fbcb684d16..34aac2192c 100644
--- a/pkg/cgroups/manager/manager.go
+++ b/pkg/cgroups/manager/manager.go
@@ -371,18 +371,12 @@ func (m *CgroupManager) deletePodMetadata(pod *v1.Pod) {
 func (m *CgroupManager) getPodMetadata(cgroupId uint64, podMetadataOut chan *PodMetadata) {
 	cm, ok := m.containerMetadataByCgrpId[cgroupId]
 	if !ok {
-		log.WithFields(logrus.Fields{
-			"container-cgroup-id": cgroupId,
-		}).Debugf("Metadata not found")
 		close(podMetadataOut)
 		return
 	}
 
 	pm, ok := m.podMetadataById[cm.podId]
 	if !ok {
-		log.WithFields(logrus.Fields{
-			"container-cgroup-id": cgroupId,
-		}).Debugf("Pod metadata not found")
 		close(podMetadataOut)
 		return
 	}
diff --git a/pkg/endpoint/bpf.go b/pkg/endpoint/bpf.go
index 695056909f..bbe0ef28f2 100644
--- a/pkg/endpoint/bpf.go
+++ b/pkg/endpoint/bpf.go
@@ -26,6 +26,7 @@ import (
 	"github.com/cilium/cilium/pkg/completion"
 	"github.com/cilium/cilium/pkg/controller"
 	"github.com/cilium/cilium/pkg/endpoint/regeneration"
+	"github.com/cilium/cilium/pkg/fqdn/restore"
 	"github.com/cilium/cilium/pkg/loadinfo"
 	"github.com/cilium/cilium/pkg/logging/logfields"
 	"github.com/cilium/cilium/pkg/maps/bwmap"
@@ -154,12 +155,10 @@ func (e *Endpoint) writeHeaderfile(prefix string) error {
 	}
 	defer f.Cleanup()
 
-	// Update DNSRules if any. This is needed because DNSRules also encode allowed destination IPs
-	// and those can change anytime we have identity updates in the cluster. If there are no
-	// DNSRules (== nil) we don't need to update here, as in that case there are no allowed
-	// destinations either.
 	if e.DNSRules != nil {
-		e.OnDNSPolicyUpdateLocked(e.owner.GetDNSRules(e.ID))
+		// Note: e.DNSRules is updated by syncEndpointHeaderFile and regenerateBPF
+		// before they call into writeHeaderfile, because GetDNSRules must not be
+		// called with endpoint.mutex held.
 		e.getLogger().WithFields(logrus.Fields{
 			logfields.Path: headerPath,
 			"DNSRules":     e.DNSRules,
@@ -583,18 +582,11 @@ func (e *Endpoint) regenerateBPF(regenContext *regenerationContext) (revnum uint
 	datapathRegenCtxt.prepareForProxyUpdates(regenContext.parentContext)
 	defer datapathRegenCtxt.completionCancel()
 
-	headerfileChanged, err = e.runPreCompilationSteps(regenContext)
 	// The following DNS rules code was previously inside the critical section
-	// above (runPreCompilationSteps()), but this caused a deadlock with the
-	// ipcache. It's not necessary to run this code within the  critical
-	// section as the only use for the DNS rules is for restoring them upon the
-	// Agent restart.
-	rules := e.owner.GetDNSRules(uint16(e.ID))
-	if err := e.lockAlive(); err != nil {
-		return 0, compilationExecuted, err
-	}
-	e.OnDNSPolicyUpdateLocked(rules)
-	e.unlock()
+	// below (runPreCompilationSteps()), but this caused a deadlock with the
+	// IPCache. Therefore, we obtain the DNSRules outside the critical section.
+	rules := e.owner.GetDNSRules(e.ID)
+	headerfileChanged, err = e.runPreCompilationSteps(regenContext, rules)
 
 	// Keep track of the side-effects of the regeneration that need to be
 	// reverted in case of failure.
@@ -741,7 +733,7 @@ func (e *Endpoint) realizeBPFState(regenContext *regenerationContext) (compilati
 // The endpoint mutex must not be held.
 //
 // Returns whether the headerfile changed and/or an error.
-func (e *Endpoint) runPreCompilationSteps(regenContext *regenerationContext) (headerfileChanged bool, preCompilationError error) {
+func (e *Endpoint) runPreCompilationSteps(regenContext *regenerationContext, rules restore.DNSRules) (headerfileChanged bool, preCompilationError error) {
 	stats := &regenContext.Stats
 	datapathRegenCtxt := regenContext.datapathRegenerationContext
 
@@ -779,6 +771,12 @@ func (e *Endpoint) runPreCompilationSteps(regenContext *regenerationContext) (he
 		close(datapathRegenCtxt.ctCleaned)
 	}
 
+	// We cannot obtain the rules while e.mutex is held, because obtaining
+	// fresh DNSRules requires the IPCache lock (which must not be taken while
+	// holding e.mutex to avoid deadlocks). Therefore, rules are obtained
+	// before the call to runPreCompilationSteps.
+	e.OnDNSPolicyUpdateLocked(rules)
+
 	// If dry mode is enabled, no further changes to BPF maps are performed
 	if option.Config.DryMode {
 
diff --git a/pkg/endpoint/endpoint.go b/pkg/endpoint/endpoint.go
index 6273ece696..1a57f9dc6e 100644
--- a/pkg/endpoint/endpoint.go
+++ b/pkg/endpoint/endpoint.go
@@ -2101,12 +2101,23 @@ func (e *Endpoint) syncEndpointHeaderFile(reasons []string) {
 	e.buildMutex.Lock()
 	defer e.buildMutex.Unlock()
 
+	// The following GetDNSRules call will acquire a read-lock on the IPCache.
+	// Because IPCache itself will potentially acquire endpoint locks in its
+	// critical section, we must _not_ hold endpoint.mutex while calling
+	// GetDNSRules, to avoid a deadlock between IPCache and the endpoint. It is
+	// okay to hold endpoint.buildMutex, however.
+	rules := e.owner.GetDNSRules(e.ID)
+
 	if err := e.lockAlive(); err != nil {
 		// endpoint was removed in the meanwhile, return
 		return
 	}
 	defer e.unlock()
 
+	// Update DNSRules if any. This is needed because DNSRules also encode allowed destination IPs
+	// and those can change anytime we have identity updates in the cluster.
+	e.OnDNSPolicyUpdateLocked(rules)
+
 	if err := e.writeHeaderfile(e.StateDirectoryPath()); err != nil {
 		e.getLogger().WithFields(logrus.Fields{
 			logfields.Reason: reasons,
diff --git a/pkg/endpoint/regeneration/owner.go b/pkg/endpoint/regeneration/owner.go
index 91a2b8076a..d1dfe3bf01 100644
--- a/pkg/endpoint/regeneration/owner.go
+++ b/pkg/endpoint/regeneration/owner.go
@@ -35,6 +35,7 @@ type Owner interface {
 
 	// GetDNSRules creates a fresh copy of DNS rules that can be used when
 	// endpoint is restored on a restart.
+	// The endpoint lock must not be held while calling this function.
 	GetDNSRules(epID uint16) restore.DNSRules
 
 	// RemoveRestoredDNSRules removes any restored DNS rules for
diff --git a/pkg/fqdn/dnsproxy/proxy.go b/pkg/fqdn/dnsproxy/proxy.go
index 0aca602ff5..8272b8198a 100644
--- a/pkg/fqdn/dnsproxy/proxy.go
+++ b/pkg/fqdn/dnsproxy/proxy.go
@@ -217,50 +217,88 @@ func (p *DNSProxy) checkRestored(endpointID uint64, destPort uint16, destIP stri
 	return false
 }
 
+// skipIPInRestorationRLocked skips IPs that are allowed but have never been used,
+// but only if at least one server has been used so far.
+// Requires the RLock to be held.
+func (p *DNSProxy) skipIPInRestorationRLocked(ip string) bool {
+	if len(p.usedServers) > 0 {
+		if _, used := p.usedServers[ip]; !used {
+			return true
+		}
+	}
+	return false
+}
+
 // GetRules creates a fresh copy of EP's DNS rules to be stored
 // for later restoration.
 func (p *DNSProxy) GetRules(endpointID uint16) (restore.DNSRules, error) {
+	// Lock ordering note: Acquiring the IPCache read lock (as LookupIPsBySecID does) while holding
+	// the proxy lock can lead to a deadlock. Avoid this by reading the state from DNSProxy while
+	// holding the read lock, then perform the IPCache lookups.
+	// Note that IPCache state may change in between calls to LookupIPsBySecID.
 	p.RLock()
-	defer p.RUnlock()
 
-	restored := make(restore.DNSRules)
+	type selRegex struct {
+		re *regexp.Regexp
+		cs policy.CachedSelector
+	}
+
+	portToSelRegex := make(map[uint16][]selRegex)
 	for port, entries := range p.allowed[uint64(endpointID)] {
-		var ipRules restore.IPRules
+		var nidRules = make([]selRegex, 0, len(entries))
+		// Copy the entries to avoid racy map accesses after we release the lock. We don't need
+		// constant time access, hence a preallocated slice instead of another map.
 		for cs, regex := range entries {
-			var IPs map[string]struct{}
-			if !cs.IsWildcard() {
-				IPs = make(map[string]struct{})
-				count := 0
-			Loop:
-				for _, nid := range cs.GetSelections() {
-					nidIPs := p.LookupIPsBySecID(nid)
-					for _, ip := range nidIPs {
-						// Skip IPs that are allowed but have never been used,
-						// but only if at least one server has been used so far.
-						if len(p.usedServers) > 0 {
-							if _, used := p.usedServers[ip]; !used {
-								continue
-							}
-						}
-						IPs[ip] = struct{}{}
-						count++
-						if count > p.maxIPsPerRestoredDNSRule {
-							log.WithFields(logrus.Fields{
-								logfields.EndpointID:            endpointID,
-								logfields.Port:                  port,
-								logfields.EndpointLabelSelector: cs,
-								logfields.Limit:                 p.maxIPsPerRestoredDNSRule,
-								logfields.Count:                 len(nidIPs),
-							}).Warning("Too many IPs for a DNS rule, skipping the rest")
-							break Loop
-						}
+			nidRules = append(nidRules, selRegex{cs: cs, re: regex})
+		}
+		portToSelRegex[port] = nidRules
+	}
+
+	// We've read what we need from the proxy. The following IPCache lookups _must_ occur outside of
+	// the critical section.
+	p.RUnlock()
+
+	restored := make(restore.DNSRules)
+	for port, selRegexes := range portToSelRegex {
+		var ipRules restore.IPRules
+		for _, selRegex := range selRegexes {
+			if selRegex.cs.IsWildcard() {
+				ipRules = append(ipRules, asIPRule(selRegex.re, nil))
+				continue
+			}
+			ips := make(map[string]struct{})
+			count := 0
+			nids := selRegex.cs.GetSelections()
+		Loop:
+			for _, nid := range nids {
+				// Note: p.RLock must not be held during this call to IPCache
+				nidIPs := p.LookupIPsBySecID(nid)
+				p.RLock()
+				for _, ip := range nidIPs {
+					if p.skipIPInRestorationRLocked(ip) {
+						continue
+					}
+					ips[ip] = struct{}{}
+					count++
+					if count > p.maxIPsPerRestoredDNSRule {
+						log.WithFields(logrus.Fields{
+							logfields.EndpointID:            endpointID,
+							logfields.Port:                  port,
+							logfields.EndpointLabelSelector: selRegex.cs,
+							logfields.Limit:                 p.maxIPsPerRestoredDNSRule,
+							logfields.Count:                 len(nidIPs),
+						}).Warning("Too many IPs for a DNS rule, skipping the rest")
+						p.RUnlock()
+						break Loop
 					}
 				}
+				p.RUnlock()
 			}
-			ipRules = append(ipRules, asIPRule(regex, IPs))
+			ipRules = append(ipRules, asIPRule(selRegex.re, ips))
 		}
 		restored[port] = ipRules
 	}
+
 	return restored, nil
 }
 
diff --git a/pkg/ipcache/ipcache.go b/pkg/ipcache/ipcache.go
index dfee080189..f88a2c330c 100644
--- a/pkg/ipcache/ipcache.go
+++ b/pkg/ipcache/ipcache.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"net"
 	"net/netip"
+	"sync/atomic"
 	"time"
 
 	"github.com/sirupsen/logrus"
@@ -81,18 +82,17 @@ type IPCache struct {
 	// controllers manages the async controllers for this IPCache
 	controllers *controller.Manager
 
-	// needNamedPorts is initially 'false', but will be changd to 'true' when the
-	// clusterwide named port mappings are needed for network policy computation
-	// for the first time. This avoids the overhead of maintaining 'namedPorts' map
-	// when it is known not to be needed.
-	// Protected by 'mutex'.
-	needNamedPorts bool
+	// needNamedPorts is initially 'false', but will atomically be changed to 'true'
+	// when the clusterwide named port mappings are needed for network policy
+	// computation for the first time. This avoids the overhead of unnecessarily
+	// triggering policy updates when it is known not to be needed.
+	needNamedPorts atomic.Bool
 
 	// namedPorts is a collection of all named ports in the cluster. This is needed
 	// only if an egress policy refers to a port by name.
 	// This map is returned to users so all updates must be made into a fresh map that
-	// is then swapped in place while 'mutex' is being held.
-	namedPorts types.NamedPortMultiMap
+	// is then swapped in place atomically.
+	namedPorts atomic.Pointer[types.NamedPortMultiMap]
 
 	// k8sSyncedChecker knows how to check for whether the K8s watcher cache
 	// has been fully synced.
@@ -121,7 +121,7 @@ func NewIPCache(c *Configuration) *IPCache {
 		ipToHostIPCache:   map[string]IPKeyPair{},
 		ipToK8sMetadata:   map[string]K8sMetadata{},
 		controllers:       controller.NewManager(),
-		namedPorts:        nil,
+		namedPorts:        atomic.Pointer[types.NamedPortMultiMap]{},
 		metadata:          newMetadata(),
 		Configuration:     c,
 	}
@@ -216,13 +216,19 @@ func (ipc *IPCache) getK8sMetadata(ip string) *K8sMetadata {
 	return nil
 }
 
-// updateNamedPorts accumulates named ports from all K8sMetadata entries to a single map
-func (ipc *IPCache) updateNamedPorts() (namedPortsChanged bool) {
-	if !ipc.needNamedPorts {
-		return false
-	}
+// updateNamedPortsRLocked accumulates named ports from all K8sMetadata entries
+// to a single map. It is required to hold _at least_ the read lock when calling
+// this function, holding the write lock is also allowed.
+func (ipc *IPCache) updateNamedPortsRLocked() (namedPortsChanged bool) {
 	// Collect new named Ports
-	npm := make(types.NamedPortMultiMap, len(ipc.namedPorts))
+	var oldLen = 0
+	var old types.NamedPortMultiMap
+	if m := ipc.namedPorts.Load(); m != nil {
+		old = *m
+		oldLen = len(old)
+	}
+
+	npm := make(types.NamedPortMultiMap, oldLen)
 	for _, km := range ipc.ipToK8sMetadata {
 		for name, port := range km.NamedPorts {
 			if npm[name] == nil {
@@ -231,15 +237,20 @@ func (ipc *IPCache) updateNamedPorts() (namedPortsChanged bool) {
 			npm[name][port] = struct{}{}
 		}
 	}
-	namedPortsChanged = !npm.Equal(ipc.namedPorts)
+	namedPortsChanged = !npm.Equal(old)
 	if namedPortsChanged {
-		// swap the new map in
-		if len(npm) == 0 {
-			ipc.namedPorts = nil
-		} else {
-			ipc.namedPorts = npm
-		}
+		// atomically swap the new map in
+		ipc.namedPorts.Store(&npm)
+	}
+
+	// Set namedPortsChanged to false if they have not (yet) been requested.
+	// This avoids triggering policy updates if named ports changed, but no
+	// policy actually needs them. We still pre-compute the namedPorts map,
+	// so we don't have to acquire the IPCache lock in GetNamedPorts
+	if !ipc.needNamedPorts.Load() {
+		namedPortsChanged = false
 	}
+
 	return namedPortsChanged
 }
 
@@ -420,7 +431,7 @@ func (ipc *IPCache) upsertLocked(
 		if namedPortsChanged {
 			// It is possible that some other POD defines same values, check if
 			// anything changes over all the PODs.
-			namedPortsChanged = ipc.updateNamedPorts()
+			namedPortsChanged = ipc.updateNamedPortsRLocked()
 		}
 	}
 
@@ -595,7 +606,7 @@ func (ipc *IPCache) deleteLocked(ip string, source source.Source) (namedPortsCha
 	// Update named ports
 	namedPortsChanged = false
 	if oldK8sMeta != nil && len(oldK8sMeta.NamedPorts) > 0 {
-		namedPortsChanged = ipc.updateNamedPorts()
+		namedPortsChanged = ipc.updateNamedPortsRLocked()
 	}
 
 	if newHostIP != nil {
@@ -617,15 +628,23 @@ func (ipc *IPCache) deleteLocked(ip string, source source.Source) (namedPortsCha
 
 // GetNamedPorts returns a copy of the named ports map. May return nil.
 func (ipc *IPCache) GetNamedPorts() (npm types.NamedPortMultiMap) {
-	ipc.mutex.Lock()
-	if !ipc.needNamedPorts {
-		ipc.needNamedPorts = true
-		ipc.updateNamedPorts()
-	}
+	// We must not acquire the IPCache mutex here, as that would establish a lock ordering of
+	// Endpoint > IPCache (as endpoint.mutex can be held while calling GetNamedPorts)
+	// Since InjectLabels requires IPCache > Endpoint, a deadlock can occur otherwise.
+
+	// needNamedPorts is initially set to 'false'. This means that we will not trigger
+	// policy updates upon changes to named ports. Once this is set to 'true' though,
+	// Upsert and Delete will start to return 'namedPortsChanged = true' if the upsert
+	// or delete changed a named port, enabling the caller to trigger a policy update.
+	// Note that at the moment, this will never be set back to false, even if no policy
+	// uses named ports anymore.
+	ipc.needNamedPorts.Store(true)
+
 	// Caller can keep using the map after the lock is released, as the map is never changed
 	// once published.
-	npm = ipc.namedPorts
-	ipc.mutex.Unlock()
+	if ptr := ipc.namedPorts.Load(); ptr != nil {
+		npm = *ptr
+	}
 	return npm
 }
 
diff --git a/pkg/policy/l4.go b/pkg/policy/l4.go
index 336b727b0b..e8483acf36 100644
--- a/pkg/policy/l4.go
+++ b/pkg/policy/l4.go
@@ -634,18 +634,19 @@ func (l4 *L4Filter) cacheFQDNSelector(sel api.FQDNSelector, selectorCache *Selec
 
 // add L7 rules for all endpoints in the L7DataMap
 func (l7 L7DataMap) addPolicyForSelector(rules *api.L7Rules, terminatingTLS, originatingTLS *TLSContext, auth *api.Auth, deny bool, sni []string, forceRedirect bool) {
-	l7policy := &PerSelectorPolicy{
-		TerminatingTLS: terminatingTLS,
-		OriginatingTLS: originatingTLS,
-		Auth:           auth,
-		IsDeny:         deny,
-		ServerNames:    NewStringSet(sni),
-		isRedirect:     !deny && (forceRedirect || terminatingTLS != nil || originatingTLS != nil || len(sni) > 0 || !rules.IsEmpty()),
-	}
-	if rules != nil {
-		l7policy.L7Rules = *rules
-	}
+	isRedirect := !deny && (forceRedirect || terminatingTLS != nil || originatingTLS != nil || len(sni) > 0 || !rules.IsEmpty())
 	for epsel := range l7 {
+		l7policy := &PerSelectorPolicy{
+			TerminatingTLS: terminatingTLS,
+			OriginatingTLS: originatingTLS,
+			Auth:           auth,
+			IsDeny:         deny,
+			ServerNames:    NewStringSet(sni),
+			isRedirect:     isRedirect,
+		}
+		if rules != nil {
+			l7policy.L7Rules = *rules
+		}
 		l7[epsel] = l7policy
 	}
 }


Test patch:
diff --git a/pkg/ipcache/ipcache_test.go b/pkg/ipcache/ipcache_test.go
index 8f8b1d1fd5..d47b159ee7 100644
--- a/pkg/ipcache/ipcache_test.go
+++ b/pkg/ipcache/ipcache_test.go
@@ -549,7 +549,7 @@ func (s *IPCacheTestSuite) TestIPCacheNamedPorts(c *C) {
 	namedPortsChanged = IPIdentityCache.Delete(endpointIP2, source.Kubernetes)
 	c.Assert(namedPortsChanged, Equals, true)
 	npm = IPIdentityCache.GetNamedPorts()
-	c.Assert(npm, IsNil)
+	c.Assert(npm, HasLen, 0)
 }
 
 type dummyListener struct {
diff --git a/pkg/policy/rule_test.go b/pkg/policy/rule_test.go
index 6a3cdd57d5..43036b707b 100644
--- a/pkg/policy/rule_test.go
+++ b/pkg/policy/rule_test.go
@@ -2645,3 +2645,91 @@ func BenchmarkRuleString(b *testing.B) {
 		_ = r.String()
 	}
 }
+
+// Test merging of L7 rules when the same rules apply to multiple selectors.
+// This was added to prevent regression of a bug where the merging of l7 rules for "foo"
+// also affected the rules for "baz".
+func (ds *PolicyTestSuite) TestMergeL7PolicyEgressWithMultipleSelectors(c *C) {
+	fromBar := &SearchContext{From: labels.ParseSelectLabelArray("bar")}
+	fromFoo := &SearchContext{From: labels.ParseSelectLabelArray("foo")}
+
+	fooSelector := []api.EndpointSelector{
+		api.NewESFromLabels(labels.ParseSelectLabel("foo")),
+	}
+	foobazSelector := []api.EndpointSelector{
+		api.NewESFromLabels(labels.ParseSelectLabel("foo")),
+		api.NewESFromLabels(labels.ParseSelectLabel("baz")),
+	}
+
+	rule1 := &rule{
+		Rule: api.Rule{
+			EndpointSelector: api.NewESFromLabels(labels.ParseSelectLabel("bar")),
+			Egress: []api.EgressRule{
+				{
+					EgressCommonRule: api.EgressCommonRule{
+						ToEndpoints: fooSelector,
+					},
+					// Note that this allows all on 80, so the result should wildcard HTTP to "foo"
+					ToPorts: []api.PortRule{{
+						Ports: []api.PortProtocol{
+							{Port: "80", Protocol: api.ProtoTCP},
+						},
+					}},
+				},
+				{
+					EgressCommonRule: api.EgressCommonRule{
+						ToEndpoints: foobazSelector,
+					},
+					ToPorts: []api.PortRule{{
+						Ports: []api.PortProtocol{
+							{Port: "80", Protocol: api.ProtoTCP},
+						},
+						Rules: &api.L7Rules{
+							HTTP: []api.PortRuleHTTP{
+								{Method: "GET"},
+							},
+						},
+					}},
+				},
+			},
+		},
+	}
+
+	expected := L4PolicyMap{"80/TCP": &L4Filter{
+		Port: 80, Protocol: api.ProtoTCP, U8Proto: 6,
+		L7Parser: ParserTypeHTTP,
+		PerSelectorPolicies: L7DataMap{
+			cachedFooSelector: &PerSelectorPolicy{
+				L7Rules: api.L7Rules{
+					HTTP: []api.PortRuleHTTP{{Method: "GET"}, {}},
+				},
+				isRedirect: true,
+			},
+			cachedBazSelector: &PerSelectorPolicy{
+				L7Rules: api.L7Rules{
+					HTTP: []api.PortRuleHTTP{{Method: "GET"}},
+				},
+				isRedirect: true,
+			},
+		},
+		Ingress:          false,
+		DerivedFromRules: labels.LabelArrayList{nil},
+	}}
+
+	state := traceState{}
+	res, err := rule1.resolveEgressPolicy(testPolicyContext, fromBar, &state, L4PolicyMap{}, nil, nil)
+	c.Assert(err, IsNil)
+	c.Assert(res, Not(IsNil))
+	c.Assert(res, checker.DeepEquals, expected)
+	c.Assert(state.selectedRules, Equals, 1)
+	c.Assert(state.matchedRules, Equals, 1)
+	res.Detach(testSelectorCache)
+	expected.Detach(testSelectorCache)
+
+	state = traceState{}
+	res, err = rule1.resolveEgressPolicy(testPolicyContext, fromFoo, &state, L4PolicyMap{}, nil, nil)
+	c.Assert(err, IsNil)
+	c.Assert(res, IsNil)
+	c.Assert(state.selectedRules, Equals, 0)
+	c.Assert(state.matchedRules, Equals, 0)
+}