Hi Cong Wang,
On 06/20/2017 12:54 PM, Cong Wang wrote:
Hello,
On Mon, Jun 19, 2017 at 8:15 PM, jeffy <jeffy.c...@rock-chips.com> wrote:
but actually they are not guaranteed to be paired:
the netdev_run_todo(see the first dump stack above) would call
netdev_wait_allrefs to rebroadcast unregister notification multiple times,
unless timed out or all of the "struct net_device"'s refs released:
* This is called when unregistering network devices.
*
* Any protocol or device that holds a reference should register
* for netdevice notification, and cleanup and put back the
* reference if they receive an UNREGISTER event.
* We can get stuck here if buggy protocols don't correctly
* call dev_put.
*/
static void netdev_wait_allrefs(struct net_device *dev)
{
...
while (refcnt != 0) {
if (time_after(jiffies, rebroadcast_time + 1 * HZ)) {
rtnl_lock();
/* Rebroadcast unregister notification */
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
__rtnl_unlock();
rcu_barrier();
rtnl_lock();
call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
Interesting, I didn't notice this corner-case, because normally
we would hit the one in rollback_registered_many(). Probably
we need to add a check
if (dev->reg_state == NETREG_UNREGISTERING)
in ip6_route_dev_notify(). Can you give it a try?
the NETREG_UNREGISTERING check works for my test:)
but i saw dev_change_net_namespace also call NETDEV_UNREGISTER &
NETDEV_REGISTER:
int dev_change_net_namespace(struct net_device *dev, struct net *net,
const char *pat)
{
...
/* Don't allow namespace local devices to be moved. */
err = -EINVAL;
if (dev->features & NETIF_F_NETNS_LOCAL)
goto out;
/* Ensure the device has been registrered */
if (dev->reg_state != NETREG_REGISTERED)
goto out;
...
/* Notify protocols, that we are about to destroy
* this device. They should clean all the things.
*
* Note that dev->reg_state stays at NETREG_REGISTERED.
* This is wanted because this way 8021q and macvlan know
* the device is just moving and can keep their slaves up.
*/
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
...
/* Notify protocols, that a new device appeared. */
call_netdevice_notifiers(NETDEV_REGISTER, dev);
maybe we should also add a check for NETDEV_REGISTER event? maybe:
if (dev->reg_state != NETREG_REGISTERED)
I guess we probably need to revise other NETDEV_UNREGISTER
handlers too.
I will send a patch tomorrow.
sounds great~
Thanks!
