DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wiles, Roger Keith" <keith.wiles@windriver.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] Add external parser support for unknown commands.
Date: Tue, 4 Nov 2014 04:52:48 +0000	[thread overview]
Message-ID: <35090085-66D7-4F23-9867-A0DCC09E7DDD@windriver.com> (raw)
In-Reply-To: <20141103234230.GA2660@localhost.localdomain>


> 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


  reply	other threads:[~2014-11-04  4:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-02 22:28 Keith Wiles
2014-11-03 10:41 ` Bruce Richardson
2014-11-03 14:08   ` Wiles, Roger Keith
2014-11-03 14:16     ` Bruce Richardson
2014-11-03 14:25       ` Wiles, Roger Keith
2014-11-03 16:06         ` Neil Horman
2014-11-03 16:50           ` Wiles, Roger Keith
2014-11-03 23:26             ` Stephen Hemminger
2014-11-03 23:42               ` Neil Horman
2014-11-04  4:52                 ` Wiles, Roger Keith [this message]
2014-11-04 11:27                   ` Neil Horman
2014-11-04 14:44                     ` Wiles, Roger Keith
2014-11-04 19:29                       ` Neil Horman
2014-11-04 20:45                         ` Wiles, Roger Keith
2014-11-05 15:11                           ` Neil Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=35090085-66D7-4F23-9867-A0DCC09E7DDD@windriver.com \
    --to=keith.wiles@windriver.com \
    --cc=dev@dpdk.org \
    --cc=nhorman@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).