On 17/02/2025 10:47, Eelco Chaudron wrote:
> 
> 
> On 12 Feb 2025, at 13:50, Roi Dayan wrote:
> 
>> From: Dima Chumak <[email protected]>
>>
>> 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 <[email protected]>
>> Acked-by: Roi Dayan <[email protected]>
> 
> I have not reviewed it yet, but it’s failing the linux clang 
> address,undefined test. Can you take a look and fix this in a v3?
> 
> Thanks,
> 
> Eelco

According to the log in github it failed in a single test

https://github.com/ovsrobot/ovs/actions/runs/13285912928/job/37100202302

1279: ofproto-dpif - MAC learning                     FAILED 
(ofproto-dpif.at:7100)

but when I run locally its passing.

1279: ofproto-dpif - MAC learning                     ok

It's exactly the next case after the json output check I added and it should
be the same output as before. I'm not sure what is different.

> 
>> ---
>>
>> Notes:
>>     v2
>>     - Fix sparse check error.
>>     - Add case checking fdb/show json output.
>>
>>  ofproto/ofproto-dpif.c | 86 ++++++++++++++++++++++++++++++++++--------
>>  tests/ofproto-dpif.at  | 10 +++++
>>  2 files changed, 80 insertions(+), 16 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index bf43d5d4bc59..b66cf8fe69f8 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);
>> +        long long port;
>> +
>> +        port = (OVS_FORCE long long) 
>> 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
>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>> index fa5f148b4c28..8a975e6ffd9c 100644
>> --- a/tests/ofproto-dpif.at
>> +++ b/tests/ofproto-dpif.at
>> @@ -7085,6 +7085,16 @@ AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 
>> 's/[[0-9]]\{1,\}$/?/'], [0], [d
>>      3     0  50:54:00:00:00:05    ?
>>  ])
>>
>> +dnl Check json output.
>> +AT_CHECK([ovs-appctl --format json --pretty fdb/show br0], [0], [dnl
>> +[[
>> +  {
>> +    "age": 0,
>> +    "mac": "50:54:00:00:00:05",
>> +    "port": 3,
>> +    "vlan": 0}]]
>> +])
>> +
>>  # Trace a packet arrival destined for the learned MAC.
>>  # (This will also learn a MAC.)
>>  OFPROTO_TRACE(
>> -- 
>> 2.21.0
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to