* [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem
@ 2026-04-26 19:09 Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 1/9] net: Introduce skb tc depth field to track packet loops Jamal Hadi Salim
` (8 more replies)
0 siblings, 9 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim
This patchset adds a 2-bit per-skb tc_depth counter that travels with
the packet to help fixes packet loops caused by mirred ingress
redirects and netem duplication in stacked qdisc trees.
The existing per-CPU mirred nest tracking loses state when a packet is
deferred through the backlog or moves between CPUs via XPS/RPS.
A per-skb field covers both cases.
Patch 1 adds the tc_depth field in a padding hole in sk_buff.
Patches 2-3 revert the check_netem_in_tree() fix and its tests,
which broke legitimate multi-netem configurations.
Patch 4 uses tc_depth to stop netem duplicate recursion.
Patch 5 uses tc_depth to catch mirred ingress redirect loops.
Patch 6 fixes the infinite loop in the mirred egress blockcast case.
Patch 7 fixes an skb leak in early return error scenarios in tcf_mirred_act
for redirect (caught by Sashiko [1]).
Patches 8-9 add mirred and netem test cases.
Changes in v4:
- Provide better context in patch 1 commit to appease Sashiko
We'll see if it will stop yapping about corner cases! I believe
I am making history being the first human to talk directly to Sashiko
in a patch submission;->
- Fix infinite loop for the mirred egress blockcast case (patch 6)
- Fix skb leak in tcf_mirred_act caught by Sashiko (patch 7)
Changes in v3:
- Renamed skb->ttl to skb->tc_depth to avoid confusion with IP TTL
- Expanded commit messages
- Split mirred and netem test cases into separate patches
- No code changes from v2
Changes in v2:
- Do not reuse skb->from_ingress (which was moved to skb->cb)
[1] https://sashiko.dev/#/patchset/20260413082027.2244884-1-hxzene%40gmail.com
Jamal Hadi Salim (5):
net: Introduce skb tc depth field to track packet loops
net/sched: Revert "net/sched: Restrict conditions for adding
duplicating netems to qdisc tree"
Revert "selftests/tc-testing: Add tests for restrictions on netem
duplication"
net/sched: fix packet loop on netem when duplicate is on
net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop
Kito Xu (veritas501) (1):
net/sched: act_mirred: Fix blockcast recursion bypass leading to stack
overflow
Victor Nogueira (3):
net/sched: act_mirred: Fix skb leak in early mirred redirect returns
selftests/tc-testing: Add mirred test cases exercising loops
selftests/tc-testing: Add netem test case exercising loops
include/linux/skbuff.h | 2 +
net/sched/act_mirred.c | 67 +-
net/sched/sch_netem.c | 47 +-
.../tc-testing/tc-tests/actions/mirred.json | 616 +++++++++++++++++-
.../tc-testing/tc-tests/infra/qdiscs.json | 5 +-
.../tc-testing/tc-tests/qdiscs/netem.json | 96 +--
6 files changed, 691 insertions(+), 142 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net 1/9] net: Introduce skb tc depth field to track packet loops
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 2/9] net/sched: Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" Jamal Hadi Salim
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim
Add a 2-bit per-skb tc depth field to track packet loops across the stack.
The previous per-CPU loop counters like MIRRED_NEST_LIMIT
assume a single call stack and lose state in two cases:
1) When a packet is queued and reprocessed later (e.g., egress->ingress
via backlog), the per-cpu state is gone by the time it is dequeued.
2) With XPS/RPS a packet may arrive on one CPU and be processed on
another.
A per-skb field solves both by travelling with the packet itself.
The field fits in existing padding, using 2 bits that were previously a
hole:
pahole before(-) and after (+) diff looks like:
__u8 slow_gro:1; /* 132: 3 1 */
__u8 csum_not_inet:1; /* 132: 4 1 */
__u8 unreadable:1; /* 132: 5 1 */
+ __u8 tc_depth:2; /* 132: 6 1 */
- /* XXX 2 bits hole, try to pack */
/* XXX 1 byte hole, try to pack */
__u16 tc_index; /* 134 2 */
There used to be a ttl field which was removed as part of tc_verd in commit
aec745e2c520 ("net-tc: remove unused tc_verd fields"). It was already
unused by that time, due to remove earlier in commit c19ae86a510c
("tc: remove unused redirect ttl").
The first user of this field is netem, which increments tc_depth on
duplicated packets before re-enqueueing them at the root qdisc. On
re-entry, netem skips duplication for any skb with tc_depth already set,
bounding recursion to a single level regardless of tree topology.
The other user is mirred which increments it on each pass
and limits to depth to MIRRED_DEFER_LIMIT (3).
The new field was called ttl in earlier versions of this patch
but renamed to tc_depth to avoid confusion with IP ttl.
Note (looking at you Sashiko!):
1. Since both mirred and netem utilize the same 2-bit tc_depth field it is
possible when netem and mirred are used together that netem qdisc to skip
the duplication step. This is a known trade-off, as a 2-bit field cannot
independently track both features' recursion depths and it is not considered
sane to have a setup that addresses both features on at the same time.
2. skb_scrub_packet does not clear tc_depth. This means a packet's loop history
is preserved even across namespaces. While this might be restrictive for
some topologies, it is also design intent to provide robustness against loops
across namespaces.
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
include/linux/skbuff.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2bcf78a4de7b..3f06254ab1b7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -821,6 +821,7 @@ enum skb_tstamp_type {
* @_sk_redir: socket redirection information for skmsg
* @_nfct: Associated connection, if any (with nfctinfo bits)
* @skb_iif: ifindex of device we arrived on
+ * @tc_depth: counter for packet duplication
* @tc_index: Traffic control index
* @hash: the packet hash
* @queue_mapping: Queue mapping for multiqueue devices
@@ -1030,6 +1031,7 @@ struct sk_buff {
__u8 csum_not_inet:1;
#endif
__u8 unreadable:1;
+ __u8 tc_depth:2;
#if defined(CONFIG_NET_SCHED) || defined(CONFIG_NET_XGRESS)
__u16 tc_index; /* traffic control index */
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 2/9] net/sched: Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree"
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 1/9] net: Introduce skb tc depth field to track packet loops Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 3/9] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" Jamal Hadi Salim
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim, Ji-Soo Chung, Gerlinde, zyc zyc,
Manas Ghandat
This reverts commit ec8e0e3d7adef940cdf9475e2352c0680189d14e.
The original patch rejects any tree containing two netems when
either has duplication set, even when they sit on unrelated classes
of the same classful parent. That broke configurations that have
worked since netem was introduced.
The re-entrancy problem the original commit was trying to solve is
handled by later patch using tc_depth flag.
Doing this revert will (re)expose the original bug with multiple
netem duplication. When this patch is backported make sure
and get the full series.
Fixes: ec8e0e3d7ade ("net/sched: Restrict conditions for adding duplicating netems to qdisc tree")
Reported-by: Ji-Soo Chung <jschung2@proton.me>
Reported-by: Gerlinde <lrGerlinde@mailfence.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220774
Reported-by: zyc zyc <zyc199902@zohomail.cn>
Closes: https://lore.kernel.org/all/19adda5a1e2.12410b78222774.9191120410578703463@zohomail.cn/
Reported-by: Manas Ghandat <ghandatmanas@gmail.com>
Closes: https://lore.kernel.org/netdev/f69b2c8f-8325-4c2e-a011-6dbc089f30e4@gmail.com/
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/sch_netem.c | 40 ----------------------------------------
1 file changed, 40 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 20df1c08b1e9..8c6dea196089 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -975,41 +975,6 @@ static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla,
return 0;
}
-static const struct Qdisc_class_ops netem_class_ops;
-
-static int check_netem_in_tree(struct Qdisc *sch, bool duplicates,
- struct netlink_ext_ack *extack)
-{
- struct Qdisc *root, *q;
- unsigned int i;
-
- root = qdisc_root_sleeping(sch);
-
- if (sch != root && root->ops->cl_ops == &netem_class_ops) {
- if (duplicates ||
- ((struct netem_sched_data *)qdisc_priv(root))->duplicate)
- goto err;
- }
-
- if (!qdisc_dev(root))
- return 0;
-
- hash_for_each(qdisc_dev(root)->qdisc_hash, i, q, hash) {
- if (sch != q && q->ops->cl_ops == &netem_class_ops) {
- if (duplicates ||
- ((struct netem_sched_data *)qdisc_priv(q))->duplicate)
- goto err;
- }
- }
-
- return 0;
-
-err:
- NL_SET_ERR_MSG(extack,
- "netem: cannot mix duplicating netems with other netems in tree");
- return -EINVAL;
-}
-
/* Parse netlink message to set options */
static int netem_change(struct Qdisc *sch, struct nlattr *opt,
struct netlink_ext_ack *extack)
@@ -1068,11 +1033,6 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
q->gap = qopt->gap;
q->counter = 0;
q->loss = qopt->loss;
-
- ret = check_netem_in_tree(sch, qopt->duplicate, extack);
- if (ret)
- goto unlock;
-
q->duplicate = qopt->duplicate;
/* for compatibility with earlier versions.
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 3/9] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication"
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 1/9] net: Introduce skb tc depth field to track packet loops Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 2/9] net/sched: Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 4/9] net/sched: fix packet loop on netem when duplicate is on Jamal Hadi Salim
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim
This reverts commit ecdec65ec78d67d3ebd17edc88b88312054abe0d.
The tests added were related to check_netem_in_tree() which was
just reverted in the previous patch.
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
.../tc-testing/tc-tests/infra/qdiscs.json | 5 +-
.../tc-testing/tc-tests/qdiscs/netem.json | 81 -------------------
2 files changed, 3 insertions(+), 83 deletions(-)
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 eefadd0546d3..d32616a46a71 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
@@ -702,6 +702,7 @@
"$TC qdisc add dev $DUMMY parent 1:1 handle 2:0 netem duplicate 100%",
"$TC filter add dev $DUMMY parent 1:0 protocol ip prio 1 u32 match ip dst 10.10.10.1/32 flowid 1:1",
"$TC class add dev $DUMMY parent 1:0 classid 1:2 hfsc ls m2 10Mbit",
+ "$TC qdisc add dev $DUMMY parent 1:2 handle 3:0 netem duplicate 100%",
"$TC filter add dev $DUMMY parent 1:0 protocol ip prio 2 u32 match ip dst 10.10.10.2/32 flowid 1:2",
"ping -c 1 10.10.10.1 -I$DUMMY > /dev/null || true",
"$TC filter del dev $DUMMY parent 1:0 protocol ip prio 1",
@@ -714,8 +715,8 @@
{
"kind": "hfsc",
"handle": "1:",
- "bytes": 294,
- "packets": 3
+ "bytes": 392,
+ "packets": 4
}
],
"matchCount": "1",
diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/netem.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/netem.json
index 718d2df2aafa..3c4444961488 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/netem.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/netem.json
@@ -336,86 +336,5 @@
"teardown": [
"$TC qdisc del dev $DUMMY handle 1: root"
]
- },
- {
- "id": "d34d",
- "name": "NETEM test qdisc duplication restriction in qdisc tree in netem_change root",
- "category": ["qdisc", "netem"],
- "plugins": {
- "requires": "nsPlugin"
- },
- "setup": [
- "$TC qdisc add dev $DUMMY root handle 1: netem limit 1",
- "$TC qdisc add dev $DUMMY parent 1: handle 2: netem limit 1"
- ],
- "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: netem duplicate 50%",
- "expExitCode": "2",
- "verifyCmd": "$TC -s qdisc show dev $DUMMY",
- "matchPattern": "qdisc netem",
- "matchCount": "2",
- "teardown": [
- "$TC qdisc del dev $DUMMY handle 1:0 root"
- ]
- },
- {
- "id": "b33f",
- "name": "NETEM test qdisc duplication restriction in qdisc tree in netem_change non-root",
- "category": ["qdisc", "netem"],
- "plugins": {
- "requires": "nsPlugin"
- },
- "setup": [
- "$TC qdisc add dev $DUMMY root handle 1: netem limit 1",
- "$TC qdisc add dev $DUMMY parent 1: handle 2: netem limit 1"
- ],
- "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 2: netem duplicate 50%",
- "expExitCode": "2",
- "verifyCmd": "$TC -s qdisc show dev $DUMMY",
- "matchPattern": "qdisc netem",
- "matchCount": "2",
- "teardown": [
- "$TC qdisc del dev $DUMMY handle 1:0 root"
- ]
- },
- {
- "id": "cafe",
- "name": "NETEM test qdisc duplication restriction in qdisc tree",
- "category": ["qdisc", "netem"],
- "plugins": {
- "requires": "nsPlugin"
- },
- "setup": [
- "$TC qdisc add dev $DUMMY root handle 1: netem limit 1 duplicate 100%"
- ],
- "cmdUnderTest": "$TC qdisc add dev $DUMMY parent 1: handle 2: netem duplicate 100%",
- "expExitCode": "2",
- "verifyCmd": "$TC -s qdisc show dev $DUMMY",
- "matchPattern": "qdisc netem",
- "matchCount": "1",
- "teardown": [
- "$TC qdisc del dev $DUMMY handle 1:0 root"
- ]
- },
- {
- "id": "1337",
- "name": "NETEM test qdisc duplication restriction in qdisc tree across branches",
- "category": ["qdisc", "netem"],
- "plugins": {
- "requires": "nsPlugin"
- },
- "setup": [
- "$TC qdisc add dev $DUMMY parent root handle 1:0 hfsc",
- "$TC class add dev $DUMMY parent 1:0 classid 1:1 hfsc rt m2 10Mbit",
- "$TC qdisc add dev $DUMMY parent 1:1 handle 2:0 netem",
- "$TC class add dev $DUMMY parent 1:0 classid 1:2 hfsc rt m2 10Mbit"
- ],
- "cmdUnderTest": "$TC qdisc add dev $DUMMY parent 1:2 handle 3:0 netem duplicate 100%",
- "expExitCode": "2",
- "verifyCmd": "$TC -s qdisc show dev $DUMMY",
- "matchPattern": "qdisc netem",
- "matchCount": "1",
- "teardown": [
- "$TC qdisc del dev $DUMMY handle 1:0 root"
- ]
}
]
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 4/9] net/sched: fix packet loop on netem when duplicate is on
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
` (2 preceding siblings ...)
2026-04-26 19:09 ` [PATCH net 3/9] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
2026-04-26 19:43 ` William Liu
2026-04-26 19:09 ` [PATCH net 5/9] net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop Jamal Hadi Salim
` (4 subsequent siblings)
8 siblings, 1 reply; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim
When netem duplicates a packet it re-enqueues the copy at the root qdisc.
If another netem sits in the tree the copy can be duplicated
again, recursing until the stack or memory is exhausted.
The original duplication guard temporarily zeroed q->duplicate around
the re-enqueue, but that does not cover all cases because it is
per-qdisc state shared across all concurrent enqueue paths
and is not safe without additional locking.
Use the skb tc_depth field introduced in an earlier patch:
- increment it on the duplicate before re-enqueue
- skip duplication for any skb whose tc_depth is already non-zero.
This marks the packet itself rather than mutating qdisc state,
therefore it is safe regardless of tree topology or concurrency.
Fixes: 0afb51e72855 ("[PKT_SCHED]: netem: reinsert for duplication")
Reported-by: William Liu <will@willsroot.io>
Reported-by: Savino Dicanosa <savy@syst3mfailure.io>
Closes: https://lore.kernel.org/netdev/8DuRWwfqjoRDLDmBMlIfbrsZg9Gx50DHJc1ilxsEBNe2D6NMoigR_eIRIG0LOjMc3r10nUUZtArXx4oZBIdUfZQrwjcQhdinnMis_0G7VEk=@willsroot.io/
Co-developed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/sch_netem.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 8c6dea196089..d595eb03dca8 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -461,7 +461,8 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
skb->prev = NULL;
/* Random duplication */
- if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor, &q->prng))
+ if (q->duplicate && skb->tc_depth == 0 &&
+ q->duplicate >= get_crandom(&q->dup_cor, &q->prng))
++count;
/* Drop packet? */
@@ -540,11 +541,9 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
*/
if (skb2) {
struct Qdisc *rootq = qdisc_root_bh(sch);
- u32 dupsave = q->duplicate; /* prevent duplicating a dup... */
- q->duplicate = 0;
+ skb2->tc_depth++; /* prevent duplicating a dup... */
rootq->enqueue(skb2, rootq, to_free);
- q->duplicate = dupsave;
skb2 = NULL;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 5/9] net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
` (3 preceding siblings ...)
2026-04-26 19:09 ` [PATCH net 4/9] net/sched: fix packet loop on netem when duplicate is on Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 6/9] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow Jamal Hadi Salim
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim
When mirred redirects to ingress (from either ingress or egress) the loop
state from sched_mirred_dev array dev is lost because of 1) the packet
deferral into the backlog and 2) the fact the sched_mirred_dev array is
cleared. In such cases, if there was a loop we won't discover it.
Here's a simple test to reproduce:
ip a add dev port0 10.10.10.11/24
tc qdisc add dev port0 clsact
tc filter add dev port0 egress protocol ip \
prio 10 matchall action mirred ingress redirect dev port1
tc qdisc add dev port1 clsact
tc filter add dev port1 ingress protocol ip \
prio 10 matchall action mirred egress redirect dev port0
ping -c 1 -W0.01 10.10.10.10
Fixes: fe946a751d9b ("net/sched: act_mirred: add loop detection")
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/act_mirred.c | 47 +++++++++++++++++++++++++++---------------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 2c5a7a321a94..dd5e7ea7ef26 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -26,6 +26,10 @@
#include <net/tc_act/tc_mirred.h>
#include <net/tc_wrapper.h>
+#define MIRRED_DEFER_LIMIT 3
+_Static_assert(MIRRED_DEFER_LIMIT <= 3,
+ "MIRRED_DEFER_LIMIT exceeds tc_depth bitfield width");
+
static LIST_HEAD(mirred_list);
static DEFINE_SPINLOCK(mirred_list_lock);
@@ -234,12 +238,15 @@ tcf_mirred_forward(bool at_ingress, bool want_ingress, struct sk_buff *skb)
{
int err;
- if (!want_ingress)
+ if (!want_ingress) {
err = tcf_dev_queue_xmit(skb, dev_queue_xmit);
- else if (!at_ingress)
- err = netif_rx(skb);
- else
- err = netif_receive_skb(skb);
+ } else {
+ skb->tc_depth++;
+ if (!at_ingress)
+ err = netif_rx(skb);
+ else
+ err = netif_receive_skb(skb);
+ }
return err;
}
@@ -426,6 +433,7 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
struct netdev_xmit *xmit;
bool m_mac_header_xmit;
struct net_device *dev;
+ bool want_ingress;
int i, m_eaction;
u32 blockid;
@@ -434,7 +442,8 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
#else
xmit = this_cpu_ptr(&softnet_data.xmit);
#endif
- if (unlikely(xmit->sched_mirred_nest >= MIRRED_NEST_LIMIT)) {
+ if (unlikely(xmit->sched_mirred_nest >= MIRRED_NEST_LIMIT ||
+ skb->tc_depth >= MIRRED_DEFER_LIMIT)) {
net_warn_ratelimited("Packet exceeded mirred recursion limit on dev %s\n",
netdev_name(skb->dev));
return TC_ACT_SHOT;
@@ -453,23 +462,27 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
tcf_action_inc_overlimit_qstats(&m->common);
return retval;
}
- for (i = 0; i < xmit->sched_mirred_nest; i++) {
- if (xmit->sched_mirred_dev[i] != dev)
- continue;
- pr_notice_once("tc mirred: loop on device %s\n",
- netdev_name(dev));
- tcf_action_inc_overlimit_qstats(&m->common);
- return retval;
- }
- xmit->sched_mirred_dev[xmit->sched_mirred_nest++] = dev;
+ m_eaction = READ_ONCE(m->tcfm_eaction);
+ want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
+ if (!want_ingress) {
+ for (i = 0; i < xmit->sched_mirred_nest; i++) {
+ if (xmit->sched_mirred_dev[i] != dev)
+ continue;
+ pr_notice_once("tc mirred: loop on device %s\n",
+ netdev_name(dev));
+ tcf_action_inc_overlimit_qstats(&m->common);
+ return retval;
+ }
+ xmit->sched_mirred_dev[xmit->sched_mirred_nest++] = dev;
+ }
m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit);
- m_eaction = READ_ONCE(m->tcfm_eaction);
retval = tcf_mirred_to_dev(skb, m, dev, m_mac_header_xmit, m_eaction,
retval);
- xmit->sched_mirred_nest--;
+ if (!want_ingress)
+ xmit->sched_mirred_nest--;
return retval;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 6/9] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
` (4 preceding siblings ...)
2026-04-26 19:09 ` [PATCH net 5/9] net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 7/9] net/sched: act_mirred: Fix skb leak in early mirred redirect returns Jamal Hadi Salim
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim
From: "Kito Xu (veritas501)" <hxzene@gmail.com>
tcf_mirred_act() checks sched_mirred_nest against MIRRED_NEST_LIMIT (4)
to prevent deep recursion. However, when the action uses blockcast
(tcfm_blockid != 0), the function returns at the tcf_blockcast() call
BEFORE reaching the counter increment. As a result, the recursion
counter never advances and the limit check is entirely bypassed.
When two devices share a TC egress block with a mirred blockcast rule,
a packet egressing on device A is mirrored to device B via blockcast;
device B's egress TC re-enters tcf_mirred_act() via blockcast and
mirrors back to A, creating an unbounded recursion loop:
tcf_mirred_act -> tcf_blockcast -> tcf_mirred_to_dev -> dev_queue_xmit
-> sch_handle_egress -> tcf_classify -> tcf_mirred_act -> (repeat)
This recursion continues until the kernel stack overflows.
The bug is reachable from an unprivileged user via
unshare(CLONE_NEWUSER | CLONE_NEWNET): user namespaces grant
CAP_NET_ADMIN in the new network namespace, which is sufficient to
create dummy devices, attach clsact qdiscs with shared blocks, and
install mirred blockcast filters.
BUG: TASK stack guard page was hit at ffffc90000b7fff8
Oops: stack guard page: 0000 [#1] SMP KASAN NOPTI
CPU: 2 UID: 1000 PID: 169 Comm: poc Not tainted 7.0.0-rc7-next-20260410
RIP: 0010:xas_find+0x17/0x480
Call Trace:
xa_find+0x17b/0x1d0
tcf_mirred_act+0x640/0x1060
tcf_action_exec+0x400/0x530
basic_classify+0x128/0x1d0
tcf_classify+0xd83/0x1150
tc_run+0x328/0x620
__dev_queue_xmit+0x797/0x3100
tcf_mirred_to_dev+0x7b1/0xf70
tcf_mirred_act+0x68a/0x1060
[repeating ~30+ times until stack overflow]
Kernel panic - not syncing: Fatal exception in interrupt
Fix this by incrementing sched_mirred_nest before calling
tcf_blockcast() and decrementing it on return, mirroring the
non-blockcast path. This ensures subsequent recursive entries see the
updated counter and are correctly limited by MIRRED_NEST_LIMIT.
Fixes: fe946a751d9b ("net/sched: act_mirred: add loop detection")
Tested-by: Victor Nogueira <victor@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Kito Xu (veritas501) <hxzene@gmail.com>
---
net/sched/act_mirred.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index dd5e7ea7ef26..ea64faf7f469 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -453,8 +453,12 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
tcf_action_update_bstats(&m->common, skb);
blockid = READ_ONCE(m->tcfm_blockid);
- if (blockid)
- return tcf_blockcast(skb, m, blockid, res, retval);
+ if (blockid) {
+ xmit->sched_mirred_dev[xmit->sched_mirred_nest++] = NULL;
+ retval = tcf_blockcast(skb, m, blockid, res, retval);
+ xmit->sched_mirred_nest--;
+ return retval;
+ }
dev = rcu_dereference_bh(m->tcfm_dev);
if (unlikely(!dev)) {
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 7/9] net/sched: act_mirred: Fix skb leak in early mirred redirect returns
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
` (5 preceding siblings ...)
2026-04-26 19:09 ` [PATCH net 6/9] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 8/9] selftests/tc-testing: Add mirred test cases exercising loops Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 9/9] selftests/tc-testing: Add netem test case " Jamal Hadi Salim
8 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Sashiko, Jamal Hadi Salim
From: Victor Nogueira <victor@mojatatu.com>
Since retval is set as TC_ACT_STOLEN in the mirred redirect case,
returning retval in cases where redirect failed will make the core
code not free the skb and thus cause a leak.
Fix this by returning TC_ACT_SHOT instead in such scenarios.
Fixes: 16085e48cb48 ("net/sched: act_mirred: Create function tcf_mirred_to_dev and improve readability")
Reported-by: Sashiko <sashiko-dev@lists.linux.dev>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
net/sched/act_mirred.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index ea64faf7f469..4c7af7bd7c0d 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -412,7 +412,7 @@ static int tcf_blockcast(struct sk_buff *skb, struct tcf_mirred *m,
block = tcf_block_lookup(dev_net(skb->dev), blockid);
if (!block || xa_empty(&block->ports)) {
tcf_action_inc_overlimit_qstats(&m->common);
- return retval;
+ return is_redirect ? TC_ACT_SHOT : retval;
}
if (is_redirect)
@@ -430,8 +430,8 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
{
struct tcf_mirred *m = to_mirred(a);
int retval = READ_ONCE(m->tcf_action);
+ bool m_mac_header_xmit, is_redirect;
struct netdev_xmit *xmit;
- bool m_mac_header_xmit;
struct net_device *dev;
bool want_ingress;
int i, m_eaction;
@@ -460,14 +460,15 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
return retval;
}
+ m_eaction = READ_ONCE(m->tcfm_eaction);
+ is_redirect = tcf_mirred_is_act_redirect(m_eaction);
+
dev = rcu_dereference_bh(m->tcfm_dev);
if (unlikely(!dev)) {
pr_notice_once("tc mirred: target device is gone\n");
tcf_action_inc_overlimit_qstats(&m->common);
- return retval;
+ goto err_out;
}
-
- m_eaction = READ_ONCE(m->tcfm_eaction);
want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
if (!want_ingress) {
for (i = 0; i < xmit->sched_mirred_nest; i++) {
@@ -476,7 +477,7 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
pr_notice_once("tc mirred: loop on device %s\n",
netdev_name(dev));
tcf_action_inc_overlimit_qstats(&m->common);
- return retval;
+ goto err_out;
}
xmit->sched_mirred_dev[xmit->sched_mirred_nest++] = dev;
}
@@ -489,6 +490,11 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
xmit->sched_mirred_nest--;
return retval;
+
+err_out:
+ if (is_redirect)
+ retval = TC_ACT_SHOT;
+ return retval;
}
static void tcf_stats_update(struct tc_action *a, u64 bytes, u64 packets,
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 8/9] selftests/tc-testing: Add mirred test cases exercising loops
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
` (6 preceding siblings ...)
2026-04-26 19:09 ` [PATCH net 7/9] net/sched: act_mirred: Fix skb leak in early mirred redirect returns Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 9/9] selftests/tc-testing: Add netem test case " Jamal Hadi Salim
8 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim
From: Victor Nogueira <victor@mojatatu.com>
Add mirred loop test cases to validate that those will be caught and other
test cases that were previously misinterpreted as loops by mirred.
This commit adds 12 test cases:
- Redirect multiport: dummy egress -> dev1 ingress -> dummy egress (Loop)
- Redirect singleport: dev1 ingress -> dev1 egress -> dev1 ingress (Loop)
- Redirect multiport: dev1 ingress -> dummy ingress -> dev1 egress (No Loop)
- Redirect multiport: dev1 ingress -> dummy ingress -> dev1 ingress (Loop)
- Redirect multiport: dev1 ingress -> dummy egress -> dev1 ingress (Loop)
- Redirect multiport: dummy egress -> dev1 ingress -> dummy egress, different prios (Loop)
- Redirect multiport: dev1 ingress -> dummy ingress -> dummy egress -> dev1 egress (No Loop)
- Redirect multiport: dev1 ingress -> dummy egress -> dev1 egress (No Loop)
- Redirect multiport: dev1 ingress -> dummy egress -> dummy ingress (No Loop)
- Redirect singleport: dev1 ingress -> dev1 ingress (Loop)
- Redirect singleport: dummy egress -> dummy ingress (No Loop)
- Redirect multiport: dev1 ingress -> dummy ingress -> dummy egress (No Loop)
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
.../tc-testing/tc-tests/actions/mirred.json | 616 +++++++++++++++++-
1 file changed, 615 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
index b056eb966871..d0cad6571691 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
@@ -1144,6 +1144,620 @@
"teardown": [
"$TC qdisc del dev $DUMMY clsact"
]
+ },
+ {
+ "id": "531c",
+ "name": "Redirect multiport: dummy egress -> dev1 ingress -> dummy egress (Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin"
+ ]
+ },
+ "setup": [
+ "$IP link set dev $DUMMY up || true",
+ "$IP addr add 10.10.10.10/24 dev $DUMMY || true",
+ "$TC qdisc add dev $DUMMY clsact",
+ "$TC filter add dev $DUMMY egress protocol ip prio 10 matchall action mirred ingress redirect dev $DEV1 index 1",
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred egress redirect dev $DUMMY index 2"
+ ],
+ "cmdUnderTest": "ping -c1 -W0.01 -I $DUMMY 10.10.10.1",
+ "expExitCode": "1",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "ingress",
+ "index": 1,
+ "stats": {
+ "packets": 3
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DUMMY clsact",
+ "$TC qdisc del dev $DEV1 clsact"
+ ]
+ },
+ {
+ "id": "b1d7",
+ "name": "Redirect singleport: dev1 ingress -> dev1 egress -> dev1 ingress (Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred egress redirect dev $DEV1 index 1"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DEV1 egress protocol ip prio 11 matchall action mirred ingress redirect dev $DEV1 index 2",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "egress",
+ "index": 1,
+ "stats": {
+ "packets": 3
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact"
+ ]
+ },
+ {
+ "id": "c66d",
+ "name": "Redirect multiport: dev1 ingress -> dummy ingress -> dev1 egress (No Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred ingress redirect dev $DUMMY index 1",
+ "$TC qdisc add dev $DUMMY clsact"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DUMMY ingress protocol ip prio 11 matchall action mirred egress redirect dev $DEV1 index 2",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "ingress",
+ "index": 1,
+ "stats": {
+ "packets": 1
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact",
+ "$TC qdisc del dev $DUMMY clsact"
+ ]
+ },
+ {
+ "id": "aa99",
+ "name": "Redirect multiport: dev1 ingress -> dummy ingress -> dev1 ingress (Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred ingress redirect dev $DUMMY index 1",
+ "$TC qdisc add dev $DUMMY clsact"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DUMMY ingress protocol ip prio 11 matchall action mirred ingress redirect dev $DEV1 index 2",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "ingress",
+ "index": 1,
+ "stats": {
+ "packets": 2,
+ "overlimits": 1
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact",
+ "$TC qdisc del dev $DUMMY clsact"
+ ]
+ },
+ {
+ "id": "37d7",
+ "name": "Redirect multiport: dev1 ingress -> dummy egress -> dev1 ingress (Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred egress redirect dev $DUMMY index 1",
+ "$TC qdisc add dev $DUMMY clsact"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DUMMY egress protocol ip prio 11 matchall action mirred ingress redirect dev $DEV1 index 2",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "egress",
+ "index": 1,
+ "stats": {
+ "packets": 3
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact",
+ "$TC qdisc del dev $DUMMY clsact"
+ ]
+ },
+ {
+ "id": "6d02",
+ "name": "Redirect multiport: dummy egress -> dev1 ingress -> dummy egress, different prios (Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin"
+ ]
+ },
+ "setup": [
+ "$IP link set dev $DUMMY up || true",
+ "$IP addr add 10.10.10.10/24 dev $DUMMY || true",
+ "$TC qdisc add dev $DUMMY clsact",
+ "$TC filter add dev $DUMMY egress protocol ip prio 10 matchall action mirred ingress redirect dev $DEV1 index 1",
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 11 matchall action mirred egress redirect dev $DUMMY index 2"
+ ],
+ "cmdUnderTest": "ping -c1 -W0.01 -I $DUMMY 10.10.10.1",
+ "expExitCode": "1",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "ingress",
+ "index": 1,
+ "stats": {
+ "packets": 3
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DUMMY clsact",
+ "$TC qdisc del dev $DEV1 clsact"
+ ]
+ },
+ {
+ "id": "8115",
+ "name": "Redirect multiport: dev1 ingress -> dummy ingress -> dummy egress -> dev1 egress (No Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred ingress redirect dev $DUMMY index 1",
+ "$TC qdisc add dev $DUMMY clsact",
+ "$TC filter add dev $DUMMY ingress protocol ip prio 11 matchall action mirred egress redirect dev $DUMMY index 2"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DUMMY egress protocol ip prio 12 matchall action mirred egress redirect dev $DEV1 index 3",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "ingress",
+ "index": 1,
+ "stats": {
+ "packets": 1
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact",
+ "$TC qdisc del dev $DUMMY clsact"
+ ]
+ },
+ {
+ "id": "9eb3",
+ "name": "Redirect multiport: dev1 ingress -> dummy egress -> dev1 egress (No Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred egress redirect dev $DUMMY index 1",
+ "$TC qdisc add dev $DUMMY clsact"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DUMMY egress protocol ip prio 11 matchall action mirred egress redirect dev $DEV1 index 2",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "egress",
+ "index": 1,
+ "stats": {
+ "packets": 1
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact",
+ "$TC qdisc del dev $DUMMY clsact"
+ ]
+ },
+ {
+ "id": "d837",
+ "name": "Redirect multiport: dev1 ingress -> dummy egress -> dummy ingress (No Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred egress redirect dev $DUMMY index 1",
+ "$TC qdisc add dev $DUMMY clsact"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DUMMY egress protocol ip prio 11 matchall action mirred ingress redirect dev $DUMMY index 2",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "egress",
+ "index": 1,
+ "stats": {
+ "packets": 1
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact",
+ "$TC qdisc del dev $DUMMY clsact"
+ ]
+ },
+ {
+ "id": "2071",
+ "name": "Redirect singleport: dev1 ingress -> dev1 ingress (Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred ingress redirect dev $DEV1 index 1",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "ingress",
+ "index": 1,
+ "stats": {
+ "packets": 1,
+ "overlimits": 1
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact"
+ ]
+ },
+ {
+ "id": "0101",
+ "name": "Redirect singleport: dummy egress -> dummy ingress (No Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin"
+ ]
+ },
+ "setup": [
+ "$IP addr add 10.10.10.10/24 dev $DUMMY || true",
+ "$TC qdisc add dev $DUMMY clsact",
+ "$TC filter add dev $DUMMY egress protocol ip prio 11 matchall action mirred ingress redirect dev $DUMMY index 1"
+ ],
+ "cmdUnderTest": "ping -c1 -W0.01 -I $DUMMY 10.10.10.1",
+ "expExitCode": "1",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "ingress",
+ "index": 1,
+ "stats": {
+ "packets": 1
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DUMMY clsact"
+ ]
+ },
+ {
+ "id": "cf97",
+ "name": "Redirect multiport: dev1 ingress -> dummy ingress -> dummy egress (No Loop)",
+ "category": [
+ "filter",
+ "mirred"
+ ],
+ "plugins": {
+ "requires": [
+ "nsPlugin",
+ "scapyPlugin"
+ ]
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 clsact",
+ "$TC filter add dev $DEV1 ingress protocol ip prio 10 matchall action mirred ingress redirect dev $DUMMY index 1",
+ "$TC qdisc add dev $DUMMY clsact"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DUMMY ingress protocol ip prio 11 matchall action mirred egress redirect dev $DUMMY index 2",
+ "scapy": [
+ {
+ "iface": "$DEV0",
+ "count": 1,
+ "packet": "Ether()/IP(dst='10.10.10.1', src='10.10.10.10')/ICMP()"
+ }
+ ],
+ "expExitCode": "0",
+ "verifyCmd": "$TC -j -s actions get action mirred index 1",
+ "matchJSON": [
+ {
+ "total acts": 0
+ },
+ {
+ "actions": [
+ {
+ "order": 1,
+ "kind": "mirred",
+ "mirred_action": "redirect",
+ "direction": "ingress",
+ "index": 1,
+ "stats": {
+ "packets": 1
+ },
+ "not_in_hw": true
+ }
+ ]
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact",
+ "$TC qdisc del dev $DUMMY clsact"
+ ]
}
-
]
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 9/9] selftests/tc-testing: Add netem test case exercising loops
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
` (7 preceding siblings ...)
2026-04-26 19:09 ` [PATCH net 8/9] selftests/tc-testing: Add mirred test cases exercising loops Jamal Hadi Salim
@ 2026-04-26 19:09 ` Jamal Hadi Salim
8 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2026-04-26 19:09 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, savy,
will, xmei5, pctammela, kuniyu, toke, willemdebruijnkernel,
hxzene, Jamal Hadi Salim
From: Victor Nogueira <victor@mojatatu.com>
Add a netem nested duplicate test case to validate that it won't
cause an infinite loop
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
.../tc-testing/tc-tests/qdiscs/netem.json | 33 ++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/netem.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/netem.json
index 3c4444961488..7c954989069d 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/netem.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/netem.json
@@ -336,5 +336,36 @@
"teardown": [
"$TC qdisc del dev $DUMMY handle 1: root"
]
- }
+ },
+ {
+ "id": "8c17",
+ "name": "Test netem's recursive duplicate",
+ "category": [
+ "qdisc",
+ "netem"
+ ],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
+ "setup": [
+ "$IP link set dev $DUMMY up || true",
+ "$IP addr add 10.10.11.10/24 dev $DUMMY || true",
+ "$TC qdisc add dev $DUMMY root handle 1: netem limit 1 duplicate 100%",
+ "$TC qdisc add dev $DUMMY parent 1: handle 2: netem duplicate 100%"
+ ],
+ "cmdUnderTest": "ping -c 1 10.10.11.11 -W 0.01",
+ "expExitCode": "1",
+ "verifyCmd": "$TC -s -j qdisc ls dev $DUMMY root",
+ "matchJSON": [
+ {
+ "kind": "netem",
+ "handle": "1:",
+ "bytes": 294,
+ "packets": 3
+ }
+ ],
+ "teardown": [
+ "$TC qdisc del dev $DUMMY handle 1: root"
+ ]
+ }
]
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net 4/9] net/sched: fix packet loop on netem when duplicate is on
2026-04-26 19:09 ` [PATCH net 4/9] net/sched: fix packet loop on netem when duplicate is on Jamal Hadi Salim
@ 2026-04-26 19:43 ` William Liu
0 siblings, 0 replies; 11+ messages in thread
From: William Liu @ 2026-04-26 19:43 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, stephen,
victor, savy, xmei5, pctammela, kuniyu, toke,
willemdebruijnkernel, hxzene
Reviewed-by: William Liu <will@willsroot.io>
On Sunday, April 26th, 2026 at 7:09 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> When netem duplicates a packet it re-enqueues the copy at the root qdisc.
> If another netem sits in the tree the copy can be duplicated
> again, recursing until the stack or memory is exhausted.
>
> The original duplication guard temporarily zeroed q->duplicate around
> the re-enqueue, but that does not cover all cases because it is
> per-qdisc state shared across all concurrent enqueue paths
> and is not safe without additional locking.
>
> Use the skb tc_depth field introduced in an earlier patch:
> - increment it on the duplicate before re-enqueue
> - skip duplication for any skb whose tc_depth is already non-zero.
>
> This marks the packet itself rather than mutating qdisc state,
> therefore it is safe regardless of tree topology or concurrency.
>
> Fixes: 0afb51e72855 ("[PKT_SCHED]: netem: reinsert for duplication")
> Reported-by: William Liu <will@willsroot.io>
> Reported-by: Savino Dicanosa <savy@syst3mfailure.io>
> Closes: https://lore.kernel.org/netdev/8DuRWwfqjoRDLDmBMlIfbrsZg9Gx50DHJc1ilxsEBNe2D6NMoigR_eIRIG0LOjMc3r10nUUZtArXx4oZBIdUfZQrwjcQhdinnMis_0G7VEk=@willsroot.io/
> Co-developed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> net/sched/sch_netem.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index 8c6dea196089..d595eb03dca8 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -461,7 +461,8 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> skb->prev = NULL;
>
> /* Random duplication */
> - if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor, &q->prng))
> + if (q->duplicate && skb->tc_depth == 0 &&
> + q->duplicate >= get_crandom(&q->dup_cor, &q->prng))
> ++count;
>
> /* Drop packet? */
> @@ -540,11 +541,9 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> */
> if (skb2) {
> struct Qdisc *rootq = qdisc_root_bh(sch);
> - u32 dupsave = q->duplicate; /* prevent duplicating a dup... */
>
> - q->duplicate = 0;
> + skb2->tc_depth++; /* prevent duplicating a dup... */
> rootq->enqueue(skb2, rootq, to_free);
> - q->duplicate = dupsave;
> skb2 = NULL;
> }
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-04-26 19:43 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-26 19:09 [PATCH net 0/9] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 1/9] net: Introduce skb tc depth field to track packet loops Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 2/9] net/sched: Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 3/9] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 4/9] net/sched: fix packet loop on netem when duplicate is on Jamal Hadi Salim
2026-04-26 19:43 ` William Liu
2026-04-26 19:09 ` [PATCH net 5/9] net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 6/9] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 7/9] net/sched: act_mirred: Fix skb leak in early mirred redirect returns Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 8/9] selftests/tc-testing: Add mirred test cases exercising loops Jamal Hadi Salim
2026-04-26 19:09 ` [PATCH net 9/9] selftests/tc-testing: Add netem test case " Jamal Hadi Salim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox