SRIOV support for non-network device (FEC) #196

damiankopyto · 2020-01-30T13:58:23Z

SRIOV feature is not just applicable for Network interfaces but any IO device that support PCIe SIG SRIOV features. One such device is based on Intel® PAC N3000 FPGA which has Network Functionality of 2x25G and FPGA part that can be programmed to support hardware offload of the Wireless (4G/5G) Forward Error Correction (FEC). This Product has already been released by Intel and is being used/deployed by customers. Intel FPGA Device plugin (https://github.com/intel/intel-device-plugins-for-kubernetes/blob/master/cmd/fpga_plugin/README.md) does not support such devices as it dose not cover SRIOV devices using DPDK and also differ in the FPGA image that gets programmed. As part of this PR we are recommending to extend the SRIOV support for non-networking devices and also rename the device plugin from “sriov-network-device-plugin” to “sriov-device-plugin”. For the Intel® PAC N3000 FPGA supporting vRAN or Cloud-native RAN the FPGA PF that supports FEC is bound to DPDK igb_uio or vfio_pci. The Virtual Function resources are allocated to the PODs running RAN/FlexRAN like workload and uses vfio_pci.

…O device that support PCIe SIG SRIOV features. One such device is based on Intel® PAC N3000 FPGA which has Network Functionality of 2x25G and FPGA part that can be programmed to support hardware offload of the Wireless (4G/5G) Forward Error Correction (FEC). This Product has already been released by Intel and is being used/deployed by customers. Intel FPGA Device plugin (https://github.com/intel/intel-device-plugins-for-kubernetes/blob/master/cmd/fpga_plugin/README.md) does not support such devices as it dose not cover SRIOV devices using DPDK and also differ in the FPGA image that gets programmed. As part of this PR we are recommending to extend the SRIOV support for non-networking devices and also rename the device plugin from “sriov-network-device-plugin” to “sriov-device-plugin”. For the Intel® PAC N3000 FPGA supporting vRAN or Cloud-native RAN the FPGA PF that supports FEC is bound to DPDK igb_uio or vfio_pci. The Virtual Function resources are allocated to the PODs running RAN/FlexRAN like workload and uses vfio_pci.

Levovar · 2020-01-30T14:12:26Z

+1 for the functionality :)

Levovar · 2020-01-30T14:13:48Z

cmd/sriovdp/manager.go

-			// exclude device in-use in host
-			if isDefaultRoute, _ := hasDefaultRoute(device.Address); !isDefaultRoute {
+			// Network devices
+			if devClass == netClass {


consider refactoring the two branches into their own handler functions for increased readability

Levovar · 2020-01-30T14:15:14Z

cmd/sriovdp/manager.go

-				if newDevice, err := resources.NewPciNetDevice(device, rm.rFactory); err == nil {
-					rm.netDeviceList = append(rm.netDeviceList, newDevice)
+				//If dev ID = 0b30 it means it is a RSU device and not FEC
+				if device.Product.ID != "0b30" {


use a well named constant instead of hard-coding a hex in the middle of the code

BTW is this code manufacturer -i.e. Intel- specific?

According to here it's the PCI device ID for the N3000 FPGA device.

It seems a little messy to need to be worrying about PCI device IDs in the middle of this code. Is there nothing else we can look at to determine the class of a device?

nicolas-chautru · 2020-01-30T16:44:47Z

cmd/sriovdp/manager.go

@@ -32,6 +32,7 @@ import (
 const (
 	socketSuffix = "sock"
 	netClass     = 0x02 // Device class - Network controller.	 ref: https://pci-ids.ucw.cz/read/PD/02 (for Sub-Classes)
+	fecClass     = 0x12 // Device class - Processing accelerator


Please rename FecClass to a more generic name as this can appy vof devices exposing more generic baseband processing accelerator. "accClass" should be enough to distinguish from "netClass"

nicolas-chautru

Very valuable patch to make this plugin seamlessly more generic.
=> Fairly cosmetic comment below to not limit the change to "FEC" devices per se but more generally to lookaside processing accelerators. FEC is only one specific example of non-network devices seeing traction in the immediate short term.

Fixing hardcoded RSU device ID. Placing net dev and acc dev handling into two distinct functions.

zshi-redhat · 2020-02-01T06:56:13Z

Hi, @damiankopyto thanks for adding new device support!

I have some basic questions regarding enabling this new type of device.

Is this a network only accelerator or it can also be used for non-network purposes? From the N3000 data sheet, it looks like so, but I'd like to confirm.
Does it require PF be bond to userspace driver in order to perform management and control actions on the device? Is there or will there be any kernel interface developed for device management?
Will there be any need to load FPGA image per VF device? How do you handle FPGA image loading?

cbf123 · 2020-02-04T21:56:17Z

cmd/sriovdp/manager.go

@@ -46,6 +48,7 @@ Network controller subclasses. ref: https://pci-ids.ucw.cz/read/PD/02
 		07	Infiniband controller
 		08	Fabric controller
 		80	Network controller
+		12	Processing Accelerator


I think this is wrong. "12" is correct for the PCI device class for processing accelerators as specified at https://pci-ids.ucw.cz/read/PD/ and as I read it, the processing accelerator subclass should be 00 for this device.

However, "12" does not appear to be a valid option for a network controller subclass as described at the URL listed on line 40.

cbf123 · 2020-02-04T22:03:05Z

cmd/sriovdp/manager.go


+			// Processing accelerators
+			if devClass == accClass {


It seems a little strange to have two totally separate if statements where only one of them can be true...normally I'd expect to see an "else if" here or even a switch statement.

cbf123 · 2020-02-04T22:19:27Z

cmd/sriovdp/manager.go

+		glog.Warningf("discoverDevices(): unable to parse device ID for device %+v %q", dev, err)
+		return
+	}
+	//Check if device ID = 0b30, it means it is a RSU (Remote System Update) device and not accelerator


It seems messy to need to be worrying about vendor-specific PCI device IDs in the middle of this code. Is there nothing else we can look at to determine the class of a device, or to make this more generic so that adding other accelerators would be simpler?

cbf123 · 2020-02-04T22:20:12Z

cmd/sriovdp/manager.go

-	netClass     = 0x02 // Device class - Network controller.	 ref: https://pci-ids.ucw.cz/read/PD/02 (for Sub-Classes)
+	netClass     = 0x02   // Device class - Network controller.	 ref: https://pci-ids.ucw.cz/read/PD/02 (for Sub-Classes)
+	accClass     = 0x12   // Device class - Processing accelerator
+	rsuDevice    = 0x0b30 // RSU device - only used for programming the FPGA


What does "RSU" mean in this context, and do we need to document that somewhere? (Also at line 334.)

cbf123 · 2020-02-04T22:22:26Z

cmd/sriovdp/manager.go

+			if newDevice.GetDriver() == "igb_uio" || newDevice.GetDriver() == "vfio-pci" {
+				rm.netDeviceList = append(rm.netDeviceList, newDevice)
+			} else {
+				glog.Infof("Excluding FEC device %-12s because it is not bound to userspace driver.", dev.Address)


I thought we were dropping the "FEC" terminology?

cbf123 · 2020-02-04T22:23:39Z

cmd/sriovdp/manager.go

+		}
+
+		if newDevice, err := resources.NewPciNetDevice(dev, rm.rFactory); err == nil {
+			if newDevice.GetDriver() == "igb_uio" || newDevice.GetDriver() == "vfio-pci" {


This looks like more device-specific logic. How hard will it be to support other accelerators in the future?

steven-webster · 2020-02-04T23:43:12Z

cmd/sriovdp/manager.go

+		} else {
+			glog.Errorf("discoverDevices() error adding new device: %q", err)
+		}
+	} else {


It might be better to check if devID == rsuDevice { log and return } at line 318. I'm thinking of the (future) case of any other accelerator classed PCI devices that would not be appropriate to discover here. It might be useful to have a list of 'blacklisted' device Ids per class that can be looped through rather than individual checks.

ahalimx86 · 2020-02-05T10:07:18Z

We are currently assessing the impact of supporting this devices with this device plugin. What I see from this changes, this was quick and easy 'fix' to add support for this specific fpga device by adding them into the same global 'pciNetDevice' list.
First of all, these devices should be not added in the pciNetDevice, in fact they should be added into their own type of pciDevice which can then have their own set of 'selector's instead the 'selector's targeted for net devices. As some of the netDevice selectors won't be meaningful for other devices(e.g. "pfNames").
If we do agree to add support for fpga devices with this plugin, then there are other ways to do it instead of making changes in pciNetDevice specific code.

zshi-redhat · 2020-02-19T04:24:53Z

Found this link which answers some of my questions above. Paste it here in case anyone is interestedl in understanding detailed configuration needed for N3000 device, https://github.com/open-ness/specs/blob/master/doc/enhanced-platform-awareness/openness-fpga.md

kad · 2020-03-10T15:38:02Z

Intel FPGA Device plugin (https://github.com/intel/intel-device-plugins-for-kubernetes/blob/master/cmd/fpga_plugin/README.md) does not support such devices as it dose not cover SRIOV devices using DPDK and also differ in the FPGA image that gets programmed.

Please fix that sentence, it is incorrect. FPGA device plugin supports FPGA IP block of N3000 card in pre-programmed mode. Programming FPGA IP block is missing in the intel-fpga kernel driver for N3000, and as soon it will be added, FPGA plugin will be able to handle it in orchestration programmed mode as well.

ahalimx86 · 2020-04-20T16:43:37Z

Closing this PR since this feature have been implemented by #214

Levovar reviewed Jan 30, 2020

View reviewed changes

nicolas-chautru reviewed Jan 30, 2020

View reviewed changes

nicolas-chautru suggested changes Jan 30, 2020

View reviewed changes

Renaming FEC to ACC (accelerator) to make name more generic.

61c4050

Fixing hardcoded RSU device ID. Placing net dev and acc dev handling into two distinct functions.

cbf123 reviewed Feb 4, 2020

View reviewed changes

steven-webster reviewed Feb 4, 2020

View reviewed changes

ahalimx86 mentioned this pull request Feb 14, 2020

Add support for other SR-IOV devices #208

Closed

ahalimx86 closed this Apr 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SRIOV support for non-network device (FEC) #196

SRIOV support for non-network device (FEC) #196

damiankopyto commented Jan 30, 2020

Levovar commented Jan 30, 2020

Levovar Jan 30, 2020

Levovar Jan 30, 2020

cbf123 Feb 4, 2020 •

edited

nicolas-chautru Jan 30, 2020

nicolas-chautru left a comment

zshi-redhat commented Feb 1, 2020

cbf123 Feb 4, 2020 •

edited

cbf123 Feb 4, 2020 •

edited

cbf123 Feb 4, 2020

cbf123 Feb 4, 2020 •

edited

cbf123 Feb 4, 2020

cbf123 Feb 4, 2020 •

edited

steven-webster Feb 4, 2020

ahalimx86 commented Feb 5, 2020 •

edited

zshi-redhat commented Feb 19, 2020

kad commented Mar 10, 2020

ahalimx86 commented Apr 20, 2020

SRIOV support for non-network device (FEC) #196

SRIOV support for non-network device (FEC) #196

Conversation

damiankopyto commented Jan 30, 2020

Levovar commented Jan 30, 2020

Levovar Jan 30, 2020

Choose a reason for hiding this comment

Levovar Jan 30, 2020

Choose a reason for hiding this comment

cbf123 Feb 4, 2020 • edited

Choose a reason for hiding this comment

nicolas-chautru Jan 30, 2020

Choose a reason for hiding this comment

nicolas-chautru left a comment

Choose a reason for hiding this comment

zshi-redhat commented Feb 1, 2020

cbf123 Feb 4, 2020 • edited

Choose a reason for hiding this comment

cbf123 Feb 4, 2020 • edited

Choose a reason for hiding this comment

cbf123 Feb 4, 2020

Choose a reason for hiding this comment

cbf123 Feb 4, 2020 • edited

Choose a reason for hiding this comment

cbf123 Feb 4, 2020

Choose a reason for hiding this comment

cbf123 Feb 4, 2020 • edited

Choose a reason for hiding this comment

steven-webster Feb 4, 2020

Choose a reason for hiding this comment

ahalimx86 commented Feb 5, 2020 • edited

zshi-redhat commented Feb 19, 2020

kad commented Mar 10, 2020

ahalimx86 commented Apr 20, 2020

cbf123 Feb 4, 2020 •

edited

cbf123 Feb 4, 2020 •

edited

cbf123 Feb 4, 2020 •

edited

cbf123 Feb 4, 2020 •

edited

cbf123 Feb 4, 2020 •

edited

ahalimx86 commented Feb 5, 2020 •

edited