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 <[email protected]> * modified json name to excludeTopology Co-authored-by: Hanamantagoud <[email protected]>
k8snetworkplumbingwg · May 10, 2022 · b1e840f · b1e840f
1 parent 58eb366
commit b1e840f
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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