From: Bruce Richardson <bruce.richardson@intel.com>
To: David Marchand <david.marchand@redhat.com>
Cc: <dev@dpdk.org>, <rjarry@redhat.com>
Subject: Re: [PATCH v4 0/7] document and simplify use of cmdline
Date: Tue, 17 Oct 2023 09:29:34 +0100 [thread overview]
Message-ID: <ZS5F7kQ59kAob5fA@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <CAJFAV8w0hUammE9iG7zRWDpRA970x=Of_bxyiHsyBOg=G9kxiQ@mail.gmail.com>
On Tue, Oct 17, 2023 at 09:10:51AM +0200, David Marchand wrote:
> Hi Bruce,
>
> On Mon, Oct 16, 2023 at 4:06 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > The DPDK commandline library is widely used by apps and examples within
> > DPDK, but it is not documented in our programmers guide and it requires
> > a lot of boilerplate code definitions in order to used. We can improve
> > this situation by creating a simple python script to automatically
> > generate the boilerplate from a list of commands.
> >
> > This patchset contains a new documentation chapter on cmdline library,
> > going through step-by-step how to add commands and create the necessary
> > token lists and parse contexts.
> >
> > Following that initial doc patch, the set then contains a
> > boilerplate-generating script, as well as a set of four patches showing
> > its use, by converting four examples to use the script instead of
> > having the hard-coded boilerplate. Once the script is used, adding a new
> > command becomes as simple as adding the desired command to the .list
> > file, and then writing the required function which will be called for
> > that command. No other boilerplate coding is necessary.
> >
> > Script obviously does not cover the full range of capabilities of the
> > commandline lib, but does cover the most used parts. The code-saving to
> > each of the examples by auto-generating the boilerplate is significant,
> > and probably more examples with commandlines can be converted over in
> > future.
>
> This is not something blocking for the series, but I had a quick try
> with testpmd to see where this series stands.
> I hit, right off the bat, some of those limitations, but I think there
> are not that difficult to deal with.
>
Thanks for investigating. Item #2 below is something I'd already been
thinking of, but had not yet implemented. The hope was to get the basics in
first and iterate thereafter, rather than trying to get it all in in one
go. However, if I have the chance, and depending on how long before this
set gets merged, I might have a try at a respin with an additional feature.
>
> - testpmd accepts both "help" and "help <section>" commands.
> But the cmdline library does not provide a way (afair) for specifiying
> this "optional" aspect.
>
> And it can't be expressed with this series current syntax since
> generated symbols would conflict if we ask for two "help" commands.
>
> My quick hack was to introduce a way to select the prefix of the
> generated symbols.
> There may be a better way, like finding a better discriminant for
> naming symbols...
> But, on the other hand, the symbols prefix might be something a
> developer wants to control (instead of currently hardcoded cmd_
> prefix).
>
> @@ -20,6 +20,12 @@ def process_command(tokens, cfile, comment):
> """Generate the structures and definitions for a single command."""
> name = []
>
> + prefix, sep, cmd_name = tokens[0].partition(':')
> + if cmd_name:
> + tokens[0] = cmd_name
> + else:
> + prefix = 'cmd'
> +
> if tokens[0].startswith('<'):
> print('Error: each command must start with at least one
> literal string', file=sys.stderr)
> sys.exit(1)
>
> (etc... I am not copying the rest of the diff)
>
> I then used as:
>
> cmd_brief:help # help: Show help
> help <STRING>section # help: Show help
>
Interesting. I actually had no plans to even consider moving something like
testpmd over. However, this is an interesting one, though I'm not really
sure I like it that much as a feature :-) I rather like having unique
prefixes for each command. I wasn't actually aware of the testpmd "help
<section>" command at all. I will have to look into it.
>
> - Multi choice fixed strings is something that is often used in
> testpmd, like here, in the help <section> command.
> Here is my quick hack:
>
> diff --git a/buildtools/dpdk-cmdline-gen.py b/buildtools/dpdk-cmdline-gen.py
> index 3b41fb0493..e8c9e079de 100755
> --- a/buildtools/dpdk-cmdline-gen.py
> +++ b/buildtools/dpdk-cmdline-gen.py
> @@ -35,7 +35,11 @@ def process_command(tokens, cfile, comment):
> for t in tokens:
> if t.startswith('<'):
> t_type, t_name = t[1:].split('>')
> - t_val = 'NULL'
> + if len(t_type.split('(')) == 2:
> + t_type, t_val = t_type.split('(')
> + t_val = '"' + t_val.split(')')[0] + '"'
> + else:
> + t_val = 'NULL'
> else:
> t_type = 'STRING'
> t_name = t
> @@ -113,7 +117,7 @@ def process_commands(infile, hfile, cfile, ctxname):
> continue
> if '#' not in line:
> line = line + '#' # ensure split always works, even if
> no help text
> - tokens, comment = line.split('#', 1)
> + tokens, comment = line.rsplit('#', 1)
> instances.append(process_command(tokens.strip().split(),
> cfile, comment.strip()))
>
> print(f'static __rte_used cmdline_parse_ctx_t {ctxname}[] = {{')
>
>
> Which translates as:
> cmd_brief:help # help: Show help
> help <STRING(all#control#display#config#ports)>section # help: Show help
>
+1
I was actualy thinking that adding support for multi-choice fixed strings
is something we should add. One thought that I had was that "#" is not a
particularly good choice of separator here. While, as you show, it can be
made to work; I think - since we are defining our own syntax here - that it
would be both simpler for the script, and simpler for the user, to have "|"
as the option separator. It should be familiar for everyone as an option
separator from regexes, unlike "#" which is more familar for comments.
So what about:
help <|all|control|display|config|ports|>section
By starting with the separator, we should avoid having to provide the
STRING type at all.
To my previous point on not liking to have a prefix-specifier, the
alternative to making testpmd work with the script is to tweak very
slightly the "help <section>".
help # show general help
help on <|all|control|display|config|ports|>section
By making the command "help on ports" rather than "help ports" we would
avoid the need for the prefix syntax.
WDYT?
/Bruce
next prev parent reply other threads:[~2023-10-17 8:29 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-02 17:00 [RFC PATCH 0/1] make cmdline library easier to use Bruce Richardson
2023-08-02 17:00 ` [RFC PATCH 1/1] cmdline/dpdk-cmdline-gen: generate boilerplate for simple cmds Bruce Richardson
2023-08-02 18:05 ` [RFC PATCH 0/1] make cmdline library easier to use Stephen Hemminger
2023-08-03 8:11 ` Bruce Richardson
2023-09-18 13:03 ` [RFC PATCH v2 0/5] use script to simplify use of cmdline lib Bruce Richardson
2023-09-18 13:03 ` [RFC PATCH v2 1/5] buildtools/dpdk-cmdline-gen: generate boilerplate for simple cmds Bruce Richardson
2023-09-18 13:03 ` [RFC PATCH v2 2/5] examples/simple_mp: auto-generate cmdline boilerplate Bruce Richardson
2023-09-18 13:03 ` [RFC PATCH v2 3/5] examples/hotplug_mp: " Bruce Richardson
2023-09-18 13:03 ` [RFC PATCH v2 4/5] buildtools/dpdk-cmdline-gen: add IP address support Bruce Richardson
2023-09-18 13:03 ` [RFC PATCH v2 5/5] examples/bond: auto-generate cmdline boilerplate Bruce Richardson
2023-10-11 13:33 ` [PATCH v3 0/5] document and simplify use of cmdline Bruce Richardson
2023-10-11 13:33 ` [PATCH v3 1/5] doc/prog_guide: new chapter on cmdline library Bruce Richardson
2023-10-11 13:33 ` [PATCH v3 2/5] buildtools: script to generate cmdline boilerplate Bruce Richardson
2023-10-13 12:23 ` Robin Jarry
2023-10-13 12:43 ` Bruce Richardson
2023-10-11 13:33 ` [PATCH v3 3/5] examples/simple_mp: auto-generate " Bruce Richardson
2023-10-11 13:33 ` [PATCH v3 4/5] examples/hotplug_mp: " Bruce Richardson
2023-10-11 13:33 ` [PATCH v3 5/5] examples/bond: " Bruce Richardson
2023-10-12 13:21 ` [PATCH v3 0/5] document and simplify use of cmdline David Marchand
2023-10-12 13:47 ` Bruce Richardson
2023-10-12 13:51 ` Bruce Richardson
2023-10-16 14:06 ` [PATCH v4 0/7] " Bruce Richardson
2023-10-16 14:06 ` [PATCH v4 1/7] doc/prog_guide: new chapter on cmdline library Bruce Richardson
2023-10-16 14:06 ` [PATCH v4 2/7] buildtools: script to generate cmdline boilerplate Bruce Richardson
2023-10-16 14:06 ` [PATCH v4 3/7] ci: allow use of DPDK tools when building examples Bruce Richardson
2023-10-17 12:24 ` Aaron Conole
2023-10-17 12:28 ` Bruce Richardson
2023-10-16 14:06 ` [PATCH v4 4/7] examples/simple_mp: auto-generate cmdline boilerplate Bruce Richardson
2023-10-16 14:06 ` [PATCH v4 5/7] examples/hotplug_mp: " Bruce Richardson
2023-10-16 14:06 ` [PATCH v4 6/7] examples/bond: " Bruce Richardson
2023-10-16 14:06 ` [PATCH v4 7/7] examples/vdpa: " Bruce Richardson
2023-10-17 7:10 ` [PATCH v4 0/7] document and simplify use of cmdline David Marchand
2023-10-17 8:29 ` Bruce Richardson [this message]
2023-10-17 12:16 ` Bruce Richardson
2023-10-17 16:23 ` David Marchand
2023-10-17 17:02 ` Bruce Richardson
2023-10-17 17:08 ` Bruce Richardson
2023-10-18 11:21 ` David Marchand
2023-10-18 11:37 ` Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 0/9] " Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 1/9] doc/prog_guide: new chapter on cmdline library Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 2/9] buildtools: script to generate cmdline boilerplate Bruce Richardson
2023-10-25 13:04 ` Robin Jarry
2023-10-25 13:33 ` Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 3/9] ci: allow use of DPDK tools when building examples Bruce Richardson
2023-10-17 14:08 ` Aaron Conole
2023-10-17 12:13 ` [PATCH v5 4/9] examples/simple_mp: auto-generate cmdline boilerplate Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 5/9] examples/hotplug_mp: " Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 6/9] examples/bond: " Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 7/9] examples/vdpa: " Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 8/9] buildtools/dpdk-cmdline-gen: support option strings Bruce Richardson
2023-10-17 12:13 ` [PATCH v5 9/9] examples/ntb: auto-generate cmdline boilerplate Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 0/9] document and simplify use of cmdline Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 1/9] doc/prog_guide: new chapter on cmdline library Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 2/9] buildtools: script to generate cmdline boilerplate Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 3/9] ci: allow use of DPDK tools when building examples Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 4/9] examples/simple_mp: auto-generate cmdline boilerplate Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 5/9] examples/hotplug_mp: " Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 6/9] examples/bond: " Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 7/9] examples/vdpa: " Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 8/9] buildtools/dpdk-cmdline-gen: support option strings Bruce Richardson
2023-10-23 13:15 ` [PATCH v6 9/9] examples/ntb: auto-generate cmdline boilerplate Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 0/9] document and simplify use of cmdline Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 1/9] doc/prog_guide: new chapter on cmdline library Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 2/9] buildtools: script to generate cmdline boilerplate Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 3/9] ci: allow use of DPDK tools when building examples Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 4/9] examples/simple_mp: auto-generate cmdline boilerplate Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 5/9] examples/hotplug_mp: " Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 6/9] examples/bond: " Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 7/9] examples/vdpa: " Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 8/9] buildtools/dpdk-cmdline-gen: support option strings Bruce Richardson
2023-10-27 11:01 ` [PATCH v7 9/9] examples/ntb: auto-generate cmdline boilerplate Bruce Richardson
2023-11-10 14:16 ` [PATCH v7 0/9] document and simplify use of cmdline David Marchand
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=ZS5F7kQ59kAob5fA@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=rjarry@redhat.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).