mbox series

[v4,00/11] improve options help

Message ID 20210321223116.1340974-1-thomas@monjalon.net (mailing list archive)
Headers
Series improve options help |

Message

Thomas Monjalon March 21, 2021, 10:31 p.m. UTC
  The main intent of this series is to provide a nice help
for the --log-level option.
More patches are added to improve options help in general.


v4:
    - fix more misses in testpmd help
    - add some "Fixes:" lines
v3:
    - fix use of RTE_LOG_MAX
    - accept (with warning) log level higher than RTE_LOG_MAX
v2:
    - fix use of the new macro RTE_LOG_MAX in level parsing
    - improve parameters type and name while moving functions


Thomas Monjalon (11):
  eal: explain argv behaviour during init
  eal: improve options usage text
  eal: use macros for help option
  eal: move private log functions
  eal: introduce maximum log level macro
  eal: catch invalid log level number
  eal: add log level help
  app: fix exit messages
  app: hook in EAL usage help
  app/regex: fix usage text
  app/testpmd: fix usage text

 app/pdump/main.c                              |  2 +
 app/proc-info/main.c                          |  2 +
 app/test-acl/main.c                           |  2 +
 app/test-bbdev/main.c                         |  3 +-
 app/test-compress-perf/comp_perf_options.h    |  2 +
 .../comp_perf_options_parse.c                 | 10 +--
 app/test-compress-perf/main.c                 |  3 +-
 app/test-crypto-perf/cperf_options.h          |  2 +
 app/test-crypto-perf/cperf_options_parsing.c  | 10 +--
 app/test-crypto-perf/main.c                   |  3 +-
 app/test-fib/main.c                           |  8 ++
 app/test-flow-perf/main.c                     | 63 ++++++++--------
 app/test-pmd/parameters.c                     | 43 +++--------
 app/test-pmd/testpmd.c                        |  2 +
 app/test-pmd/testpmd.h                        |  1 +
 app/test-regex/main.c                         | 10 +--
 app/test-sad/main.c                           |  7 ++
 lib/librte_eal/common/eal_common_log.c        | 50 ++++++++-----
 lib/librte_eal/common/eal_common_options.c    | 73 ++++++++++++-------
 lib/librte_eal/common/eal_log.h               | 32 ++++++++
 lib/librte_eal/common/eal_private.h           | 29 --------
 lib/librte_eal/freebsd/eal.c                  | 10 +--
 lib/librte_eal/include/rte_eal.h              |  2 +
 lib/librte_eal/include/rte_log.h              | 12 +++
 lib/librte_eal/linux/eal.c                    | 17 +++--
 lib/librte_eal/linux/eal_log.c                |  4 +-
 lib/librte_eal/version.map                    |  1 +
 lib/librte_eal/windows/eal.c                  | 15 ++--
 lib/librte_eal/windows/eal_log.c              |  6 +-
 29 files changed, 247 insertions(+), 177 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_log.h
  

Comments

David Marchand March 24, 2021, 3:03 p.m. UTC | #1
On Sun, Mar 21, 2021 at 11:31 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> The main intent of this series is to provide a nice help
> for the --log-level option.

The changes on usage() and the --log-level help are not related and
could have been separated in two series.

Like a series with:
  eal: use macros for help option
  eal: move private log functions
  eal: introduce maximum log level macro
  eal: catch invalid log level number
  eal: add log level help

About these --log-level related patches, I am ok with them (with just
a comment on sorting logtypes).

Then a series with:
  eal: explain argv behaviour during init
  eal: improve options usage text
  app: fix exit messages
  app: hook in EAL usage help
  app/regex: fix usage text
  app/testpmd: fix usage text

For the usage() patches, the main enhancement comes from hooking the
app/ usage() to rte_set_application_usage_hook().

As for the "eal: improve options usage text" patch, there are two
changes in behavior.

Taking testpmd as an example:

"""
Usage: ./build/app/dpdk-testpmd [EAL options] -- [testpmd options]

  --interactive: run in interactive mode.
  --cmdline-file: execute cli commands before startup.
  --auto-start: start forwarding on init [always when non-interactive].
  --help: display this message and quit.
  --tx-first: start forwarding sending a burst first (only if
interactive is disabled).
  --stats-period=PERIOD: statistics will be shown every PERIOD seconds
(only if interactive is disabled).
[snip]
  --hairpin-mode=0xXX: bitmask set the hairpin port mode.
    0x10 - explicit Tx rule, 0x02 - hairpin ports paired
    0x01 - hairpin ports loop, 0x00 - hairpin port self

EAL common options:
  -c COREMASK         Hexadecimal bitmask of cores to run on
  -l CORELIST         List of cores to run on
                      The argument format is <c1>[-c2][,c3[-c4],...]
                      where c1, c2, etc are core indexes between 0 and 128
  --lcores COREMAP    Map lcore set to physical cpu set
                      The argument format is
[snip]
"""

- The "Usage: " lists [EAL options] first, so I would expect them to
be listed first.
Hence I am not a fan of the reordering.


- Testpmd options are not identified anymore while it was easier
before to find out about it with the "===== Application Usage ====="
banner.
Applications now must add a banner to differentiate their options from
the EAL ones.
  
Thomas Monjalon March 24, 2021, 4:55 p.m. UTC | #2
24/03/2021 16:03, David Marchand:
> The changes on usage() and the --log-level help are not related and
> could have been separated in two series.

Yes, I will split. It will help merging the non-controversial patches.


> - The "Usage: " lists [EAL options] first, so I would expect them to
> be listed first.
> Hence I am not a fan of the reordering.

If not reordering, we print the EAL options before giving
the line "Usage: ..." with the generic syntax of the application.


> - Testpmd options are not identified anymore while it was easier
> before to find out about it with the "===== Application Usage ====="
> banner.
> Applications now must add a banner to differentiate their options from
> the EAL ones.

I think the banner "Application Usage" is strange when printing
(sic) application usage.
The EAL options are already clearly identified,
so I though it was clear enough:

EAL common options:
[...]
EAL options for DEBUG use only:
[...]
EAL Linux options:
[...]