[dpdk-dev] [PATCH v4 01/13] eal: add param register infrastructure
Thomas Monjalon
thomas at monjalon.net
Tue Oct 16 15:42:36 CEST 2018
Hi,
11/10/2018 18:58, Kevin Laatz:
> This commit adds infrastructure to EAL that allows an application to
> register it's init function with EAL. This allows libraries to be
> initialized at the end of EAL init.
>
> This infrastructure allows libraries that depend on EAL to be initialized
> as part of EAL init, removing circular dependency issues.
Let's try to have a clear documentation for this new infra.
> +/**
> + * @file
> + *
> + * This API introduces the ability to register callbacks with EAL. When
You should explain when the callback is called, and what is the role of
the callback.
> + * registering a callback, the application also provides a flag as part of the
> + * struct used to register. If the flag is passed to EAL when ruuning a DPDK
What do you call a flag? Are you talking about an option to be parsed?
> + * application, the relevant callback will be called at the end of EAL init.
> + * For example, passing --telemetry will make the telemetry init be called at
> + * the end of EAl init.
> + *
> + * The register API can be used to resolve circular dependency issue between
> + * EAL and the library.
You need to explain what is the circular dependency issue.
[...]
> +struct rte_param {
Please add a global comment for this struct, explain what it represents.
> + TAILQ_ENTRY(rte_param) next; /** The next entry in the TAILQ*/
> + char *eal_flag; /** The EAL flag */
eal_flag, The EAL flag
Hum...
Please use different words to explain what is a flag.
If it is something to be parsed by getopt, it should be called
an option, not a flag.
> + char *help_text; /** Help text for the callback */
What the help text is used for? When is it printed?
> + rte_param_cb cb; /** Function pointer of the callback */
> + int enabled; /** Enabled flag, should be 0 by default */
What means enabled in this context?
> +};
> +
> +/**
> + * @internal Check if the passed flag is valid
> + *
> + * @param flag
> + * The flag to be parsed
Here too, you need to be more explicit about flag meaning.
> + *
> + * @return
> + * 0 on success
> + * @return
> + * -1 on fail
> + */
> +int
> +rte_param_parse(char *flag);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Register a function with EAL. Registering the function will enable the
> + * function to be called at the end of EAL init.
> + *
> + * @param reg_param
> + * rte_param structure
No, this is not a helpful comment.
> + */
> +void __rte_experimental
> +rte_param_register(struct rte_param *reg_param);
More information about the dev
mailing list