* [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem
@ 2026-03-16 21:10 Jamal Hadi Salim
2026-03-16 21:10 ` [PATCH net v2 1/6] net: Introduce skb ttl field to track packet loops Jamal Hadi Salim
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Jamal Hadi Salim @ 2026-03-16 21:10 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, will,
xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel,
Jamal Hadi Salim
We introduce a 2-bit global skb->ttl counter. Patch #1 describes how we put
together those bits. Patches #2 and patch #5 use these bits.
I added Fixes tags to patch #1 in case it is useful for backporting.
Patch #3 and #4 revert William's earlier netem commits. Patch #6 introduces
tdc test cases.
Changes in v2:
Do not reuse skb->from_ingress (which was move skb->cb)
Jamal Hadi Salim (5):
net: Introduce skb ttl field to track packet loops
net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop
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
Victor Nogueira (1):
selftests/tc-testing: Add netem/mirred test cases exercising loops
include/linux/skbuff.h | 2 +
net/sched/act_mirred.c | 45 +-
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, 674 insertions(+), 137 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH net v2 1/6] net: Introduce skb ttl field to track packet loops 2026-03-16 21:10 [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim @ 2026-03-16 21:10 ` Jamal Hadi Salim 2026-03-16 21:10 ` [PATCH net v2 2/6] net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop Jamal Hadi Salim ` (5 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-16 21:10 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel, Jamal Hadi Salim In order to keep track of loops across the stack we need to _remember the global loop state in the skb_. We introduce a 2 bit per-skb ttl field to keep track of this state. The following shows the before and after pahole diff: 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 ttl: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 removed as part of tc_verd in commit aec745e2c520 ("net-tc: remove unused tc_verd fields"). It was already unused by that time removed earlier in commit c19ae86a510c ("tc: remove unused redirect ttl"). An existing per-cpu loop count, MIRRED_NEST_LIMIT, exists; however, this count assumes a single call stack assumption and suffers from two challenges: 1)if we queue the packet somewhere and then restart processing later the per-cpu state is lost (example, it gets wiped out the moment we go egress->ingress and queue the packet in the backlog and later packets are being pulled from backlog) 2) If we have X/RPS where a packet came in one CPU but may end up on a different CPU. Our first attempt was to "liberate" the skb->from_ingress bit into the skb->cb field (v1) and after a lot of deeper reviews found that it does get trampled in case of hardware offload via the mlnx driver. Our second attempt (which we didnt post) was to "liberate" the skb->tc_skip_classify bit into the skb->cb - but that led us to a path of making changes that are sensitive such as making mods to dev queue xmit. This is our third attempt. Use cases: 1) Mirred increments the ttl whenever it sees an skb. This in combination with MIRRED_NEST_LIMIT helps us resolve both challenges mentioned above. This is ilustrated in patch #2. 2) netem increments the ttl when using the "duplicate" feature and catches it when it sees the packet the second time. This is ilustrated in patch #5. Fixes: fe946a751d9b ("net/sched: act_mirred: add loop detection") Fixes: 0afb51e72855 ("[PKT_SCHED]: netem: reinsert for duplication") Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> --- include/linux/skbuff.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index daa4e4944ce3..f1326c4b4bcc 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -848,6 +848,7 @@ enum skb_tstamp_type { * CHECKSUM_UNNECESSARY (max 3) * @unreadable: indicates that at least 1 of the fragments in this skb is * unreadable. + * @ttl: time to live counter for packet loops. * @dst_pending_confirm: need to confirm neighbour * @decrypted: Decrypted SKB * @slow_gro: state present at GRO time, slower prepare step required @@ -1030,6 +1031,7 @@ struct sk_buff { __u8 csum_not_inet:1; #endif __u8 unreadable:1; + __u8 ttl: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] 19+ messages in thread
* [PATCH net v2 2/6] net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop 2026-03-16 21:10 [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim 2026-03-16 21:10 ` [PATCH net v2 1/6] net: Introduce skb ttl field to track packet loops Jamal Hadi Salim @ 2026-03-16 21:10 ` Jamal Hadi Salim 2026-03-16 21:10 ` [PATCH net v2 3/6] Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" Jamal Hadi Salim ` (4 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-16 21:10 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel, 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 Another bug fixed here is a false positive, example current code will claim this is a loop when its not: tc qdisc add dev port0 clsact tc qdisc add dev port1 clsact # port0 ingress --> port1 ingress tc filter add dev port0 ingress protocol ip \ prio 10 matchall action mirred ingress redirect dev port1 # port1 ingress --> port1 egress tc filter add dev port1 ingress protocol ip \ prio 10 matchall action mirred egress redirect dev port1 #port1 egress --> port0 egress tc filter add dev port1 egress protocol ip \ prio 11 matchall action mirred egress redirect dev port0 ping -c 1 -W0.01 10.10.10.10 And see the ping fail Fixes: fe946a751d9b ("net/sched: act_mirred: add loop detection") Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> --- net/sched/act_mirred.c | 45 ++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 05e0b14b5773..9ef261e19e40 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -26,6 +26,8 @@ #include <net/tc_act/tc_mirred.h> #include <net/tc_wrapper.h> +#define MIRRED_DEFER_LIMIT 3 + static LIST_HEAD(mirred_list); static DEFINE_SPINLOCK(mirred_list_lock); @@ -234,12 +236,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->ttl++; + if (!at_ingress) + err = netif_rx(skb); + else + err = netif_receive_skb(skb); + } return err; } @@ -426,6 +431,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 +440,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->ttl >= 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 +460,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] 19+ messages in thread
* [PATCH net v2 3/6] Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" 2026-03-16 21:10 [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim 2026-03-16 21:10 ` [PATCH net v2 1/6] net: Introduce skb ttl field to track packet loops Jamal Hadi Salim 2026-03-16 21:10 ` [PATCH net v2 2/6] net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop Jamal Hadi Salim @ 2026-03-16 21:10 ` Jamal Hadi Salim 2026-03-17 0:54 ` Stephen Hemminger 2026-03-16 21:10 ` [PATCH net v2 4/6] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" Jamal Hadi Salim ` (3 subsequent siblings) 6 siblings, 1 reply; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-16 21:10 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel, Jamal Hadi Salim, Ji-Soo Chung, Gerlinde, zyc zyc, Manas Ghandat This reverts commit ec8e0e3d7adef940cdf9475e2352c0680189d14e. 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> --- 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 5de1c932944a..0ccf74a9cb82 100644 --- a/net/sched/sch_netem.c +++ b/net/sched/sch_netem.c @@ -974,41 +974,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) @@ -1067,11 +1032,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] 19+ messages in thread
* Re: [PATCH net v2 3/6] Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" 2026-03-16 21:10 ` [PATCH net v2 3/6] Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" Jamal Hadi Salim @ 2026-03-17 0:54 ` Stephen Hemminger 0 siblings, 0 replies; 19+ messages in thread From: Stephen Hemminger @ 2026-03-17 0:54 UTC (permalink / raw) To: Jamal Hadi Salim Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel, Ji-Soo Chung, Gerlinde, zyc zyc, Manas Ghandat On Mon, 16 Mar 2026 17:10:49 -0400 Jamal Hadi Salim <jhs@mojatatu.com> wrote: > This reverts commit ec8e0e3d7adef940cdf9475e2352c0680189d14e. > > 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> > --- I would prefer that the commit messag have the reason why this was reverted. Someone in the future might not see the netdev history. Acked-by: Stephen Hemminger <stephen@networkplumber.org> ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net v2 4/6] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" 2026-03-16 21:10 [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim ` (2 preceding siblings ...) 2026-03-16 21:10 ` [PATCH net v2 3/6] Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" Jamal Hadi Salim @ 2026-03-16 21:10 ` Jamal Hadi Salim 2026-03-17 0:55 ` Stephen Hemminger 2026-03-16 21:10 ` [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on Jamal Hadi Salim ` (2 subsequent siblings) 6 siblings, 1 reply; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-16 21:10 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel, Jamal Hadi Salim This reverts commit ecdec65ec78d67d3ebd17edc88b88312054abe0d. Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> --- .../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 6a39640aa2a8..ceb993ed04b2 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] 19+ messages in thread
* Re: [PATCH net v2 4/6] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" 2026-03-16 21:10 ` [PATCH net v2 4/6] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" Jamal Hadi Salim @ 2026-03-17 0:55 ` Stephen Hemminger 2026-03-18 19:26 ` Jamal Hadi Salim 0 siblings, 1 reply; 19+ messages in thread From: Stephen Hemminger @ 2026-03-17 0:55 UTC (permalink / raw) To: Jamal Hadi Salim Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Mon, 16 Mar 2026 17:10:50 -0400 Jamal Hadi Salim <jhs@mojatatu.com> wrote: > This reverts commit ecdec65ec78d67d3ebd17edc88b88312054abe0d. > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Are the tests being replaced in a later patch. Want more tests so that this is covered. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v2 4/6] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" 2026-03-17 0:55 ` Stephen Hemminger @ 2026-03-18 19:26 ` Jamal Hadi Salim 0 siblings, 0 replies; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-18 19:26 UTC (permalink / raw) To: Stephen Hemminger Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Mon, Mar 16, 2026 at 8:55 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Mon, 16 Mar 2026 17:10:50 -0400 > Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > > This reverts commit ecdec65ec78d67d3ebd17edc88b88312054abe0d. > > > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> > > Are the tests being replaced in a later patch. > Want more tests so that this is covered. Not sure i followed - are you suggesting this patch should come later? Patch 6 covers the tests. Do you feel we need more than what we have there? cheers, jamal ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on 2026-03-16 21:10 [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim ` (3 preceding siblings ...) 2026-03-16 21:10 ` [PATCH net v2 4/6] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" Jamal Hadi Salim @ 2026-03-16 21:10 ` Jamal Hadi Salim 2026-03-17 0:57 ` Stephen Hemminger 2026-03-16 21:10 ` [PATCH net v2 6/6] selftests/tc-testing: Add netem/mirred test cases exercising loops Jamal Hadi Salim 2026-03-17 23:36 ` [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Stephen Hemminger 6 siblings, 1 reply; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-16 21:10 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel, Jamal Hadi Salim As stated by William [1]: "netem_enqueue's duplication prevention logic breaks when a netem resides in a qdisc tree with other netems - this can lead to a soft lockup and OOM loop in netem_dequeue, as seen in [2]. Ensure that a duplicating netem cannot exist in a tree with other netems." In this patch, we use the first approach suggested in [1] (the skb ttl field) to detect and stop a possible netem duplicate infinite loop. [1] https://lore.kernel.org/netdev/20250708164141.875402-1-will@willsroot.io/ [2] https://lore.kernel.org/netdev/8DuRWwfqjoRDLDmBMlIfbrsZg9Gx50DHJc1ilxsEBNe2D6NMoigR_eIRIG0LOjMc3r10nUUZtArXx4oZBIdUfZQrwjcQhdinnMis_0G7VEk=@willsroot.io/ 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> --- 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 0ccf74a9cb82..6949d1b7867a 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->ttl && + q->duplicate >= get_crandom(&q->dup_cor, &q->prng)) ++count; /* Drop packet? */ @@ -539,11 +540,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->ttl++; /* prevent duplicating a dup... */ rootq->enqueue(skb2, rootq, to_free); - q->duplicate = dupsave; skb2 = NULL; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on 2026-03-16 21:10 ` [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on Jamal Hadi Salim @ 2026-03-17 0:57 ` Stephen Hemminger 2026-03-18 19:34 ` Jamal Hadi Salim 0 siblings, 1 reply; 19+ messages in thread From: Stephen Hemminger @ 2026-03-17 0:57 UTC (permalink / raw) To: Jamal Hadi Salim Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Mon, 16 Mar 2026 17:10:51 -0400 Jamal Hadi Salim <jhs@mojatatu.com> wrote: > --- 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->ttl && > + q->duplicate >= get_crandom(&q->dup_cor, &q->prng)) > ++count; > Would prefer that if ttl is exhausted packet was dropped. Rather than silently not duplicating. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on 2026-03-17 0:57 ` Stephen Hemminger @ 2026-03-18 19:34 ` Jamal Hadi Salim 2026-03-19 1:25 ` William Liu 2026-03-23 19:33 ` Jamal Hadi Salim 0 siblings, 2 replies; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-18 19:34 UTC (permalink / raw) To: Stephen Hemminger Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Mon, Mar 16, 2026 at 8:57 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Mon, 16 Mar 2026 17:10:51 -0400 > Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > > --- 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->ttl && > > + q->duplicate >= get_crandom(&q->dup_cor, &q->prng)) > > ++count; > > > > Would prefer that if ttl is exhausted packet was dropped. > Rather than silently not duplicating. so return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; Note, this is a change of behavior, no? Actually - when i was looking the first time the bug got reported in my view the bigger issue is when skb_clone() fails (which happens nastily when you have that repro from William). Unfortunately, the surgery was too big so i gave up. cheers, jamal ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on 2026-03-18 19:34 ` Jamal Hadi Salim @ 2026-03-19 1:25 ` William Liu 2026-03-23 23:14 ` Stephen Hemminger 2026-03-23 19:33 ` Jamal Hadi Salim 1 sibling, 1 reply; 19+ messages in thread From: William Liu @ 2026-03-19 1:25 UTC (permalink / raw) To: Jamal Hadi Salim Cc: Stephen Hemminger, netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Wednesday, March 18th, 2026 at 7:34 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > On Mon, Mar 16, 2026 at 8:57 PM Stephen Hemminger > <stephen@networkplumber.org> wrote: > > > > On Mon, 16 Mar 2026 17:10:51 -0400 > > Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > > > > --- 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->ttl && > > > + q->duplicate >= get_crandom(&q->dup_cor, &q->prng)) > > > ++count; > > > > > > > Would prefer that if ttl is exhausted packet was dropped. > > Rather than silently not duplicating. I think silently not duplicating better matches the semantics of preventing duplication of duplicates, which is the goal of the fix. > so return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; > > Note, this is a change of behavior, no? > Actually - when i was looking the first time the bug got reported in > my view the bigger issue is when > skb_clone() fails (which happens nastily when you have that repro from > William). Unfortunately, the surgery was too big so i gave up. > If the GFP_ATOMIC allocation is failing, we are in a serious edge case and the kernel is in trouble anyways so the current approach of null checking seems reasonable enough. > cheers, > jamal > Best, Will ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on 2026-03-19 1:25 ` William Liu @ 2026-03-23 23:14 ` Stephen Hemminger 0 siblings, 0 replies; 19+ messages in thread From: Stephen Hemminger @ 2026-03-23 23:14 UTC (permalink / raw) To: William Liu Cc: Jamal Hadi Salim, netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Thu, 19 Mar 2026 01:25:59 +0000 William Liu <will@willsroot.io> wrote: > > > Would prefer that if ttl is exhausted packet was dropped. > > > Rather than silently not duplicating. > > I think silently not duplicating better matches the semantics of preventing duplication of duplicates, which is the goal of the fix. Ok, I was more considering mirred -> netem case. Whether that is a reasonable configuration is debatable. For the case of duplicate of duplicate, yes that should not happen. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on 2026-03-18 19:34 ` Jamal Hadi Salim 2026-03-19 1:25 ` William Liu @ 2026-03-23 19:33 ` Jamal Hadi Salim 1 sibling, 0 replies; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-23 19:33 UTC (permalink / raw) To: Stephen Hemminger Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Wed, Mar 18, 2026 at 3:34 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > On Mon, Mar 16, 2026 at 8:57 PM Stephen Hemminger > <stephen@networkplumber.org> wrote: > > > > On Mon, 16 Mar 2026 17:10:51 -0400 > > Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > > > > --- 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->ttl && > > > + q->duplicate >= get_crandom(&q->dup_cor, &q->prng)) > > > ++count; > > > > > > > Would prefer that if ttl is exhausted packet was dropped. > > Rather than silently not duplicating. > > so return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; > > Note, this is a change of behavior, no? > Actually - when i was looking the first time the bug got reported in > my view the bigger issue is when > skb_clone() fails (which happens nastily when you have that repro from > William). Unfortunately, the surgery was too big so i gave up. > Stephen, can we leave this part for updates later? i.e lets resolve this and improve after.... cheers, jamal > cheers, > jamal ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net v2 6/6] selftests/tc-testing: Add netem/mirred test cases exercising loops 2026-03-16 21:10 [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim ` (4 preceding siblings ...) 2026-03-16 21:10 ` [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on Jamal Hadi Salim @ 2026-03-16 21:10 ` Jamal Hadi Salim 2026-03-17 0:58 ` Stephen Hemminger 2026-03-17 23:36 ` [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Stephen Hemminger 6 siblings, 1 reply; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-16 21:10 UTC (permalink / raw) To: netdev Cc: davem, edumazet, kuba, pabeni, horms, jiri, stephen, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel, 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. Also add a netem nested duplicate test case to validate that it won't cause an infinite loop This commit adds 14 test cases: - Redirect multiport: dummy egress -> dev1 ingress -> dummy egress (Loop) - Redirect multiport: 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 (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 -> 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) - Test netem's recursive duplicate Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Victor Nogueira <victor@mojatatu.com> --- .../tc-testing/tc-tests/actions/mirred.json | 616 +++++++++++++++++- .../tc-testing/tc-tests/qdiscs/netem.json | 33 +- 2 files changed, 647 insertions(+), 2 deletions(-) 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..9eb32cdf1ed3 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 multiport: 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 (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 multiport: 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" + ] } - ] 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] 19+ messages in thread
* Re: [PATCH net v2 6/6] selftests/tc-testing: Add netem/mirred test cases exercising loops 2026-03-16 21:10 ` [PATCH net v2 6/6] selftests/tc-testing: Add netem/mirred test cases exercising loops Jamal Hadi Salim @ 2026-03-17 0:58 ` Stephen Hemminger 0 siblings, 0 replies; 19+ messages in thread From: Stephen Hemminger @ 2026-03-17 0:58 UTC (permalink / raw) To: Jamal Hadi Salim Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Mon, 16 Mar 2026 17:10:52 -0400 Jamal Hadi Salim <jhs@mojatatu.com> wrote: > - Test netem's recursive duplicate > > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> > Signed-off-by: Victor Nogueira <victor@mojatatu.com> > --- > .../tc-testing/tc-tests/actions/mirred.json | 616 +++++++++++++++++- > .../tc-testing/tc-tests/qdiscs/netem.json | 33 +- Please split mirred and netem into separate commits. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem 2026-03-16 21:10 [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim ` (5 preceding siblings ...) 2026-03-16 21:10 ` [PATCH net v2 6/6] selftests/tc-testing: Add netem/mirred test cases exercising loops Jamal Hadi Salim @ 2026-03-17 23:36 ` Stephen Hemminger 2026-03-18 19:41 ` Jamal Hadi Salim 6 siblings, 1 reply; 19+ messages in thread From: Stephen Hemminger @ 2026-03-17 23:36 UTC (permalink / raw) To: Jamal Hadi Salim Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Mon, 16 Mar 2026 17:10:46 -0400 Jamal Hadi Salim <jhs@mojatatu.com> wrote: > We introduce a 2-bit global skb->ttl counter. Patch #1 describes how we put > together those bits. Patches #2 and patch #5 use these bits. > I added Fixes tags to patch #1 in case it is useful for backporting. > Patch #3 and #4 revert William's earlier netem commits. Patch #6 introduces > tdc test cases. > > Changes in v2: > Do not reuse skb->from_ingress (which was move skb->cb) > > Jamal Hadi Salim (5): > net: Introduce skb ttl field to track packet loops > net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop > 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 > > Victor Nogueira (1): > selftests/tc-testing: Add netem/mirred test cases exercising loops > > include/linux/skbuff.h | 2 + > net/sched/act_mirred.c | 45 +- > 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, 674 insertions(+), 137 deletions(-) > I would also like to add additional guard rails here for netem, and you might consider the same for mirred. 1. Do not allow duplicate option outside of root namespace. 2. Cap the duplicate fraction at some rationale lower bound like 25% ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem 2026-03-17 23:36 ` [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Stephen Hemminger @ 2026-03-18 19:41 ` Jamal Hadi Salim [not found] ` <CAOaVG17Jp8QB_=r3-eWM6bvrLAixFuj+wYB_zv5+OZfiY0LyYA@mail.gmail.com> 0 siblings, 1 reply; 19+ messages in thread From: Jamal Hadi Salim @ 2026-03-18 19:41 UTC (permalink / raw) To: Stephen Hemminger Cc: netdev, davem, edumazet, kuba, pabeni, horms, jiri, victor, will, xmei5, pctammela, savy, kuniyu, toke, willemdebruijnkernel On Tue, Mar 17, 2026 at 7:36 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Mon, 16 Mar 2026 17:10:46 -0400 > Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > > We introduce a 2-bit global skb->ttl counter. Patch #1 describes how we put > > together those bits. Patches #2 and patch #5 use these bits. > > I added Fixes tags to patch #1 in case it is useful for backporting. > > Patch #3 and #4 revert William's earlier netem commits. Patch #6 introduces > > tdc test cases. > > > > Changes in v2: > > Do not reuse skb->from_ingress (which was move skb->cb) > > > > Jamal Hadi Salim (5): > > net: Introduce skb ttl field to track packet loops > > net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop > > 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 > > > > Victor Nogueira (1): > > selftests/tc-testing: Add netem/mirred test cases exercising loops > > > > include/linux/skbuff.h | 2 + > > net/sched/act_mirred.c | 45 +- > > 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, 674 insertions(+), 137 deletions(-) > > > > I would also like to add additional guard rails here for netem, and you > might consider the same for mirred. > > 1. Do not allow duplicate option outside of root namespace. > 2. Cap the duplicate fraction at some rationale lower bound like 25% I never understood that logic: don't allow root because it might cause a security bug? here's a different view: See: https://lore.kernel.org/netdev/20251201162524.18c919fd@kernel.org/#t cheers, jamal ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <CAOaVG17Jp8QB_=r3-eWM6bvrLAixFuj+wYB_zv5+OZfiY0LyYA@mail.gmail.com>]
* Re: [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem [not found] ` <CAOaVG17Jp8QB_=r3-eWM6bvrLAixFuj+wYB_zv5+OZfiY0LyYA@mail.gmail.com> @ 2026-03-19 1:08 ` William Liu 0 siblings, 0 replies; 19+ messages in thread From: William Liu @ 2026-03-19 1:08 UTC (permalink / raw) To: Stephen Hemminger Cc: Jamal Hadi Salim, netdev, David Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, horms, Jiri Pirko, Victor Nogueira, xmei5, Pedro Tammela, Savino Dicanosa, kuniyu, willemdebruijnkernel, toke@toke.dk I am of the same opinion as Jamal here. If the root cause is addressed, then I don't think there is a need to add additional restrictions because of a past bug. On Wednesday, March 18th, 2026 at 11:16 PM, Stephen Hemminger <stephen@networkplumber.org> wrote: > Only want to block duplicate flag, all other usage is fine > > On Wed, Mar 18, 2026, 12:41 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > > On Tue, Mar 17, 2026 at 7:36 PM Stephen Hemminger > > <stephen@networkplumber.org> wrote: > > > > > > On Mon, 16 Mar 2026 17:10:46 -0400 > > > Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > > > > > > We introduce a 2-bit global skb->ttl counter. Patch #1 describes how we put > > > > together those bits. Patches #2 and patch #5 use these bits. > > > > I added Fixes tags to patch #1 in case it is useful for backporting. > > > > Patch #3 and #4 revert William's earlier netem commits. Patch #6 introduces > > > > tdc test cases. > > > > > > > > Changes in v2: > > > > Do not reuse skb->from_ingress (which was move skb->cb) > > > > > > > > Jamal Hadi Salim (5): > > > > net: Introduce skb ttl field to track packet loops > > > > net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop > > > > 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 > > > > > > > > Victor Nogueira (1): > > > > selftests/tc-testing: Add netem/mirred test cases exercising loops > > > > > > > > include/linux/skbuff.h | 2 + > > > > net/sched/act_mirred.c | 45 +- > > > > 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, 674 insertions(+), 137 deletions(-) > > > > > > > > > > I would also like to add additional guard rails here for netem, and you > > > might consider the same for mirred. > > > > > > 1. Do not allow duplicate option outside of root namespace. > > > 2. Cap the duplicate fraction at some rationale lower bound like 25% > > > > I never understood that logic: don't allow root because it might cause > > a security bug? > > here's a different view: > > See: https://lore.kernel.org/netdev/20251201162524.18c919fd@kernel.org/#t > > > > cheers, > > jamal ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-03-23 23:14 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-16 21:10 [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Jamal Hadi Salim
2026-03-16 21:10 ` [PATCH net v2 1/6] net: Introduce skb ttl field to track packet loops Jamal Hadi Salim
2026-03-16 21:10 ` [PATCH net v2 2/6] net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop Jamal Hadi Salim
2026-03-16 21:10 ` [PATCH net v2 3/6] Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" Jamal Hadi Salim
2026-03-17 0:54 ` Stephen Hemminger
2026-03-16 21:10 ` [PATCH net v2 4/6] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" Jamal Hadi Salim
2026-03-17 0:55 ` Stephen Hemminger
2026-03-18 19:26 ` Jamal Hadi Salim
2026-03-16 21:10 ` [PATCH net v2 5/6] net/sched: fix packet loop on netem when duplicate is on Jamal Hadi Salim
2026-03-17 0:57 ` Stephen Hemminger
2026-03-18 19:34 ` Jamal Hadi Salim
2026-03-19 1:25 ` William Liu
2026-03-23 23:14 ` Stephen Hemminger
2026-03-23 19:33 ` Jamal Hadi Salim
2026-03-16 21:10 ` [PATCH net v2 6/6] selftests/tc-testing: Add netem/mirred test cases exercising loops Jamal Hadi Salim
2026-03-17 0:58 ` Stephen Hemminger
2026-03-17 23:36 ` [PATCH net v2 0/6] net/sched: Fix packet loops in mirred and netem Stephen Hemminger
2026-03-18 19:41 ` Jamal Hadi Salim
[not found] ` <CAOaVG17Jp8QB_=r3-eWM6bvrLAixFuj+wYB_zv5+OZfiY0LyYA@mail.gmail.com>
2026-03-19 1:08 ` William Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox