Re: [PATCH net-next v2 2/4] net/sched/sch_cake: reduce parent backlog when clearing a tin

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit "net/sched/sch_cake: reduce parent backlog when
  clearing a tin" fixes a real, user-visible accounting bug but has no…

--

net/sched/sch_cake: reduce parent backlog when clearing a tin

The patch changes cake_configure_rates() so that, on a reconfigure
(!rate_adjust), it snapshots CAKE's qlen and backlog before purging the
unused tins with cake_clear_tin(). Afterwards it calls
qdisc_tree_reduce_backlog() with the delta, so classful parents like HTB
no longer keep counting the purged packets. The reset path is left
alone because qdisc_purge_queue() already reduces the tree there.

Link: https://sashiko.dev/#/patchset/20260816012109.2865223-1-ooonea@gmail.com
Link: https://lore.kernel.org/netdev/20260816012109.2865223-1-ooonea@gmail.com/
Link: https://lore.kernel.org/netdev/8733ugtry8.fsf@toke.dk/

> The issue:
> CAKE has purged unused tins on DiffServ reconfiguration since commit
> 83f8fd69af4f ("sch_cake: Add DiffServ handling") without propagating the
> resulting backlog reduction to classful parents. The issue was noticed
> while reviewing 7cbfb180945c, but predates that change.

[ ... ]

> Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260816012109.2865223-1-ooonea@gmail.com
> Link: https://lore.kernel.org/netdev/20260816012109.2865223-1-ooonea@gmail.com/
> Link: https://lore.kernel.org/netdev/8733ugtry8.fsf@toke.dk/
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

[Severity: Low]
This isn't a bug, but should this have a Fixes: trailer?

The changelog already names the commit that introduced the missing
parent accounting in cake_configure_rates()->cake_clear_tin(). The
trailer block has Reported-by, Link, Reviewed-by, Acked-by and
Signed-off-by, but no Fixes: tag.

Without the tag, stable and backport tooling may not match this fix to
83f8fd69af4f. Stable kernels would then keep the wrong HTB parent
qlen/backlog after a diffserv8 to diffserv3 change.

Would something like this work?

Fixes: 83f8fd69af4f ("sch_cake: Add DiffServ handling")

The code change itself looks correct to me. It reduces the parent only
for the reconfigure purge, and the reset path is already covered by
qdisc_purge_queue().

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-5RTM.v2.r0.20261015051735%40mojatatu.com