On 1/8/26 10:25 AM, Frode Nordahl wrote:
> On 1/7/26 22:30, Ilya Maximets wrote:
>> Hi, Frode. Thanks for the update!
>>
>> See some comments below.
>
> Hello, Ilya. Happy New Year, and thank you for taking the time to review.
Happy New Year indeed!
>
>>
>> On 12/19/25 10:42 AM, Frode Nordahl wrote:
>>> The tc command from iproute2 changed its rounding behavior in commit
>>> d947f365602b ("tc: Fix rounding in tc_calc_xmittime and tc_calc_xmitsize.").
>>> This caused the ovn-egress-qos tests to fail because they were matching
>>
>> Maybe mention relevant OVS tests here?
>
> Ack, that would be "QoS - basic configuration" and "QoS - 64bit" in the
> system-traffic suite AFAICT.
>
>>> exact burst and cburst values in tc output.
>>>
>>> The rounding fix means that burst and cburst values may differ slightly
>>> from previous versions. For example, values that were previously 750000
>>> might now be 749999 or similar variations.
>>>
>>> To maintain compatibility with both old and new versions of tc, the test
>>> assertions now use pattern matching with dots that:
>>> - Matches the most significant digit of the value
>>> - Uses dots to match any character for remaining digits
>>> - Maintains the correct total number of digits
>>> - Preserves the unit suffix (e.g., 'b' for bytes)
>>>
>>> For example, '375000b' now matches '3.....b' which accepts any 6-digit
>>> value starting with 3, allowing for rounding differences while still
>>> validating the general magnitude is correct.
>>>
>>> This follows the same approach as the related OVN patch:
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2025-December/428593.html
>>>
>>> Reported-at: https://launchpad.net/bugs/2129005
>>> Assisted-by: claude-sonnet-4.5, GitHub Copilot CLI
>>> Signed-off-by: Frode Nordahl <[email protected]>
>>> ---
>>> v2:
>>> - Replace regex with dot (``.``) in line with the solution accepted
>>> for the related OVN patch.
>>> - Update Assisted-by tag to be in lnie with the newly documented format.
>>> v1: https://mail.openvswitch.org/pipermail/ovs-dev/2025-December/428533.html
>>>
>>> tests/system-traffic.at | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>>> index 58a46af0a..fa747d42e 100644
>>> --- a/tests/system-traffic.at
>>> +++ b/tests/system-traffic.at
>>> @@ -3141,8 +3141,8 @@ OVS_WAIT_UNTIL([tc qdisc show dev ovs-tap0 | grep -q
>>> htb])
>>> OVS_WAIT_UNTIL([tc qdisc show dev ovs-tap1 | grep -q htb])
>>>
>>> dnl Check the configuration.
>>> -m4_define([HTB_CONF0], [rate 2Mbit ceil 3Mbit burst 375000b cburst
>>> 375000b])
>>> -m4_define([HTB_CONF1], [rate 4Mbit ceil 5Gbit burst 500000b cburst
>>> 500000b])
>>> +m4_define([HTB_CONF0], [rate 2Mbit ceil 3Mbit burst 3.....b cburst
>>> 3.....b])
>>> +m4_define([HTB_CONF1], [rate 4Mbit ceil 5Gbit burst 5.....b cburst
>>> 5.....b])
>>> AT_CHECK([tc class show dev ovs-tap0 | grep -q 'class htb .* HTB_CONF0'])
>>> AT_CHECK([tc class show dev ovs-tap0 | grep -q 'class htb .* HTB_CONF1'])
>>> AT_CHECK([tc class show dev ovs-tap1 | grep -q 'class htb .* HTB_CONF0'])
>>> @@ -3172,7 +3172,7 @@ AT_CHECK([ovs-vsctl set port ovs-p0 qos=@qos -- set
>>> port ovs-p1 qos=@qos dnl
>>> OVS_WAIT_UNTIL([tc qdisc show dev ovs-p0 | grep -q htb])
>>> OVS_WAIT_UNTIL([tc qdisc show dev ovs-p1 | grep -q htb])
>>>
>>> -m4_define([HTB_CONF], [rate 40Gbit ceil 50Gbit burst 620000b cburst
>>> 618750b])
>>> +m4_define([HTB_CONF], [rate 40Gbit ceil 50Gbit burst 6.....b cburst
>>> 6.....b])
>>> AT_CHECK([tc class show dev ovs-p0 | grep -q 'class htb .* HTB_CONF'])
>>> AT_CHECK([tc class show dev ovs-p1 | grep -q 'class htb .* HTB_CONF'])
>>>
>>
>> There is a 64-bit QoS test as well that is matching on burst values,
>> do we need to fix it as well? It uses a bit different method of checking,
>> so it may need a slightly different solution.
>
> The last hunk in the above patch is from the "QoS - 64bit" test, is
> there another one that I don't see?
Sorry, I meant 'Ingress Policing - 64-bit' test, which has the following check:
AT_CHECK([tc -o -s -d filter show dev ovs-p0 ingress |
sed -n 's/.*\(rate [[0-9]]*[[a-zA-Z]]* burst [[0-9]]*[[a-zA-Z]]*\).*/\1/; T;
p; q'],
[0],[dnl
rate 50Gbit burst 74500000b
])
Or is the rounding change only relevant for the 'class show' and not the
'filter show' ?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
