usertools: fix bind failed can not regression

Message ID HK0PR06MB27406E9CB6E7C190C2DDE324D0450@HK0PR06MB2740.apcprd06.prod.outlook.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series usertools: fix bind failed can not regression |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/iol-compilation success Compile Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Hui Wei Nov. 26, 2019, 6:49 a.m. UTC
  nic bind to mlx5_core, enable sriov, nic has one pf and 8 vfs,
bind pf to vfio-pci failed, can not regress to mlx5_core

Signed-off-by: Wei Hui <huiweics@hotmail.com>
---
 usertools/dpdk-devbind.py | 165 ++++++++++++++++++++++----------------
 1 file changed, 94 insertions(+), 71 deletions(-)
  

Comments

Thomas Monjalon Nov. 26, 2019, 10:54 p.m. UTC | #1
26/11/2019 07:49, Hui Wei:
> nic bind to mlx5_core, enable sriov, nic has one pf and 8 vfs,
> bind pf to vfio-pci failed, can not regress to mlx5_core

I don't understand what you try to do.
For info, mlx5 does not bind to VFIO,
but uses a bifurcated driver model.
  

Patch

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 7b5cbc12c..59e3960af 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -379,6 +379,7 @@  def bind_one(dev_id, driver, force):
     is already bound to a different driver, it will be unbound first'''
     dev = devices[dev_id]
     saved_driver = None  # used to rollback any unbind in case of failure
+    bind_success = False
 
     # prevent disconnection of our ssh session
     if dev["Ssh_if"] and not force:
@@ -386,16 +387,12 @@  def bind_one(dev_id, driver, force):
               "Not modifying" % dev_id, file=sys.stderr)
         return
 
-    # unbind any existing drivers we don't want
+    # does not need to do anything
     if has_driver(dev_id):
         if dev["Driver_str"] == driver:
             print("Notice: %s already bound to driver %s, skipping" %
                   (dev_id, driver), file=sys.stderr)
             return
-        else:
-            saved_driver = dev["Driver_str"]
-            unbind_one(dev_id, force)
-            dev["Driver_str"] = ""  # clear driver string
 
     # For kernels >= 3.15 driver_override can be used to specify the driver
     # for a device rather than relying on the driver to provide a positive
@@ -403,40 +400,45 @@  def bind_one(dev_id, driver, force):
     # the vendor and device ID, adding them to the driver new_id,
     # will erroneously bind other devices too which has the additional burden
     # of unbinding those devices
-    if driver in dpdk_drivers:
-        filename = "/sys/bus/pci/devices/%s/driver_override" % dev_id
-        if os.path.exists(filename):
-            try:
-                f = open(filename, "w")
-            except:
-                print("Error: bind failed for %s - Cannot open %s"
-                      % (dev_id, filename), file=sys.stderr)
-                return
-            try:
-                f.write("%s" % driver)
-                f.close()
-            except:
-                print("Error: bind failed for %s - Cannot write driver %s to "
-                      "PCI ID " % (dev_id, driver), file=sys.stderr)
-                return
-        # For kernels < 3.15 use new_id to add PCI id's to the driver
-        else:
-            filename = "/sys/bus/pci/drivers/%s/new_id" % driver
-            try:
-                f = open(filename, "w")
-            except:
-                print("Error: bind failed for %s - Cannot open %s"
-                      % (dev_id, filename), file=sys.stderr)
-                return
-            try:
-                # Convert Device and Vendor Id to int to write to new_id
-                f.write("%04x %04x" % (int(dev["Vendor"],16),
-                        int(dev["Device"], 16)))
-                f.close()
-            except:
-                print("Error: bind failed for %s - Cannot write new PCI ID to "
-                      "driver %s" % (dev_id, driver), file=sys.stderr)
-                return
+    filename = "/sys/bus/pci/devices/%s/driver_override" % dev_id
+    if os.path.exists(filename):
+        try:
+            f = open(filename, "w")
+        except:
+            print("Error: bind failed for %s - Cannot open %s"
+                  % (dev_id, filename), file=sys.stderr)
+            return
+        try:
+            f.write("%s" % driver)
+            f.close()
+        except:
+            print("Error: bind failed for %s - Cannot write driver %s to "
+                  "PCI ID " % (dev_id, driver), file=sys.stderr)
+            return
+    # For kernels < 3.15 use new_id to add PCI id's to the driver
+    else:
+        filename = "/sys/bus/pci/drivers/%s/new_id" % driver
+        try:
+            f = open(filename, "w")
+        except:
+            print("Error: bind failed for %s - Cannot open %s"
+                  % (dev_id, filename), file=sys.stderr)
+            return
+        try:
+            # Convert Device and Vendor Id to int to write to new_id
+            f.write("%04x %04x" % (int(dev["Vendor"],16),
+                    int(dev["Device"], 16)))
+            f.close()
+        except:
+            print("Error: bind failed for %s - Cannot write new PCI ID to "
+                  "driver %s" % (dev_id, driver), file=sys.stderr)
+            return
+
+    # unbind any existing drivers we don't want
+    if has_driver(dev_id):
+        saved_driver = dev["Driver_str"]
+        unbind_one(dev_id, force)
+        del dev["Driver_str"]  # clear driver string
 
     # do the bind by writing to /sys
     filename = "/sys/bus/pci/drivers/%s/bind" % driver
@@ -445,42 +447,63 @@  def bind_one(dev_id, driver, force):
     except:
         print("Error: bind failed for %s - Cannot open %s"
               % (dev_id, filename), file=sys.stderr)
-        if saved_driver is not None:  # restore any previous driver
-            bind_one(dev_id, saved_driver, force)
-        return
-    try:
-        f.write(dev_id)
-        f.close()
-    except:
-        # for some reason, closing dev_id after adding a new PCI ID to new_id
-        # results in IOError. however, if the device was successfully bound,
-        # we don't care for any errors and can safely ignore IOError
-        tmp = get_pci_device_details(dev_id, True)
-        if "Driver_str" in tmp and tmp["Driver_str"] == driver:
-            return
-        print("Error: bind failed for %s - Cannot bind to driver %s"
-              % (dev_id, driver), file=sys.stderr)
-        if saved_driver is not None:  # restore any previous driver
-            bind_one(dev_id, saved_driver, force)
-        return
-
-    # For kernels > 3.15 driver_override is used to bind a device to a driver.
-    # Before unbinding it, overwrite driver_override with empty string so that
-    # the device can be bound to any other driver
-    filename = "/sys/bus/pci/devices/%s/driver_override" % dev_id
-    if os.path.exists(filename):
-        try:
-            f = open(filename, "w")
-        except:
-            sys.exit("Error: unbind failed for %s - Cannot open %s"
-                  % (dev_id, filename))
+    else:
         try:
-            f.write("\00")
+            f.write(dev_id)
             f.close()
         except:
-            sys.exit("Error: unbind failed for %s - Cannot open %s"
-                  % (dev_id, filename))
+            # for some reason, closing dev_id after adding a new PCI ID to new_id
+            # results in IOError. however, if the device was successfully bound,
+            # we don't care for any errors and can safely ignore IOError
+            tmp = get_pci_device_details(dev_id, True)
+            if "Driver_str" in tmp and tmp["Driver_str"] == driver:
+                return
+            print("Error: bind failed for %s - Cannot bind to driver %s"
+                  % (dev_id, driver), file=sys.stderr)
+        else:
+            bind_success = True
 
+    if False == bind_success:
+        if saved_driver is not None:  # restore any previous driver
+            bind_one(dev_id, saved_driver, force)
+            dev["Driver_str"] = saved_driver
+        else:
+            # At this point, pci device previous does not bind to any driver, or it will
+            # be restored it's previous driver.
+            # For kernels > 3.15 driver_override is used to bind a device to a driver.
+            # Before unbinding it, overwrite driver_override with empty string so that
+            # the device can be bound to any other driver
+            filename = "/sys/bus/pci/devices/%s/driver_override" % dev_id
+            if os.path.exists(filename):
+                try:
+                    f = open(filename, "w")
+                except:
+                    sys.exit("Error: restore failed for %s - Cannot open %s"
+                          % (dev_id, filename))
+                try:
+                    f.write("\00")
+                    f.close()
+                except:
+                    sys.exit("Error: restore failed for %s - Cannot write %s"
+                          % (dev_id, filename))
+            # For kernels < 3.15 use remove_id to delete PCI id's from the driver
+            else:
+                filename = "/sys/bus/pci/drivers/%s/remove_id" % driver
+                try:
+                    f = open(filename, "w")
+                except:
+                    print("Error: restore failed for %s - Cannot open %s"
+                          % (dev_id, filename), file=sys.stderr)
+                    return
+                try:
+                    # Convert Device and Vendor Id to int to write to remove_id
+                    f.write("%04x %04x" % (int(dev["Vendor"],16),
+                            int(dev["Device"], 16)))
+                    f.close()
+                except:
+                    print("Error: restore failed for %s - Cannot write new PCI ID to "
+                          "driver %s remove_id" % (dev_id, driver), file=sys.stderr)
+                    return
 
 def unbind_all(dev_list, force=False):
     """Unbind method, takes a list of device locations"""