From 81a0e6a0c3ef0c386f801a77b9a0a55b63594ef3 Mon Sep 17 00:00:00 2001 From: Ian Howell Date: Thu, 5 Oct 2023 04:06:50 -0500 Subject: [PATCH] Add `acpiIndex` selector (#488) acpiIndex selector selects devices based on their ACPI index if available. This is useful e.g in VM environment where a device's ACPI index can be predefined during VM creation time, device selection can then be based on that index. --- README.md | 11 ++++-- pkg/devices/gen_pci.go | 18 ++++++++- pkg/factory/factory.go | 11 ++++++ pkg/netdevice/netDeviceProvider.go | 56 +++++++-------------------- pkg/resources/deviceSelectors.go | 20 ++++++++++ pkg/resources/deviceSelectors_test.go | 20 ++++++++++ pkg/types/mocks/AccelDevice.go | 14 +++++++ pkg/types/mocks/AuxNetDevice.go | 14 +++++++ pkg/types/mocks/HostDevice.go | 14 +++++++ pkg/types/mocks/NetDevice.go | 14 +++++++ pkg/types/mocks/PciDevice.go | 14 +++++++ pkg/types/mocks/PciNetDevice.go | 14 +++++++ pkg/types/mocks/ResourceFactory.go | 16 ++++++++ pkg/types/types.go | 4 ++ pkg/utils/utils.go | 18 +++++++++ 15 files changed, 210 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 51356a00f..d3fe6b1e7 100644 --- a/README.md +++ b/README.md @@ -309,6 +309,7 @@ These selectors are applicable when "deviceType" is "accelerator". | "devices" | N | Target Devices' device Hex code as string | `string` list Default: `null` | "devices": ["154c", "1889", "1018"] | | "drivers" | N | Target device driver names as string | `string` list Default: `null` | "drivers": ["vfio-pci"] | | "pciAddresses" | N | Target device's pci address as string | `string` list Default: `null` | "pciAddresses": ["0000:03:02.0"] | +| "acpiIndexes" | N | Target device's acpi index as string | `string` list Default: `null` | "acpiIndexes": ["101"] | #### Network devices selectors @@ -321,6 +322,7 @@ These selectors are applicable when "deviceType" is "netDevice" (note: this is d | "devices" | N | Target Devices' device Hex code as string | `string` list Default: `null` | "devices": ["154c", "1889", "1018"] | | "drivers" | N | Target device driver names as string | `string` list Default: `null` | "drivers": ["vfio-pci"] | | "pciAddresses" | N | Target device's pci address as string | `string` list Default: `null` | "pciAddresses": ["0000:03:02.0"] | +| "acpiIndexes" | N | Target device's acpi index as string | `string` list Default: `null` | "acpiIndexes": ["101"] | | "pfNames" | N | functions from PF matches list of PF names | `string` list Default: `null` | "pfNames": ["enp2s2f0"] (See follow-up sections for some advance usage of "pfNames") | | "rootDevices" | N | functions from PF matches list of PF PCI addresses | `string` list Default: `null` | "rootDevices": ["0000:86:00.0"] (See follow-up sections for some advance usage of "rootDevices") | | "linkTypes" | N | The link type of the net device associated with the PCI device | `string` list Default: `null` | "linkTypes": ["ether"] | @@ -441,10 +443,11 @@ The device plugin will initially discover all PCI network resources in the host 2. "devices" - The device hex code of device 3. "drivers" - The driver name the device is registered with 4. "pciAddresses" - The pci address of the device in BDF notation (if device type is accelerator or netDevice) -5. "auxTypes" - The type of auxiliary network device (if device type is auxNetDevice). -6. "pfNames" - The Physical function name -7. "rootDevices" - The Physical function PCI address -8. "linkTypes" - The link type of the net device associated with the PCI device +5. "acpiIndexes" - The acpi index of the device +6. "auxTypes" - The type of auxiliary network device (if device type is auxNetDevice). +7. "pfNames" - The Physical function name +8. "rootDevices" - The Physical function PCI address +9. "linkTypes" - The link type of the net device associated with the PCI device If a single device matches multiple selector objects, it will only be allocated to the first one. diff --git a/pkg/devices/gen_pci.go b/pkg/devices/gen_pci.go index 7034e4b21..5e4e59534 100644 --- a/pkg/devices/gen_pci.go +++ b/pkg/devices/gen_pci.go @@ -19,19 +19,28 @@ package devices import ( "github.com/jaypipes/ghw" + + "github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/utils" ) // GenPciDevice implementation type GenPciDevice struct { - pciAddr string + pciAddr string + acpiIndex string } // NewGenPciDevice returns GenPciDevice instance func NewGenPciDevice(dev *ghw.PCIDevice) (*GenPciDevice, error) { pciAddr := dev.Address + acpiIndex, err := utils.GetAcpiIndex(dev.Address) + if err != nil { + return nil, err + } + return &GenPciDevice{ - pciAddr: pciAddr, + pciAddr: pciAddr, + acpiIndex: acpiIndex, }, nil } @@ -39,3 +48,8 @@ func NewGenPciDevice(dev *ghw.PCIDevice) (*GenPciDevice, error) { func (pd *GenPciDevice) GetPciAddr() string { return pd.pciAddr } + +// GetAcpiIndex returns ACPI index of the device +func (pd *GenPciDevice) GetAcpiIndex() string { + return pd.acpiIndex +} diff --git a/pkg/factory/factory.go b/pkg/factory/factory.go index 3fb6d8ad9..5738d2302 100644 --- a/pkg/factory/factory.go +++ b/pkg/factory/factory.go @@ -93,6 +93,8 @@ func (rf *resourceFactory) GetSelector(attr string, values []string) (types.Devi return resources.NewRootDeviceSelector(values), nil case "linkTypes": return resources.NewLinkTypeSelector(values), nil + case "acpiIndexes": + return resources.NewAcpiIndexSelector(values), nil case "ddpProfiles": return resources.NewDdpSelector(values), nil case "auxTypes": @@ -102,6 +104,15 @@ func (rf *resourceFactory) GetSelector(attr string, values []string) (types.Devi } } +func (rf *resourceFactory) FilterBySelector(selectorName string, values []string, devicesToFilter []types.HostDevice) []types.HostDevice { + if len(values) > 0 { + if selector, err := rf.GetSelector(selectorName, values); err == nil { + return selector.Filter(devicesToFilter) + } + } + return devicesToFilter +} + // GetResourcePool returns an instance of resourcePool func (rf *resourceFactory) GetResourcePool(rc *types.ResourceConfig, filteredDevice []types.HostDevice) (types.ResourcePool, error) { devicePool := make(map[string]types.HostDevice) diff --git a/pkg/netdevice/netDeviceProvider.go b/pkg/netdevice/netDeviceProvider.go index f106d87fa..11ec41319 100644 --- a/pkg/netdevice/netDeviceProvider.go +++ b/pkg/netdevice/netDeviceProvider.go @@ -94,64 +94,36 @@ func (np *netDeviceProvider) GetFilteredDevices(devices []types.HostDevice, } rf := np.rFactory + // filter by vendor list - if nf.Vendors != nil && len(nf.Vendors) > 0 { - if selector, err := rf.GetSelector("vendors", nf.Vendors); err == nil { - filteredDevice = selector.Filter(filteredDevice) - } - } + filteredDevice = rf.FilterBySelector("vendors", nf.Vendors, filteredDevice) // filter by device list - if nf.Devices != nil && len(nf.Devices) > 0 { - if selector, err := rf.GetSelector("devices", nf.Devices); err == nil { - filteredDevice = selector.Filter(filteredDevice) - } - } + filteredDevice = rf.FilterBySelector("devices", nf.Devices, filteredDevice) // filter by driver list - if nf.Drivers != nil && len(nf.Drivers) > 0 { - if selector, err := rf.GetSelector("drivers", nf.Drivers); err == nil { - filteredDevice = selector.Filter(filteredDevice) - } - } + filteredDevice = rf.FilterBySelector("drivers", nf.Drivers, filteredDevice) // filter by pciAddresses list - if nf.PciAddresses != nil && len(nf.PciAddresses) > 0 { - if selector, err := rf.GetSelector("pciAddresses", nf.PciAddresses); err == nil { - filteredDevice = selector.Filter(filteredDevice) - } - } + filteredDevice = rf.FilterBySelector("pciAddresses", nf.PciAddresses, filteredDevice) + + // filter by acpiIndexes list + filteredDevice = rf.FilterBySelector("acpiIndexes", nf.AcpiIndexes, filteredDevice) // filter by PfNames list - if nf.PfNames != nil && len(nf.PfNames) > 0 { - if selector, err := rf.GetSelector("pfNames", nf.PfNames); err == nil { - filteredDevice = selector.Filter(filteredDevice) - } - } + filteredDevice = rf.FilterBySelector("pfNames", nf.PfNames, filteredDevice) // filter by RootDevices list - if nf.RootDevices != nil && len(nf.RootDevices) > 0 { - if selector, err := rf.GetSelector("rootDevices", nf.RootDevices); err == nil { - filteredDevice = selector.Filter(filteredDevice) - } - } + filteredDevice = rf.FilterBySelector("rootDevices", nf.RootDevices, filteredDevice) // filter by linkTypes list - if nf.LinkTypes != nil && len(nf.LinkTypes) > 0 { - if len(nf.LinkTypes) > 1 { - glog.Warningf("Link type selector should have a single value.") - } - if selector, err := rf.GetSelector("linkTypes", nf.LinkTypes); err == nil { - filteredDevice = selector.Filter(filteredDevice) - } + if len(nf.LinkTypes) > 1 { + glog.Warningf("Link type selector should have a single value.") } + filteredDevice = rf.FilterBySelector("linkTypes", nf.LinkTypes, filteredDevice) // filter by DDP Profiles list - if nf.DDPProfiles != nil && len(nf.DDPProfiles) > 0 { - if selector, err := rf.GetSelector("ddpProfiles", nf.DDPProfiles); err == nil { - filteredDevice = selector.Filter(filteredDevice) - } - } + filteredDevice = rf.FilterBySelector("ddpProfiles", nf.DDPProfiles, filteredDevice) // filter for rdma devices if nf.IsRdma { diff --git a/pkg/resources/deviceSelectors.go b/pkg/resources/deviceSelectors.go index 355d8c96d..c3164178f 100644 --- a/pkg/resources/deviceSelectors.go +++ b/pkg/resources/deviceSelectors.go @@ -93,6 +93,26 @@ func (s *pciAddressSelector) Filter(inDevices []types.HostDevice) []types.HostDe return filteredList } +// NewAcpiIndexSelector returns a NetDevSelector interface for netDev list +func NewAcpiIndexSelector(acpiIndexes []string) types.DeviceSelector { + return &acpiIndexSelector{acpiIndexes: acpiIndexes} +} + +type acpiIndexSelector struct { + acpiIndexes []string +} + +func (s *acpiIndexSelector) Filter(inDevices []types.HostDevice) []types.HostDevice { + filteredList := make([]types.HostDevice, 0) + for _, dev := range inDevices { + acpiIndex := dev.(types.PciDevice).GetAcpiIndex() + if contains(s.acpiIndexes, acpiIndex) { + filteredList = append(filteredList, dev) + } + } + return filteredList +} + // NewPfNameSelector returns a NetDevSelector interface for netDev list func NewPfNameSelector(pfNames []string) types.DeviceSelector { return &pfNameSelector{pfNames: pfNames} diff --git a/pkg/resources/deviceSelectors_test.go b/pkg/resources/deviceSelectors_test.go index 3bbfc5aaf..a841dcc78 100644 --- a/pkg/resources/deviceSelectors_test.go +++ b/pkg/resources/deviceSelectors_test.go @@ -297,4 +297,24 @@ var _ = Describe("DeviceSelectors", func() { }) }) }) + + Describe("acpiIndex selector", func() { + Context("filtering", func() { + It("should return devices matching the correct acpi index", func() { + acpiIndexes := []string{"101"} + sel := resources.NewAcpiIndexSelector(acpiIndexes) + + dev0 := mocks.PciNetDevice{} + dev0.On("GetAcpiIndex").Return("101") + dev1 := mocks.PciNetDevice{} + dev1.On("GetAcpiIndex").Return("102") + + in := []types.HostDevice{&dev0, &dev1} + filtered := sel.Filter(in) + + Expect(filtered).To(ContainElement(&dev0)) + Expect(filtered).NotTo(ContainElement(&dev1)) + }) + }) + }) }) diff --git a/pkg/types/mocks/AccelDevice.go b/pkg/types/mocks/AccelDevice.go index 3cb869d4a..3eddfbbe9 100644 --- a/pkg/types/mocks/AccelDevice.go +++ b/pkg/types/mocks/AccelDevice.go @@ -30,6 +30,20 @@ func (_m *AccelDevice) GetAPIDevice() *v1beta1.Device { return r0 } +// GetAcpiIndex provides a mock function with given fields: +func (_m *AccelDevice) GetAcpiIndex() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + // GetDeviceCode provides a mock function with given fields: func (_m *AccelDevice) GetDeviceCode() string { ret := _m.Called() diff --git a/pkg/types/mocks/AuxNetDevice.go b/pkg/types/mocks/AuxNetDevice.go index f5bd06977..047eb630b 100644 --- a/pkg/types/mocks/AuxNetDevice.go +++ b/pkg/types/mocks/AuxNetDevice.go @@ -30,6 +30,20 @@ func (_m *AuxNetDevice) GetAPIDevice() *v1beta1.Device { return r0 } +// GetAcpiIndex provides a mock function with given fields: +func (_m *AuxNetDevice) GetAcpiIndex() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + // GetAuxType provides a mock function with given fields: func (_m *AuxNetDevice) GetAuxType() string { ret := _m.Called() diff --git a/pkg/types/mocks/HostDevice.go b/pkg/types/mocks/HostDevice.go index 48ea1168a..cbbe6fc77 100644 --- a/pkg/types/mocks/HostDevice.go +++ b/pkg/types/mocks/HostDevice.go @@ -30,6 +30,20 @@ func (_m *HostDevice) GetAPIDevice() *v1beta1.Device { return r0 } +// GetAcpiIndex provides a mock function with given fields: +func (_m *HostDevice) GetAcpiIndex() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + // GetDeviceCode provides a mock function with given fields: func (_m *HostDevice) GetDeviceCode() string { ret := _m.Called() diff --git a/pkg/types/mocks/NetDevice.go b/pkg/types/mocks/NetDevice.go index e538a5995..5bdd48026 100644 --- a/pkg/types/mocks/NetDevice.go +++ b/pkg/types/mocks/NetDevice.go @@ -30,6 +30,20 @@ func (_m *NetDevice) GetAPIDevice() *v1beta1.Device { return r0 } +// GetAcpiIndex provides a mock function with given fields: +func (_m *NetDevice) GetAcpiIndex() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + // GetDeviceCode provides a mock function with given fields: func (_m *NetDevice) GetDeviceCode() string { ret := _m.Called() diff --git a/pkg/types/mocks/PciDevice.go b/pkg/types/mocks/PciDevice.go index 59507fd1f..9b6546c96 100644 --- a/pkg/types/mocks/PciDevice.go +++ b/pkg/types/mocks/PciDevice.go @@ -30,6 +30,20 @@ func (_m *PciDevice) GetAPIDevice() *v1beta1.Device { return r0 } +// GetAcpiIndex provides a mock function with given fields: +func (_m *PciDevice) GetAcpiIndex() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + // GetDeviceCode provides a mock function with given fields: func (_m *PciDevice) GetDeviceCode() string { ret := _m.Called() diff --git a/pkg/types/mocks/PciNetDevice.go b/pkg/types/mocks/PciNetDevice.go index 89485dd13..e17fe7267 100644 --- a/pkg/types/mocks/PciNetDevice.go +++ b/pkg/types/mocks/PciNetDevice.go @@ -30,6 +30,20 @@ func (_m *PciNetDevice) GetAPIDevice() *v1beta1.Device { return r0 } +// GetAcpiIndex provides a mock function with given fields: +func (_m *PciNetDevice) GetAcpiIndex() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + // GetDDPProfiles provides a mock function with given fields: func (_m *PciNetDevice) GetDDPProfiles() string { ret := _m.Called() diff --git a/pkg/types/mocks/ResourceFactory.go b/pkg/types/mocks/ResourceFactory.go index 564c03afc..adae081d9 100644 --- a/pkg/types/mocks/ResourceFactory.go +++ b/pkg/types/mocks/ResourceFactory.go @@ -13,6 +13,22 @@ type ResourceFactory struct { mock.Mock } +// FilterBySelector provides a mock function with given fields: _a0, _a1, _a2 +func (_m *ResourceFactory) FilterBySelector(_a0 string, _a1 []string, _a2 []types.HostDevice) []types.HostDevice { + ret := _m.Called(_a0, _a1, _a2) + + var r0 []types.HostDevice + if rf, ok := ret.Get(0).(func(string, []string, []types.HostDevice) []types.HostDevice); ok { + r0 = rf(_a0, _a1, _a2) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]types.HostDevice) + } + } + + return r0 +} + // GetDefaultInfoProvider provides a mock function with given fields: _a0, _a1 func (_m *ResourceFactory) GetDefaultInfoProvider(_a0 string, _a1 string) []types.DeviceInfoProvider { ret := _m.Called(_a0, _a1) diff --git a/pkg/types/types.go b/pkg/types/types.go index 56e80ee71..757368f8e 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -125,6 +125,7 @@ type GenericNetDeviceSelectors struct { RootDevices []string `json:"rootDevices,omitempty"` LinkTypes []string `json:"linkTypes,omitempty"` IsRdma bool // the resource support rdma + AcpiIndexes []string `json:"acpiIndexes,omitempty"` } // NetDeviceSelectors contains network device related selectors fields @@ -179,6 +180,7 @@ type ResourceFactory interface { GetDeviceProvider(DeviceType) DeviceProvider GetDeviceFilter(*ResourceConfig) ([]interface{}, error) GetNadUtils() NadUtils + FilterBySelector(string, []string, []HostDevice) []HostDevice } // ResourcePool represents a generic resource entity @@ -246,6 +248,8 @@ type PciDevice interface { HostDevice // GetPciAddr returns PCI address of the device GetPciAddr() string + // GetAcpiIndex returns ACPI index of the device + GetAcpiIndex() string } // NetDevice provides an interface to get generic network device information diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 9ee66261e..8ee7f78ab 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -382,6 +382,24 @@ func GetDriverName(pciAddr string) (string, error) { return filepath.Base(driverInfo), nil } +// GetAcpiIndex returns the ACPI index attached to a pci device from its pci address +func GetAcpiIndex(pciAddr string) (string, error) { + acpiIndexLink := filepath.Join(sysBusPci, pciAddr, "acpi_index") + _, err := os.Stat(acpiIndexLink) + if err != nil { + if os.IsNotExist(err) { + return "", nil + } + return "", fmt.Errorf("error getting ACPI index for device %s %v", pciAddr, err) + } + + acpiIndex, err := os.ReadFile(acpiIndexLink) + if err != nil { + return "", fmt.Errorf("error getting ACPI index for device %s %v", pciAddr, err) + } + return string(bytes.TrimSpace(acpiIndex)), nil +} + // GetVFID returns VF ID index (within specific PF) based on PCI address func GetVFID(pciAddr string) (vfID int, err error) { pfDir := filepath.Join(sysBusPci, pciAddr, "physfn")