netdev - Re: Requirements for a shutdown function?

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]

Date:   Tue, 9 May 2017 11:46:40 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Timur Tabi <timur@...eaurora.org>, netdev@...r.kernel.org
Subject: Re: Requirements for a shutdown function?

On 05/09/2017 09:58 AM, Timur Tabi wrote:
> I'm trying to add a platform_driver.shutdown function to my Ethernet driver
> (drivers/net/ethernet/qualcomm/emac/*), but I can't find any definitive
> information as to what a network driver shutdown callback is supposed to do.
>  I also don't know what testcase I should use to verify that my function is
> working.

A good test case for exercising a .shutdown() function is kexec'ing a
new kernel for instance.

> 
> I see only four instances of a platform_driver.shutdown function in
> drivers/net/ethernet:
> 
> $ git grep -A 20 -w platform_driver | grep '\.shutdown'
> apm/xgene-v2/main.c-	.shutdown = xge_shutdown,
> apm/xgene/xgene_enet_main.c-	.shutdown = xgene_enet_shutdown,
> marvell/mv643xx_eth.c-	.shutdown	= mv643xx_eth_shutdown,
> marvell/pxa168_eth.c-	.shutdown = pxa168_eth_shutdown,
> 
> (Other shutdown functions are for pci_driver.shutdown).
> 
> For the xgene drivers, the shutdown function just calls the 'remove'
> function.  Isn't that overkill?  Why bother with a shutdown function if it's
> just the same thing as removing the driver outright?

Yes, that appears unnecessary.

> 
> mv643xx_eth_shutdown() seems more reasonable.  All it does is halt the TX
> and RX queues.
> 
> pxa168_eth_shutdown() is a little more heavyweight: halts the queues, and
> stops the DMA and calls phy_stop().
> 
> Can anyone help me figure out what my driver really should do?

You should put your HW in a state where it won't be doing DMA, or have
any adverse side effects to the system, putting it in a low power state
is also a good approach.
-- 
Florian