[PATCH net 1/2] net/sched: Abort __tc_modify_qdisc if parent is a clsact/ingress qdisc

[PATCH net 1/2] net/sched: Abort __tc_modify_qdisc if parent is a clsact/ingress qdisc

[Victor Nogueira]

Wang reported an illegal configuration [1] where the user attempts to add a
child qdisc to the ingress qdisc as follows:

tc qdisc add dev eth0 handle ffff:0 ingress
tc qdisc add dev eth0 handle ffe0:0 parent ffff:a fq

To solve this, we reject any configuration attempt to add a child qdisc to
ingress or clsact.

[1] https://lore.kernel.org/netdev/20251105022213.1981982-1-wangliang74@huawei.com/

Fixes: 5e50da01d0ce ("[NET_SCHED]: Fix endless loops (part 2): "simple" qdiscs")
Reported-by: Wang Liang <wangliang74@huawei.com>
Closes: https://lore.kernel.org/netdev/20251105022213.1981982-1-wangliang74@huawei.com/
Reviewed-by: Pedro Tammela <pctammela@mojatatu.ai>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 net/sched/sch_api.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 1e058b46d3e1..f56b18c8aebf 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1599,6 +1599,11 @@ static int __tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
 					NL_SET_ERR_MSG(extack, "Failed to find specified qdisc");
 					return -ENOENT;
 				}
+				if (p->flags & TCQ_F_INGRESS) {
+					NL_SET_ERR_MSG(extack,
+						       "Cannot add children to ingress/clsact qdisc");
+					return -EOPNOTSUPP;
+				}
 				q = qdisc_leaf(p, clid, extack);
 				if (IS_ERR(q))
 					return PTR_ERR(q);
-- 
2.51.0

[PATCH net 2/2] selftests/tc-testing: Create tests trying to add children to clsact/ingress qdiscs

[Victor Nogueira]

In response to Wang's bug report [1], add the following test cases:

- Try and fail to add an fq child to an ingress qdisc
- Try and fail to add an fq child to a clsact qdisc

[1] https://lore.kernel.org/netdev/20251105022213.1981982-1-wangliang74@huawei.com/

Reviewed-by: Pedro Tammela <pctammela@mojatatu.ai>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 .../tc-testing/tc-tests/infra/qdiscs.json     | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json b/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
index 998e5a2f4579..0091bcd91c2c 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
@@ -961,5 +961,49 @@
         "teardown": [
             "$TC qdisc del dev $DUMMY root"
         ]
+    },
+    {
+        "id": "4989",
+        "name": "Try to add an fq child to an ingress qdisc",
+        "category": [
+            "qdisc",
+            "ingress"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY handle ffff:0 ingress"
+        ],
+        "cmdUnderTest": "$TC qdisc add dev $DUMMY parent ffff:0 handle ffe0:0 fq",
+        "expExitCode": "2",
+        "verifyCmd": "$TC -j qdisc ls dev $DUMMY handle ffe0:",
+        "matchJSON": [],
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY ingress"
+        ]
+    },
+    {
+        "id": "c2b0",
+        "name": "Try to add an fq child to a clsact qdisc",
+        "category": [
+            "qdisc",
+            "ingress"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY handle ffff:0 clsact"
+        ],
+        "cmdUnderTest": "$TC qdisc add dev $DUMMY parent ffff:0 handle ffe0:0 fq",
+        "expExitCode": "2",
+        "verifyCmd": "$TC -j qdisc ls dev $DUMMY handle ffe0:",
+        "matchJSON": [],
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY clsact"
+        ]
     }
 ]
-- 
2.51.0

Re: [PATCH net 1/2] net/sched: Abort __tc_modify_qdisc if parent is a clsact/ingress qdisc

[Cong Wang]

On Thu, Nov 06, 2025 at 05:56:20PM -0300, Victor Nogueira wrote:
> Wang reported an illegal configuration [1] where the user attempts to add a
> child qdisc to the ingress qdisc as follows:
> 
> tc qdisc add dev eth0 handle ffff:0 ingress
> tc qdisc add dev eth0 handle ffe0:0 parent ffff:a fq
> 
> To solve this, we reject any configuration attempt to add a child qdisc to
> ingress or clsact.
> 
> [1] https://lore.kernel.org/netdev/20251105022213.1981982-1-wangliang74@huawei.com/
> 
> Fixes: 5e50da01d0ce ("[NET_SCHED]: Fix endless loops (part 2): "simple" qdiscs")
> Reported-by: Wang Liang <wangliang74@huawei.com>
> Closes: https://lore.kernel.org/netdev/20251105022213.1981982-1-wangliang74@huawei.com/
> Reviewed-by: Pedro Tammela <pctammela@mojatatu.ai>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>

Reviewed-by: Cong Wang <cwang@multikernel.io>

Thanks!

Re: [PATCH net 2/2] selftests/tc-testing: Create tests trying to add children to clsact/ingress qdiscs

[Cong Wang]

On Thu, Nov 06, 2025 at 05:56:21PM -0300, Victor Nogueira wrote:
> In response to Wang's bug report [1], add the following test cases:
> 
> - Try and fail to add an fq child to an ingress qdisc
> - Try and fail to add an fq child to a clsact qdisc
> 
> [1] https://lore.kernel.org/netdev/20251105022213.1981982-1-wangliang74@huawei.com/
> 
> Reviewed-by: Pedro Tammela <pctammela@mojatatu.ai>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>

Reviewed-by: Cong Wang <cwang@multikernel.io>

Thanks

Re: [PATCH net 1/2] net/sched: Abort __tc_modify_qdisc if parent is a clsact/ingress qdisc

[patchwork-bot+netdevbpf]

Hello:

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

On Thu,  6 Nov 2025 17:56:20 -0300 you wrote:
> Wang reported an illegal configuration [1] where the user attempts to add a
> child qdisc to the ingress qdisc as follows:
> 
> tc qdisc add dev eth0 handle ffff:0 ingress
> tc qdisc add dev eth0 handle ffe0:0 parent ffff:a fq
> 
> To solve this, we reject any configuration attempt to add a child qdisc to
> ingress or clsact.
> 
> [...]

Here is the summary with links:
  - [net,1/2] net/sched: Abort __tc_modify_qdisc if parent is a clsact/ingress qdisc
    https://git.kernel.org/netdev/net/c/e781122d76f0
  - [net,2/2] selftests/tc-testing: Create tests trying to add children to clsact/ingress qdiscs
    https://git.kernel.org/netdev/net/c/60260ad93586

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