Patches 1 and 3 are fixes for tdc that were discovered when running it using defconfig + tc-testing config and against the latest iproute2.
Patch 2 improves the taprio script that waits for scheduler changes.
Finally, Patch 4 enables all tdc tests.
Pedro Tammela (4): selftests: tc-testing: add missing netfilter config selftests: tc-testing: check if 'jq' is available in taprio script selftests: tc-testing: adjust fq test to latest iproute2 selftests: tc-testing: enable all tdc tests
tools/testing/selftests/tc-testing/config | 1 + .../selftests/tc-testing/scripts/taprio_wait_for_admin.sh | 5 +++++ tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq.json | 2 +- tools/testing/selftests/tc-testing/tdc.sh | 3 +-- 4 files changed, 8 insertions(+), 3 deletions(-)
On a default config + tc-testing config build, tdc will miss all the netfilter related tests because it's missing: CONFIG_NETFILTER=y
Signed-off-by: Pedro Tammela pctammela@mojatatu.com --- tools/testing/selftests/tc-testing/config | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config index c60acba951c2..db176fe7d0c3 100644 --- a/tools/testing/selftests/tc-testing/config +++ b/tools/testing/selftests/tc-testing/config @@ -8,6 +8,7 @@ CONFIG_VETH=y # # Core Netfilter Configuration # +CONFIG_NETFILTER=y CONFIG_NETFILTER_ADVANCED=y CONFIG_NF_CONNTRACK=m CONFIG_NF_CONNTRACK_MARK=y
If 'jq' is not available the taprio tests that use this script will run forever. Check if it exists before entering the while loop.
Signed-off-by: Pedro Tammela pctammela@mojatatu.com --- .../selftests/tc-testing/scripts/taprio_wait_for_admin.sh | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh b/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh index f5335e8ad6b4..68f2c6eaa802 100755 --- a/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh +++ b/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh @@ -3,6 +3,11 @@ TC="$1"; shift ETH="$1"; shift
+if ! command -v jq &> /dev/null; then + echo "Please install jq" + exit 1 +fi + # The taprio architecture changes the admin schedule from a hrtimer and not # from process context, so we need to wait in order to make sure that any # schedule change actually took place.
hi Pedro,
On Tue, Jan 23, 2024 at 1:28 PM Pedro Tammela pctammela@mojatatu.com wrote:
If 'jq' is not available the taprio tests that use this script will run forever. Check if it exists before entering the while loop.
Signed-off-by: Pedro Tammela pctammela@mojatatu.com
.../selftests/tc-testing/scripts/taprio_wait_for_admin.sh | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh b/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh index f5335e8ad6b4..68f2c6eaa802 100755 --- a/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh +++ b/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh @@ -3,6 +3,11 @@ TC="$1"; shift ETH="$1"; shift
+if ! command -v jq &> /dev/null; then
- echo "Please install jq"
- exit 1
+fi
nit: what about returning $KSFT_SKIP (that is 4) if jq is not there? so the test does not fail. thanks!
On 23/01/2024 10:17, Davide Caratti wrote:
hi Pedro,
On Tue, Jan 23, 2024 at 1:28 PM Pedro Tammela pctammela@mojatatu.com wrote:
If 'jq' is not available the taprio tests that use this script will run forever. Check if it exists before entering the while loop.
Signed-off-by: Pedro Tammela pctammela@mojatatu.com
.../selftests/tc-testing/scripts/taprio_wait_for_admin.sh | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh b/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh index f5335e8ad6b4..68f2c6eaa802 100755 --- a/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh +++ b/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh @@ -3,6 +3,11 @@ TC="$1"; shift ETH="$1"; shift
+if ! command -v jq &> /dev/null; then
- echo "Please install jq"
- exit 1
+fi
nit: what about returning $KSFT_SKIP (that is 4) if jq is not there? so the test does not fail. thanks!
Since these scripts are run in the setup phase, it has a special treatment.
Take for example this run: ok 1 ba39 - Add taprio Qdisc to multi-queue device (8 queues) ok 2 9462 - Add taprio Qdisc with multiple sched-entry ok 3 8d92 - Add taprio Qdisc with txtime-delay ok 4 d092 - Delete taprio Qdisc with valid handle ok 5 8471 - Show taprio class ok 6 0a85 - Add taprio Qdisc to single-queue device ok 7 3e1e - Add taprio Qdisc with an invalid cycle-time ok 8 39b4 - Reject grafting taprio as child qdisc of software taprio # skipped - "-----> prepare stage" did not complete successfully
ok 9 e8a1 - Reject grafting taprio as child qdisc of offloaded taprio # skipped - skipped - previous setup failed 9 39b4
ok 10 a7bf - Graft cbs as child of software taprio # skipped - skipped - previous setup failed 9 39b4
ok 11 6a83 - Graft cbs as child of offloaded taprio # skipped - skipped - previous setup failed 9 39b4
As of today it returns 0, success in ksft, even though it clearly wasn't. Looking at the code any failures in the setup/teardown phase will stop the run, skip all the remaining tests but still return success.
About returning skip from the script, aside from marking it as skip and continuing the suite, we would need to run a silent teardown, one that executes all commands in the specified teardown but ignores errors. In this case we are assuming all setup steps follow KSFT return codes. Not sure if it it's reasonable or not...
As your suggestion is not a blocker, I would rather address the above problems in a follow up series since they will require some refactoring. WDYT?
hello Pedro, thanks for your answer!
On Tue, Jan 23, 2024 at 5:47 PM Pedro Tammela pctammela@mojatatu.com wrote:
On 23/01/2024 10:17, Davide Caratti wrote:
hi Pedro,
On Tue, Jan 23, 2024 at 1:28 PM Pedro Tammela pctammela@mojatatu.com wrote:
If 'jq' is not available the taprio tests that use this script will run forever. Check if it exists before entering the while loop.
[...]
nit: what about returning $KSFT_SKIP (that is 4) if jq is not there? so the test does not fail. thanks!
Since these scripts are run in the setup phase, it has a special treatment.
Take for example this run: ok 1 ba39 - Add taprio Qdisc to multi-queue device (8 queues) ok 2 9462 - Add taprio Qdisc with multiple sched-entry ok 3 8d92 - Add taprio Qdisc with txtime-delay ok 4 d092 - Delete taprio Qdisc with valid handle ok 5 8471 - Show taprio class ok 6 0a85 - Add taprio Qdisc to single-queue device ok 7 3e1e - Add taprio Qdisc with an invalid cycle-time ok 8 39b4 - Reject grafting taprio as child qdisc of software taprio # skipped - "-----> prepare stage" did not complete successfully
ok 9 e8a1 - Reject grafting taprio as child qdisc of offloaded taprio # skipped - skipped - previous setup failed 9 39b4
[...]
As of today it returns 0, success in ksft, even though it clearly wasn't. Looking at the code any failures in the setup/teardown phase will stop the run, skip all the remaining tests but still return success.
About returning skip from the script, aside from marking it as skip and continuing the suite, we would need to run a silent teardown, one that executes all commands in the specified teardown but ignores errors. In this case we are assuming all setup steps follow KSFT return codes. Not sure if it it's reasonable or not...
wouldn't this be fixed by adding this line:
"dependsOn" : "command -v jq >/dev/null"
to test scenarios 39b4 and e8a1 ? I'm asking this because jq is used also in verifyCmd after the script, to parse results. Background for this question: I see tdc skipping both setup and teardown stages for each test case in taprio.json where this line:
"dependsOn": "command -v ciao >/dev/null",
is present. Rather than doing a setup + silent teardown, just do nothing and go to the next test.
As your suggestion is not a blocker, I would rather address the above problems in a follow up series since they will require some refactoring. WDYT?
no objections, but I'm curious to see if "dependsOn" would fix this case :) thanks!
On 24/01/2024 07:31, Davide Caratti wrote:
hello Pedro, thanks for your answer!
On Tue, Jan 23, 2024 at 5:47 PM Pedro Tammela pctammela@mojatatu.com wrote:
On 23/01/2024 10:17, Davide Caratti wrote:
hi Pedro,
On Tue, Jan 23, 2024 at 1:28 PM Pedro Tammela pctammela@mojatatu.com wrote:
If 'jq' is not available the taprio tests that use this script will run forever. Check if it exists before entering the while loop.
[...]
nit: what about returning $KSFT_SKIP (that is 4) if jq is not there? so the test does not fail. thanks!
Since these scripts are run in the setup phase, it has a special treatment.
Take for example this run: ok 1 ba39 - Add taprio Qdisc to multi-queue device (8 queues) ok 2 9462 - Add taprio Qdisc with multiple sched-entry ok 3 8d92 - Add taprio Qdisc with txtime-delay ok 4 d092 - Delete taprio Qdisc with valid handle ok 5 8471 - Show taprio class ok 6 0a85 - Add taprio Qdisc to single-queue device ok 7 3e1e - Add taprio Qdisc with an invalid cycle-time ok 8 39b4 - Reject grafting taprio as child qdisc of software taprio # skipped - "-----> prepare stage" did not complete successfully
ok 9 e8a1 - Reject grafting taprio as child qdisc of offloaded taprio # skipped - skipped - previous setup failed 9 39b4
[...]
As of today it returns 0, success in ksft, even though it clearly wasn't. Looking at the code any failures in the setup/teardown phase will stop the run, skip all the remaining tests but still return success.
About returning skip from the script, aside from marking it as skip and continuing the suite, we would need to run a silent teardown, one that executes all commands in the specified teardown but ignores errors. In this case we are assuming all setup steps follow KSFT return codes. Not sure if it it's reasonable or not...
wouldn't this be fixed by adding this line:
"dependsOn" : "command -v jq >/dev/null"
to test scenarios 39b4 and e8a1 ? I'm asking this because jq is used also in verifyCmd after the script, to parse results. Background for this question: I see tdc skipping both setup and teardown stages for each test case in taprio.json where this line:
"dependsOn": "command -v ciao >/dev/null",is present. Rather than doing a setup + silent teardown, just do nothing and go to the next test.
Oh indeed! Thanks for the suggestion this is much nicer. I will send a v2!
As your suggestion is not a blocker, I would rather address the above problems in a follow up series since they will require some refactoring. WDYT?
no objections, but I'm curious to see if "dependsOn" would fix this case :) thanks!
Adjust the fq verify regex to the latest iproute2
Signed-off-by: Pedro Tammela pctammela@mojatatu.com --- tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq.json index be293e7c6d18..3a537b2ec4c9 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq.json +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq.json @@ -77,7 +77,7 @@ "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root fq quantum 9000", "expExitCode": "0", "verifyCmd": "$TC qdisc show dev $DUMMY", - "matchPattern": "qdisc fq 1: root refcnt [0-9]+ limit 10000p flow_limit 100p buckets.*orphan_mask 1023 quantum 9000b", + "matchPattern": "qdisc fq 1: root refcnt [0-9]+ limit 10000p flow_limit 100p.*quantum 9000b", "matchCount": "1", "teardown": [ "$TC qdisc del dev $DUMMY handle 1: root"
For the longest time tdc ran only actions and qdiscs tests. It's time to enable all the remaining tests so every user visible piece of TC is tested by the downstream CIs.
Signed-off-by: Pedro Tammela pctammela@mojatatu.com --- tools/testing/selftests/tc-testing/tdc.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/testing/selftests/tc-testing/tdc.sh b/tools/testing/selftests/tc-testing/tdc.sh index c53ede8b730d..cddff1772e10 100755 --- a/tools/testing/selftests/tc-testing/tdc.sh +++ b/tools/testing/selftests/tc-testing/tdc.sh @@ -63,5 +63,4 @@ try_modprobe sch_hfsc try_modprobe sch_hhf try_modprobe sch_htb try_modprobe sch_teql -./tdc.py -J`nproc` -c actions -./tdc.py -J`nproc` -c qdisc +./tdc.py -J`nproc`
linux-kselftest-mirror@lists.linaro.org
-
Davide Caratti -
Pedro Tammela