[PATCH v5 1/4] eal: add lcore info in telemetry
Morten Brørup
mb at smartsharesystems.com
Wed Jan 18 11:21:51 CET 2023
> From: Kevin Laatz [mailto:kevin.laatz at intel.com]
> Sent: Wednesday, 18 January 2023 10.42
> To: Robin Jarry; dev at dpdk.org
> Cc: Tyler Retzlaff; Morten Brørup
> Subject: Re: [PATCH v5 1/4] eal: add lcore info in telemetry
>
> On 16/12/2022 10:21, Robin Jarry wrote:
> > Report the same information than rte_lcore_dump() in the telemetry
> > API into /eal/lcore/list and /eal/lcore/info,ID.
> >
> > Example:
> >
> > --> /eal/lcore/info,3
> > {
> > "/eal/lcore/info": {
> > "lcore_id": 3,
> > "socket": 0,
> > "role": "RTE",
> > "cpuset": [
> > 3
> > ]
> > }
> > }
> >
> > Signed-off-by: Robin Jarry <rjarry at redhat.com>
> > Acked-by: Morten Brørup <mb at smartsharesystems.com>
> > ---
> Hi Robin,
>
> Thanks for taking the time to work on this. It is a good implementation
> for debug use-cases.
>
> I have 2 suggestions which would improve the usability of the data:
> 1. Could we make the lcore_id paramater on /eal/lcore/info optional?
> This would allow users to read info for all lcores in the application
> at
> once.
+1 to this suggestion.
> 2. Could we add 2 additional telemetry endpoints? One which returns an
> array of busy_cycles values and the other returns an array of
> total_cycles values. These arrays could be used in conjunction with the
> /eal/lcore/list endpoint to quickly read the usage related metrics.
> I've
> included an example diff below [1].
I prefer this done in a more generic way, see below.
>
> We have a use-case beyond debugging in which we read telemetry every
> few
> milliseconds. From a performance point of view, adding the 2 additional
> endpoints would be very beneficial.
>
> Thanks,
> Kevin
>
> [1]
>
> diff --git a/lib/eal/common/eal_common_lcore.c
> b/lib/eal/common/eal_common_lcore.c
> index 210636d21d..94ddb276c5 100644
> --- a/lib/eal/common/eal_common_lcore.c
> +++ b/lib/eal/common/eal_common_lcore.c
> @@ -569,6 +569,32 @@ handle_lcore_info(const char *cmd __rte_unused,
> const char *params, struct rte_t
> return rte_lcore_iterate(lcore_telemetry_info_cb, &info);
> }
>
> +static int
> +lcore_telemetry_busy_cycles_cb(unsigned int lcore_id, void *arg)
> +{
> + struct rte_tel_data *d = arg;
> + struct rte_lcore_usage usage;
> + rte_lcore_usage_cb usage_cb;
> + unsigned long cycles = 0;
> +
> + memset(&usage, 0, sizeof(usage));
> + usage_cb = lcore_usage_cb;
> + if (usage_cb != NULL && usage_cb(lcore_id, &usage) == 0)
> + cycles = usage.busy_cycles;
> +
> + return rte_tel_data_add_array_u64(d, cycles);
> +}
> +
> +static int
> +handle_lcore_busy_cycles(const char *cmd __rte_unused,
> + const char *params __rte_unused, struct rte_tel_data
> *d)
> +{
> + int ret = rte_tel_data_start_array(d, RTE_TEL_U64_VAL);
> + if (ret)
> + return ret;
> + return rte_lcore_iterate(lcore_telemetry_busy_cycles_cb, d);
> +}
> +
> RTE_INIT(lcore_telemetry)
> {
> rte_telemetry_register_cmd(
> @@ -577,5 +603,8 @@ RTE_INIT(lcore_telemetry)
> rte_telemetry_register_cmd(
> "/eal/lcore/info", handle_lcore_info,
> "Returns lcore info. Parameters: int
> lcore_id");
> + rte_telemetry_register_cmd(
> + "/eal/lcore/busy_cycles",
> handle_lcore_busy_cycles,
> + "List of busy cycle values. Takes no
> parameters");
> }
> #endif /* !RTE_EXEC_ENV_WINDOWS */
This should be generalized to support any named field in the rte_lcore_usage structure.
The general path could be: /eal/lcore/usage
With optional parameter lcore_id. This should return one object (or an array of such objects, if lcore_id is not given) with all usage fields and their values, e.g.:
{
"lcore_id": 7,
"total_cycles": 1234,
"usage_cycles": 567
}
The paths to support the array-optimized feature you are requesting could be: /eal/lcores/usage/total_cycles and /eal/lcores/usage/usage_cycles.
These paths should return the arrays as suggested. I only request that you change "/lcore" to plural "/lcores" and add "/usage" to the path before the field name in the usage table.
Alternatively, you could add a path /eal/lcores/usage_array, taking the field names as parameters and outputting multiple arrays like this:
/eal/lcores/usage_array,total_cycles,usage_cycles
{
"total_cycles": [1234, 1234, 1234],
"usage_cycles": [567, 678, 789]
}
But I don't know if this breaks with DPDK's standard REST interface. It would be easier if we had decided on something like OData, instead of inventing our own.
More information about the dev
mailing list