Re: [PATCH v2 net-next 2/4] net: dev: add batching to net_device notifiers

On Sat, Feb 6, 2016 at 10:58 AM, Julian Anastasov [off-list ref] 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);

quoted

                      }
                      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.

quoted

 static void rollback_registered_many(struct list_head *head)
 {

quoted

      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 [off-list ref]

Thanks,

Salam