On 06.03.2018 13:26, Oliver Hartkopp wrote:
> - DaveM
>
> Hi Kirill,
>
> On 03/05/2018 04:22 PM, Kirill Tkhai wrote:
>
>> Thanks for the explanation, and module unloading should be nice. Just to
>> clarify,
>> I worry not about rules, but about netdevices.
>>
>> unshare -n ip link add type vcan
>>
>> This command creates net ns, adds vcan there and exits. Then net ns is
>> destroyed.
>> Since vcan has rtnl_link_ops, it unregistered by default_device_exit_batch().
>> Real can devices are moved to init_net in default_device_exit(), as they
>> don't
>> have rtnl_link_ops set.
>
> In fact most of the real CAN drivers have rtnl_link_ops:
>
> https://elixir.bootlin.com/linux/latest/source/drivers/net/can/dev.c#L1162
> https://elixir.bootlin.com/linux/latest/source/drivers/net/can/dev.c#L1225
>
> Just slcan.c which is something like slip.c is missing these feature.
>
> AFAIK real CAN netdevices are created in init_net at system startup, and you
> can move them to a netns later.
>
> When we already have rtnl_link_ops in the real CAN drivers - what happens to
> them when the namespace is destroyed? Are they still moved to init_net, or do
> we need to add some default handler in the current rtnl_link_ops setup?
They are unregistered in default_device_exit_batch():
list_for_each_entry(net, net_list, exit_list) {
for_each_netdev_reverse(net, dev) {
if (dev->rtnl_link_ops && dev->rtnl_link_ops->dellink)
dev->rtnl_link_ops->dellink(dev,
&dev_kill_list);
else
unregister_netdevice_queue(dev, &dev_kill_list);
}
}
unregister_netdevice_many(&dev_kill_list);
So that, my question in the start of the topic is about that. If we unregister
them,
what devices we're going to meet in can_pernet_exit()?
Also, can devices not having rtnl_link_ops are moved into init_net in
default_device_exit():
for_each_netdev_safe(net, dev, aux) {
if (dev->features & NETIF_F_NETNS_LOCAL)
continue;
/* Leave virtual devices for the generic cleanup */
if (dev->rtnl_link_ops)
continue;
err = dev_change_net_namespace(dev, &init_net, fb_name);
}
>> So, for_each_netdev_rcu() cycle in can_pernet_exit() should be useless
>> (there are
>> no can devices in the list of net's devices). This looks so for me, please
>> say
>> what devices are there if my assumption is wrong.
>
> See above?
>
>>>> Since can_pernet_ops is pernet subsys, it's executed after
>>>> default_device_exit()
>>>> from default_device_ops pernet device, as devices exit methods are
>>>> executed first
>>>> (see net/core/net_namespace.c).
>>>
>>> Hm - a device exit fires the NETDEV_UNREGISTER notifier which removes the
>>> user-generated filters (e.g. in raw_notifier() in net/can/raw.c).
>>> Finally the can_dev_rcv_lists structure is free'd in af_can.c.
>>>
>>> Marc Kleine-Budde recently proposed a patch to create the can_dev_rcv_lists
>>> at
>>> netdevice creation time (-> the space is allocated by alloc_netdev() and
>>> removed by free_netdev()). This would remove the handling (allocate & free)
>>> of
>>> ml_priv by af_can.c. Would this rework fix the described issue?
>>
>> Could you please give me a link to the patches? I can't find them in
>> patchwork.
>
> There was a patchset of 14 patches from Marc where some of the refactoring &
> renaming already made it into mainline - but the patches to move the
> can_dev_rcv_lists data structure into the network device space have not been
> pushed:
>
> https://marc.info/?l=linux-can&m=150169588319315&w=2
> https://marc.info/?l=linux-can&r=1&b=201708&w=2
>
> This patch & documentation describes Marc's proposed idea best:
> https://marc.info/?l=linux-can&m=150169589619340&w=2
Yes, this one looks good:
https://marc.info/?l=linux-can&m=150169589119335&w=2
Regards,
Kirill
