On 17 Jun 2026, at 22:46, 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}}
>
> Signed-off-by: Timothy Redaelli <[email protected]>

Hi Timothy,

Some small comments below. The rest looks good to me.

//Eelco

> ---
>  ofproto/ofproto-dpif-upcall.c | 94 +++++++++++++++++++++++++++++------
>  tests/ofproto-dpif.at         | 29 +++++++++++
>  2 files changed, 108 insertions(+), 15 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 8e4897202..1616b66cc 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"
> @@ -3160,13 +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_udpif = json_object_create();
> +        struct json *json_revalidators = json_object_create();

Move this line up one row.

> +        struct json *json_flows = 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 = json_object_create();
> +            char *key = xasprintf("%u", revalidator->id);

Reverse christmas tree order?

> +            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, "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;
> +    uint64_t n_offloaded_flows;
>
>      LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
>          unsigned int flow_limit;
> @@ -3176,23 +3229,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 +3254,23 @@ 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;

Add a new line here.

> +        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 b5df060b0..4df10cc80 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -13879,6 +13879,10 @@ AT_SETUP([ofproto-dpif - Cleanup missing datapath 
> flows])
>  OVS_VSWITCHD_START
>  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)],
> @@ -13899,6 +13903,31 @@ 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 has the expected structure.
> +AT_CHECK([ovs-appctl --format json --pretty upcall/show | dnl
> +          sed 's/"dummy@[[^"]]*"/"<datapath>"/g' | 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
> +{
> +  "<datapath>": {
> +    "dump-duration-ms": <cleared>,
> +    "flows": {
> +      "average": <cleared>,
> +      "current": <cleared>,
> +      "limit": <cleared>,
> +      "maximum": <cleared>,
> +      "offloaded": 0},
> +    "revalidators": {
> +      "<revalidator>": {
> +        "keys": <cleared>}},
> +    "ufid-enabled": true}}
> +])
> +
>  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.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

Reply via email to