[lxc-devel] [lxd/master] NIC: Updates physical and sriov NICs to use generic PCI driver override technique

tomponline on Github lxc-bot at linuxcontainers.org
Thu Jun 11 15:09:10 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 415 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200611/0f23f6d7/attachment.bin>
-------------- next part --------------
From 518cf3cd3a6b1bb096b79c13e4566e1150c17018 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 11 Jun 2020 16:04:47 +0100
Subject: [PATCH 1/4] lxd/device/device/utils/generic: Adds PCI management
 functions for overriding driver

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/device_utils_generic.go | 158 +++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/lxd/device/device_utils_generic.go b/lxd/device/device_utils_generic.go
index 3b1c051f6a..4efad795f8 100644
--- a/lxd/device/device_utils_generic.go
+++ b/lxd/device/device_utils_generic.go
@@ -1,10 +1,168 @@
 package device
 
 import (
+	"bufio"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"strings"
+	"time"
+
+	"github.com/pkg/errors"
+
+	"github.com/lxc/lxd/lxd/revert"
+	"github.com/lxc/lxd/shared"
 )
 
 // deviceJoinPath joins together prefix and text delimited by a "." for device path generation.
 func deviceJoinPath(parts ...string) string {
 	return strings.Join(parts, ".")
 }
+
+// pciDevice represents info about a PCI uevent device.
+type pciDevice struct {
+	ID       string
+	SlotName string
+	Driver   string
+}
+
+// pciParseUeventFile returns the PCI device info for a given uevent file.
+func pciParseUeventFile(ueventFilePath string) (pciDevice, error) {
+	dev := pciDevice{}
+
+	file, err := os.Open(ueventFilePath)
+	if err != nil {
+		return dev, err
+	}
+	defer file.Close()
+
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		// Looking for something like this "PCI_SLOT_NAME=0000:05:10.0"
+		fields := strings.SplitN(scanner.Text(), "=", 2)
+		if len(fields) == 2 {
+			if fields[0] == "PCI_SLOT_NAME" {
+				dev.SlotName = fields[1]
+			} else if fields[0] == "PCI_ID" {
+				dev.ID = fields[1]
+			} else if fields[0] == "DRIVER" {
+				dev.Driver = fields[1]
+			}
+		}
+	}
+
+	err = scanner.Err()
+	if err != nil {
+		return dev, err
+	}
+
+	if dev.SlotName == "" {
+		return dev, fmt.Errorf("Device uevent file could not be parsed")
+	}
+
+	return dev, nil
+}
+
+// pciDeviceUnbind unbinds a PCI device from the OS using its PCI Slot Name.
+func pciDeviceUnbind(pciDev pciDevice) error {
+	driverUnbindPath := fmt.Sprintf("/sys/bus/pci/devices/%s/driver/unbind", pciDev.SlotName)
+	err := ioutil.WriteFile(driverUnbindPath, []byte(pciDev.SlotName), 0600)
+	if err != nil {
+		return errors.Wrapf(err, "Failed unbinding device %q via %q", pciDev.SlotName, driverUnbindPath)
+	}
+
+	return nil
+}
+
+// pciDeviceSetDriverOverride registers an override driver for a PCI device using its PCI Slot Name.
+func pciDeviceSetDriverOverride(pciDev pciDevice, driverOverride string) error {
+	overridePath := filepath.Join("/sys/bus/pci/devices", pciDev.SlotName, "driver_override")
+
+	// The "\n" at end is important to allow the driver override to be cleared by passing "" in.
+	err := ioutil.WriteFile(overridePath, []byte(fmt.Sprintf("%s\n", driverOverride)), 0600)
+	if err != nil {
+		return errors.Wrapf(err, "Failed setting driver override %q for device %q via %q", driverOverride, pciDev.SlotName, overridePath)
+	}
+
+	return nil
+}
+
+// pciDeviceProbe probes a PCI device using its PCI Slot Name.
+func pciDeviceProbe(pciDev pciDevice) error {
+	driveProbePath := "/sys/bus/pci/drivers_probe"
+	err := ioutil.WriteFile(driveProbePath, []byte(pciDev.SlotName), 0600)
+	if err != nil {
+		return errors.Wrapf(err, "Failed probing device %q via %q", pciDev.SlotName, driveProbePath)
+	}
+
+	return nil
+}
+
+// pciDeviceProbeWait waits for PCI device to be activated with the specified driver after being probed.
+// If the specified driver is empty, wait until the device is activated on any driver.
+func pciDeviceProbeWait(pciDev pciDevice) error {
+	var driverPath string
+
+	if pciDev.Driver == "" {
+		driverPath = fmt.Sprintf("/sys/bus/pci/devices/%s/driver", pciDev.SlotName)
+	} else {
+		driverPath = fmt.Sprintf("/sys/bus/pci/drivers/%s/%s", pciDev.Driver, pciDev.SlotName)
+	}
+
+	for i := 0; i < 10; i++ {
+		if shared.PathExists(driverPath) {
+			return nil
+		}
+
+		time.Sleep(50 * time.Millisecond)
+	}
+
+	return fmt.Errorf("Device took too long activate at %q", driverPath)
+}
+
+// pciDeviceDriverOverride unbinds the device, sets the driver override preference, then probes the device, and
+// waits for it to be activated with the specified driver.
+func pciDeviceDriverOverride(pciDev pciDevice, driverOverride string) error {
+	revert := revert.New()
+	defer revert.Fail()
+
+	// Unbind the device from the host (ignore if not bound).
+	err := pciDeviceUnbind(pciDev)
+	if err != nil && os.IsNotExist(err) {
+		return err
+	}
+
+	revert.Add(func() {
+		// Reset the driver override and rebind to original driver (if needed).
+		pciDeviceUnbind(pciDev)
+		pciDeviceSetDriverOverride(pciDev, pciDev.Driver)
+		pciDeviceProbe(pciDev)
+	})
+
+	// Set driver override.
+	err = pciDeviceSetDriverOverride(pciDev, driverOverride)
+	if err != nil {
+		return err
+	}
+
+	// Probe device to bind it to overridden driver.
+	err = pciDeviceProbe(pciDev)
+	if err != nil {
+		return err
+	}
+
+	vfioDev := pciDevice{
+		Driver:   driverOverride,
+		SlotName: pciDev.SlotName,
+	}
+
+	// Wait for the device to be bound to the overridden driver.
+	err = pciDeviceProbeWait(vfioDev)
+	if err != nil {
+		return err
+	}
+
+	revert.Success()
+	return nil
+}

From 3341fb1a24a00f596ce6221a5a6a6505e3dc78f1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 11 Jun 2020 16:05:24 +0100
Subject: [PATCH 2/4] lxd/device/device/utils/network: Removes network specific
 PCI bind/unbind functions

Replaced by generic PCI driver override functions.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/device_utils_network.go | 101 -----------------------------
 1 file changed, 101 deletions(-)

diff --git a/lxd/device/device_utils_network.go b/lxd/device/device_utils_network.go
index 6a15a345d8..4b7df7ece6 100644
--- a/lxd/device/device_utils_network.go
+++ b/lxd/device/device_utils_network.go
@@ -1,13 +1,11 @@
 package device
 
 import (
-	"bufio"
 	"crypto/rand"
 	"encoding/hex"
 	"fmt"
 	"io/ioutil"
 	"net"
-	"os"
 	"regexp"
 	"strconv"
 	"strings"
@@ -745,87 +743,6 @@ func networkParsePortRange(r string) (int64, int64, error) {
 	return base, size, nil
 }
 
-// pciDevice represents info about a PCI uevent device.
-type pciDevice struct {
-	ID       string
-	SlotName string
-	Driver   string
-}
-
-// networkGetDevicePCISlot returns the PCI device info for a given uevent file.
-func networkGetDevicePCIDevice(ueventFilePath string) (pciDevice, error) {
-	dev := pciDevice{}
-
-	file, err := os.Open(ueventFilePath)
-	if err != nil {
-		return dev, err
-	}
-	defer file.Close()
-
-	scanner := bufio.NewScanner(file)
-	for scanner.Scan() {
-		// Looking for something like this "PCI_SLOT_NAME=0000:05:10.0"
-		fields := strings.SplitN(scanner.Text(), "=", 2)
-		if len(fields) == 2 {
-			if fields[0] == "PCI_SLOT_NAME" {
-				dev.SlotName = fields[1]
-			} else if fields[0] == "PCI_ID" {
-				dev.ID = fields[1]
-			} else if fields[0] == "DRIVER" {
-				dev.Driver = fields[1]
-			}
-		}
-	}
-
-	err = scanner.Err()
-	if err != nil {
-		return dev, err
-	}
-
-	if dev.SlotName == "" {
-		return dev, fmt.Errorf("Device uevent file could not be parsed")
-	}
-
-	return dev, nil
-}
-
-// networkDeviceUnbind unbinds a network device from the OS using its PCI Slot Name and driver name.
-func networkDeviceUnbind(pciDev pciDevice) error {
-	driverUnbindPath := fmt.Sprintf("/sys/bus/pci/drivers/%s/unbind", pciDev.Driver)
-	err := ioutil.WriteFile(driverUnbindPath, []byte(pciDev.SlotName), 0600)
-	if err != nil {
-		return errors.Wrapf(err, "Failed unbinding device %q via %q", pciDev.SlotName, driverUnbindPath)
-	}
-
-	return nil
-}
-
-// networkDeviceBind binds a network device to the OS using its PCI Slot Name and driver name.
-func networkDeviceBind(pciDev pciDevice) error {
-	driverBindPath := fmt.Sprintf("/sys/bus/pci/drivers/%s/bind", pciDev.Driver)
-	err := ioutil.WriteFile(driverBindPath, []byte(pciDev.SlotName), 0600)
-	if err != nil {
-		return errors.Wrapf(err, "Failed binding device %q via %q", pciDev.SlotName, driverBindPath)
-	}
-
-	return nil
-}
-
-// networkDeviceBindWait waits for network device to appear after being binded to a driver.
-func networkDeviceBindWait(pciDev pciDevice) error {
-	devicePath := fmt.Sprintf("/sys/bus/pci/drivers/%s/%s", pciDev.Driver, pciDev.SlotName)
-
-	for i := 0; i < 10; i++ {
-		if shared.PathExists(devicePath) {
-			return nil
-		}
-
-		time.Sleep(50 * time.Millisecond)
-	}
-
-	return fmt.Errorf("Bind of device %q took too long", devicePath)
-}
-
 // networkInterfaceBindWait waits for network interface to appear after being binded to a driver.
 func networkInterfaceBindWait(ifName string) error {
 	for i := 0; i < 10; i++ {
@@ -838,21 +755,3 @@ func networkInterfaceBindWait(ifName string) error {
 
 	return fmt.Errorf("Bind of interface %q took too long", ifName)
 }
-
-// networkVFIOPCIRegister registers the PCI device with the VFIO-PCI driver.
-// Should also bind the device to the vfio-pci driver if it is present. Requires the vfio-pci module is loaded.
-func networkVFIOPCIRegister(pciDev pciDevice) error {
-	// vfio-pci module takes device IDs as "n n" but networkGetDevicePCIDevice returns them as "n:n".
-	devIDParts := strings.SplitN(pciDev.ID, ":", 2)
-	if len(devIDParts) < 2 {
-		return fmt.Errorf("Invalid device ID from %q", pciDev.ID)
-	}
-
-	vfioPCINewIDPath := "/sys/bus/pci/drivers/vfio-pci/new_id"
-	err := ioutil.WriteFile(vfioPCINewIDPath, []byte(fmt.Sprintf("%s %s", devIDParts[0], devIDParts[1])), 0600)
-	if err != nil {
-		return errors.Wrapf(err, "Failed registering PCI device ID %q to %q", pciDev.ID, vfioPCINewIDPath)
-	}
-
-	return nil
-}

From f116762f7f0397fdaa96989d0dcaf59e866121e7 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 11 Jun 2020 16:06:00 +0100
Subject: [PATCH 3/4] lxd/device/nic/physical: Updates to use generic PCI
 management functions

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_physical.go | 37 +++----------------------------------
 1 file changed, 3 insertions(+), 34 deletions(-)

diff --git a/lxd/device/nic_physical.go b/lxd/device/nic_physical.go
index 1205baf9e8..f869ed687b 100644
--- a/lxd/device/nic_physical.go
+++ b/lxd/device/nic_physical.go
@@ -129,7 +129,7 @@ func (d *nicPhysical) Start() (*deviceConfig.RunConfig, error) {
 	} else if d.inst.Type() == instancetype.VM {
 		// Get PCI information about the network interface.
 		ueventPath := fmt.Sprintf("/sys/class/net/%s/device/uevent", saveData["host_name"])
-		pciDev, err := networkGetDevicePCIDevice(ueventPath)
+		pciDev, err := pciParseUeventFile(ueventPath)
 		if err != nil {
 			return nil, errors.Wrapf(err, "Failed to get PCI device info for %q", saveData["host_name"])
 		}
@@ -137,28 +137,7 @@ func (d *nicPhysical) Start() (*deviceConfig.RunConfig, error) {
 		saveData["last_state.pci.slot.name"] = pciDev.SlotName
 		saveData["last_state.pci.driver"] = pciDev.Driver
 
-		// Unbind the interface from the host.
-		err = networkDeviceUnbind(pciDev)
-		if err != nil {
-			return nil, err
-		}
-
-		revert.Add(func() { networkDeviceBind(pciDev) })
-
-		// Register the device with the vfio-pci module.
-		err = networkVFIOPCIRegister(pciDev)
-		if err != nil {
-			return nil, err
-		}
-
-		vfioDev := pciDevice{
-			Driver:   "vfio-pci",
-			SlotName: pciDev.SlotName,
-		}
-
-		revert.Add(func() { networkDeviceUnbind(vfioDev) })
-
-		err = networkDeviceBindWait(vfioDev)
+		err = pciDeviceDriverOverride(pciDev, "vfio-pci")
 		if err != nil {
 			return nil, err
 		}
@@ -224,17 +203,7 @@ func (d *nicPhysical) postStop() error {
 			SlotName: v["last_state.pci.slot.name"],
 		}
 
-		err := networkDeviceUnbind(vfioDev)
-		if err != nil {
-			return err
-		}
-
-		hostDev := pciDevice{
-			Driver:   v["last_state.pci.driver"],
-			SlotName: v["last_state.pci.slot.name"],
-		}
-
-		err = networkDeviceBind(hostDev)
+		err := pciDeviceDriverOverride(vfioDev, v["last_state.pci.driver"])
 		if err != nil {
 			return err
 		}

From e32ef714792dcd2290e150229d3be0bd6607697c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 11 Jun 2020 16:06:23 +0100
Subject: [PATCH 4/4] lxd/device/nic/sriov: Updates to use generic PCI
 management functions

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_sriov.go | 56 +++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 36 deletions(-)

diff --git a/lxd/device/nic_sriov.go b/lxd/device/nic_sriov.go
index 8cba0fd09e..724d432e71 100644
--- a/lxd/device/nic_sriov.go
+++ b/lxd/device/nic_sriov.go
@@ -376,12 +376,12 @@ func (d *nicSRIOV) setupSriovParent(vfDevice string, vfID int, volatile map[stri
 	}
 
 	// Unbind VF device from the host so that the settings will take effect when we rebind it.
-	err = networkDeviceUnbind(vfPCIDev)
+	err = pciDeviceUnbind(vfPCIDev)
 	if err != nil {
 		return vfPCIDev, err
 	}
 
-	revert.Add(func() { networkDeviceBind(vfPCIDev) })
+	revert.Add(func() { pciDeviceProbe(vfPCIDev) })
 
 	// Setup VF VLAN if specified.
 	if d.config["vlan"] != "" {
@@ -442,7 +442,7 @@ func (d *nicSRIOV) setupSriovParent(vfDevice string, vfID int, volatile map[stri
 
 	if d.inst.Type() == instancetype.Container {
 		// Bind VF device onto the host so that the settings will take effect.
-		err = networkDeviceBind(vfPCIDev)
+		err = pciDeviceProbe(vfPCIDev)
 		if err != nil {
 			return vfPCIDev, err
 		}
@@ -457,19 +457,7 @@ func (d *nicSRIOV) setupSriovParent(vfDevice string, vfID int, volatile map[stri
 		}
 	} else if d.inst.Type() == instancetype.VM {
 		// Register VF device with vfio-pci driver so it can be passed to VM.
-		err = networkVFIOPCIRegister(vfPCIDev)
-		if err != nil {
-			return vfPCIDev, err
-		}
-
-		vfioDev := pciDevice{
-			Driver:   "vfio-pci",
-			SlotName: vfPCIDev.SlotName,
-		}
-
-		revert.Add(func() { networkDeviceUnbind(vfioDev) })
-
-		err = networkDeviceBindWait(vfioDev)
+		err = pciDeviceDriverOverride(vfPCIDev, "vfio-pci")
 		if err != nil {
 			return vfPCIDev, err
 		}
@@ -630,7 +618,7 @@ func (d *nicSRIOV) networkGetVirtFuncInfo(devName string, vfID int) (VirtFuncInf
 // networkGetVFDevicePCISlot returns the PCI slot name for a network virtual function device.
 func (d *nicSRIOV) networkGetVFDevicePCISlot(vfID string) (pciDevice, error) {
 	ueventFile := fmt.Sprintf("/sys/class/net/%s/device/virtfn%s/uevent", d.config["parent"], vfID)
-	pciDev, err := networkGetDevicePCIDevice(ueventFile)
+	pciDev, err := pciParseUeventFile(ueventFile)
 	if err != nil {
 		return pciDev, err
 	}
@@ -646,38 +634,33 @@ func (d *nicSRIOV) restoreSriovParent(volatile map[string]string) error {
 		return nil
 	}
 
+	revert := revert.New()
+	defer revert.Fail()
+
 	// Get VF device's PCI info so we can unbind and rebind it from the host.
 	vfPCIDev, err := d.networkGetVFDevicePCISlot(volatile["last_state.vf.id"])
 	if err != nil {
 		return err
 	}
 
-	if d.inst.Type() == instancetype.Container {
-		// Unbind VF device from the host so that the settings will take effect when we rebind it.
-		err = networkDeviceUnbind(vfPCIDev)
-		if err != nil {
-			return err
-		}
-	} else if d.inst.Type() == instancetype.VM {
-		// Unbind VF device from vfio-pci driver so that we can rebind it on host.
-		vfioDev := pciDevice{
-			Driver:   "vfio-pci",
-			SlotName: vfPCIDev.SlotName,
-		}
+	// Unbind VF device from the host so that the restored settings will take effect when we rebind it.
+	err = pciDeviceUnbind(vfPCIDev)
+	if err != nil {
+		return err
+	}
 
-		err := networkDeviceUnbind(vfioDev)
+	if d.inst.Type() == instancetype.VM {
+		// Before we bind the device back to the host, ensure we restore the original driver info as it
+		// should be currently set to vfio-pci.
+		err = pciDeviceSetDriverOverride(vfPCIDev, volatile["last_state.pci.driver"])
 		if err != nil {
 			return err
 		}
-
-		// Before we bind the device back to the host, ensure we restore the original driver info as it
-		// should be currently set to vfio-pci.
-		vfPCIDev.Driver = volatile["last_state.pci.driver"]
 	}
 
 	// However we return from this function, we must try to rebind the VF so its not orphaned.
 	// The OS won't let an already bound device be bound again so is safe to call twice.
-	defer networkDeviceBind(vfPCIDev)
+	revert.Add(func() { pciDeviceProbe(vfPCIDev) })
 
 	// Reset VF VLAN if specified
 	if volatile["last_state.vf.vlan"] != "" {
@@ -710,7 +693,7 @@ func (d *nicSRIOV) restoreSriovParent(volatile map[string]string) error {
 	}
 
 	// Bind VF device onto the host so that the settings will take effect.
-	err = networkDeviceBind(vfPCIDev)
+	err = pciDeviceProbe(vfPCIDev)
 	if err != nil {
 		return err
 	}
@@ -730,5 +713,6 @@ func (d *nicSRIOV) restoreSriovParent(volatile map[string]string) error {
 		return err
 	}
 
+	revert.Success()
 	return nil
 }


More information about the lxc-devel mailing list