On 9/25/26 12:16 PM, Timothy Redaelli via dev wrote:
> When --format json is passed to ovs-appctl, fdb/stats-show returns a
> JSON object keyed by bridge name. Each bridge contains two sub-objects:
The command requires exactly one bridge...
> "entries" (with "current", "maximum", and "static" counts) and "events"
> (with "evicted", "expired", "learned", and "moved" counters).
>
> The implementation is split into separate text and JSON helper functions
> to keep the main handler simple.
>
> Example output:
> {"br0": {"entries": {"current": 17, "maximum": 8192, "static": 17},
> "events": {"evicted": 0, "expired": 0,
> "learned": 17, "moved": 0}}}
>
> Reported-at: https://issues.redhat.com/browse/FDP-2444
> Acked-by: Eelco Chaudron <[email protected]>
> Signed-off-by: Timothy Redaelli <[email protected]>
> ---
> NEWS | 2 +
> ofproto/ofproto-dpif.c | 94 ++++++++++++++++++++++++++++----------
> tests/ofproto-dpif.at | 15 ++++++
> vswitchd/ovs-vswitchd.8.in | 3 ++
> 4 files changed, 90 insertions(+), 24 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index de1a030ad..0153fa0cb 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,7 @@
> Post-v4.0.0
> --------------------
> + - ovs-appctl:
> + * Added JSON output support (--format json) for 'fdb/stats-show'.
>
>
> v4.0.0 - 17 Aug 2026
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 0f0f71d14..97af249ba 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -6437,43 +6437,89 @@ ofproto_unixctl_fdb_stats_clear(struct unixctl_conn
> *conn, int argc,
> unixctl_command_reply(conn, "statistics successfully cleared");
> }
>
> +static struct json *
> +ofproto_unixctl_fdb_stats_show_json(const struct ofproto_dpif *ofproto)
> +{
> + struct json *entries = json_object_create();
> + struct json *events = json_object_create();
> + struct json *bridge = json_object_create();
> + struct json *json = json_object_create();
> +
> + ovs_rwlock_rdlock(&ofproto->ml->rwlock);
> + json_object_put(entries, "current",
> + json_integer_create(hmap_count(&ofproto->ml->table)));
> + json_object_put(entries, "maximum",
> + json_integer_create(ofproto->ml->max_entries));
> + json_object_put(entries, "static",
> + json_integer_create(ofproto->ml->static_entries));
> + json_object_put(bridge, "entries", entries);
> +
> + json_object_put(events, "evicted",
> + json_integer_create(ofproto->ml->total_evicted));
> + json_object_put(events, "expired",
> + json_integer_create(ofproto->ml->total_expired));
> + json_object_put(events, "learned",
> + json_integer_create(ofproto->ml->total_learned));
> + json_object_put(events, "moved",
> + json_integer_create(ofproto->ml->total_moved));
> + json_object_put(bridge, "events", events);
> + ovs_rwlock_unlock(&ofproto->ml->rwlock);
> +
> + json_object_put(json, ofproto->up.name, bridge);
> + return json;
> +}
> +
> +static void
> +ofproto_unixctl_fdb_stats_show_text(const struct ofproto_dpif *ofproto,
> + struct ds *ds)
> +{
> + ovs_rwlock_rdlock(&ofproto->ml->rwlock);
> + ds_put_format(ds, "Statistics for bridge \"%s\":\n", ofproto->up.name);
> + ds_put_format(ds, " Current/maximum MAC entries in the table: %"
> + PRIuSIZE"/%"PRIuSIZE"\n",
> + hmap_count(&ofproto->ml->table),
> + ofproto->ml->max_entries);
> + ds_put_format(ds,
> + " Current static MAC entries in the table : %"
> + PRIuSIZE"\n", ofproto->ml->static_entries);
> + ds_put_format(ds,
> + " Total number of learned MAC entries : %"
> + PRIu64"\n", ofproto->ml->total_learned);
> + ds_put_format(ds,
> + " Total number of expired MAC entries : %"
> + PRIu64"\n", ofproto->ml->total_expired);
> + ds_put_format(ds,
> + " Total number of evicted MAC entries : %"
> + PRIu64"\n", ofproto->ml->total_evicted);
> + ds_put_format(ds,
> + " Total number of port moved MAC entries : %"
> + PRIu64"\n", ofproto->ml->total_moved);
Hmm. Why changing the way these lines were formatted? The PRIu64 fits
on the same line as the main text in all cases, except for the top one.
And the ofproto->ml->max_entries also fits in on the same line with the
hmap_count. Just makes the diff larger for no apparent reason.
> + ovs_rwlock_unlock(&ofproto->ml->rwlock);
> +}
> +
> static void
> ofproto_unixctl_fdb_stats_show(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> const char *argv[], void *aux OVS_UNUSED)
> {
> - struct ds ds = DS_EMPTY_INITIALIZER;
> const struct ofproto_dpif *ofproto;
> +
> ofproto = ofproto_dpif_lookup_by_name(argv[1]);
> if (!ofproto) {
> unixctl_command_reply_error(conn, "no such bridge");
> return;
> }
>
> - ds_put_format(&ds, "Statistics for bridge \"%s\":\n", argv[1]);
> - ovs_rwlock_rdlock(&ofproto->ml->rwlock);
> + if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> + struct json *json = ofproto_unixctl_fdb_stats_show_json(ofproto);
>
> - ds_put_format(&ds, " Current/maximum MAC entries in the table: %"
> - PRIuSIZE"/%"PRIuSIZE"\n",
> - hmap_count(&ofproto->ml->table), ofproto->ml->max_entries);
> - ds_put_format(&ds,
> - " Current static MAC entries in the table :
> %"PRIuSIZE"\n",
> - ofproto->ml->static_entries);
> - ds_put_format(&ds,
> - " Total number of learned MAC entries : %"PRIu64"\n",
> - ofproto->ml->total_learned);
> - ds_put_format(&ds,
> - " Total number of expired MAC entries : %"PRIu64"\n",
> - ofproto->ml->total_expired);
> - ds_put_format(&ds,
> - " Total number of evicted MAC entries : %"PRIu64"\n",
> - ofproto->ml->total_evicted);
> - ds_put_format(&ds,
> - " Total number of port moved MAC entries : %"PRIu64"\n",
> - ofproto->ml->total_moved);
> + unixctl_command_reply_json(conn, json);
> + } else {
> + struct ds ds = DS_EMPTY_INITIALIZER;
>
> - ovs_rwlock_unlock(&ofproto->ml->rwlock);
> - unixctl_command_reply(conn, ds_cstr(&ds));
> - ds_destroy(&ds);
> + ofproto_unixctl_fdb_stats_show_text(ofproto, &ds);
> + unixctl_command_reply(conn, ds_cstr(&ds));
> + ds_destroy(&ds);
> + }
> }
>
> static void
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index ee6ac873d..207c61ad5 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -8303,6 +8303,21 @@ AT_CHECK_UNQUOTED([ovs-appctl fdb/stats-show br0 |
> grep static], [0], [dnl
> Current static MAC entries in the table : 17
> ])
>
> +dnl Check JSON output.
> +AT_CHECK([ovs-appctl --format json --pretty fdb/stats-show br0], [0], [dnl
> +{
> + "br0": {
If the bridge name is a required argument, why do we need to report it here?
> + "entries": {
> + "current": 17,
> + "maximum": 8192,
> + "static": 17},
> + "events": {
> + "evicted": 0,
> + "expired": 0,
> + "learned": 18,
> + "moved": 0}}}
> +])
> +
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
> diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
> index 3d80bceb6..72517341b 100644
> --- a/vswitchd/ovs-vswitchd.8.in
> +++ b/vswitchd/ovs-vswitchd.8.in
> @@ -191,6 +191,9 @@ Clear \fIbridge\fR MAC address learning table statistics,
> or all
> statistics if no \fIbridge\fR is given.
> .IP "\fBfdb/stats-show\fR \fIbridge\fR"
> Show MAC address learning table statistics for the specified \fIbridge\fR.
> +JSON output is an object keyed by bridge name, whose value holds an
> +"entries" object with the current, maximum and static entry counts and an
> +"events" object with the evicted, expired, learned and moved counters.
Why are we describing the output for JSON case? We should not commit to
any specific format, as it's not a stable API. Also, this description
doesn't really give any more information on what these fields are. The
user can already see what's written here in the output of the command.
I'd suggest to just drop this man page change.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
