[dpdk-dev] [PATCH] Add external parser support for unknown commands.

[dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Keith Wiles]

Allow for a external parser to handle the command line if the
command is not found and the developer has called the routine
int cmdline_set_external_parser(struct cmdline * cl,
                                cmdline_external_parser_t parser);
function to set the function pointer.

The function for the external parser function should return CMDLINE_PARSE_NOMATCH
if not able to match the command requested or zero is handled.

Prototype of external routine:
int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);

Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
---
 lib/librte_cmdline/cmdline.h       |  4 ++++
 lib/librte_cmdline/cmdline_parse.c | 12 +++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
index 4c28d37..f15d996 100644
--- a/lib/librte_cmdline/cmdline.h
+++ b/lib/librte_cmdline/cmdline.h
@@ -65,6 +65,8 @@
 extern "C" {
 #endif
 
+typedef int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buf);
+
 struct cmdline {
 	int s_in;
 	int s_out;
@@ -74,6 +76,7 @@ struct cmdline {
 #ifdef RTE_EXEC_ENV_LINUXAPP
 	struct termios oldterm;
 #endif
+	cmdline_external_parser_t	external_parser;
 };
 
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
@@ -85,6 +88,7 @@ int cmdline_in(struct cmdline *cl, const char *buf, int size);
 int cmdline_write_char(struct rdline *rdl, char c);
 void cmdline_interact(struct cmdline *cl);
 void cmdline_quit(struct cmdline *cl);
+void cmdline_set_external_parser(struct cmdline * cl, cmdline_external_parser_t parser);
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index 940480d..a65ae70 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -212,6 +212,14 @@ match_inst(cmdline_parse_inst_t *inst, const char *buf,
 	return i;
 }
 
+/* Set or disable external parser */
+void
+cmdline_set_external_parser(struct cmdline *cl, cmdline_external_parser_t parser)
+{
+	/* If parser is NULL it allows for disabling external parser */
+	if ( cl )
+		cl->external_parser = parser;
+}
 
 int
 cmdline_parse(struct cmdline *cl, const char * buf)
@@ -320,7 +328,9 @@ cmdline_parse(struct cmdline *cl, const char * buf)
 	/* no match */
 	else {
 		debug_printf("No match err=%d\n", err);
-		return err;
+		if ( cl->external_parser == NULL )
+			return err;
+		return cl->external_parser(cl, buf);
 	}
 
 	return linelen;
-- 
2.1.0

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Bruce Richardson]

On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
> Allow for a external parser to handle the command line if the
> command is not found and the developer has called the routine
> int cmdline_set_external_parser(struct cmdline * cl,
>                                 cmdline_external_parser_t parser);
> function to set the function pointer.
> 
> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
> if not able to match the command requested or zero is handled.
> 
> Prototype of external routine:
> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
> 
> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>

Hi Keith,

what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]

/Bruce

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Wiles, Roger Keith]

> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
>> Allow for a external parser to handle the command line if the
>> command is not found and the developer has called the routine
>> int cmdline_set_external_parser(struct cmdline * cl,
>>                                cmdline_external_parser_t parser);
>> function to set the function pointer.
>> 
>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
>> if not able to match the command requested or zero is handled.
>> 
>> Prototype of external routine:
>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
>> 
>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
> 
> Hi Keith,
> 
> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]

Hi Bruce,

I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).

One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.

For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.

Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.

Does this help and do I really need to add all of this to the commit message :-)
> 
> /Bruce

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Bruce Richardson]

On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
> 
> > On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >
> > On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
> >> Allow for a external parser to handle the command line if the
> >> command is not found and the developer has called the routine
> >> int cmdline_set_external_parser(struct cmdline * cl,
> >>                                cmdline_external_parser_t parser);
> >> function to set the function pointer.
> >>
> >> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
> >> if not able to match the command requested or zero is handled.
> >>
> >> Prototype of external routine:
> >> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
> >>
> >> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
> >
> > Hi Keith,
> >
> > what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
> 
> Hi Bruce,
> 
> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
> 
> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
> 
> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
> 
> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
> 
> Does this help and do I really need to add all of this to the commit message :-)
>
Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?

/Bruce

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Wiles, Roger Keith]

> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
>> 
>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>> 
>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
>>>> Allow for a external parser to handle the command line if the
>>>> command is not found and the developer has called the routine
>>>> int cmdline_set_external_parser(struct cmdline * cl,
>>>>                               cmdline_external_parser_t parser);
>>>> function to set the function pointer.
>>>> 
>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
>>>> if not able to match the command requested or zero is handled.
>>>> 
>>>> Prototype of external routine:
>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
>>>> 
>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
>>> 
>>> Hi Keith,
>>> 
>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
>> 
>> Hi Bruce,
>> 
>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
>> 
>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
>> 
>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
>> 
>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
>> 
>> Does this help and do I really need to add all of this to the commit message :-)
>> 
> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?

I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.

The default case is the same behavior today, with giving a no match error unless they add the external parser.
> 
> /Bruce

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Neil Horman]

On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
> 
> > On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> > 
> > On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
> >> 
> >>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>> 
> >>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
> >>>> Allow for a external parser to handle the command line if the
> >>>> command is not found and the developer has called the routine
> >>>> int cmdline_set_external_parser(struct cmdline * cl,
> >>>>                               cmdline_external_parser_t parser);
> >>>> function to set the function pointer.
> >>>> 
> >>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
> >>>> if not able to match the command requested or zero is handled.
> >>>> 
> >>>> Prototype of external routine:
> >>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
> >>>> 
> >>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
> >>> 
> >>> Hi Keith,
> >>> 
> >>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
> >> 
> >> Hi Bruce,
> >> 
> >> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
> >> 
> >> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
> >> 
> >> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
> >> 
> >> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
> >> 
> >> Does this help and do I really need to add all of this to the commit message :-)
> >> 
> > Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
> 
> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
> 
> The default case is the same behavior today, with giving a no match error unless they add the external parser.

It seems alot simpler than that to me.  Looking at the test applications, the
command line parser expects the application to create an array of
cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
to support other languages, it seems to make more sense to just use foreign
language bindings to merge your coding language support with the DPDK
(ostensibly you will already have to do that if you want to use other parts of
the DPDK).

Am I missing something?
Neil


> > 
> > /Bruce
> 
> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> 

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Wiles, Roger Keith]

> On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
>> 
>>> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>> 
>>> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
>>>> 
>>>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>> 
>>>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
>>>>>> Allow for a external parser to handle the command line if the
>>>>>> command is not found and the developer has called the routine
>>>>>> int cmdline_set_external_parser(struct cmdline * cl,
>>>>>>                              cmdline_external_parser_t parser);
>>>>>> function to set the function pointer.
>>>>>> 
>>>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
>>>>>> if not able to match the command requested or zero is handled.
>>>>>> 
>>>>>> Prototype of external routine:
>>>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
>>>>>> 
>>>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
>>>>> 
>>>>> Hi Keith,
>>>>> 
>>>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
>>>> 
>>>> Hi Bruce,
>>>> 
>>>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
>>>> 
>>>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
>>>> 
>>>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
>>>> 
>>>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
>>>> 
>>>> Does this help and do I really need to add all of this to the commit message :-)
>>>> 
>>> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
>> 
>> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
>> 
>> The default case is the same behavior today, with giving a no match error unless they add the external parser.
> 
> It seems alot simpler than that to me.  Looking at the test applications, the
> command line parser expects the application to create an array of
> cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
> to support other languages, it seems to make more sense to just use foreign
> language bindings to merge your coding language support with the DPDK
> (ostensibly you will already have to do that if you want to use other parts of
> the DPDK).
Hi Neil,

A true language binding like Lua or one of those other languages :-) you are correct to believe binding directly using ‘C’ code is the right solution . In Pktgen I use Lua as the direct language binding and extend Lua with specific Pktgen functions.

What I am doing here is to add a default case to cmdline code, which just happens to allow me to parse the cmdline in the application. Being able to execute say a line of script code is not really the requirement IMO. Being able to extend the cmdline code with a default case is a good feature and allows the developer to extend cmdline for some simple cases. The cmdline code is kind of simple, but does require a fair amount of structures, code and understanding to write a complex extendable command line interface. It does seem hard to find a clean, simple and usable embedded command line code base is not very easy to locate. 

Adding a true language binding really requires using code to extend the language as I did with Lua and Pktgen. It could have been done with any language I just picked Lua, but the patch does not really add support for a language other then giving some support for someone to handle the no_match case.

The use case for this feature is not just for Pktgen, but another solution I hope everyone will find useful when I get it more complete.

Thanks
++Keith

PS. on a different topic I was thinking about suggesting and writing a patch to add Lua with DPDK specific binding and extensions. (also allowing those `other` languages too :-) Being able to use a scripting language and be able to call DPDK API’s could be useful. How useful not sure at this time. (If you want to talk about this topic please start a new thread).
> 
> Am I missing something?
> Neil
> 
> 
>>> 
>>> /Bruce
>> 
>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Stephen Hemminger]

On Mon, 3 Nov 2014 16:50:15 +0000
"Wiles, Roger Keith" <keith.wiles@windriver.com> wrote:

> 
> > On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
> >> 
> >>> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>> 
> >>> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
> >>>> 
> >>>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>>>> 
> >>>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
> >>>>>> Allow for a external parser to handle the command line if the
> >>>>>> command is not found and the developer has called the routine
> >>>>>> int cmdline_set_external_parser(struct cmdline * cl,
> >>>>>>                              cmdline_external_parser_t parser);
> >>>>>> function to set the function pointer.
> >>>>>> 
> >>>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
> >>>>>> if not able to match the command requested or zero is handled.
> >>>>>> 
> >>>>>> Prototype of external routine:
> >>>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
> >>>>>> 
> >>>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
> >>>>> 
> >>>>> Hi Keith,
> >>>>> 
> >>>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
> >>>> 
> >>>> Hi Bruce,
> >>>> 
> >>>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
> >>>> 
> >>>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
> >>>> 
> >>>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
> >>>> 
> >>>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
> >>>> 
> >>>> Does this help and do I really need to add all of this to the commit message :-)
> >>>> 
> >>> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
> >> 
> >> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
> >> 
> >> The default case is the same behavior today, with giving a no match error unless they add the external parser.
> > 
> > It seems alot simpler than that to me.  Looking at the test applications, the
> > command line parser expects the application to create an array of
> > cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
> > to support other languages, it seems to make more sense to just use foreign
> > language bindings to merge your coding language support with the DPDK
> > (ostensibly you will already have to do that if you want to use other parts of
> > the DPDK).
> Hi Neil,
> 
> A true language binding like Lua or one of those other languages :-) you are correct to believe binding directly using ‘C’ code is the right solution . In Pktgen I use Lua as the direct language binding and extend Lua with specific Pktgen functions.
> 
> What I am doing here is to add a default case to cmdline code, which just happens to allow me to parse the cmdline in the application. Being able to execute say a line of script code is not really the requirement IMO. Being able to extend the cmdline code with a default case is a good feature and allows the developer to extend cmdline for some simple cases. The cmdline code is kind of simple, but does require a fair amount of structures, code and understanding to write a complex extendable command line interface. It does seem hard to find a clean, simple and usable embedded command line code base is not very easy to locate. 
> 
> Adding a true language binding really requires using code to extend the language as I did with Lua and Pktgen. It could have been done with any language I just picked Lua, but the patch does not really add support for a language other then giving some support for someone to handle the no_match case.
> 
> The use case for this feature is not just for Pktgen, but another solution I hope everyone will find useful when I get it more complete.
> 
> Thanks
> ++Keith
> 
> PS. on a different topic I was thinking about suggesting and writing a patch to add Lua with DPDK specific binding and extensions. (also allowing those `other` languages too :-) Being able to use a scripting language and be able to call DPDK API’s could be useful. How useful not sure at this time. (If you want to talk about this topic please start a new thread).
> > 
> > Am I missing something?
> > Neil
> > 
> > 
> >>> 
> >>> /Bruce
> >> 
> >> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> 
> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

I wouldn't invest a lot of sweat in the command line parser.
The one in the DPDK is "good enough" for what it needs to do, but really isn't
very complete and flexible. Seems like the kind of thing that doesn't really even
need to be in DPDK. Better off being part of some other library.

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Neil Horman]

On Mon, Nov 03, 2014 at 03:26:50PM -0800, Stephen Hemminger wrote:
> On Mon, 3 Nov 2014 16:50:15 +0000
> "Wiles, Roger Keith" <keith.wiles@windriver.com> wrote:
> 
> > 
> > > On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > > 
> > > On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
> > >> 
> > >>> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> > >>> 
> > >>> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
> > >>>> 
> > >>>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> > >>>>> 
> > >>>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
> > >>>>>> Allow for a external parser to handle the command line if the
> > >>>>>> command is not found and the developer has called the routine
> > >>>>>> int cmdline_set_external_parser(struct cmdline * cl,
> > >>>>>>                              cmdline_external_parser_t parser);
> > >>>>>> function to set the function pointer.
> > >>>>>> 
> > >>>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
> > >>>>>> if not able to match the command requested or zero is handled.
> > >>>>>> 
> > >>>>>> Prototype of external routine:
> > >>>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
> > >>>>>> 
> > >>>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
> > >>>>> 
> > >>>>> Hi Keith,
> > >>>>> 
> > >>>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
> > >>>> 
> > >>>> Hi Bruce,
> > >>>> 
> > >>>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
> > >>>> 
> > >>>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
> > >>>> 
> > >>>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
> > >>>> 
> > >>>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
> > >>>> 
> > >>>> Does this help and do I really need to add all of this to the commit message :-)
> > >>>> 
> > >>> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
> > >> 
> > >> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
> > >> 
> > >> The default case is the same behavior today, with giving a no match error unless they add the external parser.
> > > 
> > > It seems alot simpler than that to me.  Looking at the test applications, the
> > > command line parser expects the application to create an array of
> > > cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
> > > to support other languages, it seems to make more sense to just use foreign
> > > language bindings to merge your coding language support with the DPDK
> > > (ostensibly you will already have to do that if you want to use other parts of
> > > the DPDK).
> > Hi Neil,
> > 
> > A true language binding like Lua or one of those other languages :-) you are correct to believe binding directly using ‘C’ code is the right solution . In Pktgen I use Lua as the direct language binding and extend Lua with specific Pktgen functions.
> > 
> > What I am doing here is to add a default case to cmdline code, which just happens to allow me to parse the cmdline in the application. Being able to execute say a line of script code is not really the requirement IMO. Being able to extend the cmdline code with a default case is a good feature and allows the developer to extend cmdline for some simple cases. The cmdline code is kind of simple, but does require a fair amount of structures, code and understanding to write a complex extendable command line interface. It does seem hard to find a clean, simple and usable embedded command line code base is not very easy to locate. 
> > 
> > Adding a true language binding really requires using code to extend the language as I did with Lua and Pktgen. It could have been done with any language I just picked Lua, but the patch does not really add support for a language other then giving some support for someone to handle the no_match case.
> > 
> > The use case for this feature is not just for Pktgen, but another solution I hope everyone will find useful when I get it more complete.
> > 
> > Thanks
> > ++Keith
> > 
> > PS. on a different topic I was thinking about suggesting and writing a patch to add Lua with DPDK specific binding and extensions. (also allowing those `other` languages too :-) Being able to use a scripting language and be able to call DPDK API’s could be useful. How useful not sure at this time. (If you want to talk about this topic please start a new thread).
> > > 
> > > Am I missing something?
> > > Neil
> > > 
> > > 
> > >>> 
> > >>> /Bruce
> > >> 
> > >> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> > 
> > Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> 
> I wouldn't invest a lot of sweat in the command line parser.
> The one in the DPDK is "good enough" for what it needs to do, but really isn't
> very complete and flexible. Seems like the kind of thing that doesn't really even
> need to be in DPDK. Better off being part of some other library.
> 
Well, something needs to be there to parse the libraries' common options, though
I agree, making eal_cmdline just a registration frontend to getopt or
getopt_long would be sufficient.

Neil

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Wiles, Roger Keith]

> On Nov 3, 2014, at 5:42 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Mon, Nov 03, 2014 at 03:26:50PM -0800, Stephen Hemminger wrote:
>> On Mon, 3 Nov 2014 16:50:15 +0000
>> "Wiles, Roger Keith" <keith.wiles@windriver.com> wrote:
>> 
>>> 
>>>> On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
>>>> 
>>>> On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
>>>>> 
>>>>>> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>>> 
>>>>>> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
>>>>>>> 
>>>>>>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>>>>> 
>>>>>>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
>>>>>>>>> Allow for a external parser to handle the command line if the
>>>>>>>>> command is not found and the developer has called the routine
>>>>>>>>> int cmdline_set_external_parser(struct cmdline * cl,
>>>>>>>>>                             cmdline_external_parser_t parser);
>>>>>>>>> function to set the function pointer.
>>>>>>>>> 
>>>>>>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
>>>>>>>>> if not able to match the command requested or zero is handled.
>>>>>>>>> 
>>>>>>>>> Prototype of external routine:
>>>>>>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
>>>>>>>>> 
>>>>>>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
>>>>>>>> 
>>>>>>>> Hi Keith,
>>>>>>>> 
>>>>>>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
>>>>>>> 
>>>>>>> Hi Bruce,
>>>>>>> 
>>>>>>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
>>>>>>> 
>>>>>>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
>>>>>>> 
>>>>>>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
>>>>>>> 
>>>>>>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
>>>>>>> 
>>>>>>> Does this help and do I really need to add all of this to the commit message :-)
>>>>>>> 
>>>>>> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
>>>>> 
>>>>> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
>>>>> 
>>>>> The default case is the same behavior today, with giving a no match error unless they add the external parser.
>>>> 
>>>> It seems alot simpler than that to me.  Looking at the test applications, the
>>>> command line parser expects the application to create an array of
>>>> cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
>>>> to support other languages, it seems to make more sense to just use foreign
>>>> language bindings to merge your coding language support with the DPDK
>>>> (ostensibly you will already have to do that if you want to use other parts of
>>>> the DPDK).
>>> Hi Neil,
>>> 
>>> A true language binding like Lua or one of those other languages :-) you are correct to believe binding directly using ‘C’ code is the right solution . In Pktgen I use Lua as the direct language binding and extend Lua with specific Pktgen functions.
>>> 
>>> What I am doing here is to add a default case to cmdline code, which just happens to allow me to parse the cmdline in the application. Being able to execute say a line of script code is not really the requirement IMO. Being able to extend the cmdline code with a default case is a good feature and allows the developer to extend cmdline for some simple cases. The cmdline code is kind of simple, but does require a fair amount of structures, code and understanding to write a complex extendable command line interface. It does seem hard to find a clean, simple and usable embedded command line code base is not very easy to locate. 
>>> 
>>> Adding a true language binding really requires using code to extend the language as I did with Lua and Pktgen. It could have been done with any language I just picked Lua, but the patch does not really add support for a language other then giving some support for someone to handle the no_match case.
>>> 
>>> The use case for this feature is not just for Pktgen, but another solution I hope everyone will find useful when I get it more complete.
>>> 
>>> Thanks
>>> ++Keith
>>> 
>>> PS. on a different topic I was thinking about suggesting and writing a patch to add Lua with DPDK specific binding and extensions. (also allowing those `other` languages too :-) Being able to use a scripting language and be able to call DPDK API’s could be useful. How useful not sure at this time. (If you want to talk about this topic please start a new thread).
>>>> 
>>>> Am I missing something?
>>>> Neil
>>>> 
>>>> 
>>>>>> 
>>>>>> /Bruce
>>>>> 
>>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>>> 
>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>> 
>> I wouldn't invest a lot of sweat in the command line parser.
>> The one in the DPDK is "good enough" for what it needs to do, but really isn't
>> very complete and flexible. Seems like the kind of thing that doesn't really even
>> need to be in DPDK. Better off being part of some other library.
>> 
> Well, something needs to be there to parse the libraries' common options, though
> I agree, making eal_cmdline just a registration frontend to getopt or
> getopt_long would be sufficient.

Until we have a better command line solution, which I think would be great, but in the mean time I would like to see this patch applied if no one has a technical reason or better suggestion.

I think this patch is fairly simple and I think we need a way to handle the default case. If someone could please review the patch, that would be great.

++Keith 

> 
> Neil
> 

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Neil Horman]

On Tue, Nov 04, 2014 at 04:52:48AM +0000, Wiles, Roger Keith wrote:
> 
> > On Nov 3, 2014, at 5:42 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > On Mon, Nov 03, 2014 at 03:26:50PM -0800, Stephen Hemminger wrote:
> >> On Mon, 3 Nov 2014 16:50:15 +0000
> >> "Wiles, Roger Keith" <keith.wiles@windriver.com> wrote:
> >> 
> >>> 
> >>>> On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >>>> 
> >>>> On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
> >>>>> 
> >>>>>> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>>>>> 
> >>>>>> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
> >>>>>>> 
> >>>>>>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>>>>>>> 
> >>>>>>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
> >>>>>>>>> Allow for a external parser to handle the command line if the
> >>>>>>>>> command is not found and the developer has called the routine
> >>>>>>>>> int cmdline_set_external_parser(struct cmdline * cl,
> >>>>>>>>>                             cmdline_external_parser_t parser);
> >>>>>>>>> function to set the function pointer.
> >>>>>>>>> 
> >>>>>>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
> >>>>>>>>> if not able to match the command requested or zero is handled.
> >>>>>>>>> 
> >>>>>>>>> Prototype of external routine:
> >>>>>>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
> >>>>>>>>> 
> >>>>>>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
> >>>>>>>> 
> >>>>>>>> Hi Keith,
> >>>>>>>> 
> >>>>>>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
> >>>>>>> 
> >>>>>>> Hi Bruce,
> >>>>>>> 
> >>>>>>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
> >>>>>>> 
> >>>>>>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
> >>>>>>> 
> >>>>>>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
> >>>>>>> 
> >>>>>>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
> >>>>>>> 
> >>>>>>> Does this help and do I really need to add all of this to the commit message :-)
> >>>>>>> 
> >>>>>> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
> >>>>> 
> >>>>> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
> >>>>> 
> >>>>> The default case is the same behavior today, with giving a no match error unless they add the external parser.
> >>>> 
> >>>> It seems alot simpler than that to me.  Looking at the test applications, the
> >>>> command line parser expects the application to create an array of
> >>>> cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
> >>>> to support other languages, it seems to make more sense to just use foreign
> >>>> language bindings to merge your coding language support with the DPDK
> >>>> (ostensibly you will already have to do that if you want to use other parts of
> >>>> the DPDK).
> >>> Hi Neil,
> >>> 
> >>> A true language binding like Lua or one of those other languages :-) you are correct to believe binding directly using ‘C’ code is the right solution . In Pktgen I use Lua as the direct language binding and extend Lua with specific Pktgen functions.
> >>> 
> >>> What I am doing here is to add a default case to cmdline code, which just happens to allow me to parse the cmdline in the application. Being able to execute say a line of script code is not really the requirement IMO. Being able to extend the cmdline code with a default case is a good feature and allows the developer to extend cmdline for some simple cases. The cmdline code is kind of simple, but does require a fair amount of structures, code and understanding to write a complex extendable command line interface. It does seem hard to find a clean, simple and usable embedded command line code base is not very easy to locate. 
> >>> 
> >>> Adding a true language binding really requires using code to extend the language as I did with Lua and Pktgen. It could have been done with any language I just picked Lua, but the patch does not really add support for a language other then giving some support for someone to handle the no_match case.
> >>> 
> >>> The use case for this feature is not just for Pktgen, but another solution I hope everyone will find useful when I get it more complete.
> >>> 
> >>> Thanks
> >>> ++Keith
> >>> 
> >>> PS. on a different topic I was thinking about suggesting and writing a patch to add Lua with DPDK specific binding and extensions. (also allowing those `other` languages too :-) Being able to use a scripting language and be able to call DPDK API’s could be useful. How useful not sure at this time. (If you want to talk about this topic please start a new thread).
> >>>> 
> >>>> Am I missing something?
> >>>> Neil
> >>>> 
> >>>> 
> >>>>>> 
> >>>>>> /Bruce
> >>>>> 
> >>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> >>> 
> >>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> >> 
> >> I wouldn't invest a lot of sweat in the command line parser.
> >> The one in the DPDK is "good enough" for what it needs to do, but really isn't
> >> very complete and flexible. Seems like the kind of thing that doesn't really even
> >> need to be in DPDK. Better off being part of some other library.
> >> 
> > Well, something needs to be there to parse the libraries' common options, though
> > I agree, making eal_cmdline just a registration frontend to getopt or
> > getopt_long would be sufficient.
> 
> Until we have a better command line solution, which I think would be great, but in the mean time I would like to see this patch applied if no one has a technical reason or better suggestion.
> 
> I think this patch is fairly simple and I think we need a way to handle the default case. If someone could please review the patch, that would be great.
> 
I have an objection, specifically, that its not necessecary.  You can already
accomplish what you want to do by adding structures to the context array in the
cmdline structure.  I realize its not as easy as just adding an external parser
function, but its the designed way to add options.  This does little more than
add addition API surface without any real need.
Neil


> ++Keith 
> 
> > 
> > Neil
> > 
> 
> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> 

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Wiles, Roger Keith]

> On Nov 4, 2014, at 5:27 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Tue, Nov 04, 2014 at 04:52:48AM +0000, Wiles, Roger Keith wrote:
>> 
>>> On Nov 3, 2014, at 5:42 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
>>> 
>>> On Mon, Nov 03, 2014 at 03:26:50PM -0800, Stephen Hemminger wrote:
>>>> On Mon, 3 Nov 2014 16:50:15 +0000
>>>> "Wiles, Roger Keith" <keith.wiles@windriver.com> wrote:
>>>> 
>>>>> 
>>>>>> On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
>>>>>> 
>>>>>> On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
>>>>>>> 
>>>>>>>> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>>>>> 
>>>>>>>> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
>>>>>>>>> 
>>>>>>>>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>>>>>>> 
>>>>>>>>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
>>>>>>>>>>> Allow for a external parser to handle the command line if the
>>>>>>>>>>> command is not found and the developer has called the routine
>>>>>>>>>>> int cmdline_set_external_parser(struct cmdline * cl,
>>>>>>>>>>>                            cmdline_external_parser_t parser);
>>>>>>>>>>> function to set the function pointer.
>>>>>>>>>>> 
>>>>>>>>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
>>>>>>>>>>> if not able to match the command requested or zero is handled.
>>>>>>>>>>> 
>>>>>>>>>>> Prototype of external routine:
>>>>>>>>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
>>>>>>>>>>> 
>>>>>>>>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
>>>>>>>>>> 
>>>>>>>>>> Hi Keith,
>>>>>>>>>> 
>>>>>>>>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
>>>>>>>>> 
>>>>>>>>> Hi Bruce,
>>>>>>>>> 
>>>>>>>>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
>>>>>>>>> 
>>>>>>>>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
>>>>>>>>> 
>>>>>>>>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
>>>>>>>>> 
>>>>>>>>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
>>>>>>>>> 
>>>>>>>>> Does this help and do I really need to add all of this to the commit message :-)
>>>>>>>>> 
>>>>>>>> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
>>>>>>> 
>>>>>>> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
>>>>>>> 
>>>>>>> The default case is the same behavior today, with giving a no match error unless they add the external parser.
>>>>>> 
>>>>>> It seems alot simpler than that to me.  Looking at the test applications, the
>>>>>> command line parser expects the application to create an array of
>>>>>> cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
>>>>>> to support other languages, it seems to make more sense to just use foreign
>>>>>> language bindings to merge your coding language support with the DPDK
>>>>>> (ostensibly you will already have to do that if you want to use other parts of
>>>>>> the DPDK).
>>>>> Hi Neil,
>>>>> 
>>>>> A true language binding like Lua or one of those other languages :-) you are correct to believe binding directly using ‘C’ code is the right solution . In Pktgen I use Lua as the direct language binding and extend Lua with specific Pktgen functions.
>>>>> 
>>>>> What I am doing here is to add a default case to cmdline code, which just happens to allow me to parse the cmdline in the application. Being able to execute say a line of script code is not really the requirement IMO. Being able to extend the cmdline code with a default case is a good feature and allows the developer to extend cmdline for some simple cases. The cmdline code is kind of simple, but does require a fair amount of structures, code and understanding to write a complex extendable command line interface. It does seem hard to find a clean, simple and usable embedded command line code base is not very easy to locate. 
>>>>> 
>>>>> Adding a true language binding really requires using code to extend the language as I did with Lua and Pktgen. It could have been done with any language I just picked Lua, but the patch does not really add support for a language other then giving some support for someone to handle the no_match case.
>>>>> 
>>>>> The use case for this feature is not just for Pktgen, but another solution I hope everyone will find useful when I get it more complete.
>>>>> 
>>>>> Thanks
>>>>> ++Keith
>>>>> 
>>>>> PS. on a different topic I was thinking about suggesting and writing a patch to add Lua with DPDK specific binding and extensions. (also allowing those `other` languages too :-) Being able to use a scripting language and be able to call DPDK API’s could be useful. How useful not sure at this time. (If you want to talk about this topic please start a new thread).
>>>>>> 
>>>>>> Am I missing something?
>>>>>> Neil
>>>>>> 
>>>>>> 
>>>>>>>> 
>>>>>>>> /Bruce
>>>>>>> 
>>>>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>>>>> 
>>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>>>> 
>>>> I wouldn't invest a lot of sweat in the command line parser.
>>>> The one in the DPDK is "good enough" for what it needs to do, but really isn't
>>>> very complete and flexible. Seems like the kind of thing that doesn't really even
>>>> need to be in DPDK. Better off being part of some other library.
>>>> 
>>> Well, something needs to be there to parse the libraries' common options, though
>>> I agree, making eal_cmdline just a registration frontend to getopt or
>>> getopt_long would be sufficient.
>> 
>> Until we have a better command line solution, which I think would be great, but in the mean time I would like to see this patch applied if no one has a technical reason or better suggestion.
>> 
>> I think this patch is fairly simple and I think we need a way to handle the default case. If someone could please review the patch, that would be great.
>> 
> I have an objection, specifically, that its not necessecary.  You can already
> accomplish what you want to do by adding structures to the context array in the
> cmdline structure.  I realize its not as easy as just adding an external parser
> function, but its the designed way to add options.  This does little more than
> add addition API surface without any real need.
> Neil

Neil

I do not agree with your comments as I see it to be a small extension to cmdline to handle the case where I will have to possibly add a huge number of commands/code to the cmdline structures. Using this method I am able to add these very simple commands without having to add more code for this use case.

Lets say you have a directory on the disk that has possibly a 100 little commands, without this minor change I would have to write 100 little structures/code for cmdline to handle each case. Another option is to write a single command to handle these commands. I used this method in Pktgen and could do ‘run foo <args>’ style commands, but it would be much simpler for the user to just type ‘foo <args>’ instead.

Having a default handler for commands just makes a lot of sense to me and I do not buy the 'added API surface without any real need' statement.

Thanks
++Keith
> 
> 
>> ++Keith 
>> 
>>> 
>>> Neil
>>> 
>> 
>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Neil Horman]

On Tue, Nov 04, 2014 at 02:44:39PM +0000, Wiles, Roger Keith wrote:
> 
> > On Nov 4, 2014, at 5:27 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > On Tue, Nov 04, 2014 at 04:52:48AM +0000, Wiles, Roger Keith wrote:
> >> 
> >>> On Nov 3, 2014, at 5:42 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >>> 
> >>> On Mon, Nov 03, 2014 at 03:26:50PM -0800, Stephen Hemminger wrote:
> >>>> On Mon, 3 Nov 2014 16:50:15 +0000
> >>>> "Wiles, Roger Keith" <keith.wiles@windriver.com> wrote:
> >>>> 
> >>>>> 
> >>>>>> On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >>>>>> 
> >>>>>> On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
> >>>>>>> 
> >>>>>>>> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>>>>>>> 
> >>>>>>>> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
> >>>>>>>>> 
> >>>>>>>>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>>>>>>>>> 
> >>>>>>>>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
> >>>>>>>>>>> Allow for a external parser to handle the command line if the
> >>>>>>>>>>> command is not found and the developer has called the routine
> >>>>>>>>>>> int cmdline_set_external_parser(struct cmdline * cl,
> >>>>>>>>>>>                            cmdline_external_parser_t parser);
> >>>>>>>>>>> function to set the function pointer.
> >>>>>>>>>>> 
> >>>>>>>>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
> >>>>>>>>>>> if not able to match the command requested or zero is handled.
> >>>>>>>>>>> 
> >>>>>>>>>>> Prototype of external routine:
> >>>>>>>>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
> >>>>>>>>>>> 
> >>>>>>>>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
> >>>>>>>>>> 
> >>>>>>>>>> Hi Keith,
> >>>>>>>>>> 
> >>>>>>>>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
> >>>>>>>>> 
> >>>>>>>>> Hi Bruce,
> >>>>>>>>> 
> >>>>>>>>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
> >>>>>>>>> 
> >>>>>>>>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
> >>>>>>>>> 
> >>>>>>>>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
> >>>>>>>>> 
> >>>>>>>>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
> >>>>>>>>> 
> >>>>>>>>> Does this help and do I really need to add all of this to the commit message :-)
> >>>>>>>>> 
> >>>>>>>> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
> >>>>>>> 
> >>>>>>> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
> >>>>>>> 
> >>>>>>> The default case is the same behavior today, with giving a no match error unless they add the external parser.
> >>>>>> 
> >>>>>> It seems alot simpler than that to me.  Looking at the test applications, the
> >>>>>> command line parser expects the application to create an array of
> >>>>>> cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
> >>>>>> to support other languages, it seems to make more sense to just use foreign
> >>>>>> language bindings to merge your coding language support with the DPDK
> >>>>>> (ostensibly you will already have to do that if you want to use other parts of
> >>>>>> the DPDK).
> >>>>> Hi Neil,
> >>>>> 
> >>>>> A true language binding like Lua or one of those other languages :-) you are correct to believe binding directly using ‘C’ code is the right solution . In Pktgen I use Lua as the direct language binding and extend Lua with specific Pktgen functions.
> >>>>> 
> >>>>> What I am doing here is to add a default case to cmdline code, which just happens to allow me to parse the cmdline in the application. Being able to execute say a line of script code is not really the requirement IMO. Being able to extend the cmdline code with a default case is a good feature and allows the developer to extend cmdline for some simple cases. The cmdline code is kind of simple, but does require a fair amount of structures, code and understanding to write a complex extendable command line interface. It does seem hard to find a clean, simple and usable embedded command line code base is not very easy to locate. 
> >>>>> 
> >>>>> Adding a true language binding really requires using code to extend the language as I did with Lua and Pktgen. It could have been done with any language I just picked Lua, but the patch does not really add support for a language other then giving some support for someone to handle the no_match case.
> >>>>> 
> >>>>> The use case for this feature is not just for Pktgen, but another solution I hope everyone will find useful when I get it more complete.
> >>>>> 
> >>>>> Thanks
> >>>>> ++Keith
> >>>>> 
> >>>>> PS. on a different topic I was thinking about suggesting and writing a patch to add Lua with DPDK specific binding and extensions. (also allowing those `other` languages too :-) Being able to use a scripting language and be able to call DPDK API’s could be useful. How useful not sure at this time. (If you want to talk about this topic please start a new thread).
> >>>>>> 
> >>>>>> Am I missing something?
> >>>>>> Neil
> >>>>>> 
> >>>>>> 
> >>>>>>>> 
> >>>>>>>> /Bruce
> >>>>>>> 
> >>>>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> >>>>> 
> >>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> >>>> 
> >>>> I wouldn't invest a lot of sweat in the command line parser.
> >>>> The one in the DPDK is "good enough" for what it needs to do, but really isn't
> >>>> very complete and flexible. Seems like the kind of thing that doesn't really even
> >>>> need to be in DPDK. Better off being part of some other library.
> >>>> 
> >>> Well, something needs to be there to parse the libraries' common options, though
> >>> I agree, making eal_cmdline just a registration frontend to getopt or
> >>> getopt_long would be sufficient.
> >> 
> >> Until we have a better command line solution, which I think would be great, but in the mean time I would like to see this patch applied if no one has a technical reason or better suggestion.
> >> 
> >> I think this patch is fairly simple and I think we need a way to handle the default case. If someone could please review the patch, that would be great.
> >> 
> > I have an objection, specifically, that its not necessecary.  You can already
> > accomplish what you want to do by adding structures to the context array in the
> > cmdline structure.  I realize its not as easy as just adding an external parser
> > function, but its the designed way to add options.  This does little more than
> > add addition API surface without any real need.
> > Neil
> 
> Neil
> 
> I do not agree with your comments as I see it to be a small extension to cmdline to handle the case where I will have to possibly add a huge number of commands/code to the cmdline structures. Using this method I am able to add these very simple commands without having to add more code for this use case.
> 
Can you provide a real example here?  usnig vague terms like "huge" really makes
more of an emotional argument than a factual one.  To cite an example the
cmdline_test program adds a command line paramter that just lets you parse a
number out of the command line.  Silly, granted, but it serves the purpose.  Its
called cmd_num, and with functions and data all told, it looks like it takes
about 17 lines of code.  Now thats more than what you're adding with you patch,
I grant you, but I assert that the "potentially huge" argument you're making
above is false, especially whan you consider that some reasonably clever coding
can likely allow you to reuse function parsing fairly easily.

> Lets say you have a directory on the disk that has possibly a 100 little commands, without this minor change I would have to write 100 little structures/code for cmdline to handle each case. Another option is to write a single command to handle these commands. I used this method in Pktgen and could do ‘run foo <args>’ style commands, but it would be much simpler for the user to just type ‘foo <args>’ instead.
> 
Yes, but thats honestly true of every command line parser, something needs to
define the tokens that identify the option, the function to handle its
interpretation and the structures to glue it all together.  That can be in the
DPDK, or in something else.  To use your example above, one presumes that your
100 little commands all have some sort of parsing structure coded elsewhere,
along with the code to do that parsing, right? You don't seem to take that
additional code into account here.  If you can remove that parsing code from
your application, then it seems to me the additional 17 lines above really isn't
that big a deal.

It seems to me that, what this boils down to is the fact that you have an
application that needs to parse a single command line with two different parser,
whcih is just kind of a lousy situation, because parsers all assume that they
are the only thing parsing the command line.  Honestly, I really question the
need for a command line parser so tightly integrated with DPDK at all.
rte_eal_init really shouldn't be acepting a straight command line buffer.  As a
library, it should more likely accept a configuration structure which it just
doles out to individual components during initalization.  That can leave
applications like yours to handle command line parsing 100% as you see fit, and
then build the DPDK configuration from that as you like.

> Having a default handler for commands just makes a lot of sense to me and I do not buy the 'added API surface without any real need' statement.
I'm not sure whats not to buy there.  I see the above as facts:

1) You're adding API surface (you added a function that is exported from the
library)

2) You don't need to do (1) (theres an already existing alternate method to do
what you want)

You can argue all day that your method is better, but it doesn't change the fact
that (1) and (2) are true. And I don't want to go adding additional methods to
the existing ones as there will be a need to support them in the future, and as
such, If we're going to start deprecating API's in favor of superior designs,
I'd like to do that as infrequently as possible.
Neil
 
> 
> Thanks
> ++Keith
> > 
> > 
> >> ++Keith 
> >> 
> >>> 
> >>> Neil
> >>> 
> >> 
> >> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> 
> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> 

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Wiles, Roger Keith]

> On Nov 4, 2014, at 1:29 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Tue, Nov 04, 2014 at 02:44:39PM +0000, Wiles, Roger Keith wrote:
>> 
>>> On Nov 4, 2014, at 5:27 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
>>> 
>>> On Tue, Nov 04, 2014 at 04:52:48AM +0000, Wiles, Roger Keith wrote:
>>>> 
>>>>> On Nov 3, 2014, at 5:42 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
>>>>> 
>>>>> On Mon, Nov 03, 2014 at 03:26:50PM -0800, Stephen Hemminger wrote:
>>>>>> On Mon, 3 Nov 2014 16:50:15 +0000
>>>>>> "Wiles, Roger Keith" <keith.wiles@windriver.com> wrote:
>>>>>> 
>>>>>>> 
>>>>>>>> On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
>>>>>>>> 
>>>>>>>> On Mon, Nov 03, 2014 at 02:25:51PM +0000, Wiles, Roger Keith wrote:
>>>>>>>>> 
>>>>>>>>>> On Nov 3, 2014, at 8:16 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>>>>>>> 
>>>>>>>>>> On Mon, Nov 03, 2014 at 02:08:46PM +0000, Wiles, Roger Keith wrote:
>>>>>>>>>>> 
>>>>>>>>>>>> On Nov 3, 2014, at 4:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>> On Sun, Nov 02, 2014 at 04:28:28PM -0600, Keith Wiles wrote:
>>>>>>>>>>>>> Allow for a external parser to handle the command line if the
>>>>>>>>>>>>> command is not found and the developer has called the routine
>>>>>>>>>>>>> int cmdline_set_external_parser(struct cmdline * cl,
>>>>>>>>>>>>>                           cmdline_external_parser_t parser);
>>>>>>>>>>>>> function to set the function pointer.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> The function for the external parser function should return CMDLINE_PARSE_NOMATCH
>>>>>>>>>>>>> if not able to match the command requested or zero is handled.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Prototype of external routine:
>>>>>>>>>>>>> int (*cmdline_external_parser_t)(struct cmdline * cl, const char * buy);
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
>>>>>>>>>>>> 
>>>>>>>>>>>> Hi Keith,
>>>>>>>>>>>> 
>>>>>>>>>>>> what is the expected use case for this? Is it for embedding other programming languages alongside the existing DPDK command-line or some other purpose? [Perhaps the use case could be called out in the patch description]
>>>>>>>>>>> 
>>>>>>>>>>> Hi Bruce,
>>>>>>>>>>> 
>>>>>>>>>>> I guess the external parser could be used for other programming languages, but the case I was looking at was to provide a default escape from the command line parser to allow my application to handle the commands not understood by the parser. Now that you point it out I could use something like ‘%<line-of-script-code>’ to execute a single line of script code, which is a good idea (thanks).
>>>>>>>>>>> 
>>>>>>>>>>> One case I am looking at is when you want to execute a command and do not want to add the support into the commands.c file for every possible command. Take the case where you have a bunch of scripts (Lua) in a directory much like a bin directory. Then you could type foo.lua or foo on the command line and execute the foo.lua having the application detect you want to load and run a Lua script after it has finished parsing for the builtin commands.
>>>>>>>>>>> 
>>>>>>>>>>> For Pktgen I had to add a command called ‘run <filename> <args…>’ to support running a script with arguments. I also needed to add a argvlist type to cmdline to not error out on that command and split up the args into a argv list like format. (Maybe I need to submit that code??) It seemed more straight forward to just pass the command line to the application to run the command. I understand that seems like a minor point, but it does make it easier to use and to support the features I want to support in my PoC.
>>>>>>>>>>> 
>>>>>>>>>>> Using this method you can just type the name instead of something like ‘run foo.lua’ or just ‘run foo’ and let the code figure out what to run. I have more plans for this features as well and have not finished the basic PoC yet. If you want a peek I can show you what I am working on currently.
>>>>>>>>>>> 
>>>>>>>>>>> Does this help and do I really need to add all of this to the commit message :-)
>>>>>>>>>>> 
>>>>>>>>>> Thanks for the explanation. However, if you are looking to have the application handle a bunch of commands itself, why does it need to use the commandline library at all? Why not just have the app handle all the commands instead of some of them?
>>>>>>>>> 
>>>>>>>>> I guess that would be reasonable, but then I would have to add support for all of the command line parsing being done in the cmdline code. Think of this as a default case for the parser and to me that makes more sense then just doing my own command line design. In the cmdline code you guys provided is a lot of features like history, control key support, arg parsing (IP, MAC) and many others. I would rather not have to write that code myself.
>>>>>>>>> 
>>>>>>>>> The default case is the same behavior today, with giving a no match error unless they add the external parser.
>>>>>>>> 
>>>>>>>> It seems alot simpler than that to me.  Looking at the test applications, the
>>>>>>>> command line parser expects the application to create an array of
>>>>>>>> cmdline_parse_ctx_t structures to support new option parsing.  If your goal is
>>>>>>>> to support other languages, it seems to make more sense to just use foreign
>>>>>>>> language bindings to merge your coding language support with the DPDK
>>>>>>>> (ostensibly you will already have to do that if you want to use other parts of
>>>>>>>> the DPDK).
>>>>>>> Hi Neil,
>>>>>>> 
>>>>>>> A true language binding like Lua or one of those other languages :-) you are correct to believe binding directly using ‘C’ code is the right solution . In Pktgen I use Lua as the direct language binding and extend Lua with specific Pktgen functions.
>>>>>>> 
>>>>>>> What I am doing here is to add a default case to cmdline code, which just happens to allow me to parse the cmdline in the application. Being able to execute say a line of script code is not really the requirement IMO. Being able to extend the cmdline code with a default case is a good feature and allows the developer to extend cmdline for some simple cases. The cmdline code is kind of simple, but does require a fair amount of structures, code and understanding to write a complex extendable command line interface. It does seem hard to find a clean, simple and usable embedded command line code base is not very easy to locate. 
>>>>>>> 
>>>>>>> Adding a true language binding really requires using code to extend the language as I did with Lua and Pktgen. It could have been done with any language I just picked Lua, but the patch does not really add support for a language other then giving some support for someone to handle the no_match case.
>>>>>>> 
>>>>>>> The use case for this feature is not just for Pktgen, but another solution I hope everyone will find useful when I get it more complete.
>>>>>>> 
>>>>>>> Thanks
>>>>>>> ++Keith
>>>>>>> 
>>>>>>> PS. on a different topic I was thinking about suggesting and writing a patch to add Lua with DPDK specific binding and extensions. (also allowing those `other` languages too :-) Being able to use a scripting language and be able to call DPDK API’s could be useful. How useful not sure at this time. (If you want to talk about this topic please start a new thread).
>>>>>>>> 
>>>>>>>> Am I missing something?
>>>>>>>> Neil
>>>>>>>> 
>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> /Bruce
>>>>>>>>> 
>>>>>>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>>>>>>> 
>>>>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>>>>>> 
>>>>>> I wouldn't invest a lot of sweat in the command line parser.
>>>>>> The one in the DPDK is "good enough" for what it needs to do, but really isn't
>>>>>> very complete and flexible. Seems like the kind of thing that doesn't really even
>>>>>> need to be in DPDK. Better off being part of some other library.
>>>>>> 
>>>>> Well, something needs to be there to parse the libraries' common options, though
>>>>> I agree, making eal_cmdline just a registration frontend to getopt or
>>>>> getopt_long would be sufficient.
>>>> 
>>>> Until we have a better command line solution, which I think would be great, but in the mean time I would like to see this patch applied if no one has a technical reason or better suggestion.
>>>> 
>>>> I think this patch is fairly simple and I think we need a way to handle the default case. If someone could please review the patch, that would be great.
>>>> 
>>> I have an objection, specifically, that its not necessecary.  You can already
>>> accomplish what you want to do by adding structures to the context array in the
>>> cmdline structure.  I realize its not as easy as just adding an external parser
>>> function, but its the designed way to add options.  This does little more than
>>> add addition API surface without any real need.
>>> Neil
>> 
>> Neil
>> 
>> I do not agree with your comments as I see it to be a small extension to cmdline to handle the case where I will have to possibly add a huge number of commands/code to the cmdline structures. Using this method I am able to add these very simple commands without having to add more code for this use case.
>> 
> Can you provide a real example here?  usnig vague terms like "huge" really makes
> more of an emotional argument than a factual one.  To cite an example the
> cmdline_test program adds a command line paramter that just lets you parse a
> number out of the command line.  Silly, granted, but it serves the purpose.  Its
> called cmd_num, and with functions and data all told, it looks like it takes
> about 17 lines of code.  Now thats more than what you're adding with you patch,
> I grant you, but I assert that the "potentially huge" argument you're making
> above is false, especially whan you consider that some reasonably clever coding
> can likely allow you to reuse function parsing fairly easily.

I think I gave you an example below, but here is one I am looking at now.

I have a number of programs and scripts in a bin directory, one of the reasons for these programs is to be able to extend the application without having to rebuild the application or adding special parsing of the arguments for just the one program. In my case the scripts can parse the arguments as it already has the support builtin and having cmdline do any processing of the commands is redundant.

The programs being loaded are shared objects via dlopen() and already have its own parsing code and trying to parse an IP address in cmdline to a binary format would just require me to convert it back to a string to pass that string in a argc/argv format. The project I am doing now is building a dpdk-shell like environment, which starts up DPDK as normal then stops at a command prompt.

In the command prompt I am able to load and execute application like l2fwd or l3fwd built as a shared object. Plus I still have a command prompt from the dpdk-sh to launch other applications or look at stats or debug the application. I am still refining the details, but being able to launch a “dpdk-sh” like system then be able to execute/debug/view stats and anything else one can think of is a very reasonable use. Using this method the user commands are simple and easy to remember, plus someone can build a new application to debug without rebuilding DPDK.

I can see this type of environment as a cleaner way for new users to understand DPDK and start playing with the system. I want to add support to run multiple applications at the same time or at least be able to grab stats and information from the application and/or DPDK without someone having to add that support to his application. It is possible with some changes to remove the cmdline parsing from the l3fwd application by adding the basic commands into the dpdk-sh environment. ** This does not mean I am forcing cmdline on applications or developers they can still use DPDK without cmdline or any other feature.

Plus I can now execute any function within the application of loaded modules or DPDK by doing a symbol lookup and call the function.

I am trying to build the dpdk-sh with very little modifications to DPDK and as another application similar to Pktgen. Today the examples directory has some great example code all developed by Intel and I would like to start seeing other applications (like Pktgen) contributed to the community. It would be even better (with some effort) to rewrite the examples directory to use dpdk-sh instead or as well.
> 
>> Lets say you have a directory on the disk that has possibly a 100 little commands, without this minor change I would have to write 100 little structures/code for cmdline to handle each case. Another option is to write a single command to handle these commands. I used this method in Pktgen and could do ‘run foo <args>’ style commands, but it would be much simpler for the user to just type ‘foo <args>’ instead.
>> 
> Yes, but thats honestly true of every command line parser, something needs to
> define the tokens that identify the option, the function to handle its
> interpretation and the structures to glue it all together.  That can be in the
> DPDK, or in something else.  To use your example above, one presumes that your
> 100 little commands all have some sort of parsing structure coded elsewhere,
> along with the code to do that parsing, right? You don't seem to take that
> additional code into account here.  If you can remove that parsing code from
> your application, then it seems to me the additional 17 lines above really isn't
> that big a deal.

Yes, the patch is very small, but adds more functionality to cmdline. Even the application needs to have parsing code to decode complex arguments. Adding that support to cmdline for just one application would be over kill. In Pktgen a couple of arguments are easier to parse in the application then to write a cmdline structure/code just not worth the 'return on investment' that no one else will use.
> 
> It seems to me that, what this boils down to is the fact that you have an
> application that needs to parse a single command line with two different parser,
> whcih is just kind of a lousy situation, because parsers all assume that they
> are the only thing parsing the command line.  Honestly, I really question the
> need for a command line parser so tightly integrated with DPDK at all.

The command line parser is not integrated with DPDK, just happens to be in the library.

Yes, I have an application just like the rest of the world and I thought this would a reasonable change to extend cmdline code. Just because you do not see a need to use this feature or have an application needing the feature does not mean anything to this discussion.

> rte_eal_init really shouldn't be acepting a straight command line buffer.  As a
> library, it should more likely accept a configuration structure which it just
> doles out to individual components during initalization.  That can leave
> applications like yours to handle command line parsing 100% as you see fit, and
> then build the DPDK configuration from that as you like.

hmmm, DPDK using command line arguments is a nice feature. If you want to pass a structure instead then please produce a patch to add that support, but you had better keep the original argument passing API too. This way you get a structure passing API and the rest of the code can use the original API. You still need argument parsing code in the application for the application needs. From the command line we need argc/argv and these happened to be passed to DPDK to parse, which gets converted into a structure anyway. If your application does not use the command line as it is embedded into another application, then using a structure make sense.

You seem to accuse me of being myopic toward command line and argc/argv design, but you seem to also be myopic for the embedded case view point. Oh well!!!

> 
>> Having a default handler for commands just makes a lot of sense to me and I do not buy the 'added API surface without any real need' statement.
> I'm not sure whats not to buy there.  I see the above as facts:
> 
> 1) You're adding API surface (you added a function that is exported from the
> library)

Yes, this is a fact but is not relevant to the issue here. Any added API will increase the testing. I could have not used an API to set the function pointer, just make the variable global to allow the application to set the function pointer. I just figures it was cleaner. So if I do remove the function the #1 means nothing.
> 
> 2) You don't need to do (1) (theres an already existing alternate method to do
> what you want)

Yes, you could work around this problem with API or tricks, but adding a default case here is the cleanest and simplest method. From a users point of view #2 is not valid.

I feel like you have not had a lot of requirements or experience with writing a complex application for DPDK with a fair number of arguments or command line commands. Maybe, I am wrong here.
> 
> You can argue all day that your method is better, but it doesn't change the fact
> that (1) and (2) are true. And I don't want to go adding additional methods to
> the existing ones as there will be a need to support them in the future, and as
> such, If we're going to start deprecating API's in favor of superior designs,
> I'd like to do that as infrequently as possible.

As I pointed out your #1 and #2 are not really a great argument here for the suggested feature not how I would use said feature is a bit outside the scope.

What is the opinion of the rest of the list, as it appears Neil and myself are not going to agree?

(For a few lines of code, I could have written a new command line library after these email :-)

If you want me to pull the patch I can, but I feel we are being a bit short sighted here.

Thanks
++Keith

> Neil
> 
>> 
>> Thanks
>> ++Keith
>>> 
>>> 
>>>> ++Keith 
>>>> 
>>>>> 
>>>>> Neil
>>>>> 
>>>> 
>>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>> 
>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.

[Neil Horman]

On Tue, Nov 04, 2014 at 08:45:53PM +0000, Wiles, Roger Keith wrote:
><snip> 
> >> 
> > Can you provide a real example here?  usnig vague terms like "huge" really makes
> > more of an emotional argument than a factual one.  To cite an example the
> > cmdline_test program adds a command line paramter that just lets you parse a
> > number out of the command line.  Silly, granted, but it serves the purpose.  Its
> > called cmd_num, and with functions and data all told, it looks like it takes
> > about 17 lines of code.  Now thats more than what you're adding with you patch,
> > I grant you, but I assert that the "potentially huge" argument you're making
> > above is false, especially whan you consider that some reasonably clever coding
> > can likely allow you to reuse function parsing fairly easily.
> 
> I think I gave you an example below, but here is one I am looking at now.
> 
This was the level I was looking for, thank you.

> I have a number of programs and scripts in a bin directory, one of the reasons for these programs is to be able to extend the application without having to rebuild the application or adding special parsing of the arguments for just the one program. In my case the scripts can parse the arguments as it already has the support builtin and having cmdline do any processing of the commands is redundant.
> 
> The programs being loaded are shared objects via dlopen() and already have its own parsing code and trying to parse an IP address in cmdline to a binary format would just require me to convert it back to a string to pass that string in a argc/argv format. The project I am doing now is building a dpdk-shell like environment, which starts up DPDK as normal then stops at a command prompt.
> 
So, just so that I'm clear, you're taking example applications from the DPDK,
rebuilding them as shared objects (which they are not currently setup to be
built as), and then running them from a program of your authorship, by loading
them (the DPDK example applications), via dlopen  and calling some arbitrary
function(s) within them.  Is that correct?  If so, that seems....odd.  Commonly
programs that extend other programs without modifying them use fork to establish
a parent/child relationship, and then use the parent to properly control the
child.  GDB is a great example here, one which handles your command line
predicament by adding a command to the gdb console to build the child command
line.  Theres no reason you can't do the same thing, and that requires even less
code than what your proposing here.  If you wanted it to be non-interactive, its
just as easy to add a command line option to your application parser that is:

--child-options="<dpdk options go here>"

which passes a complete command line to the dpdk application without having to
convolute the work of two parsers.

> In the command prompt I am able to load and execute application like l2fwd or l3fwd built as a shared object. Plus I still have a command prompt from the dpdk-sh to launch other applications or look at stats or debug the application. I am still refining the details, but being able to launch a “dpdk-sh” like system then be able to execute/debug/view stats and anything else one can think of is a very reasonable use. Using this method the user commands are simple and easy to remember, plus someone can build a new application to debug without rebuilding DPDK.
> 
This sounds even more like the GDB example above.  Its set args command should
be your guide to a good implementation.

> I can see this type of environment as a cleaner way for new users to understand DPDK and start playing with the system. I want to add support to run multiple applications at the same time or at least be able to grab stats and information from the application and/or DPDK without someone having to add that support to his application. It is possible with some changes to remove the cmdline parsing from the l3fwd application by adding the basic commands into the dpdk-sh environment. ** This does not mean I am forcing cmdline on applications or developers they can still use DPDK without cmdline or any other feature.
> 
Sounding more and more like a GDB type of environment...

> Plus I can now execute any function within the application of loaded modules or DPDK by doing a symbol lookup and call the function.
> 
Like GDB....

> I am trying to build the dpdk-sh with very little modifications to DPDK and as another application similar to Pktgen. Today the examples directory has some great example code all developed by Intel and I would like to start seeing other applications (like Pktgen) contributed to the community. It would be even better (with some effort) to rewrite the examples directory to use dpdk-sh instead or as well.

Then you should definately use the GDB model, as that requires zero changes to
anything in the DPDK.

> > 
> >> Lets say you have a directory on the disk that has possibly a 100 little commands, without this minor change I would have to write 100 little structures/code for cmdline to handle each case. Another option is to write a single command to handle these commands. I used this method in Pktgen and could do ‘run foo <args>’ style commands, but it would be much simpler for the user to just type ‘foo <args>’ instead.
> >> 
> > Yes, but thats honestly true of every command line parser, something needs to
> > define the tokens that identify the option, the function to handle its
> > interpretation and the structures to glue it all together.  That can be in the
> > DPDK, or in something else.  To use your example above, one presumes that your
> > 100 little commands all have some sort of parsing structure coded elsewhere,
> > along with the code to do that parsing, right? You don't seem to take that
> > additional code into account here.  If you can remove that parsing code from
> > your application, then it seems to me the additional 17 lines above really isn't
> > that big a deal.
> 
> Yes, the patch is very small, but adds more functionality to cmdline. Even the application needs to have parsing code to decode complex arguments. Adding that support to cmdline for just one application would be over kill. In Pktgen a couple of arguments are easier to parse in the application then to write a cmdline structure/code just not worth the 'return on investment' that no one else will use.
> > 
> > It seems to me that, what this boils down to is the fact that you have an
> > application that needs to parse a single command line with two different parser,
> > whcih is just kind of a lousy situation, because parsers all assume that they
> > are the only thing parsing the command line.  Honestly, I really question the
> > need for a command line parser so tightly integrated with DPDK at all.
> 
> The command line parser is not integrated with DPDK, just happens to be in the library.
> 
I wish that were true.  The command line parser in DPDK is its own library, and
_should_ be separate from the DPDK, but initalization of the eal library depends
heavily on it.

> Yes, I have an application just like the rest of the world and I thought this would a reasonable change to extend cmdline code. Just because you do not see a need to use this feature or have an application needing the feature does not mean anything to this discussion.
> 
Thats why we're having this debate though isn't it?  Because opinions matter
when they're backed up by evidence.  I'm asking you to provide evidence for why
you need to increase the maintenence burden of the command line parser in DPDK,
and have yet to hear a good answer.  You're example above was quite
illuminating, and I think goes to show your patch is neither needed nor
desireable.  You indicated yourself that you would like to avoid changing the
DPDK as much as possible.  So don't change it.  There are lots of ways to
develop the application you want without having to create this linkage
between command line parsers.

> > rte_eal_init really shouldn't be acepting a straight command line buffer.  As a
> > library, it should more likely accept a configuration structure which it just
> > doles out to individual components during initalization.  That can leave
> > applications like yours to handle command line parsing 100% as you see fit, and
> > then build the DPDK configuration from that as you like.
> 
> hmmm, DPDK using command line arguments is a nice feature. If you want to pass a structure instead then please produce a patch to add that support, but you had better keep the original argument passing API too. This way you get a structure passing API and the rest of the code can use the original API. You still need argument parsing code in the application for the application needs. From the command line we need argc/argv and these happened to be passed to DPDK to parse, which gets converted into a structure anyway. If your application does not use the command line as it is embedded into another application, then using a structure make sense.
> 
So, yes, having DPDK command line arguments is nice, having them integrated with
the other libraries in DPDK isn't.  And thats my issue with your patch.  The
fact that they are integrated so tightly at the moment implies that any
additional API surface is a potential new maintenence headache, so you can
understand why I'm not interested in arbitrarily adding API surface when its not
necessecary.

> You seem to accuse me of being myopic toward command line and argc/argv design, but you seem to also be myopic for the embedded case view point. Oh well!!!
> 
You're not being myopic, on the contrary, you're being very pragmatic.  You're
just being very pragmatic in pursuit of your own interests, which, from the
description of your use case above, I would wager is largely isolated to you.  I
don't want to maintain a API interface that has only one user.

> > 
> >> Having a default handler for commands just makes a lot of sense to me and I do not buy the 'added API surface without any real need' statement.
> > I'm not sure whats not to buy there.  I see the above as facts:
> > 
> > 1) You're adding API surface (you added a function that is exported from the
> > library)
> 
> Yes, this is a fact but is not relevant to the issue here. Any added API will increase the testing. I could have not used an API to set the function pointer, just make the variable global to allow the application to set the function pointer. I just figures it was cleaner. So if I do remove the function the #1 means nothing.
You're missing my point, you're adding an interface to the DPDK (be it a
variable or a function doesn't matter).  What I want you to do is avoid
modifying the DPDK at all for your application here (see 2 below)

> > 
> > 2) You don't need to do (1) (theres an already existing alternate method to do
> > what you want)
> 
> Yes, you could work around this problem with API or tricks, but adding a default case here is the cleanest and simplest method. From a users point of view #2 is not valid.
> 
Its not a trick or work-around, its the proscribed method for building an application with the
DPDK.  Now, I grant you, your application is divergent from the common
application model, but that doesn't mean the DPDK needs to support it
explicitly.  As I noted, GDB does this with any arbitrary program, theres no
reason you can't do it also.

> I feel like you have not had a lot of requirements or experience with writing
a complex application for DPDK with a fair number of arguments or command line
commands. Maybe, I am wrong here.

You are.

> > 
> > You can argue all day that your method is better, but it doesn't change the fact
> > that (1) and (2) are true. And I don't want to go adding additional methods to
> > the existing ones as there will be a need to support them in the future, and as
> > such, If we're going to start deprecating API's in favor of superior designs,
> > I'd like to do that as infrequently as possible.
> 
> As I pointed out your #1 and #2 are not really a great argument here for the suggested feature not how I would use said feature is a bit outside the scope.
> 
> What is the opinion of the rest of the list, as it appears Neil and myself are not going to agree?
> 
> (For a few lines of code, I could have written a new command line library after these email :-)
> 
> If you want me to pull the patch I can, but I feel we are being a bit short sighted here.

You are correct, we're not going to agree.  I'll let your use case that you
described here be the guiding example.

Neil

> 
> Thanks
> ++Keith
> 
> > Neil
> > 
> >> 
> >> Thanks
> >> ++Keith
> >>> 
> >>> 
> >>>> ++Keith 
> >>>> 
> >>>>> 
> >>>>> Neil
> >>>>> 
> >>>> 
> >>>> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> >> 
> >> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> 
> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>