On 4/30/26 10:23 AM, Takeru Hayasaka wrote:
> Previously fdb/show required exactly one bridge argument.  This
> change allows specifying multiple bridge names in a single call,
> reducing the number of unixctl round-trips for clients that need
> to poll FDB entries across many bridges (e.g. EVPN agents managing
> multiple VNIs).
> 
> The output format is now consistent regardless of how many bridges
> are passed.  Text output always prefixes each bridge's table with
> a "bridge <name>" header line.  JSON output is always an array of
> objects, each containing "bridge" and "entries" keys.  This is a
> backward-incompatible change for the single-bridge case but keeps
> the per-bridge formatting fully encapsulated in the helper
> functions.
> 
> Signed-off-by: Takeru Hayasaka <[email protected]>

Hi, Takeru.  Thanks for the patch and sorry for the dealy.

I wonder if what you actually want is to show the fdb tables of
all the bridges?  Or most of the bridges?  We have many different
commands for which the standard behavior is to print information
for all bridges/datapaths/ports in case a specific target is not
specified, i.e. the call has no arguemnts.  So, I wonder if it
would be better to instead of supporting multiple arguments with
support the case of no arguments and show information for all
bridges in this case, as that would be more in line with other
commands we have.

I was also thinking that it may be better to report this via the
datapbase.  However, that may be a little costly on systems with
large FDB tables.  So, appctl solution is probbaly a reasonable
choice.

Some more comments below.

> ---
> v2:
> - Moved the "bridge <name>" prefixing into the per-bridge text and
>   JSON helpers so the formatting lives in one place and the caller
>   no longer has to switch on the bridge count (suggested by Mike
>   Pattrick).
> - As a result, single-bridge output now also includes the header
>   line ("bridge <name>" in text mode, {"bridge", "entries"} wrapper
>   in JSON mode).  Updated existing fdb/show tests to match.
> - Updated the unixctl usage hint to "bridge..." for consistency
>   with other variadic commands such as "ofctl/send".
> - Updated the ovs-vswitchd(8) man page to document multi-bridge
>   support and the output format.
> - Added tests for an empty-FDB JSON response and for the error
>   path when one of the requested bridges does not exist.
> - NEWS entry expanded to describe the format change.
> 
>  NEWS                       |   8 +-
>  ofproto/ofproto-dpif.c     |  46 +++++++----
>  tests/ofproto-dpif.at      | 160 ++++++++++++++++++++++++++-----------
>  tests/stp.at               |   3 +
>  vswitchd/ovs-vswitchd.8.in |   7 +-
>  5 files changed, 159 insertions(+), 65 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 1a3044cbfb2f..cc8ec48d37b3 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -3,7 +3,13 @@ Post-v3.7.0
>     - Userspace datapath:
>       * ARP/ND lookups for native tunnel are now rate limited. The holdout
>         timer can be configured with 'tnl/neigh/retrans_time'.
> -

We keep 2 empty lines between sections for major releases, i.e. this one
should state.

> +   - ovs-appctl:
> +     * "fdb/show" now accepts multiple bridge names in a single call,
> +       reducing the number of unixctl round-trips for clients that poll
> +       FDB entries across many bridges.  The output format now always
> +       prefixes a "bridge <name>" header line in text mode and wraps
> +       each bridge's entries in {"bridge", "entries"} in JSON mode,
> +       regardless of how many bridges were requested.

This is a little too verbose for a NEWS entry, IMO.  We could probably
just keep the first line.

>  
>  v3.7.0 - 16 Feb 2026
>  --------------------
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index a02afe8ef335..3333139e7c39 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -6178,6 +6178,7 @@ ofproto_unixctl_fdb_show_text(const struct ofproto_dpif 
> *ofproto,
>  {
>      const struct mac_entry *e;
>  
> +    ds_put_format(ds, "bridge %s\n", ofproto->up.name);
>      ds_put_cstr(ds, " port  VLAN  MAC                Age\n");

Since we're printing a table, I'd suggest the bridge name should
just become a column in it.  Printing multiple tables looks a
little awkward in the output.  We can have two variants of the
table, for multiple bridges with have the bridge column, for one
bridge we skip it.  Should not be too hard to implement.
Would probbaly need to split the header printing an the data
printing into separate functions and add an extra argument to
indicate if the bridge column is needed or not.  Though we should
only print the bridge name for the first row to avoid cluttering
the view, e.g.

 Bridge         Port  VLAN  MAC                Age
 -------------------------------------------------
 br-int            5    10  50:54:00:00:00:01    7
                   4    10  50:54:00:00:00:02    7
                   3    10  50:54:00:00:00:03    7
 -------------------------------------------------
 br-external       8    20  50:55:00:00:00:01    1
                   7    20  50:55:00:00:00:02    2
                   6    20  50:55:00:00:00:03    3

The bridge name should probably be 15 spaces wide to accommodate
the most common cases (IFNAMSIZ).

What do you think?

>      ovs_rwlock_rdlock(&ofproto->ml->rwlock);
>      LIST_FOR_EACH (e, lru_node, &ofproto->ml->lrus) {
> @@ -6200,9 +6201,10 @@ ofproto_unixctl_fdb_show_text(const struct 
> ofproto_dpif *ofproto,
>  
>  static void
>  ofproto_unixctl_fdb_show_json(const struct ofproto_dpif *ofproto,
> -                              struct json **fdb_entries)
> +                              struct json **bridge_entry)
>  {
>      struct json **json_entries = NULL;
> +    struct json *obj = json_object_create();
>      const struct mac_entry *entry;
>      size_t num_entries;
>      int i = 0;
> @@ -6238,29 +6240,45 @@ ofproto_unixctl_fdb_show_json(const struct 
> ofproto_dpif *ofproto,
>      }
>  done_unlock:
>      ovs_rwlock_unlock(&ofproto->ml->rwlock);
> -    *fdb_entries = json_array_create(json_entries, i);
> +    json_object_put_string(obj, "bridge", ofproto->up.name);
> +    json_object_put(obj, "entries", json_array_create(json_entries, i));
> +    *bridge_entry = obj;
>  }
>  
>  static void
> -ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> -                          const char *argv[] OVS_UNUSED, void *aux 
> OVS_UNUSED)
> +ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc,
> +                          const char *argv[], void *aux OVS_UNUSED)
>  {
> -    const struct ofproto_dpif *ofproto = 
> ofproto_dpif_lookup_by_name(argv[1]);
> -
> -    if (!ofproto) {
> -        unixctl_command_reply_error(conn, "no such bridge");
> -        return;
> +    for (int i = 1; i < argc; i++) {
> +        if (!ofproto_dpif_lookup_by_name(argv[i])) {
> +            unixctl_command_reply_error(conn, "no such bridge");
> +            return;
> +        }
>      }
>  
>      if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> -        struct json *fdb_entries;
> +        struct json **bridge_entries = xmalloc((argc - 1)
> +                                               * sizeof *bridge_entries);
> +
> +        for (int i = 1; i < argc; i++) {
> +            const struct ofproto_dpif *ofproto =
> +                ofproto_dpif_lookup_by_name(argv[i]);
> +
> +            ofproto_unixctl_fdb_show_json(ofproto, &bridge_entries[i - 1]);
> +        }
>  
> -        ofproto_unixctl_fdb_show_json(ofproto, &fdb_entries);
> -        unixctl_command_reply_json(conn, fdb_entries);
> +        unixctl_command_reply_json(
> +            conn, json_array_create(bridge_entries, argc - 1));
>      } else {
>          struct ds ds = DS_EMPTY_INITIALIZER;
>  
> -        ofproto_unixctl_fdb_show_text(ofproto, &ds);
> +        for (int i = 1; i < argc; i++) {
> +            const struct ofproto_dpif *ofproto =
> +                ofproto_dpif_lookup_by_name(argv[i]);
> +
> +            ofproto_unixctl_fdb_show_text(ofproto, &ds);
> +        }
> +
>          unixctl_command_reply(conn, ds_cstr(&ds));
>          ds_destroy(&ds);
>      }
> @@ -7131,7 +7149,7 @@ ofproto_unixctl_init(void)
>                               ofproto_unixctl_fdb_delete, NULL);
>      unixctl_command_register("fdb/flush", "[bridge]", 0, 1,
>                               ofproto_unixctl_fdb_flush, NULL);
> -    unixctl_command_register("fdb/show", "bridge", 1, 1,
> +    unixctl_command_register("fdb/show", "bridge...", 1, INT_MAX,
>                               ofproto_unixctl_fdb_show, NULL);
>      unixctl_command_register("fdb/stats-clear", "[bridge]", 0, 1,
>                               ofproto_unixctl_fdb_stats_clear, NULL);
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 39e43d376849..514bd2a2f249 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -7228,21 +7228,24 @@ 
> br_flow="arp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=ff:ff:ff:
>  # Test command: ofproto/trace odp_flow
>  AT_CHECK([ovs-appctl ofproto/trace "$odp_flow"], [0], [stdout])
>  # Check for no MAC learning entry
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>  ])
>  
>  # Test command: ofproto/trace br_name br_flow
>  AT_CHECK([ovs-appctl ofproto/trace br0 "$br_flow"], [0], [stdout])
>  # Check for no MAC learning entry
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>  ])
>  
>  # Test command: ofproto/trace odp_flow -generate
>  AT_CHECK([ovs-appctl ofproto/trace "$odp_flow" -generate], [0], [stdout])
>  # Check for the MAC learning entry
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>      3     0  50:54:00:00:00:05    ?
>  ])
> @@ -7252,7 +7255,8 @@ AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
>    "in_port(1),eth(src=50:54:00:00:00:06,dst=50:54:00:00:00:05)" \
>    -generate], [0], [stdout])
>  # Check for both MAC learning entries
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>      3     0  50:54:00:00:00:05    ?
>      1     0  50:54:00:00:00:06    ?
> @@ -7263,7 +7267,8 @@ AT_CHECK([ovs-appctl ofproto/trace br0 \
>    "in_port=2,dl_src=50:54:00:00:00:07,dl_dst=50:54:00:00:00:06" \
>    -generate], [0], [stdout])
>  # Check for both MAC learning entries.
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>      3     0  50:54:00:00:00:05    ?
>      1     0  50:54:00:00:00:06    ?
> @@ -7543,6 +7548,14 @@ add_of_ports br0 1 2 3
>  
>  
> arp='eth_type(0x0806),arp(sip=192.168.0.1,tip=192.168.0.2,op=1,sha=50:54:00:00:00:05,tha=00:00:00:00:00:00)'
>  
> +dnl Check json output for an empty FDB.
> +AT_CHECK([ovs-appctl --format json --pretty fdb/show br0], [0], [dnl
> +[[
> +  {
> +    "bridge": "br0",
> +    "entries": []}]]
> +])
> +
>  # Trace an ARP packet arriving on p3, to create a MAC learning entry.
>  OFPROTO_TRACE(
>    [ovs-dummy],
> @@ -7551,7 +7564,8 @@ OFPROTO_TRACE(
>    [1,2,100])
>  
>  # Check for the MAC learning entry.
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>      3     0  50:54:00:00:00:05    ?
>  ])
> @@ -7565,7 +7579,8 @@ OFPROTO_TRACE(
>    [3])
>  
>  # Check for both MAC learning entries.
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>      3     0  50:54:00:00:00:05    ?
>      1     0  50:54:00:00:00:06    ?
> @@ -7576,15 +7591,18 @@ AT_CHECK([ovs-appctl --format json --pretty fdb/show 
> br0 \
>            | sed 's/"age": [[0-9]]*/"age": ?/g'], [0], [dnl
>  [[
>    {
> -    "age": ?,
> -    "mac": "50:54:00:00:00:05",
> -    "port": 3,
> -    "vlan": 0},
> -  {
> -    "age": ?,
> -    "mac": "50:54:00:00:00:06",
> -    "port": 1,
> -    "vlan": 0}]]
> +    "bridge": "br0",
> +    "entries": [
> +      {
> +        "age": ?,
> +        "mac": "50:54:00:00:00:05",
> +        "port": 3,
> +        "vlan": 0},
> +      {
> +        "age": ?,
> +        "mac": "50:54:00:00:00:06",
> +        "port": 1,
> +        "vlan": 0}]}]]
>  ])
>  
>  # Trace a packet arrival that updates the first learned MAC entry.
> @@ -7595,7 +7613,8 @@ OFPROTO_TRACE(
>    [1,3,100])
>  
>  # Check that the MAC learning entry was updated.
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>      1     0  50:54:00:00:00:06    ?
>      2     0  50:54:00:00:00:05    ?
> @@ -7621,7 +7640,8 @@ OFPROTO_TRACE(
>    [4,101])
>  
>  # Check that the MAC learning entries were added.
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br1 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br1 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br1
>   port  VLAN  MAC                Age
>      4     0  50:54:00:00:00:06    ?
>      5     0  50:54:00:00:00:07    ?
> @@ -7630,15 +7650,56 @@ AT_CHECK_UNQUOTED([ovs-appctl fdb/show br1 | sed 
> 's/[[0-9]]\{1,\}$/?/'], [0], [d
>  # Delete port p1 and see that its MAC learning entry disappeared, and
>  # that the MAC learning entry for the same MAC was also deleted from br1.
>  AT_CHECK([ovs-vsctl del-port p1])
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br0
>   port  VLAN  MAC                Age
>      2     0  50:54:00:00:00:05    ?
>  ])
> -AT_CHECK_UNQUOTED([ovs-appctl fdb/show br1 | sed 's/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br1 | sed '/^ /s/[[0-9]]\{1,\}$/?/'], 
> [0], [dnl
> +bridge br1
> + port  VLAN  MAC                Age
> +    5     0  50:54:00:00:00:07    ?
> +])
> +
> +# Test fdb/show with multiple bridges.
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 br1 | sed '/^ 
> /s/[[0-9]]\{1,\}$/?/'], [0], [dnl
> +bridge br0
> + port  VLAN  MAC                Age
> +    2     0  50:54:00:00:00:05    ?
> +bridge br1
>   port  VLAN  MAC                Age
>      5     0  50:54:00:00:00:07    ?
>  ])
>  
> +dnl Check json output with multiple bridges.
> +AT_CHECK([ovs-appctl --format json --pretty fdb/show br0 br1 \
> +          | sed 's/"age": [[0-9]]*/"age": ?/g'], [0], [dnl
> +[[
> +  {
> +    "bridge": "br0",
> +    "entries": [
> +      {
> +        "age": ?,
> +        "mac": "50:54:00:00:00:05",
> +        "port": 2,
> +        "vlan": 0}]},
> +  {
> +    "bridge": "br1",
> +    "entries": [
> +      {
> +        "age": ?,
> +        "mac": "50:54:00:00:00:07",
> +        "port": 5,
> +        "vlan": 0}]}]]
> +])

This doesn't look like a normal JSON.  We should try to create a more
structured object.  The words "bridge" and "entries" are not really
useful here.  The bridge name should just be a key and the entries array
be a value, i.e.:

  {
    "br0": [
      {
        "age": ?,
        "mac": "50:54:00:00:00:05",
        "port": 2,
        "vlan": 0
      }
    ],
    "br1": [
      {
        "age": ?,
        "mac": "50:54:00:00:00:07",
        "port": 5,
        "vlan": 0
      }
    ]
  }

There is no single unique key for the entries themselves, so those
would need to stay an array or objects.  But the bridge name is unique,
so it can be a key.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to