[dpdk-dev,v1] app/testpmd: support command echo in CLI batch loading

Message ID 20171226142555.156122-1-xuemingl@mellanox.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Xueming Li Dec. 26, 2017, 2:25 p.m. UTC
  Use first bit of verbose_level to enable CLI echo of batch loading.

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 app/test-pmd/cmdline.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
  

Comments

Wenzhuo Lu Jan. 10, 2018, 8:35 a.m. UTC | #1
Hi Xueming,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xueming Li
> Sent: Tuesday, December 26, 2017 10:26 PM
> Cc: Xueming Li <xuemingl@mellanox.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ
> <olivier.matz@6wind.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [dpdk-dev] [PATCH v1] app/testpmd: support command echo in CLI
> batch loading
> 
> Use first bit of verbose_level to enable CLI echo of batch loading.
After this patch, the first bit of verbose_level is ambiguous. It can still enable/disable the log print. 
Is it by design?
  
Xueming Li Jan. 10, 2018, 8:51 a.m. UTC | #2
> -----Original Message-----

> From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]

> Sent: Wednesday, January 10, 2018 4:36 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> <olivier.matz@6wind.com>; Burakov, Anatoly <anatoly.burakov@intel.com>

> Subject: RE: [dpdk-dev] [PATCH v1] app/testpmd: support command echo in

> CLI batch loading

> 

> Hi Xueming,

> 

> > -----Original Message-----

> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xueming Li

> > Sent: Tuesday, December 26, 2017 10:26 PM

> > Cc: Xueming Li <xuemingl@mellanox.com>; Wu, Jingjing

> > <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> > <olivier.matz@6wind.com>; Burakov, Anatoly <anatoly.burakov@intel.com>

> > Subject: [dpdk-dev] [PATCH v1] app/testpmd: support command echo in

> > CLI batch loading

> >

> > Use first bit of verbose_level to enable CLI echo of batch loading.

> After this patch, the first bit of verbose_level is ambiguous. It can

> still enable/disable the log print.

> Is it by design?

You are correct, there are some code in testpmd simply testing verbose>0.
How about changing all the test to: verbose & 1? I have another patchset
using other bits of verbose...
  
Wenzhuo Lu Jan. 10, 2018, 12:25 p.m. UTC | #3
Hi Xueming,

> -----Original Message-----

> From: Xueming(Steven) Li [mailto:xuemingl@mellanox.com]

> Sent: Wednesday, January 10, 2018 4:52 PM

> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>

> Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> <olivier.matz@6wind.com>; Burakov, Anatoly <anatoly.burakov@intel.com>

> Subject: RE: [dpdk-dev] [PATCH v1] app/testpmd: support command echo in

> CLI batch loading

> 

> 

> > -----Original Message-----

> > From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]

> > Sent: Wednesday, January 10, 2018 4:36 PM

> > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> > <olivier.matz@6wind.com>; Burakov, Anatoly <anatoly.burakov@intel.com>

> > Subject: RE: [dpdk-dev] [PATCH v1] app/testpmd: support command echo

> > in CLI batch loading

> >

> > Hi Xueming,

> >

> > > -----Original Message-----

> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xueming Li

> > > Sent: Tuesday, December 26, 2017 10:26 PM

> > > Cc: Xueming Li <xuemingl@mellanox.com>; Wu, Jingjing

> > > <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> > > <olivier.matz@6wind.com>; Burakov, Anatoly

> > > <anatoly.burakov@intel.com>

> > > Subject: [dpdk-dev] [PATCH v1] app/testpmd: support command echo in

> > > CLI batch loading

> > >

> > > Use first bit of verbose_level to enable CLI echo of batch loading.

> > After this patch, the first bit of verbose_level is ambiguous. It can

> > still enable/disable the log print.

> > Is it by design?

> You are correct, there are some code in testpmd simply testing verbose>0.

> How about changing all the test to: verbose & 1? I have another patchset

> using other bits of verbose...

I don't object to give  more meaning to verbose. Agree with "verbose & 1". As you said, now only 0 and 1 has meaning. 
Don't forget to comment this variable carefully. And also update the doc 'testpmd_funcs.rst' to let the users know how to use the new functions of this CLI 'set verbose (level)'.
  
Xueming Li Jan. 10, 2018, 2:14 p.m. UTC | #4
Hi Wenzhuo, 

Thanks for your suggestion, will update later.

Regards,
Xueming

> -----Original Message-----

> From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]

> Sent: Wednesday, January 10, 2018 8:26 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> <olivier.matz@6wind.com>; Burakov, Anatoly <anatoly.burakov@intel.com>

> Subject: RE: [dpdk-dev] [PATCH v1] app/testpmd: support command echo in

> CLI batch loading

> 

> Hi Xueming,

> 

> > -----Original Message-----

> > From: Xueming(Steven) Li [mailto:xuemingl@mellanox.com]

> > Sent: Wednesday, January 10, 2018 4:52 PM

> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>

> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> > <olivier.matz@6wind.com>; Burakov, Anatoly <anatoly.burakov@intel.com>

> > Subject: RE: [dpdk-dev] [PATCH v1] app/testpmd: support command echo

> > in CLI batch loading

> >

> >

> > > -----Original Message-----

> > > From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]

> > > Sent: Wednesday, January 10, 2018 4:36 PM

> > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> > > <olivier.matz@6wind.com>; Burakov, Anatoly

> > > <anatoly.burakov@intel.com>

> > > Subject: RE: [dpdk-dev] [PATCH v1] app/testpmd: support command echo

> > > in CLI batch loading

> > >

> > > Hi Xueming,

> > >

> > > > -----Original Message-----

> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xueming Li

> > > > Sent: Tuesday, December 26, 2017 10:26 PM

> > > > Cc: Xueming Li <xuemingl@mellanox.com>; Wu, Jingjing

> > > > <jingjing.wu@intel.com>; dev@dpdk.org; Olivier MATZ

> > > > <olivier.matz@6wind.com>; Burakov, Anatoly

> > > > <anatoly.burakov@intel.com>

> > > > Subject: [dpdk-dev] [PATCH v1] app/testpmd: support command echo

> > > > in CLI batch loading

> > > >

> > > > Use first bit of verbose_level to enable CLI echo of batch loading.

> > > After this patch, the first bit of verbose_level is ambiguous. It

> > > can still enable/disable the log print.

> > > Is it by design?

> > You are correct, there are some code in testpmd simply testing verbose>0.

> > How about changing all the test to: verbose & 1? I have another

> > patchset using other bits of verbose...

> I don't object to give  more meaning to verbose. Agree with "verbose & 1".

> As you said, now only 0 and 1 has meaning.

> Don't forget to comment this variable carefully. And also update the doc

> 'testpmd_funcs.rst' to let the users know how to use the new functions of

> this CLI 'set verbose (level)'.
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f71d96301..0c58dea7b 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -40,6 +40,7 @@ 
 #include <termios.h>
 #include <unistd.h>
 #include <inttypes.h>
+#include <fcntl.h>
 #ifndef __linux__
 #ifndef __FreeBSD__
 #include <net/socket.h>
@@ -15776,9 +15777,19 @@  cmdline_parse_ctx_t main_ctx[] = {
 void
 cmdline_read_from_file(const char *filename)
 {
+	int fd;
 	struct cmdline *cl;
 
-	cl = cmdline_file_new(main_ctx, "testpmd> ", filename);
+	if (!filename)
+		return;
+	fd = open(filename, O_RDONLY, 0);
+	if (fd < 0) {
+		printf("File open() failed\n");
+		return;
+	}
+
+	cl = cmdline_new(main_ctx, "testpmd> ", fd,
+			 verbose_level & 0x8000 ? STDOUT_FILENO : -1);
 	if (cl == NULL) {
 		printf("Failed to create file based cmdline context: %s\n",
 		       filename);