On 9/25/26 12:16 PM, Timothy Redaelli via dev wrote:
> When --format json is passed to ovs-appctl, upcall/show returns a JSON
> object keyed by datapath name. Each entry contains a "flows" sub-object
> (with "average", "current", "limit", "maximum", and "offloaded" counts),
> "dump-duration-ms", "ufid-enabled", and a "revalidators" object indexed
> by revalidator ID.
>
> The implementation is split into separate text and JSON helper functions.
>
> Example output:
> {"ovs-system": {"dump-duration-ms": 1,
> "flows": {"average": 0, "current": 0,
> "limit": 10000, "maximum": 0,
> "offloaded": 0},
> "revalidators": {"4": {"keys": 0}},
> "ufid-enabled": true}}
>
> Acked-by: Eelco Chaudron <[email protected]>
> Signed-off-by: Timothy Redaelli <[email protected]>
> ---
> NEWS | 1 +
> ofproto/ofproto-dpif-upcall.c | 95 +++++++++++++++++++++++++++++------
> tests/ofproto-dpif.at | 49 +++++++++++++++++-
> 3 files changed, 130 insertions(+), 15 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 0153fa0cb..b4c67dbc8 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -2,6 +2,7 @@ Post-v4.0.0
> --------------------
> - ovs-appctl:
> * Added JSON output support (--format json) for 'fdb/stats-show'.
> + * Added JSON output support (--format json) for 'upcall/show'.
There is no need to add so much duplicated text for each command. Just
edit the previous entry adding one more command to the list. Same for
all of the subsequent patches.
>
>
> v4.0.0 - 17 Aug 2026
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 8e4897202..b3b80c8f4 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -28,6 +28,7 @@
> #include "openvswitch/dynamic-string.h"
> #include "fail-open.h"
> #include "guarded-list.h"
> +#include "openvswitch/json.h"
> #include "latch.h"
> #include "openvswitch/list.h"
> #include "netlink.h"
> @@ -3160,11 +3161,65 @@ dp_purge_cb(void *aux, unsigned pmd_id)
> udpif_resume_revalidators(udpif);
> }
>
> +static struct json *
> +upcall_unixctl_show_json(void)
> +{
> + struct json *json_udpifs = json_object_create();
> + struct udpif *udpif;
> +
> + LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
> + struct json *json_revalidators = json_object_create();
> + struct json *json_flows = json_object_create();
> + struct json *json_udpif = json_object_create();
> + uint64_t n_offloaded_flows;
> + unsigned int flow_limit;
> + bool ufid_enabled;
> + size_t i;
> +
> + atomic_read_relaxed(&udpif->flow_limit, &flow_limit);
> + ufid_enabled = udpif_use_ufid(udpif);
> + n_offloaded_flows = dpif_offload_flow_count(udpif->dpif);
> +
> + json_object_put(json_flows, "average",
> + json_integer_create(udpif->avg_n_flows));
> + json_object_put(json_flows, "current",
> + json_integer_create(udpif_get_n_flows(udpif)));
> + json_object_put(json_flows, "limit",
> + json_integer_create(flow_limit));
> + json_object_put(json_flows, "maximum",
> + json_integer_create(udpif->max_n_flows));
> + json_object_put(json_flows, "offloaded",
> + json_integer_create(n_offloaded_flows));
> + json_object_put(json_udpif, "dump-duration-ms",
> + json_integer_create(udpif->dump_duration));
> + json_object_put(json_udpif, "flows", json_flows);
> +
> + for (i = 0; i < udpif->n_revalidators; i++) {
> + struct revalidator *revalidator = &udpif->revalidators[i];
> + struct json *json_rv;
> + int j, elements = 0;
> + char *key;
> +
> + json_rv = json_object_create();
> + key = xasprintf("%u", revalidator->id);
We may want to skip revalidators with id == 0, i.e. the ones that were
created, but not fully initialized yet. It is not a problem for the
text output, as we'll print info about all of them anyway, but in JSON
we'll replace the data with the same key.
Also, why is it a string and not an integer?
> + for (j = i; j < N_UMAPS; j += udpif->n_revalidators) {
> + elements += cmap_count(&udpif->ukeys[j].cmap);
> + }
> + json_object_put(json_rv, "keys", json_integer_create(elements));
> + json_object_put_nocopy(json_revalidators, key, json_rv);
> + }
> + json_object_put(json_udpif, "revalidators", json_revalidators);
> + json_object_put(json_udpif, "ufid-enabled",
> + json_boolean_create(ufid_enabled));
> +
> + json_object_put(json_udpifs, dpif_name(udpif->dpif), json_udpif);
> + }
> + return json_udpifs;
> +}
> +
> static void
> -upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> - const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> +upcall_unixctl_show_text(struct ds *ds)
> {
> - struct ds ds = DS_EMPTY_INITIALIZER;
> uint64_t n_offloaded_flows;
> struct udpif *udpif;
>
> @@ -3176,23 +3231,23 @@ upcall_unixctl_show(struct unixctl_conn *conn, int
> argc OVS_UNUSED,
> atomic_read_relaxed(&udpif->flow_limit, &flow_limit);
> ufid_enabled = udpif_use_ufid(udpif);
>
> - ds_put_format(&ds, "%s:\n", dpif_name(udpif->dpif));
> - ds_put_format(&ds, " flows : (current %lu)"
> + ds_put_format(ds, "%s:\n", dpif_name(udpif->dpif));
> + ds_put_format(ds, " flows : (current %lu)"
> " (avg %u) (max %u) (limit %u)\n", udpif_get_n_flows(udpif),
> udpif->avg_n_flows, udpif->max_n_flows, flow_limit);
> n_offloaded_flows = dpif_offload_flow_count(udpif->dpif);
> if (n_offloaded_flows) {
> - ds_put_format(&ds, " offloaded flows : %"PRIu64"\n",
> + ds_put_format(ds, " offloaded flows : %"PRIu64"\n",
> n_offloaded_flows);
> }
> - ds_put_format(&ds, " dump duration : %lldms\n",
> udpif->dump_duration);
> - ds_put_format(&ds, " ufid enabled : ");
> + ds_put_format(ds, " dump duration : %lldms\n",
> udpif->dump_duration);
> + ds_put_format(ds, " ufid enabled : ");
> if (ufid_enabled) {
> - ds_put_format(&ds, "true\n");
> + ds_put_format(ds, "true\n");
> } else {
> - ds_put_format(&ds, "false\n");
> + ds_put_format(ds, "false\n");
> }
> - ds_put_char(&ds, '\n');
> + ds_put_char(ds, '\n');
>
> for (i = 0; i < udpif->n_revalidators; i++) {
> struct revalidator *revalidator = &udpif->revalidators[i];
> @@ -3201,12 +3256,24 @@ upcall_unixctl_show(struct unixctl_conn *conn, int
> argc OVS_UNUSED,
> for (j = i; j < N_UMAPS; j += udpif->n_revalidators) {
> elements += cmap_count(&udpif->ukeys[j].cmap);
> }
> - ds_put_format(&ds, " %u: (keys %d)\n", revalidator->id,
> elements);
> + ds_put_format(ds, " %u: (keys %d)\n", revalidator->id,
> elements);
> }
> }
> +}
>
> - unixctl_command_reply(conn, ds_cstr(&ds));
> - ds_destroy(&ds);
> +static void
> +upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> +{
> + if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> + unixctl_command_reply_json(conn, upcall_unixctl_show_json());
> + } else {
> + struct ds ds = DS_EMPTY_INITIALIZER;
> +
> + upcall_unixctl_show_text(&ds);
> + unixctl_command_reply(conn, ds_cstr(&ds));
> + ds_destroy(&ds);
> + }
> }
>
> /* Disable using the megaflows.
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 207c61ad5..b7469bee2 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -13922,9 +13922,13 @@ AT_CLEANUP
>
> AT_SETUP([ofproto-dpif - Cleanup missing datapath flows])
>
> -OVS_VSWITCHD_START
> +OVS_VSWITCHD_START([], [], [=override])
> add_of_ports br0 1 2
>
> +dnl Set to 1 revalidator thread early for deterministic output.
> +AT_CHECK([ovs-vsctl set open_vswitch . other_config:n-revalidator-threads=1])
> +OVS_WAIT_UNTIL([ovs-appctl upcall/show | grep -c '(keys' | grep -q '^1$'])
> +
> m4_define([ICMP_PKT], [m4_join([,],
> [eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800)],
> [ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no)],
> @@ -13945,6 +13949,49 @@ dnl Make sure the ukey exists.
> AT_CHECK([ovs-appctl upcall/show | grep '(keys' | awk '{print $3}' | \
> grep -q '1)'], [0])
>
> +dnl Add a second datapath to check the JSON output with multiple datapaths.
> +AT_CHECK([ovs-vsctl add-br br1 -- set bridge br1 datapath-type=netdev])
> +OVS_WAIT_UNTIL([test $(ovs-appctl upcall/show | grep -c '^[[a-z]]*@') = 2])
> +
> +dnl Check upcall/show JSON output has the expected structure.
> +AT_CHECK([ovs-appctl --format json --pretty upcall/show | dnl
> + sed 's/"dump-duration-ms": [[0-9]][[0-9]]*/"dump-duration-ms":
> <cleared>/g' | dnl
> + sed 's/"average": [[0-9]][[0-9]]*/"average": <cleared>/g' | dnl
> + sed 's/"current": [[0-9]][[0-9]]*/"current": <cleared>/g' | dnl
> + sed 's/"limit": [[0-9]][[0-9]]*/"limit": <cleared>/g' | dnl
> + sed 's/"maximum": [[0-9]][[0-9]]*/"maximum": <cleared>/g' | dnl
> + sed 's/"keys": [[0-9]][[0-9]]*/"keys": <cleared>/g' | dnl
> + sed 's/"[[0-9]][[0-9]]*": {/"<revalidator>": {/g'], [0], [dnl
Can we just replace all numbers instead of listing every field separately?
> +{
> + "dummy@ovs-dummy": {
> + "dump-duration-ms": <cleared>,
> + "flows": {
> + "average": <cleared>,
> + "current": <cleared>,
> + "limit": <cleared>,
> + "maximum": <cleared>,
> + "offloaded": 0},
> + "revalidators": {
> + "<revalidator>": {
> + "keys": <cleared>}},
> + "ufid-enabled": true},
> + "netdev@ovs-netdev": {
> + "dump-duration-ms": <cleared>,
> + "flows": {
> + "average": <cleared>,
> + "current": <cleared>,
> + "limit": <cleared>,
> + "maximum": <cleared>,
> + "offloaded": 0},
> + "revalidators": {
> + "<revalidator>": {
> + "keys": <cleared>}},
> + "ufid-enabled": true}}
> +])
These commands do not belong in this test. The test is a specific regression
test for the flow revalidation and this json dump has nothing to do with it.
Either find a more generic test that just checks basic functionality or create
a separate test just for the json stuff.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
