netdev - Re: [PATCH] r8169: do not call rtl8169

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: <jikqc7fz4nmwd3ol4f2uazcjc3zgtbtzcrudhsccmvfm3pjbfk@mkcj6gnkrljj>
Date: Fri, 18 Apr 2025 22:49:54 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Niklas Cassel <cassel@...nel.org>
Cc: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>, 
	Heiner Kallweit <hkallweit1@...il.com>, nic_swsd@...ltek.com, Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] r8169: do not call rtl8169_down() twice on shutdown

On Wed, Apr 16, 2025 at 06:03:36PM +0200, Niklas Cassel wrote:
> Hello Krishna Chaitanya,
> 
> On Wed, Apr 16, 2025 at 09:15:19PM +0530, Krishna Chaitanya Chundru wrote:
> > On 4/16/2025 7:43 PM, Niklas Cassel wrote:
> > > 
> > > So perhaps we should hold off with this patch.
> > > 
> > I disagree on this, it might be causing issue with net driver, but we
> > might face some other issues as explained above if we don't call
> > pci_stop_root_bus().
> 
> When I wrote hold off with this patch, I meant the patch in $subject,
> not your patch.
> 
> 
> When it comes to your patch, I think that the commit log needs to explain
> why it is so special.
> 
> Because AFAICT, all other PCIe controller drivers call pci_stop_root_bus()
> in the .remove() callback, not in the .shutdown() callback.
> 

And this driver is special because, it has no 'remove()' callback as it
implements an irqchip controller. So we have to shutdown the devices cleanly in
the 'shutdown' callback.

Also do note that the driver core will not call the 'remove()' callback unless
the driver as a module is unloaded during poweroff/reboot scenarios. So the
controller drivers need to properly power down the devices in their 'shutdown()'
callback IMO.

> Doing things differently from all other PCIe controller drivers is usually
> a red flag.
> 

Yes, even if it is the right thing to do ;) But I agree that the commit message
needs some improvement.

- Mani

-- 
மணிவண்ணன் சதாசிவம்