Hi,

I'm converting/reviewing pernet_operations either they allow several net 
namespaces
to be created/destroyed in parallel or not. Please, see the details in my recent
patches in net-next.git, if your are interested.

There is a strange place in can_pernet_ops pernet subsys, I found:

static void can_pernet_exit(struct net *net)
{
        ...
        rcu_read_lock();
        for_each_netdev_rcu(net, dev) {
                if (dev->type == ARPHRD_CAN && dev->ml_priv) {
                        struct can_dev_rcv_lists *d = dev->ml_priv; 

                        BUG_ON(d->entries);
                        kfree(d);
                        dev->ml_priv = NULL;
                }
        }
        rcu_read_unlock()
        ...
}

This code clears dev->ml_priv from can devices, and it looks strange.
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).

There are no NETIF_F_NETNS_LOCAL devices among can devices, though there is
check of can_link_ops in safe_candev_priv(). I haven't found can devices may
have net_device::rtnl_link_ops. But the code seems want to allow them. Anyway,
it's wrong in any case:

1)If there are can devices, which may be skipped by default_device_exit(),
can_pernet_exit() must use rtnl_lock() instead of rcu_read_lock(), and
it must move such devices to init_net. See wifi cfg80211_pernet_exit() for 
example.

2)If there are no such the devices, the code between rcu_read_lock() and 
rcu_read_unlock()
is useless, and must be deleted, as it never works and confuses a reader.

Thanks,
Kirill

Reply via email to