Re: [PATCH v3 5/9] eal: define the EAL parameters in argparse format

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jul 21, 2025 at 10:41:33AM +0200, David Marchand wrote:
> On Fri, Jul 18, 2025 at 4:34 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > +struct eal_init_args {
> > +       /* define a struct member for each EAL option, member name is the same as option name.
> > +        * Parameters that take an argument e.g. -l, are char *,
> > +        * parameters that take no options e.g. --no-huge, are bool.
> > +        * parameters that can be given multiple times e.g. -a, are arg_lists,
> > +        * parameters that are optional e.g. --huge-unlink,
> > +        *   are char * but are set to (void *)1 if the parameter is not given.
> > +        * NOTE: List is to be kept alphabetically by option name
> > +        */
> 
> We will have to be careful to keep this struct below, in sync with the
> options list.
> Could we construct (at build time) this structure via the options list?
> 
> The rules above would be enforced at the same time.
> 
> (macros + #ifdef for Linux/FreeBSD may be an issue for MSVC though...)
> 

I wonder how much an issue keeping them in sync would actually be? When
adding a new option to the options list, it takes a reference to the struct
field, meaning that we can't forget to add an entry here when adding the
option. If we add here but forget to add to the options list, then obvious
the option just doesn't get parsed so testing quickly would show up the
issue.

I suppose the main concern would be around renaming or deleting options -
though in the latter case having an extra stray element in the struct isn't
a big deal.

I might have a shot at auto-generating the struct, but I'm not convinced
the complexity will be worth the benefit.

/Bruce

> > +       struct arg_list allow;
> > +       char *base_virtaddr;
> > +       struct arg_list block;
> > +       char *coremask;
> > +       bool create_uio_dev;
> > +       struct arg_list driver_path;
> > +       char *file_prefix;
> > +       char *force_max_simd_bitwidth;
> > +       char *huge_dir;
> > +       char *huge_unlink;   /* parameter optional */
> > +       char *huge_worker_stack;  /* parameter optional */
> > +       bool in_memory;
> > +       char *iova_mode;
> > +       char *lcores;
> > +       bool legacy_mem;
> > +       char *log_color;   /* parameter optional */
> > +       char *log_level;
> > +       char *log_timestamp;  /* parameter optional */
> > +       char *main_lcore;
> > +       bool match_allocations;
> > +       char *mbuf_pool_ops_name;
> > +       char *memory_channels;
> > +       char *memory_ranks;
> > +       char *memory_size;
> > +       bool no_hpet;
> > +       bool no_huge;
> > +       bool no_pci;
> > +       bool no_shconf;
> > +       bool no_telemetry;
> > +       char *numa_mem;
> > +       char *numa_limit;
> > +       char *proc_type;
> > +       char *service_coremask;
> > +       char *service_corelist;
> > +       bool single_file_segments;
> > +       char *syslog;   /* parameter optional */
> > +       bool telemetry;
> > +       char *trace;
> > +       char *trace_bufsz;
> > +       char *trace_dir;
> > +       char *trace_mode;
> > +       struct arg_list vdev;
> > +       bool version;
> > +       char *vfio_intr;
> > +       char *vfio_vf_token;
> > +       bool vmware_tsc_map;
> > +};
> > +struct eal_init_args args;
> > +
> 
> [snip]
> 
> > +
> > +struct rte_argparse eal_argparse  = {
> > +       .prog_name = "",
> > +       .usage = "<DPDK EAL options>",
> > +       .epilog = "For more information on EAL options, see the DPDK documentation at: \n"
> > +                       "\thttps://doc.dpdk.org/guides/" RTE_EXEC_ENV_NAME "_gsg/",
> 
> Options don't change that often, but I would prefer we point at the
> right release doc, rather than origin/main.
> 
> diff --git a/config/meson.build b/config/meson.build
> index 40f33816aa..c3c054bae9 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -70,6 +70,7 @@ abi_version = run_command(find_program('cat',
> 'more'), abi_version_file,
>  so_version = abi_version.split('.')[0]
> 
>  # extract all version information into the build configuration
> +dpdk_conf.set('RTE_VER_MAJOR', major_version)
>  dpdk_conf.set('RTE_VER_YEAR', pver.get(0).to_int())
>  dpdk_conf.set('RTE_VER_MONTH', pver.get(1).to_int())
>  if pver.get(2).contains('-rc')
> diff --git a/lib/eal/common/eal_common_options.c
> b/lib/eal/common/eal_common_options.c
> index 787f759196..e11d76589f 100644
> --- a/lib/eal/common/eal_common_options.c
> +++ b/lib/eal/common/eal_common_options.c
> @@ -196,7 +196,11 @@ struct rte_argparse eal_argparse  = {
>         .prog_name = "",
>         .usage = "<DPDK EAL options>",
>         .epilog = "For more information on EAL options, see the DPDK
> documentation at: \n"
> +#ifdef RTE_VER_RELEASE == 99
> +
> "\thttps://doc.dpdk.org/guides-"RTE_STR(RTE_VER_MAJOR)"/"
> RTE_EXEC_ENV_NAME "_gsg/",
> +#else
>                         "\thttps://doc.dpdk.org/guides/"
> RTE_EXEC_ENV_NAME "_gsg/",
> +#endif
>         .exit_on_error = true,
>         .callback = arg_list_callback,
>         .print_help = eal_usage,
> 
> 
> -- 
> David Marchand
>