netdev - Re: tc: network egress frozen during qdisc update with debug kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250320-demonic-marmoset-of-economy-bba7ed@leitao>
Date: Thu, 20 Mar 2025 11:52:56 -0700
From: Breno Leitao <leitao@...ian.org>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: "Paul E. McKenney" <paulmck@...nel.org>, longman@...hat.com,
	bvanassche@....org, Eric Dumazet <edumazet@...gle.com>,
	kuba@...nel.org, xiyou.wangcong@...il.com, jiri@...nulli.us,
	kuniyu@...zon.com, rcu@...r.kernel.org, kasan-dev@...glegroups.com,
	netdev@...r.kernel.org
Subject: Re: tc: network egress frozen during qdisc update with debug kernel

Hello Jamal,

On Wed, Mar 19, 2025 at 05:05:08PM -0400, Jamal Hadi Salim wrote:
> On Wed, Mar 19, 2025 at 2:12 PM Breno Leitao <leitao@...ian.org> wrote:
> >
> > On Wed, Mar 19, 2025 at 09:05:07AM -0700, Paul E. McKenney wrote:
> >
> > > > I think we should redesign lockdep_unregister_key() to work on a separately
> > > > allocated piece of memory,
> > > > then use kfree_rcu() in it.
> > > >
> > > > Ie not embed a "struct lock_class_key" in the struct Qdisc, but a pointer to
> > > >
> > > > struct ... {
> > > >      struct lock_class_key;
> > > >      struct rcu_head  rcu;
> > > > }
> > >
> > > Works for me!
> >
> > I've tested a different approach, using synchronize_rcu_expedited()
> > instead of synchronize_rcu(), given how critical this function is
> > called, and the command performance improves dramatically.
> >
> > This approach has some IPI penalties, but, it might be quicker to review
> > and get merged, mitigating the network issue.
> >
> > Does it sound a bad approach?
> >
> > Date:   Wed Mar 19 10:23:56 2025 -0700
> >
> >     lockdep: Speed up lockdep_unregister_key() with expedited RCU synchronization
> >
> >     lockdep_unregister_key() is called from critical code paths, including
> >     sections where rtnl_lock() is held. When replacing a qdisc in a network
> >     device, network egress traffic is disabled while __qdisc_destroy() is
> >     called for every queue. This function calls lockdep_unregister_key(),
> >     which was blocked waiting for synchronize_rcu() to complete.
> >
> >     For example, a simple tc command to replace a qdisc could take 13
> >     seconds:
> >
> >       # time /usr/sbin/tc qdisc replace dev eth0 root handle 0x1234: mq
> >         real    0m13.195s
> >         user    0m0.001s
> >         sys     0m2.746s
> >
> 
> Could you please add the "after your change"  output as well?

Sure. I will send the official patch tomorrow, and I will update it with
the information as well.

Thanks
--breno