DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected
@ 2021-02-16  9:44 Bruce Richardson
  2021-02-16 10:40 ` Burakov, Anatoly
  2021-02-16 11:39 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
  0 siblings, 2 replies; 8+ messages in thread
From: Bruce Richardson @ 2021-02-16  9:44 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Kevin Laatz

When the dpdk-telemetry client connects to a DPDK instance, we can use the
PID provided in the initial connection message to query from /proc the name
of the process we are connected to, and display that to the user. We use
the "cmdline" procfs entry for the query since that is available on both
Linux and FreeBSD (assuming procfs is mounted on the BSD instance).

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 usertools/dpdk-telemetry.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
index 181859658f..82b91f346f 100755
--- a/usertools/dpdk-telemetry.py
+++ b/usertools/dpdk-telemetry.py
@@ -45,6 +45,11 @@ def handle_socket(path):
         return
     json_reply = read_socket(sock, 1024)
     output_buf_len = json_reply["max_output_len"]
+    pid = json_reply["pid"]
+    if os.path.exists('/proc/' + str(pid) + '/cmdline'):
+        with open('/proc/' + str(pid) + '/cmdline') as f:
+            argv0 = f.read(1024).split('\0')[0]
+            print("Connected to application: '" + os.path.basename(argv0) + "'")
 
     # get list of commands for readline completion
     sock.send("/".encode())
-- 
2.27.0


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

* Re: [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected
  2021-02-16  9:44 [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected Bruce Richardson
@ 2021-02-16 10:40 ` Burakov, Anatoly
  2021-02-16 11:02   ` Bruce Richardson
  2021-02-16 11:39 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
  1 sibling, 1 reply; 8+ messages in thread
From: Burakov, Anatoly @ 2021-02-16 10:40 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: Kevin Laatz

On 16-Feb-21 9:44 AM, Bruce Richardson wrote:
> When the dpdk-telemetry client connects to a DPDK instance, we can use the
> PID provided in the initial connection message to query from /proc the name
> of the process we are connected to, and display that to the user. We use
> the "cmdline" procfs entry for the query since that is available on both
> Linux and FreeBSD (assuming procfs is mounted on the BSD instance).
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   usertools/dpdk-telemetry.py | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
> index 181859658f..82b91f346f 100755
> --- a/usertools/dpdk-telemetry.py
> +++ b/usertools/dpdk-telemetry.py
> @@ -45,6 +45,11 @@ def handle_socket(path):
>           return
>       json_reply = read_socket(sock, 1024)
>       output_buf_len = json_reply["max_output_len"]
> +    pid = json_reply["pid"]
> +    if os.path.exists('/proc/' + str(pid) + '/cmdline'):
> +        with open('/proc/' + str(pid) + '/cmdline') as f:

First of all, this is better done using os.path.join:

path = os.path.join('/proc', str(pid), 'cmdline')
if os.path.exists(path):
     with open(path) as f:
         ...

More importantly this isn't terribly Pythonic as it's not over-using 
exceptions :) IMO a better way would be:

try:
     with open(path) as f:
         ...
except IOError as e:
     # ignore if doesn't exist
     if e.errno != errno.ENOENT:
         raise

> +            argv0 = f.read(1024).split('\0')[0]
> +            print("Connected to application: '" + os.path.basename(argv0) + "'")

Also, formatting is better than concatenation, e.g. at least:

bname = os.path.basename(argv0)
print("Connected to application: '{}'".format(bname))

>   
>       # get list of commands for readline completion
>       sock.send("/".encode())
> 


-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected
  2021-02-16 10:40 ` Burakov, Anatoly
@ 2021-02-16 11:02   ` Bruce Richardson
  2021-02-16 11:13     ` Burakov, Anatoly
  0 siblings, 1 reply; 8+ messages in thread
From: Bruce Richardson @ 2021-02-16 11:02 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Kevin Laatz

On Tue, Feb 16, 2021 at 10:40:36AM +0000, Burakov, Anatoly wrote:
> On 16-Feb-21 9:44 AM, Bruce Richardson wrote:
> > When the dpdk-telemetry client connects to a DPDK instance, we can use the
> > PID provided in the initial connection message to query from /proc the name
> > of the process we are connected to, and display that to the user. We use
> > the "cmdline" procfs entry for the query since that is available on both
> > Linux and FreeBSD (assuming procfs is mounted on the BSD instance).
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >   usertools/dpdk-telemetry.py | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
> > index 181859658f..82b91f346f 100755
> > --- a/usertools/dpdk-telemetry.py
> > +++ b/usertools/dpdk-telemetry.py
> > @@ -45,6 +45,11 @@ def handle_socket(path):
> >           return
> >       json_reply = read_socket(sock, 1024)
> >       output_buf_len = json_reply["max_output_len"]
> > +    pid = json_reply["pid"]
> > +    if os.path.exists('/proc/' + str(pid) + '/cmdline'):
> > +        with open('/proc/' + str(pid) + '/cmdline') as f:
> 
> First of all, this is better done using os.path.join:
> 
> path = os.path.join('/proc', str(pid), 'cmdline')
> if os.path.exists(path):
>     with open(path) as f:
>         ...

Ok, I forgot that os.path.join can take > 2 parameters.

> 
> More importantly this isn't terribly Pythonic as it's not over-using
> exceptions :) IMO a better way would be:
> 
> try:
>     with open(path) as f:
>         ...
> except IOError as e:
>     # ignore if doesn't exist
>     if e.errno != errno.ENOENT:
>         raise
> 

Yes, I was thinking that I just wanted any exceptions to be raised, but you
right that I should just ignore the not-found one and skip the printout.

> > +            argv0 = f.read(1024).split('\0')[0]
> > +            print("Connected to application: '" + os.path.basename(argv0) + "'")
> 
> Also, formatting is better than concatenation, e.g. at least:
> 
> bname = os.path.basename(argv0)
> print("Connected to application: '{}'".format(bname))
> 
And f-strings are best of all, but we need python 3.6 for those. :-) I
consider use of format vs concat a matter of taste, but I'll update as you
suggest.

Thanks for the review.

/Bruce

PS: Python 3.5 has had its final release late last year:
https://www.python.org/downloads/release/python-3510/
We should soon consider updating our minimum required version to Python
3.6, allowing us to use f-strings in our python code rather than
concatenation or explicit format calls.

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

* Re: [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected
  2021-02-16 11:02   ` Bruce Richardson
@ 2021-02-16 11:13     ` Burakov, Anatoly
  0 siblings, 0 replies; 8+ messages in thread
From: Burakov, Anatoly @ 2021-02-16 11:13 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Kevin Laatz

On 16-Feb-21 11:02 AM, Bruce Richardson wrote:
> On Tue, Feb 16, 2021 at 10:40:36AM +0000, Burakov, Anatoly wrote:
>> On 16-Feb-21 9:44 AM, Bruce Richardson wrote:
>>> When the dpdk-telemetry client connects to a DPDK instance, we can use the
>>> PID provided in the initial connection message to query from /proc the name
>>> of the process we are connected to, and display that to the user. We use
>>> the "cmdline" procfs entry for the query since that is available on both
>>> Linux and FreeBSD (assuming procfs is mounted on the BSD instance).
>>>
>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>>> ---
>>>    usertools/dpdk-telemetry.py | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
>>> index 181859658f..82b91f346f 100755
>>> --- a/usertools/dpdk-telemetry.py
>>> +++ b/usertools/dpdk-telemetry.py
>>> @@ -45,6 +45,11 @@ def handle_socket(path):
>>>            return
>>>        json_reply = read_socket(sock, 1024)
>>>        output_buf_len = json_reply["max_output_len"]
>>> +    pid = json_reply["pid"]
>>> +    if os.path.exists('/proc/' + str(pid) + '/cmdline'):
>>> +        with open('/proc/' + str(pid) + '/cmdline') as f:
>>
>> First of all, this is better done using os.path.join:
>>
>> path = os.path.join('/proc', str(pid), 'cmdline')
>> if os.path.exists(path):
>>      with open(path) as f:
>>          ...
> 
> Ok, I forgot that os.path.join can take > 2 parameters.
> 
>>
>> More importantly this isn't terribly Pythonic as it's not over-using
>> exceptions :) IMO a better way would be:
>>
>> try:
>>      with open(path) as f:
>>          ...
>> except IOError as e:
>>      # ignore if doesn't exist
>>      if e.errno != errno.ENOENT:
>>          raise
>>
> 
> Yes, I was thinking that I just wanted any exceptions to be raised, but you
> right that I should just ignore the not-found one and skip the printout.

'raise' will raise the exception up the stack, so you'll still get your 
exceptions that way - you'll just skip the one that says the file 
doesn't exist. plus, it also has a benefit of avoiding TOCTOU race :)

> 
>>> +            argv0 = f.read(1024).split('\0')[0]
>>> +            print("Connected to application: '" + os.path.basename(argv0) + "'")
>>
>> Also, formatting is better than concatenation, e.g. at least:
>>
>> bname = os.path.basename(argv0)
>> print("Connected to application: '{}'".format(bname))
>>
> And f-strings are best of all, but we need python 3.6 for those. :-) I
> consider use of format vs concat a matter of taste, but I'll update as you
> suggest.
> 
> Thanks for the review.
> 
> /Bruce
> 
> PS: Python 3.5 has had its final release late last year:
> https://www.python.org/downloads/release/python-3510/
> We should soon consider updating our minimum required version to Python
> 3.6, allowing us to use f-strings in our python code rather than
> concatenation or explicit format calls.
> 

I seem to have a lot of difficulty remembering syntax for f-strings :P

-- 
Thanks,
Anatoly

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

* [dpdk-dev] [PATCH v2] usertools/dpdk-telemetry: print name of app when connected
  2021-02-16  9:44 [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected Bruce Richardson
  2021-02-16 10:40 ` Burakov, Anatoly
@ 2021-02-16 11:39 ` Bruce Richardson
  2021-02-16 12:19   ` Burakov, Anatoly
  2021-02-17 13:57   ` Kevin Laatz
  1 sibling, 2 replies; 8+ messages in thread
From: Bruce Richardson @ 2021-02-16 11:39 UTC (permalink / raw)
  To: dev; +Cc: anatoly.burakov, Bruce Richardson, Kevin Laatz

When the dpdk-telemetry client connects to a DPDK instance, we can use the
PID provided in the initial connection message to query from /proc the name
of the process we are connected to, and display that to the user. We use
the "cmdline" procfs entry for the query since that is available on both
Linux and FreeBSD (assuming procfs is mounted on the BSD instance).

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

---
V2:
  Updated with stylistic feedback from Anatoly, and split name query into a
  separate function.
---
 usertools/dpdk-telemetry.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
index 181859658..b2acd92ba 100755
--- a/usertools/dpdk-telemetry.py
+++ b/usertools/dpdk-telemetry.py
@@ -11,6 +11,7 @@
 import os
 import glob
 import json
+import errno
 import readline

 # global vars
@@ -32,6 +33,20 @@ def read_socket(sock, buf_len, echo=True):
     return ret


+def get_app_name(pid):
+    """ return the app name for a given pid, for printing """
+    proc_cmdline = os.path.join('/proc', str(pid), 'cmdline')
+    try:
+        with open(proc_cmdline) as f:
+            argv0 = f.read(1024).split('\0')[0]
+            return os.path.basename(argv0)
+    except IOError as e:
+        # ignore file not found errors
+        if e.errno != errno.ENOENT:
+            raise
+    return None
+
+
 def handle_socket(path):
     """ Connect to socket and handle user input """
     sock = socket.socket(socket.AF_UNIX, socket.SOCK_SEQPACKET)
@@ -45,6 +60,9 @@ def handle_socket(path):
         return
     json_reply = read_socket(sock, 1024)
     output_buf_len = json_reply["max_output_len"]
+    app_name = get_app_name(json_reply["pid"])
+    if app_name:
+        print('Connected to application: "%s"' % app_name)

     # get list of commands for readline completion
     sock.send("/".encode())
--
2.27.0


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

* Re: [dpdk-dev] [PATCH v2] usertools/dpdk-telemetry: print name of app when connected
  2021-02-16 11:39 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
@ 2021-02-16 12:19   ` Burakov, Anatoly
  2021-02-17 13:57   ` Kevin Laatz
  1 sibling, 0 replies; 8+ messages in thread
From: Burakov, Anatoly @ 2021-02-16 12:19 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: Kevin Laatz

On 16-Feb-21 11:39 AM, Bruce Richardson wrote:
> When the dpdk-telemetry client connects to a DPDK instance, we can use the
> PID provided in the initial connection message to query from /proc the name
> of the process we are connected to, and display that to the user. We use
> the "cmdline" procfs entry for the query since that is available on both
> Linux and FreeBSD (assuming procfs is mounted on the BSD instance).
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> ---
> V2:
>    Updated with stylistic feedback from Anatoly, and split name query into a
>    separate function.
> ---
>   usertools/dpdk-telemetry.py | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
> index 181859658..b2acd92ba 100755
> --- a/usertools/dpdk-telemetry.py
> +++ b/usertools/dpdk-telemetry.py
> @@ -11,6 +11,7 @@
>   import os
>   import glob
>   import json
> +import errno
>   import readline
> 
>   # global vars
> @@ -32,6 +33,20 @@ def read_socket(sock, buf_len, echo=True):
>       return ret
> 
> 
> +def get_app_name(pid):
> +    """ return the app name for a given pid, for printing """
> +    proc_cmdline = os.path.join('/proc', str(pid), 'cmdline')
> +    try:
> +        with open(proc_cmdline) as f:
> +            argv0 = f.read(1024).split('\0')[0]
> +            return os.path.basename(argv0)
> +    except IOError as e:
> +        # ignore file not found errors
> +        if e.errno != errno.ENOENT:
> +            raise
> +    return None
> +
> +
>   def handle_socket(path):
>       """ Connect to socket and handle user input """
>       sock = socket.socket(socket.AF_UNIX, socket.SOCK_SEQPACKET)
> @@ -45,6 +60,9 @@ def handle_socket(path):
>           return
>       json_reply = read_socket(sock, 1024)
>       output_buf_len = json_reply["max_output_len"]
> +    app_name = get_app_name(json_reply["pid"])
> +    if app_name:
> +        print('Connected to application: "%s"' % app_name)
> 
>       # get list of commands for readline completion
>       sock.send("/".encode())
> --
> 2.27.0
> 

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

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH v2] usertools/dpdk-telemetry: print name of app when connected
  2021-02-16 11:39 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
  2021-02-16 12:19   ` Burakov, Anatoly
@ 2021-02-17 13:57   ` Kevin Laatz
  2021-03-25 17:00     ` Thomas Monjalon
  1 sibling, 1 reply; 8+ messages in thread
From: Kevin Laatz @ 2021-02-17 13:57 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: anatoly.burakov

On 16/02/2021 11:39, Bruce Richardson wrote:
> When the dpdk-telemetry client connects to a DPDK instance, we can use the
> PID provided in the initial connection message to query from /proc the name
> of the process we are connected to, and display that to the user. We use
> the "cmdline" procfs entry for the query since that is available on both
> Linux and FreeBSD (assuming procfs is mounted on the BSD instance).
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>
Acked-by: Kevin Laatz <kevin.laatz@intel.com>

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

* Re: [dpdk-dev] [PATCH v2] usertools/dpdk-telemetry: print name of app when connected
  2021-02-17 13:57   ` Kevin Laatz
@ 2021-03-25 17:00     ` Thomas Monjalon
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2021-03-25 17:00 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, anatoly.burakov, Kevin Laatz

17/02/2021 14:57, Kevin Laatz:
> On 16/02/2021 11:39, Bruce Richardson wrote:
> > When the dpdk-telemetry client connects to a DPDK instance, we can use the
> > PID provided in the initial connection message to query from /proc the name
> > of the process we are connected to, and display that to the user. We use
> > the "cmdline" procfs entry for the query since that is available on both
> > Linux and FreeBSD (assuming procfs is mounted on the BSD instance).
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Kevin Laatz <kevin.laatz@intel.com>

Applied, thanks



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

end of thread, other threads:[~2021-03-25 17:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16  9:44 [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected Bruce Richardson
2021-02-16 10:40 ` Burakov, Anatoly
2021-02-16 11:02   ` Bruce Richardson
2021-02-16 11:13     ` Burakov, Anatoly
2021-02-16 11:39 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
2021-02-16 12:19   ` Burakov, Anatoly
2021-02-17 13:57   ` Kevin Laatz
2021-03-25 17:00     ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).