On 17 Jun 2026, at 22:46, Timothy Redaelli via dev wrote:
> When --format json is passed to ovs-appctl, dpctl/show returns a JSON
> object keyed by datapath name. Each entry contains "flows", "lookups"
> (hit/lost/missed), and "ports" (keyed by port name, each with
> "port-number" and "type"). Optional sections "masks" and "cache" are
> present as null when the datapath does not support them. Port "config"
> and "statistics" sub-objects are included when applicable.
>
> The output_format field is added to struct dpctl_params so the handler
> can select the appropriate callback (show_dpif or show_dpif_json).
> The JSON accumulator is stored in dpctl_params.json, and the reply
> is sent by dpctl_unixctl_handler based on whether json is set. On
> error, the JSON object is discarded and a text error reply is sent.
>
> Example output:
> {"ovs-system": {"flows": 0,
> "lookups": {"hit": 0, "lost": 0, "missed": 0},
> "masks": null, "cache": null,
> "ports": {"br0": {"port-number": 0,
> "type": "internal"}}}}
>
> Signed-off-by: Timothy Redaelli <[email protected]>
Hi Timothy,
See some comments below.
Cheers,
Eelco
> ---
> NEWS | 3 +
> lib/dpctl.c | 233 ++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/dpctl.h | 11 +++
> tests/dpctl.at | 19 ++++
> 4 files changed, 263 insertions(+), 3 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index a64fa065d..f01bc7c7c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,9 @@ Post-v3.7.0
> 'dpdk-probe-at-init' config option, see ovs-vswitchd.conf.db(5)).
> * OVS validated with DPDK 25.11.2.
> - ovs-appctl:
> + * Added JSON output support (--format json) for fdb/stats-show,
> + upcall/show, dpif-netdev/pmd-sleep-show, dpif-netdev/pmd-perf-show,
> + and dpctl/show commands.
> * 'dpif-netdev/pmd-stats-show' command was removed in favor of the more
> informative and better structured 'dpif-netdev/pmd-perf-show', which
> now also provides statistics for the "main" thread.
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index 752168b5a..19a5d7098 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -49,6 +49,7 @@
> #include "smap.h"
> #include "sset.h"
> #include "timeval.h"
> +#include "openvswitch/json.h"
The includes in this file are sorted alphabetically on the filename,
ignoring the directory prefix. Can "openvswitch/json.h" be moved up
to between "flow.h" and "openvswitch/match.h"? I know its odd...
> #include "unixctl.h"
> #include "util.h"
> #include "openvswitch/ofp-flow.h"
> @@ -635,6 +636,215 @@ show_dpif_cache(struct dpif *dpif, struct dpctl_params
> *dpctl_p)
> show_dpif_cache__(dpif, dpctl_p);
> }
>
> +/* Returns a JSON integer for 'value', or a JSON null if 'value' is
> + * UINT64_MAX, which indicates that the statistic is unsupported. */
> +static struct json *
> +json_integer_or_null_create(uint64_t value)
> +{
> + return value != UINT64_MAX
> + ? json_integer_create(value)
> + : json_null_create();
> +}
> +
> +static void
> +show_dpif_json(struct dpif *dpif, struct dpctl_params *dpctl_p)
> +{
> + struct json *json_dps = dpctl_p->json;
> + struct json *json_dp = json_object_create();
> + struct dpif_dp_stats stats;
> +
> + if (!dpif_get_dp_stats(dpif, &stats)) {
> + uint64_t n_pkts = stats.n_hit + stats.n_missed;
> + struct json *json_lookups = json_object_create();
Christmas tree order
> +
> + json_object_put(json_lookups, "hit",
> + json_integer_create(stats.n_hit));
> + json_object_put(json_lookups, "lost",
> + json_integer_create(stats.n_lost));
> + json_object_put(json_lookups, "missed",
> + json_integer_create(stats.n_missed));
> + json_object_put(json_dp, "flows",
> json_integer_create(stats.n_flows));
> + json_object_put(json_dp, "lookups", json_lookups);
> +
> + if (stats.n_masks != UINT32_MAX) {
> + double avg = n_pkts ? (double) stats.n_mask_hit / n_pkts : 0.0;
> + struct json *json_masks = json_object_create();
> +
> + json_object_put(json_masks, "hit",
> + json_integer_create(stats.n_mask_hit));
> + json_object_put(json_masks, "hit-per-packet",
> + json_real_create(avg));
> + json_object_put(json_masks, "total",
> + json_integer_create(stats.n_masks));
> + json_object_put(json_dp, "masks", json_masks);
> + } else {
> + json_object_put(json_dp, "masks", json_null_create());
> + }
> +
> + if (stats.n_cache_hit != UINT64_MAX) {
> + double avg_hits = n_pkts
> + ? (double) stats.n_cache_hit / n_pkts * 100 : 0.0;
> + struct json *json_cache = json_object_create();
> +
> + json_object_put(json_cache, "hit",
> + json_integer_create(stats.n_cache_hit));
> + json_object_put(json_cache, "hit-rate-percentage",
> + json_real_create(avg_hits));
> + json_object_put(json_dp, "cache", json_cache);
> + } else {
> + json_object_put(json_dp, "cache", json_null_create());
> + }
> + } else {
> + json_object_put(json_dp, "cache", json_null_create());
> + json_object_put(json_dp, "flows", json_null_create());
> + json_object_put(json_dp, "lookups", json_null_create());
> + json_object_put(json_dp, "masks", json_null_create());
> + }
> +
> + uint32_t nr_caches;
Define at the top.
> + if (!dpif_cache_get_supported_levels(dpif, &nr_caches) && nr_caches > 0)
> {
> + struct json *json_caches = json_object_create();
> +
> + for (int i = 0; i < nr_caches; i++) {
> + const char *name;
> + uint32_t size;
> +
> + if (dpif_cache_get_name(dpif, i, &name) ||
> + dpif_cache_get_size(dpif, i, &size)) {
> + continue;
> + }
> + struct json *json_c = json_object_create();
> +
> + json_object_put(json_c, "size", json_integer_create(size));
> + json_object_put(json_caches, name, json_c);
> + }
> + json_object_put(json_dp, "caches", json_caches);
> + } else {
> + json_object_put(json_dp, "caches", json_null_create());
> + }
> +
> + struct json *json_ports = json_object_create();
> + struct dpif_port_dump dump;
> + struct dpif_port dpif_port;
> + odp_port_t *port_nos = NULL;
> + size_t allocated_port_nos = 0, n_port_nos = 0;
Define at the top?
> +
> + DPIF_PORT_FOR_EACH (&dpif_port, &dump, dpif) {
> + if (n_port_nos >= allocated_port_nos) {
> + port_nos = x2nrealloc(port_nos, &allocated_port_nos,
> + sizeof *port_nos);
> + }
> + port_nos[n_port_nos++] = dpif_port.port_no;
> + }
> +
> + if (port_nos) {
> + qsort(port_nos, n_port_nos, sizeof *port_nos, compare_port_nos);
> + }
> +
> + for (int i = 0; i < n_port_nos; i++) {
struct json *json_port;
> + struct netdev *netdev;
> +
> + if (dpif_port_query_by_number(dpif, port_nos[i], &dpif_port, true)) {
> + continue;
> + }
> +
> + struct json *json_port = json_object_create();
json_port = json_object_create();
> + json_object_put(json_port, "port-number",
> + json_integer_create(odp_to_u32(dpif_port.port_no)));
> + json_object_put_string(json_port, "type", dpif_port.type);
> +
> + if (strcmp(dpif_port.type, "system")) {
> + int error = netdev_open(dpif_port.name, dpif_port.type, &netdev);
> +
> + if (!error) {
> + struct smap config;
> +
> + smap_init(&config);
error = netdev_get_config(), see comment below.
> + if (!netdev_get_config(netdev, &config) &&
> + smap_count(&config) > 0) {
> + json_object_put(json_port, "config",
> + smap_to_json(&config));
> + } else {
> + json_object_put(json_port, "config",
> + json_null_create());
> + }
> + smap_destroy(&config);
> + netdev_close(netdev);
> + } else {
Can we do 'if (error) {' so we can skip the above null
create?
> + json_object_put(json_port, "config", json_null_create());
> + }
> + } else {
> + json_object_put(json_port, "config", json_null_create());
> + }
> +
> + if (dpctl_p->print_statistics) {
> + struct netdev_stats s;
> + int error = netdev_open(dpif_port.name, dpif_port.type, &netdev);
Order
> + if (!error) {
> + error = netdev_get_stats(netdev, &s);
Newline
> + netdev_close(netdev);
> + if (!error) {
> + struct json *json_stats = json_object_create();
> +
> + json_object_put(json_stats, "collisions",
> +
> json_integer_or_null_create(s.collisions));
> + json_object_put(json_stats, "rx-bytes",
> + json_integer_or_null_create(s.rx_bytes));
> + json_object_put(json_stats, "rx-dropped",
> +
> json_integer_or_null_create(s.rx_dropped));
> + json_object_put(json_stats, "rx-errors",
> +
> json_integer_or_null_create(s.rx_errors));
> + json_object_put(json_stats, "rx-frame-errors",
> + json_integer_or_null_create(
> + s.rx_frame_errors));
> + json_object_put(json_stats, "rx-over-errors",
> + json_integer_or_null_create(
> + s.rx_over_errors));
> + json_object_put(json_stats, "rx-packets",
> +
> json_integer_or_null_create(s.rx_packets));
> + json_object_put(json_stats, "tx-aborted-errors",
> + json_integer_or_null_create(
> + s.tx_aborted_errors));
> + json_object_put(json_stats, "tx-bytes",
> + json_integer_or_null_create(s.tx_bytes));
> + json_object_put(json_stats, "tx-carrier-errors",
> + json_integer_or_null_create(
> + s.tx_carrier_errors));
> + json_object_put(json_stats, "tx-dropped",
> +
> json_integer_or_null_create(s.tx_dropped));
> + json_object_put(json_stats, "tx-errors",
> +
> json_integer_or_null_create(s.tx_errors));
> + json_object_put(json_stats, "tx-packets",
> +
> json_integer_or_null_create(s.tx_packets));
> + json_object_put(json_stats, "upcall-errors",
> + json_integer_or_null_create(
> + s.upcall_errors));
> + json_object_put(json_stats, "upcall-packets",
> + json_integer_or_null_create(
> + s.upcall_packets));
> + json_object_put(json_port, "statistics", json_stats);
> + } else {
> + json_object_put(json_port, "statistics",
> + json_null_create());
> + }
> + } else {
Can we do 'if (error) {' so we can skip the above null create?
Or maybe move it down even lower to skip one more null create.
> + json_object_put(json_port, "statistics", json_null_create());
> + }
> + } else {
> + json_object_put(json_port, "statistics", json_null_create());
> + }
> +
> + json_object_put(json_ports, dpif_port.name, json_port);
> + dpif_port_destroy(&dpif_port);
> + }
> +
> + free(port_nos);
> + json_object_put(json_dp, "ports", json_ports);
> + json_object_put(json_dps, dpif_name(dpif), json_dp);
> +}
> +
> static void
> show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p)
> {
> @@ -833,6 +1043,15 @@ static int
> dpctl_show(int argc, const char *argv[], struct dpctl_params *dpctl_p)
> {
> int error, lasterror = 0;
> + dps_for_each_cb cb;
> +
> + if (dpctl_p->output_format == UNIXCTL_OUTPUT_FMT_JSON) {
> + dpctl_p->json = json_object_create();
> + cb = show_dpif_json;
> + } else {
> + cb = show_dpif;
> + }
> +
> if (argc > 1) {
> int i;
> for (i = 1; i < argc; i++) {
> @@ -841,7 +1060,7 @@ dpctl_show(int argc, const char *argv[], struct
> dpctl_params *dpctl_p)
>
> error = parsed_dpif_open(name, false, &dpif);
> if (!error) {
> - show_dpif(dpif, dpctl_p);
> + cb(dpif, dpctl_p);
> dpif_close(dpif);
> } else {
> dpctl_error(dpctl_p, error, "opening datapath %s failed",
> @@ -850,7 +1069,7 @@ dpctl_show(int argc, const char *argv[], struct
> dpctl_params *dpctl_p)
> }
> }
> } else {
> - lasterror = dps_for_each(dpctl_p, show_dpif);
> + lasterror = dps_for_each(dpctl_p, cb);
> }
>
> return lasterror;
> @@ -3153,6 +3372,7 @@ dpctl_unixctl_handler(struct unixctl_conn *conn, int
> argc, const char *argv[],
> .is_appctl = true,
> .output = dpctl_unixctl_print,
> .aux = &ds,
> + .output_format = unixctl_command_get_output_format(conn),
> };
>
> /* Parse options (like getopt). Unfortunately it does
> @@ -3220,7 +3440,14 @@ dpctl_unixctl_handler(struct unixctl_conn *conn, int
> argc, const char *argv[],
> error = handler(argc, argv, &dpctl_p) != 0;
> }
>
> - if (error) {
> + if (dpctl_p.json) {
> + if (error) {
> + json_destroy(dpctl_p.json);
> + unixctl_command_reply_error(conn, ds_cstr(&ds));
Is ds set in all error cases for json?
> + } else {
> + unixctl_command_reply_json(conn, dpctl_p.json);
> + }
> + } else if (error) {
> unixctl_command_reply_error(conn, ds_cstr(&ds));
> } else {
> unixctl_command_reply(conn, ds_cstr(&ds));
> diff --git a/lib/dpctl.h b/lib/dpctl.h
> index 9d0052152..fe475471e 100644
> --- a/lib/dpctl.h
> +++ b/lib/dpctl.h
> @@ -19,6 +19,9 @@
> #include <stdbool.h>
>
> #include "compiler.h"
> +#include "unixctl.h"
> +
> +struct json;
>
> struct dpctl_params {
> /* True if it is called by ovs-appctl command. */
> @@ -51,6 +54,14 @@ struct dpctl_params {
>
> /* 'usage' (if != NULL) gets called for the "help" command. */
> void (*usage)(void *aux);
> +
> + /* Output format requested by the caller. */
> + enum unixctl_output_fmt output_format;
> +
> + /* JSON object for accumulating structured output. Command handlers
> + * set this to a non-NULL value when producing JSON output; the caller
> + * uses it to decide whether to send a JSON or text reply. */
> + struct json *json;
> };
>
> int dpctl_run_command(int argc, const char *argv[],
> diff --git a/tests/dpctl.at b/tests/dpctl.at
> index a87f67f98..7fbeb6745 100644
> --- a/tests/dpctl.at
> +++ b/tests/dpctl.at
> @@ -25,6 +25,25 @@ dummy@br0:
> flows: 0
> port 0: br0 (dummy-internal)
> ])
Add a blank line.
> +dnl Check dpctl/show JSON output.
> +AT_CHECK([ovs-appctl --format json --pretty dpctl/show dummy@br0], [0], [dnl
> +{
> + "dummy@br0": {
> + "cache": null,
> + "caches": null,
> + "flows": 0,
> + "lookups": {
> + "hit": 0,
> + "lost": 0,
> + "missed": 0},
> + "masks": null,
> + "ports": {
> + "br0": {
> + "config": null,
> + "port-number": 0,
> + "statistics": null,
> + "type": "dummy-internal"}}}}
> +])
Add black line here?
> AT_CHECK([ovs-appctl dpctl/add-if dummy@br0 vif1.0,type=dummy,port_no=5])
> AT_CHECK([ovs-appctl dpctl/show dummy@br0], [0], [dnl
> dummy@br0:
> --
> 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
