[dpdk-dev] [PATCH v6 18/20] igb_uio: Removed PCI ID table from igb_uio

[Anatoly Burakov <anatoly.burakov@intel.com>]

Removing PCI ID list to make igb_uio more similar to a generic driver
like vfio-pci or pci_uio_generic. This is done to make it easier for
the binding script to support multiple drivers.

Note that since igb_uio no longer has a PCI ID list, it can now be
bound to any device, not just those explicitly supported by DPDK. In
other words, it now behaves similar to PCI stub, VFIO and other generic
PCI drivers.

Therefore to bind a new device to igb_uio, the user will now have to
first write its PCI ID to "new_id" file inside the igb_uio driver
directory, and only then write the PCI ID to "bind". This is reflected
in changes to PCI binding script as well.

There's a weird behaviour of sysfs when a new device ID is added to
new_id. Subsequent writing to "bind" will result in IOError on
closing the file. This error is harmless but it triggers the
exception anyway, so in order to work around that, we check if the
device was actually bound to the driver before raising an error.
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c |  21 +-----
 tools/igb_uio_bind.py                     | 118 +++++++++++++++---------------
 2 files changed, 59 insertions(+), 80 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 8e467a2..60b8ca4 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -65,25 +65,6 @@ struct rte_uio_pci_dev {
 static char *intr_mode = NULL;
 static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 
-/* PCI device id table */
-static struct pci_device_id igbuio_pci_ids[] = {
-#define RTE_PCI_DEV_ID_DECL_EM(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IGB(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IGBVF(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IXGBE(vend, dev) {PCI_DEVICE(vend, dev)},
-#define RTE_PCI_DEV_ID_DECL_IXGBEVF(vend, dev) {PCI_DEVICE(vend, dev)},
-#ifdef RTE_LIBRTE_VIRTIO_PMD
-#define RTE_PCI_DEV_ID_DECL_VIRTIO(vend, dev) {PCI_DEVICE(vend, dev)},
-#endif
-#ifdef RTE_LIBRTE_VMXNET3_PMD
-#define RTE_PCI_DEV_ID_DECL_VMXNET3(vend, dev) {PCI_DEVICE(vend, dev)},
-#endif
-#include <rte_pci_dev_ids.h>
-{ 0, },
-};
-
-MODULE_DEVICE_TABLE(pci, igbuio_pci_ids);
-
 static inline struct rte_uio_pci_dev *
 igbuio_get_uio_pci_dev(struct uio_info *info)
 {
@@ -619,7 +600,7 @@ igbuio_config_intr_mode(char *intr_str)
 
 static struct pci_driver igbuio_pci_driver = {
 	.name = "igb_uio",
-	.id_table = igbuio_pci_ids,
+	.id_table = NULL,
 	.probe = igbuio_pci_probe,
 	.remove = igbuio_pci_remove,
 };
diff --git a/tools/igb_uio_bind.py b/tools/igb_uio_bind.py
index 18dbeda..e87a05e 100755
--- a/tools/igb_uio_bind.py
+++ b/tools/igb_uio_bind.py
@@ -42,8 +42,6 @@ ETHERNET_CLASS = "0200"
 # global dict ethernet devices present. Dictionary indexed by PCI address.
 # Each device within this is itself a dictionary of device properties
 devices = {}
-# list of vendor:device pairs (again stored as dictionary) supported by igb_uio
-module_dev_ids = []
 
 def usage():
     '''Print usage information for the program'''
@@ -147,9 +145,7 @@ def find_module(mod):
                 return path
 
 def check_modules():
-    '''Checks that the needed modules (igb_uio) is loaded, and then
-    determine from the .ko file, what its supported device ids are'''
-    global module_dev_ids
+    '''Checks that igb_uio is loaded'''
 
     fd = file("/proc/modules")
     loaded_mods = fd.readlines()
@@ -166,40 +162,35 @@ def check_modules():
         print "Error - module %s not loaded" %mod
         sys.exit(1)
 
-    # now find the .ko and get list of supported vendor/dev-ids
-    modpath = find_module(mod)
-    if modpath is None:
-        print "Cannot find module file %s" % (mod + ".ko")
-        sys.exit(1)
-    depmod_output = check_output(["depmod", "-n", modpath]).splitlines()
-    for line in depmod_output:
-        if not line.startswith("alias"):
-            continue
-        if not line.endswith(mod):
-            continue
-        lineparts = line.split()
-        if not(lineparts[1].startswith("pci:")):
-            continue;
-        else:
-            lineparts[1] = lineparts[1][4:]
-        vendor = lineparts[1][:9]
-        device = lineparts[1][9:18]
-        if vendor.startswith("v") and device.startswith("d"):
-            module_dev_ids.append({"Vendor": int(vendor[1:],16),
-                                   "Device": int(device[1:],16)})
-
-def is_supported_device(dev_id):
-    '''return true if device is supported by igb_uio, false otherwise'''
-    for dev in module_dev_ids:
-        if (dev["Vendor"] == devices[dev_id]["Vendor"] and
-            dev["Device"] == devices[dev_id]["Device"]):
-            return True
-    return False
-
 def has_driver(dev_id):
     '''return true if a device is assigned to a driver. False otherwise'''
     return "Driver_str" in devices[dev_id]
 
+def get_pci_device_details(dev_id):
+    '''This function gets additional details for a PCI device'''
+    device = {}
+
+    extra_info = check_output(["lspci", "-vmmks", dev_id]).splitlines()
+
+    # parse lspci details
+    for line in extra_info:
+        if len(line) == 0:
+            continue
+        name, value = line.split("\t", 1)
+        name = name.strip(":") + "_str"
+        device[name] = value
+    # check for a unix interface name
+    sys_path = "/sys/bus/pci/devices/%s/net/" % dev_id
+    if exists(sys_path):
+        device["Interface"] = ",".join(os.listdir(sys_path))
+    else:
+        device["Interface"] = ""
+    # check if a port is used for ssh connection
+    device["Ssh_if"] = False
+    device["Active"] = ""
+
+    return device
+
 def get_nic_details():
     '''This function populates the "devices" dictionary. The keys used are
     the pci addresses (domain:bus:slot.func). The values are themselves
@@ -237,23 +228,10 @@ def get_nic_details():
 
     # based on the basic info, get extended text details
     for d in devices.keys():
-        extra_info = check_output(["lspci", "-vmmks", d]).splitlines()
-        # parse lspci details
-        for line in extra_info:
-            if len(line) == 0:
-                continue
-            name, value = line.split("\t", 1)
-            name = name.strip(":") + "_str"
-            devices[d][name] = value
-        # check for a unix interface name
-        sys_path = "/sys/bus/pci/devices/%s/net/" % d
-        if exists(sys_path):
-            devices[d]["Interface"] = ",".join(os.listdir(sys_path))
-        else:
-            devices[d]["Interface"] = ""
-        # check if a port is used for ssh connection
-        devices[d]["Ssh_if"] = False
-        devices[d]["Active"] = ""
+        # get additional info and add it to existing data
+        devices[d] = dict(devices[d].items() +
+                          get_pci_device_details(d).items())
+
         for _if in ssh_if:
             if _if in devices[d]["Interface"].split(","):
                 devices[d]["Ssh_if"] = True
@@ -261,14 +239,12 @@ def get_nic_details():
                 break;
 
         # add igb_uio to list of supporting modules if needed
-        if is_supported_device(d):
-            if "Module_str" in devices[d]:
-                if "igb_uio" not in devices[d]["Module_str"]:
-                    devices[d]["Module_str"] = devices[d]["Module_str"] + ",igb_uio"
-            else:
-                devices[d]["Module_str"] = "igb_uio"
-        if "Module_str" not in devices[d]:
-            devices[d]["Module_str"] = "<none>"
+        if "Module_str" in devices[d]:
+            if "igb_uio" not in devices[d]["Module_str"]:
+                devices[d]["Module_str"] = devices[d]["Module_str"] + ",igb_uio"
+        else:
+            devices[d]["Module_str"] = "igb_uio"
+
         # make sure the driver and module strings do not have any duplicates
         if has_driver(d):
             modules = devices[d]["Module_str"].split(",")
@@ -343,6 +319,22 @@ def bind_one(dev_id, driver, force):
             unbind_one(dev_id, force)
             dev["Driver_str"] = "" # clear driver string
 
+    # if we are binding to one of DPDK drivers, add PCI id's to that driver
+    if driver == "igb_uio":
+        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)
+            return
+        try:
+            f.write("%04x %04x" % (dev["Vendor"], dev["Device"]))
+            f.close()
+        except:
+            print "Error: bind failed for %s - Cannot write new PCI ID to " \
+                "driver %s" % (dev_id, driver)
+            return
+
     # do the bind by writing to /sys
     filename = "/sys/bus/pci/drivers/%s/bind" % driver
     try:
@@ -356,6 +348,12 @@ def bind_one(dev_id, driver, force):
         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)
+        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)
         if saved_driver is not None: # restore any previous driver
             bind_one(dev_id, saved_driver, force)
-- 
1.8.1.4