From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Horman Subject: Re: [PATCH] Add external parser support for unknown commands. Date: Wed, 5 Nov 2014 10:11:06 -0500 Message-ID: <20141105151106.GA9306@localhost.localdomain> References: <7D39110F-D305-4C48-8BD6-37F6DCD2E434@windriver.com> <20141103160602.GA6625@localhost.localdomain> <6F166021-567F-4B30-8EAE-EDB68A86C6CC@windriver.com> <20141103152650.262e4da3@urahara> <20141103234230.GA2660@localhost.localdomain> <35090085-66D7-4F23-9867-A0DCC09E7DDD@windriver.com> <20141104112742.GB9995@hmsreliant.think-freely.org> <6F5C8CE3-5882-4222-BF8F-B8A29A3EB1BD@windriver.com> <20141104192904.GD9995@hmsreliant.think-freely.org> <394E147B-5A0E-4557-8FD0-496BA25A4CB6@windriver.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: "dev-VfR2kkLFssw@public.gmane.org" To: "Wiles, Roger Keith" Return-path: Content-Disposition: inline In-Reply-To: <394E147B-5A0E-4557-8FD0-496BA25A4CB6-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" On Tue, Nov 04, 2014 at 08:45:53PM +0000, Wiles, Roger Keith wrote: >=20 > >>=20 > > Can you provide a real example here? usnig vague terms like "huge" r= eally 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 pu= rpose. Its > > called cmd_num, and with functions and data all told, it looks like i= t 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 cle= ver coding > > can likely allow you to reuse function parsing fairly easily. >=20 > I think I gave you an example below, but here is one I am looking at no= w. >=20 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 withou= t having to rebuild the application or adding special parsing of the argu= ments for just the one program. In my case the scripts can parse the argu= ments as it already has the support builtin and having cmdline do any pro= cessing of the commands is redundant. >=20 > The programs being loaded are shared objects via dlopen() and already h= ave 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 p= ass that string in a argc/argv format. The project I am doing now is buil= ding a dpdk-shell like environment, which starts up DPDK as normal then s= tops at a command prompt. >=20 So, just so that I'm clear, you're taking example applications from the D= PDK, 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 lo= ading them (the DPDK example applications), via dlopen and calling some arbitr= ary function(s) within them. Is that correct? If so, that seems....odd. Co= mmonly programs that extend other programs without modifying them use fork to es= tablish 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 com= mand line. Theres no reason you can't do the same thing, and that requires ev= en less code than what your proposing here. If you wanted it to be non-interacti= ve, its just as easy to add a command line option to your application parser that= is: --child-options=3D"" which passes a complete command line to the dpdk application without havi= ng to convolute the work of two parsers. > In the command prompt I am able to load and execute application like l2= fwd 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 launc= h a =E2=80=9Cdpdk-sh=E2=80=9D like system then be able to execute/debug/v= iew stats and anything else one can think of is a very reasonable use. Us= ing this method the user commands are simple and easy to remember, plus s= omeone can build a new application to debug without rebuilding DPDK. >=20 This sounds even more like the GDB example above. Its set args command s= hould be your guide to a good implementation. > I can see this type of environment as a cleaner way for new users to un= derstand 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 s= tats and information from the application and/or DPDK without someone hav= ing to add that support to his application. It is possible with some chan= ges to remove the cmdline parsing from the l3fwd application by adding th= e basic commands into the dpdk-sh environment. ** This does not mean I am= forcing cmdline on applications or developers they can still use DPDK wi= thout cmdline or any other feature. >=20 Sounding more and more like a GDB type of environment... > Plus I can now execute any function within the application of loaded mo= dules or DPDK by doing a symbol lookup and call the function. >=20 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 directo= ry has some great example code all developed by Intel and I would like to= start seeing other applications (like Pktgen) contributed to the communi= ty. It would be even better (with some effort) to rewrite the examples di= rectory to use dpdk-sh instead or as well. Then you should definately use the GDB model, as that requires zero chang= es to anything in the DPDK. > >=20 > >> Lets say you have a directory on the disk that has possibly a 100 li= ttle commands, without this minor change I would have to write 100 little= structures/code for cmdline to handle each case. Another option is to wr= ite a single command to handle these commands. I used this method in Pktg= en and could do =E2=80=98run foo =E2=80=99 style commands, but it w= ould be much simpler for the user to just type =E2=80=98foo =E2=80=99= instead. > >>=20 > > Yes, but thats honestly true of every command line parser, something = needs to > > define the tokens that identify the option, the function to handle it= s > > 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 els= ewhere, > > 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 co= de from > > your application, then it seems to me the additional 17 lines above r= eally isn't > > that big a deal. >=20 > Yes, the patch is very small, but adds more functionality to cmdline. E= ven the application needs to have parsing code to decode complex argument= s. 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 applicat= ion then to write a cmdline structure/code just not worth the 'return on = investment' that no one else will use. > >=20 > > It seems to me that, what this boils down to is the fact that you hav= e an > > application that needs to parse a single command line with two differ= ent parser, > > whcih is just kind of a lousy situation, because parsers all assume t= hat they > > are the only thing parsing the command line. Honestly, I really ques= tion the > > need for a command line parser so tightly integrated with DPDK at all= . >=20 > The command line parser is not integrated with DPDK, just happens to be= in the library. >=20 I wish that were true. The command line parser in DPDK is its own librar= y, 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 though= t 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. >=20 Thats why we're having this debate though isn't it? Because opinions mat= ter when they're backed up by evidence. I'm asking you to provide evidence f= or 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 buf= fer. As a > > library, it should more likely accept a configuration structure which= it just > > doles out to individual components during initalization. That can le= ave > > applications like yours to handle command line parsing 100% as you se= e fit, and > > then build the DPDK configuration from that as you like. >=20 > 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 suppo= rt, but you had better keep the original argument passing API too. This w= ay you get a structure passing API and the rest of the code can use the o= riginal 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 struc= ture anyway. If your application does not use the command line as it is e= mbedded into another application, then using a structure make sense. >=20 So, yes, having DPDK command line arguments is nice, having them integrat= ed 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 ca= n 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!!! >=20 You're not being myopic, on the contrary, you're being very pragmatic. Y= ou're just being very pragmatic in pursuit of your own interests, which, from t= he 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. > >=20 > >> Having a default handler for commands just makes a lot of sense to m= e and I do not buy the 'added API surface without any real need' statemen= t. > > I'm not sure whats not to buy there. I see the above as facts: > >=20 > > 1) You're adding API surface (you added a function that is exported f= rom the > > library) >=20 > Yes, this is a fact but is not relevant to the issue here. Any added AP= I will increase the testing. I could have not used an API to set the func= tion pointer, just make the variable global to allow the application to s= et 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) > >=20 > > 2) You don't need to do (1) (theres an already existing alternate met= hod to do > > what you want) >=20 > 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 poi= nt of view #2 is not valid. >=20 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 w= riting a complex application for DPDK with a fair number of arguments or command= line commands. Maybe, I am wrong here. You are. > >=20 > > You can argue all day that your method is better, but it doesn't chan= ge the fact > > that (1) and (2) are true. And I don't want to go adding additional m= ethods to > > the existing ones as there will be a need to support them in the futu= re, 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. >=20 > As I pointed out your #1 and #2 are not really a great argument here fo= r the suggested feature not how I would use said feature is a bit outside= the scope. >=20 > What is the opinion of the rest of the list, as it appears Neil and mys= elf are not going to agree? >=20 > (For a few lines of code, I could have written a new command line libra= ry after these email :-) >=20 > If you want me to pull the patch I can, but I feel we are being a bit s= hort sighted here. You are correct, we're not going to agree. I'll let your use case that y= ou described here be the guiding example. Neil >=20 > Thanks > ++Keith >=20 > > Neil > >=20 > >>=20 > >> Thanks > >> ++Keith > >>>=20 > >>>=20 > >>>> ++Keith=20 > >>>>=20 > >>>>>=20 > >>>>> Neil > >>>>>=20 > >>>>=20 > >>>> Keith Wiles, Principal Technologist with CTO office, Wind River mo= bile 972-213-5533 > >>=20 > >> Keith Wiles, Principal Technologist with CTO office, Wind River mobi= le 972-213-5533 >=20 > Keith Wiles, Principal Technologist with CTO office, Wind River mobile = 972-213-5533 >=20