netdev - Re: [PATCH 1/2] net: dsa: RCU-protect dsa_ptr in struct net

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240917103041.1645536-1-gur.stavi@huawei.com>
Date: Tue, 17 Sep 2024 13:30:41 +0300
From: Gur Stavi <gur.stavi@...wei.com>
To: <alexander.sverdlin@...mens.com>
CC: <Mark-MC.Lee@...iatek.com>, <UNGLinuxDriver@...rochip.com>,
	<alexandre.belloni@...tlin.com>, <andrew@...n.ch>,
	<angelogioacchino.delregno@...labora.com>, <arinc.unal@...nc9.com>,
	<bcm-kernel-feedback-list@...adcom.com>, <bridge@...ts.linux.dev>,
	<claudiu.manoil@....com>, <daniel@...rotopia.org>, <davem@...emloft.net>,
	<dqfext@...il.com>, <edumazet@...gle.com>, <f.fainelli@...il.com>,
	<gur.stavi@...wei.com>, <kuba@...nel.org>,
	<linux-mediatek@...ts.infradead.org>, <lorenzo@...nel.org>,
	<matthias.bgg@...il.com>, <nbd@....name>, <netdev@...r.kernel.org>,
	<olteanv@...il.com>, <pabeni@...hat.com>, <razor@...ckwall.org>,
	<roopa@...dia.com>, <sean.wang@...iatek.com>, <stable@...r.kernel.org>
Subject: Re: [PATCH 1/2] net: dsa: RCU-protect dsa_ptr in struct net_device

> Hello Gur!
>
> On Tue, 2024-09-17 at 11:10 +0300, Gur Stavi wrote:
> > > @@ -1594,10 +1592,11 @@ void dsa_switch_shutdown(struct dsa_switch *ds)
> > >   	}
> > >
> > >   	/* Disconnect from further netdevice notifiers on the conduit,
> > > -	 * since netdev_uses_dsa() will now return false.
> > > +	 * from now on, netdev_uses_dsa_currently() will return false.
> > >   	 */
> > >   	dsa_switch_for_each_cpu_port(dp, ds)
> > > -		dp->conduit->dsa_ptr = NULL;
> > > +		rcu_assign_pointer(dp->conduit->dsa_ptr, NULL);
> > > +	synchronize_rcu();
> > >
> > >   	rtnl_unlock();
> > >   out:
> >
> > Hi, I am a newbie here. Thanks for the opportunity for learning more
> > about rcu.
> > Wouldn't it make more sense to call synchronize_rcu after rtnl_unlock?
>
> This is indeed a question which is usually resolved other way around
> (making locked section shorter), but in this particular case I thought that:
> - we actually don't need giving rtnl lock sooner, which would potentially
>   make synchronize_rcu() call longer (if another thread manages to wake up
>   and claim the rtnl lock before synchronize_rcu())
> - we are in shutdown phase, we don't need to minimize lock contention, we
>   need to minimize the overall shutdown time

But isn't shutdown still multithreaded?
10 threads may have similar shutdown task: remove objects from different
rcu protected data structures while holding rtnl. Then synchronize RCU
and release the objects.
Synchronizing RCU from within the lock will completely serialize all
threads and postpone shutdown whereas outside the lock multiple
synchronize_rcu could run in parallel.