Re: [PATCH 2/2] ethdev: fix race on ports for telemetry commands

["Robin Jarry" <rjarry@redhat.com>]

David Marchand, Oct 02, 2024 at 21:18:
> On Wed, Oct 2, 2024 at 9:09 PM Robin Jarry <rjarry@redhat.com> wrote:
>> I was going to suggest adding a rte_spinlock_t* parameter to a new
>> telemetry register function that would need to be held while the
>> callback is invoked. Or if we want to keep doors open to other kinds of
>> lock, a wrapper callback.
>
> Well, as you had experimented this approach, we know this does not
> work: the ethdev lock is in dpdk shared memory which is not available
> yet at the time RTE_INIT() is called.
>
> A single callback is strange, I guess you mean pre/post callbacks then.

It could be a single function that will wrap the callbacks. E.g.:

static int
eth_dev_telemetry_with_lock(
    telemetry_cb fn, const char *cmd, const char *params, struct rte_tel_data *d)
{
    int ret;
    rte_spinlock_lock(rte_mcfg_ethdev_get_lock());
    ret = fn(cmd, params, d);
    rte_spinlock_unlock(rte_mcfg_ethdev_get_lock());
    return ret;
}

RTE_INIT(ethdev_init_telemetry)
{
    ....
    rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats,
            "Returns the common stats for a port. Parameters: int port_id",
            eth_dev_telemetry_with_lock);
    ....
}

I'm not sure which solution is the uglier :D