All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: netdev@vger.kernel.org, William Liu <will@willsroot.io>,
	Savino Dicanosa <savy@syst3mfailure.io>
Subject: Re: [Patch net v6 4/8] net_sched: Implement the right netem duplication behavior
Date: Tue, 30 Dec 2025 09:28:50 -0800	[thread overview]
Message-ID: <20251230092850.43251a09@phoenix.local> (raw)
In-Reply-To: <20251227194135.1111972-5-xiyou.wangcong@gmail.com>

On Sat, 27 Dec 2025 11:41:31 -0800
Cong Wang <xiyou.wangcong@gmail.com> wrote:

> In the old behavior, duplicated packets were sent back to the root qdisc,
> which could create dangerous infinite loops in hierarchical setups -
> imagine a scenario where each level of a multi-stage netem hierarchy kept
> feeding duplicates back to the top, potentially causing system instability
> or resource exhaustion.
> 
> The new behavior elegantly solves this by enqueueing duplicates to the same
> qdisc that created them, ensuring that packet duplication occurs exactly
> once per netem stage in a controlled, predictable manner. This change
> enables users to safely construct complex network emulation scenarios using
> netem hierarchies (like the 4x multiplication demonstrated in testing)
> without worrying about runaway packet generation, while still preserving
> the intended duplication effects.
> 
> Another advantage of this approach is that it eliminates the enqueue reentrant
> behaviour which triggered many vulnerabilities. See the last patch in this
> patchset which updates the test cases for such vulnerabilities.
> 
> Now users can confidently chain multiple netem qdiscs together to achieve
> sophisticated network impairment combinations, knowing that each stage will
> apply its effects exactly once to the packet flow, making network testing
> scenarios more reliable and results more deterministic.
> 
> I tested netem packet duplication in two configurations:
> 1. Nest netem-to-netem hierarchy using parent/child attachment
> 2. Single netem using prio qdisc with netem leaf
> 
> Setup commands and results:
> 
> Single netem hierarchy (prio + netem):
>   tc qdisc add dev lo root handle 1: prio bands 3 priomap 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
>   tc filter add dev lo parent 1:0 protocol ip matchall classid 1:1
>   tc qdisc add dev lo parent 1:1 handle 10: netem limit 4 duplicate 100%
> 
> Result: 2x packet multiplication (1→2 packets)
>   2 echo requests + 4 echo replies = 6 total packets
> 
> Expected behavior: Only one netem stage exists in this hierarchy, so
> 1 ping becomes 2 packets (100% duplication). The 2 echo requests generate
> 2 echo replies, which also get duplicated to 4 replies, yielding the
> predictable total of 6 packets (2 requests + 4 replies).
> 
> Nest netem hierarchy (netem + netem):
>   tc qdisc add dev lo root handle 1: netem limit 1000 duplicate 100%
>   tc qdisc add dev lo parent 1: handle 2: netem limit 1000 duplicate 100%
> 
> Result: 4x packet multiplication (1→2→4 packets)
>   4 echo requests + 16 echo replies = 20 total packets
> 
> Expected behavior: Root netem duplicates 1 ping to 2 packets, child netem
> receives 2 packets and duplicates each to create 4 total packets. Since
> ping operates bidirectionally, 4 echo requests generate 4 echo replies,
> which also get duplicated through the same hierarchy (4→8→16), resulting
> in the predictable total of 20 packets (4 requests + 16 replies).
> 
> The new netem duplication behavior does not break the documented
> semantics of "creates a copy of the packet before queuing." The man page
> description remains true since duplication occurs before the queuing
> process, creating both original and duplicate packets that are then
> enqueued. The documentation does not specify which qdisc should receive
> the duplicates, only that copying happens before queuing. The implementation
> choice to enqueue duplicates to the same qdisc (rather than root) is an
> internal detail that maintains the documented behavior while preventing
> infinite loops in hierarchical configurations.
> 
> Fixes: 0afb51e72855 ("[PKT_SCHED]: netem: reinsert for duplication")
> Reported-by: William Liu <will@willsroot.io>
> Reported-by: Savino Dicanosa <savy@syst3mfailure.io>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

It is worth testing for the case where netem is used as a leaf qdisc.
I worry that this could cause the parent qdisc to get accounting wrong.
I.e if HTB calls netem and netem queues 2 packets, the qlen in HTB
would be incorrect.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

  reply	other threads:[~2025-12-30 17:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-27 19:41 [Patch net v6 0/8] netem: Fix skb duplication logic and prevent infinite loops Cong Wang
2025-12-27 19:41 ` [Patch net v6 1/8] net_sched: Check the return value of qfq_choose_next_agg() Cong Wang
2025-12-30 17:24   ` Stephen Hemminger
2025-12-27 19:41 ` [Patch net v6 2/8] Revert "net/sched: Restrict conditions for adding duplicating netems to qdisc tree" Cong Wang
2025-12-27 19:41 ` [Patch net v6 3/8] Revert "selftests/tc-testing: Add tests for restrictions on netem duplication" Cong Wang
2025-12-27 19:41 ` [Patch net v6 4/8] net_sched: Implement the right netem duplication behavior Cong Wang
2025-12-30 17:28   ` Stephen Hemminger [this message]
2026-01-04 19:19     ` Jakub Kicinski
2026-01-07 23:33     ` Cong Wang
2026-01-08  2:18       ` Jakub Kicinski
2025-12-27 19:41 ` [Patch net v6 5/8] selftests/tc-testing: Add a nested netem duplicate test Cong Wang
2025-12-27 19:41 ` [Patch net v6 6/8] selftests/tc-testing: Add a test case for piro with netem duplicate Cong Wang
2026-01-04 19:15   ` Jakub Kicinski
2026-01-07 23:34     ` Cong Wang
2025-12-27 19:41 ` [Patch net v6 7/8] selftests/tc-testing: Add a test case for mq " Cong Wang
2025-12-27 19:41 ` [Patch net v6 8/8] selftests/tc-testing: Update test cases " Cong Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251230092850.43251a09@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=netdev@vger.kernel.org \
    --cc=savy@syst3mfailure.io \
    --cc=will@willsroot.io \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.