On 17 Jun 2026, at 22:46, Timothy Redaelli via dev wrote:
> When --format json is passed to ovs-appctl, pmd-perf-show returns a
> JSON object keyed by thread identifier ("pmd-cNN" for PMD threads,
> "main" for the main thread). Each entry contains:
>
> - "core" and "numa" fields (numeric or null)
> - "measurement-duration-s" (seconds as real)
> - "iterations" (object with cycle/iteration stats, or null for main)
> - "rx-packets" (count, kpps, cycles-per-packet)
> - "datapath-passes" (count, per-packet ratio, all hit types, upcalls)
> - "tx-packets" (count, kpps, batches, packets-per-batch)
>
> Values that depend on TSC cycle accounting (cycles-per-packet,
> us-per-upcall) are null for the main thread.
>
> The JSON formatting function is placed in dpif-netdev-perf.c alongside
> the text formatter, since both need access to the file-local tsc_hz.
>
> Signed-off-by: Timothy Redaelli <[email protected]>
Hi Timothy,
See some comments below. I have copied in Ilya for some of the code
style questions, as he was planning to review the series anyway.
//Eelco
> ---
> lib/dpif-netdev-perf.c | 129 +++++++++++++++++++++++++++++++++++++++++
> lib/dpif-netdev-perf.h | 5 ++
> lib/dpif-netdev.c | 65 +++++++++++++++++----
> tests/pmd.at | 78 +++++++++++++++++++++++++
> 4 files changed, 266 insertions(+), 11 deletions(-)
>
> diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c
> index 4b3b35852..536d15a07 100644
> --- a/lib/dpif-netdev-perf.c
> +++ b/lib/dpif-netdev-perf.c
> @@ -315,6 +315,135 @@ pmd_perf_format_overall_stats(struct ds *str, struct
> pmd_perf_stats *s,
> }
> }
>
> +void
> +pmd_perf_format_overall_stats_json(struct json *json,
> + struct pmd_perf_stats *s,
> + double duration, bool show_iterations)
> +{
> + uint64_t stats[PMD_N_STATS];
> +
> + if (duration == 0) {
> + json_object_put(json, "iterations", json_null_create());
> + json_object_put(json, "rx-packets", json_null_create());
> + json_object_put(json, "datapath-passes", json_null_create());
> + json_object_put(json, "tx-packets", json_null_create());
> + return;
> + }
> +
> + double us_per_cycle = 1000000.0 / tsc_hz;
> +
> + pmd_perf_read_counters(s, stats);
> + uint64_t tot_cycles = stats[PMD_CYCLES_ITER_IDLE] +
> + stats[PMD_CYCLES_ITER_BUSY];
> + uint64_t rx_packets = stats[PMD_STAT_RECV];
> + uint64_t tx_packets = stats[PMD_STAT_SENT_PKTS];
> + uint64_t tx_batches = stats[PMD_STAT_SENT_BATCHES];
> + uint64_t passes = stats[PMD_STAT_RECV] +
> + stats[PMD_STAT_RECIRC];
> + uint64_t upcalls = stats[PMD_STAT_MISS];
> + uint64_t upcall_cycles = stats[PMD_CYCLES_UPCALL];
> + uint64_t tot_iter = histogram_samples(&s->pkts);
> + uint64_t idle_iter = s->pkts.bin[0];
> + uint64_t busy_iter = tot_iter >= idle_iter ? tot_iter - idle_iter : 0;
> + uint64_t sleep_iter = stats[PMD_SLEEP_ITER];
> + uint64_t tot_sleep_cycles = stats[PMD_CYCLES_SLEEP];
I earlier mentioned I prefer definitions at the top, but given the
number of variables here I think it is fine as is. Ilya, what is
your take?
> + /* Iterations. */
> + if (show_iterations) {
> + struct json *iter = json_object_create();
New line.
> + json_object_put(iter, "total", json_integer_create(tot_iter));
> + json_object_put(iter, "us-per-iteration",
> + json_real_create(tot_iter
> + ? (tot_cycles + tot_sleep_cycles)
> + * us_per_cycle / tot_iter
> + : 0));
> + json_object_put(iter, "used-tsc-cycles",
> + json_integer_create(tot_cycles));
> + json_object_put(iter, "used-tsc-pct",
> + json_real_create(
> + 100.0 * (tot_cycles / duration) / tsc_hz));
> + json_object_put(iter, "idle", json_integer_create(idle_iter));
> + json_object_put(iter, "idle-pct",
> + json_real_create(tot_cycles
> + ? 100.0 * stats[PMD_CYCLES_ITER_IDLE] /
> tot_cycles
> + : 0));
> + json_object_put(iter, "busy", json_integer_create(busy_iter));
> + json_object_put(iter, "busy-pct",
> + json_real_create(tot_cycles
> + ? 100.0 * stats[PMD_CYCLES_ITER_BUSY] /
> tot_cycles
> + : 0));
> + json_object_put(iter, "sleep", json_integer_create(sleep_iter));
> + json_object_put(iter, "sleep-pct",
> + json_real_create(tot_iter
> + ? 100.0 * sleep_iter / tot_iter : 0));
> + json_object_put(iter, "sleep-time-us",
> + json_real_create(tot_sleep_cycles * us_per_cycle));
> + json_object_put(iter, "sleep-us-per-iteration",
> + json_real_create(sleep_iter
> + ? (tot_sleep_cycles * us_per_cycle) / sleep_iter
> + : 0));
All these indentations look odd compared to the rest of the code base.
Mayeb this looks better?
json_object_put(iter, "us-per-iteration",
json_real_create(
tot_iter
? (tot_cycles + tot_sleep_cycles)
* us_per_cycle / tot_iter
: 0));
Ilya, WDYT?
> + json_object_put(json, "iterations", iter);
> + } else {
> + json_object_put(json, "iterations", json_null_create());
> + }
> +
> + /* Rx packets. */
> + struct json *rx = json_object_create();
New line or definition at the top?
> + json_object_put(rx, "count", json_integer_create(rx_packets));
> + json_object_put(rx, "kpps",
> + json_real_create(rx_packets
> + ? (rx_packets / duration) / 1000 : 0));
> + json_object_put(rx, "cycles-per-packet",
> + tot_iter && rx_packets
> + ? json_real_create(
> + 1.0 * stats[PMD_CYCLES_ITER_BUSY] / rx_packets)
> + : json_null_create());
> + json_object_put(json, "rx-packets", rx);
> +
> + /* Datapath passes. */
> + struct json *dp = json_object_create();
New line or definition at the top?
> + json_object_put(dp, "count", json_integer_create(passes));
> + json_object_put(dp, "passes-per-packet",
> + json_real_create(rx_packets
> + ? 1.0 * passes / rx_packets : 0));
> + json_object_put(dp, "phwol-hits",
> + json_integer_create(stats[PMD_STAT_PHWOL_HIT]));
> + json_object_put(dp, "simple-match-hits",
> + json_integer_create(stats[PMD_STAT_SIMPLE_HIT]));
> + json_object_put(dp, "emc-hits",
> + json_integer_create(stats[PMD_STAT_EXACT_HIT]));
> + json_object_put(dp, "smc-hits",
> + json_integer_create(stats[PMD_STAT_SMC_HIT]));
> + json_object_put(dp, "megaflow-hits",
> + json_integer_create(stats[PMD_STAT_MASKED_HIT]));
> + json_object_put(dp, "subtable-lookups-per-megaflow-hit",
> + json_real_create(stats[PMD_STAT_MASKED_HIT]
> + ? 1.0 * stats[PMD_STAT_MASKED_LOOKUP]
> + / stats[PMD_STAT_MASKED_HIT]
> + : 0));
> + json_object_put(dp, "upcalls", json_integer_create(upcalls));
> + json_object_put(dp, "us-per-upcall",
> + tot_iter
> + ? json_real_create(upcalls
> + ? (upcall_cycles * us_per_cycle) / upcalls : 0)
> + : json_null_create());
> + json_object_put(dp, "lost-upcalls",
> + json_integer_create(stats[PMD_STAT_LOST]));
> + json_object_put(json, "datapath-passes", dp);
> +
> + /* Tx packets. */
> + struct json *tx = json_object_create();
New line or definition at the top?
> + json_object_put(tx, "count", json_integer_create(tx_packets));
> + json_object_put(tx, "kpps",
> + json_real_create(tx_packets
> + ? (tx_packets / duration) / 1000 : 0));
> + json_object_put(tx, "batches", json_integer_create(tx_batches));
> + json_object_put(tx, "packets-per-batch",
> + json_real_create(tx_batches
> + ? 1.0 * tx_packets / tx_batches : 0));
> + json_object_put(json, "tx-packets", tx);
> +}
> +
> void
> pmd_perf_format_histograms(struct ds *str, struct pmd_perf_stats *s)
> {
> diff --git a/lib/dpif-netdev-perf.h b/lib/dpif-netdev-perf.h
> index cf4b6e103..9b0cb789a 100644
> --- a/lib/dpif-netdev-perf.h
> +++ b/lib/dpif-netdev-perf.h
> @@ -29,6 +29,7 @@
> #include <rte_cycles.h>
> #endif
>
> +#include "openvswitch/json.h"
> #include "openvswitch/vlog.h"
> #include "ovs-atomic.h"
> #include "timeval.h"
> @@ -423,6 +424,10 @@ struct pmd_perf_params {
>
> void pmd_perf_format_overall_stats(struct ds *str, struct pmd_perf_stats *s,
> double duration, bool format_iterations);
> +void pmd_perf_format_overall_stats_json(struct json *json,
Just 'struct json *,' is engouht here, same for below
'struct pmd_perf_stats *,'.
> + struct pmd_perf_stats *s,
> + double duration,
> + bool show_iterations);
> void pmd_perf_format_histograms(struct ds *str, struct pmd_perf_stats *s);
> void pmd_perf_format_iteration_history(struct ds *str,
> struct pmd_perf_stats *s,
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index b9233ba99..e3b782354 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -641,6 +641,32 @@ format_pmd_thread(struct ds *reply, struct
> dp_netdev_pmd_thread *pmd)
> ds_put_cstr(reply, ":\n");
> }
>
> +static struct json *
> +pmd_info_show_perf_json(struct dp_netdev_pmd_thread *pmd)
> +{
> + long long now = time_msec();
> + double duration = (now - pmd->perf_stats.start_ms) / 1000.0;
> + bool show_iterations = (pmd->core_id != NON_PMD_CORE_ID);
> +
Remove newline
> + struct json *json = json_object_create();
New line
> + json_object_put(json, "core",
> + (pmd->core_id != OVS_CORE_UNSPEC
> + && pmd->core_id != NON_PMD_CORE_ID)
> + ? json_integer_create(pmd->core_id)
> + : json_null_create());
> + json_object_put(json, "numa",
> + pmd->numa_id != OVS_NUMA_UNSPEC
> + ? json_integer_create(pmd->numa_id)
> + : json_null_create());
> + json_object_put(json, "measurement-duration-s",
> + json_real_create(duration));
> +
> + pmd_perf_format_overall_stats_json(json, &pmd->perf_stats,
> + duration, show_iterations);
> +
> + return json;
> +}
> +
> static void
> pmd_info_show_perf(struct ds *reply,
> struct dp_netdev_pmd_thread *pmd,
> @@ -942,6 +968,7 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc,
> const char *argv[],
> / INTERVAL_USEC_TO_SEC;
> bool show_header = true;
> uint64_t max_sleep;
> + struct json *json_result = NULL;
Reverse christmas tree.
> ovs_mutex_lock(&dp_netdev_mutex);
>
> @@ -981,15 +1008,17 @@ 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);
> - return;
> + if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> + if (type == PMD_INFO_PERF_SHOW) {
> + json_result = json_object_create();
> + } else if (type == PMD_INFO_SLEEP_SHOW) {
> + 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);
> + return;
> + }
> }
>
> for (size_t i = 0; i < n; i++) {
> @@ -1016,7 +1045,17 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int
> argc, const char *argv[],
> } else if (type == PMD_INFO_CLEAR_STATS) {
> pmd_perf_stats_clear(&pmd->perf_stats);
> } else if (type == PMD_INFO_PERF_SHOW) {
> - pmd_info_show_perf(&reply, pmd, (struct pmd_perf_params *)aux);
> + if (json_result) {
> + bool is_main = pmd->core_id == NON_PMD_CORE_ID;
> + char *key = is_main
> + ? xstrdup("main")
> + : xasprintf("pmd-c%02u", pmd->core_id);
> + json_object_put_nocopy(json_result, key,
> + pmd_info_show_perf_json(pmd));
> + } else {
> + pmd_info_show_perf(&reply, pmd,
> + (struct pmd_perf_params *) aux);
> + }
I would not do this json part here, move all JSON handling to the
'if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {'
branch.
> } else if (type == PMD_INFO_SLEEP_SHOW) {
> if (show_header) {
> ds_put_format(&reply, "Default max sleep: %4"PRIu64" us\n",
> @@ -1032,7 +1071,11 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int
> argc, const char *argv[],
>
> ovs_mutex_unlock(&dp_netdev_mutex);
>
> - unixctl_command_reply(conn, ds_cstr(&reply));
> + if (json_result) {
> + unixctl_command_reply_json(conn, json_result);
> + } else {
> + unixctl_command_reply(conn, ds_cstr(&reply));
> + }
> ds_destroy(&reply);
> }
>
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 79ee2d470..9a2865b8f 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -484,6 +484,84 @@ pmd thread numa_id <cleared> core_id <cleared>:
> Tx batches: 20 (1.00 pkts/batch)
> ])
>
> +dnl Check JSON output.
> +dnl Mask dynamic values: core id, measurement duration, all iteration fields,
> +dnl kpps, cycles-per-packet, and us-per-upcall.
> +AT_CHECK([ovs-appctl --format json --pretty dpif-netdev/pmd-perf-show | dnl
> + sed 's/"pmd-c[[0-9]][[0-9]]"/"pmd-c<cleared>"/g' | dnl
> + sed 's/"core": [[0-9]][[0-9]]*/"core": <cleared>/g' | dnl
> + sed 's/"measurement-duration-s":
> [[0-9.]][[0-9.]]*/"measurement-duration-s": <cleared>/g' | dnl
> + sed 's/"kpps": [[0-9.]][[0-9.]]*/"kpps": <cleared>/g' | dnl
> + sed 's/"cycles-per-packet": [[0-9.]][[0-9.]]*/"cycles-per-packet":
> <cleared>/g' | dnl
> + sed 's/"us-per-upcall": [[0-9.]][[0-9.]]*/"us-per-upcall":
> <cleared>/g' | dnl
> + sed '/"iterations": {/,/}/{ s/: [[0-9.e+-]][[0-9.e+-]]*/:
> <cleared>/g; }'], [0], [dnl
> +{
> + "main": {
> + "core": null,
> + "datapath-passes": {
> + "count": 0,
> + "emc-hits": 0,
> + "lost-upcalls": 0,
> + "megaflow-hits": 0,
> + "passes-per-packet": 0,
> + "phwol-hits": 0,
> + "simple-match-hits": 0,
> + "smc-hits": 0,
> + "subtable-lookups-per-megaflow-hit": 0,
> + "upcalls": 0,
> + "us-per-upcall": null},
> + "iterations": null,
> + "measurement-duration-s": <cleared>,
> + "numa": null,
> + "rx-packets": {
> + "count": 0,
> + "cycles-per-packet": null,
> + "kpps": <cleared>},
> + "tx-packets": {
> + "batches": 0,
> + "count": 0,
> + "kpps": <cleared>,
> + "packets-per-batch": 0}},
> + "pmd-c<cleared>": {
> + "core": <cleared>,
> + "datapath-passes": {
> + "count": 20,
> + "emc-hits": 19,
> + "lost-upcalls": 0,
> + "megaflow-hits": 0,
> + "passes-per-packet": 1,
> + "phwol-hits": 0,
> + "simple-match-hits": 0,
> + "smc-hits": 0,
> + "subtable-lookups-per-megaflow-hit": 0,
> + "upcalls": 1,
> + "us-per-upcall": <cleared>},
> + "iterations": {
> + "busy": <cleared>,
> + "busy-pct": <cleared>,
> + "idle": <cleared>,
> + "idle-pct": <cleared>,
> + "sleep": <cleared>,
> + "sleep-pct": <cleared>,
> + "sleep-time-us": <cleared>,
> + "sleep-us-per-iteration": <cleared>,
> + "total": <cleared>,
> + "us-per-iteration": <cleared>,
> + "used-tsc-cycles": <cleared>,
> + "used-tsc-pct": <cleared>},
> + "measurement-duration-s": <cleared>,
> + "numa": 0,
> + "rx-packets": {
> + "count": 20,
> + "cycles-per-packet": <cleared>,
> + "kpps": <cleared>},
> + "tx-packets": {
> + "batches": 20,
> + "count": 20,
> + "kpps": <cleared>,
> + "packets-per-batch": 1}}}
> +])
> +
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
> --
> 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
