ipcache: Introduce the ability to inherit CIDR prefixes

[gandro]

This PR introduces a new feature in the IPCache metadata subsystem:
The ability for prefixes with local identities to inherit CIDR labels
from encompassing prefixes. This is needed to implement
cilium/design-cfps#17 where we want to introduce a new way to manage
identities for IPs allowlisted by DNS lookups. Without going into too
much detail, the key point of that CFP is that IPs returned by DNS
lookups will be associated with a fqdn: label.

Those IPs returned by a DNS lookup however might also be selected by a
CIDR prefix, and thus might also optionally need a cidr: label if (but
only if) a selecting CIDR policy is present.

To give an example: If we perform a DNS lookup on one.one.one.one on a
pod with DNS redirects enabled, we may observe IP 1.1.1.1 and thus
associated that IP with a fqdn: label. If at the same time, we also
have a separate CIDR policy allowing traffic to 1.0.0.0/8, then that
IP also needs the cidr:1.0.0.0/8 label, to make it is selected by both
the FQDN and the CIDR policy.

To implement this, the IPCache metadata subsystem is slightly augmented:

1. Whenever metadata for a prefix is upserted or removed, we no longer
   enqueue only the prefix it self, but also all potentially affected
   prefixes or IPs contained by it. This allows InjectLabels to then
   down-propagate the changes to any child prefixes.
2. In resolveIdentity (called by InjectLabels), we check if there
   are any parent prefixes with a CIDR label that we want to inherit.

At the moment, only local identities without a reserved: label may
inherit a CIDR label from a parent prefix. This currently therefore only
applies to identities containing fqdn: labels. Going forward, this
infrastructure might however also be useful to implement non-K8s label
sources.

[gandro]

/test

[gandro]

note: This case should not be hit anymore, even before this PR. But we still have unit tests relying on it. To avoid adding even more commits to this PR, I decided to not emit a "should not be reached" warning log message here and leave this for a follow-up PR.

[nathanjsweet]

Very cool!

[gandro]

Dropped the last ipcache: Only update policy maps for new identities commit as requested. It's now its own PR #32628

[gandro]

/test

[christarazi]

Could you elaborate on

A note on the implementation: To find affected child prefixes, we
currently perform a linear scan over the metadata map. While this is a
O(n) operation, finding CIDR parents in a trie could be done in O(log n), but at the moment we expect n to be in hundreds to the low
thousands at most. For most real world cases, we therefore expect the
current implementation to perform just as well if not better than a
trie. Also note that the linear scan is skipped if the prefix is a
single IP and thus cannot contain any other prefixes besides itself.

What is part of n where we expect it to be in the hundreds or low thousands? What if we add pod IPs into this as #31409 would do?

[gandro]

What is part of n where we expect it to be in the hundreds or low thousands? What if we add pod IPs into this as #31409 would do?

So that sentence was written with the status quo, where only CIDR prefixes (per node), node IPs (global) and DNS lookups (per node) are added to metadata. Of course, once we add pod IPs (global) to also use the async API, the order of magnitude changes and the statement might no longer be true.

It's hard to say at what point an allocation-heavy but asymptotically more optimal data-structure like a trie outperforms a naive linear scan. Let me try to add some benchmarks to see how expensive the current approach is.

[gandro]

I've pushed the code for a benchmark comparing the findAffectedChildPrefixes function when implemented on a Golang map vs a bitlpm trie. There are of course a lot of variables that can impact the trie behavior (how are the prefixes distributed etc), but for the Golang map the algorithm is pretty much agnostic to those changes, and the Golang hash map behavior which is what we are mainly interested in here. In other words, the goal of the benchmark is not to test the behavior of our trie implementation, but mostly figure out "how bad" the naive linear scan really is.

Here are the average times it takes one invocation of findAffectedChildPrefixes - i.e. the cost of upserting a non-leaf prefix, i.e. the cost of ingesting a policy containing a ToCIDR rule (upserts for single IPs do not trigger findAffectedChildPrefixes):

IPs/CIDRs	LPM Trie	Hash Map
1k/10	0.003335 ms	0.029799 ms
10k/100	0.004634 ms	0.25511 ms
100k/1k	0.009221 ms	2.028705 ms
1M/10k	0.015925 ms	26.684717 ms

It's pretty clear that the the trie is O(log n) where as the map is O(n).

In terms of of allocations performed, trie also seems roughly O(log n), where as the map is actually O(1):

IPs/CIDRs	LPM Trie	Hash Map
1k/10	26 allocs/op	6 allocs/op
10k/100	36 allocs/op	6 allocs/op
100k/1k	47 allocs/op	6 allocs/op
1M/10k	56 allocs/op	6 allocs/op

---

As is, I would say the 10k/100 run is about the expected order of magnitude we can expect in a large-ish cluster (5000 nodes, as pods are excluded in this) today. 100k/1k is probably what we can expect for a large-ish cluster after #31409 (i.e. 5k nodes, 90k pods). The 1 million entries can be treated as a theoretical worst case today (it would require ~500k nodes). Also note IPCache currently supports up to 512k entries by default.

In my opinion, the status quo is acceptable. Once #31409 lands, I think the case for the trie becomes much more interesting (though the increased memory pressure needs to be investigated).

And keep in mind, this cost only occurs whenever a CIDR policy is changed (and /32 CIDRs do not trigger this). It does not affect upserts from the FQDN subsystem, does not affect upserts from the node manager and does not upserts from CEPs once #31409 lands.

Given these results, I would prefer to not block this PR on the overall worse performance and defer the trie backend as a performance optimization to later. But please let me know what you think!

[gandro]

/test

[christarazi]

Thanks for adding the benchmark! I agree with not blocking the PR. I was trying to understand just how much the impact was in case we faced issues with users and how much work it would be to move towards the trie as the mitigation.