Hi,
Welcome to DPDK and thanks for the contribution. It looks like a useful fix.
Since you are a new contributor the user guide on "Contributing Code to DPDK"
explains some of the steps involved:
http://dpdk.org/doc/guides/contributing/patches.html
Some comments below.
> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of souvikdey33
> Sent: Thursday, August 25, 2016 3:26 AM
> To: nhorman at tuxdriver.com; dev at dpdk.org
> Cc: souvikdey33 <sodey at sonusnet.com>
> Subject: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.
As you will see in the guide above the subject line should be lowercase and
shouldn't end with a full stop. Also, the prefix would be better as "tools".
Something like this:
tools: fix issue with virtio interfaces
The word fix on the command line normally means you should add a "Fixes" line
to the body but in this case the issue was probably always there (or at least
since virtio support was added) so you can probably omit it.
>
> This change is required to have the interface name for virtio interfaces.
> When we execute the status command the for virtio inerfaces we get Sample
> output without the change:
> 0000:00:04.0 'Virtio network device' if= drv=virtio-pci
> unused=virtio_pci,igb_uio Though for other drivers this works.
> Sample output with the change:
> 0000:00:04.0 'Virtio network device' if=eth0 drv=virtio-pci
> unused=virtio_pci,igb_uio
>
> souvikdey33 (1):
> Signed-off-by: souvikdey33 <sodey at sonusnet.com>
You should add your real name to the sign off.
> diff --git a/tools/dpdk-devbind.py b/tools/dpdk-devbind.py index
> b69ca2a..9829e25 100755
> --- a/tools/dpdk-devbind.py
> +++ b/tools/dpdk-devbind.py
> @@ -36,6 +36,8 @@ import sys
> import os
> import getopt
> import subprocess
> +import commands
The commands module is deprecated in Python 2 and removed in Python 3.
Python 2 and 3 should both be supported by the DPDK tools. In which case
you can use subprocess.check_output(), or similar, instead.
> +
> from os.path import exists, abspath, dirname, basename
>
> # The PCI base class for NETWORK devices @@ -222,8 +224,15 @@ def
> get_pci_device_details(dev_id):
> device[name] = value
> # check for a unix interface name
> sys_path = "/sys/bus/pci/devices/%s/net/" % dev_id
> + #The path for virtio devices are different. Get the correct path.
> + virtio = "/sys/bus/pci/devices/%s/" % dev_id
This space/tab indentation gives a Python error.
> + cmd = " ls %s | grep 'virt' " %virtio
> + virtio = commands.getoutput(cmd)
> + virtio_sys_path = "/sys/bus/pci/devices/%s/%s/net/" %
> +(dev_id,virtio)
> if exists(sys_path):
> device["Interface"] = ",".join(os.listdir(sys_path))
> + elif exists(virt_path):
> + device["Interface"] = ",".join(os.listdir(virtio_sys_path))
> else:
> device["Interface"] = ""
> # check if a port is used for ssh connection
There a number of small Python formatting issues in the patch. The DPDK Python
code follows the pep8 guidelines:
http://dpdk.org/doc/guides/contributing/coding_style.html#python-code
Here are the warnings:
$ pep8 tools/dpdk-devbind.py
tools/dpdk-devbind.py:227:5: E265 block comment should start with '# '
tools/dpdk-devbind.py:228:1: E101 indentation contains mixed spaces and
tabs
tools/dpdk-devbind.py:228:1: W191 indentation contains tabs
tools/dpdk-devbind.py:228:2: E113 unexpected indentation
tools/dpdk-devbind.py:229:1: E101 indentation contains mixed spaces and
tabs
tools/dpdk-devbind.py:229:36: E225 missing whitespace around operator
tools/dpdk-devbind.py:231:66: E231 missing whitespace after ','
Could you fix those issues and submit a V2 of the patch.
Thanks.
John
