From: Kevin Laatz [mailto:kevin.la...@intel.com]
Sent: Wednesday, 18 January 2023 10.42
To: Robin Jarry; dev@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 <rja...@redhat.com>
Acked-by: Morten Brørup <m...@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]
}
