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?
>
>>>
>>>
>>> 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
