> On Nov 4, 2014, at 5:27 AM, Neil Horman <nhorman at 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 at 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 at windriver.com> wrote: >>>> >>>>> >>>>>> On Nov 3, 2014, at 10:06 AM, Neil Horman <nhorman at 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 at >>>>>>>> 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 at >>>>>>>>>> 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 at 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
