DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/2] usertools: use argparse module to get input and add an argment
@ 2023-01-09  6:55 Huisong Li
  2023-01-09  6:55 ` [PATCH 1/2] usertools: use argparse module to get input parameter Huisong Li
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Huisong Li @ 2023-01-09  6:55 UTC (permalink / raw)
  To: dev
  Cc: bruce.richardson, mb, andrew.rybchenko, huangdaode, liudongdong3,
	fengchengwen, lihuisong

The telemetry client script uses argparse module to get input and addes
an optional argment for file prefix.

Huisong Li (2):
  usertools: use argparse module to get input parameter
  usertools: add an argment for file prefix

 usertools/dpdk-telemetry-client.py | 39 ++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 8 deletions(-)

-- 
2.22.0


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

* [PATCH 1/2] usertools: use argparse module to get input parameter
  2023-01-09  6:55 [PATCH 0/2] usertools: use argparse module to get input and add an argment Huisong Li
@ 2023-01-09  6:55 ` Huisong Li
  2023-01-09  9:14   ` Bruce Richardson
  2023-01-09  6:55 ` [PATCH 2/2] usertools: add an argment for file prefix Huisong Li
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Huisong Li @ 2023-01-09  6:55 UTC (permalink / raw)
  To: dev
  Cc: bruce.richardson, mb, andrew.rybchenko, huangdaode, liudongdong3,
	fengchengwen, lihuisong

The telemetry client script uses argparse module to get input parameter.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 usertools/dpdk-telemetry-client.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
index df41d04fbe..fd69955b32 100755
--- a/usertools/dpdk-telemetry-client.py
+++ b/usertools/dpdk-telemetry-client.py
@@ -6,6 +6,7 @@
 import os
 import sys
 import time
+import argparse
 
 BUFFER_SIZE = 200000
 
@@ -115,13 +116,12 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
 if __name__ == "__main__":
 
     sleep_time = 1
-    file_path = ""
-    if len(sys.argv) == 2:
-        file_path = sys.argv[1]
-    else:
-        print("Warning - No filepath passed, using default (" + DEFAULT_FP + ").")
-        file_path = DEFAULT_FP
+    parser = argparse.ArgumentParser()
+    parser.add_argument('-s', '--sock_path', default=DEFAULT_FP,
+                        help='Provide socket file path connected by legacy client')
+    args = parser.parse_args()
+
     client = Client()
-    client.getFilepath(file_path)
+    client.getFilepath(args.sock_path)
     client.register()
     client.interactiveMenu(sleep_time)
-- 
2.22.0


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

* [PATCH 2/2] usertools: add an argment for file prefix
  2023-01-09  6:55 [PATCH 0/2] usertools: use argparse module to get input and add an argment Huisong Li
  2023-01-09  6:55 ` [PATCH 1/2] usertools: use argparse module to get input parameter Huisong Li
@ 2023-01-09  6:55 ` Huisong Li
  2023-01-09  9:16   ` Bruce Richardson
  2023-01-09 17:07 ` [PATCH] usertools: fix python warnings Stephen Hemminger
  2023-01-10  7:31 ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument Huisong Li
  3 siblings, 1 reply; 15+ messages in thread
From: Huisong Li @ 2023-01-09  6:55 UTC (permalink / raw)
  To: dev
  Cc: bruce.richardson, mb, andrew.rybchenko, huangdaode, liudongdong3,
	fengchengwen, lihuisong

Currently, the file prefix for DPDK runtime directory is 'rte' which was
fixed in the telemetry client script. The user had to modify the prefix
each time to run this script if the file prefix of application isn't 'rte'.
So this patch addes an optional argment for the file prefix, like,
"./dpdk-telemetry-client.py -f rte_xxx -s file_path"

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 usertools/dpdk-telemetry-client.py | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
index fd69955b32..f1561af4c6 100755
--- a/usertools/dpdk-telemetry-client.py
+++ b/usertools/dpdk-telemetry-client.py
@@ -15,6 +15,8 @@
 API_UNREG = "{\"action\":2,\"command\":\"clients\",\"data\":{\"client_path\":\""
 GLOBAL_METRICS_REQ = "{\"action\":0,\"command\":\"global_stat_values\",\"data\":null}"
 DEFAULT_FP = "/var/run/dpdk/default_client"
+DEFAULT_PREFIX = 'rte'
+RUNTIME_SOCKET_NAME = 'telemetry'
 
 class Socket:
 
@@ -36,6 +38,7 @@ class Client:
     def __init__(self): # Creates a client instance
         self.socket = Socket()
         self.file_path = None
+        self.run_path = None
         self.choice = None
         self.unregistered = 0
 
@@ -49,6 +52,10 @@ def __del__(self):
     def getFilepath(self, file_path): # Gets arguments from Command-Line and assigns to instance of client
         self.file_path = file_path
 
+    def setRunpath(self, file_path):
+        self.run_path = os.path.join(get_dpdk_runtime_dir(args.file_prefix),
+                                     RUNTIME_SOCKET_NAME)
+
     def register(self): # Connects a client to DPDK-instance
         if os.path.exists(self.file_path):
             os.unlink(self.file_path)
@@ -57,7 +64,7 @@ def register(self): # Connects a client to DPDK-instance
         except socket.error as msg:
             print ("Error - Socket binding error: " + str(msg) + "\n")
         self.socket.recv_fd.settimeout(2)
-        self.socket.send_fd.connect("/var/run/dpdk/rte/telemetry")
+        self.socket.send_fd.connect(self.run_path)
         JSON = (API_REG + self.file_path + "\"}}")
         self.socket.send_fd.sendall(JSON.encode())
 
@@ -113,15 +120,31 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
             except:
                 pass
 
+
+def get_dpdk_runtime_dir(fp):
+    """ Using the same logic as in DPDK's EAL, get the DPDK runtime directory
+    based on the file-prefix and user """
+    run_dir = os.environ.get('RUNTIME_DIRECTORY')
+    if not run_dir:
+        if (os.getuid() == 0):
+            run_dir = '/var/run'
+        else:
+            run_dir = os.environ.get('XDG_RUNTIME_DIR', '/tmp')
+    return os.path.join(run_dir, 'dpdk', fp)
+
+
 if __name__ == "__main__":
 
     sleep_time = 1
     parser = argparse.ArgumentParser()
+    parser.add_argument('-f', '--file-prefix', default=DEFAULT_PREFIX,
+                        help='Provide file-prefix for DPDK runtime directory')
     parser.add_argument('-s', '--sock_path', default=DEFAULT_FP,
                         help='Provide socket file path connected by legacy client')
     args = parser.parse_args()
 
     client = Client()
     client.getFilepath(args.sock_path)
+    client.setRunpath(args.file_prefix)
     client.register()
     client.interactiveMenu(sleep_time)
-- 
2.22.0


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

* Re: [PATCH 1/2] usertools: use argparse module to get input parameter
  2023-01-09  6:55 ` [PATCH 1/2] usertools: use argparse module to get input parameter Huisong Li
@ 2023-01-09  9:14   ` Bruce Richardson
  2023-01-09 12:26     ` lihuisong (C)
  0 siblings, 1 reply; 15+ messages in thread
From: Bruce Richardson @ 2023-01-09  9:14 UTC (permalink / raw)
  To: Huisong Li
  Cc: dev, mb, andrew.rybchenko, huangdaode, liudongdong3, fengchengwen

On Mon, Jan 09, 2023 at 02:55:46PM +0800, Huisong Li wrote:
> The telemetry client script uses argparse module to get input parameter.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  usertools/dpdk-telemetry-client.py | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

This is an old script using the older telemetry V1 interface, so I'd
generally recommend users switch to using scripts for the v2 interface.
That said, no reason not to improve the script while we have it.

> diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
> index df41d04fbe..fd69955b32 100755
> --- a/usertools/dpdk-telemetry-client.py
> +++ b/usertools/dpdk-telemetry-client.py
> @@ -6,6 +6,7 @@
>  import os
>  import sys
>  import time
> +import argparse
>  
>  BUFFER_SIZE = 200000
>  
> @@ -115,13 +116,12 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
>  if __name__ == "__main__":
>  
>      sleep_time = 1
> -    file_path = ""
> -    if len(sys.argv) == 2:
> -        file_path = sys.argv[1]
> -    else:
> -        print("Warning - No filepath passed, using default (" + DEFAULT_FP + ").")
> -        file_path = DEFAULT_FP
> +    parser = argparse.ArgumentParser()
> +    parser.add_argument('-s', '--sock_path', default=DEFAULT_FP,
> +                        help='Provide socket file path connected by legacy client')
> +    args = parser.parse_args()
> +

While I like using argparse rather than handling args directly, this breaks
compatibility.  For anyone already using this script via automation, this
would break things, as the path needs to be provided via a "-s" parameter,
rather than just tacked on as argv[1].

>      client = Client()
> -    client.getFilepath(file_path)
> +    client.getFilepath(args.sock_path)
>      client.register()
>      client.interactiveMenu(sleep_time)
> -- 
> 2.22.0
> 

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

* Re: [PATCH 2/2] usertools: add an argment for file prefix
  2023-01-09  6:55 ` [PATCH 2/2] usertools: add an argment for file prefix Huisong Li
@ 2023-01-09  9:16   ` Bruce Richardson
  0 siblings, 0 replies; 15+ messages in thread
From: Bruce Richardson @ 2023-01-09  9:16 UTC (permalink / raw)
  To: Huisong Li
  Cc: dev, mb, andrew.rybchenko, huangdaode, liudongdong3, fengchengwen

On Mon, Jan 09, 2023 at 02:55:47PM +0800, Huisong Li wrote:
> Currently, the file prefix for DPDK runtime directory is 'rte' which was
> fixed in the telemetry client script. The user had to modify the prefix
> each time to run this script if the file prefix of application isn't 'rte'.
> So this patch addes an optional argment for the file prefix, like,
> "./dpdk-telemetry-client.py -f rte_xxx -s file_path"
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  usertools/dpdk-telemetry-client.py | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

* Re: [PATCH 1/2] usertools: use argparse module to get input parameter
  2023-01-09  9:14   ` Bruce Richardson
@ 2023-01-09 12:26     ` lihuisong (C)
  2023-01-09 14:32       ` Bruce Richardson
  0 siblings, 1 reply; 15+ messages in thread
From: lihuisong (C) @ 2023-01-09 12:26 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dev, mb, andrew.rybchenko, huangdaode, liudongdong3, fengchengwen


在 2023/1/9 17:14, Bruce Richardson 写道:
> On Mon, Jan 09, 2023 at 02:55:46PM +0800, Huisong Li wrote:
>> The telemetry client script uses argparse module to get input parameter.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   usertools/dpdk-telemetry-client.py | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
> This is an old script using the older telemetry V1 interface, so I'd
> generally recommend users switch to using scripts for the v2 interface.
> That said, no reason not to improve the script while we have it.
Yes. After all, the telemetry v1 interface and this script are still exist.
>
>> diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
>> index df41d04fbe..fd69955b32 100755
>> --- a/usertools/dpdk-telemetry-client.py
>> +++ b/usertools/dpdk-telemetry-client.py
>> @@ -6,6 +6,7 @@
>>   import os
>>   import sys
>>   import time
>> +import argparse
>>   
>>   BUFFER_SIZE = 200000
>>   
>> @@ -115,13 +116,12 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
>>   if __name__ == "__main__":
>>   
>>       sleep_time = 1
>> -    file_path = ""
>> -    if len(sys.argv) == 2:
>> -        file_path = sys.argv[1]
>> -    else:
>> -        print("Warning - No filepath passed, using default (" + DEFAULT_FP + ").")
>> -        file_path = DEFAULT_FP
>> +    parser = argparse.ArgumentParser()
>> +    parser.add_argument('-s', '--sock_path', default=DEFAULT_FP,
>> +                        help='Provide socket file path connected by legacy client')
>> +    args = parser.parse_args()
>> +
> While I like using argparse rather than handling args directly, this breaks
> compatibility.  For anyone already using this script via automation, this
> would break things, as the path needs to be provided via a "-s" parameter,
> rather than just tacked on as argv[1].
If there isn't the modification patch 2/2 mentioned, this script cannot be
directly used in most scenarios. From the first commit of this script, it's
just used as a demo client example. See
commit d1b94da4a4e0 ("usertools: add client script for telemetry")
 From this point of view, can this compatibility issue be ignored?
>
>>       client = Client()
>> -    client.getFilepath(file_path)
>> +    client.getFilepath(args.sock_path)
>>       client.register()
>>       client.interactiveMenu(sleep_time)
>> -- 
>> 2.22.0
>>
> .

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

* Re: [PATCH 1/2] usertools: use argparse module to get input parameter
  2023-01-09 12:26     ` lihuisong (C)
@ 2023-01-09 14:32       ` Bruce Richardson
  2023-01-10  6:24         ` lihuisong (C)
  0 siblings, 1 reply; 15+ messages in thread
From: Bruce Richardson @ 2023-01-09 14:32 UTC (permalink / raw)
  To: lihuisong (C)
  Cc: dev, mb, andrew.rybchenko, huangdaode, liudongdong3, fengchengwen

On Mon, Jan 09, 2023 at 08:26:37PM +0800, lihuisong (C) wrote:
> 
> 在 2023/1/9 17:14, Bruce Richardson 写道:
> > On Mon, Jan 09, 2023 at 02:55:46PM +0800, Huisong Li wrote:
> > > The telemetry client script uses argparse module to get input parameter.
> > > 
> > > Signed-off-by: Huisong Li <lihuisong@huawei.com>
> > > ---
> > >   usertools/dpdk-telemetry-client.py | 14 +++++++-------
> > >   1 file changed, 7 insertions(+), 7 deletions(-)
> > > 
> > This is an old script using the older telemetry V1 interface, so I'd
> > generally recommend users switch to using scripts for the v2 interface.
> > That said, no reason not to improve the script while we have it.
> Yes. After all, the telemetry v1 interface and this script are still exist.
> > 
> > > diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
> > > index df41d04fbe..fd69955b32 100755
> > > --- a/usertools/dpdk-telemetry-client.py
> > > +++ b/usertools/dpdk-telemetry-client.py
> > > @@ -6,6 +6,7 @@
> > >   import os
> > >   import sys
> > >   import time
> > > +import argparse
> > >   BUFFER_SIZE = 200000
> > > @@ -115,13 +116,12 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
> > >   if __name__ == "__main__":
> > >       sleep_time = 1
> > > -    file_path = ""
> > > -    if len(sys.argv) == 2:
> > > -        file_path = sys.argv[1]
> > > -    else:
> > > -        print("Warning - No filepath passed, using default (" + DEFAULT_FP + ").")
> > > -        file_path = DEFAULT_FP
> > > +    parser = argparse.ArgumentParser()
> > > +    parser.add_argument('-s', '--sock_path', default=DEFAULT_FP,
> > > +                        help='Provide socket file path connected by legacy client')
> > > +    args = parser.parse_args()
> > > +
> > While I like using argparse rather than handling args directly, this breaks
> > compatibility.  For anyone already using this script via automation, this
> > would break things, as the path needs to be provided via a "-s" parameter,
> > rather than just tacked on as argv[1].
> If there isn't the modification patch 2/2 mentioned, this script cannot be
> directly used in most scenarios. From the first commit of this script, it's
> just used as a demo client example. See
> commit d1b94da4a4e0 ("usertools: add client script for telemetry")
> From this point of view, can this compatibility issue be ignored?

Agree with you that patch 2/2 is necessary to make script useful for most
cases, and also agree that argparse is the better way to do argument
handling. However, I also think that we can keep compatibility in this
matter - you can add optional positional arguments in argparse to support
backward compatibility.

	parser.add_argument('sock_path', nargs='?', default=...)

should work, I believe, for this case.

/Bruce

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

* [PATCH] usertools: fix python warnings
  2023-01-09  6:55 [PATCH 0/2] usertools: use argparse module to get input and add an argment Huisong Li
  2023-01-09  6:55 ` [PATCH 1/2] usertools: use argparse module to get input parameter Huisong Li
  2023-01-09  6:55 ` [PATCH 2/2] usertools: add an argment for file prefix Huisong Li
@ 2023-01-09 17:07 ` Stephen Hemminger
  2023-02-06  7:47   ` Thomas Monjalon
  2023-01-10  7:31 ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument Huisong Li
  3 siblings, 1 reply; 15+ messages in thread
From: Stephen Hemminger @ 2023-01-09 17:07 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

This fixes most of the flake8 style warnings on the telemetry
script.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 usertools/dpdk-telemetry-client.py | 37 +++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
index f1561af4c6b2..8e8efc151218 100755
--- a/usertools/dpdk-telemetry-client.py
+++ b/usertools/dpdk-telemetry-client.py
@@ -4,7 +4,6 @@
 
 import socket
 import os
-import sys
 import time
 import argparse
 
@@ -18,6 +17,7 @@
 DEFAULT_PREFIX = 'rte'
 RUNTIME_SOCKET_NAME = 'telemetry'
 
+
 class Socket:
 
     def __init__(self):
@@ -33,9 +33,11 @@ def __del__(self):
         except:
             print("Error - Sockets could not be closed")
 
+
 class Client:
 
-    def __init__(self): # Creates a client instance
+    def __init__(self):
+        # Creates a client instance
         self.socket = Socket()
         self.file_path = None
         self.run_path = None
@@ -45,24 +47,26 @@ def __init__(self): # Creates a client instance
     def __del__(self):
         try:
             if self.unregistered == 0:
-                self.unregister();
+                self.unregister()
         except:
             print("Error - Client could not be destroyed")
 
-    def getFilepath(self, file_path): # Gets arguments from Command-Line and assigns to instance of client
+    def getFilepath(self, file_path):
+        # Gets arguments from Command-Line and assigns to instance of client
         self.file_path = file_path
 
     def setRunpath(self, file_path):
         self.run_path = os.path.join(get_dpdk_runtime_dir(args.file_prefix),
                                      RUNTIME_SOCKET_NAME)
 
-    def register(self): # Connects a client to DPDK-instance
+    def register(self):
+        # Connects a client to DPDK-instance
         if os.path.exists(self.file_path):
             os.unlink(self.file_path)
         try:
             self.socket.recv_fd.bind(self.file_path)
         except socket.error as msg:
-            print ("Error - Socket binding error: " + str(msg) + "\n")
+            print("Error - Socket binding error: " + str(msg) + "\n")
         self.socket.recv_fd.settimeout(2)
         self.socket.send_fd.connect(self.run_path)
         JSON = (API_REG + self.file_path + "\"}}")
@@ -71,30 +75,36 @@ def register(self): # Connects a client to DPDK-instance
         self.socket.recv_fd.listen(1)
         self.socket.client_fd = self.socket.recv_fd.accept()[0]
 
-    def unregister(self): # Unregister a given client
+    def unregister(self):
+        # Unregister a given client
         self.socket.client_fd.send((API_UNREG + self.file_path + "\"}}").encode())
         self.socket.client_fd.close()
 
-    def requestMetrics(self): # Requests metrics for given client
+    def requestMetrics(self):
+        # Requests metrics for given client
         self.socket.client_fd.send(METRICS_REQ.encode())
         data = self.socket.client_fd.recv(BUFFER_SIZE).decode()
         print("\nResponse: \n", data)
 
-    def repeatedlyRequestMetrics(self, sleep_time): # Recursively requests metrics for given client
+    def repeatedlyRequestMetrics(self, sleep_time):
+        # Recursively requests metrics for given client
         print("\nPlease enter the number of times you'd like to continuously request Metrics:")
         n_requests = int(input("\n:"))
-        print("\033[F") #Removes the user input from screen, cleans it up
+        # Removes the user input from screen, cleans it up
+        print("\033[F")
         print("\033[K")
         for i in range(n_requests):
             self.requestMetrics()
             time.sleep(sleep_time)
 
-    def requestGlobalMetrics(self): #Requests global metrics for given client
+    def requestGlobalMetrics(self):
+        # Requests global metrics for given client
         self.socket.client_fd.send(GLOBAL_METRICS_REQ.encode())
         data = self.socket.client_fd.recv(BUFFER_SIZE).decode()
         print("\nResponse: \n", data)
 
-    def interactiveMenu(self, sleep_time): # Creates Interactive menu within the script
+    def interactiveMenu(self, sleep_time):
+        # Creates Interactive menu within the script
         while self.choice != 4:
             print("\nOptions Menu")
             print("[1] Send for Metrics for all ports")
@@ -104,7 +114,8 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
 
             try:
                 self.choice = int(input("\n:"))
-                print("\033[F") #Removes the user input for screen, cleans it up
+                # Removes the user input for screen, cleans it up
+                print("\033[F")
                 print("\033[K")
                 if self.choice == 1:
                     self.requestMetrics()
-- 
2.39.0


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

* Re: [PATCH 1/2] usertools: use argparse module to get input parameter
  2023-01-09 14:32       ` Bruce Richardson
@ 2023-01-10  6:24         ` lihuisong (C)
  0 siblings, 0 replies; 15+ messages in thread
From: lihuisong (C) @ 2023-01-10  6:24 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dev, mb, andrew.rybchenko, huangdaode, liudongdong3, fengchengwen


在 2023/1/9 22:32, Bruce Richardson 写道:
> On Mon, Jan 09, 2023 at 08:26:37PM +0800, lihuisong (C) wrote:
>> 在 2023/1/9 17:14, Bruce Richardson 写道:
>>> On Mon, Jan 09, 2023 at 02:55:46PM +0800, Huisong Li wrote:
>>>> The telemetry client script uses argparse module to get input parameter.
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> ---
>>>>    usertools/dpdk-telemetry-client.py | 14 +++++++-------
>>>>    1 file changed, 7 insertions(+), 7 deletions(-)
>>>>
>>> This is an old script using the older telemetry V1 interface, so I'd
>>> generally recommend users switch to using scripts for the v2 interface.
>>> That said, no reason not to improve the script while we have it.
>> Yes. After all, the telemetry v1 interface and this script are still exist.
>>>> diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
>>>> index df41d04fbe..fd69955b32 100755
>>>> --- a/usertools/dpdk-telemetry-client.py
>>>> +++ b/usertools/dpdk-telemetry-client.py
>>>> @@ -6,6 +6,7 @@
>>>>    import os
>>>>    import sys
>>>>    import time
>>>> +import argparse
>>>>    BUFFER_SIZE = 200000
>>>> @@ -115,13 +116,12 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
>>>>    if __name__ == "__main__":
>>>>        sleep_time = 1
>>>> -    file_path = ""
>>>> -    if len(sys.argv) == 2:
>>>> -        file_path = sys.argv[1]
>>>> -    else:
>>>> -        print("Warning - No filepath passed, using default (" + DEFAULT_FP + ").")
>>>> -        file_path = DEFAULT_FP
>>>> +    parser = argparse.ArgumentParser()
>>>> +    parser.add_argument('-s', '--sock_path', default=DEFAULT_FP,
>>>> +                        help='Provide socket file path connected by legacy client')
>>>> +    args = parser.parse_args()
>>>> +
>>> While I like using argparse rather than handling args directly, this breaks
>>> compatibility.  For anyone already using this script via automation, this
>>> would break things, as the path needs to be provided via a "-s" parameter,
>>> rather than just tacked on as argv[1].
>> If there isn't the modification patch 2/2 mentioned, this script cannot be
>> directly used in most scenarios. From the first commit of this script, it's
>> just used as a demo client example. See
>> commit d1b94da4a4e0 ("usertools: add client script for telemetry")
>>  From this point of view, can this compatibility issue be ignored?
> Agree with you that patch 2/2 is necessary to make script useful for most
> cases, and also agree that argparse is the better way to do argument
> handling. However, I also think that we can keep compatibility in this
> matter - you can add optional positional arguments in argparse to support
> backward compatibility.
>
> 	parser.add_argument('sock_path', nargs='?', default=...)
>
> should work, I believe, for this case.
It works well. Thank you for your suggestion. I will fix it in v2.

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

* [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument
  2023-01-09  6:55 [PATCH 0/2] usertools: use argparse module to get input and add an argment Huisong Li
                   ` (2 preceding siblings ...)
  2023-01-09 17:07 ` [PATCH] usertools: fix python warnings Stephen Hemminger
@ 2023-01-10  7:31 ` Huisong Li
  2023-01-10  7:31   ` [PATCH v2 1/2] usertools: use argparse module to get input parameter Huisong Li
                     ` (2 more replies)
  3 siblings, 3 replies; 15+ messages in thread
From: Huisong Li @ 2023-01-10  7:31 UTC (permalink / raw)
  To: dev
  Cc: bruce.richardson, mb, andrew.rybchenko, huangdaode, liudongdong3,
	fengchengwen, lihuisong

The telemetry client script uses argparse to get local socket file path
and file prefix path.

---
 v2: use an optional positional arguments for local socket path to keep
     backward compatibility.

Huisong Li (2):
  usertools: use argparse module to get input parameter
  usertools: add an argument for file prefix

 usertools/dpdk-telemetry-client.py | 39 ++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 8 deletions(-)

-- 
2.22.0


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

* [PATCH v2 1/2] usertools: use argparse module to get input parameter
  2023-01-10  7:31 ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument Huisong Li
@ 2023-01-10  7:31   ` Huisong Li
  2023-01-10  9:12     ` Bruce Richardson
  2023-01-10  7:31   ` [PATCH v2 2/2] usertools: add an argument for file prefix Huisong Li
  2023-02-06  8:30   ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument Thomas Monjalon
  2 siblings, 1 reply; 15+ messages in thread
From: Huisong Li @ 2023-01-10  7:31 UTC (permalink / raw)
  To: dev
  Cc: bruce.richardson, mb, andrew.rybchenko, huangdaode, liudongdong3,
	fengchengwen, lihuisong

The telemetry client script uses argparse module to get input parameter.
argparse uses an optional positional arguments for local socket path to
keep backward compatibility.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 usertools/dpdk-telemetry-client.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
index df41d04fbe..144a8fb5e0 100755
--- a/usertools/dpdk-telemetry-client.py
+++ b/usertools/dpdk-telemetry-client.py
@@ -6,6 +6,7 @@
 import os
 import sys
 import time
+import argparse
 
 BUFFER_SIZE = 200000
 
@@ -115,13 +116,12 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
 if __name__ == "__main__":
 
     sleep_time = 1
-    file_path = ""
-    if len(sys.argv) == 2:
-        file_path = sys.argv[1]
-    else:
-        print("Warning - No filepath passed, using default (" + DEFAULT_FP + ").")
-        file_path = DEFAULT_FP
+    parser = argparse.ArgumentParser()
+    parser.add_argument('sock_path', nargs='?', default=DEFAULT_FP,
+                        help='Provide socket file path connected by legacy client')
+    args = parser.parse_args()
+
     client = Client()
-    client.getFilepath(file_path)
+    client.getFilepath(args.sock_path)
     client.register()
     client.interactiveMenu(sleep_time)
-- 
2.22.0


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

* [PATCH v2 2/2] usertools: add an argument for file prefix
  2023-01-10  7:31 ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument Huisong Li
  2023-01-10  7:31   ` [PATCH v2 1/2] usertools: use argparse module to get input parameter Huisong Li
@ 2023-01-10  7:31   ` Huisong Li
  2023-02-06  8:30   ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument Thomas Monjalon
  2 siblings, 0 replies; 15+ messages in thread
From: Huisong Li @ 2023-01-10  7:31 UTC (permalink / raw)
  To: dev
  Cc: bruce.richardson, mb, andrew.rybchenko, huangdaode, liudongdong3,
	fengchengwen, lihuisong

Currently, the file prefix for DPDK runtime directory is 'rte' which was
fixed in the telemetry client script. The user had to modify the prefix
each time to run this script if the file prefix of application isn't 'rte'.
So this patch adds an optional argument for the file prefix, like,
"./dpdk-telemetry-client.py -f rte_xxx sock_path"

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 usertools/dpdk-telemetry-client.py | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
index 144a8fb5e0..0581ef8a00 100755
--- a/usertools/dpdk-telemetry-client.py
+++ b/usertools/dpdk-telemetry-client.py
@@ -15,6 +15,8 @@
 API_UNREG = "{\"action\":2,\"command\":\"clients\",\"data\":{\"client_path\":\""
 GLOBAL_METRICS_REQ = "{\"action\":0,\"command\":\"global_stat_values\",\"data\":null}"
 DEFAULT_FP = "/var/run/dpdk/default_client"
+DEFAULT_PREFIX = 'rte'
+RUNTIME_SOCKET_NAME = 'telemetry'
 
 class Socket:
 
@@ -36,6 +38,7 @@ class Client:
     def __init__(self): # Creates a client instance
         self.socket = Socket()
         self.file_path = None
+        self.run_path = None
         self.choice = None
         self.unregistered = 0
 
@@ -49,6 +52,10 @@ def __del__(self):
     def getFilepath(self, file_path): # Gets arguments from Command-Line and assigns to instance of client
         self.file_path = file_path
 
+    def setRunpath(self, file_path):
+        self.run_path = os.path.join(get_dpdk_runtime_dir(args.file_prefix),
+                                     RUNTIME_SOCKET_NAME)
+
     def register(self): # Connects a client to DPDK-instance
         if os.path.exists(self.file_path):
             os.unlink(self.file_path)
@@ -57,7 +64,7 @@ def register(self): # Connects a client to DPDK-instance
         except socket.error as msg:
             print ("Error - Socket binding error: " + str(msg) + "\n")
         self.socket.recv_fd.settimeout(2)
-        self.socket.send_fd.connect("/var/run/dpdk/rte/telemetry")
+        self.socket.send_fd.connect(self.run_path)
         JSON = (API_REG + self.file_path + "\"}}")
         self.socket.send_fd.sendall(JSON.encode())
 
@@ -113,15 +120,31 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
             except:
                 pass
 
+
+def get_dpdk_runtime_dir(fp):
+    """ Using the same logic as in DPDK's EAL, get the DPDK runtime directory
+    based on the file-prefix and user """
+    run_dir = os.environ.get('RUNTIME_DIRECTORY')
+    if not run_dir:
+        if (os.getuid() == 0):
+            run_dir = '/var/run'
+        else:
+            run_dir = os.environ.get('XDG_RUNTIME_DIR', '/tmp')
+    return os.path.join(run_dir, 'dpdk', fp)
+
+
 if __name__ == "__main__":
 
     sleep_time = 1
     parser = argparse.ArgumentParser()
+    parser.add_argument('-f', '--file-prefix', default=DEFAULT_PREFIX,
+                        help='Provide file-prefix for DPDK runtime directory')
     parser.add_argument('sock_path', nargs='?', default=DEFAULT_FP,
                         help='Provide socket file path connected by legacy client')
     args = parser.parse_args()
 
     client = Client()
     client.getFilepath(args.sock_path)
+    client.setRunpath(args.file_prefix)
     client.register()
     client.interactiveMenu(sleep_time)
-- 
2.22.0


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

* Re: [PATCH v2 1/2] usertools: use argparse module to get input parameter
  2023-01-10  7:31   ` [PATCH v2 1/2] usertools: use argparse module to get input parameter Huisong Li
@ 2023-01-10  9:12     ` Bruce Richardson
  0 siblings, 0 replies; 15+ messages in thread
From: Bruce Richardson @ 2023-01-10  9:12 UTC (permalink / raw)
  To: Huisong Li
  Cc: dev, mb, andrew.rybchenko, huangdaode, liudongdong3, fengchengwen

On Tue, Jan 10, 2023 at 03:31:45PM +0800, Huisong Li wrote:
> The telemetry client script uses argparse module to get input parameter.
> argparse uses an optional positional arguments for local socket path to
> keep backward compatibility.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

* Re: [PATCH] usertools: fix python warnings
  2023-01-09 17:07 ` [PATCH] usertools: fix python warnings Stephen Hemminger
@ 2023-02-06  7:47   ` Thomas Monjalon
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Monjalon @ 2023-02-06  7:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, bruce.richardson

09/01/2023 18:07, Stephen Hemminger:
> This fixes most of the flake8 style warnings on the telemetry
> script.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied, thanks.




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

* Re: [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument
  2023-01-10  7:31 ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument Huisong Li
  2023-01-10  7:31   ` [PATCH v2 1/2] usertools: use argparse module to get input parameter Huisong Li
  2023-01-10  7:31   ` [PATCH v2 2/2] usertools: add an argument for file prefix Huisong Li
@ 2023-02-06  8:30   ` Thomas Monjalon
  2 siblings, 0 replies; 15+ messages in thread
From: Thomas Monjalon @ 2023-02-06  8:30 UTC (permalink / raw)
  To: Huisong Li
  Cc: dev, bruce.richardson, mb, andrew.rybchenko, huangdaode,
	liudongdong3, fengchengwen

> Huisong Li (2):
>   usertools: use argparse module to get input parameter
>   usertools: add an argument for file prefix

Applied, thanks.




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

end of thread, other threads:[~2023-02-06  8:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-09  6:55 [PATCH 0/2] usertools: use argparse module to get input and add an argment Huisong Li
2023-01-09  6:55 ` [PATCH 1/2] usertools: use argparse module to get input parameter Huisong Li
2023-01-09  9:14   ` Bruce Richardson
2023-01-09 12:26     ` lihuisong (C)
2023-01-09 14:32       ` Bruce Richardson
2023-01-10  6:24         ` lihuisong (C)
2023-01-09  6:55 ` [PATCH 2/2] usertools: add an argment for file prefix Huisong Li
2023-01-09  9:16   ` Bruce Richardson
2023-01-09 17:07 ` [PATCH] usertools: fix python warnings Stephen Hemminger
2023-02-06  7:47   ` Thomas Monjalon
2023-01-10  7:31 ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument Huisong Li
2023-01-10  7:31   ` [PATCH v2 1/2] usertools: use argparse module to get input parameter Huisong Li
2023-01-10  9:12     ` Bruce Richardson
2023-01-10  7:31   ` [PATCH v2 2/2] usertools: add an argument for file prefix Huisong Li
2023-02-06  8:30   ` [PATCH v2 0/2] usertools: usertools: use argparse to get input and add an argument 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).