On Sat, Feb 6, 2016 at 10:58 AM, Julian Anastasov <j...@ssi.bg> wrote:
> > I now see that we should split the loop > here, so that NETDEV_DOWN_BATCH is called only once per net: > > bool down = false; > > for_each_netdev(net, dev) { > if (dev == last) > break; > > if (dev->flags & IFF_UP) { > call_netdevice_notifier(nb, NETDEV_GOING_DOWN, > dev); > call_netdevice_notifier(nb, NETDEV_DOWN, dev); > down = true; > } > } > > rt_cache_flush and arp_ifdown_all will be called > on NETDEV_UNREGISTER_BATCH, so use 'down' flag: > > if (down) > call_netdevice_notifier(nb, NETDEV_DOWN_BATCH, > net->loopback_dev); > for_each_netdev(net, dev) { > if (dev == last) > goto outroll; > call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev); > } > call_netdevice_notifier(nb, NETDEV_UNREGISTER_BATCH, > net->loopback_dev); > > >> } >> call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev); >> } >> + call_netdevice_notifier(nb, NETDEV_UNREGISTER_BATCH, >> + net->loopback_dev); >> } >> >> outroll: >> + call_netdevice_notifier(nb, NETDEV_UNREGISTER_BATCH, last); >> raw_notifier_chain_unregister(&netdev_chain, nb); >> goto unlock; >> } I am not sure we need to worry too much about optimizing the failure code path to justify splitting into two loops. >> static void rollback_registered_many(struct list_head *head) >> { >> list_for_each_entry(dev, head, unreg_list) { >> - struct sk_buff *skb = NULL; >> - >> /* Shutdown queueing discipline. */ >> dev_shutdown(dev); >> >> @@ -6475,6 +6497,20 @@ static void rollback_registered_many(struct list_head >> *head) >> this device. They should clean all the things. >> */ >> call_netdevice_notifiers(NETDEV_UNREGISTER, dev); >> + } >> + >> + /* call batch notifiers which act on net namespaces */ >> + list_for_each_entry(dev, head, unreg_list) { >> + net_add_event_list(&net_head, dev_net(dev)); > > Looks like we can move the above net_add_event_list with > the comment into the previous loop after NETDEV_UNREGISTER, > we will save some cycles. > I didn't move it into the previous loop because the NETDEV_UNREGISTER notifier can end up calling rollback_registered_many (for example the vlan driver unregistering all vlans on top of a device) in which case we would be using the event_list in the net namespace. > Julian Anastasov <j...@ssi.bg> Thanks, Salam