This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
selftests/tc-testing: taprio: add case for the software minimum interval
This adds a tdc case (id f4b2) to
tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json
which asserts that a pure software taprio schedule with 50000 ns entries is
rejected, since the entries are below TAPRIO_MIN_SW_INTERVAL_NS.
> 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 cd19d05925e40..3fe66839f8aed 100644
> --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json
> +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json
> @@ -154,6 +154,28 @@
> "echo \"1\" > /sys/bus/netdevsim/del_device"
> ]
> },
> + {
> + "id": "f4b2",
> + "name": "Add taprio Qdisc with software interval below the minimum",
> + "category": [
> + "qdisc",
> + "taprio"
> + ],
> + "plugins": {
> + "requires": "nsPlugin"
> + },
> + "setup": [
> + "echo \"1 1 8\" > /sys/bus/netdevsim/new_device"
> + ],
> + "cmdUnderTest": "$TC qdisc add dev $ETH root handle 1: taprio num_tc
> 2 queues 1@0 1@1 sched-entry S 01 50000 sched-entry S 02 50000 clockid
> CLOCK_TAI",
> + "expExitCode": "2",
[Severity: Low]
This isn't a bug, but should there also be a companion case that pins the
other half of the rule? The kernel rule this case targets is conditional in
taprio_min_interval():
net/sched/sch_taprio.c:taprio_min_interval() {
s64 min_interval = length_to_duration(q, ETH_ZLEN);
/* Only pure software schedules arm the per-entry hrtimer. */
if (!FULL_OFFLOAD_IS_ENABLED(q->flags) &&
!TXTIME_ASSIST_IS_ENABLED(q->flags))
min_interval = max_t(s64, min_interval,
TAPRIO_MIN_SW_INTERVAL_NS);
return min_interval;
}
The new f4b2 case only covers the branch being taken. Reading the rest of
taprio.json, no case appears able to flip that branch: the flags-carrying
cases ba39, 9462, 8d92, d092, 8471, 0a85 and 3e1e all use 300000 ns
intervals with "flags 0x1", and e8a1/6a83 use 20000000 ns with "flags 0x2",
so they pass whether or not the 100 us floor is gated on the flags.
Would that mean a change making the floor unconditional, and thus rejecting
hardware-offloaded or txtime-assist schedules with sub-100 us gates, still
leaves the whole taprio.json suite reporting PASS, including f4b2?
A mirror case using "flags 0x1" with the same 50000 ns entries and
expExitCode 0 would cover the exemption. On netdevsim this looks feasible:
netdevsim has no get_link_ksettings, so taprio_set_picos_per_byte() falls
back to SPEED_10, giving picos_per_byte 800000 and
length_to_duration(ETH_ZLEN) of 48000 ns, so 50000 ns entries and the
derived 100000 ns cycle are above the unconditional floor.
> + "verifyCmd": "$TC qdisc show dev $ETH",
> + "matchPattern": "qdisc taprio 1: root refcnt",
> + "matchCount": "0",
> + "teardown": [
> + "echo \"1\" > /sys/bus/netdevsim/del_device"
> + ]
> + },
[ ... ]
