All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Wiles, Keith" <keith.wiles@intel.com>
Cc: dev@dpdk.org, Olivier Matz <olivier.matz@6wind.com>, techboard@dpdk.org
Subject: Re: [RFC] New CLI for DPDK
Date: Tue, 28 Mar 2017 12:06:30 +0200	[thread overview]
Message-ID: <1976507.q6Vb44MXhO@xps13> (raw)
In-Reply-To: <5CD3E4F5-4284-4D68-8008-642E9C3685B3@intel.com>

2017-03-24 14:48, Wiles, Keith:
> 
> > On Mar 24, 2017, at 8:09 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> > 
> > 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)
> 
> I agree, but we do need to use a better version.
> 
> > 
> > Few efforts were made to enhance this library because it's not the heart
> > of dpdk.
> 
> True CLI is not the heart of DPDK, but adding a better supported CLI could be a reasonable addition to DPDK for developers to create a real produce instead of cmdline, which is stated was not a CLI for a real product.

We can ask to the techboard whether CLI is in DPDK scope.
My position: it is neither in the scope of the repo nor in the scope of dpdk.org.

> > 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

I agree we could convert testpmd and examples to a new CLI lib.
Then we could remove librte_cmdline from DPDK.

> Replacing applications with a new CLI is background work and we do not need to toss out cmdline until all have been converted, which could take a while. We can move cmdline out of DPDK, but the problem is most of the current applications require cmdline and the developer would be required to pull DPDK and cmdline. I would suggest we have both in DPDK and slowly convert the apps to use the new CLI.
> 
> If we split up DPDK repo into smaller repos and use something like the ‘repo’ command with a catalog file then it would be mostly transparent to the developer we have multiple repos. Until that day it does seem more reasonable to have CLI in DPDK repo, but I am willing to have it in a different repo for now.

It is reasonnable for an application to have some dependencies.
It is commonly managed in three ways:
- detect the lib on the system
- download the lib as part of the build process
- ask user to download it

> > So, in my opinion, the CLI you are proposing should be hosted on another
> > repo.
> 
> I will contact Thomas to setup a new repo for the CLI.

We had a private discussion.
I have tried to put here my conclusions.

  reply	other threads:[~2017-03-28 10:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-10 15:25 [RFC] New CLI for DPDK 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
2017-03-24 14:48     ` Wiles, Keith
2017-03-28 10:06       ` Thomas Monjalon [this message]
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=1976507.q6Vb44MXhO@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=keith.wiles@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=techboard@dpdk.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.