Hi Keith, On Thu, 23 Mar 2017 16:13:21 +0000, "Wiles, Keith" <keith.wi...@intel.com> wrote: > > On Mar 10, 2017, at 9:25 AM, Wiles, Keith <keith.wi...@intel.com> wrote: > > > > I would like to request for comments on a new CLI design and get any > > feedback. I have attached the cli.rst text, which is still a work in > > progress for you review. > > > > I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of > > the repo in DPDK.org. > > > > http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev > > > > I would like to submit the CLI library to be used in DPDK, if that seems > > reasonable to everyone. I need more testing of the API and Pktgen, but I > > feel it has a simpler design, easier to understand and hopefully make it > > easier for developers to add commands. > > > > As an example I quickly converted over testpmd from CMDLINE to CLI (I just > > add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c > > file from 12.6K lines to about 4.5K lines. I did not fully test the code, > > but the ones I did test seem to work. > > > > I do not expect DPDK to convert to the new CLI only if it makes sense and I > > am not suggesting to replace CMDLINE library. > > > > If you play with the new CLI in pktgen and see any problems or want to > > suggest new features or changes please let me know. > > > > Comments on the cli.rst text is also welcome, but the cli.rst is not > > complete. I think this file needs to be broken into two one to explain the > > example and another to explain CLI internals. > > Any more comments on the CLI code for DPDK. > > Should I submit a patch for DPDK or would it be better to push this into its > own repo on DPDK? >
I agree that there is a large possibility of improvements in librte_cmdline. Just for reference, I think this design was not that bad at the time it was introduced: - declaring commands as static variables makes sense when running on baremetal without malloc library - implementing a readline-like part was also needed because not available on baremetal - masking structures to the user was harder due to the lack of malloc - having a cli is really helpful for a program like testpmd (both for a user or for an automatic test program) Few efforts were made to enhance this library because it's not the heart of dpdk. The current status is acceptable to me: this library is mostly used internally in dpdk for test apps, and application developers are free to use a better one if they want. But adding another cli lib in dpdk does not make sense to me. I think the proper direction would be to remove the cli from dpdk and use a good cli library that is available in distros. But for that: - we need to find one - we need to update all examples/tests that use the cmdline, and that will be a lot of work - we need to enhance dpdk framework to better manage deps with external libraries So, in my opinion, the CLI you are proposing should be hosted on another repo. Regards, Olivier
