Skip to content

Commit

Permalink
policy: Move GetNets to selectorcache
Browse files Browse the repository at this point in the history 
Have selector cache precompute the most specific CIDR for an identity
when the identity is added, rathter than computing it when needed for
each MapStateEntry.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
  • Loading branch information
@jrajahalme @joamaki
jrajahalme authored and joamaki committed Sep 1, 2023
1 parent 08903e0 commit cb246d7
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 67 deletions.
95 changes: 33 additions & 62 deletions pkg/policy/mapstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const (
type MapState map[Key]MapStateEntry

type Identities interface {
GetLabelsLocked(identity.NumericIdentity) labels.LabelArray
GetNetsLocked(identity.NumericIdentity) []*net.IPNet
}

// Key is the userspace representation of a policy key in BPF. It is
Expand Down Expand Up @@ -136,9 +136,6 @@ type MapStateEntry struct {
// dependents contains the keys for entries create based on this entry. These entries
// will be deleted once all of the owners are deleted.
dependents Keys

// cachedNets caches the subnets (if any) associated with this MapStateEntry.
cachedNets []*net.IPNet
}

// NewMapStateEntry creates a map state entry. If redirect is true, the
Expand Down Expand Up @@ -193,59 +190,34 @@ func (e *MapStateEntry) HasDependent(key Key) bool {
return ok
}

var worldNets = map[identity.NumericIdentity][]*net.IPNet{
identity.ReservedIdentityWorld: {
{IP: net.IPv4zero, Mask: net.CIDRMask(0, net.IPv4len*8)},
{IP: net.IPv6zero, Mask: net.CIDRMask(0, net.IPv6len*8)},
},
identity.ReservedIdentityWorldIPv4: {
{IP: net.IPv4zero, Mask: net.CIDRMask(0, net.IPv4len*8)},
},
identity.ReservedIdentityWorldIPv6: {
{IP: net.IPv6zero, Mask: net.CIDRMask(0, net.IPv6len*8)},
},
}

// getNets returns the most specific CIDR for an identity. For the "World" identity
// it returns both IPv4 and IPv6.
func (e *MapStateEntry) getNets(identities Identities, ident uint32) []*net.IPNet {
// Caching results is not dangerous in this situation as the entry
// is ephemerally tied to the lifecycle of the MapState object that
// it will be in.
if e.cachedNets != nil {
return e.cachedNets
}
func getNets(identities Identities, ident uint32) []*net.IPNet {
// World identities are handled explicitly for two reasons:
// 1. 'identities' may be nil, but world identities are still expected to be considered
// 2. SelectorCache is not be informed of reserved/world identities in all test cases
id := identity.NumericIdentity(ident)
switch id {
case identity.ReservedIdentityWorld:
e.cachedNets = []*net.IPNet{
{IP: net.IPv4zero, Mask: net.CIDRMask(0, net.IPv4len*8)},
{IP: net.IPv6zero, Mask: net.CIDRMask(0, net.IPv6len*8)},
}
return e.cachedNets
case identity.ReservedIdentityWorldIPv4:
e.cachedNets = []*net.IPNet{
{IP: net.IPv4zero, Mask: net.CIDRMask(0, net.IPv4len*8)},
}
return e.cachedNets
case identity.ReservedIdentityWorldIPv6:
e.cachedNets = []*net.IPNet{
{IP: net.IPv6zero, Mask: net.CIDRMask(0, net.IPv6len*8)},
}
return e.cachedNets
if id <= identity.ReservedIdentityWorldIPv6 {
return worldNets[id]
}
// CIDR identities have a local scope, so we can skip the rest if id is not of local scope.
if !id.HasLocalScope() || identities == nil {
return nil
}
lbls := identities.GetLabelsLocked(id)
var (
maskSize int
mostSpecificCidr *net.IPNet
)
for _, lbl := range lbls {
if lbl.Source == labels.LabelSourceCIDR {
_, netIP, err := net.ParseCIDR(lbl.Key)
if err == nil {
if ms, _ := netIP.Mask.Size(); ms > maskSize {
mostSpecificCidr = netIP
maskSize = ms
}
}
}
}
if mostSpecificCidr != nil {
e.cachedNets = []*net.IPNet{mostSpecificCidr}
return e.cachedNets
}
return nil
return identities.GetNetsLocked(id)
}

// AddDependent adds 'key' to the set of dependent keys.
Expand Down Expand Up @@ -401,7 +373,6 @@ func (e MapStateEntry) String() string {
func (keys MapState) denyPreferredInsert(newKey Key, newEntry MapStateEntry, identities Identities, features policyFeatures) {
// Enforce nil values from NewMapStateEntry
newEntry.dependents = nil
newEntry.cachedNets = nil

keys.denyPreferredInsertWithChanges(newKey, newEntry, identities, features, ChangeState{})
}
Expand Down Expand Up @@ -503,13 +474,13 @@ func (keys MapState) deleteKeyWithChanges(key Key, owner MapStateOwner, changes
}
}

// entryIdentityIsSupersetOf compares two entries and keys to see if the primary identity contains
// identityIsSupersetOf compares two entries and keys to see if the primary identity contains
// the compared identity. This means that either that primary identity is 0 (i.e. it is a superset
// of every other identity), or one of the subnets of the primary identity fully contains or is
// equal to one of the subnets in the compared identity (note:this covers cases like "reserved:world").
func entryIdentityIsSupersetOf(primaryKey Key, primaryEntry MapStateEntry, compareKey Key, compareEntry MapStateEntry, identities Identities) bool {
func identityIsSupersetOf(primaryIdentity, compareIdentity uint32, identities Identities) bool {
// If the identities are equal then neither is a superset (for the purposes of our business logic).
if primaryKey.Identity == compareKey.Identity {
if primaryIdentity == compareIdentity {
return false
}

Expand Down Expand Up @@ -566,9 +537,9 @@ func entryIdentityIsSupersetOf(primaryKey Key, primaryEntry MapStateEntry, compa
// correctly implemented between the new and old entries, taking into
// account whether the identities may represent CIDRs that have a
// superset relationship.
return primaryKey.Identity == 0 && compareKey.Identity != 0 ||
ip.NetsContainsAny(primaryEntry.getNets(identities, primaryKey.Identity),
compareEntry.getNets(identities, compareKey.Identity))
return primaryIdentity == 0 && compareIdentity != 0 ||
ip.NetsContainsAny(getNets(identities, primaryIdentity),
getNets(identities, compareIdentity))
}

// protocolsMatch checks to see if two given keys match on protocol.
Expand Down Expand Up @@ -615,7 +586,7 @@ func (keys MapState) denyPreferredInsertWithChanges(newKey Key, newEntry MapStat
continue
}
if !v.IsDeny {
if entryIdentityIsSupersetOf(k, v, newKey, newEntry, identities) {
if identityIsSupersetOf(k.Identity, newKey.Identity, identities) {
if newKey.PortProtoIsBroader(k) {
// If this iterated-allow-entry is a superset of the new-entry
// and it has a more specific port-protocol than the new-entry
Expand All @@ -632,15 +603,15 @@ func (keys MapState) denyPreferredInsertWithChanges(newKey Key, newEntry MapStat
newEntry.AddDependent(newKeyCpy)
}
} else if (newKey.Identity == k.Identity ||
entryIdentityIsSupersetOf(newKey, newEntry, k, v, identities)) &&
identityIsSupersetOf(newKey.Identity, k.Identity, identities)) &&
(newKey.PortProtoIsBroader(k) || newKey.PortProtoIsEqual(k)) {
// If the new-entry is a superset (or equal) of the iterated-allow-entry and
// the new-entry has a broader (or equal) port-protocol then we
// should delete the iterated-allow-entry
keys.deleteKeyWithChanges(k, nil, changes)
}
} else if (newKey.Identity == k.Identity ||
entryIdentityIsSupersetOf(k, v, newKey, newEntry, identities)) &&
identityIsSupersetOf(k.Identity, newKey.Identity, identities)) &&
k.DestPort == 0 && k.Nexthdr == 0 &&
!v.HasDependent(newKey) {
// If this iterated-deny-entry is a supserset (or equal) of the new-entry and
Expand All @@ -653,7 +624,7 @@ func (keys MapState) denyPreferredInsertWithChanges(newKey Key, newEntry MapStat
// but there *may* be performance tradeoffs.
return
} else if (newKey.Identity == k.Identity ||
entryIdentityIsSupersetOf(newKey, newEntry, k, v, identities)) &&
identityIsSupersetOf(newKey.Identity, k.Identity, identities)) &&
newKey.DestPort == 0 && newKey.Nexthdr == 0 &&
!newEntry.HasDependent(k) {
// If this iterated-deny-entry is a subset (or equal) of the new-entry and
Expand All @@ -677,7 +648,7 @@ func (keys MapState) denyPreferredInsertWithChanges(newKey Key, newEntry MapStat
}
// NOTE: We do not delete redundant allow entries.
if v.IsDeny {
if entryIdentityIsSupersetOf(newKey, newEntry, k, v, identities) {
if identityIsSupersetOf(newKey.Identity, k.Identity, identities) {
if k.PortProtoIsBroader(newKey) {
// If the new-entry is *only* superset of the iterated-deny-entry
// and the new-entry has a more specific port-protocol than the
Expand All @@ -695,7 +666,7 @@ func (keys MapState) denyPreferredInsertWithChanges(newKey Key, newEntry MapStat
keys.addDependentOnEntry(k, v, denyKeyCpy, changes)
}
} else if (k.Identity == newKey.Identity ||
entryIdentityIsSupersetOf(k, v, newKey, newEntry, identities)) &&
identityIsSupersetOf(k.Identity, newKey.Identity, identities)) &&
(k.PortProtoIsBroader(newKey) || k.PortProtoIsEqual(newKey)) &&
!v.HasDependent(newKey) {
// If the iterated-deny-entry is a superset (or equal) of the new-entry and has a
Expand Down
46 changes: 41 additions & 5 deletions pkg/policy/selectorcache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ type identitySelector interface {
type scIdentity struct {
NID identity.NumericIdentity
lbls labels.LabelArray
namespace string // value of the namespace label, or ""
nets []*net.IPNet // Most specific CIDR for the identity, if any.
computed bool // nets has been computed
namespace string // value of the namespace label, or ""
}

// scIdentityCache is a cache of Identities keyed by the numeric identity
Expand All @@ -171,10 +173,37 @@ func newIdentity(nid identity.NumericIdentity, lbls labels.LabelArray) scIdentit
return scIdentity{
NID: nid,
lbls: lbls,
nets: getLocalScopeNets(nid, lbls),
namespace: lbls.Get(labels.LabelSourceK8sKeyPrefix + k8sConst.PodNamespaceLabel),
computed: true,
}
}

// getLocalScopeNets returns the most specific CIDR for a local scope identity.
func getLocalScopeNets(id identity.NumericIdentity, lbls labels.LabelArray) []*net.IPNet {
if id.HasLocalScope() {
var (
maskSize int
mostSpecificCidr *net.IPNet
)
for _, lbl := range lbls {
if lbl.Source == labels.LabelSourceCIDR {
_, netIP, err := net.ParseCIDR(lbl.Key)
if err == nil {
if ms, _ := netIP.Mask.Size(); ms > maskSize {
mostSpecificCidr = netIP
maskSize = ms
}
}
}
}
if mostSpecificCidr != nil {
return []*net.IPNet{mostSpecificCidr}
}
}
return nil
}

func getIdentityCache(ids cache.IdentityCache) scIdentityCache {
idCache := make(map[identity.NumericIdentity]scIdentity, len(ids))
for nid, lbls := range ids {
Expand Down Expand Up @@ -1093,11 +1122,18 @@ func (sc *SelectorCache) RemoveIdentitiesFQDNSelectors(fqdnSels []api.FQDNSelect
sc.releaseIdentityMappings(identitiesToRelease)
}

// GetLabels must be called while holding sc.mutex!
func (sc *SelectorCache) GetLabelsLocked(id identity.NumericIdentity) labels.LabelArray {
// GetNetsLocked returns the most specific CIDR for an identity. For the "World" identity
// it returns both IPv4 and IPv6.
func (sc *SelectorCache) GetNetsLocked(id identity.NumericIdentity) []*net.IPNet {
ident, ok := sc.idCache[id]
if !ok {
return labels.LabelArray{}
return nil
}
if !ident.computed {
log.WithFields(logrus.Fields{
logfields.Identity: id,
logfields.Labels: ident.lbls,
}).Warning("GetNetsLocked: Identity with missing nets!")
}
return ident.lbls
return ident.nets
}

0 comments on commit cb246d7

Please sign in to comment.