diff mbox

With disable_ipv6 set to 1 on an interface, ff00:/8 and fe80::/64 are still added on device UP

Message ID 4C460856.5090701@hp.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Brian Haley July 20, 2010, 8:34 p.m. UTC 
Hi Mahesh,

Cc-ing netdev...

On 07/20/2010 12:07 PM, Mahesh Kelkar wrote:
> Brian,
> 
> I came across a patch that you submitted in 2009 (2009-05-29 20:48:49):
> IPv6: Add 'autoconf' and 'disable_ipv6' module parameters
> 
> Question:
> With disable_ipv6 set to 1 on the interface, when device/interface
> reaches UP state, the link local address is not added, but ipv6 routes
> i.e. ff00::/8 & fe80::/64 routes are still added to the route table:
> In net/ipv6/addrconf.c
> addrconf_notify => addrconf_dev_config => addrconf_add_dev =>
> addrconf_add_mroute & addrconf_add_lroute
> The link local address is not assigned because of the check
> (idev->cnf.disable_ipv6) added in ipv6_add_addr.
> 
> - Is there any particular reason for doing this? (i.e. not assigning
> the link local address to interface, but adding link local & mcast
> routes)
> - when disable_ipv6 is set to 1, is there any reason not to skip the
> NETDEV_UP processing in the addrconf_notify in addrconf.c

I believe the easiest way to fix this is the following patch, can
you please test it?

Thanks,

-Brian

---

If the interface has IPv6 disabled, don't add a multicast or
link-local route since we won't be adding a link-local address.

Reported-by: Mahesh Kelkar <maheshkelkar@gmail.com>
Signed-off-by: Brian Haley <brian.haley@hp.com>
---
--
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

Comments

David Miller July 20, 2010, 8:48 p.m. UTC | #1 
From: Brian Haley <brian.haley@hp.com>
Date: Tue, 20 Jul 2010 16:34:30 -0400

> I believe the easiest way to fix this is the following patch, can
> you please test it?
 ...
> If the interface has IPv6 disabled, don't add a multicast or
> link-local route since we won't be adding a link-local address.
> 
> Reported-by: Mahesh Kelkar <maheshkelkar@gmail.com>
> Signed-off-by: Brian Haley <brian.haley@hp.com>

This looks good to me, let me know when it has been tested.
--
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
Mahesh Kelkar July 22, 2010, 2:03 p.m. UTC | #2 
Brian,

Overall the patch seem to work.

On one occasion I saw an error when it tried get rtnl_trylock() in
"addrconf_disable_ipv6" in addrconf.c. I am investigating into it. If
you could think of anything, please let me know.

I also came across another odd behavior (unrelated to disable_ipv6 but
related to multicast & link local route):
A. configure unicast Ipv6 address (say 123:2:3:4:5:6:7:8/64) on an
interface. (link-local will be assigned when interface comes up)
B. Bring the interface down (ip link set eth0 down),

you will get following set of netlink notifications (ip monitor all):
1. Deleted - unicast address connected route (123:2:3:4::/64)
2. Deleted - link local (fe80::/64) route
3. Deleted - multicast (ff00::/8) route
4. Deleted - unicast address (123:2:3:4:5:6:7:8/64)
5. Deleted - link local address

C. re-configure the unicast Ipv6 address (say 123:2:3:4:5:6:7:8/64) on
the interface. (link-local will NOT be assigned as interface is down)

You wil see following netlink notifications:
6. Added - unicast address (123:2:3:4:5:6:7:8/64)
7. Added - unicast address connected route (123:2:3:4::/64)
8. Added - multicast (ff00::/8) route
9. Added - link local (fe80::/64) route
etc.

I am not sure why #7, #8 & #9 occured. It doesn't happen in case of
IPv4. The routes show up when interface reaches up state. Perhaps my
kernel is old and that could be reason for this beahvior.

BTW I am using 2.6.21 with following cherry-picked disable_ipv6 patches:
- ipv6: Add disable_ipv6 sysctl to disable IPv6 operaion on specific
interface(commit:778d80be52699596bf70e0eb0761cf5e1e46088d)
- ipv6: Plug sk_buff leak in ipv6_rcv (net/ipv6/ip6_input.c) (commit:
71f6f6dfdf7c7a67462386d9ea05c1095a89c555)
- IPv6: Add 'autoconf' and 'disable_ipv6' module parameters (ONLY
interface specific behavior)

Thanks very much for your help.
Mahesh

On Tue, Jul 20, 2010 at 4:48 PM, David Miller <davem@davemloft.net> wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Tue, 20 Jul 2010 16:34:30 -0400
>
>> I believe the easiest way to fix this is the following patch, can
>> you please test it?
>  ...
>> If the interface has IPv6 disabled, don't add a multicast or
>> link-local route since we won't be adding a link-local address.
>>
>> Reported-by: Mahesh Kelkar <maheshkelkar@gmail.com>
>> Signed-off-by: Brian Haley <brian.haley@hp.com>
>
> This looks good to me, let me know when it has been tested.
>
--
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
David Miller July 22, 2010, 8:41 p.m. UTC | #3 
From: Brian Haley <brian.haley@hp.com>
Date: Tue, 20 Jul 2010 16:34:30 -0400

> If the interface has IPv6 disabled, don't add a multicast or
> link-local route since we won't be adding a link-local address.
> 
> Reported-by: Mahesh Kelkar <maheshkelkar@gmail.com>
> Signed-off-by: Brian Haley <brian.haley@hp.com>

Applied, thanks Brian.
--
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 e81155d..ab70a3f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1763,7 +1763,10 @@  static struct inet6_dev *addrconf_add_dev(struct net_device *dev)
 
 	idev = ipv6_find_idev(dev);
 	if (!idev)
-		return NULL;
+		return ERR_PTR(-ENOBUFS);
+
+	if (idev->cnf.disable_ipv6)
+		return ERR_PTR(-EACCES);
 
 	/* Add default multicast route */
 	addrconf_add_mroute(dev);
@@ -2132,8 +2135,9 @@  static int inet6_addr_add(struct net *net, int ifindex, struct in6_addr *pfx,
 	if (!dev)
 		return -ENODEV;
 
-	if ((idev = addrconf_add_dev(dev)) == NULL)
-		return -ENOBUFS;
+	idev = addrconf_add_dev(dev);
+	if (IS_ERR(idev))
+		return PTR_ERR(idev);
 
 	scope = ipv6_addr_scope(pfx);
 
@@ -2380,7 +2384,7 @@  static void addrconf_dev_config(struct net_device *dev)
 	}
 
 	idev = addrconf_add_dev(dev);
-	if (idev == NULL)
+	if (IS_ERR(idev))
 		return;
 
 	memset(&addr, 0, sizeof(struct in6_addr));
@@ -2471,7 +2475,7 @@  static void addrconf_ip6_tnl_config(struct net_device *dev)
 	ASSERT_RTNL();
 
 	idev = addrconf_add_dev(dev);
-	if (!idev) {
+	if (IS_ERR(idev)) {
 		printk(KERN_DEBUG "init ip6-ip6: add_dev failed\n");
 		return;
 	}