[dpdk-dev] [RFC] devbind: conform to PEP 8 recommended style

[dpdk-dev] [RFC] devbind: conform to PEP 8 recommended style

[Stephen Hemminger]

This fixes most of the warnings from the Flake8 style checker.
The ones remaing are long lines (we allow > 79 characters)
and a line break warning.  The line break style changed in later
versions of PEP 8 and the tool is not updated.

https://www.flake8rules.com/rules/W503.html

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 74d16e4c4b13..540138842bd5 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -91,6 +91,7 @@
 force_flag = False
 args = []
 
+
 # check if a specific kernel module is loaded
 def module_is_loaded(module):
     global loaded_modules
@@ -178,11 +179,13 @@ def get_pci_device_details(dev_id, probe_lspci):
 
     return device
 
+
 def clear_data():
     '''This function clears any old data'''
     global devices
     devices = {}
 
+
 def get_device_details(devices_type):
     '''This function populates the "devices" dictionary. The keys used are
     the pci addresses (domain:bus:slot.func). The values are themselves
@@ -228,7 +231,7 @@ def get_device_details(devices_type):
         rt_info = route.split()
         for i in range(len(rt_info) - 1):
             if rt_info[i] == "dev":
-                ssh_if.append(rt_info[i+1])
+                ssh_if.append(rt_info[i + 1])
 
     # based on the basic info, get extended text details
     for d in devices.keys():
@@ -282,6 +285,7 @@ def device_type_match(dev, devices_type):
                 return True
     return False
 
+
 def dev_id_from_dev_name(dev_name):
     '''Take a device "name" - a string passed in by user to identify a NIC
     device, and determine the device id - i.e. the domain:bus:slot.func - for
@@ -321,9 +325,9 @@ def unbind_one(dev_id, force):
     filename = "/sys/bus/pci/drivers/%s/unbind" % dev["Driver_str"]
     try:
         f = open(filename, "a")
-    except:
-        sys.exit("Error: unbind failed for %s - Cannot open %s" %
-                 (dev_id, filename))
+    except OSError as err:
+        sys.exit("Error: unbind failed for %s - Cannot open %s: %s" %
+                 (dev_id, filename, err))
     f.write(dev_id)
     f.close()
 
@@ -361,58 +365,58 @@ def bind_one(dev_id, driver, force):
         if exists(filename):
             try:
                 f = open(filename, "w")
-            except:
-                print("Error: bind failed for %s - Cannot open %s"
-                      % (dev_id, filename), file=sys.stderr)
+            except OSError as err:
+                print("Error: bind failed for %s - Cannot open %s: %s"
+                      % (dev_id, filename, err), file=sys.stderr)
                 return
             try:
                 f.write("%s" % driver)
                 f.close()
-            except:
+            except OSError as err:
                 print("Error: bind failed for %s - Cannot write driver %s to "
-                      "PCI ID " % (dev_id, driver), file=sys.stderr)
+                      "PCI ID: %s" % (dev_id, driver, err), 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)
+            except OSError as err:
+                print("Error: bind failed for %s - Cannot open %s: %s"
+                      % (dev_id, filename, err), 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:
+            except OSError as err:
                 print("Error: bind failed for %s - Cannot write new PCI ID to "
-                      "driver %s" % (dev_id, driver), file=sys.stderr)
+                      "driver %s: %s" % (dev_id, driver, err), file=sys.stderr)
                 return
 
     # do the bind by writing to /sys
     filename = "/sys/bus/pci/drivers/%s/bind" % driver
     try:
         f = open(filename, "a")
-    except:
-        print("Error: bind failed for %s - Cannot open %s"
-              % (dev_id, filename), file=sys.stderr)
+    except OSError as err:
+        print("Error: bind failed for %s - Cannot open %s: %s"
+              % (dev_id, filename, err), 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:
+    except OSError as err:
         # 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)
+        print("Error: bind failed for %s - Cannot bind to driver %s: %s"
+              % (dev_id, driver, err), file=sys.stderr)
         if saved_driver is not None:  # restore any previous driver
             bind_one(dev_id, saved_driver, force)
         return
@@ -424,15 +428,15 @@ def bind_one(dev_id, driver, force):
     if exists(filename):
         try:
             f = open(filename, "w")
-        except:
-            sys.exit("Error: unbind failed for %s - Cannot open %s"
-                     % (dev_id, filename))
+        except OSError as err:
+            sys.exit("Error: unbind failed for %s - Cannot open %s: %s"
+                     % (dev_id, filename, err))
         try:
             f.write("\00")
             f.close()
-        except:
-            sys.exit("Error: unbind failed for %s - Cannot open %s"
-                     % (dev_id, filename))
+        except OSError as err:
+            sys.exit("Error: unbind failed for %s - Cannot write %s: %s"
+                     % (dev_id, filename, err))
 
 
 def unbind_all(dev_list, force=False):
@@ -466,7 +470,7 @@ def bind_all(dev_list, driver, force=False):
         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. " \
+        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
@@ -496,8 +500,8 @@ def bind_all(dev_list, driver, force=False):
                 continue
 
             # update information about this device
-            devices[d] = dict(devices[d].items() +
-                              get_pci_device_details(d, True).items())
+            devices[d] = dict(devices[d].items()
+                              + get_pci_device_details(d, True).items())
 
             # check if updated information indicates that the device was bound
             if "Driver_str" in devices[d]:
@@ -511,7 +515,7 @@ def display_devices(title, dev_list, extra_params=None):
      device's dictionary.'''
     strings = []  # this holds the strings to print. We sort before printing
     print("\n%s" % title)
-    print("="*len(title))
+    print("=" * len(title))
     if not dev_list:
         strings.append("<none>")
     else:
@@ -527,6 +531,7 @@ def display_devices(title, dev_list, extra_params=None):
     strings.sort()
     print("\n".join(strings))  # print one per line
 
+
 def show_device_status(devices_type, device_name, if_field=False):
     global dpdk_drivers
     kernel_drv = []
@@ -569,6 +574,7 @@ def show_device_status(devices_type, device_name, if_field=False):
         display_devices("Other %s devices" % device_name, no_drv,
                         "unused=%(Module_str)s")
 
+
 def show_status():
     '''Function called when the script is passed the "--status" option.
     Displays to the user what devices are bound to the igb_uio driver, the
@@ -652,7 +658,7 @@ def parse_args():
         '--status-dev',
         help="Print the status of given device group.",
         choices=['baseband', 'compress', 'crypto', 'event',
-                'mempool', 'misc', 'net', 'regex'])
+                 'mempool', 'misc', 'net', 'regex'])
     bind_group = parser.add_mutually_exclusive_group()
     bind_group.add_argument(
         '-b',
@@ -714,6 +720,7 @@ def parse_args():
         new_args.extend(pci_glob(arg))
     args = new_args
 
+
 def do_arg_actions():
     '''do the actual action requested by the user'''
     global b_flag
@@ -761,5 +768,6 @@ def main():
     get_device_details(misc_devices)
     do_arg_actions()
 
+
 if __name__ == "__main__":
     main()
-- 
2.30.2

Re: [dpdk-dev] [RFC] devbind: conform to PEP 8 recommended style

[Bruce Richardson]

On Wed, Sep 01, 2021 at 02:27:07PM -0700, Stephen Hemminger wrote:
> This fixes most of the warnings from the Flake8 style checker.
> The ones remaing are long lines (we allow > 79 characters)
> and a line break warning.  The line break style changed in later
> versions of PEP 8 and the tool is not updated.
> 
> https://www.flake8rules.com/rules/W503.html
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
>  1 file changed, 39 insertions(+), 31 deletions(-)
>
Thanks for the cleanup.

Acked-by: Bruce Richardson <bruce.richardson@intel.com> 

Re: [dpdk-dev] [RFC] devbind: conform to PEP 8 recommended style

[Burakov, Anatoly]

On 01-Sep-21 10:27 PM, Stephen Hemminger wrote:
> This fixes most of the warnings from the Flake8 style checker.
> The ones remaing are long lines (we allow > 79 characters)
> and a line break warning.  The line break style changed in later
> versions of PEP 8 and the tool is not updated.
> 
> https://www.flake8rules.com/rules/W503.html
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

We could improve a lot of code in there, but this is a good baseline.

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [RFC] devbind: conform to PEP 8 recommended style

[Stephen Hemminger]

On Thu, 2 Sep 2021 09:10:47 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Wed, Sep 01, 2021 at 02:27:07PM -0700, Stephen Hemminger wrote:
> > This fixes most of the warnings from the Flake8 style checker.
> > The ones remaing are long lines (we allow > 79 characters)
> > and a line break warning.  The line break style changed in later
> > versions of PEP 8 and the tool is not updated.
> > 
> > https://www.flake8rules.com/rules/W503.html
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
> >  1 file changed, 39 insertions(+), 31 deletions(-)
> >  
> Thanks for the cleanup.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com> 

I don't use devbind (except in artifical testing), could someone with
hardware make sure that nothing changed here.

Re: [dpdk-dev] [RFC] devbind: conform to PEP 8 recommended style

[Bruce Richardson]

On Thu, Sep 02, 2021 at 08:24:58AM -0700, Stephen Hemminger wrote:
> On Thu, 2 Sep 2021 09:10:47 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > On Wed, Sep 01, 2021 at 02:27:07PM -0700, Stephen Hemminger wrote:
> > > This fixes most of the warnings from the Flake8 style checker.
> > > The ones remaing are long lines (we allow > 79 characters)
> > > and a line break warning.  The line break style changed in later
> > > versions of PEP 8 and the tool is not updated.
> > > 
> > > https://www.flake8rules.com/rules/W503.html
> > > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > >  usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
> > >  1 file changed, 39 insertions(+), 31 deletions(-)
> > >  
> > Thanks for the cleanup.
> > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com> 
> 
> I don't use devbind (except in artifical testing), could someone with
> hardware make sure that nothing changed here.

I didn't go testing with all possible options and inputs, but basic
operation for binding unbinding devices and giving error messages for
invalid device names works fine with this patch applied.

Tested-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [RFC] devbind: conform to PEP 8 recommended style

[Stephen Hemminger]

On Thu, 2 Sep 2021 16:50:14 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Thu, Sep 02, 2021 at 08:24:58AM -0700, Stephen Hemminger wrote:
> > On Thu, 2 Sep 2021 09:10:47 +0100
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> >   
> > > On Wed, Sep 01, 2021 at 02:27:07PM -0700, Stephen Hemminger wrote:  
> > > > This fixes most of the warnings from the Flake8 style checker.
> > > > The ones remaing are long lines (we allow > 79 characters)
> > > > and a line break warning.  The line break style changed in later
> > > > versions of PEP 8 and the tool is not updated.
> > > > 
> > > > https://www.flake8rules.com/rules/W503.html
> > > > 
> > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > ---
> > > >  usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
> > > >  1 file changed, 39 insertions(+), 31 deletions(-)
> > > >    
> > > Thanks for the cleanup.
> > > 
> > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>   
> > 
> > I don't use devbind (except in artifical testing), could someone with
> > hardware make sure that nothing changed here.  
> 
> I didn't go testing with all possible options and inputs, but basic
> operation for binding unbinding devices and giving error messages for
> invalid device names works fine with this patch applied.
> 
> Tested-by: Bruce Richardson <bruce.richardson@intel.com>

Thanks

Re: [dpdk-dev] [RFC] devbind: conform to PEP 8 recommended style

[Thomas Monjalon]

02/09/2021 18:04, Stephen Hemminger:
> On Thu, 2 Sep 2021 16:50:14 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> > On Thu, Sep 02, 2021 at 08:24:58AM -0700, Stephen Hemminger wrote:
> > > On Thu, 2 Sep 2021 09:10:47 +0100
> > > Bruce Richardson <bruce.richardson@intel.com> wrote:
> > >   
> > > > On Wed, Sep 01, 2021 at 02:27:07PM -0700, Stephen Hemminger wrote:  
> > > > > This fixes most of the warnings from the Flake8 style checker.
> > > > > The ones remaing are long lines (we allow > 79 characters)
> > > > > and a line break warning.  The line break style changed in later
> > > > > versions of PEP 8 and the tool is not updated.
> > > > > 
> > > > > https://www.flake8rules.com/rules/W503.html
> > > > > 
> > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > > ---
> > > > >  usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
> > > > >  1 file changed, 39 insertions(+), 31 deletions(-)
> > > > >    
> > > > Thanks for the cleanup.
> > > > 
> > > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>   
> > > 
> > > I don't use devbind (except in artifical testing), could someone with
> > > hardware make sure that nothing changed here.  
> > 
> > I didn't go testing with all possible options and inputs, but basic
> > operation for binding unbinding devices and giving error messages for
> > invalid device names works fine with this patch applied.
> > 
> > Tested-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Thanks

Applied, thanks.