lkml.org 
[lkml]   [2023]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v4 net-next 10/11] selftests/tc-testing: test that taprio can only be attached as root
    Date
    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
    \
     
     \ /
      Last update: 2023-08-07 21:39    [W:4.097 / U:0.432 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site