Re: [PATCH vxlan v2 1/8] vxlan: Fix module cleanup.

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 26 Jun 2013 13:13:29 -0700
Pravin B Shelar <pshelar@nicira.com> wrote:

> vxlan private per net object can be accessed in device unregister.
> therefore unregister pernet device at the end.
> 
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
>  drivers/net/vxlan.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 227b54a..0ba1e7e 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1916,9 +1916,9 @@ late_initcall(vxlan_init_module);
>  
>  static void __exit vxlan_cleanup_module(void)
>  {
> -	unregister_pernet_device(&vxlan_net_ops);
>  	rtnl_link_unregister(&vxlan_link_ops);
>  	destroy_workqueue(vxlan_wq);
> +	unregister_pernet_device(&vxlan_net_ops);
>  	rcu_barrier();
>  }
>  module_exit(vxlan_cleanup_module);

No. This won't work.
The problem is that:
  rtnl_link_unregister calls __rtnl_kill_links which calls
  vxlan_dellink

vxlan_dellink assumes link is last thing and that the vxlan
device is already down. vxlan_dellink will unregister the device
and schedule work queue for closing socket.

When you call unregister_pernet_device last, nothing happens.
Since all vxlan device's are unregistered, the per-net vxlan
device list would be empty. And since vxlan_stop was never called.
The age_timer would never be stopped, and the multicast group
would never left, and the forwarding table will leak memory.


commit b7153984074e51a50dad905871b705e0d67aa147
Author: Stephen Hemminger <stephen@networkplumber.org>
Date:   Mon Jun 17 14:16:09 2013 -0700

    vxlan: fix out of order operation on module removal
    
    If vxlan is removed with active vxlan's it would crash because
    rtnl_link_unregister (which calls vxlan_dellink), was invoked
    before unregister_pernet_device (which calls vxlan_stop).
    
    Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 284c6c0..d3005d3 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1771,8 +1771,8 @@ late_initcall(vxlan_init_module);
 
 static void __exit vxlan_cleanup_module(void)
 {
-	rtnl_link_unregister(&vxlan_link_ops);
 	unregister_pernet_device(&vxlan_net_ops);
+	rtnl_link_unregister(&vxlan_link_ops);
 	rcu_barrier();
 }
 module_exit(vxlan_cleanup_module);