From 182761cdc05e04b4fe803876fbaede692f111c20 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Wed, 20 Aug 2025 00:05:32 -0700 Subject: [PATCH 01/13] feat: Use PCI devices as the base for discovery This change refactors the device discovery mechanism to use PCI devices as the fundamental unit of inventory. Network interface attributes are then discovered and associated with these PCI devices. For Network interfaces which are not associated with a PCI Device (like virtual interfaces), they are added as their own device. Previously, network interfaces were the primary resource. However, when a network interface is moved into a pod's network namespace, it disappears from the host's view, causing the resource to be removed from the node's ResourceSlice. This could lead to race conditions and incorrect device availability information. By treating the PCI device as the stable, base resource, we ensure that the device remains visible in the ResourceSlice even when its associated network interface is moved. This commit also introduces the `ghw` library for PCI device discovery and removes the manual sysfs parsing. --- go.mod | 6 + go.sum | 16 ++ pkg/driver/dra_hooks.go | 27 ++- pkg/driver/nri_hooks.go | 37 ++-- pkg/inventory/db.go | 363 +++++++++++++++++++++++----------------- pkg/inventory/sysfs.go | 44 ----- pkg/names/names.go | 44 +++-- pkg/names/names_test.go | 46 +++-- 8 files changed, 318 insertions(+), 265 deletions(-) diff --git a/go.mod b/go.mod index 43bcf897..12aa24af 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/google/cel-go v0.26.0 github.com/google/go-cmp v0.7.0 github.com/insomniacslk/dhcp v0.0.0-20250417080101-5f8cf70e8c5f + github.com/jaypipes/ghw v0.17.0 github.com/mdlayher/genetlink v1.3.2 github.com/mdlayher/netlink v1.7.2 github.com/prometheus/client_golang v1.23.0 @@ -38,6 +39,7 @@ require ( cel.dev/expr v0.24.0 // indirect cloud.google.com/go/auth v0.16.5 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect + github.com/StackExchange/wmi v1.2.1 // indirect github.com/antlr4-go/antlr/v4 v4.13.1 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect @@ -49,6 +51,7 @@ require ( github.com/fxamacker/cbor/v2 v2.9.0 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect + github.com/go-ole/go-ole v1.2.6 // indirect github.com/go-openapi/jsonpointer v0.21.1 // indirect github.com/go-openapi/jsonreference v0.21.0 // indirect github.com/go-openapi/swag v0.23.1 // indirect @@ -59,6 +62,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect github.com/googleapis/gax-go/v2 v2.15.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/jaypipes/pcidb v1.0.1 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/josharian/native v1.1.0 // indirect github.com/json-iterator/go v1.1.12 // indirect @@ -66,6 +70,7 @@ require ( github.com/mailru/easyjson v0.9.0 // indirect github.com/mdlayher/packet v1.1.2 // indirect github.com/mdlayher/socket v0.5.1 // indirect + github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect @@ -107,6 +112,7 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect + howett.net/plist v1.0.0 // indirect k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect k8s.io/kubelet v0.34.0-rc.0 // indirect sigs.k8s.io/randfill v1.0.0 // indirect diff --git a/go.sum b/go.sum index 4ccc941d..c9262046 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,8 @@ cloud.google.com/go/container v1.44.0 h1:JEHeW535svvNwJrjrlQ/cdjd15LCWrPKnHsulru cloud.google.com/go/container v1.44.0/go.mod h1:tVK2o4UZUTkg9WpBcgj4qRzwGA1dSFdWA3mil3YkLIQ= github.com/Mellanox/rdmamap v1.1.0 h1:A/W1wAXw+6vm58f3VklrIylgV+eDJlPVIMaIKuxgUT4= github.com/Mellanox/rdmamap v1.1.0/go.mod h1:fN+/V9lf10ABnDCwTaXRjeeWijLt2iVLETnK+sx/LY8= +github.com/StackExchange/wmi v1.2.1 h1:VIkavFPXSjcnS+O8yTq7NI32k0R5Aj+v39y29VYDOSA= +github.com/StackExchange/wmi v1.2.1/go.mod h1:rcmrprowKIVzvc+NUiLncP2uuArMWLCbu9SBzvHz7e8= github.com/antlr4-go/antlr/v4 v4.13.1 h1:SqQKkuVZ+zWkMMNkjy5FZe5mr5WURWnlpmOuzYWrPrQ= github.com/antlr4-go/antlr/v4 v4.13.1/go.mod h1:GKmUxMtwp6ZgGwZSva4eWPC5mS6vUAmOABFgjdkM7Nw= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= @@ -44,6 +46,9 @@ github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/go-ole/go-ole v1.2.5/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= +github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY= +github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-openapi/jsonpointer v0.21.1 h1:whnzv/pNXtK2FbX/W9yJfRmE2gsmkfahjMKB0fZvcic= github.com/go-openapi/jsonpointer v0.21.1/go.mod h1:50I1STOfbY1ycR8jGz8DaMeLCdXiI6aDteEdRNNzpdk= github.com/go-openapi/jsonreference v0.21.0 h1:Rs+Y7hSXT83Jacb7kFyjn4ijOuVGSvOdF2+tg1TRrwQ= @@ -81,6 +86,11 @@ github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/insomniacslk/dhcp v0.0.0-20250417080101-5f8cf70e8c5f h1:dd33oobuIv9PcBVqvbEiCXEbNTomOHyj3WFuC5YiPRU= github.com/insomniacslk/dhcp v0.0.0-20250417080101-5f8cf70e8c5f/go.mod h1:zhFlBeJssZ1YBCMZ5Lzu1pX4vhftDvU10WUVb1uXKtM= +github.com/jaypipes/ghw v0.17.0 h1:EVLJeNcy5z6GK/Lqby0EhBpynZo+ayl8iJWY0kbEUJA= +github.com/jaypipes/ghw v0.17.0/go.mod h1:In8SsaDqlb1oTyrbmTC14uy+fbBMvp+xdqX51MidlD8= +github.com/jaypipes/pcidb v1.0.1 h1:WB2zh27T3nwg8AE8ei81sNRb9yWBii3JGNJtT7K9Oic= +github.com/jaypipes/pcidb v1.0.1/go.mod h1:6xYUz/yYEyOkIkUt2t2J2folIuZ4Yg6uByCGFXMCeE4= +github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/josharian/native v1.1.0 h1:uuaP0hAbW7Y4l0ZRQ6C9zfb7Mg1mbFKry/xzDAfmtLA= @@ -112,6 +122,8 @@ github.com/mdlayher/packet v1.1.2 h1:3Up1NG6LZrsgDVn6X4L9Ge/iyRyxFEFD9o6Pr3Q1nQY github.com/mdlayher/packet v1.1.2/go.mod h1:GEu1+n9sG5VtiRE4SydOmX5GTwyyYlteZiFU+x0kew4= github.com/mdlayher/socket v0.5.1 h1:VZaqt6RkGkt2OE9l3GcC6nZkqD3xKeQLyfleW/uBcos= github.com/mdlayher/socket v0.5.1/go.mod h1:TjPLHI1UgwEv5J1B5q0zTZq12A/6H7nKmtTanQE37IQ= +github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= +github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -232,6 +244,7 @@ golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190606203320-7fc4e5ec1444/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -275,9 +288,12 @@ gopkg.in/evanphx/json-patch.v4 v4.12.0 h1:n6jtcsulIzXPJaxegRbvFNNrZDjbij7ny3gmSP gopkg.in/evanphx/json-patch.v4 v4.12.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= +gopkg.in/yaml.v1 v1.0.0-20140924161607-9f9df34309c0/go.mod h1:WDnlLJ4WF5VGsH/HVa3CI79GS0ol3YnhVnKP89i0kNg= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +howett.net/plist v1.0.0 h1:7CrbWYbPPO/PyNy38b2EB/+gYbjCe2DXBxgtOOZbSQM= +howett.net/plist v1.0.0/go.mod h1:lqaXoTrLY4hg8tnEzNru53gicrbv7rrk+2xJA/7hw9g= k8s.io/api v0.34.0-rc.0 h1:0jrCG9TkH250E5wTqchpijMoMxmQ6FEl9xNum25FFSk= k8s.io/api v0.34.0-rc.0/go.mod h1:H2VCZSAruVExfN0k+jj42LXFGQOvRhZDVbY1JlHGtHE= k8s.io/apimachinery v0.34.0-rc.0 h1:PjXj9Fp/f53PN7JMtGnR7AldAbdztqNqT3SjABe99D8= diff --git a/pkg/driver/dra_hooks.go b/pkg/driver/dra_hooks.go index 018380b1..3488b28f 100644 --- a/pkg/driver/dra_hooks.go +++ b/pkg/driver/dra_hooks.go @@ -25,7 +25,6 @@ import ( "github.com/google/dranet/pkg/apis" "github.com/google/dranet/pkg/filter" - "github.com/google/dranet/pkg/names" "github.com/Mellanox/rdmamap" "github.com/vishvananda/netlink" @@ -165,11 +164,26 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour }, Network: netconf, } - ifName := names.GetOriginalName(result.Device) + // TODO(gauravkghildiyal): There's a (self-resolving) race condition + // here which is triggered when a pod claiming a device is rapidly + // deleted and recreated. When the pod is deleted, + // UnprepareResourceClaim deletes the claim and does not wait for the + // network device to be freed up. Deletion of the claim indicates to the + // kube-scheduler that the device is available for future use so it can + // assign the same device to the newly created pod. But it's possible + // that the old pod's sandbox still exists (and is in the process of + // being deleted) so the network interface is still in the old pod's + // network namespace. Although the problem self resolves when kubelet + // retries PrepareResourceClaims for the new pod, but it should be + // possible to instrument a better lifecycle. + ifName, err := np.netdb.GetNetInterfaceName(result.Device) + if err != nil { + errorList = append(errorList, fmt.Errorf("failed to get network interface name for device %s: %v", ifName, err)) + } // Get Network configuration and merge it link, err := nlHandle.LinkByName(ifName) if err != nil { - errorList = append(errorList, fmt.Errorf("fail to get network interface %s", ifName)) + errorList = append(errorList, fmt.Errorf("failed to get netlink to interface %s: %v", ifName, err)) continue } @@ -298,15 +312,10 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour } } - device := kubeletplugin.Device{ - Requests: []string{result.Request}, - PoolName: result.Pool, - DeviceName: result.Device, - } // TODO: support for multiple pods sharing the same device // we'll create the subinterface here for _, uid := range podUIDs { - np.podConfigStore.Set(uid, device.DeviceName, podCfg) + np.podConfigStore.Set(uid, result.Device, podCfg) } klog.V(4).Infof("Claim Resources for pods %v : %#v", podUIDs, podCfg) } diff --git a/pkg/driver/nri_hooks.go b/pkg/driver/nri_hooks.go index 0c1ea27b..a87a7e8b 100644 --- a/pkg/driver/nri_hooks.go +++ b/pkg/driver/nri_hooks.go @@ -21,8 +21,6 @@ import ( "fmt" "time" - "github.com/google/dranet/pkg/names" - "github.com/containerd/nri/pkg/api" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -52,7 +50,7 @@ func (np *NetworkDriver) Synchronize(_ context.Context, pods []*api.PodSandbox, // host network pods are skipped if ns != "" { // store the Pod metadata in the db - np.netdb.AddPodNetns(podKey(pod), ns) + np.netdb.AddPodNetNs(podKey(pod), ns) } } @@ -106,7 +104,7 @@ func (np *NetworkDriver) RunPodSandbox(ctx context.Context, pod *api.PodSandbox) return fmt.Errorf("RunPodSandbox pod %s/%s using host network can not claim host devices", pod.Namespace, pod.Name) } // store the Pod metadata in the db - np.netdb.AddPodNetns(podKey(pod), ns) + np.netdb.AddPodNetNs(podKey(pod), ns) // Track all the status updates needed for the resource claims of the pod. statusUpdates := map[types.NamespacedName]*resourceapply.ResourceClaimStatusApplyConfiguration{} @@ -126,7 +124,13 @@ func (np *NetworkDriver) RunPodSandbox(ctx context.Context, pod *api.PodSandbox) WithDriver(np.driverName). WithPool(np.nodeName) - ifName := names.GetOriginalName(deviceName) + ifName, err := np.netdb.GetNetInterfaceName(deviceName) + if err != nil { + // Tip: We desired, tweak the code here to simply log + // this without returning and continue exeuction for cases where the + // device has no associated network interface. + return fmt.Errorf("failed to get network interface %s", ifName) + } klog.V(2).Infof("RunPodSandbox processing Network device: %s", ifName) // TODO config options to rename the device and pass parameters @@ -232,7 +236,7 @@ func (np *NetworkDriver) StopPodSandbox(ctx context.Context, pod *api.PodSandbox klog.V(2).Infof("StopPodSandbox Pod %s/%s UID %s", pod.Namespace, pod.Name, pod.Uid) start := time.Now() defer func() { - np.netdb.RemovePodNetns(podKey(pod)) + np.netdb.RemovePodNetNs(podKey(pod)) klog.V(2).Infof("StopPodSandbox Pod %s/%s UID %s took %v", pod.Namespace, pod.Name, pod.Uid, time.Since(start)) }() @@ -247,7 +251,7 @@ func (np *NetworkDriver) StopPodSandbox(ctx context.Context, pod *api.PodSandbox // some version of containerd does not send the network namespace information on this hook so // we workaround it using the local copy we have in the db to associate interfaces with Pods via // the network namespace id. - ns = np.netdb.GetPodNamespace(podKey(pod)) + ns = np.netdb.GetPodNetNs(podKey(pod)) if ns == "" { klog.Infof("StopPodSandbox pod %s/%s using host network ... skipping", pod.Namespace, pod.Name) return nil @@ -255,10 +259,19 @@ func (np *NetworkDriver) StopPodSandbox(ctx context.Context, pod *api.PodSandbox } for deviceName, config := range podConfig { - ifName := names.GetOriginalName(deviceName) - - if err := nsDetachNetdev(ns, config.Network.Interface.Name, ifName); err != nil { - klog.Infof("fail to return network device %s : %v", deviceName, err) + // TODO(gauravkghildiyal): Interface name here will in most cases be + // empty which means we're mostly relying on the fallback of the kernel + // having to do this cleanup. That is hopefully not the worst thing + // which could happen, but there are easy ways to improve this + // behaviour, like detaching the specific network interface which has + // the the given PCI address (i.e. deviceName is the normalized PCI address). + ifName, err := np.netdb.GetNetInterfaceName(deviceName) + if err == nil { + if err := nsDetachNetdev(ns, config.Network.Interface.Name, ifName); err != nil { + klog.Infof("fail to return network device %s : %v", deviceName, err) + } + } else { + klog.V(2).Infof("Failed to identify network interface for device %s: %v; expecting kernel to do the cleanup", deviceName, err) } if !np.rdmaSharedMode && config.RDMADevice.LinkDev != "" { @@ -272,7 +285,7 @@ func (np *NetworkDriver) StopPodSandbox(ctx context.Context, pod *api.PodSandbox func (np *NetworkDriver) RemovePodSandbox(_ context.Context, pod *api.PodSandbox) error { klog.V(2).Infof("RemovePodSandbox Pod %s/%s UID %s", pod.Namespace, pod.Name, pod.Uid) - np.netdb.RemovePodNetns(podKey(pod)) + np.netdb.RemovePodNetNs(podKey(pod)) return nil } diff --git a/pkg/inventory/db.go b/pkg/inventory/db.go index 70320ec3..357c521e 100644 --- a/pkg/inventory/db.go +++ b/pkg/inventory/db.go @@ -19,14 +19,18 @@ package inventory import ( "context" "fmt" + "maps" "net" "strings" "sync" "time" - "github.com/Mellanox/rdmamap" + "github.com/google/dranet/pkg/apis" "github.com/google/dranet/pkg/cloudprovider" "github.com/google/dranet/pkg/names" + + "github.com/Mellanox/rdmamap" + "github.com/jaypipes/ghw" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" "golang.org/x/time/rate" @@ -52,9 +56,15 @@ var ( type DB struct { instance *cloudprovider.CloudInstance - mu sync.RWMutex - podStore map[int]string // key: netnsid path value: Pod namespace/name - podNsStore map[string]string // key pod value: netns path + mu sync.RWMutex + // netNsForPod gives the network namespace for a pod, indexed by the pods + // "namespaced/name". + netNsForPod map[string]string + // deviceStore is an in-memory cache of the available devices on the node. + // It is keyed by the normalized PCI address of the device. The value is a + // resourceapi.Device object that contains the device's attributes. + // The deviceStore is periodically updated by the Run method. + deviceStore map[string]resourceapi.Device rateLimiter *rate.Limiter notifications chan []resourceapi.Device @@ -63,65 +73,41 @@ type DB struct { func New() *DB { return &DB{ + netNsForPod: map[string]string{}, + deviceStore: map[string]resourceapi.Device{}, rateLimiter: rate.NewLimiter(rate.Every(minInterval), 1), - podStore: map[int]string{}, - podNsStore: map[string]string{}, notifications: make(chan []resourceapi.Device), } } -func (db *DB) AddPodNetns(pod string, netnsPath string) { +func (db *DB) AddPodNetNs(pod string, netNsPath string) { db.mu.Lock() defer db.mu.Unlock() - ns, err := netns.GetFromPath(netnsPath) + ns, err := netns.GetFromPath(netNsPath) if err != nil { - klog.Infof("fail to get pod %s network namespace %s handle: %v", pod, netnsPath, err) + klog.Infof("Failed to get pod %s network namespace %s handle: %v", pod, netNsPath, err) return } defer ns.Close() - id, err := netlink.GetNetNsIdByFd(int(ns)) - if err != nil { - klog.Infof("fail to get pod %s network namespace %s netnsid: %v", pod, netnsPath, err) - return - } - db.podStore[id] = pod - db.podNsStore[pod] = netnsPath + db.netNsForPod[pod] = netNsPath } -func (db *DB) RemovePodNetns(pod string) { +func (db *DB) RemovePodNetNs(pod string) { db.mu.Lock() defer db.mu.Unlock() - delete(db.podNsStore, pod) - for k, v := range db.podStore { - if v == pod { - delete(db.podStore, k) - return - } - } -} - -// GetPodName allows to get the Pod name from the namespace Id -// that comes in the link id from the veth pair interface -func (db *DB) GetPodName(netnsid int) string { - db.mu.RLock() - defer db.mu.RUnlock() - return db.podStore[netnsid] + delete(db.netNsForPod, pod) } // GetPodNamespace allows to get the Pod network namespace -func (db *DB) GetPodNamespace(pod string) string { +func (db *DB) GetPodNetNs(pod string) string { db.mu.RLock() defer db.mu.RUnlock() - return db.podNsStore[pod] + return db.netNsForPod[pod] } func (db *DB) Run(ctx context.Context) error { defer close(db.notifications) - nlHandle, err := netlink.NewHandle() - if err != nil { - return fmt.Errorf("error creating netlink handle %v", err) - } // Resources are published periodically or if there is a netlink notification // indicating a new interfaces was added or changed nlChannel := make(chan netlink.LinkUpdate) @@ -144,43 +130,27 @@ func (db *DB) Run(ctx context.Context) error { klog.Error(err, "unexpected rate limited error trying to get system interfaces") } - devices := []resourceapi.Device{} - ifaces, err := nlHandle.LinkList() - if err != nil { - klog.Error(err, "unexpected error trying to get system interfaces") - } - for _, iface := range ifaces { - klog.V(7).InfoS("Checking network interface", "name", iface.Attrs().Name) - if gwInterfaces.Has(iface.Attrs().Name) { - klog.V(4).Infof("iface %s is an uplink interface", iface.Attrs().Name) - continue - } - - if ignoredInterfaceNames.Has(iface.Attrs().Name) { - klog.V(4).Infof("iface %s is in the list of ignored interfaces", iface.Attrs().Name) - continue - } - - // skip loopback interfaces - if iface.Attrs().Flags&net.FlagLoopback != 0 { - continue - } - - // publish this network interface - device, err := db.netdevToDRAdev(iface) - if err != nil { - klog.V(2).Infof("could not obtain attributes for iface %s : %v", iface.Attrs().Name, err) + devices := db.discoverPCIDevices() + devices = db.discoverNetworkInterfaces(devices) + devices = db.discoverRDMADevices(devices) + devices = db.addCloudAttributes(devices) + + // Remove default interface. + filteredDevices := []resourceapi.Device{} + for _, device := range devices { + ifName := device.Attributes[apis.AttrInterfaceName].StringValue + if ifName != nil && gwInterfaces.Has(string(*ifName)) { + klog.V(4).Infof("Ignoring interface %s from discovery since it is an uplink interface", *ifName) continue } - - devices = append(devices, *device) - klog.V(4).Infof("Found following network interface %s", iface.Attrs().Name) + filteredDevices = append(filteredDevices, device) } - klog.V(4).Infof("Found %d devices", len(devices)) - if len(devices) > 0 || db.hasDevices { - db.hasDevices = len(devices) > 0 - db.notifications <- devices + klog.V(4).Infof("Found %d devices", len(filteredDevices)) + if len(filteredDevices) > 0 || db.hasDevices { + db.hasDevices = len(filteredDevices) > 0 + db.updateDeviceStore(filteredDevices) + db.notifications <- filteredDevices } select { // trigger a reconcile @@ -200,26 +170,123 @@ func (db *DB) GetResources(ctx context.Context) <-chan []resourceapi.Device { return db.notifications } -func (db *DB) netdevToDRAdev(link netlink.Link) (*resourceapi.Device, error) { - ifName := link.Attrs().Name - device := resourceapi.Device{ - Attributes: make(map[resourceapi.QualifiedName]resourceapi.DeviceAttribute), - Capacity: make(map[resourceapi.QualifiedName]resourceapi.DeviceCapacity), +func (db *DB) discoverPCIDevices() []resourceapi.Device { + devices := []resourceapi.Device{} + + pci, err := ghw.PCI() + if err != nil { + klog.Errorf("Could not get PCI devices: %v", err) + return devices + } + + for _, pciDev := range pci.Devices { + if !isNetworkDevice(pciDev) { + continue + } + device := resourceapi.Device{ + Name: names.NormalizePCIAddress(pciDev.Address), + Attributes: make(map[resourceapi.QualifiedName]resourceapi.DeviceAttribute), + Capacity: make(map[resourceapi.QualifiedName]resourceapi.DeviceCapacity), + } + device.Attributes[apis.AttrPCIAddress] = resourceapi.DeviceAttribute{StringValue: &pciDev.Address} + if pciDev.Vendor != nil { + device.Attributes[apis.AttrPCIVendor] = resourceapi.DeviceAttribute{StringValue: &pciDev.Vendor.Name} + } + if pciDev.Product != nil { + device.Attributes[apis.AttrPCIDevice] = resourceapi.DeviceAttribute{StringValue: &pciDev.Product.Name} + } + if pciDev.Subsystem != nil { + device.Attributes[apis.AttrPCISubsystem] = resourceapi.DeviceAttribute{StringValue: &pciDev.Subsystem.ID} + } + + if pciDev.Node != nil { + device.Attributes[apis.AttrNUMANode] = resourceapi.DeviceAttribute{IntValue: ptr.To(int64(pciDev.Node.ID))} + } + + pcieRootAttr, err := deviceattribute.GetPCIeRootAttributeByPCIBusID(pciDev.Address) + if err != nil { + klog.Infof("Could not get pci root attribute: %v", err) + } else { + device.Attributes[pcieRootAttr.Name] = pcieRootAttr.Value + } + devices = append(devices, device) + } + return devices +} + +func (db *DB) discoverNetworkInterfaces(pciDevices []resourceapi.Device) []resourceapi.Device { + links, err := netlink.LinkList() + if err != nil { + klog.Errorf("Could not list network interfaces: %v", err) + return pciDevices } - // Set the device name. It will be normalized only if necessary. - device.Name = names.SetDeviceName(ifName) - // expose the real interface name as an attribute in case it is normalized. - device.Attributes["dra.net/ifName"] = resourceapi.DeviceAttribute{StringValue: &ifName} - - linkType := link.Type() - linkAttrs := link.Attrs() - - // identify the namespace holding the link as the other end of a veth pair - netnsid := link.Attrs().NetNsID - if podName := db.GetPodName(netnsid); podName != "" { - device.Attributes["dra.net/pod"] = resourceapi.DeviceAttribute{StringValue: &podName} + + pciDeviceMap := make(map[string]*resourceapi.Device) + for i := range pciDevices { + pciDeviceMap[pciDevices[i].Name] = &pciDevices[i] } + otherDevices := []resourceapi.Device{} + + for _, link := range links { + ifName := link.Attrs().Name + if ignoredInterfaceNames.Has(ifName) { + klog.V(4).Infof("Network Interface %s is in the list of ignored interfaces, excluding it from discovery", ifName) + continue + } + + // skip loopback interfaces + if link.Attrs().Flags&net.FlagLoopback != 0 { + klog.V(4).Infof("Network Interface %s is a loopback interface, excluding it from discovery", ifName) + continue + } + + pciAddr, err := pciAddressForNetInterface(ifName) + if err == nil { + // It's a PCI device. + + normalizedAddress := names.NormalizePCIAddress(pciAddr.String()) + var exists bool + device, exists = pciDeviceMap[normalizedAddress] + if !exists { + // We don't expect this to happen. + klog.Errorf("Network interface %s has PCI address %q, but it was not found in initial PCI scan.", ifName, pciAddr) + continue + } + addLinkAttributes(device, link) + } else { + // Not a PCI device. + + if !isVirtual(ifName, sysnetPath) { + // If we failed to identify the PCI address of the network + // interface and the network interface is also not a virtual + // device, use a best-effort strategy where the network + // interface is assumed to be virtual. + klog.Warningf("PCI address not found for non-virtual interface %s, proceeding as if it were virtual. Error: %v", ifName, err) + } + newDevice := &resourceapi.Device{ + Name: names.NormalizeInterfaceName(ifName), + Attributes: make(map[resourceapi.QualifiedName]resourceapi.DeviceAttribute), + } + otherDevices = append(otherDevices, newDevice) + device = &otherDevices[len(otherDevices)-1] + } + addLinkAttributes(device, link) + } + + return append(pciDevices, otherDevices...) +} + +func addLinkAttributes(device *resourceapi.Device, link netlink.Link) { + ifName := link.Attrs().Name + device.Attributes[apis.AttrInterfaceName] = resourceapi.DeviceAttribute{StringValue: &ifName} + device.Attributes[apis.AttrMac] = resourceapi.DeviceAttribute{StringValue: ptr.To(link.Attrs().HardwareAddr.String())} + device.Attributes[apis.AttrMTU] = resourceapi.DeviceAttribute{IntValue: ptr.To(int64(link.Attrs().MTU))} + device.Attributes[apis.AttrEncapsulation] = resourceapi.DeviceAttribute{StringValue: ptr.To(link.Attrs().EncapType)} + device.Attributes[apis.AttrAlias] = resourceapi.DeviceAttribute{StringValue: ptr.To(link.Attrs().Alias)} + device.Attributes[apis.AttrState] = resourceapi.DeviceAttribute{StringValue: ptr.To(link.Attrs().OperState.String())} + device.Attributes[apis.AttrType] = resourceapi.DeviceAttribute{StringValue: ptr.To(link.Type())} + v4 := sets.Set[string]{} v6 := sets.Set[string]{} if ips, err := netlink.AddrList(link, netlink.FAMILY_ALL); err == nil && len(ips) > 0 { @@ -235,103 +302,97 @@ func (db *DB) netdevToDRAdev(link netlink.Link) (*resourceapi.Device, error) { } } if v4.Len() > 0 { - device.Attributes["dra.net/ipv4"] = resourceapi.DeviceAttribute{StringValue: ptr.To(strings.Join(v4.UnsortedList(), ","))} + device.Attributes[apis.AttrIPv4] = resourceapi.DeviceAttribute{StringValue: ptr.To(strings.Join(v4.UnsortedList(), ","))} } if v6.Len() > 0 { - device.Attributes["dra.net/ipv6"] = resourceapi.DeviceAttribute{StringValue: ptr.To(strings.Join(v6.UnsortedList(), ","))} + device.Attributes[apis.AttrIPv6] = resourceapi.DeviceAttribute{StringValue: ptr.To(strings.Join(v6.UnsortedList(), ","))} } - mac := link.Attrs().HardwareAddr.String() - device.Attributes["dra.net/mac"] = resourceapi.DeviceAttribute{StringValue: &mac} - mtu := int64(link.Attrs().MTU) - device.Attributes["dra.net/mtu"] = resourceapi.DeviceAttribute{IntValue: &mtu} } - device.Attributes["dra.net/encapsulation"] = resourceapi.DeviceAttribute{StringValue: &linkAttrs.EncapType} - operState := linkAttrs.OperState.String() - device.Attributes["dra.net/state"] = resourceapi.DeviceAttribute{StringValue: &operState} - device.Attributes["dra.net/alias"] = resourceapi.DeviceAttribute{StringValue: &linkAttrs.Alias} - device.Attributes["dra.net/type"] = resourceapi.DeviceAttribute{StringValue: &linkType} - - // Get eBPF properties from the interface using the legacy tc hooks isEbpf := false filterNames, ok := getTcFilters(link) if ok { isEbpf = true - device.Attributes["dra.net/tcFilterNames"] = resourceapi.DeviceAttribute{StringValue: ptr.To(strings.Join(filterNames, ","))} + device.Attributes[apis.AttrTCFilterNames] = resourceapi.DeviceAttribute{StringValue: ptr.To(strings.Join(filterNames, ","))} } - // Get eBPF properties from the interface using the tcx hooks programNames, ok := getTcxFilters(link) if ok { isEbpf = true - device.Attributes["dra.net/tcxProgramNames"] = resourceapi.DeviceAttribute{StringValue: ptr.To(strings.Join(programNames, ","))} + device.Attributes[apis.AttrTCXProgramNames] = resourceapi.DeviceAttribute{StringValue: ptr.To(strings.Join(programNames, ","))} } - device.Attributes["dra.net/ebpf"] = resourceapi.DeviceAttribute{BoolValue: &isEbpf} + device.Attributes[apis.AttrEBPF] = resourceapi.DeviceAttribute{BoolValue: &isEbpf} - isRDMA := rdmamap.IsRDmaDeviceForNetdevice(ifName) - device.Attributes["dra.net/rdma"] = resourceapi.DeviceAttribute{BoolValue: &isRDMA} - // from https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin/blob/ed1c14dd4c313c7dd9fe4730a60358fbeffbfdd4/pkg/netdevice/netDeviceProvider.go#L99 isSRIOV := sriovTotalVFs(ifName) > 0 - device.Attributes["dra.net/sriov"] = resourceapi.DeviceAttribute{BoolValue: &isSRIOV} + device.Attributes[apis.AttrSRIOV] = resourceapi.DeviceAttribute{BoolValue: &isSRIOV} if isSRIOV { vfs := int64(sriovNumVFs(ifName)) - device.Attributes["dra.net/sriovVfs"] = resourceapi.DeviceAttribute{IntValue: &vfs} + device.Attributes[apis.AttrSRIOVVfs] = resourceapi.DeviceAttribute{IntValue: &vfs} } if isVirtual(ifName, sysnetPath) { - device.Attributes["dra.net/virtual"] = resourceapi.DeviceAttribute{BoolValue: ptr.To(true)} + device.Attributes[apis.AttrVirtual] = resourceapi.DeviceAttribute{BoolValue: ptr.To(true)} } else { - addPCIAttributes(&device, ifName, sysnetPath) - } - - mac := link.Attrs().HardwareAddr.String() - for name, attribute := range getProviderAttributes(mac, db.instance) { - device.Attributes[name] = attribute + device.Attributes[apis.AttrVirtual] = resourceapi.DeviceAttribute{BoolValue: ptr.To(false)} } - - return &device, nil } -func addPCIAttributes(device *resourceapi.Device, ifName string, path string) { - device.Attributes["dra.net/virtual"] = resourceapi.DeviceAttribute{BoolValue: ptr.To(false)} - - address, err := pciAddressForNetInterface(ifName) - if err != nil { - klog.Infof("Could not get bdf address : %v", err) - } else { - if err := setPciRootAttr(device, address); err != nil { - klog.Infof("Could not get pci root attribute : %v", err) +func (db *DB) discoverRDMADevices(devices []resourceapi.Device) []resourceapi.Device { + for i := range devices { + isRDMA := false + if ifName := devices[i].Attributes[apis.AttrInterfaceName].StringValue; ifName != nil && *ifName != "" { + isRDMA = rdmamap.IsRDmaDeviceForNetdevice(*ifName) + } else if pciAddr := devices[i].Attributes[apis.AttrPCIAddress].StringValue; pciAddr != nil && *pciAddr != "" { + rdmaDevices := rdmamap.GetRdmaDevicesForPcidev(*pciAddr) + isRDMA = len(rdmaDevices) != 0 } + devices[i].Attributes[apis.AttrRDMA] = resourceapi.DeviceAttribute{BoolValue: &isRDMA} } + return devices +} - entry, err := ids(ifName, path) - if err == nil { - if entry.Vendor != "" { - device.Attributes["dra.net/pciVendor"] = resourceapi.DeviceAttribute{StringValue: &entry.Vendor} - } - if entry.Device != "" { - device.Attributes["dra.net/pciDevice"] = resourceapi.DeviceAttribute{StringValue: &entry.Device} +func (db *DB) addCloudAttributes(devices []resourceapi.Device) []resourceapi.Device { + for i := range devices { + device := &devices[i] + mac, ok := device.Attributes[apis.AttrMac] + if !ok || mac.StringValue == nil { + continue } - if entry.Subsystem != "" { - device.Attributes["dra.net/pciSubsystem"] = resourceapi.DeviceAttribute{StringValue: &entry.Subsystem} - } - } else { - klog.Infof("could not get pci vendor information : %v", err) + maps.Copy(device.Attributes, getProviderAttributes(*mac.StringValue, db.instance)) } + return devices +} - numa, err := numaNode(ifName, path) - if err == nil { - device.Attributes["dra.net/numaNode"] = resourceapi.DeviceAttribute{IntValue: &numa} +func (db *DB) updateDeviceStore(devices []resourceapi.Device) { + deviceStore := map[string]resourceapi.Device{} + for _, device := range devices { + deviceStore[device.Name] = device } + db.mu.Lock() + db.deviceStore = deviceStore + db.mu.Unlock() } -func setPciRootAttr(device *resourceapi.Device, address *pciAddress) error { - // TODO(#199): Investigate ways to test correctness of PCI attributes - // discovery e2e (like standard PCI root attribute) - pcieRootAttr, err := deviceattribute.GetPCIeRootAttributeByPCIBusID(address.String()) - if err != nil { - return err +func (db *DB) GetDevice(deviceName string) (resourceapi.Device, bool) { + db.mu.Lock() + defer db.mu.Unlock() + device, exists := db.deviceStore[deviceName] + return device, exists +} + +func (db *DB) GetNetInterfaceName(deviceName string) (string, error) { + device, exists := db.GetDevice(deviceName) + if !exists { + return "", fmt.Errorf("device %s not found in store", deviceName) + } + if device.Attributes[apis.AttrInterfaceName].StringValue == nil { + return "", fmt.Errorf("device %s has no interface name", deviceName) } - device.Attributes[pcieRootAttr.Name] = pcieRootAttr.Value - return nil + return *device.Attributes[apis.AttrInterfaceName].StringValue, nil +} + +// isNetworkDevice checks the class is 0x2, defined for all types of network controllers +// https://pcisig.com/sites/default/files/files/PCI_Code-ID_r_1_11__v24_Jan_2019.pdf +func isNetworkDevice(dev *ghw.PCIDevice) bool { + return dev.Class.ID == "02" } diff --git a/pkg/inventory/sysfs.go b/pkg/inventory/sysfs.go index 02ab2867..24cb4069 100644 --- a/pkg/inventory/sysfs.go +++ b/pkg/inventory/sysfs.go @@ -25,7 +25,6 @@ import ( "strconv" "strings" - "github.com/google/dranet/pkg/pcidb" "k8s.io/klog/v2" ) @@ -101,19 +100,6 @@ func sriovNumVFs(name string) int { return t } -func numaNode(ifName string, syspath string) (int64, error) { - // /sys/class/net//device/numa_node - numeNode, err := os.ReadFile(filepath.Join(syspath, ifName, "device/numa_node")) - if err != nil { - return 0, err - } - numa, err := strconv.ParseInt(strings.TrimSpace(string(numeNode)), 10, 32) - if err != nil { - return 0, err - } - return numa, nil -} - // pciAddress BDF Notation // [domain:]bus:device.function // https://wiki.xenproject.org/wiki/Bus:Device.Function_(BDF)_Notation @@ -201,33 +187,3 @@ func pciAddressForNetInterface(ifName string) (*pciAddress, error) { } return address, nil } - -func ids(ifName string, path string) (*pcidb.Entry, error) { - // PCI data - var device, subsystemVendor, subsystemDevice []byte - vendor, err := os.ReadFile(filepath.Join(path, ifName, "device/vendor")) - if err != nil { - return nil, err - } - // device, subsystemVendor and subsystemDevice are best effort - device, err = os.ReadFile(filepath.Join(sysdevPath, ifName, "device/device")) - if err == nil { - subsystemVendor, err = os.ReadFile(filepath.Join(sysdevPath, ifName, "device/subsystem_vendor")) - if err == nil { - subsystemDevice, _ = os.ReadFile(filepath.Join(sysdevPath, ifName, "device/subsystem_device")) - } - } - - // remove the 0x prefix - entry, err := pcidb.GetDevice( - strings.TrimPrefix(strings.TrimSpace(string(vendor)), "0x"), - strings.TrimPrefix(strings.TrimSpace(string(device)), "0x"), - strings.TrimPrefix(strings.TrimSpace(string(subsystemVendor)), "0x"), - strings.TrimPrefix(strings.TrimSpace(string(subsystemDevice)), "0x"), - ) - - if err != nil { - return nil, err - } - return entry, nil -} diff --git a/pkg/names/names.go b/pkg/names/names.go index 281b7566..a74962c8 100644 --- a/pkg/names/names.go +++ b/pkg/names/names.go @@ -25,19 +25,22 @@ import ( ) const ( - // NormalizedPrefix is added to device names that had to be encoded - // because their original interface name was not DNS-1123 compliant. - NormalizedPrefix = "normalized-" + // NormalizedInterfacePrefix is prefix used when normalizing a network + // interface. + NormalizedInterfacePrefix = "net" + // NormalizedPCIPrefix is the prefix used when normalizing a PCI Address. + NormalizedPCIPrefix = "pci" ) -// SetDeviceName determines the appropriate name for a device in Kubernetes. -// If the original interface name (ifName) is already a valid DNS-1123 label, -// it's returned as is. Otherwise, it's encoded using Base32, prefixed with -// NormalizedPrefix, and returned. +// NormalizeInterfaceName determines the appropriate name for an interface in +// Kubernetes. If the original interface name (ifName) is already a valid +// DNS-1123 label, it's returned as is. Otherwise, it's encoded using Base32, +// prefixed with NormalizedPrefix, and returned. +// // Linux interface names (often limited by IFNAMSIZ, typically 16) plus the // base32 encoding and the normalized prefix (11) are within the DNS-1123 label, // which has a maximum length of 63. -func SetDeviceName(ifName string) string { +func NormalizeInterfaceName(ifName string) string { if ifName == "" { return "" } @@ -47,25 +50,16 @@ func SetDeviceName(ifName string) string { klog.V(4).Infof("Interface name '%s' is not DNS-1123 compliant, normalizing.", ifName) encodedPayload := base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString([]byte(ifName)) - normalizedName := NormalizedPrefix + strings.ToLower(encodedPayload) + normalizedName := NormalizedInterfacePrefix + strings.ToLower(encodedPayload) return normalizedName } -// GetOriginalName retrieves the original interface name from a deviceName. -// If deviceName was prefixed with NormalizedPrefix (indicating it was encoded), -// it decodes the name. Otherwise, it assumes deviceName is the original name. -func GetOriginalName(deviceName string) string { - if strings.HasPrefix(deviceName, NormalizedPrefix) { - encodedPart := strings.TrimPrefix(deviceName, NormalizedPrefix) - encodedPart = strings.ToUpper(encodedPart) // base32 uses uppercase only - decodedBytes, err := base32.StdEncoding.WithPadding(base32.NoPadding).DecodeString(encodedPart) - if err != nil { - klog.Warningf("Failed to decode Base32 device name payload '%s' from full name '%s': %v. Returning the full deviceName as fallback.", - encodedPart, deviceName, err) - return deviceName - } - return string(decodedBytes) - } - return deviceName +// NormalizePCIAddress takes a PCI address and converts it into a DNS-1123 +// acceptable format. +func NormalizePCIAddress(pciAddress string) string { + // Replace ":" and "." with "-" to make it DNS-1123 compliant. + // A PCI address like "0000:8a:00.0" becomes "0000-8a-00-0". + r := strings.NewReplacer(":", "-", ".", "-") + return NormalizedPCIPrefix + "-" + r.Replace(pciAddress) } diff --git a/pkg/names/names_test.go b/pkg/names/names_test.go index 2efbfe2a..7c238451 100644 --- a/pkg/names/names_test.go +++ b/pkg/names/names_test.go @@ -21,7 +21,7 @@ import ( "testing" ) -func TestSetDeviceName(t *testing.T) { +func TestNormalizeInterfaceName(t *testing.T) { tests := []struct { name string ifName string @@ -29,47 +29,45 @@ func TestSetDeviceName(t *testing.T) { }{ {"already compliant", "eth0", "eth0"}, {"already compliant with hyphen", "my-device-1", "my-device-1"}, - {"needs normalization colons", "eth:0", NormalizedPrefix + "mv2gqorq"}, - {"needs normalization uppercase", "ETH0", NormalizedPrefix + "ivkeqma"}, - {"needs normalization underscore", "eth_int", NormalizedPrefix + "mv2gqx3jnz2a"}, + {"needs normalization colons", "eth:0", NormalizedInterfacePrefix + "mv2gqorq"}, + {"needs normalization uppercase", "ETH0", NormalizedInterfacePrefix + "ivkeqma"}, + {"needs normalization underscore", "eth_int", NormalizedInterfacePrefix + "mv2gqx3jnz2a"}, {"empty string", "", ""}, { name: "long name needs normalization", ifName: "very_long_interface_name_that_is_not_dns_compliant_at_all_and_exceeds_limits", // base32 of the above is much longer, this is just to check prefixing - want: NormalizedPrefix + "ozsxe6k7nrxw4z27nfxhizlsmzqwgzk7nzqw2zk7orugc5c7nfzv63tporpwi3ttl5rw63lqnruwc3tul5qxix3bnrwf6ylomrpwk6ddmvswi427nruw22luom", + want: NormalizedInterfacePrefix + "ozsxe6k7nrxw4z27nfxhizlsmzqwgzk7nzqw2zk7orugc5c7nfzv63tporpwi3ttl5rw63lqnruwc3tul5qxix3bnrwf6ylomrpwk6ddmvswi427nruw22luom", }, {"already compliant max length", strings.Repeat("a", 63), strings.Repeat("a", 63)}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := SetDeviceName(tt.ifName); got != tt.want { - t.Errorf("SetDeviceName(%q) = %q, want %q", tt.ifName, got, tt.want) + if got := NormalizeInterfaceName(tt.ifName); got != tt.want { + t.Errorf("NormalizeInterfaceName(%q) = %q, want %q", tt.ifName, got, tt.want) } }) } } -func TestSetAndGetOriginalName(t *testing.T) { - testIfNames := []string{ - "eth0", - "my-nic", - "eth:0:1", - "veth_pair_A", - "UPPERCASE_NIC", - "a.b.c.d.e.f", - strings.Repeat("a_b", 30), // long non-compliant name - "", +func TestNormalizePCIAddress(t *testing.T) { + testCases := []struct { + name string + pciAddress string + want string + }{ + { + name: "Standard PCI Address", + pciAddress: "0000:8a:00.0", + want: NormalizedPCIPrefix + "-0000-8a-00-0", + }, } - for _, ifName := range testIfNames { - t.Run(ifName, func(t *testing.T) { - deviceName := SetDeviceName(ifName) - originalName := GetOriginalName(deviceName) - if originalName != ifName { - t.Errorf("SetDeviceName -> GetOriginalName round trip failed for %q: SetDeviceName returned %q, GetOriginalName returned %q", - ifName, deviceName, originalName) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if got := NormalizePCIAddress(tc.pciAddress); got != tc.want { + t.Errorf("NormalizePCIAddress(%v) = %v, want %v", tc.pciAddress, got, tc.want) } }) } From 651afcc86d937ed566cbd4e8b6c6ef4eceabff74 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Wed, 20 Aug 2025 00:06:06 -0700 Subject: [PATCH 02/13] fix: Add a check to the CEL expression in default filter --- cmd/dranet/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/dranet/app.go b/cmd/dranet/app.go index 36e8ba17..69d37793 100644 --- a/cmd/dranet/app.go +++ b/cmd/dranet/app.go @@ -58,7 +58,7 @@ func init() { flag.StringVar(&kubeconfig, "kubeconfig", "", "absolute path to the kubeconfig file") flag.StringVar(&bindAddress, "bind-address", ":9177", "The IP address and port for the metrics and healthz server to serve on") flag.StringVar(&hostnameOverride, "hostname-override", "", "If non-empty, will be used as the name of the Node that kube-network-policies is running on. If unset, the node name is assumed to be the same as the node's hostname.") - flag.StringVar(&celExpression, "filter", `attributes["dra.net/type"].StringValue != "veth"`, "CEL expression to filter network interface attributes (v1.DeviceAttribute).") + flag.StringVar(&celExpression, "filter", `!("dra.net/type" in attributes) || attributes["dra.net/type"].StringValue != "veth"`, "CEL expression to filter network interface attributes (v1.DeviceAttribute).") flag.Usage = func() { fmt.Fprint(os.Stderr, "Usage: dranet [options]\n\n") From d441585ed8d24806c23d811f93be6ca6df4665d5 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Wed, 20 Aug 2025 00:06:22 -0700 Subject: [PATCH 03/13] refactor: Introduce constants for device attributes --- pkg/apis/attributes.go | 44 ++++++++++++++++++++++++++++++++++++ pkg/cloudprovider/gce/gce.go | 10 ++++++++ 2 files changed, 54 insertions(+) create mode 100644 pkg/apis/attributes.go diff --git a/pkg/apis/attributes.go b/pkg/apis/attributes.go new file mode 100644 index 00000000..6615ad00 --- /dev/null +++ b/pkg/apis/attributes.go @@ -0,0 +1,44 @@ +/* +Copyright 2025 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apis + +const ( + AttrPrefix = "dra.net" + + // TODO: Document meaning of these attributes and re-evaluate if all are needed. + AttrInterfaceName = AttrPrefix + "/" + "ifName" + AttrPCIAddress = AttrPrefix + "/" + "pciAddress" + AttrMac = AttrPrefix + "/" + "mac" + AttrPCIVendor = AttrPrefix + "/" + "pciVendor" + AttrPCIDevice = AttrPrefix + "/" + "pciDevice" + AttrPCISubsystem = AttrPrefix + "/" + "pciSubsystem" + AttrNUMANode = AttrPrefix + "/" + "numaNode" + AttrMTU = AttrPrefix + "/" + "mtu" + AttrEncapsulation = AttrPrefix + "/" + "encapsulation" + AttrAlias = AttrPrefix + "/" + "alias" + AttrState = AttrPrefix + "/" + "state" + AttrType = AttrPrefix + "/" + "type" + AttrIPv4 = AttrPrefix + "/" + "ipv4" + AttrIPv6 = AttrPrefix + "/" + "ipv6" + AttrTCFilterNames = AttrPrefix + "/" + "tcFilterNames" + AttrTCXProgramNames = AttrPrefix + "/" + "tcxProgramNames" + AttrEBPF = AttrPrefix + "/" + "ebpf" + AttrSRIOV = AttrPrefix + "/" + "sriov" + AttrSRIOVVfs = AttrPrefix + "/" + "sriovVfs" + AttrVirtual = AttrPrefix + "/" + "virtual" + AttrRDMA = AttrPrefix + "/" + "rdma" +) diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go index 6e5d62b8..4a52f284 100644 --- a/pkg/cloudprovider/gce/gce.go +++ b/pkg/cloudprovider/gce/gce.go @@ -41,6 +41,16 @@ const ( GPUDirectRDMA GPUDirectSupport = "GPUDirect-RDMA" ) +const ( + GCEAttrPrefix = "gce.dra.net" + + AttrGCEBlock = GCEAttrPrefix + "/" + "block" + AttrGCESubblock = GCEAttrPrefix + "/" + "subblock" + AttrGCEHost = GCEAttrPrefix + "/" + "host" + AttrGCENetworkName = GCEAttrPrefix + "/" + "networkName" + AttrGCENetworkProjectNumber = GCEAttrPrefix + "/" + "networkProjectNumber" +) + var ( // https://cloud.google.com/compute/docs/accelerator-optimized-machines#network-protocol // machine types have a one to one mapping to a network protocol in google cloud From f1ba313a9cc5588083eb9ccedc616eab9cd770f3 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Wed, 20 Aug 2025 15:49:56 -0700 Subject: [PATCH 04/13] fix: Add OriginalInterfaceName to PodCfg to allow restoration of the interface name --- pkg/driver/dra_hooks.go | 17 ++++------------- pkg/driver/nri_hooks.go | 15 ++------------- pkg/driver/pod_device_config.go | 8 ++++++++ pkg/inventory/db.go | 2 +- 4 files changed, 15 insertions(+), 27 deletions(-) diff --git a/pkg/driver/dra_hooks.go b/pkg/driver/dra_hooks.go index 3488b28f..589a8f14 100644 --- a/pkg/driver/dra_hooks.go +++ b/pkg/driver/dra_hooks.go @@ -164,21 +164,9 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour }, Network: netconf, } - // TODO(gauravkghildiyal): There's a (self-resolving) race condition - // here which is triggered when a pod claiming a device is rapidly - // deleted and recreated. When the pod is deleted, - // UnprepareResourceClaim deletes the claim and does not wait for the - // network device to be freed up. Deletion of the claim indicates to the - // kube-scheduler that the device is available for future use so it can - // assign the same device to the newly created pod. But it's possible - // that the old pod's sandbox still exists (and is in the process of - // being deleted) so the network interface is still in the old pod's - // network namespace. Although the problem self resolves when kubelet - // retries PrepareResourceClaims for the new pod, but it should be - // possible to instrument a better lifecycle. ifName, err := np.netdb.GetNetInterfaceName(result.Device) if err != nil { - errorList = append(errorList, fmt.Errorf("failed to get network interface name for device %s: %v", ifName, err)) + errorList = append(errorList, fmt.Errorf("failed to get network interface name for device %s: %v", result.Device, err)) } // Get Network configuration and merge it link, err := nlHandle.LinkByName(ifName) @@ -186,8 +174,11 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour errorList = append(errorList, fmt.Errorf("failed to get netlink to interface %s: %v", ifName, err)) continue } + podCfg.OriginalInterfaceName = ifName if podCfg.Network.Interface.Name == "" { + // If the interface name was not explicitly overridden, use the same + // interface name within the pod's network namespace. podCfg.Network.Interface.Name = ifName } diff --git a/pkg/driver/nri_hooks.go b/pkg/driver/nri_hooks.go index a87a7e8b..40d7c215 100644 --- a/pkg/driver/nri_hooks.go +++ b/pkg/driver/nri_hooks.go @@ -259,19 +259,8 @@ func (np *NetworkDriver) StopPodSandbox(ctx context.Context, pod *api.PodSandbox } for deviceName, config := range podConfig { - // TODO(gauravkghildiyal): Interface name here will in most cases be - // empty which means we're mostly relying on the fallback of the kernel - // having to do this cleanup. That is hopefully not the worst thing - // which could happen, but there are easy ways to improve this - // behaviour, like detaching the specific network interface which has - // the the given PCI address (i.e. deviceName is the normalized PCI address). - ifName, err := np.netdb.GetNetInterfaceName(deviceName) - if err == nil { - if err := nsDetachNetdev(ns, config.Network.Interface.Name, ifName); err != nil { - klog.Infof("fail to return network device %s : %v", deviceName, err) - } - } else { - klog.V(2).Infof("Failed to identify network interface for device %s: %v; expecting kernel to do the cleanup", deviceName, err) + if err := nsDetachNetdev(ns, config.Network.Interface.Name, config.OriginalInterfaceName); err != nil { + klog.Infof("fail to return network device %s : %v", deviceName, err) } if !np.rdmaSharedMode && config.RDMADevice.LinkDev != "" { diff --git a/pkg/driver/pod_device_config.go b/pkg/driver/pod_device_config.go index 890c88cc..9a79dbaf 100644 --- a/pkg/driver/pod_device_config.go +++ b/pkg/driver/pod_device_config.go @@ -29,6 +29,14 @@ import ( type PodConfig struct { Claim types.NamespacedName + // OriginalInterfaceName is the name of the network interface as seen in the + // host's network namespace. + // + // TODO(improvement): Instead of OriginalInterfaceName being a stirng, we + // can instead have an OriginalInterfaceConfig of type apis.NetworkConfig + // which can retain information beyond the interface's name. + OriginalInterfaceName string + // Network contains all network-related configurations (interface, routes, // ethtool, sysctl) to be applied for this device in the Pod's namespace. Network apis.NetworkConfig diff --git a/pkg/inventory/db.go b/pkg/inventory/db.go index 357c521e..0628a69f 100644 --- a/pkg/inventory/db.go +++ b/pkg/inventory/db.go @@ -386,7 +386,7 @@ func (db *DB) GetNetInterfaceName(deviceName string) (string, error) { return "", fmt.Errorf("device %s not found in store", deviceName) } if device.Attributes[apis.AttrInterfaceName].StringValue == nil { - return "", fmt.Errorf("device %s has no interface name", deviceName) + return "", fmt.Errorf("device %s has no interface name in local store", deviceName) } return *device.Attributes[apis.AttrInterfaceName].StringValue, nil } From bacf7cfcd4090e1ba184acd85d33a96b91c4e6f1 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Wed, 20 Aug 2025 16:16:00 -0700 Subject: [PATCH 05/13] chore: Address review comments from michaelasp --- pkg/inventory/db.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/inventory/db.go b/pkg/inventory/db.go index 0628a69f..230474df 100644 --- a/pkg/inventory/db.go +++ b/pkg/inventory/db.go @@ -57,9 +57,9 @@ type DB struct { instance *cloudprovider.CloudInstance mu sync.RWMutex - // netNsForPod gives the network namespace for a pod, indexed by the pods + // podNetNsStore gives the network namespace for a pod, indexed by the pods // "namespaced/name". - netNsForPod map[string]string + podNetNsStore map[string]string // deviceStore is an in-memory cache of the available devices on the node. // It is keyed by the normalized PCI address of the device. The value is a // resourceapi.Device object that contains the device's attributes. @@ -73,7 +73,7 @@ type DB struct { func New() *DB { return &DB{ - netNsForPod: map[string]string{}, + podNetNsStore: map[string]string{}, deviceStore: map[string]resourceapi.Device{}, rateLimiter: rate.NewLimiter(rate.Every(minInterval), 1), notifications: make(chan []resourceapi.Device), @@ -85,24 +85,24 @@ func (db *DB) AddPodNetNs(pod string, netNsPath string) { defer db.mu.Unlock() ns, err := netns.GetFromPath(netNsPath) if err != nil { - klog.Infof("Failed to get pod %s network namespace %s handle: %v", pod, netNsPath, err) + klog.Errorf("Failed to get pod %s network namespace %s handle: %v", pod, netNsPath, err) return } defer ns.Close() - db.netNsForPod[pod] = netNsPath + db.podNetNsStore[pod] = netNsPath } func (db *DB) RemovePodNetNs(pod string) { db.mu.Lock() defer db.mu.Unlock() - delete(db.netNsForPod, pod) + delete(db.podNetNsStore, pod) } // GetPodNamespace allows to get the Pod network namespace func (db *DB) GetPodNetNs(pod string) string { db.mu.RLock() defer db.mu.RUnlock() - return db.netNsForPod[pod] + return db.podNetNsStore[pod] } func (db *DB) Run(ctx context.Context) error { @@ -214,6 +214,14 @@ func (db *DB) discoverPCIDevices() []resourceapi.Device { return devices } +// discoveryNetworkInterfaces updates the devices based on information retried +// from network interfaces. For each network interface, the two possible +// outcomes are: +// - If the network interface is associated with some parent PCI device, the +// existing PCI device is modified with additional attributes related to the +// network interface. +// - For Network interfaces which are not associated with a PCI Device (like +// virtual interfaces), they are added as their own device. func (db *DB) discoverNetworkInterfaces(pciDevices []resourceapi.Device) []resourceapi.Device { links, err := netlink.LinkList() if err != nil { From 0b6e7c909f4863c83e54e724c0d691262967bea4 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 21 Aug 2025 12:09:39 -0700 Subject: [PATCH 06/13] fix: Do not publish network interface which has unknown PCI device --- pkg/inventory/db.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/inventory/db.go b/pkg/inventory/db.go index 230474df..776c79fc 100644 --- a/pkg/inventory/db.go +++ b/pkg/inventory/db.go @@ -255,7 +255,7 @@ func (db *DB) discoverNetworkInterfaces(pciDevices []resourceapi.Device) []resou normalizedAddress := names.NormalizePCIAddress(pciAddr.String()) var exists bool - device, exists = pciDeviceMap[normalizedAddress] + device, exists := pciDeviceMap[normalizedAddress] if !exists { // We don't expect this to happen. klog.Errorf("Network interface %s has PCI address %q, but it was not found in initial PCI scan.", ifName, pciAddr) @@ -276,10 +276,9 @@ func (db *DB) discoverNetworkInterfaces(pciDevices []resourceapi.Device) []resou Name: names.NormalizeInterfaceName(ifName), Attributes: make(map[resourceapi.QualifiedName]resourceapi.DeviceAttribute), } - otherDevices = append(otherDevices, newDevice) - device = &otherDevices[len(otherDevices)-1] + addLinkAttributes(newDevice, link) + otherDevices = append(otherDevices, *newDevice) } - addLinkAttributes(device, link) } return append(pciDevices, otherDevices...) From c5c655070ef2984d0a91ffa0544b1547b63f2586 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 21 Aug 2025 12:24:00 -0700 Subject: [PATCH 07/13] docs: Add comment about usage of dra.net as the domain --- pkg/apis/attributes.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/apis/attributes.go b/pkg/apis/attributes.go index 6615ad00..4f2ffeec 100644 --- a/pkg/apis/attributes.go +++ b/pkg/apis/attributes.go @@ -17,6 +17,10 @@ limitations under the License. package apis const ( + // TODO: Reconsider the domain being used when project becomes owned by some + // SIG. The issue with "dra.net" is that http://dra.net is an actual + // domain that is totally unrelated to this project and it can be a source + // of confusion and problems. AttrPrefix = "dra.net" // TODO: Document meaning of these attributes and re-evaluate if all are needed. From 4793314da7273dafb0620931e521ba41cd84e232 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 21 Aug 2025 15:52:02 -0700 Subject: [PATCH 08/13] feat: Extract embedded PCI DB file when needed --- cmd/dranet/app.go | 5 ++ pkg/inventory/db.go | 4 +- pkg/pcidb/pcidb.go | 143 ++++++---------------------------------- pkg/pcidb/pcidb_test.go | 103 ----------------------------- 4 files changed, 29 insertions(+), 226 deletions(-) delete mode 100644 pkg/pcidb/pcidb_test.go diff --git a/cmd/dranet/app.go b/cmd/dranet/app.go index 69d37793..d6dc5555 100644 --- a/cmd/dranet/app.go +++ b/cmd/dranet/app.go @@ -31,6 +31,7 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/ext" "github.com/google/dranet/pkg/driver" + "github.com/google/dranet/pkg/pcidb" "github.com/prometheus/client_golang/prometheus/promhttp" resourcev1 "k8s.io/api/resource/v1" @@ -90,6 +91,10 @@ func main() { _ = http.ListenAndServe(bindAddress, mux) }() + if err := pcidb.Setup(); err != nil { + klog.Fatalf("Failed to setup PCI DB: %v", err) + } + var config *rest.Config var err error if kubeconfig != "" { diff --git a/pkg/inventory/db.go b/pkg/inventory/db.go index 776c79fc..21e9dab2 100644 --- a/pkg/inventory/db.go +++ b/pkg/inventory/db.go @@ -173,7 +173,9 @@ func (db *DB) GetResources(ctx context.Context) <-chan []resourceapi.Device { func (db *DB) discoverPCIDevices() []resourceapi.Device { devices := []resourceapi.Device{} - pci, err := ghw.PCI() + pci, err := ghw.PCI( + ghw.WithDisableTools(), + ) if err != nil { klog.Errorf("Could not get PCI devices: %v", err) return devices diff --git a/pkg/pcidb/pcidb.go b/pkg/pcidb/pcidb.go index e80af464..f887019c 100644 --- a/pkg/pcidb/pcidb.go +++ b/pkg/pcidb/pcidb.go @@ -17,142 +17,41 @@ limitations under the License. package pcidb import ( - "bufio" - "bytes" - "compress/gzip" - _ "embed" "fmt" - "regexp" - "strings" + "os" + "path/filepath" "github.com/google/dranet/third_party" + "k8s.io/klog/v2" ) var ( pcidb = third_party.PCIDBGZ - // Vendor entries start with a 4-digit hexadecimal vendor ID, - // followed by one or more spaces, and the name of the vendor - // extending to the end of the line. - reVendor = regexp.MustCompile(`(^[a-f0-9]{4})\s+(.*)$`) - // Each device entry consists of a single TAB character, a 4-digit hexadecimal - // device ID, followed by one or more spaces, and the name of the - // device extending to the end of the line. - reDevice = regexp.MustCompile(`^\t([a-f0-9]{4})\s+(.*)$`) - // Subsystem entries are placed below the device entry. They start - // with two TAB characters, a 4-digit hexadecimal vendor ID (which - // must be defined elsewhere in the list), a single space, a 4-digit - // hexadecimal subsystem ID, one or more spaces, and the name of the - // subsystem extending to the end of the line. - reSubsystem = regexp.MustCompile(`^\t{2}([a-f0-9]{4})\s([a-f0-9]{4})\s+(.*)$`) ) -// Entry for the OCI ID database -// https://man7.org/linux/man-pages/man5/pci.ids.5.html -type Entry struct { - Vendor string - Device string - Subsystem string -} - -// getPCI iterates over the file until it finds the associated entry -// and returns the names it finds. -// Expect values in hexadecimal format without the 0x prefix -// Vendor: 025e --> Solidigm -// Device: 0b60 --> NVMe DC SSD [Sentinel Rock Plus controller] -// SubVendor: 025e , SubDevice: 8008 --> NVMe DC SSD U.2 15mm [D7-P5510] -func GetDevice(vendor, device, subvendor, subdevice string) (*Entry, error) { - // we require at least a vendor - if len(vendor) != 4 { - return nil, fmt.Errorf("vendor ID must be 4-digit hexadecimal") - } - - if len(device) > 0 && len(device) != 4 { - return nil, fmt.Errorf("device ID must be 4-digit hexadecimal") - } - - if len(device) == 0 && - (len(subvendor) > 0 || len(subdevice) > 0) { - return nil, fmt.Errorf("device ID must be set if subvendor or subdevice are specified") - } - - if len(subdevice) != len(subvendor) { - return nil, fmt.Errorf("both subvendor and subdevice must be specified if one is specified") - } - - if len(subvendor) > 0 && len(subvendor) != 4 { - return nil, fmt.Errorf("subvendor ID must be 4-digit hexadecimal") +func Setup() error { + if value, exists := os.LookupEnv("PCIDB_PATH"); exists { + // If an explicit path has been configured for PCI DB, use that and + // don't extract the embedded db. + klog.Infof("Using pre-configured value for PCIDB_PATH=%q", value) + return nil } - if len(subdevice) > 0 && len(subdevice) != 4 { - return nil, fmt.Errorf("subdevice ID must be 4-digit hexadecimal") - } - - gzReader, err := gzip.NewReader(bytes.NewReader(pcidb)) + // PCIDB_PATH was not set, which means we should attempt to use the embedded + // file as the db source. + tempDir, err := os.MkdirTemp("", "pcidb") if err != nil { - return nil, err + return fmt.Errorf("failed to create temporary directory for PCI DB: %v", err) } - defer gzReader.Close() - - entry := &Entry{} - scanner := bufio.NewScanner(gzReader) - // # Syntax: - // # vendor vendor_name - // # device device_name <-- single tab - // # subvendor subdevice subsystem_name <-- two tabs - for scanner.Scan() { - line := scanner.Text() - // find first the vendor - if entry.Vendor == "" { - matches := reVendor.FindStringSubmatch(line) - if len(matches) != 3 { - continue - } - if matches[1] != strings.ToLower(vendor) { - continue - } - entry.Vendor = matches[2] - continue - } - // finish if we need only the vendor - if len(device) == 0 { - return entry, nil - } - // find the device - if entry.Device == "" { - matches := reDevice.FindStringSubmatch(line) - if len(matches) != 3 { - continue - } - if matches[1] != strings.ToLower(device) { - continue - } - entry.Device = matches[2] - continue - } - // finish if we need only the vendor and the device - if len(subdevice) == 0 && len(subvendor) == 0 { - return entry, nil - } - // finally find the subsystem - if entry.Subsystem == "" { - matches := reSubsystem.FindStringSubmatch(line) - if len(matches) != 4 { - continue - } - if matches[1] != strings.ToLower(subvendor) { - continue - } - if matches[2] != strings.ToLower(subdevice) { - continue - } - entry.Subsystem = matches[3] - // nothing else - return entry, nil - } + filePath := filepath.Join(tempDir, "pci.ids.gz") + err = os.WriteFile(filePath, pcidb, 0644) + if err != nil { + return fmt.Errorf("failed to write pci.ids.gz file: %v", err) } - if err := scanner.Err(); err != nil { - return entry, err + if err := os.Setenv("PCIDB_PATH", filePath); err != nil { + return fmt.Errorf("failed to set PCIDB_PATH environment variable: %v", err) } - return entry, fmt.Errorf("entry not found") + klog.Infof("Successfuly set value of PCIDB_PATH=%q", filePath) + return nil } diff --git a/pkg/pcidb/pcidb_test.go b/pkg/pcidb/pcidb_test.go deleted file mode 100644 index ae05dc8a..00000000 --- a/pkg/pcidb/pcidb_test.go +++ /dev/null @@ -1,103 +0,0 @@ -/* -Copyright 2024 Google LLC - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - https://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package pcidb - -import ( - _ "embed" - "reflect" - "testing" -) - -func Test_getPCI(t *testing.T) { - - tests := []struct { - name string - vendor string - device string - subvendor string - subdevice string - want *Entry - wantErr bool - }{ - { - name: "empty", - wantErr: true, - }, - { - name: "wrong vendor - 5 digits", - vendor: "12345", - wantErr: true, - }, - { - name: "device and no vendor", - device: "1234", - wantErr: true, - }, - { - name: "vendor 001c", - vendor: "001c", - want: &Entry{ - Vendor: "PEAK-System Technik GmbH", - }, - }, - { - name: "vendor 001c device 0001", - vendor: "001c", - device: "0001", - want: &Entry{ - Vendor: "PEAK-System Technik GmbH", - Device: "PCAN-PCI CAN-Bus controller", - }, - }, - { - name: "vendor 001c device 0001 subsystem 001c 0005", - vendor: "001c", - device: "0001", - subvendor: "001c", - subdevice: "0005", - want: &Entry{ - Vendor: "PEAK-System Technik GmbH", - Device: "PCAN-PCI CAN-Bus controller", - Subsystem: "2 Channel CAN Bus SJC1000 (Optically Isolated)", - }, - }, - { - name: "subsystem does not exist", - vendor: "001c", - device: "0001", - subvendor: "001c", - subdevice: "fd05", - want: &Entry{ - Vendor: "PEAK-System Technik GmbH", - Device: "PCAN-PCI CAN-Bus controller", - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := GetDevice(tt.vendor, tt.device, tt.subvendor, tt.subdevice) - if (err != nil) != tt.wantErr { - t.Errorf("getPCI() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("getPCI() = %v, want %v", got, tt.want) - } - }) - } -} From 70cf828a3e93317105ffa07e653967160777dbe3 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 21 Aug 2025 16:16:23 -0700 Subject: [PATCH 09/13] refactor: Rename PodConfig subfields to have NetworkInterfaceConfigInHost and NetworkInterfaceConfigInPod --- pkg/driver/dra_hooks.go | 30 ++++++++++++++-------------- pkg/driver/nri_hooks.go | 22 ++++++++------------ pkg/driver/pod_device_config.go | 20 +++++++++---------- pkg/driver/pod_device_config_test.go | 30 ++++++++++++++-------------- 4 files changed, 47 insertions(+), 55 deletions(-) diff --git a/pkg/driver/dra_hooks.go b/pkg/driver/dra_hooks.go index 589a8f14..7a950d32 100644 --- a/pkg/driver/dra_hooks.go +++ b/pkg/driver/dra_hooks.go @@ -162,7 +162,7 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour Namespace: claim.Namespace, Name: claim.Name, }, - Network: netconf, + NetworkInterfaceConfigInPod: netconf, } ifName, err := np.netdb.GetNetInterfaceName(result.Device) if err != nil { @@ -174,17 +174,17 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour errorList = append(errorList, fmt.Errorf("failed to get netlink to interface %s: %v", ifName, err)) continue } - podCfg.OriginalInterfaceName = ifName + podCfg.NetworkInterfaceConfigInHost.Interface.Name = ifName - if podCfg.Network.Interface.Name == "" { + if podCfg.NetworkInterfaceConfigInPod.Interface.Name == "" { // If the interface name was not explicitly overridden, use the same // interface name within the pod's network namespace. - podCfg.Network.Interface.Name = ifName + podCfg.NetworkInterfaceConfigInPod.Interface.Name = ifName } // If DHCP is requested, do a DHCP request to gather the network parameters (IPs and Routes) // ... but we DO NOT apply them in the root namespace - if podCfg.Network.Interface.DHCP != nil && *podCfg.Network.Interface.DHCP { + if podCfg.NetworkInterfaceConfigInPod.Interface.DHCP != nil && *podCfg.NetworkInterfaceConfigInPod.Interface.DHCP { klog.V(2).Infof("trying to get network configuration via DHCP") contextCancel, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() @@ -192,10 +192,10 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour if err != nil { errorList = append(errorList, fmt.Errorf("fail to get configuration via DHCP for %s: %w", ifName, err)) } else { - podCfg.Network.Interface.Addresses = []string{ip} - podCfg.Network.Routes = append(podCfg.Network.Routes, routes...) + podCfg.NetworkInterfaceConfigInPod.Interface.Addresses = []string{ip} + podCfg.NetworkInterfaceConfigInPod.Routes = append(podCfg.NetworkInterfaceConfigInPod.Routes, routes...) } - } else if len(podCfg.Network.Interface.Addresses) == 0 { + } else if len(podCfg.NetworkInterfaceConfigInPod.Interface.Addresses) == 0 { // If there is no custom addresses and no DHCP, then use the existing ones // get the existing IP addresses nlAddresses, err := nlHandle.AddrList(link, netlink.FAMILY_ALL) @@ -209,13 +209,13 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour if address.Scope != unix.RT_SCOPE_UNIVERSE { continue } - podCfg.Network.Interface.Addresses = append(podCfg.Network.Interface.Addresses, address.IPNet.String()) + podCfg.NetworkInterfaceConfigInPod.Interface.Addresses = append(podCfg.NetworkInterfaceConfigInPod.Interface.Addresses, address.IPNet.String()) } } } // Obtain the existing supported ethtool features and validate the config - if podCfg.Network.Ethtool != nil { + if podCfg.NetworkInterfaceConfigInPod.Ethtool != nil { client, err := newEthtoolClient(0) if err != nil { errorList = append(errorList, fmt.Errorf("fail to create ethtool client %v", err)) @@ -231,7 +231,7 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour // translate features to the actual kernel names ethtoolFeatures := map[string]bool{} - for feature, value := range podCfg.Network.Ethtool.Features { + for feature, value := range podCfg.NetworkInterfaceConfigInPod.Ethtool.Features { aliases := ifFeatures.Get(feature) if len(aliases) == 0 { errorList = append(errorList, fmt.Errorf("feature %s not supported by interface", feature)) @@ -241,7 +241,7 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour ethtoolFeatures[alias] = value } } - podCfg.Network.Ethtool.Features = ethtoolFeatures + podCfg.NetworkInterfaceConfigInPod.Ethtool.Features = ethtoolFeatures } // Obtain the routes associated to the interface @@ -272,7 +272,7 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour routeCfg.Source = route.Src.String() } routeCfg.Scope = uint8(route.Scope) - podCfg.Network.Routes = append(podCfg.Network.Routes, routeCfg) + podCfg.NetworkInterfaceConfigInPod.Routes = append(podCfg.NetworkInterfaceConfigInPod.Routes, routeCfg) } // Get RDMA configuration: link and char devices @@ -295,8 +295,8 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour // Remove the pinned programs before the NRI hooks since it // has to walk the entire bpf virtual filesystem and is slow // TODO: check if there is some other way to do this - if podCfg.Network.Interface.DisableEBPFPrograms != nil && - *podCfg.Network.Interface.DisableEBPFPrograms { + if podCfg.NetworkInterfaceConfigInPod.Interface.DisableEBPFPrograms != nil && + *podCfg.NetworkInterfaceConfigInPod.Interface.DisableEBPFPrograms { err := unpinBPFPrograms(ifName) if err != nil { klog.Infof("error unpinning ebpf programs for %s : %v", ifName, err) diff --git a/pkg/driver/nri_hooks.go b/pkg/driver/nri_hooks.go index 40d7c215..3c095ef1 100644 --- a/pkg/driver/nri_hooks.go +++ b/pkg/driver/nri_hooks.go @@ -124,18 +124,12 @@ func (np *NetworkDriver) RunPodSandbox(ctx context.Context, pod *api.PodSandbox) WithDriver(np.driverName). WithPool(np.nodeName) - ifName, err := np.netdb.GetNetInterfaceName(deviceName) - if err != nil { - // Tip: We desired, tweak the code here to simply log - // this without returning and continue exeuction for cases where the - // device has no associated network interface. - return fmt.Errorf("failed to get network interface %s", ifName) - } + ifName := config.NetworkInterfaceConfigInHost.Interface.Name klog.V(2).Infof("RunPodSandbox processing Network device: %s", ifName) // TODO config options to rename the device and pass parameters // use https://github.com/opencontainers/runtime-spec/pull/1271 - networkData, err := nsAttachNetdev(ifName, ns, config.Network.Interface) + networkData, err := nsAttachNetdev(ifName, ns, config.NetworkInterfaceConfigInPod.Interface) if err != nil { klog.Infof("RunPodSandbox error moving device %s to namespace %s: %v", deviceName, ns, err) return fmt.Errorf("error moving network device %s to namespace %s: %v", deviceName, ns, err) @@ -157,8 +151,8 @@ func (np *NetworkDriver) RunPodSandbox(ctx context.Context, pod *api.PodSandbox) ifNameInNs := networkData.InterfaceName // Apply Ethtool configurations - if config.Network.Ethtool != nil { - err = applyEthtoolConfig(ns, ifNameInNs, config.Network.Ethtool) + if config.NetworkInterfaceConfigInPod.Ethtool != nil { + err = applyEthtoolConfig(ns, ifNameInNs, config.NetworkInterfaceConfigInPod.Ethtool) if err != nil { klog.Infof("RunPodSandbox error applying ethtool config for %s in ns %s: %v", ifNameInNs, ns, err) return fmt.Errorf("error applying ethtool config for %s in ns %s: %v", ifNameInNs, ns, err) @@ -166,8 +160,8 @@ func (np *NetworkDriver) RunPodSandbox(ctx context.Context, pod *api.PodSandbox) } // Check if the ebpf programs should be disabled - if config.Network.Interface.DisableEBPFPrograms != nil && - *config.Network.Interface.DisableEBPFPrograms { + if config.NetworkInterfaceConfigInPod.Interface.DisableEBPFPrograms != nil && + *config.NetworkInterfaceConfigInPod.Interface.DisableEBPFPrograms { err := detachEBPFPrograms(ns, ifNameInNs) if err != nil { klog.Infof("error disabling ebpf programs for %s in ns %s: %v", ifNameInNs, ns, err) @@ -176,7 +170,7 @@ func (np *NetworkDriver) RunPodSandbox(ctx context.Context, pod *api.PodSandbox) } // Configure routes - err = applyRoutingConfig(ns, ifNameInNs, config.Network.Routes) + err = applyRoutingConfig(ns, ifNameInNs, config.NetworkInterfaceConfigInPod.Routes) if err != nil { klog.Infof("RunPodSandbox error configuring device %s namespace %s routing: %v", deviceName, ns, err) return fmt.Errorf("error configuring device %s routes on namespace %s: %v", deviceName, ns, err) @@ -259,7 +253,7 @@ func (np *NetworkDriver) StopPodSandbox(ctx context.Context, pod *api.PodSandbox } for deviceName, config := range podConfig { - if err := nsDetachNetdev(ns, config.Network.Interface.Name, config.OriginalInterfaceName); err != nil { + if err := nsDetachNetdev(ns, config.NetworkInterfaceConfigInPod.Interface.Name, config.NetworkInterfaceConfigInHost.Interface.Name); err != nil { klog.Infof("fail to return network device %s : %v", deviceName, err) } diff --git a/pkg/driver/pod_device_config.go b/pkg/driver/pod_device_config.go index 9a79dbaf..d49641a1 100644 --- a/pkg/driver/pod_device_config.go +++ b/pkg/driver/pod_device_config.go @@ -29,17 +29,15 @@ import ( type PodConfig struct { Claim types.NamespacedName - // OriginalInterfaceName is the name of the network interface as seen in the - // host's network namespace. - // - // TODO(improvement): Instead of OriginalInterfaceName being a stirng, we - // can instead have an OriginalInterfaceConfig of type apis.NetworkConfig - // which can retain information beyond the interface's name. - OriginalInterfaceName string - - // Network contains all network-related configurations (interface, routes, - // ethtool, sysctl) to be applied for this device in the Pod's namespace. - Network apis.NetworkConfig + // NetworkInterfaceConfigInHost is the config of the network interface as + // seen in the host's network namespace BEFORE it was moved to the pod's + // network namespace. + NetworkInterfaceConfigInHost apis.NetworkConfig + + // NetworkInterfaceConfigInPod contains all network-related configurations + // (interface, routes, ethtool, sysctl) to be applied for this device in the + // Pod's namespace. + NetworkInterfaceConfigInPod apis.NetworkConfig // RDMADevice holds RDMA-specific configurations if the network device // has associated RDMA capabilities. diff --git a/pkg/driver/pod_device_config_test.go b/pkg/driver/pod_device_config_test.go index 5f615341..23a66fc3 100644 --- a/pkg/driver/pod_device_config_test.go +++ b/pkg/driver/pod_device_config_test.go @@ -41,7 +41,7 @@ func TestPodConfigStore_SetAndGet(t *testing.T) { podUID := types.UID("test-pod-uid-1") deviceName := "eth0" config := PodConfig{ - Network: apis.NetworkConfig{ + NetworkInterfaceConfigInPod: apis.NetworkConfig{ Interface: apis.InterfaceConfig{Name: "eth0-pod"}, Routes: []apis.RouteConfig{ {Destination: "0.0.0.0/0", Gateway: "192.168.1.1"}, @@ -83,7 +83,7 @@ func TestPodConfigStore_SetAndGet(t *testing.T) { // Test overwriting newConfig := PodConfig{ - Network: apis.NetworkConfig{ + NetworkInterfaceConfigInPod: apis.NetworkConfig{ Interface: apis.InterfaceConfig{Name: "eth0-new"}, Ethtool: &apis.EthtoolConfig{PrivateFlags: map[string]bool{"custom-flag": false}}, }, @@ -104,9 +104,9 @@ func TestPodConfigStore_DeletePod(t *testing.T) { podUID2 := types.UID("test-pod-uid-2") dev1 := "eth0" dev2 := "eth1" - config1 := PodConfig{Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1eth0"}}} - config2 := PodConfig{Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1eth1"}}} - config3 := PodConfig{Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p2eth0"}}} + config1 := PodConfig{NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1eth0"}}} + config2 := PodConfig{NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1eth1"}}} + config3 := PodConfig{NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p2eth0"}}} store.Set(podUID1, dev1, config1) store.Set(podUID1, dev2, config2) @@ -141,9 +141,9 @@ func TestPodConfigStore_GetPodConfigs(t *testing.T) { podUID2 := types.UID("test-pod-uid-2") dev1 := "eth0" dev2 := "eth1" - config1 := PodConfig{Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1eth0"}}} - config2 := PodConfig{Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1eth1"}}} - config3 := PodConfig{Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p2eth0"}}} + config1 := PodConfig{NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1eth0"}}} + config2 := PodConfig{NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1eth1"}}} + config3 := PodConfig{NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p2eth0"}}} store.Set(podUID1, dev1, config1) store.Set(podUID1, dev2, config2) @@ -187,7 +187,7 @@ func TestPodConfigStore_ThreadSafety(t *testing.T) { defer wg.Done() podUID := types.UID(fmt.Sprintf("pod-%d", i)) deviceName := fmt.Sprintf("eth%d", i%2) - config := PodConfig{Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: fmt.Sprintf("dev-%d", i)}}} + config := PodConfig{NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: fmt.Sprintf("dev-%d", i)}}} store.Set(podUID, deviceName, config) retrieved, _ := store.Get(podUID, deviceName) if !reflect.DeepEqual(retrieved, config) { @@ -216,10 +216,10 @@ func TestPodConfigStore_DeleteClaim(t *testing.T) { dev1 := "eth0" dev2 := "eth1" - config1_1 := PodConfig{Claim: claim1, Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1d1c1"}}} // Pod1, Dev1, Claim1 - config1_2 := PodConfig{Claim: claim1, Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1d2c1"}}} // Pod1, Dev2, Claim1 - config2_1 := PodConfig{Claim: claim1, Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p2d1c1"}}} // Pod2, Dev1, Claim1 - config3_1 := PodConfig{Claim: claim2, Network: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p3d1c2"}}} // Pod3, Dev1, Claim2 + config1_1 := PodConfig{Claim: claim1, NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1d1c1"}}} // Pod1, Dev1, Claim1 + config1_2 := PodConfig{Claim: claim1, NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p1d2c1"}}} // Pod1, Dev2, Claim1 + config2_1 := PodConfig{Claim: claim1, NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p2d1c1"}}} // Pod2, Dev1, Claim1 + config3_1 := PodConfig{Claim: claim2, NetworkInterfaceConfigInPod: apis.NetworkConfig{Interface: apis.InterfaceConfig{Name: "p3d1c2"}}} // Pod3, Dev1, Claim2 tests := []struct { name string @@ -298,7 +298,7 @@ func TestPodConfigStore_NoDuplicateDevices(t *testing.T) { podUID := types.UID("test-pod-uid-1") deviceName1 := "eth0" config1 := PodConfig{ - Network: apis.NetworkConfig{ + NetworkInterfaceConfigInPod: apis.NetworkConfig{ Interface: apis.InterfaceConfig{Name: "eth0-pod"}, }, RDMADevice: RDMAConfig{ @@ -312,7 +312,7 @@ func TestPodConfigStore_NoDuplicateDevices(t *testing.T) { } deviceName2 := "eth1" config2 := PodConfig{ - Network: apis.NetworkConfig{ + NetworkInterfaceConfigInPod: apis.NetworkConfig{ Interface: apis.InterfaceConfig{Name: "eth2-pod"}, }, RDMADevice: RDMAConfig{ From 0be7289822a7ffa146724a8904a6718165901f60 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 21 Aug 2025 16:37:51 -0700 Subject: [PATCH 10/13] fix: Don't aggregate new errors if we failed to get network interface of device --- pkg/driver/dra_hooks.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/driver/dra_hooks.go b/pkg/driver/dra_hooks.go index 7a950d32..ec1936aa 100644 --- a/pkg/driver/dra_hooks.go +++ b/pkg/driver/dra_hooks.go @@ -167,6 +167,7 @@ func (np *NetworkDriver) prepareResourceClaim(ctx context.Context, claim *resour ifName, err := np.netdb.GetNetInterfaceName(result.Device) if err != nil { errorList = append(errorList, fmt.Errorf("failed to get network interface name for device %s: %v", result.Device, err)) + continue } // Get Network configuration and merge it link, err := nlHandle.LinkByName(ifName) From fba218fd645fe3a5cefe7b974c31e2755e18afff Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 21 Aug 2025 17:56:04 -0700 Subject: [PATCH 11/13] fix: Check for existence of interface attributes before referring to them in tests This is now required because not all devices have the exact same attributes. E.g. PCI Devices whose network interface is moved to the pod's network namespace don't have the network interface related attributes like: - dra.net/alias: - dra.net/ebpf: - dra.net/encapsulation: - dra.net/ifName: - dra.net/ipv4: - dra.net/mac: - dra.net/mtu: - gce.dra.net/networkName: - gce.dra.net/networkProjectNumber: - gce.dra.net/rdmaType This means that the user needs to check for their existence if referring to them. --- tests/manifests/repeatresourceclaimtemplate.yaml | 2 +- tests/manifests/resourceclaim.yaml | 2 +- tests/manifests/resourceclaim_advanced.yaml | 2 +- tests/manifests/resourceclaim_bigtcp.yaml | 2 +- tests/manifests/resourceclaim_disable_ebpf.yaml | 2 +- tests/manifests/resourceclaim_route.yaml | 2 +- tests/manifests/resourceclaimtemplate.yaml | 2 +- tests/manifests/resourceclaimtemplate_double.yaml | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/manifests/repeatresourceclaimtemplate.yaml b/tests/manifests/repeatresourceclaimtemplate.yaml index f1c09ffe..58611b27 100644 --- a/tests/manifests/repeatresourceclaimtemplate.yaml +++ b/tests/manifests/repeatresourceclaimtemplate.yaml @@ -34,7 +34,7 @@ spec: deviceClassName: multinic selectors: - cel: - expression: device.attributes["dra.net"].ifName == "dummy8" + expression: has(device.attributes["dra.net"].ifName) && device.attributes["dra.net"].ifName == "dummy8" --- apiVersion: apps/v1 kind: Deployment diff --git a/tests/manifests/resourceclaim.yaml b/tests/manifests/resourceclaim.yaml index f0d03091..fd7ab5e3 100644 --- a/tests/manifests/resourceclaim.yaml +++ b/tests/manifests/resourceclaim.yaml @@ -24,7 +24,7 @@ spec: deviceClassName: dra.net selectors: - cel: - expression: device.attributes["dra.net"].type == "dummy" + expression: has(device.attributes["dra.net"].type) && device.attributes["dra.net"].type == "dummy" config: - opaque: driver: dra.net diff --git a/tests/manifests/resourceclaim_advanced.yaml b/tests/manifests/resourceclaim_advanced.yaml index b1cd1090..d7e3e466 100644 --- a/tests/manifests/resourceclaim_advanced.yaml +++ b/tests/manifests/resourceclaim_advanced.yaml @@ -24,7 +24,7 @@ spec: deviceClassName: dra.net selectors: - cel: - expression: device.attributes["dra.net"].ifName == "dummy0" + expression: has(device.attributes["dra.net"].ifName) && device.attributes["dra.net"].ifName == "dummy0" config: - opaque: driver: dra.net diff --git a/tests/manifests/resourceclaim_bigtcp.yaml b/tests/manifests/resourceclaim_bigtcp.yaml index 8a962116..7afd27a6 100644 --- a/tests/manifests/resourceclaim_bigtcp.yaml +++ b/tests/manifests/resourceclaim_bigtcp.yaml @@ -24,7 +24,7 @@ spec: deviceClassName: dra.net selectors: - cel: - expression: device.attributes["dra.net"].ifName == "dummy0" + expression: has(device.attributes["dra.net"].ifName) && device.attributes["dra.net"].ifName == "dummy0" config: - opaque: driver: dra.net diff --git a/tests/manifests/resourceclaim_disable_ebpf.yaml b/tests/manifests/resourceclaim_disable_ebpf.yaml index 077ad546..2421cf8f 100644 --- a/tests/manifests/resourceclaim_disable_ebpf.yaml +++ b/tests/manifests/resourceclaim_disable_ebpf.yaml @@ -24,7 +24,7 @@ spec: deviceClassName: dra.net selectors: - cel: - expression: device.attributes["dra.net"].ifName == "dummy0" + expression: has(device.attributes["dra.net"].ifName) && device.attributes["dra.net"].ifName == "dummy0" config: - opaque: driver: dra.net diff --git a/tests/manifests/resourceclaim_route.yaml b/tests/manifests/resourceclaim_route.yaml index 6b775993..7ac10a61 100644 --- a/tests/manifests/resourceclaim_route.yaml +++ b/tests/manifests/resourceclaim_route.yaml @@ -24,7 +24,7 @@ spec: deviceClassName: dra.net selectors: - cel: - expression: device.attributes["dra.net"].type == "dummy" + expression: has(device.attributes["dra.net"].type) && device.attributes["dra.net"].type == "dummy" config: - opaque: driver: dra.net diff --git a/tests/manifests/resourceclaimtemplate.yaml b/tests/manifests/resourceclaimtemplate.yaml index 2fa214f4..59bfbc18 100644 --- a/tests/manifests/resourceclaimtemplate.yaml +++ b/tests/manifests/resourceclaimtemplate.yaml @@ -34,7 +34,7 @@ spec: deviceClassName: multinic selectors: - cel: - expression: device.attributes["dra.net"].ifName == "dummy0" + expression: has(device.attributes["dra.net"].ifName) && device.attributes["dra.net"].ifName == "dummy0" --- apiVersion: apps/v1 kind: Deployment diff --git a/tests/manifests/resourceclaimtemplate_double.yaml b/tests/manifests/resourceclaimtemplate_double.yaml index 92488745..7d976ca5 100644 --- a/tests/manifests/resourceclaimtemplate_double.yaml +++ b/tests/manifests/resourceclaimtemplate_double.yaml @@ -35,7 +35,7 @@ spec: deviceClassName: multinic selectors: - cel: - expression: device.attributes["dra.net"].ifName == "dummy0" || device.attributes["dra.net"].ifName == "dummy1" + expression: has(device.attributes["dra.net"].ifName) && (device.attributes["dra.net"].ifName == "dummy0" || device.attributes["dra.net"].ifName == "dummy1") --- apiVersion: apps/v1 kind: Deployment From 202abcf90c11c1220586a8642b1634f76dea9e8a Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Thu, 21 Aug 2025 22:46:54 -0700 Subject: [PATCH 12/13] chore: Address review comments from BenTheElder --- go.mod | 2 +- go.sum | 4 ++-- pkg/inventory/db.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 12aa24af..8f9baf56 100644 --- a/go.mod +++ b/go.mod @@ -112,7 +112,7 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - howett.net/plist v1.0.0 // indirect + howett.net/plist v1.0.1 // indirect k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect k8s.io/kubelet v0.34.0-rc.0 // indirect sigs.k8s.io/randfill v1.0.0 // indirect diff --git a/go.sum b/go.sum index c9262046..3e6f28b8 100644 --- a/go.sum +++ b/go.sum @@ -292,8 +292,8 @@ gopkg.in/yaml.v1 v1.0.0-20140924161607-9f9df34309c0/go.mod h1:WDnlLJ4WF5VGsH/HVa gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -howett.net/plist v1.0.0 h1:7CrbWYbPPO/PyNy38b2EB/+gYbjCe2DXBxgtOOZbSQM= -howett.net/plist v1.0.0/go.mod h1:lqaXoTrLY4hg8tnEzNru53gicrbv7rrk+2xJA/7hw9g= +howett.net/plist v1.0.1 h1:37GdZ8tP09Q35o9ych3ehygcsL+HqKSwzctveSlarvM= +howett.net/plist v1.0.1/go.mod h1:lqaXoTrLY4hg8tnEzNru53gicrbv7rrk+2xJA/7hw9g= k8s.io/api v0.34.0-rc.0 h1:0jrCG9TkH250E5wTqchpijMoMxmQ6FEl9xNum25FFSk= k8s.io/api v0.34.0-rc.0/go.mod h1:H2VCZSAruVExfN0k+jj42LXFGQOvRhZDVbY1JlHGtHE= k8s.io/apimachinery v0.34.0-rc.0 h1:PjXj9Fp/f53PN7JMtGnR7AldAbdztqNqT3SjABe99D8= diff --git a/pkg/inventory/db.go b/pkg/inventory/db.go index 21e9dab2..199fa2f0 100644 --- a/pkg/inventory/db.go +++ b/pkg/inventory/db.go @@ -378,8 +378,8 @@ func (db *DB) updateDeviceStore(devices []resourceapi.Device) { deviceStore[device.Name] = device } db.mu.Lock() + defer db.mu.Unlock() db.deviceStore = deviceStore - db.mu.Unlock() } func (db *DB) GetDevice(deviceName string) (resourceapi.Device, bool) { From cceb50a802f844af7e64cd7f98b85714a5a4286f Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Sun, 24 Aug 2025 21:26:49 -0700 Subject: [PATCH 13/13] build: Bump dep github.com/jaypipes/ghw to v0.19.0 to minimize other deps --- go.mod | 9 ++++----- go.sum | 20 ++++++++------------ 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index 8f9baf56..f6265018 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/google/cel-go v0.26.0 github.com/google/go-cmp v0.7.0 github.com/insomniacslk/dhcp v0.0.0-20250417080101-5f8cf70e8c5f - github.com/jaypipes/ghw v0.17.0 + github.com/jaypipes/ghw v0.19.0 github.com/mdlayher/genetlink v1.3.2 github.com/mdlayher/netlink v1.7.2 github.com/prometheus/client_golang v1.23.0 @@ -39,7 +39,6 @@ require ( cel.dev/expr v0.24.0 // indirect cloud.google.com/go/auth v0.16.5 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect - github.com/StackExchange/wmi v1.2.1 // indirect github.com/antlr4-go/antlr/v4 v4.13.1 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect @@ -62,7 +61,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect github.com/googleapis/gax-go/v2 v2.15.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/jaypipes/pcidb v1.0.1 // indirect + github.com/jaypipes/pcidb v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/josharian/native v1.1.0 // indirect github.com/json-iterator/go v1.1.12 // indirect @@ -70,7 +69,6 @@ require ( github.com/mailru/easyjson v0.9.0 // indirect github.com/mdlayher/packet v1.1.2 // indirect github.com/mdlayher/socket v0.5.1 // indirect - github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect @@ -87,6 +85,7 @@ require ( github.com/tetratelabs/wazero v1.9.0 // indirect github.com/u-root/uio v0.0.0-20240224005618-d2acac8f3701 // indirect github.com/x448/float16 v0.8.4 // indirect + github.com/yusufpapurcu/wmi v1.2.4 // indirect go.etcd.io/etcd/client/pkg/v3 v3.6.4 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 // indirect @@ -112,7 +111,7 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - howett.net/plist v1.0.1 // indirect + howett.net/plist v1.0.2-0.20250314012144-ee69052608d9 // indirect k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect k8s.io/kubelet v0.34.0-rc.0 // indirect sigs.k8s.io/randfill v1.0.0 // indirect diff --git a/go.sum b/go.sum index 3e6f28b8..a4595705 100644 --- a/go.sum +++ b/go.sum @@ -14,8 +14,6 @@ cloud.google.com/go/container v1.44.0 h1:JEHeW535svvNwJrjrlQ/cdjd15LCWrPKnHsulru cloud.google.com/go/container v1.44.0/go.mod h1:tVK2o4UZUTkg9WpBcgj4qRzwGA1dSFdWA3mil3YkLIQ= github.com/Mellanox/rdmamap v1.1.0 h1:A/W1wAXw+6vm58f3VklrIylgV+eDJlPVIMaIKuxgUT4= github.com/Mellanox/rdmamap v1.1.0/go.mod h1:fN+/V9lf10ABnDCwTaXRjeeWijLt2iVLETnK+sx/LY8= -github.com/StackExchange/wmi v1.2.1 h1:VIkavFPXSjcnS+O8yTq7NI32k0R5Aj+v39y29VYDOSA= -github.com/StackExchange/wmi v1.2.1/go.mod h1:rcmrprowKIVzvc+NUiLncP2uuArMWLCbu9SBzvHz7e8= github.com/antlr4-go/antlr/v4 v4.13.1 h1:SqQKkuVZ+zWkMMNkjy5FZe5mr5WURWnlpmOuzYWrPrQ= github.com/antlr4-go/antlr/v4 v4.13.1/go.mod h1:GKmUxMtwp6ZgGwZSva4eWPC5mS6vUAmOABFgjdkM7Nw= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= @@ -46,7 +44,6 @@ github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= -github.com/go-ole/go-ole v1.2.5/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY= github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-openapi/jsonpointer v0.21.1 h1:whnzv/pNXtK2FbX/W9yJfRmE2gsmkfahjMKB0fZvcic= @@ -86,10 +83,10 @@ github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/insomniacslk/dhcp v0.0.0-20250417080101-5f8cf70e8c5f h1:dd33oobuIv9PcBVqvbEiCXEbNTomOHyj3WFuC5YiPRU= github.com/insomniacslk/dhcp v0.0.0-20250417080101-5f8cf70e8c5f/go.mod h1:zhFlBeJssZ1YBCMZ5Lzu1pX4vhftDvU10WUVb1uXKtM= -github.com/jaypipes/ghw v0.17.0 h1:EVLJeNcy5z6GK/Lqby0EhBpynZo+ayl8iJWY0kbEUJA= -github.com/jaypipes/ghw v0.17.0/go.mod h1:In8SsaDqlb1oTyrbmTC14uy+fbBMvp+xdqX51MidlD8= -github.com/jaypipes/pcidb v1.0.1 h1:WB2zh27T3nwg8AE8ei81sNRb9yWBii3JGNJtT7K9Oic= -github.com/jaypipes/pcidb v1.0.1/go.mod h1:6xYUz/yYEyOkIkUt2t2J2folIuZ4Yg6uByCGFXMCeE4= +github.com/jaypipes/ghw v0.19.0 h1:Yf4Qy4Nb+U6W8CvTJ6lnVj2H6sG4mTz0c5QOnfwb0LE= +github.com/jaypipes/ghw v0.19.0/go.mod h1:UkqErkfrTUNgvABKyZVEzbz+wMNtvoT9RXGSloV8TUs= +github.com/jaypipes/pcidb v1.1.0 h1:f1Xh6LXRlYDw0aLyCwOEYaE0ifZBTdlCfLK8YW7A/so= +github.com/jaypipes/pcidb v1.1.0/go.mod h1:x27LT2krrUgjf875KxQXKB0Ha/YXLdZRVmw6hH0G7g8= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= @@ -122,8 +119,6 @@ github.com/mdlayher/packet v1.1.2 h1:3Up1NG6LZrsgDVn6X4L9Ge/iyRyxFEFD9o6Pr3Q1nQY github.com/mdlayher/packet v1.1.2/go.mod h1:GEu1+n9sG5VtiRE4SydOmX5GTwyyYlteZiFU+x0kew4= github.com/mdlayher/socket v0.5.1 h1:VZaqt6RkGkt2OE9l3GcC6nZkqD3xKeQLyfleW/uBcos= github.com/mdlayher/socket v0.5.1/go.mod h1:TjPLHI1UgwEv5J1B5q0zTZq12A/6H7nKmtTanQE37IQ= -github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= -github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -191,6 +186,8 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0= +github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= go.etcd.io/etcd/client/pkg/v3 v3.6.4 h1:9HBYrjppeOfFjBjaMTRxT3R7xT0GLK8EJMVC4xg6ok0= go.etcd.io/etcd/client/pkg/v3 v3.6.4/go.mod h1:sbdzr2cl3HzVmxNw//PH7aLGVtY4QySjQFuaCgcRFAI= go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= @@ -288,12 +285,11 @@ gopkg.in/evanphx/json-patch.v4 v4.12.0 h1:n6jtcsulIzXPJaxegRbvFNNrZDjbij7ny3gmSP gopkg.in/evanphx/json-patch.v4 v4.12.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= -gopkg.in/yaml.v1 v1.0.0-20140924161607-9f9df34309c0/go.mod h1:WDnlLJ4WF5VGsH/HVa3CI79GS0ol3YnhVnKP89i0kNg= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -howett.net/plist v1.0.1 h1:37GdZ8tP09Q35o9ych3ehygcsL+HqKSwzctveSlarvM= -howett.net/plist v1.0.1/go.mod h1:lqaXoTrLY4hg8tnEzNru53gicrbv7rrk+2xJA/7hw9g= +howett.net/plist v1.0.2-0.20250314012144-ee69052608d9 h1:eeH1AIcPvSc0Z25ThsYF+Xoqbn0CI/YnXVYoTLFdGQw= +howett.net/plist v1.0.2-0.20250314012144-ee69052608d9/go.mod h1:fyFX5Hj5tP1Mpk8obqA9MZgXT416Q5711SDT7dQLTLk= k8s.io/api v0.34.0-rc.0 h1:0jrCG9TkH250E5wTqchpijMoMxmQ6FEl9xNum25FFSk= k8s.io/api v0.34.0-rc.0/go.mod h1:H2VCZSAruVExfN0k+jj42LXFGQOvRhZDVbY1JlHGtHE= k8s.io/apimachinery v0.34.0-rc.0 h1:PjXj9Fp/f53PN7JMtGnR7AldAbdztqNqT3SjABe99D8=