From: Stephen Hemminger <stephen@networkplumber.org>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>,
netdev@vger.kernel.org, will@willsroot.io
Subject: Re: [Patch v4 net 0/6] netem: Fix skb duplication logic and prevent infinite loops
Date: Fri, 1 Aug 2025 15:04:10 -0700 [thread overview]
Message-ID: <20250801150410.396a565e@hermes.local> (raw)
In-Reply-To: <aIpvuNyyvud0sJOl@pop-os.localdomain>
On Wed, 30 Jul 2025 12:17:12 -0700
Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Jul 21, 2025 at 10:00:30AM -0400, Jamal Hadi Salim wrote:
> > On Sat, Jul 19, 2025 at 6:04 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > >
> > > This patchset fixes the infinite loops due to duplication in netem, the
> > > real root cause of this problem is enqueuing to the root qdisc, which is
> > > now changed to enqueuing to the same qdisc. This is more reasonable,
> > > more predictable from users' perspective, less error-proone and more elegant.
> > >
> > > Please see more details in patch 1/6 which contains two pages of detailed
> > > explanation including why it is safe and better.
> > >
> > > This replaces the patches from William, with much less code and without
> > > any workaround. More importantly, this does not break any use case.
> > >
> >
> > Cong, you are changing user expected behavior.
> > So instead of sending to the root qdisc, you are looping on the same
> > qdisc. I dont recall what the history is for the decision to go back
> > to the root qdisc - but one reason that sounds sensible is we want to
> > iterate through the tree hierarchy again. Stephen may remember.
> > The fact that the qfq issue is hit indicates the change has
> > consequences - and given the check a few lines above, more than likely
> > you are affecting the qlen by what you did.
>
> Please refer the changelog of patch 1/6, let me quote it here for you:
>
> 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.
>
> I think it is reasonable to use man page as our agreement with users. I
> am open to other alternative agreements, if you have one. I hope using
> man page is not of my own preference here.
>
> Thanks.
There is a chance that cases that mix duplication, corruption and drop
parameters would see different results after this patch.
prev parent reply other threads:[~2025-08-01 22:04 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-19 22:03 [Patch v4 net 0/6] netem: Fix skb duplication logic and prevent infinite loops Cong Wang
2025-07-19 22:03 ` [Patch v4 net 1/6] net_sched: Implement the right netem duplication behavior Cong Wang
2025-07-19 22:03 ` [Patch v4 net 2/6] net_sched: Check the return value of qfq_choose_next_agg() Cong Wang
2025-07-20 23:34 ` Xiang Mei
2025-07-19 22:03 ` [Patch v4 net 3/6] selftests/tc-testing: Add a nested netem duplicate test Cong Wang
2025-07-19 22:03 ` [Patch v4 net 4/6] selftests/tc-testing: Add a test case for piro with netem duplicate Cong Wang
2025-07-19 22:03 ` [Patch v4 net 5/6] selftests/tc-testing: Add a test case for mq " Cong Wang
2025-07-19 22:03 ` [Patch v4 net 6/6] selftests/tc-testing: Update test cases " Cong Wang
2025-07-21 14:00 ` [Patch v4 net 0/6] netem: Fix skb duplication logic and prevent infinite loops Jamal Hadi Salim
2025-07-21 14:27 ` Jamal Hadi Salim
2025-07-21 14:54 ` Stephen Hemminger
2025-07-30 19:17 ` Cong Wang
2025-08-01 22:04 ` Stephen Hemminger [this message]
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=20250801150410.396a565e@hermes.local \
--to=stephen@networkplumber.org \
--cc=jhs@mojatatu.com \
--cc=netdev@vger.kernel.org \
--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.