On Tue, Feb 2, 2016 at 12:55 PM, Julian Anastasov <[email protected]> wrote:
>
> Hello,
>
> On Mon, 1 Feb 2016, Salam Noureddine wrote:
>
>> @@ -1572,8 +1582,12 @@ rollback:
>> call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev);
>> + call_netdevice_notifier(nb, NETDEV_UNREGISTER_BATCH,
>> + dev);
>
> If the rule is once per net, the above call...
>
>> }
>
> should be here:
>
> call_netdevice_notifier(nb, NETDEV_UNREGISTER_BATCH,
> net->loopback_dev);
>
> and also once after outroll label?:
That's a good optimization to add. I was mostly focusing on the device
unregister path.
>
> call_netdevice_notifier(nb, NETDEV_UNREGISTER_BATCH, last);
>
>> }
>>
>> @@ -1614,8 +1628,12 @@ int unregister_netdevice_notifier(struct
>> notifier_block *nb)
>> call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev);
>> + call_netdevice_notifier(nb, NETDEV_UNREGISTER_BATCH,
>> + dev);
>
> Above call...
>
>> }
>
> should be here, for net->loopback_dev?
> Also, is it ok to call NETDEV_DOWN_BATCH many times, as result,
> sometimes after NETDEV_UNREGISTER?
Same here, I can add this optimization. I think it is fine to call the
BATCH notifiers
for every interface. It is just better to do it for many interfaces at
the same time.
>> + list_for_each_entry_safe(net, net_tmp, &net_head, event_list) {
>> + call_netdevice_notifiers(NETDEV_UNREGISTER_BATCH,
>> + net->loopback_dev);
>> + net_del_event_list(net);
>> + }
>> +
>
> NETDEV_UNREGISTER* should not be called before
> following synchronize_net and NETDEV_UNREGISTER. May be
> we should split the loop: loop (dev_shutdown+NETDEV_UNREGISTER)
> followed by above NETDEV_UNREGISTER_BATCH then again the
> loop for all remaining calls
>
>> synchronize_net();
>>
>> list_for_each_entry(dev, head, unreg_list)
>
> Regards
>
> --
> Julian Anastasov <[email protected]>
The call to NETDEV_UNREGISTER_BATCH is actually after NETDEV_UNREGISTER,
it seems the other way around in the patch because it is showing part
of netdev_wait_allrefs
and not rollback_registered_may.
Thanks,
Salam
