[PATCH net v3 9/9] selftests: tc-testing: update ETS test 41f5 for clamped quanta

[PATCH net v3 9/9] selftests: tc-testing: update ETS test 41f5 for clamped quanta

[Jamal Hadi Salim]

Commit "net/sched: ets: clamp quantum in parse and fallback paths"
moved the quantum floor into ets_quantum_parse(), so every explicitly
configured quantum is now clamped to [256, 1 << 20], not just the
psched_mtu() fallback.

Test 41f5 passes "quanta 4294967294 1 1" and matches the values back
verbatim, so all three bands now differ from what it expects:

  before: bands 3 quanta 4294967294 1 1
  after:  bands 3 quanta 1048576 256 256

Update the match pattern accordingly.

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
index ee09e6d6fdf3..d2eab61c099a 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
@@ -987,7 +987,7 @@
     },
     {
         "id": "41f5",
-        "name": "ETS offload where the sum of quanta wraps u32",
+        "name": "ETS offload with out-of-range quanta clamped",
         "category": [
             "qdisc",
             "ets"
@@ -1002,7 +1002,7 @@
         "cmdUnderTest": "$TC qdisc add dev $ETH root ets quanta 4294967294 1 1",
         "expExitCode": "0",
         "verifyCmd": "$TC qdisc show dev $ETH",
-        "matchPattern": "qdisc ets .*bands 3 quanta 4294967294 1 1",
+        "matchPattern": "qdisc ets .*bands 3 quanta 1048576 256 256",
         "matchCount": "1",
         "teardown": [
             "echo \"1\" > /sys/bus/netdevsim/del_device"
-- 
2.43.0

Re: [PATCH net v3 9/9] selftests: tc-testing: update ETS test 41f5 for clamped quanta

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)

- [Medium] Incomplete selftest update / undisclosed behavioural change:
  the accompanying commit 0bbbcd21591a ("net/sched: ets: clamp quantum…

Pre-existing issues:
- [Low] Concern that test 41f5 ("ETS offload with out-of-range quanta
  clamped") only checks the software qdisc dump via `tc qdisc show` and…

--

selftests: tc-testing: update ETS test 41f5 for clamped quanta

This updates tdc test 41f5 in qdiscs/ets.json so that its name and match
pattern reflect the new quantum clamping to [256, 1 << 20] introduced by
"net/sched: ets: clamp quantum in parse and fallback paths".  The test
still passes "quanta 4294967294 1 1" but now expects "quanta 1048576 256
256" in the qdisc dump.

> diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
> index ee09e6d6fdf3c..d2eab61c099a8 100644
> --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
> +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
> @@ -987,7 +987,7 @@
>      },
>      {
>          "id": "41f5",
> -        "name": "ETS offload where the sum of quanta wraps u32",
> +        "name": "ETS offload with out-of-range quanta clamped",
>          "category": [
>              "qdisc",
>              "ets"
> @@ -1002,7 +1002,7 @@
>          "cmdUnderTest": "$TC qdisc add dev $ETH root ets quanta 4294967294 1 1",
>          "expExitCode": "0",
>          "verifyCmd": "$TC qdisc show dev $ETH",
> -        "matchPattern": "qdisc ets .*bands 3 quanta 4294967294 1 1",
> +        "matchPattern": "qdisc ets .*bands 3 quanta 1048576 256 256",
>          "matchCount": "1",

[Severity: Low]
This isn't a bug introduced by this patch, but a note on what the renamed
test now covers.

The verification is only "$TC qdisc show dev $ETH", which reports the
software qdisc quanta.  The quanta actually handed to the driver are
filled in separately by ets_offload_change() in net/sched/sch_ets.c:

	qopt.replace_params.quanta[i] = quantum;
	qopt.replace_params.weights[i] = weight;

and netdevsim's nsim_setup_tc_ets() in drivers/net/netdevsim/tc.c does
nothing with them:

	case TC_ETS_REPLACE:
	case TC_ETS_DESTROY:
		break;

so nothing tdc can observe reflects the offload request itself.  That is
pre-existing and matches the idiom used by the other cases in this file.

Separately, since each quantum is now capped at 1 << 20 and there are at
most 16 bands, is the u32 sum-wrap condition the test was originally
written for still reachable at all?  If not, the rename retires that
coverage without a replacement.

[Severity: Medium]
Should the two neighbouring tests in this same file be updated in the
same patch?

The kernel-side commit in this series also removed the explicit
zero-quantum rejection from ets_quantum_parse(), which now reads:

net/sched/sch_ets.c:ets_quantum_parse() {
	*quantum = clamp_t(u32, nla_get_u32(attr), 256, 1 << 20);
	return 0;
}

The previous "ETS quantum cannot be zero" -EINVAL and extack are gone,
and nothing else in the path rejects 0: ets_quanta_policy declares
TCA_ETS_QUANTA_BAND as a bare NLA_U32 with no range, and
ets_qdisc_quanta_parse() only runs __nla_validate_nested() plus the band
count check before calling ets_quantum_parse().  A band quantum of 0 is
therefore clamped to 256 and the qdisc is created.

Tests e51a "Zero element in quanta" and e7f2 "Sole zero element in
quanta" still encode the old behaviour, for example e51a:

        "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root ets quanta 1000 0 800 700",
        "expExitCode": "1",
        "verifyCmd": "$TC qdisc show dev $DUMMY",
        "matchPattern": "qdisc ets",
        "matchCount": "0"

If tc forwards the literal 0 to the kernel, which is what 41f5 suggests
given that it passes 4294967294 and 1 through unmodified, then the add
now returns 0 instead of 1 and "tc qdisc show" prints "qdisc ets", so
both tests fail.  If tc filters the 0 locally the tests keep passing but
no longer exercise any kernel check, and nothing covers the new
accept-and-clamp semantics.

Either way, could the removal of the zero-quantum -EINVAL and its extack
be mentioned in the changelog?  Neither this commit message nor the
kernel one notes that the netlink error contract for
TCA_ETS_QUANTA_BAND == 0 changed, and sch_drr.c deliberately kept its
explicit zero reject.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-0CFC.v3.20260901204856%40mojatatu.com

Re: [PATCH net v3 9/9] selftests: tc-testing: update ETS test 41f5 for clamped quanta

[Jamal Hadi Salim]

On Fri, Sep 4, 2026 at 5:42 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)
>
> - [Medium] Incomplete selftest update / undisclosed behavioural change:
>   the accompanying commit 0bbbcd21591a ("net/sched: ets: clamp quantum…

Note: the e51a/e7f2 breakage branch is wrong (verified: iproute2
rejects the literal 0 client-side, both tests pass on the patched
kernel per our tests).
Note2: The underlying point is fair, though: the error-contract change
for TCA_ETS_QUANTA_BAND == 0 deserves one changelog sentence.

> Pre-existing issues:
> - [Low] Concern that test 41f5 ("ETS offload with out-of-range quanta
>   clamped") only checks the software qdisc dump via `tc qdisc show` and…

That matches the file's existing goal; the retired sum-wrap case is
indeed unreachable with the 1 MiB per-band ceiling (16 bands x 1 MiB
<< 2^32).

IMO, other than the changelog change for "medium" - nothing else to see here.

Phew! Cant believe i went over all that. I thought this new era was
about keeping up with huge influx of bug reports - but the influx from
the bots is higher. I am sure the s/w quality will improve but man oh
man this is too much work (change the changelog, no human has ever
asked me that!)

cheers,
jamal


> --
>
> selftests: tc-testing: update ETS test 41f5 for clamped quanta
>
> This updates tdc test 41f5 in qdiscs/ets.json so that its name and match
> pattern reflect the new quantum clamping to [256, 1 << 20] introduced by
> "net/sched: ets: clamp quantum in parse and fallback paths".  The test
> still passes "quanta 4294967294 1 1" but now expects "quanta 1048576 256
> 256" in the qdisc dump.
>
> > diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
> > index ee09e6d6fdf3c..d2eab61c099a8 100644
> > --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
> > +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ets.json
> > @@ -987,7 +987,7 @@
> >      },
> >      {
> >          "id": "41f5",
> > -        "name": "ETS offload where the sum of quanta wraps u32",
> > +        "name": "ETS offload with out-of-range quanta clamped",
> >          "category": [
> >              "qdisc",
> >              "ets"
> > @@ -1002,7 +1002,7 @@
> >          "cmdUnderTest": "$TC qdisc add dev $ETH root ets quanta 4294967294 1 1",
> >          "expExitCode": "0",
> >          "verifyCmd": "$TC qdisc show dev $ETH",
> > -        "matchPattern": "qdisc ets .*bands 3 quanta 4294967294 1 1",
> > +        "matchPattern": "qdisc ets .*bands 3 quanta 1048576 256 256",
> >          "matchCount": "1",
>
> [Severity: Low]
> This isn't a bug introduced by this patch, but a note on what the renamed
> test now covers.
>
> The verification is only "$TC qdisc show dev $ETH", which reports the
> software qdisc quanta.  The quanta actually handed to the driver are
> filled in separately by ets_offload_change() in net/sched/sch_ets.c:
>
>         qopt.replace_params.quanta[i] = quantum;
>         qopt.replace_params.weights[i] = weight;
>
> and netdevsim's nsim_setup_tc_ets() in drivers/net/netdevsim/tc.c does
> nothing with them:
>
>         case TC_ETS_REPLACE:
>         case TC_ETS_DESTROY:
>                 break;
>
> so nothing tdc can observe reflects the offload request itself.  That is
> pre-existing and matches the idiom used by the other cases in this file.
>
> Separately, since each quantum is now capped at 1 << 20 and there are at
> most 16 bands, is the u32 sum-wrap condition the test was originally
> written for still reachable at all?  If not, the rename retires that
> coverage without a replacement.
>
> [Severity: Medium]
> Should the two neighbouring tests in this same file be updated in the
> same patch?
>
> The kernel-side commit in this series also removed the explicit
> zero-quantum rejection from ets_quantum_parse(), which now reads:
>
> net/sched/sch_ets.c:ets_quantum_parse() {
>         *quantum = clamp_t(u32, nla_get_u32(attr), 256, 1 << 20);
>         return 0;
> }
>
> The previous "ETS quantum cannot be zero" -EINVAL and extack are gone,
> and nothing else in the path rejects 0: ets_quanta_policy declares
> TCA_ETS_QUANTA_BAND as a bare NLA_U32 with no range, and
> ets_qdisc_quanta_parse() only runs __nla_validate_nested() plus the band
> count check before calling ets_quantum_parse().  A band quantum of 0 is
> therefore clamped to 256 and the qdisc is created.
>
> Tests e51a "Zero element in quanta" and e7f2 "Sole zero element in
> quanta" still encode the old behaviour, for example e51a:
>
>         "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root ets quanta 1000 0 800 700",
>         "expExitCode": "1",
>         "verifyCmd": "$TC qdisc show dev $DUMMY",
>         "matchPattern": "qdisc ets",
>         "matchCount": "0"
>
> If tc forwards the literal 0 to the kernel, which is what 41f5 suggests
> given that it passes 4294967294 and 1 through unmodified, then the add
> now returns 0 instead of 1 and "tc qdisc show" prints "qdisc ets", so
> both tests fail.  If tc filters the 0 locally the tests keep passing but
> no longer exercise any kernel check, and nothing covers the new
> accept-and-clamp semantics.
>
> Either way, could the removal of the zero-quantum -EINVAL and its extack
> be mentioned in the changelog?  Neither this commit message nor the
> kernel one notes that the netlink error contract for
> TCA_ETS_QUANTA_BAND == 0 changed, and sch_drr.c deliberately kept its
> explicit zero reject.
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-0CFC.v3.20260901204856%40mojatatu.com