netdev - Re: [RFC PATCH] net: ethtool: do runtime PM outside RTNL

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: <c4404a84e6490295a8aba37bab7d3152c44ff0ba.camel@sipsolutions.net>
Date: Mon, 04 Dec 2023 23:25:46 +0100
From: Johannes Berg <johannes@...solutions.net>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Marc MERLIN <marc@...lins.org>, netdev@...r.kernel.org, Jesse Brandeburg
	 <jesse.brandeburg@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.com>, 
	intel-wired-lan@...ts.osuosl.org, Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [RFC PATCH] net: ethtool: do runtime PM outside RTNL

On Mon, 2023-12-04 at 14:22 -0800, Jakub Kicinski wrote:
> On Mon, 04 Dec 2023 22:32:25 +0100 Johannes Berg wrote:
> > Well, I was hoping that
> > 
> >  (a) ethtool folks / Jakub would comment if this makes sense, but I
> >      don't see a good reason to do things the other way around (other
> >      than "code is simpler"); and
> 
> My opinion on RPM is pretty uneducated. But taking rtnl_lock to resume
> strikes me as shortsighted. RPM functionality should be fairly
> self-contained, and deserving of a separate lock.
> Or at the very least having looked at the igc RPM code in the past,
> I'm a bit cautious about bending the core to fit it, as it is hardly
> a model...

I could agree with that. The reason it seems to do that is that it
invokes some other ifup/down related code in suspend/resume...

On the other hand it also seems a bit odd to do something as unrelated
to networking as runtime PM (which you can also trigger through various
other paths such as sysfs) under RTNL, holding RTNL for longer than
seems necessary, and creating more contention on it?

johannes