On 2 Apr 2026, at 16:49, Ilya Maximets wrote:
> On 4/2/26 3:43 PM, Eelco Chaudron wrote:
>>
>>
>> On 2 Apr 2026, at 14:28, Ilya Maximets wrote:
>>
>>> On 4/2/26 11:19 AM, Eelco Chaudron via dev wrote:
>>>>
>>>>
>>>> On 13 Mar 2026, at 21:50, Timothy Redaelli via dev wrote:
>>>>
>>>>> When --format json is passed to ovs-appctl, fdb/stats-show returns a
>>>>> flat JSON object with the bridge name and all counters from the MAC
>>>>> learning table rather than the existing text representation.
>>>>>
>>>>> Example output:
>>>>> {"bridge":"br0","current_entries":17,"max_entries":8192,
>>>>> "static_entries":17,"total_expired":0,"total_evicted":0,
>>>>> "total_learned":17,"total_moved":0}
>>>
>>> General note for all patches in the set:
>>>
>>> 1. Don't use underscores in JSON output. Always use dashes.
>>>
>>> 2. Not applicable to this patch, but in other patches, use full words
>>> whenever possible, i.e. s/pkts/packets/, except for some things
>>> like 'ms' for milliseconds, which are acceptable short names.
>>> There is no point in seving a little bit of space in JSON.
>>>
>>> 3. All of the patches need a NEW entry.
>
> *NEWS
>
>>>
>>>>>
>>>>> Reported-at: https://issues.redhat.com/browse/FDP-2444
>>>>> Signed-off-by: Timothy Redaelli <[email protected]>
>>>>
>>>> Hi Timothy,
>>>>
>>>> Thanks for the series. I have a few comments on this patch below.
>>>>
>>>> Cheers,
>>>>
>>>>
>>>> Eelco
>>>>
>>>>> ---
>
> <snip>
>
>>>>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>>>>> index 39e43d376..9dfdfe29b 100644
>>>>> --- a/tests/ofproto-dpif.at
>>>>> +++ b/tests/ofproto-dpif.at
>>>>> @@ -8248,6 +8248,15 @@ 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 | dnl
>>>>> + grep '"bridge"\|entries'], [0], [dnl
>>>>
>>>> I would not re-format the output here, just use the raw output of the
>>>> --pretty option.
>>>>
>>>> AT_CHECK([ovs-appctl --format json --pretty fdb/stats-show br0], [0], [dnl
>>>> {
>>>> "br0": {
>>>> "current_entries": 17,
>>>> "max_entries": 8192,
>>>> "static_entries": 17,
>>>> "total_evicted": 0,
>>>> "total_expired": 0,
>>>> "total_learned": 18,
>>>> "total_moved": 0}}
>>>> ])
>>>>
>>>> Looking at the naming, I think static_entries should be renamed to
>>>> current_static_entries to make
>>>> its meaning clearer. Thoughts?
>>>
>>> Not sure about that particular point, but I'd suggest we group the stats
>>> here into two separate blocks:
>>>
>>> "br0": {
>>> "entries": {
>>> "current": 17,
>>> "max": 8192,
>>> "static": 17
>>> },
>>> "events": {
>>> "evicted": 0,
>>> "expired": 0,
>>> "learned": 18,
>>> "moved": 0
>>> }
>>> }
>>>
>>> WDYT?
>>
>> Grouping makes sense, than current might not be needed, maybe max should be
>> maximum?
>
> max --> maximum makes sense. Why the current not needed?
I meant to say; current_static -> static.
>>
>>>>
>>>>
>>>> The unfiltered output and split of the show functions should be true for
>>>> all other patches also.
>>>>
>>>>> + "bridge": "br0",
>>>>> + "current_entries": 17,
>>>>> + "max_entries": 8192,
>>>>> + "static_entries": 17,
>>>>> +])
>>>>> +
>>>>> OVS_VSWITCHD_STOP
>>>>> AT_CLEANUP
>>>>>
>>>>> --
>>>>> 2.53.0
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev