* Requirements for a shutdown function?
@ 2017-05-09 16:58 Timur Tabi
2017-05-09 18:46 ` Florian Fainelli
0 siblings, 1 reply; 8+ messages in thread
From: Timur Tabi @ 2017-05-09 16:58 UTC (permalink / raw)
To: netdev
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.
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?
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?
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Requirements for a shutdown function?
2017-05-09 16:58 Requirements for a shutdown function? Timur Tabi
@ 2017-05-09 18:46 ` Florian Fainelli
2017-05-09 18:51 ` Timur Tabi
0 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2017-05-09 18:46 UTC (permalink / raw)
To: Timur Tabi, netdev
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Requirements for a shutdown function?
2017-05-09 18:46 ` Florian Fainelli
@ 2017-05-09 18:51 ` Timur Tabi
2017-05-09 19:06 ` Florian Fainelli
0 siblings, 1 reply; 8+ messages in thread
From: Timur Tabi @ 2017-05-09 18:51 UTC (permalink / raw)
To: Florian Fainelli, netdev
On 05/09/2017 01:46 PM, Florian Fainelli wrote:
> A good test case for exercising a .shutdown() function is kexec'ing a
> new kernel for instance.
I tried that. I run iperf in one window while launching kexec in another.
Even without a shutdown function, network traffic appear to halt on its own
and the kexec succeeds.
Is it possible that the network stack detects a kexec and automatically
stops all network devices?
> 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.
My in-house driver stops the RX and TX queues. I'm guessing that's good
enough, but I don't have a failing test case to prove it.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Requirements for a shutdown function?
2017-05-09 18:51 ` Timur Tabi
@ 2017-05-09 19:06 ` Florian Fainelli
2017-05-10 20:17 ` Timur Tabi
0 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2017-05-09 19:06 UTC (permalink / raw)
To: Timur Tabi, netdev
On 05/09/2017 11:51 AM, Timur Tabi wrote:
> On 05/09/2017 01:46 PM, Florian Fainelli wrote:
>> A good test case for exercising a .shutdown() function is kexec'ing a
>> new kernel for instance.
>
> I tried that. I run iperf in one window while launching kexec in another.
> Even without a shutdown function, network traffic appear to halt on its own
> and the kexec succeeds.
>
> Is it possible that the network stack detects a kexec and automatically
> stops all network devices?
No. why would it? However the device driver model does call into your
driver's remove function and that one does a right job already because
it does an network device unregister, and so on.
There is no strict requirement for implementing a .shutdown() function
AFAICT and it does not necessarily make sense to have one depending on
the bus type. For platform/MMIO devices, it hardly has any value, but on
e.g: PCI, it could be added as an additional step to perform a full
device shutdown.
>
>> 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.
>
> My in-house driver stops the RX and TX queues. I'm guessing that's good
> enough, but I don't have a failing test case to prove it.
>
That's probably good enough, yes.
--
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Requirements for a shutdown function?
2017-05-09 19:06 ` Florian Fainelli
@ 2017-05-10 20:17 ` Timur Tabi
2017-05-10 21:47 ` Florian Fainelli
0 siblings, 1 reply; 8+ messages in thread
From: Timur Tabi @ 2017-05-10 20:17 UTC (permalink / raw)
To: Florian Fainelli, netdev
On 05/09/2017 02:06 PM, Florian Fainelli wrote:
> On 05/09/2017 11:51 AM, Timur Tabi wrote:
>> Is it possible that the network stack detects a kexec and automatically
>> stops all network devices?
>
> No. why would it? However the device driver model does call into your
> driver's remove function and that one does a right job already because
> it does an network device unregister, and so on.
I ran some more tests. When I launch kexec, the driver's
net_device_ops.ndo_stop function is called, which already stops the interface.
So it looks to me as if the network stack does close the interface during a
kexec. With the interface closed, there's no point in having a shutdown
function, is there?
>> My in-house driver stops the RX and TX queues. I'm guessing that's good
>> enough, but I don't have a failing test case to prove it.
>>
>
> That's probably good enough, yes.
Except that it turns out that the queues are already stopped by then because
the emac_close() function has already been called.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Requirements for a shutdown function?
2017-05-10 20:17 ` Timur Tabi
@ 2017-05-10 21:47 ` Florian Fainelli
2017-05-10 22:11 ` Timur Tabi
0 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2017-05-10 21:47 UTC (permalink / raw)
To: Timur Tabi, netdev
On 05/10/2017 01:17 PM, Timur Tabi wrote:
> On 05/09/2017 02:06 PM, Florian Fainelli wrote:
>> On 05/09/2017 11:51 AM, Timur Tabi wrote:
>
>>> Is it possible that the network stack detects a kexec and automatically
>>> stops all network devices?
>>
>> No. why would it? However the device driver model does call into your
>> driver's remove function and that one does a right job already because
>> it does an network device unregister, and so on.
>
> I ran some more tests. When I launch kexec, the driver's
> net_device_ops.ndo_stop function is called, which already stops the interface.
>
> So it looks to me as if the network stack does close the interface during a
> kexec. With the interface closed, there's no point in having a shutdown
> function, is there?
AFAIR kexec takes care of shutting down network devices explicitly
(unless instructed otherwise with -x/--no-ifdown) so this may be where
this is coming from.
Reading through drivers/base/core.c it does not appear that ->remove()
is called and then ->shutdown() gets called, only ->shutdown() gets
called from device_shutdown() called from kernel/reboot.c. It seems to
me like if you want to be on the safe side you would want to implement a
shutdown function that is identical to what your remove function does.
>
>>> My in-house driver stops the RX and TX queues. I'm guessing that's good
>>> enough, but I don't have a failing test case to prove it.
>>>
>>
>> That's probably good enough, yes.
>
> Except that it turns out that the queues are already stopped by then because
> the emac_close() function has already been called.
--
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Requirements for a shutdown function?
2017-05-10 21:47 ` Florian Fainelli
@ 2017-05-10 22:11 ` Timur Tabi
2017-05-10 22:17 ` Florian Fainelli
0 siblings, 1 reply; 8+ messages in thread
From: Timur Tabi @ 2017-05-10 22:11 UTC (permalink / raw)
To: Florian Fainelli, netdev
On 05/10/2017 04:47 PM, Florian Fainelli wrote:
> AFAIR kexec takes care of shutting down network devices explicitly
> (unless instructed otherwise with -x/--no-ifdown) so this may be where
> this is coming from.
>
> Reading through drivers/base/core.c it does not appear that ->remove()
> is called and then ->shutdown() gets called, only ->shutdown() gets
> called from device_shutdown() called from kernel/reboot.c. It seems to
> me like if you want to be on the safe side you would want to implement a
> shutdown function that is identical to what your remove function does.
I finally found a testcase where the shutdown function is useful. If you do
a "reboot -f", it will call shutdown but not close.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Requirements for a shutdown function?
2017-05-10 22:11 ` Timur Tabi
@ 2017-05-10 22:17 ` Florian Fainelli
0 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2017-05-10 22:17 UTC (permalink / raw)
To: Timur Tabi, netdev
On 05/10/2017 03:11 PM, Timur Tabi wrote:
> On 05/10/2017 04:47 PM, Florian Fainelli wrote:
>> AFAIR kexec takes care of shutting down network devices explicitly
>> (unless instructed otherwise with -x/--no-ifdown) so this may be where
>> this is coming from.
>>
>> Reading through drivers/base/core.c it does not appear that ->remove()
>> is called and then ->shutdown() gets called, only ->shutdown() gets
>> called from device_shutdown() called from kernel/reboot.c. It seems to
>> me like if you want to be on the safe side you would want to implement a
>> shutdown function that is identical to what your remove function does.
>
> I finally found a testcase where the shutdown function is useful. If you do
> a "reboot -f", it will call shutdown but not close.
Correct yes. Sorry, I did not recall which one of kexec or reboot would
call it, but both would actually now that I looked at what happens on
one of my systems again.
--
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-05-10 22:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-09 16:58 Requirements for a shutdown function? Timur Tabi
2017-05-09 18:46 ` Florian Fainelli
2017-05-09 18:51 ` Timur Tabi
2017-05-09 19:06 ` Florian Fainelli
2017-05-10 20:17 ` Timur Tabi
2017-05-10 21:47 ` Florian Fainelli
2017-05-10 22:11 ` Timur Tabi
2017-05-10 22:17 ` Florian Fainelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).