netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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.

      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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).