Messages in this thread Patch in this message | | | From | Vladimir Oltean <> | Subject | [PATCH v4 net-next 10/11] selftests/tc-testing: test that taprio can only be attached as root | Date | Mon, 7 Aug 2023 22:33:23 +0300 |
| |
Check that the "Can only be attached as root qdisc" error message from taprio is effective by attempting to attach it to a class of another taprio qdisc. That operation should fail.
In the bug that was squashed by change "net/sched: taprio: try again to report q->qdiscs[] to qdisc_leaf()", grafting a child taprio to a root software taprio would be misinterpreted as a change() to the root taprio. Catch this by looking at whether the base-time of the root taprio has changed to follow the base-time of the child taprio, something which should have absolutely never happened assuming correct semantics.
Vinicius points out that looking at "base_time" in the tc qdisc show output is unreliable because user space is in a race with the kernel applying the setting. So we create a helper bash script which waits while there is any pending schedule.
Link: https://lore.kernel.org/netdev/87il9w0xx7.fsf@intel.com/ Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Pedro Tammela <pctammela@mojatatu.com> --- v3->v4: introduce (and use) taprio_wait_for_admin.sh. The test depends upon this taprio patch to work properly: https://patchwork.kernel.org/project/netdevbpf/patch/20230807160827.4087483-2-vladimir.oltean@nxp.com/ Note: I couldn't find a way around the horrible tcmd.safe_substitute(NAMES) and the string quoting (no possibility to perform complex shell operations) than to put the logic I needed in a separate shell script and execute that.
v2->v3: none v1->v2: patch is new
.../tc-testing/taprio_wait_for_admin.sh | 16 ++++++ .../tc-testing/tc-tests/qdiscs/taprio.json | 50 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100755 tools/testing/selftests/tc-testing/taprio_wait_for_admin.sh
diff --git a/tools/testing/selftests/tc-testing/taprio_wait_for_admin.sh b/tools/testing/selftests/tc-testing/taprio_wait_for_admin.sh new file mode 100755 index 000000000000..f5335e8ad6b4 --- /dev/null +++ b/tools/testing/selftests/tc-testing/taprio_wait_for_admin.sh @@ -0,0 +1,16 @@ +#!/bin/bash + +TC="$1"; shift +ETH="$1"; shift + +# 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. +while :; do + has_admin="$($TC -j qdisc show dev $ETH root | jq '.[].options | has("admin")')" + if [ "$has_admin" = "false" ]; then + break; + fi + + sleep 1 +done diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json index 860b55df0d6d..f475967c509f 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json @@ -156,5 +156,55 @@ "teardown": [ "echo \"1\" > /sys/bus/netdevsim/del_device" ] + }, + { + "id": "39b4", + "name": "Reject grafting taprio as child qdisc of software taprio", + "category": [ + "qdisc", + "taprio" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [ + "echo \"1 1 8\" > /sys/bus/netdevsim/new_device", + "$TC qdisc replace dev $ETH handle 8001: parent root stab overhead 24 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 0 sched-entry S ff 20000000 clockid CLOCK_TAI", + "./taprio_wait_for_admin.sh $TC $ETH" + ], + "cmdUnderTest": "$TC qdisc replace dev $ETH parent 8001:7 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 200 sched-entry S ff 20000000 clockid CLOCK_TAI", + "expExitCode": "2", + "verifyCmd": "bash -c \"./taprio_wait_for_admin.sh $TC $ETH && $TC -j qdisc show dev $ETH root | jq '.[].options.base_time'\"", + "matchPattern": "0", + "matchCount": "1", + "teardown": [ + "$TC qdisc del dev $ETH root", + "echo \"1\" > /sys/bus/netdevsim/del_device" + ] + }, + { + "id": "e8a1", + "name": "Reject grafting taprio as child qdisc of offloaded taprio", + "category": [ + "qdisc", + "taprio" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [ + "echo \"1 1 8\" > /sys/bus/netdevsim/new_device", + "$TC qdisc replace dev $ETH handle 8001: parent root stab overhead 24 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 0 sched-entry S ff 20000000 flags 0x2", + "./taprio_wait_for_admin.sh $TC $ETH" + ], + "cmdUnderTest": "$TC qdisc replace dev $ETH parent 8001:7 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 200 sched-entry S ff 20000000 flags 0x2", + "expExitCode": "2", + "verifyCmd": "bash -c \"./taprio_wait_for_admin.sh $TC $ETH && $TC -j qdisc show dev $ETH root | jq '.[].options.base_time'\"", + "matchPattern": "0", + "matchCount": "1", + "teardown": [ + "$TC qdisc del dev $ETH root", + "echo \"1\" > /sys/bus/netdevsim/del_device" + ] } ] -- 2.34.1
| |