On 17 Jun 2026, at 22:46, Timothy Redaelli via dev wrote:
> When --format json is passed to ovs-appctl, pmd-sleep-show returns a
> JSON object with "default-max-sleep-us" and a "threads" object. Each
> thread is keyed by a stable identifier ("pmd-cNN" derived from the core
> ID) and contains "core", "max-sleep-us", and "numa" fields.
>
> The "pmd-cNN" key format is chosen over the full thread name
> ("pmd-cNN/id:XXX") because the thread ID suffix is dynamic and would
> make the key unstable across restarts.
>
> The implementation is split into separate text and JSON helper functions.
>
> Example output:
> {"default-max-sleep-us": 100,
> "threads": {"pmd-c03": {"core": 3, "max-sleep-us": 100,
> "numa": 1}}}
>
> Signed-off-by: Timothy Redaelli <[email protected]>
Hi Timothy,
The patch in general looks good to me. Some small nits below.
//Eelco
> ---
> lib/dpif-netdev.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> tests/pmd.at | 19 +++++++++++++++++++
> 2 files changed, 67 insertions(+)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index a9bd27573..b9233ba99 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -65,6 +65,7 @@
> #include "odp-execute.h"
> #include "odp-util.h"
> #include "openvswitch/dynamic-string.h"
> +#include "openvswitch/json.h"
> #include "openvswitch/list.h"
> #include "openvswitch/match.h"
> #include "openvswitch/ofp-parse.h"
> @@ -877,6 +878,41 @@ dpif_netdev_pmd_rebalance(struct unixctl_conn *conn, int
> argc,
> ds_destroy(&reply);
> }
>
> +static struct json *
> +pmd_info_sleep_show_json(struct dp_netdev *dp,
> + struct dp_netdev_pmd_thread **pmd_list, size_t n)
> +{
> + struct json *json_result = json_object_create();
> + struct json *json_threads = json_object_create();
> +
> + json_object_put(json_result, "default-max-sleep-us",
> + json_integer_create(dp->pmd_max_sleep_default));
> +
> + for (size_t i = 0; i < n; i++) {
> + struct dp_netdev_pmd_thread *pmd = pmd_list[i];
> + uint64_t max_sleep;
> +
> + if (!pmd || pmd->core_id == NON_PMD_CORE_ID) {
> + continue;
> + }
> +
> + struct json *json_pmd = json_object_create();
> + char *key = xasprintf("pmd-c%02u", pmd->core_id);
I am not a fan of inline definitions with assignments. Can the
declarations be moved to the top of the block:
struct dp_netdev_pmd_thread *pmd = pmd_list[i];
struct json *json_pmd;
uint64_t max_sleep;
char *key;
With just the assignments remaining here:
json_pmd = json_object_create();
key = xasprintf("pmd-c%02u", pmd->core_id);
> + atomic_read_relaxed(&pmd->max_sleep, &max_sleep);
> + json_object_put(json_pmd, "core",
> + json_integer_create(pmd->core_id));
> + json_object_put(json_pmd, "max-sleep-us",
> + json_integer_create(max_sleep));
> + json_object_put(json_pmd, "numa",
> + json_integer_create(pmd->numa_id));
> + json_object_put_nocopy(json_threads, key, json_pmd);
> + }
> +
> + json_object_put(json_result, "threads", json_threads);
> + return json_result;
> +}
> +
> static void
> pmd_info_show_sleep(struct ds *reply, unsigned core_id, int numa_id,
> uint64_t pmd_max_sleep)
> @@ -944,6 +980,18 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int
> argc, const char *argv[],
> }
>
> sorted_poll_thread_list(dp, &pmd_list, &n);
> +
> + if (type == PMD_INFO_SLEEP_SHOW
> + && unixctl_command_get_output_format(conn)
> + == UNIXCTL_OUTPUT_FMT_JSON) {
> + struct json *json_result = pmd_info_sleep_show_json(dp, pmd_list, n);
> + free(pmd_list);
> + ovs_mutex_unlock(&dp_netdev_mutex);
> + unixctl_command_reply_json(conn, json_result);
> + ds_destroy(&reply);
Don't think we need the ds_destroy() here as its not being used yet.
> + return;
> + }
> +
> for (size_t i = 0; i < n; i++) {
> struct dp_netdev_pmd_thread *pmd = pmd_list[i];
> if (!pmd) {
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 66fccc1d5..79ee2d470 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -1213,6 +1213,25 @@ pmd thread numa_id 8 core_id 5:
> max sleep: 1 us
> ])
>
> +dnl Check JSON output.
> +AT_CHECK([ovs-appctl --format json --pretty dpif-netdev/pmd-sleep-show],
> [0], [dnl
> +{
> + "default-max-sleep-us": 1,
> + "threads": {
> + "pmd-c00": {
> + "core": 0,
> + "max-sleep-us": 1,
> + "numa": 0},
> + "pmd-c03": {
> + "core": 3,
> + "max-sleep-us": 1,
> + "numa": 1},
> + "pmd-c05": {
> + "core": 5,
> + "max-sleep-us": 1,
> + "numa": 8}}}
> +])
> +
> dnl Check high value max sleep
> get_log_next_line_num
> AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="10000"])
> --
> 2.54.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
