[dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected

[dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected

[Bruce Richardson]

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

Re: [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected

[Burakov, Anatoly]

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

Re: [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected

[Bruce Richardson]

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.

Re: [dpdk-dev] [PATCH] usertools/dpdk-telemetry: print name of app when connected

[Burakov, Anatoly]

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

[dpdk-dev] [PATCH v2] usertools/dpdk-telemetry: print name of app when connected

[Bruce Richardson]

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

Re: [dpdk-dev] [PATCH v2] usertools/dpdk-telemetry: print name of app when connected

[Burakov, Anatoly]

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

Re: [dpdk-dev] [PATCH v2] usertools/dpdk-telemetry: print name of app when connected

[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: Kevin Laatz <kevin.laatz@intel.com>

Re: [dpdk-dev] [PATCH v2] usertools/dpdk-telemetry: print name of app when connected

[Thomas Monjalon]

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