Re: [PATCH net-next 1/4] ipip: allow to deactivate the creation of fb dev

[David Miller <davem@davemloft.net>]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Fri, 16 Nov 2012 17:14:13 +0100

> Now that tunnels can be configured via rtnetlink, this device is not mandatory.
> The default is conservative.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

I'm not too thrilled about this change, mostly because of my dislike of
module parameters in general.

But in this case there appears to be real bugs in the two sets of changes
where you add this setup_fb thing.

> @@ -1057,7 +1066,8 @@ static void __net_exit ipip_exit_net(struct net *net)
>  
>  	rtnl_lock();
>  	ipip_destroy_tunnels(ipn, &list);
> -	unregister_netdevice_queue(ipn->fb_tunnel_dev, &list);
> +	if (setup_fb)
> +		unregister_netdevice_queue(ipn->fb_tunnel_dev, &list);
>  	unregister_netdevice_many(&list);
>  	rtnl_unlock();
>  }

Users can modify module parameter values via sysfs after the module
is loaded, so you need a more internal and protected state to use
to decide whether you really need to unregister the thing or not.

But to me it's just symptomatic of what a bad idea this is in the
first place.