On 1/8/26 10:59, Ilya Maximets wrote:
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:
Ah, thanks for the clarification.
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' ?
The ingress policing code in tc does appear to use the same tc_calc_*
functions under the hood, so these tests would likely be subject to the
same issue.
I did not see them because they do not currently fail, but I guess it
would be pertinent to give them the same treatment just in case.
--
Frode Nordahl
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
