[PATCH net v3 1/2] net/sched: sch_hfsc: bound the classify inner-filter walk with a drift budget

[PATCH net v3 1/2] net/sched: sch_hfsc: bound the classify inner-filter walk with a drift budget

[Jamal Hadi Salim]

hfsc_classify() applies the "filter may only point downwards" level check
only when the filter result carries no bound class. A filter created with
a flowid gets res.class set once at bind time, so the check never runs for
it during classification. hfsc_adjust_levels() can later raise a class's
level without revalidating existing bindings, leaving two binds that were
each legal at bind time pointing at each other; the classify walk then
bounces between two interior classes forever with the qdisc lock held and
BH disabled — a soft lockup from a single packet. The stuck walk trips
the watchdog:

  watchdog: BUG: soft lockup - CPU#3 stuck for 13s! [ping:444]
  RIP: 0010:u32_classify+0x542/0x17f0
  ...
  tcf_classify+0x66/0xa0
  hfsc_enqueue+0x166/0xdf0

Bound the traversal with a budget of non-descending hops, the only way a
configured walk can move without descending the class tree once levels
drift after bind time. The budget is cumulative over the whole walk and
is deliberately not reset on a descending hop: a chain that alternates a
descent with a lateral hop would return the budget every lap and never
trip. Descending hops never decrement it, so legitimately deep trees are
unaffected and a terminating lateral chain still classifies normally.
Drop the packet with a rate-limited warning once the budget is exhausted,
mirroring the merged HTB fix.

This is a follow-up to commit 729c4896ab82 ("net/sched: sch_htb: limit
htb_classify inner-class filter hops"), which bounded the same classify
loop on the HTB side but left the HFSC walk unbounded.

Conditions to recreate the bug:
- CONFIG_NET_SCHED, CONFIG_NET_SCH_HFSC, CONFIG_NET_CLS_U32,
  CONFIG_LOCKUP_DETECTOR.
- Build a cycle with two legal-at-bind-time flowid binds and a level
  drift: class X 1:1 (child of root) with leaf child 1:10; class Y 1:2
  (sibling of X) with children 1:20 and 1:200; root u32 filter flowid
  1:1; filter on X flowid 1:2 (legal when Y is a leaf); after Y's level
  rises to 2, filter on Y flowid 1:1 (legal then). Send one packet (ping
  on the device). Unfixed kernel: classify spins with the qdisc lock
  held; with softlockup_panic=1 it panics.
- Reachable from unprivileged user via unshare -Urn (CAP_NET_ADMIN).

Fixes: a2f79227138c ("net_sched: sch_hfsc: fix classification loops")
Reported-by: Sashiko (gemini + nipa) <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/QDISC-CTUU.v2.20260913192614@mojatatu.com/
Link: https://sashiko.dev/#/patchset/QDISC-CTUU.v2.20260913192614@mojatatu.com
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-CTUU.v2.20260913192614%40mojatatu.com
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Tested-by: hybris <hybris@mojatatu.ai>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v3:
   -  Reword the budget comment and the warning to describe the
      cumulative non-descending hop budget that is implemented, and
      explain why it is deliberately not reset on a descending hop.
      (Sashiko: gemini, nipa)
v2:
   -  Replace the absolute hop bound (root->level) with a budget that
      counts only non-descending hops, so a legal lateral chain still
      classifies. (Sashiko)
   -  Reword the "strict descent" changelog claim (Sashiko).
---
 net/sched/sch_hfsc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index e87f5021a199..284490fd6ca9 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -386,6 +386,15 @@ cftree_update(struct hfsc_class *cl)
 #define	SM_MASK		((1ULL << SM_SHIFT) - 1)
 #define	ISM_MASK	((1ULL << ISM_SHIFT) - 1)

+/*
+ * Cap on the non-descending hops a classify walk may take before its
+ * filter chain is treated as misconfigured. A flowid binding that was
+ * legal at bind time can become lateral once hfsc_adjust_levels()
+ * raises a class level; a few such hops are legitimate, an unbounded
+ * run means the chain cycles.
+ */
+#define	HFSC_CLASSIFY_MAX_DRIFT	8
+
 static inline u64
 seg_x2y(u64 x, u64 sm)
 {
@@ -1133,6 +1142,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 	struct hfsc_class *head, *cl;
 	struct tcf_result res;
 	struct tcf_proto *tcf;
+	unsigned int drift;
 	int result;

 	if (TC_H_MAJ(skb->priority ^ sch->handle) == 0 &&
@@ -1142,6 +1152,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)

 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 	head = &q->root;
+	drift = HFSC_CLASSIFY_MAX_DRIFT;
 	tcf = rcu_dereference_bh(q->root.filter_list);
 	while (tcf && (result = tcf_classify_qdisc(skb, tcf, &res, false)) >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
@@ -1167,6 +1178,17 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 		if (cl->level == 0)
 			return cl; /* hit leaf class */

+		/*
+		 * flowid binds skip the level check above (res.class is set
+		 * at bind time and levels drift after), so a walk can follow
+		 * lateral hops without descending; a bounded number of them
+		 * is legal, more means the chain cycles.
+		 */
+		if (cl->level >= head->level && drift-- == 0) {
+			pr_warn_ratelimited("hfsc: classify hop budget exhausted, dropping packet\n");
+			return NULL;
+		}
+
 		/* apply inner filter chain */
 		tcf = rcu_dereference_bh(cl->filter_list);
 		head = cl;
--
2.43.0

[PATCH net v3 2/2] selftests: tc-testing: add a lateral-drift hfsc classify-walk test

[Jamal Hadi Salim]

The classify-loop fix bounds a walk's non-descending hops, so the guard
must not misfire on a legal walk that reaches its leaf through a
level-drift lateral chain. Add a case that builds exactly that chain and
asserts traffic still reaches the chain's own leaf.

A lateral hop can only exist because a bind was legal when it was made
and a later class add raised the target's level, so the setup binds each
hop while the target is still a leaf and only then deepens it: bind
1:1 -> 1:2 while 1:2 is a leaf, add 1:20 under 1:2, add 1:3 and bind
1:2 -> 1:3 while 1:3 is a leaf, then add 1:30 and 1:31 under 1:3 and
bind 1:3 -> 1:31. The walk root -> 1:1 -> 1:2 -> 1:3 -> 1:31 then takes
two lateral hops and must reach leaf 1:31.

The default class is 1:30, distinct from the asserted leaf, and the
verify pattern is anchored to the 1:31 stats line, so neither a
fall-through to the default nor a nonzero count on another class can
satisfy the check. On the patched kernel the test passes; with the bound
forced to zero the walk falls to the default and 1:31 stays idle, so the
test fails.

Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Tested-by: hybris <hybris@mojatatu.ai>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v3:
   -  Build the lateral chain by binding each hop while its target is
      still a leaf and deepening afterwards, instead of deepening first
      (Sashiko: nipa, gemini)
   -  Make the default class 1:30 distinct from the asserted leaf 1:31
      and anchor the verify pattern to the 1:31 stats line. (Sashiko:
      nipa)
   -  Drop the trailing "sleep 1" so ping's exit status is the one
      checked and no command escapes the test netns. (Sashiko: nipa)
v2:
   -  First version of the lateral-drift regression test.
---
 .../tc-testing/tc-tests/qdiscs/hfsc.json      | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hfsc.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hfsc.json
index c98c339424d4..4f6bbb8b57f9 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hfsc.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hfsc.json
@@ -169,5 +169,39 @@
         "teardown": [
             "$TC qdisc del dev $DUMMY handle 1: root"
         ]
+    },
+    {
+        "id": "8c39",
+        "name": "HFSC classify walk still reaches leaf after lateral drift",
+        "category": [
+            "qdisc",
+            "hfsc"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "ip link set lo up",
+            "$TC qdisc add dev lo handle 1: root hfsc default 30",
+            "$TC class add dev lo parent 1: classid 1:1 hfsc rt m2 100kbit",
+            "$TC class add dev lo parent 1:1 classid 1:10 hfsc rt m2 50kbit",
+            "$TC class add dev lo parent 1: classid 1:2 hfsc rt m2 100kbit",
+            "$TC filter add dev lo parent 1: protocol ip prio 1 u32 match u8 0 0 at 0 flowid 1:1",
+            "$TC filter add dev lo parent 1:1 protocol ip prio 1 u32 match u8 0 0 at 0 flowid 1:2",
+            "$TC class add dev lo parent 1:2 classid 1:20 hfsc rt m2 10kbit",
+            "$TC class add dev lo parent 1: classid 1:3 hfsc rt m2 100kbit",
+            "$TC filter add dev lo parent 1:2 protocol ip prio 1 u32 match u8 0 0 at 0 flowid 1:3",
+            "$TC class add dev lo parent 1:3 classid 1:30 hfsc rt m2 10kbit",
+            "$TC class add dev lo parent 1:3 classid 1:31 hfsc rt m2 100kbit",
+            "$TC filter add dev lo parent 1:3 protocol ip prio 1 u32 match u8 0 0 at 0 flowid 1:31"
+        ],
+        "cmdUnderTest": "ping -n -c 10 -W 1 127.0.0.1",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s class show dev lo",
+        "matchPattern": "class hfsc 1:31 parent 1:3 rt[^\\n]*\\n Sent [0-9]+ bytes [1-9][0-9]* pkt",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev lo handle 1: root"
+        ]
     }
 ]
-- 
2.43.0

Re: [PATCH net v3 2/2] selftests: tc-testing: add a lateral-drift hfsc classify-walk test

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · Low: 1

- [Medium] The commit message of "selftests: tc-testing: add a lateral-
  drift hfsc classify-walk test" misdescribes the kernel behaviour it…
- [Medium] The hop guard this test is written to validate
  (HFSC_CLASSIFY_MAX_DRIFT in hfsc_classify(), added by the immediately…
- [Low] The new setup step invokes a bare `ip` binary (`"ip link set lo
  up"`) instead of the `$IP` keyword that tdc_config.py exists to…

--

selftests: tc-testing: add a lateral-drift hfsc classify-walk test

This adds tdc case 8c39 to tools/testing/selftests/tc-testing/tc-tests/
qdiscs/hfsc.json.  The setup builds a two-hop lateral chain by binding
each filter while its target is still a leaf and only then adding
children to deepen it, then pings 127.0.0.1 and checks that class 1:31
counted packets.

No Link: tags present in the commit header.

Two notes are about the changelog and about the coverage the case
provides, and one is about the harness keyword used in setup.

> The default class is 1:30, distinct from the asserted leaf, and the
> verify pattern is anchored to the 1:31 stats line, so neither a
> fall-through to the default nor a nonzero count on another class can
> satisfy the check. On the patched kernel the test passes; with the bound
> forced to zero the walk falls to the default and 1:31 stays idle, so the
> test fails.

[Severity: Medium]
Is "the walk falls to the default" the actual behaviour when the bound is
exhausted?  In hfsc_classify() the budget arm returns NULL rather than
breaking out of the loop:

net/sched/sch_hfsc.c:hfsc_classify() {
	...
	if (cl->level >= head->level && drift-- == 0) {
		pr_warn_ratelimited("hfsc: classify hop budget exhausted, dropping packet\n");
		return NULL;
	}
	...
	/* classification failed, try default class */
	cl = hfsc_find_class(TC_H_MAKE(TC_H_MAJ(sch->handle),
				       READ_ONCE(q->defcls)), sch);
}

The default-class lookup sits after the while loop, so it is only reached
from the two break paths (invalid classid, or a non-descending hop with no
bound class), not from the drift branch.

hfsc_enqueue() then turns that NULL into a drop:

net/sched/sch_hfsc.c:hfsc_enqueue() {
	cl = hfsc_classify(skb, sch, &err);
	if (cl == NULL) {
		if (err & __NET_XMIT_BYPASS)
			qdisc_qstats_drop(sch);
		__qdisc_drop(skb, to_free);
	...
}

So with HFSC_CLASSIFY_MAX_DRIFT forced to 0 the first lateral hop
(head = 1:1 level 1, cl = 1:2 level 1) discards every ICMP packet and
ping -n -c 10 -W 1 127.0.0.1 exits nonzero.

tdc.py compares exit_code != int(tidx["expExitCode"]) in test_runner() and
marks the case failed before running verifyCmd, so the negative control
fails on the ping exit status and the 1:31 stats pattern described as the
discriminator never gets evaluated.

Could the changelog describe the exhaustion path as a packet drop instead
of a fall-through to the default class?

> diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hfsc.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hfsc.json
> index c98c339424d4e..4f6bbb8b57f93 100644
> --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hfsc.json
> +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hfsc.json
> @@ -169,5 +169,39 @@
>          "teardown": [
>              "$TC qdisc del dev $DUMMY handle 1: root"
>          ]
> +    },
> +    {
> +        "id": "8c39",
> +        "name": "HFSC classify walk still reaches leaf after lateral drift",
> +        "category": [
> +            "qdisc",
> +            "hfsc"
> +        ],
> +        "plugins": {
> +            "requires": "nsPlugin"
> +        },
> +        "setup": [
> +            "ip link set lo up",

[Severity: Low]
This isn't a bug, but should this use the $IP keyword like the rest of the
suite?  replace_keywords() in tdc.py only substitutes $-prefixed names from
NAMES, and exec_cmd() runs commands with:

	proc = subprocess.Popen(command,
		shell=True,
		stdout=subprocess.PIPE,
		stderr=subprocess.PIPE,
		env=ENVIR)

with ENVIR = { } in tdc_config.py, so a bare ip resolves through the
shell's built-in default PATH rather than NAMES['IP'].  A tdc_config_local.py
that points 'IP' at a locally built iproute2 would not affect this step.
This appears to be the only bare ip invocation under
tools/testing/selftests/tc-testing/tc-tests.

> +            "$TC qdisc add dev lo handle 1: root hfsc default 30",
> +            "$TC class add dev lo parent 1: classid 1:1 hfsc rt m2 100kbit",
> +            "$TC class add dev lo parent 1:1 classid 1:10 hfsc rt m2 50kbit",
> +            "$TC class add dev lo parent 1: classid 1:2 hfsc rt m2 100kbit",
> +            "$TC filter add dev lo parent 1: protocol ip prio 1 u32 match u8 0 0 at 0 flowid 1:1",
> +            "$TC filter add dev lo parent 1:1 protocol ip prio 1 u32 match u8 0 0 at 0 flowid 1:2",
> +            "$TC class add dev lo parent 1:2 classid 1:20 hfsc rt m2 10kbit",

[Severity: Medium]
The bind-then-deepen construction used here is repeatable, so should the
case also cover the point where the guard starts dropping legal traffic?

hfsc_bind_tcf() only rejects a bind when the target is not deeper:

net/sched/sch_hfsc.c:hfsc_bind_tcf() {
	if (cl != NULL) {
		if (p != NULL && p->level <= cl->level)
			return 0;
	...
}

so binding 1:k -> 1:(k+1) succeeds while 1:(k+1) is still a leaf, and the
later class add raises that level through hfsc_adjust_levels() with no
revalidation of the existing bind and no error to the administrator:

net/sched/sch_hfsc.c:hfsc_adjust_levels() {
	do {
		level = 0;
		list_for_each_entry(p, &cl->children, siblings) {
			if (p->level >= level)
				level = p->level + 1;
		}
		WRITE_ONCE(cl->level, level);
	} while ((cl = cl->cl_parent) != NULL);
}

Repeating the same two setup steps for ten sibling interior classes gives
nine lateral hops of level 1 -> level 1.  Hops one through eight consume
HFSC_CLASSIFY_MAX_DRIFT and hop nine hits:

	if (cl->level >= head->level && drift-- == 0) {
		pr_warn_ratelimited("hfsc: classify hop budget exhausted, dropping packet\n");
		return NULL;
	}

which hfsc_enqueue() turns into qdisc_qstats_drop(sch) plus
__qdisc_drop(skb, to_free), i.e. all traffic on that chain is discarded
with only a rate-limited warning, on a configuration where every tc
command succeeded and which classified to its leaf before the guard.

That chain is finite and cycle-free, which does not match the comment
above HFSC_CLASSIFY_MAX_DRIFT:

	 * legal at bind time can become lateral once hfsc_adjust_levels()
	 * raises a class level; a few such hops are legitimate, an unbounded
	 * run means the chain cycles.

The new case builds two lateral hops out of the budget of eight, so it
does not exercise the boundary.  Would it be worth extending the case past
eight hops, or making the exhaustion arm break to the default class the way
the neighbouring misconfiguration exits do?

> +            "$TC class add dev lo parent 1: classid 1:3 hfsc rt m2 100kbit",
> +            "$TC filter add dev lo parent 1:2 protocol ip prio 1 u32 match u8 0 0 at 0 flowid 1:3",
> +            "$TC class add dev lo parent 1:3 classid 1:30 hfsc rt m2 10kbit",
> +            "$TC class add dev lo parent 1:3 classid 1:31 hfsc rt m2 100kbit",
> +            "$TC filter add dev lo parent 1:3 protocol ip prio 1 u32 match u8 0 0 at 0 flowid 1:31"
> +        ],
> +        "cmdUnderTest": "ping -n -c 10 -W 1 127.0.0.1",
> +        "expExitCode": "0",
> +        "verifyCmd": "$TC -s class show dev lo",
> +        "matchPattern": "class hfsc 1:31 parent 1:3 rt[^\\n]*\\n Sent [0-9]+ bytes [1-9][0-9]* pkt",
> +        "matchCount": "1",
> +        "teardown": [
> +            "$TC qdisc del dev lo handle 1: root"
> +        ]
>      }
>  ]

[ ... ]

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

Re: [PATCH net v3 1/2] net/sched: sch_hfsc: bound the classify inner-filter walk with a drift budget

[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

- [Medium] net/sched/sch_hfsc.c: the new drift budget in hfsc_classify()
  conflates "budget exhausted" with "the chain cycles", and the two new…

--

net/sched: sch_hfsc: bound the classify inner-filter walk with a drift budget

The patch adds a budget of non-descending hops to the hfsc_classify()
inner-filter walk so that a filter chain made cyclic by post-bind level
drift can no longer spin with the qdisc lock held.  Once the budget is
exhausted the packet is dropped with a rate-limited warning.

Link: https://sashiko.dev/#/patchset/QDISC-CTUU.v2.20260913192614@mojatatu.com
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-CTUU.v2.20260913192614%40mojatatu.com

> diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
> index e87f5021a1995..284490fd6ca91 100644
> --- a/net/sched/sch_hfsc.c
> +++ b/net/sched/sch_hfsc.c
> @@ -386,6 +386,15 @@ cftree_update(struct hfsc_class *cl)
>  #define	SM_MASK		((1ULL << SM_SHIFT) - 1)
>  #define	ISM_MASK	((1ULL << ISM_SHIFT) - 1)
>  
> +/*
> + * Cap on the non-descending hops a classify walk may take before its
> + * filter chain is treated as misconfigured. A flowid binding that was
> + * legal at bind time can become lateral once hfsc_adjust_levels()
> + * raises a class level; a few such hops are legitimate, an unbounded
> + * run means the chain cycles.
> + */
> +#define	HFSC_CLASSIFY_MAX_DRIFT	8
> +
>  static inline u64
>  seg_x2y(u64 x, u64 sm)
>  {
> @@ -1133,6 +1142,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
>  	struct hfsc_class *head, *cl;
>  	struct tcf_result res;
>  	struct tcf_proto *tcf;
> +	unsigned int drift;
>  	int result;
>  
>  	if (TC_H_MAJ(skb->priority ^ sch->handle) == 0 &&
> @@ -1142,6 +1152,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
>  
>  	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>  	head = &q->root;
> +	drift = HFSC_CLASSIFY_MAX_DRIFT;
>  	tcf = rcu_dereference_bh(q->root.filter_list);
>  	while (tcf && (result = tcf_classify_qdisc(skb, tcf, &res, false)) >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
> @@ -1167,6 +1178,17 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
>  		if (cl->level == 0)
>  			return cl; /* hit leaf class */
>  
> +		/*
> +		 * flowid binds skip the level check above (res.class is set
> +		 * at bind time and levels drift after), so a walk can follow
> +		 * lateral hops without descending; a bounded number of them
> +		 * is legal, more means the chain cycles.
> +		 */
> +		if (cl->level >= head->level && drift-- == 0) {
> +			pr_warn_ratelimited("hfsc: classify hop budget exhausted, dropping packet\n");
> +			return NULL;
> +		}
> +

[Severity: Medium]

The walk does terminate: any cycle over head classes has to contain at
least one non-descending hop per lap, descending hops strictly lower
head->level, and the budget never comes back.  A couple of questions
about the wording and about the drop, though.

Is drift really a "run" of non-descending hops?  It is initialised once
before the loop:

	drift = HFSC_CLASSIFY_MAX_DRIFT;

and is never reset, so it is a cumulative total for the whole walk.  The
macro comment says "an unbounded run means the chain cycles" and the
inline comment says "a bounded number of them is legal, more means the
chain cycles", neither of which mentions that the counter is cumulative
and must not be reset.

The v3 notes say the comments were reworded to "describe the cumulative
non-descending hop budget that is implemented, and explain why it is
deliberately not reset on a descending hop", but that description only
appears in the changelog.  Since resetting the budget on a descent would
re-open the loop (a cycle can hide one lateral hop per lap behind
arbitrarily many descents), should the in-tree comments spell the
never-reset property out?

Can this also drop packets for an acyclic chain that would have reached a
leaf?  hfsc_bind_tcf() validates a bind only against the levels present
at bind time:

net/sched/sch_hfsc.c:hfsc_bind_tcf() {
	if (cl != NULL) {
		if (p != NULL && p->level <= cl->level)
			return 0;
		qdisc_class_get(&cl->cl_common);
	}
}

So each bind can be created while its target is still a leaf, and adding
a child under that target afterwards raises its level in
hfsc_adjust_levels() without revalidating the bind.  Chaining ten sibling
classes that way (C0 bound to C1, C1 bound to C2, ... C8 bound to C9,
each bind made before the target gained a child) gives eight lateral hops
that consume the budget, and the ninth hop C8->C9 satisfies

	if (cl->level >= head->level && drift-- == 0)

so hfsc_classify() returns NULL even though C9's filter points at a leaf
and the walk contains no cycle.  hfsc_enqueue() then drops the packet:

net/sched/sch_hfsc.c:hfsc_enqueue() {
	cl = hfsc_classify(skb, sch, &err);
	if (cl == NULL) {
		if (err & __NET_XMIT_BYPASS)
			qdisc_qstats_drop(sch);
		__qdisc_drop(skb, to_free);
		return err;
	}
}

Is the asymmetry with the neighbouring path for the same class of
misconfiguration intended?  The classid case breaks out and falls through
to the default class:

			if (cl->level >= head->level)
				break; /* filter may only point downwards */

while budget exhaustion returns NULL and bypasses the default-class
fallback.  Would break be the better match here too?

Related to that, where does 8 come from?  HTB derives its bound from
TC_HTB_MAXDEPTH via ++hops > TC_HTB_MAXDEPTH, but HFSC has no comparable
structural limit (hfsc_change_class() bounds neither chain length nor
class count), so a chain of more than eight lateral hops is not
necessarily cyclic.  Could the warning say something like "non-descending
hop budget exhausted" instead of attributing the case to a cycle?

>  		/* apply inner filter chain */
>  		tcf = rcu_dereference_bh(cl->filter_list);
>  		head = cl;

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

Re: [PATCH net v3 1/2] net/sched: sch_hfsc: bound the classify inner-filter walk with a drift budget

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 17 Sep 2026 06:57:32 -0400 you wrote:
> hfsc_classify() applies the "filter may only point downwards" level check
> only when the filter result carries no bound class. A filter created with
> a flowid gets res.class set once at bind time, so the check never runs for
> it during classification. hfsc_adjust_levels() can later raise a class's
> level without revalidating existing bindings, leaving two binds that were
> each legal at bind time pointing at each other; the classify walk then
> bounces between two interior classes forever with the qdisc lock held and
> BH disabled — a soft lockup from a single packet. The stuck walk trips
> the watchdog:
> 
> [...]

Here is the summary with links:
  - [net,v3,1/2] net/sched: sch_hfsc: bound the classify inner-filter walk with a drift budget
    https://git.kernel.org/netdev/net/c/8a60ade2277e
  - [net,v3,2/2] selftests: tc-testing: add a lateral-drift hfsc classify-walk test
    https://git.kernel.org/netdev/net/c/1e24c4f2ee44

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html