Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
flat view
From: Mcnamara, John <hidden>
Date: 2016-08-25 09:51:18
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@dpdk.org] On Behalf Of souvikdey33 Sent: Thursday, August 25, 2016 3:26 AM To: nhorman@tuxdriver.com; dev@dpdk.org Cc: souvikdey33 <redacted> 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 [off-list ref]
You should add your real name to the sign off.
quoted hunk ↗ jump to hunk
diff --git a/tools/dpdk-devbind.py b/tools/dpdk-devbind.py indexb69ca2a..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_idThis space/tab indentation gives a Python error.
quoted hunk ↗ jump to hunk
+ 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