From patchwork Thu Jul 25 14:21:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anatoly Burakov X-Patchwork-Id: 57112 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4C1631C361; Thu, 25 Jul 2019 16:21:17 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id CE62E1C34D for ; Thu, 25 Jul 2019 16:21:13 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Jul 2019 07:21:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,307,1559545200"; d="scan'208";a="345454102" Received: from silpixa00399498.ir.intel.com (HELO silpixa00399498.ger.corp.intel.com) ([10.237.223.125]) by orsmga005.jf.intel.com with ESMTP; 25 Jul 2019 07:21:11 -0700 From: Anatoly Burakov To: dev@dpdk.org Cc: thomas@monjalon.net, john.mcnamara@intel.com, stephen@networkplumber.org Date: Thu, 25 Jul 2019 15:21:08 +0100 Message-Id: X-Mailer: git-send-email 2.17.1 In-Reply-To: References: In-Reply-To: References: Subject: [dpdk-dev] [PATCH v4 1/3] usertools/devbind: add error on forgetting to specify driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" A common user error is to forget driver to which the PCI devices should be bound to. Currently, the error message in this case looks unhelpful misleading and indecipherable to anyone but people who know how devbind works. Fix this by checking if the driver string is actually a valid device string. If it is, we assume that the user has just forgot to specify the driver, and display appropriate error. We also assume that no one will name their driver in a format that looks like a PCI address, but that seems like a reasonable assumption to make. Signed-off-by: Anatoly Burakov --- usertools/dpdk-devbind.py | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py index 542ecffcc..b32506e3d 100755 --- a/usertools/dpdk-devbind.py +++ b/usertools/dpdk-devbind.py @@ -342,9 +342,8 @@ def dev_id_from_dev_name(dev_name): if dev_name in devices[d]["Interface"].split(","): return devices[d]["Slot"] # if nothing else matches - error - print("Unknown device: %s. " - "Please specify device in \"bus:slot.func\" format" % dev_name) - sys.exit(1) + raise ValueError("Unknown device: %s. " + "Please specify device in \"bus:slot.func\" format" % dev_name) def unbind_one(dev_id, force): @@ -493,7 +492,12 @@ def unbind_all(dev_list, force=False): unbind_one(devices[d]["Slot"], force) return - dev_list = map(dev_id_from_dev_name, dev_list) + try: + dev_list = map(dev_id_from_dev_name, dev_list) + except ValueError as ex: + print(ex) + sys.exit(1) + for d in dev_list: unbind_one(d, force) @@ -502,7 +506,23 @@ def bind_all(dev_list, driver, force=False): """Bind method, takes a list of device locations""" global devices - dev_list = map(dev_id_from_dev_name, dev_list) + # a common user error is to forget to specify the driver the devices need to + # be bound to. check if the driver is a valid device, and if it is, show + # a meaningful error. + try: + dev_id_from_dev_name(driver) + # if we've made it this far, this means that the "driver" was a valid + # device string, so it's probably not a valid driver name. + sys.exit("Error: Driver '%s' does not look like a valid driver. " \ + "Did you forget to specify the driver to bind devices to?" % driver) + except ValueError: + # driver generated error - it's not a valid device ID, so all is well + pass + + try: + dev_list = map(dev_id_from_dev_name, dev_list) + except ValueError as ex: + sys.exit(ex) for d in dev_list: bind_one(d, driver, force) From patchwork Thu Jul 25 14:21:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anatoly Burakov X-Patchwork-Id: 57113 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 304521C366; Thu, 25 Jul 2019 16:21:20 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id A3A891C35D for ; Thu, 25 Jul 2019 16:21:15 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Jul 2019 07:21:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,307,1559545200"; d="scan'208";a="345454111" Received: from silpixa00399498.ir.intel.com (HELO silpixa00399498.ger.corp.intel.com) ([10.237.223.125]) by orsmga005.jf.intel.com with ESMTP; 25 Jul 2019 07:21:13 -0700 From: Anatoly Burakov To: dev@dpdk.org Cc: thomas@monjalon.net, john.mcnamara@intel.com, stephen@networkplumber.org Date: Thu, 25 Jul 2019 15:21:09 +0100 Message-Id: X-Mailer: git-send-email 2.17.1 In-Reply-To: References: In-Reply-To: References: Subject: [dpdk-dev] [PATCH v4 2/3] usertools/devbind: check if module is loaded before binding X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Currently, if an attempt is made to bind a device to a driver that is not loaded, a confusing and misleading error message appears. Fix it so that, before binding to the driver, we actually check if it is loaded in the kernel first. Signed-off-by: Anatoly Burakov --- usertools/dpdk-devbind.py | 58 +++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py index b32506e3d..d3b16240f 100755 --- a/usertools/dpdk-devbind.py +++ b/usertools/dpdk-devbind.py @@ -60,6 +60,8 @@ devices = {} # list of supported DPDK drivers dpdk_drivers = ["igb_uio", "vfio-pci", "uio_pci_generic"] +# list of currently loaded kernel modules +loaded_modules = None # command-line arg flags b_flag = None @@ -146,6 +148,28 @@ def check_output(args, stderr=None): return subprocess.Popen(args, stdout=subprocess.PIPE, stderr=stderr).communicate()[0] +# check if a specific kernel module is loaded +def module_is_loaded(module): + global loaded_modules + + if loaded_modules: + return module in loaded_modules + + # Get list of sysfs modules (both built-in and dynamically loaded) + sysfs_path = '/sys/module/' + + # Get the list of directories in sysfs_path + sysfs_mods = [m for m in os.listdir(sysfs_path) + if os.path.isdir(os.path.join(sysfs_path, m))] + + # special case for vfio_pci (module is named vfio-pci, + # but its .ko is named vfio_pci) + sysfs_mods = [a if a != 'vfio_pci' else 'vfio-pci' for a in sysfs_mods] + + loaded_modules = sysfs_mods + + return module in sysfs_mods + def check_modules(): '''Checks that igb_uio is loaded''' @@ -155,35 +179,13 @@ def check_modules(): mods = [{"Name": driver, "Found": False} for driver in dpdk_drivers] # first check if module is loaded - try: - # Get list of sysfs modules (both built-in and dynamically loaded) - sysfs_path = '/sys/module/' - - # Get the list of directories in sysfs_path - sysfs_mods = [os.path.join(sysfs_path, o) for o - in os.listdir(sysfs_path) - if os.path.isdir(os.path.join(sysfs_path, o))] - - # Extract the last element of '/sys/module/abc' in the array - sysfs_mods = [a.split('/')[-1] for a in sysfs_mods] - - # special case for vfio_pci (module is named vfio-pci, - # but its .ko is named vfio_pci) - sysfs_mods = [a if a != 'vfio_pci' else 'vfio-pci' for a in sysfs_mods] - - for mod in mods: - if mod["Name"] in sysfs_mods: - mod["Found"] = True - except: - pass + for mod in mods: + if module_is_loaded(mod["Name"]): + mod["Found"] = True # check if we have at least one loaded module if True not in [mod["Found"] for mod in mods] and b_flag is not None: - if b_flag in dpdk_drivers: - print("Error - no supported modules(DPDK driver) are loaded") - sys.exit(1) - else: - print("Warning - no supported modules(DPDK driver) are loaded") + print("Warning: no supported DPDK kernel modules are loaded") # change DPDK driver list to only contain drivers that are loaded dpdk_drivers = [mod["Name"] for mod in mods if mod["Found"]] @@ -519,6 +521,10 @@ def bind_all(dev_list, driver, force=False): # driver generated error - it's not a valid device ID, so all is well pass + # check if we're attempting to bind to a driver that isn't loaded + if not module_is_loaded(driver): + sys.exit("Error: Driver '%s' is not loaded." % driver) + try: dev_list = map(dev_id_from_dev_name, dev_list) except ValueError as ex: From patchwork Thu Jul 25 14:21:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anatoly Burakov X-Patchwork-Id: 57114 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 715D61C370; Thu, 25 Jul 2019 16:21:23 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id B5F5A1C35D for ; Thu, 25 Jul 2019 16:21:16 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Jul 2019 07:21:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,307,1559545200"; d="scan'208";a="345454121" Received: from silpixa00399498.ir.intel.com (HELO silpixa00399498.ger.corp.intel.com) ([10.237.223.125]) by orsmga005.jf.intel.com with ESMTP; 25 Jul 2019 07:21:15 -0700 From: Anatoly Burakov To: dev@dpdk.org Cc: thomas@monjalon.net, john.mcnamara@intel.com, stephen@networkplumber.org Date: Thu, 25 Jul 2019 15:21:10 +0100 Message-Id: X-Mailer: git-send-email 2.17.1 In-Reply-To: References: In-Reply-To: References: Subject: [dpdk-dev] [PATCH v4 3/3] usertools/devbind: print all errors to stderr X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Bring consistency to error messages and output them to stderr. Als, whenever the script tells the user to "check usage", don't tell the user to do it and just display usage instead. Signed-off-by: Anatoly Burakov --- usertools/dpdk-devbind.py | 58 ++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py index d3b16240f..7b5cbc12c 100755 --- a/usertools/dpdk-devbind.py +++ b/usertools/dpdk-devbind.py @@ -3,6 +3,7 @@ # Copyright(c) 2010-2014 Intel Corporation # +from __future__ import print_function import sys import os import getopt @@ -185,7 +186,7 @@ def check_modules(): # check if we have at least one loaded module if True not in [mod["Found"] for mod in mods] and b_flag is not None: - print("Warning: no supported DPDK kernel modules are loaded") + print("Warning: no supported DPDK kernel modules are loaded", file=sys.stderr) # change DPDK driver list to only contain drivers that are loaded dpdk_drivers = [mod["Name"] for mod in mods if mod["Found"]] @@ -352,14 +353,14 @@ def unbind_one(dev_id, force): '''Unbind the device identified by "dev_id" from its current driver''' dev = devices[dev_id] if not has_driver(dev_id): - print("%s %s %s is not currently managed by any driver\n" % - (dev["Slot"], dev["Device_str"], dev["Interface"])) + print("Notice: %s %s %s is not currently managed by any driver" % + (dev["Slot"], dev["Device_str"], dev["Interface"]), file=sys.stderr) return # prevent us disconnecting ourselves if dev["Ssh_if"] and not force: - print("Routing table indicates that interface %s is active. " - "Skipping unbind" % (dev_id)) + print("Warning: routing table indicates that interface %s is active. " + "Skipping unbind" % dev_id, file=sys.stderr) return # write to /sys to unbind @@ -367,9 +368,8 @@ def unbind_one(dev_id, force): try: f = open(filename, "a") except: - print("Error: unbind failed for %s - Cannot open %s" - % (dev_id, filename)) - sys.exit(1) + sys.exit("Error: unbind failed for %s - Cannot open %s" % + (dev_id, filename)) f.write(dev_id) f.close() @@ -382,15 +382,15 @@ def bind_one(dev_id, driver, force): # prevent disconnection of our ssh session if dev["Ssh_if"] and not force: - print("Routing table indicates that interface %s is active. " - "Not modifying" % (dev_id)) + print("Warning: routing table indicates that interface %s is active. " + "Not modifying" % dev_id, file=sys.stderr) return # unbind any existing drivers we don't want if has_driver(dev_id): if dev["Driver_str"] == driver: - print("%s already bound to driver %s, skipping\n" - % (dev_id, driver)) + print("Notice: %s already bound to driver %s, skipping" % + (dev_id, driver), file=sys.stderr) return else: saved_driver = dev["Driver_str"] @@ -410,14 +410,14 @@ def bind_one(dev_id, driver, force): f = open(filename, "w") except: print("Error: bind failed for %s - Cannot open %s" - % (dev_id, filename)) + % (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)) + "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: @@ -426,7 +426,7 @@ def bind_one(dev_id, driver, force): f = open(filename, "w") except: print("Error: bind failed for %s - Cannot open %s" - % (dev_id, filename)) + % (dev_id, filename), file=sys.stderr) return try: # Convert Device and Vendor Id to int to write to new_id @@ -435,7 +435,7 @@ def bind_one(dev_id, driver, force): f.close() except: print("Error: bind failed for %s - Cannot write new PCI ID to " - "driver %s" % (dev_id, driver)) + "driver %s" % (dev_id, driver), file=sys.stderr) return # do the bind by writing to /sys @@ -444,7 +444,7 @@ def bind_one(dev_id, driver, force): f = open(filename, "a") except: print("Error: bind failed for %s - Cannot open %s" - % (dev_id, filename)) + % (dev_id, filename), file=sys.stderr) if saved_driver is not None: # restore any previous driver bind_one(dev_id, saved_driver, force) return @@ -459,7 +459,7 @@ def bind_one(dev_id, driver, force): 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)) + % (dev_id, driver), file=sys.stderr) if saved_driver is not None: # restore any previous driver bind_one(dev_id, saved_driver, force) return @@ -472,16 +472,14 @@ def bind_one(dev_id, driver, force): try: f = open(filename, "w") except: - print("Error: unbind failed for %s - Cannot open %s" + sys.exit("Error: unbind failed for %s - Cannot open %s" % (dev_id, filename)) - sys.exit(1) try: f.write("\00") f.close() except: - print("Error: unbind failed for %s - Cannot open %s" + sys.exit("Error: unbind failed for %s - Cannot open %s" % (dev_id, filename)) - sys.exit(1) def unbind_all(dev_list, force=False): @@ -676,8 +674,7 @@ def parse_args(): force_flag = True if opt == "-b" or opt == "-u" or opt == "--bind" or opt == "--unbind": if b_flag is not None: - print("Error - Only one bind or unbind may be specified\n") - sys.exit(1) + sys.exit("Error: binding and unbinding are mutually exclusive") if opt == "-u" or opt == "--unbind": b_flag = "none" else: @@ -692,14 +689,14 @@ def do_arg_actions(): global args if b_flag is None and not status_flag: - print("Error: No action specified for devices." - "Please give a -b or -u option") - print("Run '%s --usage' for further information" % sys.argv[0]) + print("Error: No action specified for devices. " + "Please give a -b or -u option", file=sys.stderr) + usage() sys.exit(1) if b_flag is not None and len(args) == 0: - print("Error: No devices specified.") - print("Run '%s --usage' for further information" % sys.argv[0]) + print("Error: No devices specified.", file=sys.stderr) + usage() sys.exit(1) if b_flag == "none" or b_flag == "None": @@ -727,8 +724,7 @@ def main(): ret = subprocess.call(['which', 'lspci'], stdout=devnull, stderr=devnull) if ret != 0: - print("'lspci' not found - please install 'pciutils'") - sys.exit(1) + sys.exit("'lspci' not found - please install 'pciutils'") parse_args() check_modules() clear_data()