diff mbox series

[ovs-dev] ofproto: Add JSON output for 'fdb/show' command.

Message ID 20250212084751.307720-1-roid@nvidia.com
State Superseded
Headers show
Series [ovs-dev] ofproto: Add JSON output for 'fdb/show' command. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/cirrus-robot success cirrus build: passed
ovsrobot/github-robot-_Build_and_Test fail github build: failed

Commit Message

Roi Dayan Feb. 12, 2025, 8:47 a.m. UTC
From: Dima Chumak <dchumak@nvidia.com>

The 'fdb/show' command now supports machine-readable JSON output in
addition to the plain-text output for humans. An example would be:

  ovs-appctl --format=json --pretty fdb/show br-phy
  [
    {
      "age": "static",
      "mac": "e4:8c:07:08:00:02",
      "port": 2,
      "vlan": 0
    },
    {
      "age": 14,
      "mac": "e4:8c:07:08:00:03",
      "port": 3,
      "vlan": 0
    }
  ]

Signed-off-by: Dima Chumak <dchumak@nvidia.com>
Acked-by: Roi Dayan <roid@nvidia.com>
---
 ofproto/ofproto-dpif.c | 86 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 70 insertions(+), 16 deletions(-)

Comments

Eelco Chaudron Feb. 12, 2025, 10:43 a.m. UTC | #1
On 12 Feb 2025, at 9:47, Roi Dayan via dev wrote:

> From: Dima Chumak <dchumak@nvidia.com>
>
> The 'fdb/show' command now supports machine-readable JSON output in
> addition to the plain-text output for humans. An example would be:
>
>   ovs-appctl --format=json --pretty fdb/show br-phy
>   [
>     {
>       "age": "static",
>       "mac": "e4:8c:07:08:00:02",
>       "port": 2,
>       "vlan": 0
>     },
>     {
>       "age": 14,
>       "mac": "e4:8c:07:08:00:03",
>       "port": 3,
>       "vlan": 0
>     }
>   ]
>
> Signed-off-by: Dima Chumak <dchumak@nvidia.com>
> Acked-by: Roi Dayan <roid@nvidia.com>

Hi Roi/Dima,

Thanks for the patch; this was something to do on my list. Please take a look at the robot response, as the patch is failing to build.

In addition, if you sent out a v2, can you include a test case for this?

Cheers,

Eelco

> ---
>  ofproto/ofproto-dpif.c | 86 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 70 insertions(+), 16 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index bf43d5d4bc59..b6228f91765a 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -6151,20 +6151,12 @@ ofbundle_get_a_port(const struct ofbundle *bundle)
>  }
>
>  static void
> -ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> -                         const char *argv[], void *aux OVS_UNUSED)
> +ofproto_unixctl_fdb_show_text(const struct ofproto_dpif *ofproto,
> +                              struct ds *ds)
>  {
> -    struct ds ds = DS_EMPTY_INITIALIZER;
> -    const struct ofproto_dpif *ofproto;
>      const struct mac_entry *e;
>
> -    ofproto = ofproto_dpif_lookup_by_name(argv[1]);
> -    if (!ofproto) {
> -        unixctl_command_reply_error(conn, "no such bridge");
> -        return;
> -    }
> -
> -    ds_put_cstr(&ds, " port  VLAN  MAC                Age\n");
> +    ds_put_cstr(ds, " port  VLAN  MAC                Age\n");
>      ovs_rwlock_rdlock(&ofproto->ml->rwlock);
>      LIST_FOR_EACH (e, lru_node, &ofproto->ml->lrus) {
>          struct ofbundle *bundle = mac_entry_get_port(ofproto->ml, e);
> @@ -6173,17 +6165,79 @@ ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
>
>          ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
>                  NULL, name, sizeof name);
> -        ds_put_format(&ds, "%5s  %4d  "ETH_ADDR_FMT"  ",
> +        ds_put_format(ds, "%5s  %4d  "ETH_ADDR_FMT"  ",
>                  name, e->vlan, ETH_ADDR_ARGS(e->mac));
>          if (MAC_ENTRY_AGE_STATIC_ENTRY == age) {
> -            ds_put_format(&ds, "static\n");
> +            ds_put_format(ds, "static\n");
>          } else {
> -            ds_put_format(&ds, "%3d\n", age);
> +            ds_put_format(ds, "%3d\n", age);
>          }
>      }
>      ovs_rwlock_unlock(&ofproto->ml->rwlock);
> -    unixctl_command_reply(conn, ds_cstr(&ds));
> -    ds_destroy(&ds);
> +}
> +
> +static void
> +ofproto_unixctl_fdb_show_json(const struct ofproto_dpif *ofproto,
> +                              struct json **fdb_entries)
> +{
> +    size_t num_entries = hmap_count(&ofproto->ml->table);
> +    struct json **json_entries = NULL;
> +    const struct mac_entry *entry;
> +    int i = 0;
> +
> +    if (!num_entries) {
> +        goto done;
> +    }
> +
> +    json_entries = xmalloc(num_entries * sizeof *json_entries);
> +    ovs_rwlock_rdlock(&ofproto->ml->rwlock);
> +    LIST_FOR_EACH (entry, lru_node, &ofproto->ml->lrus) {
> +        struct ofbundle *bundle = mac_entry_get_port(ofproto->ml, entry);
> +        struct json *json_entry = json_object_create();
> +        int age = mac_entry_age(ofproto->ml, entry);
> +        ofp_port_t port;
> +
> +        port = ofbundle_get_a_port(bundle)->up.ofp_port;
> +        json_object_put(json_entry, "port", json_integer_create(port));
> +        json_object_put(json_entry, "vlan", json_integer_create(entry->vlan));
> +        json_object_put_format(json_entry, "mac", ETH_ADDR_FMT,
> +                               ETH_ADDR_ARGS(entry->mac));
> +        if (MAC_ENTRY_AGE_STATIC_ENTRY == age) {
> +            json_object_put_string(json_entry, "age", "static");
> +        } else {
> +            json_object_put(json_entry, "age", json_integer_create(age));
> +        }
> +        json_entries[i++] = json_entry;
> +    }
> +    ovs_rwlock_unlock(&ofproto->ml->rwlock);
> +done:
> +    *fdb_entries = json_array_create(json_entries, num_entries);
> +}
> +
> +static void
> +ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> +                          const char *argv[] OVS_UNUSED, 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;
> +    }
> +
> +    if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> +        struct json *fdb_entries;
> +
> +        ofproto_unixctl_fdb_show_json(ofproto, &fdb_entries);
> +        unixctl_command_reply_json(conn, fdb_entries);
> +        json_destroy(fdb_entries);
> +    } else {
> +        struct ds ds = DS_EMPTY_INITIALIZER;
> +
> +        ofproto_unixctl_fdb_show_text(ofproto, &ds);
> +        unixctl_command_reply(conn, ds_cstr(&ds));
> +        ds_destroy(&ds);
> +    }
>  }
>
>  static void
> -- 
> 2.21.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Roi Dayan Feb. 12, 2025, 11:04 a.m. UTC | #2
On 12/02/2025 12:43, Eelco Chaudron wrote:
> 
> 
> On 12 Feb 2025, at 9:47, Roi Dayan via dev wrote:
> 
>> From: Dima Chumak <dchumak@nvidia.com>
>>
>> The 'fdb/show' command now supports machine-readable JSON output in
>> addition to the plain-text output for humans. An example would be:
>>
>>   ovs-appctl --format=json --pretty fdb/show br-phy
>>   [
>>     {
>>       "age": "static",
>>       "mac": "e4:8c:07:08:00:02",
>>       "port": 2,
>>       "vlan": 0
>>     },
>>     {
>>       "age": 14,
>>       "mac": "e4:8c:07:08:00:03",
>>       "port": 3,
>>       "vlan": 0
>>     }
>>   ]
>>
>> Signed-off-by: Dima Chumak <dchumak@nvidia.com>
>> Acked-by: Roi Dayan <roid@nvidia.com>
> 
> Hi Roi/Dima,
> 
> Thanks for the patch; this was something to do on my list. Please take a look at the robot response, as the patch is failing to build.
> 
> In addition, if you sent out a v2, can you include a test case for this?
> 
> Cheers,
> 
> Eelco

Strange I did build and ran the dump again just before sending. I'll check it out.
I'll check about adding test case for json output.
thanks


> 
>> ---
>>  ofproto/ofproto-dpif.c | 86 ++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 70 insertions(+), 16 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index bf43d5d4bc59..b6228f91765a 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -6151,20 +6151,12 @@ ofbundle_get_a_port(const struct ofbundle *bundle)
>>  }
>>
>>  static void
>> -ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
>> -                         const char *argv[], void *aux OVS_UNUSED)
>> +ofproto_unixctl_fdb_show_text(const struct ofproto_dpif *ofproto,
>> +                              struct ds *ds)
>>  {
>> -    struct ds ds = DS_EMPTY_INITIALIZER;
>> -    const struct ofproto_dpif *ofproto;
>>      const struct mac_entry *e;
>>
>> -    ofproto = ofproto_dpif_lookup_by_name(argv[1]);
>> -    if (!ofproto) {
>> -        unixctl_command_reply_error(conn, "no such bridge");
>> -        return;
>> -    }
>> -
>> -    ds_put_cstr(&ds, " port  VLAN  MAC                Age\n");
>> +    ds_put_cstr(ds, " port  VLAN  MAC                Age\n");
>>      ovs_rwlock_rdlock(&ofproto->ml->rwlock);
>>      LIST_FOR_EACH (e, lru_node, &ofproto->ml->lrus) {
>>          struct ofbundle *bundle = mac_entry_get_port(ofproto->ml, e);
>> @@ -6173,17 +6165,79 @@ ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
>>
>>          ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
>>                  NULL, name, sizeof name);
>> -        ds_put_format(&ds, "%5s  %4d  "ETH_ADDR_FMT"  ",
>> +        ds_put_format(ds, "%5s  %4d  "ETH_ADDR_FMT"  ",
>>                  name, e->vlan, ETH_ADDR_ARGS(e->mac));
>>          if (MAC_ENTRY_AGE_STATIC_ENTRY == age) {
>> -            ds_put_format(&ds, "static\n");
>> +            ds_put_format(ds, "static\n");
>>          } else {
>> -            ds_put_format(&ds, "%3d\n", age);
>> +            ds_put_format(ds, "%3d\n", age);
>>          }
>>      }
>>      ovs_rwlock_unlock(&ofproto->ml->rwlock);
>> -    unixctl_command_reply(conn, ds_cstr(&ds));
>> -    ds_destroy(&ds);
>> +}
>> +
>> +static void
>> +ofproto_unixctl_fdb_show_json(const struct ofproto_dpif *ofproto,
>> +                              struct json **fdb_entries)
>> +{
>> +    size_t num_entries = hmap_count(&ofproto->ml->table);
>> +    struct json **json_entries = NULL;
>> +    const struct mac_entry *entry;
>> +    int i = 0;
>> +
>> +    if (!num_entries) {
>> +        goto done;
>> +    }
>> +
>> +    json_entries = xmalloc(num_entries * sizeof *json_entries);
>> +    ovs_rwlock_rdlock(&ofproto->ml->rwlock);
>> +    LIST_FOR_EACH (entry, lru_node, &ofproto->ml->lrus) {
>> +        struct ofbundle *bundle = mac_entry_get_port(ofproto->ml, entry);
>> +        struct json *json_entry = json_object_create();
>> +        int age = mac_entry_age(ofproto->ml, entry);
>> +        ofp_port_t port;
>> +
>> +        port = ofbundle_get_a_port(bundle)->up.ofp_port;
>> +        json_object_put(json_entry, "port", json_integer_create(port));
>> +        json_object_put(json_entry, "vlan", json_integer_create(entry->vlan));
>> +        json_object_put_format(json_entry, "mac", ETH_ADDR_FMT,
>> +                               ETH_ADDR_ARGS(entry->mac));
>> +        if (MAC_ENTRY_AGE_STATIC_ENTRY == age) {
>> +            json_object_put_string(json_entry, "age", "static");
>> +        } else {
>> +            json_object_put(json_entry, "age", json_integer_create(age));
>> +        }
>> +        json_entries[i++] = json_entry;
>> +    }
>> +    ovs_rwlock_unlock(&ofproto->ml->rwlock);
>> +done:
>> +    *fdb_entries = json_array_create(json_entries, num_entries);
>> +}
>> +
>> +static void
>> +ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
>> +                          const char *argv[] OVS_UNUSED, 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;
>> +    }
>> +
>> +    if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
>> +        struct json *fdb_entries;
>> +
>> +        ofproto_unixctl_fdb_show_json(ofproto, &fdb_entries);
>> +        unixctl_command_reply_json(conn, fdb_entries);
>> +        json_destroy(fdb_entries);
>> +    } else {
>> +        struct ds ds = DS_EMPTY_INITIALIZER;
>> +
>> +        ofproto_unixctl_fdb_show_text(ofproto, &ds);
>> +        unixctl_command_reply(conn, ds_cstr(&ds));
>> +        ds_destroy(&ds);
>> +    }
>>  }
>>
>>  static void
>> -- 
>> 2.21.0
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index bf43d5d4bc59..b6228f91765a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6151,20 +6151,12 @@  ofbundle_get_a_port(const struct ofbundle *bundle)
 }
 
 static void
-ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
-                         const char *argv[], void *aux OVS_UNUSED)
+ofproto_unixctl_fdb_show_text(const struct ofproto_dpif *ofproto,
+                              struct ds *ds)
 {
-    struct ds ds = DS_EMPTY_INITIALIZER;
-    const struct ofproto_dpif *ofproto;
     const struct mac_entry *e;
 
-    ofproto = ofproto_dpif_lookup_by_name(argv[1]);
-    if (!ofproto) {
-        unixctl_command_reply_error(conn, "no such bridge");
-        return;
-    }
-
-    ds_put_cstr(&ds, " port  VLAN  MAC                Age\n");
+    ds_put_cstr(ds, " port  VLAN  MAC                Age\n");
     ovs_rwlock_rdlock(&ofproto->ml->rwlock);
     LIST_FOR_EACH (e, lru_node, &ofproto->ml->lrus) {
         struct ofbundle *bundle = mac_entry_get_port(ofproto->ml, e);
@@ -6173,17 +6165,79 @@  ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
 
         ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
                 NULL, name, sizeof name);
-        ds_put_format(&ds, "%5s  %4d  "ETH_ADDR_FMT"  ",
+        ds_put_format(ds, "%5s  %4d  "ETH_ADDR_FMT"  ",
                 name, e->vlan, ETH_ADDR_ARGS(e->mac));
         if (MAC_ENTRY_AGE_STATIC_ENTRY == age) {
-            ds_put_format(&ds, "static\n");
+            ds_put_format(ds, "static\n");
         } else {
-            ds_put_format(&ds, "%3d\n", age);
+            ds_put_format(ds, "%3d\n", age);
         }
     }
     ovs_rwlock_unlock(&ofproto->ml->rwlock);
-    unixctl_command_reply(conn, ds_cstr(&ds));
-    ds_destroy(&ds);
+}
+
+static void
+ofproto_unixctl_fdb_show_json(const struct ofproto_dpif *ofproto,
+                              struct json **fdb_entries)
+{
+    size_t num_entries = hmap_count(&ofproto->ml->table);
+    struct json **json_entries = NULL;
+    const struct mac_entry *entry;
+    int i = 0;
+
+    if (!num_entries) {
+        goto done;
+    }
+
+    json_entries = xmalloc(num_entries * sizeof *json_entries);
+    ovs_rwlock_rdlock(&ofproto->ml->rwlock);
+    LIST_FOR_EACH (entry, lru_node, &ofproto->ml->lrus) {
+        struct ofbundle *bundle = mac_entry_get_port(ofproto->ml, entry);
+        struct json *json_entry = json_object_create();
+        int age = mac_entry_age(ofproto->ml, entry);
+        ofp_port_t port;
+
+        port = ofbundle_get_a_port(bundle)->up.ofp_port;
+        json_object_put(json_entry, "port", json_integer_create(port));
+        json_object_put(json_entry, "vlan", json_integer_create(entry->vlan));
+        json_object_put_format(json_entry, "mac", ETH_ADDR_FMT,
+                               ETH_ADDR_ARGS(entry->mac));
+        if (MAC_ENTRY_AGE_STATIC_ENTRY == age) {
+            json_object_put_string(json_entry, "age", "static");
+        } else {
+            json_object_put(json_entry, "age", json_integer_create(age));
+        }
+        json_entries[i++] = json_entry;
+    }
+    ovs_rwlock_unlock(&ofproto->ml->rwlock);
+done:
+    *fdb_entries = json_array_create(json_entries, num_entries);
+}
+
+static void
+ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                          const char *argv[] OVS_UNUSED, 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;
+    }
+
+    if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
+        struct json *fdb_entries;
+
+        ofproto_unixctl_fdb_show_json(ofproto, &fdb_entries);
+        unixctl_command_reply_json(conn, fdb_entries);
+        json_destroy(fdb_entries);
+    } else {
+        struct ds ds = DS_EMPTY_INITIALIZER;
+
+        ofproto_unixctl_fdb_show_text(ofproto, &ds);
+        unixctl_command_reply(conn, ds_cstr(&ds));
+        ds_destroy(&ds);
+    }
 }
 
 static void