Skip to content

shaper: detect a cake_mq root qdisc instead of assuming cake (#353) - #391

Closed
ooonea wants to merge 1 commit into
lynxthecat:masterfrom
ooonea:feat/cake-mq-root-qdisc-detect
Closed

shaper: detect a cake_mq root qdisc instead of assuming cake (#353)#391
ooonea wants to merge 1 commit into
lynxthecat:masterfrom
ooonea:feat/cake-mq-root-qdisc-detect

Conversation

@ooonea

@ooonea ooonea commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Closes the gap reported in #353. @lynxthecat asked me in that thread whether I saw a good way to address it; this is the shape I proposed there, now written out.

The failure mode

set_shaper_rate() hardcodes cake in its tc qdisc change root call. If the root qdisc is cake_mq, the kernel rejects the change with Invalid qdisc name — but the call sends stderr to /dev/null and its exit status is never checked, so cake-autorate keeps emitting SHAPER log lines while shaping nothing. It fails silently, which is worse than failing loudly: from the logs everything looks fine.

This is reachable today, not hypothetical: sqm-scripts picks cake_mq whenever USE_MQ=1 and the device has more than one queue (functions.sh:966-968), and OpenWrt carries the kernel side in backport-6.12 (700-02..700-07) plus the iproute2 side in 001-1-tc-cake-add-cake_mq-support. Configuring the cake_mq instance directly and letting the config reach the sub-qdiscs is the designed behaviour — 700-04 is literally "Share config across cake_mq sub-qdiscs".

The change

~15 lines. get_root_qdisc_kind() reads tc qdisc show root dev <if> once per direction at startup and stores the kind in a new qdisc_kind[dl|ul]; set_shaper_rate() uses that instead of the literal. Anything unrecognised — htb, noqueue, empty output, a leaf line — falls back to cake, so a conventional setup is byte-identical to today. No per-iteration cost, no new config knob.

What I tested, and what I could not

Tested:

  • 6/6 detection cases against real tc output strings: cake_mq root → cake_mq; cake root, htb root, noqueue, a parent 1:1 leaf line, and empty output → cake
  • against a real tc qdisc show root, in a network namespace with a dummy interface: a genuine cake root is detected as cake; fq_codel and htb roots fall back to cake
  • bash -n clean, shellcheck -S warning reports nothing on the new lines

Not tested, and this is the part that needs someone with the hardware: I have no cake_mq-capable box, so I have not observed an actual tc qdisc change root dev <if> cake_mq bandwidth <X> taking effect end to end. The detection is verified; that the subsequent rate change propagates to the sub-qdiscs rests on reading 700-04/701-02 rather than on my having seen it work.

Marking this a draft for exactly that reason. @Snuupy, @CHC383 — you raised #353 and suggested the grep -q cake_mq approach; if either of you can run this on a multi-queue setup with USE_MQ=1 and confirm the shaper actually tracks, I will flip it out of draft. If it turns out change on a cake_mq root needs different handling, better to find that now than after merge.

@lynxthecat

Copy link
Copy Markdown
Owner

This looks pretty good to me. Pedantic me wondered about qdisc_type rather than qdisc_kind. Any thoughts on which is a better fit?

@ooonea

ooonea commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

I went with kind because it is iproute2's own word for this rather than a preference of mine. tc qdisc help ends its usage with [ [ QDISC_KIND ] [ help | OPTIONS ] ] and spells out QDISC_KIND := { [p|b]fifo | tbf | prio | red | etc. }; the parser function behind it is get_qdisc_kind(), and on the netlink side the attribute carrying the string is TCA_KIND. tc qdisc show then prints it immediately after the word qdisc, which is exactly what the regex here anchors on.

There is a local reason too: type is already spoken for twice in cake-autorate.sh — the message class in log_msg (local type=${1}) and the shell variable type in the config validation (typeof, user_type, valid_type). qdisc_kind keeps the three apart at a glance. That said it is your codebase, so if you prefer qdisc_type say the word and I will rename it.

Still keeping this in draft until someone with a multi-queue box can confirm the tc qdisc change actually lands on a cake_mq root — the detection is covered by fixtures and a real tc qdisc show root in a netns here, but I have no hardware to prove the change propagates.

@lynxthecat

lynxthecat commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Perhaps I can test on my RT3200 actually?

@ooonea

ooonea commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

That would be great! The detection half is already covered (fixtures plus a live tc qdisc show parse), but whether the change calls behave on a real cake_mq root is exactly the part I could not verify without multi-queue hardware. If it works on your RT3200, I'll mark this ready for review.

…cat#353)

set_shaper_rate() hardcoded "cake" in its `tc qdisc change root` call. Where
the root qdisc is cake_mq -- which sqm-scripts selects whenever USE_MQ=1 and
the device has more than one queue (functions.sh:966-968) -- that call fails
with "Invalid qdisc name", and because stderr goes to /dev/null and the exit
status is ignored, cake-autorate carries on logging SHAPER lines while shaping
nothing at all. Silent inertness rather than an error.

Detect the root qdisc kind once per direction at startup and use whatever is
actually there. Anything that is not cake_mq falls back to "cake", so a
conventional single-cake setup behaves byte-identically to before.

cake_mq is the v7.0 series backported by OpenWrt into backport-6.12
(700-02..700-07), with userspace support in the iproute2 patch
001-1-tc-cake-add-cake_mq-support. Configuring the cake_mq instance directly
and letting the config propagate to the sub-qdiscs is the intended path --
700-04, "Share config across cake_mq sub-qdiscs".

Assisted-by: Claude Code
@ooonea
ooonea force-pushed the feat/cake-mq-root-qdisc-detect branch from ad92569 to c177ef4 Compare August 9, 2026 12:03
@ooonea ooonea closed this Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants