ipcache/md: Convert metadata key to netip.Prefix

This commit moves most of the newer ipcache logic over to "net/ipnet" so
that we can have stronger types when dealing with later additions to the
core IP structures. Previously with the string keys, the expectation was
that users of the metadata cache would insert IP addresses rather than
prefixes. However, features like CIDR policy need to insert using
prefixes. We could ambiguously insert with either IPs or CIDRs like the
main IPCache structure does, but this can easily introduce subtle bugs.
Instead, swap out the key for the ipcache.metadata structure for
netip.Prefix to force use of prefixes as the key even for existing use
cases that are based around IP addresses. As a side benefit, netip can
be used as map keys and it's supposed to be more efficient than the
prior net types.

This commit does not currently swap the main IPCache structure over to
net/ipnet, as this would force us to deal with conflicts between IP and
prefix keys in the main map. We can first introduce this in the metadata
cache, then introduce a newer IPCache metadata API that handles
conflicts (upcoming commit), migrate the existing codepaths that inject
into the ipcache to the new API, and then finally migrate the main
IPCache structure over to net/netip types.

Co-authored-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Joe Stringer <joe@cilium.io>

daemon/cmd/datapath.go

@@ -6,6 +6,7 @@ package cmd
 import (
 	"fmt"
 	"net"
+	"net/netip"
 	"os"
 	"strings"
 	"time"
@@ -257,7 +258,7 @@ func (d *Daemon) syncEndpointsAndHostIPs() error {
 		// ipcache. Until then, it is expected to succeed.
 		d.ipcache.Upsert(ipIDPair.PrefixString(), nil, hostKey, nil, ipcache.Identity{
 			ID:     ipIDPair.ID,
-			Source: d.sourceByIP(ipIDPair.IP.String(), source.Local),
+			Source: d.sourceByIP(ipIDPair.IP, source.Local),
 		})
 	}
 
@@ -271,7 +272,7 @@ func (d *Daemon) syncEndpointsAndHostIPs() error {
 				log.Debugf("Removed outdated host ip %s from endpoint map", hostIP)
 			}
 
-			d.ipcache.Delete(hostIP, d.sourceByIP(hostIP, source.Local))
+			d.ipcache.Delete(hostIP, d.sourceByIP(ip, source.Local))
 		}
 	}
 
@@ -290,11 +291,16 @@ func (d *Daemon) syncEndpointsAndHostIPs() error {
 	return nil
 }
 
-func (d *Daemon) sourceByIP(prefix string, defaultSrc source.Source) source.Source {
-	if lbls := d.ipcache.GetIDMetadataByIP(prefix); lbls.Has(
-		labels.LabelKubeAPIServer[labels.IDNameKubeAPIServer],
-	) {
-		return source.KubeAPIServer
+func (d *Daemon) sourceByIP(ip net.IP, defaultSrc source.Source) source.Source {
+	if addr, ok := netip.AddrFromSlice(ip); ok {
+		lbls := d.ipcache.GetIDMetadataByIP(addr)
+		if lbls.Has(labels.LabelKubeAPIServer[labels.IDNameKubeAPIServer]) {
+			return source.KubeAPIServer
+		}
+	} else {
+		log.WithFields(logrus.Fields{
+			logfields.IPAddr: ip,
+		}).Warning("BUG: Invalid addr detected in host stack. Please report this bug to the Cilium developers.")
 	}
 	return defaultSrc
 }

pkg/ipcache/cidr.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"net/netip"
 	"strings"
 
 	"github.com/sirupsen/logrus"
@@ -52,30 +53,30 @@ func (ipc *IPCache) AllocateCIDRs(
 	defer cancel()
 
 	ipc.Lock()
-	allocatedIdentities := make(map[string]*identity.Identity, len(prefixes))
+	allocatedIdentities := make(map[netip.Prefix]*identity.Identity, len(prefixes))
 	for i, p := range prefixes {
 		if p == nil {
 			continue
 		}
 
 		lbls := cidr.GetCIDRLabels(p)
-		lbls.MergeLabels(ipc.GetIDMetadataByIP(p.IP.String()))
+		prefix := ip.IPNetToPrefix(p)
+		lbls.MergeLabels(ipc.metadata.get(prefix).ToLabels())
 		oldNID := identity.InvalidIdentity
 		if oldNIDs != nil && len(oldNIDs) > i {
 			oldNID = oldNIDs[i]
 		}
-		id, isNew, err := ipc.allocate(allocateCtx, p, lbls, oldNID)
+		id, isNew, err := ipc.allocate(allocateCtx, prefix, lbls, oldNID)
 		if err != nil {
 			ipc.IdentityAllocator.ReleaseSlice(context.Background(), nil, usedIdentities)
 			ipc.Unlock()
 			return nil, err
 		}
 
-		prefixStr := p.String()
 		usedIdentities = append(usedIdentities, id)
-		allocatedIdentities[prefixStr] = id
+		allocatedIdentities[prefix] = id
 		if isNew {
-			newlyAllocatedIdentities[prefixStr] = id
+			newlyAllocatedIdentities[prefix.String()] = id
 		}
 	}
 	ipc.Unlock()
@@ -169,11 +170,7 @@ func (ipc *IPCache) UpsertGeneratedIdentities(newlyAllocatedIdentities map[strin
 //
 // It is up to the caller to provide the full set of labels for identity
 // allocation.
-func (ipc *IPCache) allocate(ctx context.Context, prefix *net.IPNet, lbls labels.Labels, oldNID identity.NumericIdentity) (*identity.Identity, bool, error) {
-	if prefix == nil {
-		return nil, false, nil
-	}
-
+func (ipc *IPCache) allocate(ctx context.Context, prefix netip.Prefix, lbls labels.Labels, oldNID identity.NumericIdentity) (*identity.Identity, bool, error) {
 	id, isNew, err := ipc.IdentityAllocator.AllocateIdentity(ctx, lbls, false, oldNID)
 	if err != nil {
 		return nil, isNew, fmt.Errorf("failed to allocate identity for cidr %s: %s", prefix, err)
@@ -186,7 +183,7 @@ func (ipc *IPCache) allocate(ctx context.Context, prefix *net.IPNet, lbls labels
 	return id, isNew, err
 }
 
-func (ipc *IPCache) releaseCIDRIdentities(ctx context.Context, identities map[string]*identity.Identity) {
+func (ipc *IPCache) releaseCIDRIdentities(ctx context.Context, identities map[netip.Prefix]*identity.Identity) {
 	// Create a critical section for identity release + removal from ipcache.
 	// Otherwise, it's possible to trigger the following race condition:
 	//
@@ -212,7 +209,7 @@ func (ipc *IPCache) releaseCIDRIdentities(ctx context.Context, identities map[st
 		}
 
 		if released {
-			ipc.deleteLocked(prefix, source.Generated)
+			ipc.deleteLocked(prefix.String(), source.Generated)
 		}
 	}
 }
@@ -224,16 +221,17 @@ func (ipc *IPCache) ReleaseCIDRIdentitiesByCIDR(prefixes []*net.IPNet) {
 	releaseCtx, cancel := context.WithTimeout(context.TODO(), option.Config.KVstoreConnectivityTimeout)
 	defer cancel()
 
-	identities := make(map[string]*identity.Identity, len(prefixes))
+	identities := make(map[netip.Prefix]*identity.Identity, len(prefixes))
 	for _, prefix := range prefixes {
 		if prefix == nil {
 			continue
 		}
 
+		p := ip.IPNetToPrefix(prefix)
 		if id := ipc.IdentityAllocator.LookupIdentity(releaseCtx, cidr.GetCIDRLabels(prefix)); id != nil {
-			identities[prefix.String()] = id
+			identities[p] = id
 		} else {
-			log.Errorf("Unable to find identity of previously used CIDR %s", prefix.String())
+			log.Errorf("Unable to find identity of previously used CIDR %s", p.String())
 		}
 	}
 
@@ -243,7 +241,7 @@ func (ipc *IPCache) ReleaseCIDRIdentitiesByCIDR(prefixes []*net.IPNet) {
 // ReleaseCIDRIdentitiesByID releases the specified identities.
 // When the last use of the identity is released, the ipcache entry is deleted.
 func (ipc *IPCache) ReleaseCIDRIdentitiesByID(ctx context.Context, identities []identity.NumericIdentity) {
-	fullIdentities := make(map[string]*identity.Identity, len(identities))
+	fullIdentities := make(map[netip.Prefix]*identity.Identity, len(identities))
 	for _, nid := range identities {
 		if id := ipc.IdentityAllocator.LookupIdentityByID(ctx, nid); id != nil {
 			cidr, ok := cidrLabelToPrefix(id.CIDRLabel.String())
@@ -254,7 +252,15 @@ func (ipc *IPCache) ReleaseCIDRIdentitiesByID(ctx context.Context, identities []
 				}).Warn("Unexpected release of non-CIDR identity, will leak this identity. Please report this issue to the developers.")
 				continue
 			}
-			fullIdentities[cidr] = id
+			prefix, err := netip.ParsePrefix(strings.TrimPrefix(cidr, labels.LabelSourceCIDR+":"))
+			if err != nil {
+				log.WithFields(logrus.Fields{
+					logfields.Identity: nid,
+					logfields.Labels:   id.Labels,
+				}).Warn("BUG: Cannot parse prefix from CIDR label during CIDR identity release. Please report this issue to the developers.")
+				continue
+			}
+			fullIdentities[prefix] = id
 		} else {
 			log.WithFields(logrus.Fields{
 				logfields.Identity: nid,