Skip to content

Commit

Permalink
Make advertising of NUMA node optional
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
rthakur-est committed Mar 22, 2022
1 parent 4433b72 commit 90dcac9
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 16 deletions.
5 changes: 3 additions & 2 deletions pkg/accelerator/accelDevice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/accelerator/accelDeviceProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
42 changes: 37 additions & 5 deletions pkg/accelerator/accelDevice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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(""))
Expand All @@ -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(""))
Expand All @@ -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())
Expand All @@ -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"))
Expand Down
2 changes: 1 addition & 1 deletion pkg/netdevice/pciNetDevice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/netdevice/pciNetDevice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
8 changes: 6 additions & 2 deletions pkg/resources/pciDevice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions pkg/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"exclude_topology,omitempty"`
Selectors *json.RawMessage `json:"selectors,omitempty"`
SelectorObj interface{}
}

// DeviceSelectors contains common device selectors fields
Expand Down

0 comments on commit 90dcac9

Please sign in to comment.