diff mbox

[RFC] Move in6_dev_hold under CONFIG_IPV6_PRIVACY

Message ID 503F9A30.6030100@pace.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

David Marchand Aug. 30, 2012, 4:52 p.m. UTC 
On 30/08/2012 17:25, Eric Dumazet wrote:
> Your patch was mangled by your email client, and its always safer to
> test a patch...

Sorry about that, I am not used to sending patch (tried again, I hope it is fine this time ...).

I did try my patch in my setup, and did not notice any problem at the moment.
I sent this patch as a rfc, mainly because I want to know if my analysis is correct (and if not, understand why).


> Dont trust the comment, its a bit misleading.
>
> Comment intent was to say :
>
> /* One reference from device.  We must do this before
>  * we eventually invoke __ipv6_regen_rndid().
>  */
>

Mmm, ok.
But if CONFIG_IPV6_PRIVACY is not selected, then I can't find where the associated in6_dev_put is done for this in6_dev_hold.
Besides, __ipv6_regen_rndid is a "CONFIG_IPV6_PRIVACY-only" stuff.

Comments

Eric Dumazet Aug. 30, 2012, 5:28 p.m. UTC | #1 
On Thu, 2012-08-30 at 18:52 +0200, David Marchand wrote:

> Mmm, ok.
> But if CONFIG_IPV6_PRIVACY is not selected, then I can't find where the associated in6_dev_put is done for this in6_dev_hold.
> Besides, __ipv6_regen_rndid is a "CONFIG_IPV6_PRIVACY-only" stuff.
> 

Okay.

Look at addrconf_ifdown(), line 2850

in6_dev_put(idev);

Will finally remove the last reference

With your patch, I guess there will be a reference problem, and possibly
call in6_dev_finish_destroy() too soon.

You are mistaken, because the reference taken in PRIVACY is at line 421,
right before calling ipv6_regen_rndid()

Look at following commit to have an idea of the underlying problem

http://git.kernel.org/?p=linux/kernel/git/davem/net.git;a=commit;h=144d56e91044181ec0ef67aeca91e9a8b5718348


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6bc85f7..263fcf3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -393,11 +393,6 @@  static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 		return NULL;
 	}

-	/* One reference from device.  We must do this before
-	 * we invoke __ipv6_regen_rndid().
-	 */
-	in6_dev_hold(ndev);
-
 	if (dev->flags & (IFF_NOARP | IFF_LOOPBACK))
 		ndev->cnf.accept_dad = -1;

@@ -410,6 +405,12 @@  static struct inet6_dev *ipv6_add_dev(struct net_device *dev)

 #ifdef CONFIG_IPV6_PRIVACY
 	INIT_LIST_HEAD(&ndev->tempaddr_list);
+
+	/* One reference from device.  We must do this before
+	 * we invoke ipv6_regen_rndid().
+	 */
+	in6_dev_hold(ndev);
+
 	setup_timer(&ndev->regen_timer, ipv6_regen_rndid, (unsigned long)ndev);
 	if ((dev->flags&IFF_LOOPBACK) ||
 	    dev->type == ARPHRD_TUNNEL ||