On Mon, 14 Oct 2024 21:32:37 +0200
Robin Jarry <rja...@redhat.com> wrote:
> While invoking telemetry commands (which may happen at any time, out of
> control of the application), an application thread may concurrently
> add/remove ports. The telemetry callbacks may then access partially
> initialized/uninitialised ethdev data.
>
> Reuse the ethdev lock that protects port allocation/destruction and the
> new telemetry callback register api that takes an additional private
> argument. Pass eth_dev_telemetry_do as the main callback and the actual
> endpoint callbacks as private argument.
>
> Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Robin Jarry <rja...@redhat.com>
> Acked-by: Bruce Richardson <bruce.richard...@intel.com>
> ---
> lib/ethdev/rte_ethdev_telemetry.c | 66 ++++++++++++++++++++++---------
> 1 file changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/lib/ethdev/rte_ethdev_telemetry.c
> b/lib/ethdev/rte_ethdev_telemetry.c
> index 6b873e7abe68..7599fa2852b6 100644
> --- a/lib/ethdev/rte_ethdev_telemetry.c
> +++ b/lib/ethdev/rte_ethdev_telemetry.c
> @@ -1395,45 +1395,73 @@ eth_dev_handle_port_tm_node_caps(const char *cmd
> __rte_unused,
> return ret;
> }
>
> +static int eth_dev_telemetry_do(const char *cmd, const char *params,
> + void *arg, struct rte_tel_data *d)
> +{
> + int ret;
> + telemetry_cb fn = arg;
> + rte_spinlock_lock(rte_mcfg_ethdev_get_lock());
> + ret = fn(cmd, params, d);
> + rte_spinlock_unlock(rte_mcfg_ethdev_get_lock());
> + return ret;
> +}
If this happens often, and the function takes a long time (like doing i/o)
it might be worth changing this to reader/writer in future.
Also, would be best to add a comment here as to what is being protected
if you do another version.
Acked-by: Stephen Hemminger <step...@networkplumber.org>
