On 10/15/26 2:17 PM, 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" (only for PMD threads) and "numa"
> - "measurement-duration-s" (seconds as real)
> - "cycles" (used, idle and busy cycles, only for PMD threads)
> - "iterations" (total, idle, busy and sleep iterations, only for PMD
> threads)
> - "sleep" (sleep time, only for PMD threads)
> - "packets" (received, recirculated, transmitted and tx batches)
> - "flow-cache-hits" and "upcalls" (only if packets were received)
> - "averages" (per packet, per batch and per upcall ratios)
>
> Percentages name what they are calculated from, the same way the text
> output does: "used-percentage-of-total" is relative to all cycles in
> the measurement interval, "idle-percentage-of-used" and
> "busy-percentage-of-used" to the used cycles, and
> "sleep-percentage-of-total" to all iterations.
>
> Keys are omitted when the value is not available for a thread, rather
> than being reported as null. Derived values that cannot be calculated
> because their denominator is zero are omitted as well, instead of being
> reported as 0. The rates in kpps and the cycles per packet are integers,
> rounded the same way as in the text output.
>
> When the detailed metrics are enabled, the command options select the
> same additional sections as in text mode: "histograms" unless -nh is
> given, "iteration-history" for -it and "millisecond-history" for -ms.
> Each histogram is an object keyed by the upper bound of each bin, with
> the sample count as value. The last bin, which collects everything
> above the last bound, is keyed "more". The two histories are arrays
> with the most recent sample first.
>
> The counters are read and the arithmetic is done in a single place:
> pmd_perf_format_overall_stats_json() builds the JSON object and
> pmd_perf_format_overall_stats() renders the text output from it, so
> the two formats cannot drift apart.
>
> Example output:
> {"pmd-c03": {"core": 3, "numa": 0, "measurement-duration-s": 1.0,
> "cycles": {"used": 9600, "used-percentage-of-total": 0.4,
> "idle": 9000, "idle-percentage-of-used": 93.75,
> "busy": 600, "busy-percentage-of-used": 6.25},
> "iterations": {"total": 100, "idle": 80, "busy": 20,
> "sleep": 0, "sleep-percentage-of-total": 0,
> "us-per-iteration": 0.04},
> "sleep": {"time-us": 0},
> "packets": {"received": 20, "recirculated": 0,
> "transmitted": 20, "tx-batches": 20},
> "flow-cache-hits": {"partial-hardware-offload": 0,
> "simple": 0, "exact": 19,
> "signature": 0, "megaflow": 0},
> "upcalls": {"success": 1, "failure": 0},
> "averages": {"datapath-passes-per-packet": 1,
> "rx-kpps": 0, "cycles-per-packet": 30,
> "us-per-upcall": 2.5, "tx-kpps": 0,
> "packets-per-tx-batch": 1}}}
>
> Signed-off-by: Timothy Redaelli <[email protected]>
> ---
> NEWS | 3 +-
> lib/dpif-netdev-perf.c | 505 ++++++++++++++++++++++++++++++++++++-----
> lib/dpif-netdev-perf.h | 11 +
> lib/dpif-netdev.c | 107 +++++++--
> tests/pmd.at | 172 ++++++++++++++
> 5 files changed, 724 insertions(+), 74 deletions(-)
>
[...]
I didn't go over the C code very carefully, as I believe it will need some
changes anyway, see below.
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 955be50d1..dfb04ae79 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -484,6 +484,178 @@ 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 cycle,
> +dnl iteration and sleep fields, kpps, cycles-per-packet, and us-per-upcall.
> +AT_CHECK([ovs-appctl --format json --pretty dpif-netdev/pmd-perf-show | dnl
> + sed -e 's/"pmd-c[[0-9]][[0-9]]"/"pmd-c<cleared>"/' dnl
> + -e '/"cycles": {/,/}/s/: [[0-9.e+-]][[0-9.e+-]]*/: <cleared>/'
> dnl
> + -e '/"iterations": {/,/}/s/: [[0-9.e+-]][[0-9.e+-]]*/:
> <cleared>/' dnl
> + -e '/"sleep": {/,/}/s/: [[0-9.e+-]][[0-9.e+-]]*/: <cleared>/'
> dnl
> + -e 's/"core": [[0-9]][[0-9]]*/"core": <cleared>/' dnl
> + -e 's/"measurement-duration-s":
> [[0-9.e+-]]*/"measurement-duration-s": <cleared>/' dnl
> + -e 's/"rx-kpps": [[0-9]][[0-9]]*/"rx-kpps": <cleared>/' dnl
> + -e 's/"tx-kpps": [[0-9]][[0-9]]*/"tx-kpps": <cleared>/' dnl
> + -e 's/"cycles-per-packet":
> [[0-9]][[0-9]]*/"cycles-per-packet": <cleared>/' dnl
> + -e 's/"us-per-upcall": [[0-9.e+-]]*/"us-per-upcall":
> <cleared>/'], [0], [dnl
As with the previous patches, please, use standard line continuation '\'
for commands here and in other places in the tests.
> +{
> + "main": {
> + "measurement-duration-s": <cleared>,
> + "packets": {
> + "received": 0,
> + "recirculated": 0,
> + "transmitted": 0,
> + "tx-batches": 0}},
> + "pmd-c<cleared>": {
> + "averages": {
> + "cycles-per-packet": <cleared>,
> + "datapath-passes-per-packet": 1,
> + "packets-per-tx-batch": 1,
> + "rx-kpps": <cleared>,
> + "tx-kpps": <cleared>,
> + "us-per-upcall": <cleared>},
> + "core": <cleared>,
> + "cycles": {
> + "busy": <cleared>,
> + "busy-percentage-of-used": <cleared>,
> + "idle": <cleared>,
> + "idle-percentage-of-used": <cleared>,
> + "used": <cleared>,
> + "used-percentage-of-total": <cleared>},
This still doesn't look like a good JSON object. We need to try to encode
more things structurally. We may also report more things in order to keep
the output more consistent. In human-readable output we have to balance
readability with the amount of information we're providing. But for JSON
we should lean into easier parsing, i.e. more common structure instead of
custom names. Maybe we could do something like this instead:
"cycles": {
"count": 123,
"used": {
"count": 100,
"percentage": 80,
"busy": {
"count": 80,
"percentage": 80
},
"idle": {
"count": 20,
"percentage": 20
},
"sleep": {
"count": 5,
"percentage": 5
}
}
}
"iterations": {
"count": 100,
"busy": {
"count": 80,
"percentage": 80
},
"idle": {
"count": 15,
"percentage": 15
},
"sleep": {
"count": 5,
"percentage": 5
}
}
Here "percentage" is always the percentage of the "count" from the one
level higher.
> + "flow-cache-hits": {
> + "exact": 19,
> + "megaflow": 0,
> + "partial-hardware-offload": 0,
> + "signature": 0,
> + "simple": 0},
> + "iterations": {
> + "busy": <cleared>,
> + "idle": <cleared>,
> + "sleep": <cleared>,
> + "sleep-percentage-of-total": <cleared>,
> + "total": <cleared>,
> + "us-per-iteration": <cleared>},
"us-per-iteration" should probably go to "averages".
> + "measurement-duration-s": <cleared>,
> + "numa": 0,
> + "packets": {
> + "received": 20,
> + "recirculated": 0,
> + "transmitted": 20,
> + "tx-batches": 20},
"tx-batches" may need to be a standalone field.
> + "sleep": {
> + "time-us": <cleared>},
This one is also a bit weird. It's not clear why ii is an object
if it only has one field. It might be good to pir this with the
measurement duration maybe? They are both time measurements of
the same thing. So, something like:
"time": {
"usec": 4232134343,
"sleep": {
"usec": 1232434,
"percentage": 42
}
}
Here it maybe makes sense to unify the units to usec, so it's easier
to relate them to each other. Once again, we don't need to prioritize
readability here by reporting measurement duration in seconds, an
application reading the data can convert.
WDYT? Eelco? Kevin?
Or maybe an alternative layout with a fully uniform structure:
"pmd-cXX": {
"cycles": xxx, // total cycles of the measurement duration
"iterations": xxx, // total iterations
"usec": xxx, // measurement duration
"used": {
"cycles": xxx, // used cycles
"iterations": xxx, // this will always be equal to "iterations" above
"usec": xxx, // time spent inside iterations
"percentage": {
"cycles": xxx, // % of total cycles
"iterations": xxx, // 100
"usec": xxx // % of total time
},
"busy": {
"cycles": xxx, // cycles in busy iterations
"iterations": xxx, // number of busy iterations
"usec": xxx, // time spent in busy iterations
"percentage": {
"cycles": xxx, // % of used.cycles
"iterations": xxx, // % of used.iterations
"usec": xxx // % of used.usec
},
},
"idle": { // Same for this block
"cycles": xxx,
"iterations": xxx,
"usec": xxx,
"percentage": {
"cycles": xxx,
"iterations": xxx,
"usec": xxx
},
}
},
"sleep": { // Sleep is separate because it is part of both busy and idle
// and we only want absolute numbers and percentages of total
// not of 'used'.
"cycles": xxx, // cycles slept
"iterations": xxx, // iterations that involved sleep
"usec": xxx, // time slept
"percentage": {
"cycles": xxx, // % of total cycles
"iterations": xxx, // % of total iterations
"usec": xxx // % of measurement duration
}
}
}
Thoughts?
> + "upcalls": {
> + "failure": 0,
> + "success": 1}}}
> +])
> +
> +dnl Check that the JSON output honours -nh, -it and -ms.
> +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-perf-metrics=true])
> +AT_CHECK([ovs-appctl time/warp 100], [0], [ignore])
> +
> +AT_CHECK([ovs-appctl --format json --pretty dpif-netdev/pmd-perf-show dnl
> + -it 2 -ms 3 > perf.json])
Instead of redirect, please capture the output with [stdout] and then
do AT_CHECK([mv stdout perf.json]), so we have the data in the testsuite
log as well instead of only in the file.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
