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

Reply via email to