[v4,0/8] add new command line argument parsing library

Message ID 20231215172632.3102502-1-euan.bourke@intel.com (mailing list archive)
Headers
Series add new command line argument parsing library |

Message

Euan Bourke Dec. 15, 2023, 5:26 p.m. UTC
  A recent thread on the mailing list[1] discussed corelist and coremask
parsing and the idea of a new library dedicated to command line parsing
was mentioned[2]. This patchset adds the library, along with the new
APIs, and edits the existing EAL, DLB2 driver and some example
application functions to use these APIs, rather than each implementing
their own copies.

The new APIs work similar to the existing functions in EAL, however
instead of filling a core array like this:
[1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
It fills it like this:
[0, 3, 4] (with the value at each index being an 'active core').

The new APIs will also return the number of cores contained in the
passed corelist/coremask, so in the above example, 3 would be returned.

Also included in this patchest is a heuristic parser which searches
for key markers in the core string, returning a enum value based off
this search to indicate if a parameter is likely a coremask or a
corelist. This heuristic function is also wrapped in a parser
function allowing apps to handle both coremasks and corelists
simultaneously.

[1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
[2] https://mails.dpdk.org/archives/dev/2023-November/280966.html


v4:
* functions now return -EINVAL instead of -1.
* enum moved to header file.
* documentation changes for rte_arg_parse_arg_type() function.
* minor changes for issues flagged during review.

v3:
* new 'combined core string parser' and 'heuristic parser'
* changes to eventdev_pipeline and l3fwd-power example applications
* various struct optimisations in arg_parser.c
* fix for windows build relating to RTE_SWAP()
* minor changes for issues flagged during review

v2:
* changes to EAL service core related parsers to call API.
* various optimisations in core_bit related functions in arg_parser.c.
* add lib to list for windows build.
* minor changes for issues flagged during review.

Euan Bourke (8):
  arg_parser: new library for command line parsing
  arg_parser: add new coremask parsing API
  eal: add support for new arg parsing library
  eal: update to service core related parsers
  event/dlb2: add new arg parsing library API support
  arg_parser: added common core string and heuristic parsers
  examples/eventdev_pipeline: update to call arg parser API
  examples/l3fwd-power: update to call arg parser API

 .mailmap                                     |   1 +
 MAINTAINERS                                  |   4 +
 doc/api/doxy-api-index.md                    |   1 +
 doc/api/doxy-api.conf.in                     |   1 +
 drivers/event/dlb2/dlb2_priv.h               |   4 +-
 drivers/event/dlb2/pf/base/dlb2_resource.c   |  51 ++--
 examples/eventdev_pipeline/main.c            |  66 +----
 examples/eventdev_pipeline/pipeline_common.h |   1 +
 examples/l3fwd-power/perf_core.c             |  52 +---
 lib/arg_parser/arg_parser.c                  | 221 ++++++++++++++
 lib/arg_parser/meson.build                   |   7 +
 lib/arg_parser/rte_arg_parser.h              | 164 +++++++++++
 lib/arg_parser/version.map                   |  13 +
 lib/eal/common/eal_common_options.c          | 285 ++++---------------
 lib/eal/meson.build                          |   2 +-
 lib/meson.build                              |   2 +
 16 files changed, 510 insertions(+), 365 deletions(-)
 create mode 100644 lib/arg_parser/arg_parser.c
 create mode 100644 lib/arg_parser/meson.build
 create mode 100644 lib/arg_parser/rte_arg_parser.h
 create mode 100644 lib/arg_parser/version.map
  

Comments

Stephen Hemminger Dec. 15, 2023, 9:53 p.m. UTC | #1
On Fri, 15 Dec 2023 17:26:24 +0000
Euan Bourke <euan.bourke@intel.com> wrote:

> A recent thread on the mailing list[1] discussed corelist and coremask
> parsing and the idea of a new library dedicated to command line parsing
> was mentioned[2]. This patchset adds the library, along with the new
> APIs, and edits the existing EAL, DLB2 driver and some example
> application functions to use these APIs, rather than each implementing
> their own copies.
> 
> The new APIs work similar to the existing functions in EAL, however
> instead of filling a core array like this:
> [1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
> It fills it like this:
> [0, 3, 4] (with the value at each index being an 'active core').
> 
> The new APIs will also return the number of cores contained in the
> passed corelist/coremask, so in the above example, 3 would be returned.
> 
> Also included in this patchest is a heuristic parser which searches
> for key markers in the core string, returning a enum value based off
> this search to indicate if a parameter is likely a coremask or a
> corelist. This heuristic function is also wrapped in a parser
> function allowing apps to handle both coremasks and corelists
> simultaneously.
> 
> [1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
> [2] https://mails.dpdk.org/archives/dev/2023-November/280966.html
> 

The code is ok, but the naming needs to change.

To me the name argparse implies that library implements something
similar to Python argparse. But that isn't what this library does.
It seems limited to just cpu masks. Perhaps cpuparse would be better name
or make it part of a larger implementation of a full library
more like Python argparse.
  
Bruce Richardson Dec. 18, 2023, 9:18 a.m. UTC | #2
On Fri, Dec 15, 2023 at 01:53:21PM -0800, Stephen Hemminger wrote:
> On Fri, 15 Dec 2023 17:26:24 +0000
> Euan Bourke <euan.bourke@intel.com> wrote:
> 
> > A recent thread on the mailing list[1] discussed corelist and coremask
> > parsing and the idea of a new library dedicated to command line parsing
> > was mentioned[2]. This patchset adds the library, along with the new
> > APIs, and edits the existing EAL, DLB2 driver and some example
> > application functions to use these APIs, rather than each implementing
> > their own copies.
> > 
> > The new APIs work similar to the existing functions in EAL, however
> > instead of filling a core array like this:
> > [1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
> > It fills it like this:
> > [0, 3, 4] (with the value at each index being an 'active core').
> > 
> > The new APIs will also return the number of cores contained in the
> > passed corelist/coremask, so in the above example, 3 would be returned.
> > 
> > Also included in this patchest is a heuristic parser which searches
> > for key markers in the core string, returning a enum value based off
> > this search to indicate if a parameter is likely a coremask or a
> > corelist. This heuristic function is also wrapped in a parser
> > function allowing apps to handle both coremasks and corelists
> > simultaneously.
> > 
> > [1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
> > [2] https://mails.dpdk.org/archives/dev/2023-November/280966.html
> > 
> 
> The code is ok, but the naming needs to change.
> 
> To me the name argparse implies that library implements something
> similar to Python argparse. But that isn't what this library does.
> It seems limited to just cpu masks. Perhaps cpuparse would be better name
> or make it part of a larger implementation of a full library
> more like Python argparse.

Yes, it is a limited set of functions, so the name is probably not ideal if
that's what it's going to remaini (though what library ever stays
un-expanded once started!). I think we need a general discussion
on-list and probably in tech-board too about argument handling, since in
the last couple of months there have been no less than 3 separate
independent proposals around functionality for arguments.

There has been:
* This set, for extracting out functions for processing coremask and
  corelist arguments. Presumably other argument parsing types may look to
  be included in future e.g. device args, perhaps.
* A set from me [1], looking to take the slightly opposite side of things, and 
  make it easier for apps to initialize EAL with a custom argument set. So
  it can be thought of as an argument-list management library.
* An "argparse" library from Chengwen [2] which does what you describe
  above and be a C implementation like the python argparse module.

We need to decide how to manage all these three, if we want to take them
all, and if they should all be merged into a single library, or some kept
separate from others.

/Bruce

[1] http://patches.dpdk.org/project/dpdk/list/?series=30120
[2] http://patches.dpdk.org/project/dpdk/list/?series=30506
  
Thomas Monjalon Jan. 24, 2024, 1:33 p.m. UTC | #3
18/12/2023 10:18, Bruce Richardson:
> On Fri, Dec 15, 2023 at 01:53:21PM -0800, Stephen Hemminger wrote:
> > On Fri, 15 Dec 2023 17:26:24 +0000
> > Euan Bourke <euan.bourke@intel.com> wrote:
> > 
> > > A recent thread on the mailing list[1] discussed corelist and coremask
> > > parsing and the idea of a new library dedicated to command line parsing
> > > was mentioned[2]. This patchset adds the library, along with the new
> > > APIs, and edits the existing EAL, DLB2 driver and some example
> > > application functions to use these APIs, rather than each implementing
> > > their own copies.
> > > 
> > > The new APIs work similar to the existing functions in EAL, however
> > > instead of filling a core array like this:
> > > [1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
> > > It fills it like this:
> > > [0, 3, 4] (with the value at each index being an 'active core').
> > > 
> > > The new APIs will also return the number of cores contained in the
> > > passed corelist/coremask, so in the above example, 3 would be returned.
> > > 
> > > Also included in this patchest is a heuristic parser which searches
> > > for key markers in the core string, returning a enum value based off
> > > this search to indicate if a parameter is likely a coremask or a
> > > corelist. This heuristic function is also wrapped in a parser
> > > function allowing apps to handle both coremasks and corelists
> > > simultaneously.
> > > 
> > > [1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
> > > [2] https://mails.dpdk.org/archives/dev/2023-November/280966.html
> > > 
> > 
> > The code is ok, but the naming needs to change.
> > 
> > To me the name argparse implies that library implements something
> > similar to Python argparse. But that isn't what this library does.
> > It seems limited to just cpu masks. Perhaps cpuparse would be better name
> > or make it part of a larger implementation of a full library
> > more like Python argparse.
> 
> Yes, it is a limited set of functions, so the name is probably not ideal if
> that's what it's going to remaini (though what library ever stays
> un-expanded once started!).

This is a string parsing library.

> I think we need a general discussion
> on-list and probably in tech-board too about argument handling, since in
> the last couple of months there have been no less than 3 separate
> independent proposals around functionality for arguments.
> 
> There has been:
> * This set, for extracting out functions for processing coremask and
>   corelist arguments. Presumably other argument parsing types may look to
>   be included in future e.g. device args, perhaps.
> * A set from me [1], looking to take the slightly opposite side of things, and 
>   make it easier for apps to initialize EAL with a custom argument set. So
>   it can be thought of as an argument-list management library.
> * An "argparse" library from Chengwen [2] which does what you describe
>   above and be a C implementation like the python argparse module.
> 
> We need to decide how to manage all these three, if we want to take them
> all, and if they should all be merged into a single library, or some kept
> separate from others.

Yes, we need to decide whether we want to keep this string parsing
as a standalone library, or we prefer to integrate it in Chengwen's
global argument parsing library.

The question is do we want to do string parsing out of the global argument parsing?
I suppose the answer is yes, at least when parsing devargs in drivers.
In this case we need this library to be standalone (and reused in Chengwen's argparse).
  
Thomas Monjalon Feb. 14, 2024, 5:01 p.m. UTC | #4
24/01/2024 14:33, Thomas Monjalon:
> 18/12/2023 10:18, Bruce Richardson:
> > On Fri, Dec 15, 2023 at 01:53:21PM -0800, Stephen Hemminger wrote:
> > > On Fri, 15 Dec 2023 17:26:24 +0000
> > > Euan Bourke <euan.bourke@intel.com> wrote:
> > > 
> > > > A recent thread on the mailing list[1] discussed corelist and coremask
> > > > parsing and the idea of a new library dedicated to command line parsing
> > > > was mentioned[2]. This patchset adds the library, along with the new
> > > > APIs, and edits the existing EAL, DLB2 driver and some example
> > > > application functions to use these APIs, rather than each implementing
> > > > their own copies.
> > > > 
> > > > The new APIs work similar to the existing functions in EAL, however
> > > > instead of filling a core array like this:
> > > > [1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
> > > > It fills it like this:
> > > > [0, 3, 4] (with the value at each index being an 'active core').
> > > > 
> > > > The new APIs will also return the number of cores contained in the
> > > > passed corelist/coremask, so in the above example, 3 would be returned.
> > > > 
> > > > Also included in this patchest is a heuristic parser which searches
> > > > for key markers in the core string, returning a enum value based off
> > > > this search to indicate if a parameter is likely a coremask or a
> > > > corelist. This heuristic function is also wrapped in a parser
> > > > function allowing apps to handle both coremasks and corelists
> > > > simultaneously.
> > > > 
> > > > [1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
> > > > [2] https://mails.dpdk.org/archives/dev/2023-November/280966.html
> > > > 
> > > 
> > > The code is ok, but the naming needs to change.
> > > 
> > > To me the name argparse implies that library implements something
> > > similar to Python argparse. But that isn't what this library does.
> > > It seems limited to just cpu masks. Perhaps cpuparse would be better name
> > > or make it part of a larger implementation of a full library
> > > more like Python argparse.
> > 
> > Yes, it is a limited set of functions, so the name is probably not ideal if
> > that's what it's going to remaini (though what library ever stays
> > un-expanded once started!).
> 
> This is a string parsing library.
> 
> > I think we need a general discussion
> > on-list and probably in tech-board too about argument handling, since in
> > the last couple of months there have been no less than 3 separate
> > independent proposals around functionality for arguments.
> > 
> > There has been:
> > * This set, for extracting out functions for processing coremask and
> >   corelist arguments. Presumably other argument parsing types may look to
> >   be included in future e.g. device args, perhaps.
> > * A set from me [1], looking to take the slightly opposite side of things, and 
> >   make it easier for apps to initialize EAL with a custom argument set. So
> >   it can be thought of as an argument-list management library.
> > * An "argparse" library from Chengwen [2] which does what you describe
> >   above and be a C implementation like the python argparse module.
> > 
> > We need to decide how to manage all these three, if we want to take them
> > all, and if they should all be merged into a single library, or some kept
> > separate from others.
> 
> Yes, we need to decide whether we want to keep this string parsing
> as a standalone library, or we prefer to integrate it in Chengwen's
> global argument parsing library.
> 
> The question is do we want to do string parsing out of the global argument parsing?
> I suppose the answer is yes, at least when parsing devargs in drivers.
> In this case we need this library to be standalone (and reused in Chengwen's argparse).

The argparse library is now merged,
so we can followup with string parsing proposed in this series.