DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: "Wiles, Keith" <keith.wiles@intel.com>
Cc: DPDK <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC] New CLI for DPDK
Date: Fri, 24 Mar 2017 14:09:54 +0100	[thread overview]
Message-ID: <20170324140954.356e3755@platinum> (raw)
In-Reply-To: <F0C49330-D004-4CC8-AA88-EEBE4CF24ABB@intel.com>

Hi Keith,

On Thu, 23 Mar 2017 16:13:21 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
> > On Mar 10, 2017, at 9:25 AM, Wiles, Keith <keith.wiles@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

  reply	other threads:[~2017-03-24 13:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-10 15:25 Wiles, Keith
2017-03-10 16:06 ` Stephen Hemminger
2017-03-10 16:22   ` Wiles, Keith
2017-03-10 16:41     ` Stephen Hemminger
2017-03-10 17:04       ` Wiles, Keith
2017-03-23 16:13 ` Wiles, Keith
2017-03-24 13:09   ` Olivier Matz [this message]
2017-03-24 14:48     ` Wiles, Keith
2017-03-28 10:06       ` Thomas Monjalon
2017-03-28 13:19         ` Wiles, Keith
2017-03-28 13:30           ` Thomas Monjalon

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=20170324140954.356e3755@platinum \
    --to=olivier.matz@6wind.com \
    --cc=dev@dpdk.org \
    --cc=keith.wiles@intel.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).