On 12/14/25 12:32 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
> 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 that:
> - Matches the most significant digit(s) of the value
> - Ensures all characters are numeric
> - Maintains the correct total number of digits
> - Preserves the unit suffix (e.g., 'b' for bytes)
>
> For example, '375000b' now matches '3[0-9][0-9][0-9][0-9][0-9]b' which
> accepts any 6-digit value starting with 3, allowing for rounding
> differences while still validating the general magnitude is correct.
Hi Frode,
Thanks for v2!
>
> Reported-at: https://launchpad.net/bugs/2129005
> Signed-off-by: GitHub Copilot <[email protected]>
Actually, according to our guidelines [0] this should probably be
Assisted-by: ... . I don't think it's correct to include a sign off
from an AI assistant. That's because the sign off should certify
that the author of the specific code agrees with the DCO. I'm not
sure an AI assistant can reliably sign off.
So maybe something like you did in the OVS counterpart patch [1]?
Assisted-by: GitHub Copilot <[email protected]>
[0]
https://github.com/ovn-org/ovn/blob/5220faeb7bf7aa0c9ee4cb9ad91a7010b12284b2/Documentation/internals/contributing/submitting-patches.rst#ai-assisted-contributions
[1]
https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
> Signed-off-by: Frode Nordahl <[email protected]>
> ---
> v2:
> - use regular expression to match on digits instead of any character.
> - replace all exact matches, not just the ones currently failing.
> - use github copilot cli to make the changes.
> v1: https://mail.openvswitch.org/pipermail/ovs-dev/2025-October/427356.html
>
> tests/system-ovn.at | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> github copilot cli prompt BEGIN:
> Context
> =======
>
> The current working directory contains a bootstrapped and built copy of the
> Open Virtual Network (OVN) project, which is also publicly available from
> https://github.com/ovn-org/ovn.git.
>
> There is a test that is currently failing due to a change in behavior of a
> system tool named ``tc``.
>
> The ``tc`` command is part of the iproute2 project
> which is publicly available from https://github.com/iproute2/iproute2.git.
> For further information and reference on the change of behavior, you can look
> at the following commit:
> d947f365602b ("tc: Fix rounding in tc_calc_xmittime and tc_calc_xmitsize.")
>
> The tests assertions for the failing tests can be found in the file named
> tests/system-ovn.at. The reason for the failure is likely that the test
> assertions match on exact values labeled ``burst`` and ``cburst`` in the
> output from the ``tc`` command.
>
> Note that the tests are expected to pass on systems with a version of ``tc``
> from both before and after the change of behavior, so the assertions must
> be written in such a way so that they are compatible with both.
>
> The copy of OVN in the current working directory can be built using the
> ``make``
> command.
>
> The command to run the test:
>
> sudo make check-kernel TESTSUITEFLAGS="-j1 -k ovn-egress-qos
>
> Assignment
> ==========
>
> 1. Locate tests with the ``ovn-egress-qos`` keyword.
> 2. Change the ``grep`` command used to match on output from the ``tc`` command
> containing ``burst`` or ``cburst``, so that it instead of exact match uses
> a
> partial match that includes:
> - most significant part of number.
> - characters must be numbers.
> - the number of characters must match.
> - the last alphabetical character(s), denoting the unit, must match.
> 3. Do one change per iteration and ensure tests pass for each iteration.
> 4. Commit your proposal to the repository and write a commit message that
> explains why the change is necessary, with reference to the change of
> behavior in ``tc``.
> END
> github copilot cli transcript:
> https://gist.github.com/fnordahl/ec57e9d6444e39ece45f619740724261
>
This was an interesting read. :)
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 303b10894..46bf1bc61 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -6718,11 +6718,11 @@ AT_CHECK([ovn-nbctl set Logical_Switch_Port ext
> options:qos_burst=6000000])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* rate 200Kbit ceil 300Kbit burst
> 375000b cburst 375000b'])
> + grep -q 'class htb .* rate 200Kbit ceil 300Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
I know Ales suggested matching on numbers here:
https://mail.openvswitch.org/pipermail/ovs-dev/2025-October/427357.html
For fun I asked another code assistant (gemini-cli with
gemini-3-pro-preview) to review your v2 and take into account
the suggestion Ales had, of explicitly matching on "[0-9]{4}" with
an extended regex. It concluded it's not nice :)
"
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* rate 200Kbit ceil 300Kbit burst
375000b cburst 375000b'])
> + grep -q 'class htb .* rate 200Kbit ceil 300Kbit burst
3[0-9][0-9][0-9][0-9][0-9]b cburst 3[0-9][0-9][0-9][0-9][0-9]b'])
This regex is definitely safe and portable.
I know there was a suggestion to use quantifiers (like `[0-9]{4}`
or `[0-9]{5}`). However, `grep` in `tests/system-ovn.at` is currently
used without `-E` (Extended Regex), so to use curly braces we'd either
need to switch to `grep -E` or use the escaped BRE syntax `[0-9]\\{5\\}`.
Given that this file avoids `grep -E` so far, keeping this verbose
version seems consistent and fine to me.
"
But as a human, I kind of disagree, to me "grep -q 'class htb .* rate
12Kbit ceil 34359Mbit burst 375000b cburst 37....b'" seemed way easier
to read and and probably good enough for our tests.
I won't ask for a v3, I can also adjust the tests myself if you agree
with me doing that, to avoid another back-and-forth on this patch.
The changes I'm thinking of are:
https://github.com/dceara/ovn/commit/55fb6a9
Looking forward to hearing back from you.
Regards,
Dumitru
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-ext'])
> OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
> - grep -q 'class htb .* rate 400Kbit ceil 600Kbit burst
> 750000b cburst 750000b'])
> + grep -q 'class htb .* rate 400Kbit ceil 600Kbit burst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> # The same now with ovs db read only
> #
> @@ -6740,24 +6740,24 @@ wake_up_ovsdb .
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* rate 200Kbit ceil 300Kbit burst
> 375000b cburst 375000b'])
> + grep -q 'class htb .* rate 200Kbit ceil 300Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-ext'])
> OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
> - grep -q 'class htb .* rate 400Kbit ceil 600Kbit burst
> 750000b cburst 750000b'])
> + grep -q 'class htb .* rate 400Kbit ceil 600Kbit burst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options
> qos_min_rate=200000])
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options
> qos_max_rate=300000])
>
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* rate 12Kbit ceil 34359Mbit burst
> 375000b cburst 373662b'])
> + grep -q 'class htb .* rate 12Kbit ceil 34359Mbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options
> qos_burst=3000000])
> OVS_WAIT_UNTIL([test "$(tc qdisc show | grep 'htb 1: dev ovs-public')" = ""])
>
> AT_CHECK([ovn-nbctl set Logical_Switch_Port ext options:qos_max_rate=800000])
> OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
> - grep -q 'class htb .* rate 400Kbit ceil 800Kbit burst
> 750000b cburst 750000b'])
> + grep -q 'class htb .* rate 400Kbit ceil 800Kbit burst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl set Logical_Switch_Port public
> options:qos_min_rate=400000])
> AT_CHECK([ovn-nbctl set Logical_Switch_Port public
> options:qos_max_rate=800000])
> @@ -6765,7 +6765,7 @@ AT_CHECK([ovn-nbctl set Logical_Switch_Port public
> options:qos_burst=6000000])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* rate 400Kbit ceil 800Kbit burst
> 750000b cburst 750000b'])
> + grep -q 'class htb .* rate 400Kbit ceil 800Kbit burst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port ext options
> qos_min_rate=400000])
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port ext options
> qos_max_rate=800000])
> @@ -6775,7 +6775,7 @@ OVS_WAIT_UNTIL([test "$(tc qdisc show | grep 'htb 1:
> dev ovs-ext')" = ""])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* rate 400Kbit ceil 800Kbit burst
> 750000b cburst 750000b'])
> + grep -q 'class htb .* rate 400Kbit ceil 800Kbit burst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options
> qos_min_rate=400000])
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options
> qos_max_rate=800000])
> @@ -6793,25 +6793,25 @@ AT_CHECK([ovn-nbctl set Logical_Switch_Port sw11
> options:qos_burst=6000000])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* rate 200Kbit ceil 350Kbit burst
> 375000b cburst 374999b'])
> + grep -q 'class htb .* rate 200Kbit ceil 350Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-ext'])
> OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
> - grep -q 'class htb .* prio 0 rate 400Kbit ceil 700Kbit burst
> 750000b cburst 749999b'])
> + grep -q 'class htb .* prio 0 rate 400Kbit ceil 700Kbit burst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02
> options:qos_min_rate=300000])
> AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02
> options:qos_max_rate=500000])
> AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02 options:qos_burst=3000000])
>
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* prio 0 rate 300Kbit ceil 500Kbit burst
> 375000b cburst 375000b'])
> + grep -q 'class htb .* prio 0 rate 300Kbit ceil 500Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl set Logical_Switch_Port sw12
> options:qos_min_rate=400000])
> AT_CHECK([ovn-nbctl set Logical_Switch_Port sw12
> options:qos_max_rate=500000])
> AT_CHECK([ovn-nbctl set Logical_Switch_Port sw12 options:qos_burst=3000000])
>
> OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
> - grep -q 'class htb .* prio 0 rate 400Kbit ceil 500Kbit burst
> 375000b cburst 375000b'])
> + grep -q 'class htb .* prio 0 rate 400Kbit ceil 500Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw02 options
> qos_min_rate=300000])
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw02 options
> qos_max_rate=500000])
> @@ -6819,9 +6819,9 @@ AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw02
> options qos_burst=3000000])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* rate 200Kbit ceil 350Kbit burst
> 375000b cburst 374999b'])
> + grep -q 'class htb .* rate 200Kbit ceil 350Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
> OVS_WAIT_UNTIL([test "$(tc class show dev ovs-public | \
> - grep 'class htb .* prio 0 rate 300Kbit ceil 500Kbit burst
> 375000b cburst 375000b')" = ""])
> + grep 'class htb .* prio 0 rate 300Kbit ceil 500Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b')" = ""])
>
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw01 options
> qos_min_rate=200000])
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw01 options
> qos_max_rate=350000])
> @@ -6830,9 +6830,9 @@ OVS_WAIT_UNTIL([test "$(tc qdisc show | grep 'htb 1:
> dev ovs-public')" = ""])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-ext'])
> OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
> - grep -q 'class htb .* prio 0 rate 400Kbit ceil 700Kbit burst
> 750000b cburst 749999b'])
> + grep -q 'class htb .* prio 0 rate 400Kbit ceil 700Kbit burst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
> OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
> - grep -q 'class htb .* prio 0 rate 400Kbit ceil 500Kbit burst
> 375000b cburst 375000b'])
> + grep -q 'class htb .* prio 0 rate 400Kbit ceil 500Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw11 options
> qos_min_rate=400000])
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw11 options
> qos_max_rate=700000])
> @@ -6840,9 +6840,9 @@ AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw11
> options qos_burst=6000000])
>
> OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-ext'])
> OVS_WAIT_UNTIL([test "$(tc class show dev ovs-ext | \
> - grep 'class htb .* prio 0 rate 400Kbit ceil 700Kbit burst
> 750000b cburst 749999b')" = ""])
> + grep 'class htb .* prio 0 rate 400Kbit ceil 700Kbit burst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 7[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b')" = ""])
> OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
> - grep -q 'class htb .* prio 0 rate 400Kbit ceil 500Kbit burst
> 375000b cburst 375000b'])
> + grep -q 'class htb .* prio 0 rate 400Kbit ceil 500Kbit burst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 3[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw12 options
> qos_min_rate=400000])
> AT_CHECK([ovn-nbctl remove Logical_Switch_Port sw12 options
> qos_max_rate=500000])
> @@ -6855,7 +6855,7 @@ AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02
> options:qos_max_rate=6000000000
> AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02 options:qos_burst=1000000])
>
> OVS_WAIT_UNTIL([tc class show dev ovs-public | \
> - grep -q 'class htb .* prio 0 rate 5Gbit ceil 6Gbit burst
> 125000b cburst 124500b'])
> + grep -q 'class htb .* prio 0 rate 5Gbit ceil 6Gbit burst
> 1[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b cburst
> 1[[0-9]][[0-9]][[0-9]][[0-9]][[0-9]]b'])
>
> OVN_CLEANUP_CONTROLLER([hv1])
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
