[dpdk-dev,v1] dpdk-devbind.py: Virtio interface issue.
Commit Message
Hi,
I already followed the 100% python way and submitted the v3 of this patch. http://dpdk.org/dev/patchwork/patch/15378/
How will your patch be different in solving the issue. There will always be multiple ways to solving things right.
V3 of my submitted patch:
-------------------------------------------
Gary
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dey, Souvik
Sent: Friday, August 26, 2016 8:21 PM
To: Stephen Hemminger
Cc: nhorman@tuxdriver.com; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.
Hi ,
I have already updated it and have re submitted the patch v3. Can you please check that http://dpdk.org/dev/patchwork/patch/15378/
--
Regards,
Souvik
-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Friday, August 26, 2016 11:55 AM
To: Dey, Souvik <sodey@sonusnet.com>
Cc: nhorman@tuxdriver.com; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.
On Wed, 24 Aug 2016 22:25:46 -0400
souvikdey33 <sodey@sonusnet.com> wrote:
> + #The path for virtio devices are different. Get the correct path.
> + virtio = "/sys/bus/pci/devices/%s/" % dev_id
> + 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))
There should be a way to do this in python without going out to shell.
This would be safer and more secure.
The code already uses os.listdir() (which is the python library version of ls) in later section. Why not use that here to check for virtio bus.
Comments
On Mon, 29 Aug 2016 23:16:35 +0000
"Dey, Souvik" <sodey@sonusnet.com> wrote:
> Hi,
>
> I already followed the 100% python way and submitted the v3 of this patch. http://dpdk.org/dev/patchwork/patch/15378/
> How will your patch be different in solving the issue. There will always be multiple ways to solving things right.
>
>
> V3 of my submitted patch:
>
> diff --git a/tools/dpdk-devbind.py b/tools/dpdk-devbind.py
> index b69ca2a..c0b46ee 100755
> --- a/tools/dpdk-devbind.py
> +++ b/tools/dpdk-devbind.py
> @@ -36,6 +36,7 @@ import sys
> import os
> import getopt
> import subprocess
> +
> from os.path import exists, abspath, dirname, basename
>
> # The PCI base class for NETWORK devices
> @@ -222,8 +223,19 @@ 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, so get the correct path
> + virtio = "/sys/bus/pci/devices/%s/" % dev_id
> + ls = subprocess.Popen(['ls', virtio], stdout=subprocess.PIPE)
> + grep = subprocess.Popen('grep virt'.split(), stdin=ls.stdout,
> + stdout=subprocess.PIPE)
> + ls.stdout.close()
> + virtio = grep.communicate()[0].rstrip()
> + ls.wait()
> + 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(virtio_sys_path):
> + device["Interface"] = ",".join(os.listdir(virtio_sys_path))
> else:
> device["Interface"] = ""
> # check if a port is used for ssh connection
When I was suggesting pure python, I meant do it without a sub shell. Popen is just
another wrapper around a sub-shell.
On Mon, Aug 29, 2016 at 11:16:35PM +0000, Dey, Souvik wrote:
> Hi,
>
> I already followed the 100% python way and submitted the v3 of this patch. http://dpdk.org/dev/patchwork/patch/15378/
> How will your patch be different in solving the issue. There will always be multiple ways to solving things right.
>
As stephen says, using popen is a bit of a hack here. You could easily use one
of several python-sysfs libraries to simplify the sysfs enumeration and
discovery process
Neil
>
> V3 of my submitted patch:
>
> diff --git a/tools/dpdk-devbind.py b/tools/dpdk-devbind.py
> index b69ca2a..c0b46ee 100755
> --- a/tools/dpdk-devbind.py
> +++ b/tools/dpdk-devbind.py
> @@ -36,6 +36,7 @@ import sys
> import os
> import getopt
> import subprocess
> +
> from os.path import exists, abspath, dirname, basename
>
> # The PCI base class for NETWORK devices
> @@ -222,8 +223,19 @@ 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, so get the correct path
> + virtio = "/sys/bus/pci/devices/%s/" % dev_id
> + ls = subprocess.Popen(['ls', virtio], stdout=subprocess.PIPE)
> + grep = subprocess.Popen('grep virt'.split(), stdin=ls.stdout,
> + stdout=subprocess.PIPE)
> + ls.stdout.close()
> + virtio = grep.communicate()[0].rstrip()
> + ls.wait()
> + 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(virtio_sys_path):
> + device["Interface"] = ",".join(os.listdir(virtio_sys_path))
> else:
> device["Interface"] = ""
> # check if a port is used for ssh connection
>
>
> -----Original Message-----
> From: Mussar, Gary [mailto:gmussar@ciena.com]
> Sent: Monday, August 29, 2016 11:10 AM
> To: Dey, Souvik <sodey@sonusnet.com>; Stephen Hemminger <stephen@networkplumber.org>
> Cc: nhorman@tuxdriver.com; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.
>
> We did this slightly differently. This is 100% python and is a bit more general. We search for the first "net" directory under the specific device directory.
>
> -------------------------------------------
> --- tools/dpdk-devbind.py 2016-08-29 11:02:35.594202888 -0400
> +++ ../dpdk/tools/dpdk-devbind.py 2016-08-29 11:00:34.897677233 -0400
> @@ -221,11 +221,11 @@
> 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"] = ""
> + device["Interface"] = ""
> + for base, dirs, files in os.walk("/sys/bus/pci/devices/%s/" % dev_id):
> + if "net" in dirs:
> + device["Interface"] = ",".join(os.listdir(os.path.join(base,"net")))
> + break
> # check if a port is used for ssh connection
> device["Ssh_if"] = False
> device["Active"] = ""
> -------------------------------------------
>
> Gary
>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dey, Souvik
> Sent: Friday, August 26, 2016 8:21 PM
> To: Stephen Hemminger
> Cc: nhorman@tuxdriver.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.
>
> Hi ,
> I have already updated it and have re submitted the patch v3. Can you please check that http://dpdk.org/dev/patchwork/patch/15378/
> --
> Regards,
> Souvik
>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, August 26, 2016 11:55 AM
> To: Dey, Souvik <sodey@sonusnet.com>
> Cc: nhorman@tuxdriver.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.
>
> On Wed, 24 Aug 2016 22:25:46 -0400
> souvikdey33 <sodey@sonusnet.com> wrote:
>
> > + #The path for virtio devices are different. Get the correct path.
> > + virtio = "/sys/bus/pci/devices/%s/" % dev_id
> > + 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))
>
> There should be a way to do this in python without going out to shell.
> This would be safer and more secure.
>
> The code already uses os.listdir() (which is the python library version of ls) in later section. Why not use that here to check for virtio bus.
>
@@ -36,6 +36,7 @@ import sys
import os
import getopt
import subprocess
+
from os.path import exists, abspath, dirname, basename
# The PCI base class for NETWORK devices
@@ -222,8 +223,19 @@ 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, so get the correct path
+ virtio = "/sys/bus/pci/devices/%s/" % dev_id
+ ls = subprocess.Popen(['ls', virtio], stdout=subprocess.PIPE)
+ grep = subprocess.Popen('grep virt'.split(), stdin=ls.stdout,
+ stdout=subprocess.PIPE)
+ ls.stdout.close()
+ virtio = grep.communicate()[0].rstrip()
+ ls.wait()
+ 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(virtio_sys_path):
+ device["Interface"] = ",".join(os.listdir(virtio_sys_path))
else:
device["Interface"] = ""
# check if a port is used for ssh connection
-----Original Message-----
From: Mussar, Gary [mailto:gmussar@ciena.com]
Sent: Monday, August 29, 2016 11:10 AM
To: Dey, Souvik <sodey@sonusnet.com>; Stephen Hemminger <stephen@networkplumber.org>
Cc: nhorman@tuxdriver.com; dev@dpdk.org
Subject: RE: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.
We did this slightly differently. This is 100% python and is a bit more general. We search for the first "net" directory under the specific device directory.
-------------------------------------------
@@ -221,11 +221,11 @@
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"] = ""
+ device["Interface"] = ""
+ for base, dirs, files in os.walk("/sys/bus/pci/devices/%s/" % dev_id):
+ if "net" in dirs:
+ device["Interface"] = ",".join(os.listdir(os.path.join(base,"net")))
+ break
# check if a port is used for ssh connection
device["Ssh_if"] = False
device["Active"] = ""