From b1e840f4753f6ca4bc9d139c1d46c27cc48af6b9 Mon Sep 17 00:00:00 2001 From: rnthakur <81964944+rthakur-est@users.noreply.github.com> Date: Tue, 10 May 2022 12:42:09 +0530 Subject: [PATCH] Make advertising of NUMA node optional (#416) * Make advertising of NUMA node optional This change adds support for making advertising of NUMA node optional. This can be done per resource pool by setting the config "exclude_topology" to true. Default behavior is to advertise NUMA node information. Signed-off-by: Ravindra Thakur * modified json name to excludeTopology Co-authored-by: Hanamantagoud --- pkg/accelerator/accelDevice.go | 5 +-- pkg/accelerator/accelDeviceProvider.go | 2 +- pkg/accelerator/accelDevice_test.go | 42 +++++++++++++++++++++++--- pkg/netdevice/pciNetDevice.go | 2 +- pkg/netdevice/pciNetDevice_test.go | 26 ++++++++++++++++ pkg/resources/pciDevice.go | 8 +++-- pkg/types/types.go | 11 ++++--- 7 files changed, 80 insertions(+), 16 deletions(-) diff --git a/pkg/accelerator/accelDevice.go b/pkg/accelerator/accelDevice.go index 26ec1d1c7..3be9c56c8 100644 --- a/pkg/accelerator/accelDevice.go +++ b/pkg/accelerator/accelDevice.go @@ -27,8 +27,9 @@ type accelDevice struct { } // NewAccelDevice returns an instance of AccelDevice interface -func NewAccelDevice(dev *ghw.PCIDevice, rFactory types.ResourceFactory) (types.AccelDevice, error) { - pciDev, err := resources.NewPciDevice(dev, rFactory, nil) +func NewAccelDevice(dev *ghw.PCIDevice, rFactory types.ResourceFactory, + rc *types.ResourceConfig) (types.AccelDevice, error) { + pciDev, err := resources.NewPciDevice(dev, rFactory, rc, nil) if err != nil { return nil, err } diff --git a/pkg/accelerator/accelDeviceProvider.go b/pkg/accelerator/accelDeviceProvider.go index 3e6ff95fc..08a5563b3 100644 --- a/pkg/accelerator/accelDeviceProvider.go +++ b/pkg/accelerator/accelDeviceProvider.go @@ -51,7 +51,7 @@ func (ap *accelDeviceProvider) GetDiscoveredDevices() []*ghw.PCIDevice { func (ap *accelDeviceProvider) GetDevices(rc *types.ResourceConfig) []types.PciDevice { newPciDevices := make([]types.PciDevice, 0) for _, device := range ap.deviceList { - if newDevice, err := NewAccelDevice(device, ap.rFactory); err == nil { + if newDevice, err := NewAccelDevice(device, ap.rFactory, rc); err == nil { newPciDevices = append(newPciDevices, newDevice) } else { glog.Errorf("accelerator GetDevices() error creating new device: %q", err) diff --git a/pkg/accelerator/accelDevice_test.go b/pkg/accelerator/accelDevice_test.go index ab9af75c6..f8ae5922b 100644 --- a/pkg/accelerator/accelDevice_test.go +++ b/pkg/accelerator/accelDevice_test.go @@ -19,6 +19,7 @@ import ( "github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/accelerator" "github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/factory" + "github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types" "github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/utils" . "github.com/onsi/ginkgo" @@ -46,8 +47,9 @@ var _ = Describe("Accelerator", func() { f := factory.NewResourceFactory("fake", "fake", true) in := &ghw.PCIDevice{Address: "0000:00:00.1"} + config := &types.ResourceConfig{} - out, err := accelerator.NewAccelDevice(in, f) + out, err := accelerator.NewAccelDevice(in, f, config) // TODO: assert other fields once implemented Expect(out.GetDriver()).To(Equal("vfio-pci")) @@ -75,8 +77,9 @@ var _ = Describe("Accelerator", func() { f := factory.NewResourceFactory("fake", "fake", true) in := &ghw.PCIDevice{Address: "0000:00:00.1"} + config := &types.ResourceConfig{} - out, err := accelerator.NewAccelDevice(in, f) + out, err := accelerator.NewAccelDevice(in, f, config) Expect(out.GetAPIDevice().Topology).To(BeNil()) Expect(out.GetNumaInfo()).To(Equal("")) @@ -99,8 +102,35 @@ var _ = Describe("Accelerator", func() { f := factory.NewResourceFactory("fake", "fake", true) in := &ghw.PCIDevice{Address: "0000:00:00.1"} + config := &types.ResourceConfig{} - out, err := accelerator.NewAccelDevice(in, f) + out, err := accelerator.NewAccelDevice(in, f, config) + + Expect(out.GetAPIDevice().Topology).To(BeNil()) + Expect(out.GetNumaInfo()).To(Equal("")) + Expect(err).NotTo(HaveOccurred()) + }) + It("should not populate topology due to config excluding topology being set", func() { + fs := &utils.FakeFilesystem{ + Dirs: []string{ + "sys/bus/pci/devices/0000:00:00.1/net/eth0", + "sys/kernel/iommu_groups/0", + "sys/bus/pci/drivers/vfio-pci", + }, + Symlinks: map[string]string{ + "sys/bus/pci/devices/0000:00:00.1/iommu_group": "../../../../kernel/iommu_groups/0", + "sys/bus/pci/devices/0000:00:00.1/driver": "../../../../bus/pci/drivers/vfio-pci", + }, + Files: map[string][]byte{"sys/bus/pci/devices/0000:00:00.1/numa_node": []byte("1")}, + } + defer fs.Use()() + utils.SetDefaultMockNetlinkProvider() + + f := factory.NewResourceFactory("fake", "fake", true) + in := &ghw.PCIDevice{Address: "0000:00:00.1"} + config := &types.ResourceConfig{ExcludeTopology: true} + + out, err := accelerator.NewAccelDevice(in, f, config) Expect(out.GetAPIDevice().Topology).To(BeNil()) Expect(out.GetNumaInfo()).To(Equal("")) @@ -120,8 +150,9 @@ var _ = Describe("Accelerator", func() { in := &ghw.PCIDevice{ Address: "0000:00:00.1", } + config := &types.ResourceConfig{} - dev, err := accelerator.NewAccelDevice(in, f) + dev, err := accelerator.NewAccelDevice(in, f, config) Expect(dev).To(BeNil()) Expect(err).To(HaveOccurred()) @@ -143,8 +174,9 @@ var _ = Describe("Accelerator", func() { in := &ghw.PCIDevice{ Address: "0000:00:00.1", } + config := &types.ResourceConfig{} - dev, err := accelerator.NewAccelDevice(in, f) + dev, err := accelerator.NewAccelDevice(in, f, config) Expect(err).NotTo(HaveOccurred()) Expect(dev).NotTo(BeNil()) Expect(dev.GetEnvVal()).To(Equal("0000:00:00.1")) diff --git a/pkg/netdevice/pciNetDevice.go b/pkg/netdevice/pciNetDevice.go index b23e267c8..34b23fade 100644 --- a/pkg/netdevice/pciNetDevice.go +++ b/pkg/netdevice/pciNetDevice.go @@ -73,7 +73,7 @@ func NewPciNetDevice(dev *ghw.PCIDevice, rFactory types.ResourceFactory, rc *typ } } - pciDev, err := resources.NewPciDevice(dev, rFactory, infoProviders) + pciDev, err := resources.NewPciDevice(dev, rFactory, rc, infoProviders) if err != nil { return nil, err } diff --git a/pkg/netdevice/pciNetDevice_test.go b/pkg/netdevice/pciNetDevice_test.go index 3baef31a4..2c28f9ef8 100644 --- a/pkg/netdevice/pciNetDevice_test.go +++ b/pkg/netdevice/pciNetDevice_test.go @@ -119,6 +119,32 @@ var _ = Describe("PciNetDevice", func() { dev, err := netdevice.NewPciNetDevice(in, f, rc) + Expect(dev.GetAPIDevice().Topology).To(BeNil()) + Expect(dev.GetNumaInfo()).To(Equal("")) + Expect(err).NotTo(HaveOccurred()) + }) + It("should not populate topology due to config option being set", func() { + fs := &utils.FakeFilesystem{ + Dirs: []string{ + "sys/bus/pci/devices/0000:00:00.1/net/eth0", + "sys/kernel/iommu_groups/0", + "sys/bus/pci/drivers/vfio-pci", + }, + Symlinks: map[string]string{ + "sys/bus/pci/devices/0000:00:00.1/iommu_group": "../../../../kernel/iommu_groups/0", + "sys/bus/pci/devices/0000:00:00.1/driver": "../../../../bus/pci/drivers/vfio-pci", + }, + Files: map[string][]byte{"sys/bus/pci/devices/0000:00:00.1/numa_node": []byte("0")}, + } + defer fs.Use()() + utils.SetDefaultMockNetlinkProvider() + + f := factory.NewResourceFactory("fake", "fake", true) + in := &ghw.PCIDevice{Address: "0000:00:00.1"} + rc := &types.ResourceConfig{ExcludeTopology: true} + + dev, err := netdevice.NewPciNetDevice(in, f, rc) + Expect(dev.GetAPIDevice().Topology).To(BeNil()) Expect(dev.GetNumaInfo()).To(Equal("")) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/resources/pciDevice.go b/pkg/resources/pciDevice.go index bdef2239c..4f990804c 100644 --- a/pkg/resources/pciDevice.go +++ b/pkg/resources/pciDevice.go @@ -46,7 +46,8 @@ func nodeToStr(nodeNum int) string { // NewPciDevice returns an instance of PciDevice interface // A list of DeviceInfoProviders can be set externally. // If empty, the default driver-based selection provided by ResourceFactory will be used -func NewPciDevice(dev *ghw.PCIDevice, rFactory types.ResourceFactory, infoProviders []types.DeviceInfoProvider) (types.PciDevice, error) { +func NewPciDevice(dev *ghw.PCIDevice, rFactory types.ResourceFactory, rc *types.ResourceConfig, + infoProviders []types.DeviceInfoProvider) (types.PciDevice, error) { pciAddr := dev.Address // Get PF PCI address @@ -70,8 +71,11 @@ func NewPciDevice(dev *ghw.PCIDevice, rFactory types.ResourceFactory, infoProvid if len(infoProviders) == 0 { infoProviders = append(infoProviders, rFactory.GetDefaultInfoProvider(pciAddr, driverName)) } + nodeNum := -1 + if !rc.ExcludeTopology { + nodeNum = utils.GetDevNode(pciAddr) + } - nodeNum := utils.GetDevNode(pciAddr) apiDevice := &pluginapi.Device{ ID: pciAddr, Health: pluginapi.Healthy, diff --git a/pkg/types/types.go b/pkg/types/types.go index 20947f206..aa15e5f4d 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -84,11 +84,12 @@ var SupportedDevices = map[DeviceType]int{ // ResourceConfig contains configuration parameters for a resource pool type ResourceConfig struct { // optional resource prefix that will overwrite global prefix specified in cli params - ResourcePrefix string `json:"resourcePrefix,omitempty"` - ResourceName string `json:"resourceName"` // the resource name will be added with resource prefix in K8s api - DeviceType DeviceType `json:"deviceType,omitempty"` - Selectors *json.RawMessage `json:"selectors,omitempty"` - SelectorObj interface{} + ResourcePrefix string `json:"resourcePrefix,omitempty"` + ResourceName string `json:"resourceName"` // the resource name will be added with resource prefix in K8s api + DeviceType DeviceType `json:"deviceType,omitempty"` + ExcludeTopology bool `json:"excludeTopology,omitempty"` + Selectors *json.RawMessage `json:"selectors,omitempty"` + SelectorObj interface{} } // DeviceSelectors contains common device selectors fields