| Message ID | 20250304193813.3225343-1-jonathan.lennox@8x8.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] tc-tests: Update tc police action tests for tc buffer size rounding fixes. | expand |
Adding the relevant maintainers and Pedro. On 3/4/25 8:38 PM, Jonathan Lennox wrote: > Before tc's recent change to fix rounding errors, several tests which > specified a burst size of "1m" would translate back to being 1048574 > bytes (2b less than 1Mb). sprint_size prints this as "1024Kb". > > With the tc fix, the burst size is instead correctly reported as > 1048576 bytes (precisely 1Mb), which sprint_size prints as "1Mb". > > This updates the expected output in the tests' matchPattern values > accordingly. > > Signed-off-by: Jonathan Lennox <jonathan.lennox@8x8.com> This is MIME multipart message, please send plaintext message instead (PW surprisingly digest it, but not my tools). AFAICS this fix will break the tests when running all version of iproute2 except the upcoming one. I think this is not good enough; you should detect the tc tool version and update expected output accordingly. If that is not possible, I think it would be better to simply revert the TC commit. Thanks, Paolo
On Tue, 11 Mar 2025 10:16:14 +0100 Paolo Abeni wrote: > AFAICS this fix will break the tests when running all version of > iproute2 except the upcoming one. I think this is not good enough; you > should detect the tc tool version and update expected output accordingly. > > If that is not possible, I think it would be better to simply revert the > TC commit. Alternatively since it's a regex match, maybe we could accept both? - "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1024Kb mtu 2Kb action reclassify", + "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst (1Mb|1024Kb) mtu 2Kb action reclassify", ? Not sure which option is most "correct" from TDC's perspective..
On Tue, Mar 11, 2025 at 5:49 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Tue, 11 Mar 2025 10:16:14 +0100 Paolo Abeni wrote: > > AFAICS this fix will break the tests when running all version of > > iproute2 except the upcoming one. I think this is not good enough; you > > should detect the tc tool version and update expected output accordingly. > > > > If that is not possible, I think it would be better to simply revert the > > TC commit. > > Alternatively since it's a regex match, maybe we could accept both? > > - "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1024Kb mtu 2Kb action reclassify", > + "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst (1Mb|1024Kb) mtu 2Kb action reclassify", > > ? Not sure which option is most "correct" from TDC's perspective.. It should work. Paolo's suggestion is also reasonable. cheers, jamal
I've submitted an updated version of the patch with the regex accepting both the old and new versions. > On Mar 11, 2025, at 7:15 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > On Tue, Mar 11, 2025 at 5:49 AM Jakub Kicinski <kuba@kernel.org> wrote: >> >> On Tue, 11 Mar 2025 10:16:14 +0100 Paolo Abeni wrote: >>> AFAICS this fix will break the tests when running all version of >>> iproute2 except the upcoming one. I think this is not good enough; you >>> should detect the tc tool version and update expected output accordingly. >>> >>> If that is not possible, I think it would be better to simply revert the >>> TC commit. >> >> Alternatively since it's a regex match, maybe we could accept both? >> >> - "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1024Kb mtu 2Kb action reclassify", >> + "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst (1Mb|1024Kb) mtu 2Kb action reclassify", >> >> ? Not sure which option is most "correct" from TDC's perspective.. > > It should work. Paolo's suggestion is also reasonable. > > cheers, > jamal
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json index dd8109768f8f..ae31dbeb45d8 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json @@ -689,7 +689,7 @@ "cmdUnderTest": "$TC actions add action police rate 7mbit burst 1m continue index 1", "expExitCode": "0", "verifyCmd": "$TC actions get action police index 1", - "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1024Kb mtu 2Kb action continue", + "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1Mb mtu 2Kb action continue", "matchCount": "1", "teardown": [ "$TC actions flush action police" @@ -716,7 +716,7 @@ "cmdUnderTest": "$TC actions add action police rate 7mbit burst 1m drop index 1", "expExitCode": "0", "verifyCmd": "$TC actions ls action police", - "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1024Kb mtu 2Kb action drop", + "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1Mb mtu 2Kb action drop", "matchCount": "1", "teardown": [ "$TC actions flush action police" @@ -743,7 +743,7 @@ "cmdUnderTest": "$TC actions add action police rate 7mbit burst 1m ok index 1", "expExitCode": "0", "verifyCmd": "$TC actions ls action police", - "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1024Kb mtu 2Kb action pass", + "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1Mb mtu 2Kb action pass", "matchCount": "1", "teardown": [ "$TC actions flush action police" @@ -770,7 +770,7 @@ "cmdUnderTest": "$TC actions add action police rate 7mbit burst 1m reclassify index 1", "expExitCode": "0", "verifyCmd": "$TC actions get action police index 1", - "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1024Kb mtu 2Kb action reclassify", + "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1Mb mtu 2Kb action reclassify", "matchCount": "1", "teardown": [ "$TC actions flush action police" @@ -797,7 +797,7 @@ "cmdUnderTest": "$TC actions add action police rate 7mbit burst 1m pipe index 1", "expExitCode": "0", "verifyCmd": "$TC actions ls action police", - "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1024Kb mtu 2Kb action pipe", + "matchPattern": "action order [0-9]*: police 0x1 rate 7Mbit burst 1Mb mtu 2Kb action pipe", "matchCount": "1", "teardown": [ "$TC actions flush action police"
Before tc's recent change to fix rounding errors, several tests which specified a burst size of "1m" would translate back to being 1048574 bytes (2b less than 1Mb). sprint_size prints this as "1024Kb". With the tc fix, the burst size is instead correctly reported as 1048576 bytes (precisely 1Mb), which sprint_size prints as "1Mb". This updates the expected output in the tests' matchPattern values accordingly. Signed-off-by: Jonathan Lennox <jonathan.lennox@8x8.com> --- .../selftests/tc-testing/tc-tests/actions/police.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)