All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] dpdk-devbind.py: Virtio interface issue.
@ 2016-08-25  2:25 souvikdey33
  2016-08-25  9:51 ` Mcnamara, John
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: souvikdey33 @ 2016-08-25  2:25 UTC (permalink / raw)
  To: nhorman, dev; +Cc: souvikdey33

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@sonusnet.com>

 tools/dpdk-devbind.py | 9 +++++++++
 1 file changed, 9 insertions(+)

 From d9e8937b8d88a22ee5519fde2c728b377bc8fb1f Mon Sep 17 00:00:00 2001
From: souvikdey33 <sodey@sonusnet.com>
Date: Wed, 24 Aug 2016 19:56:36 -0400
Subject: [PATCH v1] Signed-off-by: souvikdey33 <sodey@sonusnet.com>

When we execute the status command the for virtio inerfaces the interface name is not shown.
Sample output without the change.
0000:00:04.0 'Virtio network device' if= drv=virtio-pci unused=virtio_pci,igb_uio
Though for other this works.
---
 tools/dpdk-devbind.py | 9 +++++++++
 1 file changed, 9 insertions(+)

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
+
 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
+    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
-- 
2.9.3.windows.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-25  2:25 [PATCH v1] dpdk-devbind.py: Virtio interface issue souvikdey33
@ 2016-08-25  9:51 ` Mcnamara, John
  2016-08-25 10:19   ` Thomas Monjalon
  2016-08-26  0:37 ` Stephen Hemminger
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Mcnamara, John @ 2016-08-25  9:51 UTC (permalink / raw)
  To: souvikdey33, nhorman, dev

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 <sodey@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@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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-25  9:51 ` Mcnamara, John
@ 2016-08-25 10:19   ` Thomas Monjalon
  2016-08-25 10:27     ` Mcnamara, John
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Monjalon @ 2016-08-25 10:19 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev, souvikdey33, nhorman

2016-08-25 09:51, Mcnamara, John:
> 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.

Even if it has always been there, we need to know the commit origin.
The "Fixes:" line makes things clear and helps when backporting.
Thanks

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-25 10:19   ` Thomas Monjalon
@ 2016-08-25 10:27     ` Mcnamara, John
  0 siblings, 0 replies; 19+ messages in thread
From: Mcnamara, John @ 2016-08-25 10:27 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, souvikdey33, nhorman

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, August 25, 2016 11:19 AM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; souvikdey33 <sodey@sonusnet.com>; nhorman@tuxdriver.com
> Subject: Re: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface
> issue.
> 
> 2016-08-25 09:51, Mcnamara, John:
> > 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.
> 
> Even if it has always been there, we need to know the commit origin.
> The "Fixes:" line makes things clear and helps when backporting.
> Thanks

In that case the fixline should be:

    Fixes: 629395b063e8 ("igb_uio: remove PCI id table")

John

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-25  2:25 [PATCH v1] dpdk-devbind.py: Virtio interface issue souvikdey33
  2016-08-25  9:51 ` Mcnamara, John
@ 2016-08-26  0:37 ` Stephen Hemminger
  2016-08-26  3:59 ` [PATCH v2] tools: fix issue with virtio interfaces souvikdey33
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Stephen Hemminger @ 2016-08-26  0:37 UTC (permalink / raw)
  To: souvikdey33; +Cc: nhorman, dev

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)

I am not a python export but in general it is better to use native language
facilities likes os.listdir() rather than using shell pipes.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] tools: fix issue with virtio interfaces
  2016-08-25  2:25 [PATCH v1] dpdk-devbind.py: Virtio interface issue souvikdey33
  2016-08-25  9:51 ` Mcnamara, John
  2016-08-26  0:37 ` Stephen Hemminger
@ 2016-08-26  3:59 ` souvikdey33
  2016-08-26  5:50 ` souvikdey33
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: souvikdey33 @ 2016-08-26  3:59 UTC (permalink / raw)
  To: nhorman, dev; +Cc: souvikdey33

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: Souvik Dey <sodey@sonusnet.com>
  Fixes: 3da038604009 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
  
 tools/dpdk-devbind.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

-- 
>From 3da0386040092fcd54ee333ceff8c427a36c6b45 Mon Sep 17 00:00:00 2001
From: souvikdey33 <sodey@sonusnet.com>
Date: Thu, 25 Aug 2016 23:31:28 -0400
Subject: [PATCH v2] Signed-off-by: Souvik Dey <sodey@sonusnet.com>

When we execute the status command the for virtio inerfaces the interface name is not shown.
Sample output without the change.
0000:00:04.0 'Virtio network device' if= drv=virtio-pci unused=virtio_pci,igb_uio
Though for other this works.
---
 tools/dpdk-devbind.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/dpdk-devbind.py b/tools/dpdk-devbind.py
index 9829e25..c0b46ee 100755
--- a/tools/dpdk-devbind.py
+++ b/tools/dpdk-devbind.py
@@ -36,7 +36,6 @@ import sys
 import os
 import getopt
 import subprocess
-import commands
 
 from os.path import exists, abspath, dirname, basename
 
@@ -224,14 +223,18 @@ 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
-    cmd = " ls %s | grep 'virt' " %virtio
-    virtio = commands.getoutput(cmd)
-    virtio_sys_path = "/sys/bus/pci/devices/%s/%s/net/" % (dev_id,virtio)
+    # 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(virt_path):
+    elif exists(virtio_sys_path):
         device["Interface"] = ",".join(os.listdir(virtio_sys_path))
     else:
         device["Interface"] = ""
-- 
2.9.3.windows.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2] tools: fix issue with virtio interfaces
  2016-08-25  2:25 [PATCH v1] dpdk-devbind.py: Virtio interface issue souvikdey33
                   ` (2 preceding siblings ...)
  2016-08-26  3:59 ` [PATCH v2] tools: fix issue with virtio interfaces souvikdey33
@ 2016-08-26  5:50 ` souvikdey33
  2016-08-26 11:35 ` [PATCH v3] " souvikdey33
  2016-08-26 15:55 ` [PATCH v1] dpdk-devbind.py: Virtio interface issue Stephen Hemminger
  5 siblings, 0 replies; 19+ messages in thread
From: souvikdey33 @ 2016-08-26  5:50 UTC (permalink / raw)
  To: nhorman, dev; +Cc: souvikdey33

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: Souvik Dey <sodey@sonusnet.com>
  Fixes: 3da038604009 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
  
 tools/dpdk-devbind.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

-- 
>From 3da0386040092fcd54ee333ceff8c427a36c6b45 Mon Sep 17 00:00:00 2001
From: souvikdey33 <sodey@sonusnet.com>
Date: Thu, 25 Aug 2016 23:31:28 -0400
Subject: [PATCH v2] Signed-off-by: Souvik Dey <sodey@sonusnet.com>

When we execute the status command the for virtio inerfaces the interface name is not shown.
Sample output without the change.
0000:00:04.0 'Virtio network device' if= drv=virtio-pci unused=virtio_pci,igb_uio
Though for other this works.
---
 tools/dpdk-devbind.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/dpdk-devbind.py b/tools/dpdk-devbind.py
index 9829e25..c0b46ee 100755
--- a/tools/dpdk-devbind.py
+++ b/tools/dpdk-devbind.py
@@ -36,7 +36,6 @@ import sys
 import os
 import getopt
 import subprocess
-import commands
 
 from os.path import exists, abspath, dirname, basename
 
@@ -224,14 +223,18 @@ 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
-    cmd = " ls %s | grep 'virt' " %virtio
-    virtio = commands.getoutput(cmd)
-    virtio_sys_path = "/sys/bus/pci/devices/%s/%s/net/" % (dev_id,virtio)
+    # 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(virt_path):
+    elif exists(virtio_sys_path):
         device["Interface"] = ",".join(os.listdir(virtio_sys_path))
     else:
         device["Interface"] = ""
-- 
2.9.3.windows.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3] tools: fix issue with virtio interfaces
  2016-08-25  2:25 [PATCH v1] dpdk-devbind.py: Virtio interface issue souvikdey33
                   ` (3 preceding siblings ...)
  2016-08-26  5:50 ` souvikdey33
@ 2016-08-26 11:35 ` souvikdey33
  2016-10-04  9:59   ` Thomas Monjalon
  2016-08-26 15:55 ` [PATCH v1] dpdk-devbind.py: Virtio interface issue Stephen Hemminger
  5 siblings, 1 reply; 19+ messages in thread
From: souvikdey33 @ 2016-08-26 11:35 UTC (permalink / raw)
  To: dev; +Cc: souvikdey33


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: Souvik Dey <sodey@sonusnet.com>

 tools/dpdk-devbind.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

-- 
>From 1fb8e8896ca8d5b33bdcc875231bfb5ff72550c6 Mon Sep 17 00:00:00 2001
From: souvikdey33 <sodey@sonusnet.com>
Date: Fri, 26 Aug 2016 07:27:43 -0400
Subject: [PATCH v3] Signed-off-by: Souvik Dey <sodey@sonusnet.com>

When we execute the status command the for virtio inerfaces the interface name is not shown.
Sample output without the change.
0000:00:04.0 'Virtio network device' if= drv=virtio-pci unused=virtio_pci,igb_uio
Though for other this works.

Fixes: e2af2c716077 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
---
 tools/dpdk-devbind.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

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
-- 
2.9.3.windows.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-25  2:25 [PATCH v1] dpdk-devbind.py: Virtio interface issue souvikdey33
                   ` (4 preceding siblings ...)
  2016-08-26 11:35 ` [PATCH v3] " souvikdey33
@ 2016-08-26 15:55 ` Stephen Hemminger
  2016-08-27  0:20   ` Dey, Souvik
  5 siblings, 1 reply; 19+ messages in thread
From: Stephen Hemminger @ 2016-08-26 15:55 UTC (permalink / raw)
  To: souvikdey33; +Cc: nhorman, dev

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-26 15:55 ` [PATCH v1] dpdk-devbind.py: Virtio interface issue Stephen Hemminger
@ 2016-08-27  0:20   ` Dey, Souvik
  2016-08-29 15:09     ` Mussar, Gary
  0 siblings, 1 reply; 19+ messages in thread
From: Dey, Souvik @ 2016-08-27  0:20 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: nhorman, dev

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-27  0:20   ` Dey, Souvik
@ 2016-08-29 15:09     ` Mussar, Gary
  2016-08-29 23:16       ` Dey, Souvik
  2016-09-01 10:59       ` Mcnamara, John
  0 siblings, 2 replies; 19+ messages in thread
From: Mussar, Gary @ 2016-08-29 15:09 UTC (permalink / raw)
  To: Dey, Souvik, Stephen Hemminger; +Cc: nhorman, dev

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-29 15:09     ` Mussar, Gary
@ 2016-08-29 23:16       ` Dey, Souvik
  2016-08-29 23:33         ` Stephen Hemminger
                           ` (2 more replies)
  2016-09-01 10:59       ` Mcnamara, John
  1 sibling, 3 replies; 19+ messages in thread
From: Dey, Souvik @ 2016-08-29 23:16 UTC (permalink / raw)
  To: Mussar, Gary, Stephen Hemminger; +Cc: nhorman, dev

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


-----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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-29 23:16       ` Dey, Souvik
@ 2016-08-29 23:33         ` Stephen Hemminger
  2016-08-30 12:56         ` Neil Horman
  2016-08-30 13:12         ` Mussar, Gary
  2 siblings, 0 replies; 19+ messages in thread
From: Stephen Hemminger @ 2016-08-29 23:33 UTC (permalink / raw)
  To: Dey, Souvik; +Cc: Mussar, Gary, nhorman, dev

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-29 23:16       ` Dey, Souvik
  2016-08-29 23:33         ` Stephen Hemminger
@ 2016-08-30 12:56         ` Neil Horman
  2016-08-30 13:12         ` Mussar, Gary
  2 siblings, 0 replies; 19+ messages in thread
From: Neil Horman @ 2016-08-30 12:56 UTC (permalink / raw)
  To: Dey, Souvik; +Cc: Mussar, Gary, Stephen Hemminger, dev

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.
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-29 23:16       ` Dey, Souvik
  2016-08-29 23:33         ` Stephen Hemminger
  2016-08-30 12:56         ` Neil Horman
@ 2016-08-30 13:12         ` Mussar, Gary
  2 siblings, 0 replies; 19+ messages in thread
From: Mussar, Gary @ 2016-08-30 13:12 UTC (permalink / raw)
  To: Dey, Souvik, Stephen Hemminger; +Cc: nhorman, dev

-----Original Message-----
From: Dey, Souvik [mailto:sodey@sonusnet.com] 
Sent: Monday, August 29, 2016 7:17 PM
To: Mussar, Gary; Stephen Hemminger
Cc: nhorman@tuxdriver.com; dev@dpdk.org
Subject: RE: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.

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.

GM> When I first tackled this problem I used Popen() and got the exact same feedback about using 100% python. The version I posted yesterday satisfied the internal reviewers. 


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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-08-29 15:09     ` Mussar, Gary
  2016-08-29 23:16       ` Dey, Souvik
@ 2016-09-01 10:59       ` Mcnamara, John
  2016-09-01 22:08         ` Dey, Souvik
  2016-09-02 12:57         ` Mussar, Gary
  1 sibling, 2 replies; 19+ messages in thread
From: Mcnamara, John @ 2016-09-01 10:59 UTC (permalink / raw)
  To: Mussar, Gary, Dey, Souvik, Stephen Hemminger; +Cc: nhorman, dev

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mussar, Gary
> Sent: Monday, August 29, 2016 4:10 PM
> 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"] = ""
> -------------------------------------------

Hi Gary,

That looks like a cleaner solution. Could you submit that as a patch.

Souvik, could you test this patch and confirm it fixes your issue.


Gary, if you submit a patch could you make a few minor changes:

> +    device["Interface"] = ""
> +    for base, dirs, files in os.walk("/sys/bus/pci/devices/%s/" % dev_id):
> +        

If "files" is unused, and it looks like it is, then replace it with "_".


> +            device["Interface"] = ",".join(os.listdir(os.path.join(base,"net")))

There is a space required after "," for PEP8 compliance.

John

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-09-01 10:59       ` Mcnamara, John
@ 2016-09-01 22:08         ` Dey, Souvik
  2016-09-02 12:57         ` Mussar, Gary
  1 sibling, 0 replies; 19+ messages in thread
From: Dey, Souvik @ 2016-09-01 22:08 UTC (permalink / raw)
  To: Mcnamara, John, Mussar, Gary, Stephen Hemminger; +Cc: nhorman, dev

Yes this patch definitely solves my issue too. 

-----Original Message-----
From: Mcnamara, John [mailto:john.mcnamara@intel.com] 
Sent: Thursday, September 1, 2016 7:00 AM
To: Mussar, Gary <gmussar@ciena.com>; 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.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mussar, Gary
> Sent: Monday, August 29, 2016 4:10 PM
> 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"] = ""
> -------------------------------------------

Hi Gary,

That looks like a cleaner solution. Could you submit that as a patch.

Souvik, could you test this patch and confirm it fixes your issue.


Gary, if you submit a patch could you make a few minor changes:

> +    device["Interface"] = ""
> +    for base, dirs, files in os.walk("/sys/bus/pci/devices/%s/" % dev_id):
> +        

If "files" is unused, and it looks like it is, then replace it with "_".


> +            device["Interface"] = 
> + ",".join(os.listdir(os.path.join(base,"net")))

There is a space required after "," for PEP8 compliance.

John

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1] dpdk-devbind.py: Virtio interface issue.
  2016-09-01 10:59       ` Mcnamara, John
  2016-09-01 22:08         ` Dey, Souvik
@ 2016-09-02 12:57         ` Mussar, Gary
  1 sibling, 0 replies; 19+ messages in thread
From: Mussar, Gary @ 2016-09-02 12:57 UTC (permalink / raw)
  To: Mcnamara, John, Dey, Souvik, Stephen Hemminger; +Cc: nhorman, dev

I will get a proper patch sent hopefully today.

Gary

-----Original Message-----
From: Mcnamara, John [mailto:john.mcnamara@intel.com] 
Sent: Thursday, September 01, 2016 7:00 AM
To: Mussar, Gary; Dey, Souvik; Stephen Hemminger
Cc: nhorman@tuxdriver.com; dev@dpdk.org
Subject: RE: [dpdk-dev] [PATCH v1] dpdk-devbind.py: Virtio interface issue.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mussar, Gary
> Sent: Monday, August 29, 2016 4:10 PM
> 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"] = ""
> -------------------------------------------

Hi Gary,

That looks like a cleaner solution. Could you submit that as a patch.

Souvik, could you test this patch and confirm it fixes your issue.


Gary, if you submit a patch could you make a few minor changes:

> +    device["Interface"] = ""
> +    for base, dirs, files in os.walk("/sys/bus/pci/devices/%s/" % dev_id):
> +        

If "files" is unused, and it looks like it is, then replace it with "_".


> +            device["Interface"] = 
> + ",".join(os.listdir(os.path.join(base,"net")))

There is a space required after "," for PEP8 compliance.

John

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] tools: fix issue with virtio interfaces
  2016-08-26 11:35 ` [PATCH v3] " souvikdey33
@ 2016-10-04  9:59   ` Thomas Monjalon
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas Monjalon @ 2016-10-04  9:59 UTC (permalink / raw)
  To: souvikdey33; +Cc: dev

2016-08-26 07:35, souvikdey33:
> 
> 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: Souvik Dey <sodey@sonusnet.com>

The patch from Gary - which do not use subprocess - has been preferred:
	http://dpdk.org/patch/15595

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-10-04  9:59 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-25  2:25 [PATCH v1] dpdk-devbind.py: Virtio interface issue souvikdey33
2016-08-25  9:51 ` Mcnamara, John
2016-08-25 10:19   ` Thomas Monjalon
2016-08-25 10:27     ` Mcnamara, John
2016-08-26  0:37 ` Stephen Hemminger
2016-08-26  3:59 ` [PATCH v2] tools: fix issue with virtio interfaces souvikdey33
2016-08-26  5:50 ` souvikdey33
2016-08-26 11:35 ` [PATCH v3] " souvikdey33
2016-10-04  9:59   ` Thomas Monjalon
2016-08-26 15:55 ` [PATCH v1] dpdk-devbind.py: Virtio interface issue Stephen Hemminger
2016-08-27  0:20   ` Dey, Souvik
2016-08-29 15:09     ` Mussar, Gary
2016-08-29 23:16       ` Dey, Souvik
2016-08-29 23:33         ` Stephen Hemminger
2016-08-30 12:56         ` Neil Horman
2016-08-30 13:12         ` Mussar, Gary
2016-09-01 10:59       ` Mcnamara, John
2016-09-01 22:08         ` Dey, Souvik
2016-09-02 12:57         ` Mussar, Gary

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.