public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Cong Wang <xiyou.wangcong@gmail.com>
To: Paolo Abeni <pabeni@redhat.com>
Cc: William Liu <will@willsroot.io>,
	netdev@vger.kernel.org, jhs@mojatatu.com, victor@mojatatu.com,
	pctammela@mojatatu.com, kuba@kernel.org,
	stephen@networkplumber.org, dcaratti@redhat.com,
	savy@syst3mfailure.io, jiri@resnulli.us, davem@davemloft.net,
	edumazet@google.com, horms@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: This breaks netem use cases
Date: Thu, 10 Jul 2025 22:44:45 -0700	[thread overview]
Message-ID: <aHCkzdhBHB8Noerp@pop-os.localdomain> (raw)
In-Reply-To: <9ea58b38-921c-45a0-85cc-a586a6857eb1@redhat.com>

On Thu, Jul 10, 2025 at 10:26:46AM +0200, Paolo Abeni wrote:
> On 7/8/25 9:42 PM, Cong Wang wrote:
> > (Cc LKML for more audience, since this clearly breaks potentially useful
> > use cases)
> > 
> > On Tue, Jul 08, 2025 at 04:43:26PM +0000, William Liu wrote:
> >> 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 [1].
> >> Ensure that a duplicating netem cannot exist in a tree with other
> >> netems.
> > 
> > As I already warned in your previous patchset, this breaks the following
> > potentially useful use case:
> > 
> > sudo tc qdisc add dev eth0 root handle 1: mq
> > sudo tc qdisc add dev eth0 parent 1:1 handle 10: netem duplicate 100%
> > sudo tc qdisc add dev eth0 parent 1:2 handle 20: netem duplicate 100%
> > 
> > I don't see any logical problem of such use case, therefore we should
> > consider it as valid, we can't break it.
> 
> My understanding is that even the solution you proposed breaks a
> currently accepted configuration:
> 
> https://lore.kernel.org/netdev/CAM0EoMmBdZBzfUAms5-0hH5qF5ODvxWfgqrbHaGT6p3-uOD6vg@mail.gmail.com/

Maybe it is not obvious, my patch does not reject users' setup. It
probably has bugs, I am more than just happy to address any bugs in the
next iteration (like for any patch), in fact it is my obligation.

My appologize if I misled any of you to believe my patch is bug-free or
perfect, it is never the case.

For Jamal's patch, it is his intention to break users' setup, and this
won't change during any iteration.

They are significantly different.

> 
> I call them (both the linked one and the inline one) 'configurations'
> instead of 'use-cases' because I don't see how any of them could have
> real users, other than: https://xkcd.com/1172/.

Please let me know if you have any other way to use netem duplication on
a multiqueue NIC _directly_ without worrying about the global spinlock.

I bet you have none. Either you need to use it indirectly (attaching to
a non-root) or you have to face the global spinlock (aka without using
mq).

I am open to your education. :)

Thanks a lot!

  reply	other threads:[~2025-07-11  5:44 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-08 16:43 [PATCH net v5 1/2] net/sched: Restrict conditions for adding duplicating netems to qdisc tree William Liu
2025-07-08 16:44 ` [PATCH net v5 2/2] selftests/tc-testing: Add tests for restrictions on netem duplication William Liu
2025-07-08 18:50   ` Jamal Hadi Salim
2025-07-08 18:50 ` [PATCH net v5 1/2] net/sched: Restrict conditions for adding duplicating netems to qdisc tree Jamal Hadi Salim
2025-07-08 19:42 ` This breaks netem use cases Cong Wang
2025-07-08 20:35   ` Jamal Hadi Salim
2025-07-08 21:32     ` Cong Wang
2025-07-08 22:26       ` Jamal Hadi Salim
2025-07-08 22:45         ` Stephen Hemminger
2025-07-11  5:19         ` Cong Wang
2025-07-10  8:26   ` Paolo Abeni
2025-07-11  5:44     ` Cong Wang [this message]
2025-07-11 22:55 ` [PATCH net v5 1/2] net/sched: Restrict conditions for adding duplicating netems to qdisc tree Jakub Kicinski
2025-07-12 16:51   ` William Liu
2025-07-14 14:58     ` Jakub Kicinski
2025-07-11 23:00 ` patchwork-bot+netdevbpf

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=aHCkzdhBHB8Noerp@pop-os.localdomain \
    --to=xiyou.wangcong@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dcaratti@redhat.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pctammela@mojatatu.com \
    --cc=savy@syst3mfailure.io \
    --cc=stephen@networkplumber.org \
    --cc=victor@mojatatu.com \
    --cc=will@willsroot.io \
    /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