On 13 Mar 2026, at 21:50, Timothy Redaelli via dev wrote:
> When --format json is passed to ovs-appctl, upcall/show returns a JSON
> array containing one object per udpif instance. Each object includes
> flow counts, dump duration, UFID support flag, and a revalidator array.
> The offloaded_flows field is always present (even when zero) to provide
> a consistent schema for consumers.
>
> Example output:
> [{"dump_duration_ms":1,"flows_avg":0,"flows_current":0,
> "flows_limit":10000,"flows_max":0,"name":"ovs-system",
> "offloaded_flows":0,"revalidators":[{"id":4,"keys":0}],
> "ufid_enabled":true}]
>
Hi Timothy,
For this patch, please also split the show function into two separate
functions like patch 1. Also, the unit test should check the full output
rather than using grep.
The current format returns an array, but it should be structured more like
patch 1 with the datapath name as the key. In addition, the flow-related
fields should be grouped together, and the revalidators should be indexed
by their ID rather than in an array. Here's the suggested structure:
{
"dummy@ovs-dummy": {
"dump_duration_ms": 1,
"flows": {
"average": 0,
"current": 0,
"limit": 10000,
"maximum": 0,
"offloaded": 0
},
"revalidators": {
"8": {"keys": 0},
"4": {"keys": 0},
"5": {"keys": 0},
"6": {"keys": 0},
"9": {"keys": 0},
"7": {"keys": 1}
},
"ufid_enabled": true
}
}
You might want to test with multiple datapaths to ensure it works
correctly.
Cheers,
Eelco
> Signed-off-by: Timothy Redaelli <[email protected]>
> ---
> ofproto/ofproto-dpif-upcall.c | 123 ++++++++++++++++++++++++----------
> tests/ofproto-dpif.at | 5 ++
> 2 files changed, 94 insertions(+), 34 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 8e4897202..c99bf4b69 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -26,6 +26,7 @@
> #include "dpif.h"
> #include "dpif-offload.h"
> #include "openvswitch/dynamic-string.h"
> +#include "openvswitch/json.h"
> #include "fail-open.h"
> #include "guarded-list.h"
> #include "latch.h"
> @@ -3164,49 +3165,103 @@ static void
> upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> {
> - struct ds ds = DS_EMPTY_INITIALIZER;
> - uint64_t n_offloaded_flows;
> struct udpif *udpif;
>
> - LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
> - unsigned int flow_limit;
> - bool ufid_enabled;
> - size_t i;
> + if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> + struct json *json_udpifs = json_array_create_empty();
>
> - 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)"
> - " (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",
> - n_offloaded_flows);
> - }
> - 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");
> - } else {
> - ds_put_format(&ds, "false\n");
> + LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
> + struct json *json_udpif = json_object_create();
> + struct json *json_revalidators = json_array_create_empty();
> + 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_string(json_udpif, "name",
> dpif_name(udpif->dpif));
> + json_object_put(json_udpif, "flows_current",
> + json_integer_create(udpif_get_n_flows(udpif)));
> + json_object_put(json_udpif, "flows_avg",
> + json_integer_create(udpif->avg_n_flows));
> + json_object_put(json_udpif, "flows_max",
> + json_integer_create(udpif->max_n_flows));
> + json_object_put(json_udpif, "flows_limit",
> + json_integer_create(flow_limit));
> + json_object_put(json_udpif, "offloaded_flows",
> + 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, "ufid_enabled",
> + json_boolean_create(ufid_enabled));
> +
> + for (i = 0; i < udpif->n_revalidators; i++) {
> + struct revalidator *revalidator = &udpif->revalidators[i];
> + struct json *json_rv = json_object_create();
> + int j, elements = 0;
> +
> + for (j = i; j < N_UMAPS; j += udpif->n_revalidators) {
> + elements += cmap_count(&udpif->ukeys[j].cmap);
> + }
> + json_object_put(json_rv, "id",
> + json_integer_create(revalidator->id));
> + json_object_put(json_rv, "keys",
> + json_integer_create(elements));
> + json_array_add(json_revalidators, json_rv);
> + }
> + json_object_put(json_udpif, "revalidators", json_revalidators);
> + json_array_add(json_udpifs, json_udpif);
> }
> - ds_put_char(&ds, '\n');
> + unixctl_command_reply_json(conn, json_udpifs);
> + } else {
> + struct ds ds = DS_EMPTY_INITIALIZER;
> + uint64_t n_offloaded_flows;
>
> - for (i = 0; i < udpif->n_revalidators; i++) {
> - struct revalidator *revalidator = &udpif->revalidators[i];
> - int j, elements = 0;
> + LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
> + unsigned int flow_limit;
> + bool ufid_enabled;
> + size_t i;
>
> - for (j = i; j < N_UMAPS; j += udpif->n_revalidators) {
> - elements += cmap_count(&udpif->ukeys[j].cmap);
> + 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)"
> + " (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",
> + n_offloaded_flows);
> + }
> + 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");
> + } else {
> + ds_put_format(&ds, "false\n");
> + }
> + ds_put_char(&ds, '\n');
> +
> + for (i = 0; i < udpif->n_revalidators; i++) {
> + struct revalidator *revalidator = &udpif->revalidators[i];
> + int j, elements = 0;
> +
> + 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);
> + 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 9dfdfe29b..911609ed5 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -13617,6 +13617,11 @@ dnl Make sure the ukey exists.
> AT_CHECK([ovs-appctl upcall/show | grep '(keys' | awk '{print $3}' | \
> grep -q '1)'], [0])
>
> +dnl Check upcall/show JSON output.
> +AT_CHECK([ovs-appctl --format json upcall/show | dnl
> + grep '"name"' | grep '"flows_limit"' | dnl
> + grep '"ufid_enabled"' | grep '"revalidators"'], [0], [ignore])
> +
> dnl Delete all datapath flows, and make sure they are gone.
> AT_CHECK([ovs-appctl dpctl/del-flows])
> AT_CHECK([ovs-appctl dpctl/dump-flows --names ], [0], [])
> --
> 2.53.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
