labels/cidr: Fix labels memoization in GetCIDRLabels

The previous version of the implementation was actually computing the
labels starting from broader prefixes to narrower ones (first "/0", then
"/1" and so on).  As soon as we had a cache hit, the recursion stopped
without calculating the remaining labels for the CIDRs up to "ones".
This produced an incorrect (shorter) set of labels for a CIDR.

Also, netip.PrefixFrom(...) does not mask the internally stored address,
thus lowering the cache hit ratio even if two different CIDRs, used as
keys in the LRU cache, are equal in terms of masked address. (e.g:
"1.1.1.1/16" and "1.1.0.0/16").

So, netip.Addr.Prefix(...) is used instead.

After the fix, the performance are roughly equal (but with an increased
chance of having a cache hit). Instead, the maximum heap usage in the
worst case (LRU cache filled up with IPv6 labels) is increased 10x.

BenchmarkCIDRLabelsCacheHeapUsageIPv4
    cidr_test.go:628: Memoization map heap usage: 5483.24 KiB
BenchmarkCIDRLabelsCacheHeapUsageIPv6
    cidr_test.go:670: Memoization map heap usage: 54721.70 KiB

name                             old time/op    new time/op    delta
GetCIDRLabels/0.0.0.0/0-8           256ns ±39%     218ns ±46%     ~     (p=0.393 n=10+10)
GetCIDRLabels/10.16.0.0/16-8       1.35µs ± 3%    1.39µs ± 5%   +2.66%  (p=0.012 n=9+10)
GetCIDRLabels/192.0.2.3/32-8       2.52µs ± 2%    2.58µs ± 2%   +2.58%  (p=0.001 n=10+9)
GetCIDRLabels/192.0.2.3/24-8       2.57µs ± 1%    2.24µs ± 3%  -12.69%  (p=0.000 n=8+10)
GetCIDRLabels/192.0.2.0/24-8       2.27µs ± 4%    2.26µs ± 3%     ~     (p=0.690 n=9+8)
GetCIDRLabels/::/0-8                277ns ± 2%     278ns ± 3%     ~     (p=0.796 n=9+9)
GetCIDRLabels/fdff::ff/128-8       9.42µs ± 1%    9.34µs ± 6%     ~     (p=0.484 n=9+10)
GetCIDRLabels/f00d:42::ff/128-8    9.58µs ± 2%    9.62µs ± 7%     ~     (p=0.905 n=10+9)
GetCIDRLabels/f00d:42::ff/96-8     9.63µs ± 1%    8.45µs ± 3%  -12.27%  (p=0.000 n=8+9)
GetCIDRLabelsConcurrent/1-8         205µs ± 3%     207µs ± 3%     ~     (p=0.356 n=9+10)
GetCIDRLabelsConcurrent/2-8         385µs ± 5%     386µs ± 7%     ~     (p=0.631 n=10+10)
GetCIDRLabelsConcurrent/4-8         784µs ± 5%     780µs ± 1%     ~     (p=0.156 n=10+9)
GetCIDRLabelsConcurrent/16-8       3.24ms ± 1%    3.25ms ± 2%     ~     (p=0.529 n=10+10)
GetCIDRLabelsConcurrent/32-8       6.40ms ± 1%    6.39ms ± 3%     ~     (p=0.497 n=9+10)
GetCIDRLabelsConcurrent/48-8       9.69ms ± 1%   10.09ms ± 6%   +4.09%  (p=0.008 n=8+9)

name                             old alloc/op   new alloc/op   delta
GetCIDRLabels/0.0.0.0/0-8            624B ± 0%      624B ± 0%     ~     (all equal)
GetCIDRLabels/10.16.0.0/16-8       2.40kB ± 0%    2.40kB ± 0%     ~     (all equal)
GetCIDRLabels/192.0.2.3/32-8       5.01kB ± 0%    5.01kB ± 0%     ~     (all equal)
GetCIDRLabels/192.0.2.3/24-8       5.01kB ± 0%    4.93kB ± 0%   -1.64%  (p=0.002 n=8+10)
GetCIDRLabels/192.0.2.0/24-8       4.93kB ± 0%    4.93kB ± 0%     ~     (all equal)
GetCIDRLabels/::/0-8                 624B ± 0%      624B ± 0%     ~     (all equal)
GetCIDRLabels/fdff::ff/128-8       18.5kB ± 0%    18.5kB ± 0%     ~     (all equal)
GetCIDRLabels/f00d:42::ff/128-8    18.5kB ± 0%    18.5kB ± 0%     ~     (p=0.108 n=9+10)
GetCIDRLabels/f00d:42::ff/96-8     18.5kB ± 0%    18.5kB ± 0%   -0.06%  (p=0.000 n=10+10)
GetCIDRLabelsConcurrent/1-8         321kB ± 0%     321kB ± 0%     ~     (p=0.127 n=10+8)
GetCIDRLabelsConcurrent/2-8         641kB ± 0%     641kB ± 0%     ~     (p=0.928 n=10+10)
GetCIDRLabelsConcurrent/4-8        1.28MB ± 0%    1.28MB ± 0%     ~     (p=0.853 n=10+10)
GetCIDRLabelsConcurrent/16-8       5.13MB ± 0%    5.13MB ± 0%     ~     (p=0.739 n=10+10)
GetCIDRLabelsConcurrent/32-8       10.3MB ± 0%    10.3MB ± 0%     ~     (p=0.218 n=10+10)
GetCIDRLabelsConcurrent/48-8       15.4MB ± 0%    15.4MB ± 0%     ~     (p=0.218 n=10+10)

name                             old allocs/op  new allocs/op  delta
GetCIDRLabels/0.0.0.0/0-8            2.00 ± 0%      2.00 ± 0%     ~     (all equal)
GetCIDRLabels/10.16.0.0/16-8         2.00 ± 0%      2.00 ± 0%     ~     (all equal)
GetCIDRLabels/192.0.2.3/32-8         2.00 ± 0%      2.00 ± 0%     ~     (all equal)
GetCIDRLabels/192.0.2.3/24-8         2.00 ± 0%      2.00 ± 0%     ~     (all equal)
GetCIDRLabels/192.0.2.0/24-8         2.00 ± 0%      2.00 ± 0%     ~     (all equal)
GetCIDRLabels/::/0-8                 2.00 ± 0%      2.00 ± 0%     ~     (all equal)
GetCIDRLabels/fdff::ff/128-8         3.00 ± 0%      3.00 ± 0%     ~     (all equal)
GetCIDRLabels/f00d:42::ff/128-8      3.00 ± 0%      3.00 ± 0%     ~     (all equal)
GetCIDRLabels/f00d:42::ff/96-8       3.00 ± 0%      3.00 ± 0%     ~     (all equal)
GetCIDRLabelsConcurrent/1-8           138 ± 0%       138 ± 0%     ~     (all equal)
GetCIDRLabelsConcurrent/2-8           277 ± 0%       277 ± 0%     ~     (all equal)
GetCIDRLabelsConcurrent/4-8           555 ± 0%       555 ± 0%     ~     (p=0.248 n=10+9)
GetCIDRLabelsConcurrent/16-8        2.22k ± 0%     2.22k ± 0%     ~     (p=0.353 n=7+10)
GetCIDRLabelsConcurrent/32-8        4.44k ± 0%     4.44k ± 0%     ~     (p=0.723 n=10+10)
GetCIDRLabelsConcurrent/48-8        6.66k ± 0%     6.66k ± 0%     ~     (p=0.090 n=10+9)

Fixes: e0f6c47 ("labels/cidr: Cache GetCIDRLabels computation")

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>

pkg/labels/cidr.go

@@ -114,7 +114,6 @@ func GetCIDRLabels(prefix netip.Prefix) Labels {
 		nil, // avoid allocating space for the intermediate results until we need it
 		addr,
 		ones,
-		0,
 	)
 	addWorldLabel(addr, lbls)
 
@@ -155,12 +154,12 @@ var (
 	worldLabelV6           = Label{Source: LabelSourceReserved, Key: IDNameWorldIPv6}
 )
 
-func computeCIDRLabels(cache *simplelru.LRU[netip.Prefix, []Label], lbls Labels, results []Label, addr netip.Addr, ones, i int) []Label {
-	if i > ones {
+func computeCIDRLabels(cache *simplelru.LRU[netip.Prefix, []Label], lbls Labels, results []Label, addr netip.Addr, ones int) []Label {
+	if ones < 0 {
 		return results
 	}
 
-	prefix := netip.PrefixFrom(addr, i)
+	prefix, _ := addr.Prefix(ones)
 
 	mu.Lock()
 	cachedLbls, ok := cache.Get(prefix)
@@ -177,20 +176,20 @@ func computeCIDRLabels(cache *simplelru.LRU[netip.Prefix, []Label], lbls Labels,
 	}
 
 	// Compute the label for this prefix (e.g. "cidr:10.0.0.0/8")
-	prefixLabel := maskedIPToLabel(prefix.Masked().Addr(), i)
+	prefixLabel := maskedIPToLabel(prefix.Addr(), ones)
 	lbls[prefixLabel.Key] = prefixLabel
 
 	// Keep computing the rest (e.g. "cidr:10.0.0.0/7", ...).
 	results = computeCIDRLabels(
 		cache,
 		lbls,
 		append(results, prefixLabel),
-		addr, ones, i+1,
+		prefix.Addr(), ones-1,
 	)
 
 	// Cache the resulting labels derived from this prefix, e.g. /8, /7, ...
 	mu.Lock()
-	cache.Add(prefix, results[i:])
+	cache.Add(prefix, results[len(results)-ones-1:])
 	mu.Unlock()
 
 	return results