Patchwork [RFC] IPV6 address management

login
register
mail settings
Submitter stephen hemminger
Date Jan. 8, 2009, 8:12 p.m.
Message ID <20090108121220.789325a6@extreme>
Download mbox | patch
Permalink /patch/17404/
State RFC
Delegated to: David Miller
Headers show

Comments

stephen hemminger - Jan. 8, 2009, 8:12 p.m.
What about this?

If it works (still testing), I'll submit it.

---
 Documentation/networking/ip-sysctl.txt |    8 ++++++
 include/linux/ipv6.h                   |    2 +
 net/ipv6/addrconf.c                    |   38 +++++++++++++++++++++-----------
 3 files changed, 35 insertions(+), 13 deletions(-)
David Miller - Jan. 8, 2009, 8:58 p.m.
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 8 Jan 2009 12:12:20 -0800

> What about this?
> 
> If it works (still testing), I'll submit it.

So what is your plan?

Make the routing daemons depend upon the non-default
behavior in order to act correctly?

Or is it to gradually get people to use the non-default
(via distribution sysctl settings etc.) and eventually
make it the default?

I disagree with both plans, and with that the facility
is basically useless.

We absolutely have to live with the behavior we have now,
and for a long time if not forever.
--
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
Ben Greear - Jan. 8, 2009, 9:11 p.m.
David Miller wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 8 Jan 2009 12:12:20 -0800
> 
>> What about this?
>>
>> If it works (still testing), I'll submit it.
> 
> So what is your plan?
> 
> Make the routing daemons depend upon the non-default
> behavior in order to act correctly?

They could check and take corrective action or tell the user
to set it one way or another.  Robust and complex apps already
have to check all sorts of things anyway.

> Or is it to gradually get people to use the non-default
> (via distribution sysctl settings etc.) and eventually
> make it the default?
> 
> I disagree with both plans, and with that the facility
> is basically useless.
> 
> We absolutely have to live with the behavior we have now,
> and for a long time if not forever.

It could be argued we need to have the default behaviour stay
the same, but that's no reason to keep things difficult for
people/programs flexible enough to deal with change.

Thanks,
Ben
stephen hemminger - Jan. 8, 2009, 9:44 p.m.
On Thu, 08 Jan 2009 12:58:30 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 8 Jan 2009 12:12:20 -0800
> 
> > What about this?
> > 
> > If it works (still testing), I'll submit it.
> 
> So what is your plan?
> 
> Make the routing daemons depend upon the non-default
> behavior in order to act correctly?
routing daemons don't deal with addresses directly now,
and getting them to do it would be painful.

> Or is it to gradually get people to use the non-default
> (via distribution sysctl settings etc.) and eventually
> make it the default?
No plan to ever change the default. Just ship with sysctl.conf
setting.

> I disagree with both plans, and with that the facility
> is basically useless.
> 
> We absolutely have to live with the behavior we have now,
> and for a long time if not forever.


--
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 - Jan. 8, 2009, 9:51 p.m.
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 8 Jan 2009 13:44:02 -0800

> On Thu, 08 Jan 2009 12:58:30 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
> > Or is it to gradually get people to use the non-default
> > (via distribution sysctl settings etc.) and eventually
> > make it the default?
> No plan to ever change the default. Just ship with sysctl.conf
> setting.

If the distributions all ship with the sysctl changed
to the non-default, our "default" is pretty meaningless
wouldn't you say?
--
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
stephen hemminger - Jan. 8, 2009, 9:56 p.m.
On Thu, 08 Jan 2009 13:51:25 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 8 Jan 2009 13:44:02 -0800
> 
> > On Thu, 08 Jan 2009 12:58:30 -0800 (PST)
> > David Miller <davem@davemloft.net> wrote:
> > 
> > > Or is it to gradually get people to use the non-default
> > > (via distribution sysctl settings etc.) and eventually
> > > make it the default?
> > No plan to ever change the default. Just ship with sysctl.conf
> > setting.
> 
> If the distributions all ship with the sysctl changed
> to the non-default, our "default" is pretty meaningless
> wouldn't you say?

It seems the only logical way to undo a poor choice
in the original design
--
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 - Jan. 8, 2009, 9:58 p.m.
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 8 Jan 2009 13:56:14 -0800

> On Thu, 08 Jan 2009 13:51:25 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Stephen Hemminger <shemminger@vyatta.com>
> > Date: Thu, 8 Jan 2009 13:44:02 -0800
> > 
> > > On Thu, 08 Jan 2009 12:58:30 -0800 (PST)
> > > David Miller <davem@davemloft.net> wrote:
> > > 
> > > > Or is it to gradually get people to use the non-default
> > > > (via distribution sysctl settings etc.) and eventually
> > > > make it the default?
> > > No plan to ever change the default. Just ship with sysctl.conf
> > > setting.
> > 
> > If the distributions all ship with the sysctl changed
> > to the non-default, our "default" is pretty meaningless
> > wouldn't you say?
> 
> It seems the only logical way to undo a poor choice
> in the original design

If the dists can do it so unilaterally, why can't we?

Everything about these proposals is a contradiction.
That's why I don't like them at all.

I say we keep the behavior, we don't change or break
anything, and people need to learn how to cope with it.
--
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
stephen hemminger - Jan. 8, 2009, 10:01 p.m.
On Thu, 08 Jan 2009 13:58:19 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 8 Jan 2009 13:56:14 -0800
> 
> > On Thu, 08 Jan 2009 13:51:25 -0800 (PST)
> > David Miller <davem@davemloft.net> wrote:
> > 
> > > From: Stephen Hemminger <shemminger@vyatta.com>
> > > Date: Thu, 8 Jan 2009 13:44:02 -0800
> > > 
> > > > On Thu, 08 Jan 2009 12:58:30 -0800 (PST)
> > > > David Miller <davem@davemloft.net> wrote:
> > > > 
> > > > > Or is it to gradually get people to use the non-default
> > > > > (via distribution sysctl settings etc.) and eventually
> > > > > make it the default?
> > > > No plan to ever change the default. Just ship with sysctl.conf
> > > > setting.
> > > 
> > > If the distributions all ship with the sysctl changed
> > > to the non-default, our "default" is pretty meaningless
> > > wouldn't you say?
> > 
> > It seems the only logical way to undo a poor choice
> > in the original design
> 
> If the dists can do it so unilaterally, why can't we?
> 
> Everything about these proposals is a contradiction.
> That's why I don't like them at all.
> 
> I say we keep the behavior, we don't change or break
> anything, and people need to learn how to cope with it.

Fine, it won't be the first or last vendor specific kernel patch.
--
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 - Jan. 8, 2009, 10:03 p.m.
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 8 Jan 2009 14:01:22 -0800

> Fine, it won't be the first or last vendor specific kernel patch.

You're just supporting my argument even more.

If all the dists ship it and turn it on, then logically we should
include it and have it on by default.

But you know we can't do that.

Therefore, it's detrimental for the dists to ship it too because it
does break things.

I know you want to shove this in, via some avenue, but it is really
so undesirable to cope with existing behavior?
--
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
stephen hemminger - Jan. 8, 2009, 10:56 p.m.
On Thu, 08 Jan 2009 14:03:39 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 8 Jan 2009 14:01:22 -0800
> 
> > Fine, it won't be the first or last vendor specific kernel patch.
> 
> You're just supporting my argument even more.
> 
> If all the dists ship it and turn it on, then logically we should
> include it and have it on by default.
> 
> But you know we can't do that.
> 
> Therefore, it's detrimental for the dists to ship it too because it
> does break things.
> 
> I know you want to shove this in, via some avenue, but it is really
> so undesirable to cope with existing behavior?

Reading back in the archives, this has come up before. The real problem
is that when link goes down and comes back the address needs to be
reverified for DAD. Maybe instead of deleting addresses they should
be moved to another list for restart.

It is a real problem for routing daemons like hostapd and zebra since
the kernel address deletion looks a lot like a user address deletion.
Since Vyatta has the luxury of a configuration/scripting layer, this
means the state space is contained (we know what the address was),
I'll just hack around it there for now until a better solution arises.
--
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

Patch

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index e2b2894..fa05b06 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -937,6 +937,14 @@  dad_transmits - INTEGER
 	The amount of Duplicate Address Detection probes to send.
 	Default: 1
 	
+delete_address_ifdown - BOOLEAN
+	Delete all addresses on device down
+	Default: TRUE
+
+	When network device is disabled: 
+	  FALSE - only delete temporary addresses (similar to IPV4)
+	  TRUE  - delete all addresses
+
 forwarding - BOOLEAN
 	Configure interface-specific Host/Router behaviour.  
 
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 641e026..b2d219a 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -166,6 +166,7 @@  struct ipv6_devconf {
 #endif
 	__s32		disable_ipv6;
 	__s32		accept_dad;
+	__s32		delete_address_ifdown;
 	void		*sysctl;
 };
 #endif
@@ -200,6 +201,7 @@  enum {
 	DEVCONF_MC_FORWARDING,
 	DEVCONF_DISABLE_IPV6,
 	DEVCONF_ACCEPT_DAD,
+	DEVCONF_DELETE_ADDRESS_IFDOWN,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2635fa2..9133c20 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -186,6 +186,7 @@  static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.accept_source_route	= 0,	/* we do not accept RH0 by default. */
 	.disable_ipv6		= 0,
 	.accept_dad		= 1,
+	.delete_address_ifdown  = 1,
 };
 
 static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
@@ -220,6 +221,7 @@  static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.accept_source_route	= 0,	/* we do not accept RH0 by default. */
 	.disable_ipv6		= 0,
 	.accept_dad		= 1,
+	.delete_address_ifdown  = 1,
 };
 
 /* IPv6 Wildcard Address and Loopback Address defined by RFC2553 */
@@ -2673,19 +2675,20 @@  static int addrconf_ifdown(struct net_device *dev, int how)
 		write_lock_bh(&idev->lock);
 	}
 #endif
-	while ((ifa = idev->addr_list) != NULL) {
-		idev->addr_list = ifa->if_next;
-		ifa->if_next = NULL;
-		ifa->dead = 1;
-		addrconf_del_timer(ifa);
-		write_unlock_bh(&idev->lock);
-
-		__ipv6_ifa_notify(RTM_DELADDR, ifa);
-		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
-		in6_ifa_put(ifa);
-
-		write_lock_bh(&idev->lock);
-	}
+	if (idev->cnf.delete_address_ifdown)
+		while ((ifa = idev->addr_list) != NULL) {
+			idev->addr_list = ifa->if_next;
+			ifa->if_next = NULL;
+			ifa->dead = 1;
+			addrconf_del_timer(ifa);
+			write_unlock_bh(&idev->lock);
+
+			__ipv6_ifa_notify(RTM_DELADDR, ifa);
+			atomic_notifier_call_chain(&inet6addr_chain,
+						   NETDEV_DOWN, ifa);
+			in6_ifa_put(ifa);
+			write_lock_bh(&idev->lock);
+		}
 	write_unlock_bh(&idev->lock);
 
 	/* Step 5: Discard multicast list */
@@ -3693,6 +3696,7 @@  static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 #endif
 	array[DEVCONF_DISABLE_IPV6] = cnf->disable_ipv6;
 	array[DEVCONF_ACCEPT_DAD] = cnf->accept_dad;
+	array[DEVCONF_DELETE_ADDRESS_IFDOWN] = cnf->delete_address_ifdown;
 }
 
 static inline size_t inet6_if_nlmsg_size(void)
@@ -4268,6 +4272,14 @@  static struct addrconf_sysctl_table
 			.proc_handler	=	&proc_dointvec,
 		},
 		{
+			.ctl_name	=	CTL_UNNUMBERED,
+			.procname	=	"delete_address_ifdown",
+			.data		=	&ipv6_devconf.delete_address_ifdown,
+			.maxlen		=	sizeof(int),
+			.mode		=	0644,
+			.proc_handler	=	&proc_dointvec,
+		},
+		{
 			.ctl_name	=	0,	/* sentinel */
 		}
 	},