netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
@ 2008-06-02 23:52 Stephen Hemminger
  2008-06-02 23:53 ` [PATCH 1/2] IPV6: " Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Stephen Hemminger @ 2008-06-02 23:52 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

This patch adds a new configuration sysctl that causes link loss to clear
FIB state in the same way as admin down. This allows for routing daemons
like Quagga which have option to remove routes when carrier is lost.

This has been a long standing problem with Quagga on Linux with complaints
on the developers list going back to 2004. Fixing it properly, so the routing 
daemon manages the RIB, and the kernel manages the FIB, requires changes to
both parts.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Patch is against net-next-2.6, so it can be integrated in 2.6.27.

--- a/Documentation/networking/ip-sysctl.txt	2008-06-02 14:35:36.000000000 -0700
+++ b/Documentation/networking/ip-sysctl.txt	2008-06-02 15:30:03.000000000 -0700
@@ -788,7 +788,10 @@ disable_policy - BOOLEAN
 disable_xfrm - BOOLEAN
 	Disable IPSEC encryption on this interface, whatever the policy
 
-
+link_detect - BOOLEAN
+	Causes loss of carrier remove interface addresses the same as
+	setting device down. When carrier is restored, the addresses
+	are restored.
 
 tag - INTEGER
 	Allows you to write a number, which can be used as required.
--- a/include/linux/inetdevice.h	2008-06-02 14:35:43.000000000 -0700
+++ b/include/linux/inetdevice.h	2008-06-02 14:36:54.000000000 -0700
@@ -97,6 +97,7 @@ static inline void ipv4_devconf_setall(s
 #define IN_DEV_PROMOTE_SECONDARIES(in_dev) \
 					IN_DEV_ORCONF((in_dev), \
 						      PROMOTE_SECONDARIES)
+#define IN_DEV_LINK_DETECT(in_dev)	IN_DEV_ORCONF((in_dev), LINK_DETECT)
 
 #define IN_DEV_RX_REDIRECTS(in_dev) \
 	((IN_DEV_FORWARD(in_dev) && \
--- a/include/linux/sysctl.h	2008-06-02 14:35:44.000000000 -0700
+++ b/include/linux/sysctl.h	2008-06-02 15:30:03.000000000 -0700
@@ -490,6 +490,7 @@ enum
 	NET_IPV4_CONF_ARP_IGNORE=19,
 	NET_IPV4_CONF_PROMOTE_SECONDARIES=20,
 	NET_IPV4_CONF_ARP_ACCEPT=21,
+	NET_IPV4_CONF_LINK_DETECT=22,
 	__NET_IPV4_CONF_MAX
 };
 
--- a/net/ipv4/devinet.c	2008-06-02 14:35:45.000000000 -0700
+++ b/net/ipv4/devinet.c	2008-06-02 14:36:54.000000000 -0700
@@ -1443,6 +1443,7 @@ static struct devinet_sysctl_table {
 					      "force_igmp_version"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
 					      "promote_secondaries"),
+		DEVINET_SYSCTL_RW_ENTRY(LINK_DETECT, "link_detect"),
 	},
 };
 
--- a/net/ipv4/fib_frontend.c	2008-06-02 14:35:45.000000000 -0700
+++ b/net/ipv4/fib_frontend.c	2008-06-02 14:36:54.000000000 -0700
@@ -944,6 +944,11 @@ static int fib_netdev_event(struct notif
 	if (!in_dev)
 		return NOTIFY_DONE;
 
+	/* Link detect causes changes in carrier to add/remove addresses from FIB */
+	if (event == NETDEV_CHANGE && netif_running(dev)
+	    && IN_DEV_LINK_DETECT(in_dev))
+		event = netif_carrier_ok(dev) ? NETDEV_UP : NETDEV_DOWN;
+
 	switch (event) {
 	case NETDEV_UP:
 		for_ifa(in_dev) {
--- a/kernel/sysctl_check.c	2008-06-02 16:31:09.000000000 -0700
+++ b/kernel/sysctl_check.c	2008-06-02 16:31:31.000000000 -0700
@@ -219,6 +219,7 @@ static const struct trans_ctl_table tran
 	{ NET_IPV4_CONF_ARP_IGNORE,		"arp_ignore" },
 	{ NET_IPV4_CONF_PROMOTE_SECONDARIES,	"promote_secondaries" },
 	{ NET_IPV4_CONF_ARP_ACCEPT,		"arp_accept" },
+	{ NET_IPV4_CONF_LINK_DETECT,		"link_detect" },
 	{}
 };
 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-02 23:52 [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost Stephen Hemminger
@ 2008-06-02 23:53 ` Stephen Hemminger
  2008-06-03 16:17   ` YOSHIFUJI Hideaki / 吉藤英明
  2008-06-03 15:05 ` [PATCH 1/2] IPV4: " Lennart Sorensen
  2008-06-06 14:56 ` Lennart Sorensen
  2 siblings, 1 reply; 24+ messages in thread
From: Stephen Hemminger @ 2008-06-02 23:53 UTC (permalink / raw)
  To: 吉藤英明, David Miller; +Cc: netdev

Similar to previous change for IPV4. Allow configuration option to remove
addresses from FIB when carrier is lost.

This patch also makes IPV6 ignore changes to carrier state when device
is admin down state. This prevents address configuration from starting
up on a buggy device driver that signals carrier changes when offline.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/Documentation/networking/ip-sysctl.txt	2008-06-02 15:30:03.000000000 -0700
+++ b/Documentation/networking/ip-sysctl.txt	2008-06-02 16:32:00.000000000 -0700
@@ -966,6 +966,10 @@ hop_limit - INTEGER
 	Default Hop Limit to set.
 	Default: 64
 
+link_detect - BOOLEAN
+	Remove addresses when link (carrier) is lost on interface.
+	Default: FALSE
+
 mtu - INTEGER
 	Default Maximum Transfer Unit
 	Default: 1280 (IPv6 required minimum)
--- a/include/linux/ipv6.h	2008-06-02 15:30:03.000000000 -0700
+++ b/include/linux/ipv6.h	2008-06-02 16:32:00.000000000 -0700
@@ -134,6 +134,7 @@ struct ipv6_devconf {
 	__s32		accept_redirects;
 	__s32		autoconf;
 	__s32		dad_transmits;
+	__s32		link_detect;
 	__s32		rtr_solicits;
 	__s32		rtr_solicit_interval;
 	__s32		rtr_solicit_delay;
@@ -194,6 +195,7 @@ enum {
 	DEVCONF_OPTIMISTIC_DAD,
 	DEVCONF_ACCEPT_SOURCE_ROUTE,
 	DEVCONF_MC_FORWARDING,
+	DEVCONF_LINK_DETECT,
 	DEVCONF_MAX
 };
 
--- a/include/linux/sysctl.h	2008-06-02 15:30:03.000000000 -0700
+++ b/include/linux/sysctl.h	2008-06-02 16:32:00.000000000 -0700
@@ -579,6 +579,7 @@ enum {
 	NET_IPV6_ACCEPT_RA_RT_INFO_MAX_PLEN=22,
 	NET_IPV6_PROXY_NDP=23,
 	NET_IPV6_ACCEPT_SOURCE_ROUTE=25,
+	NET_IPV6_LINK_DETECT=26,
 	__NET_IPV6_MAX
 };
 
--- a/kernel/sysctl_check.c	2008-06-02 16:31:31.000000000 -0700
+++ b/kernel/sysctl_check.c	2008-06-02 16:32:00.000000000 -0700
@@ -495,6 +495,7 @@ static const struct trans_ctl_table tran
 	{ NET_IPV6_ACCEPT_RA_RT_INFO_MAX_PLEN,	"accept_ra_rt_info_max_plen" },
 	{ NET_IPV6_PROXY_NDP,			"proxy_ndp" },
 	{ NET_IPV6_ACCEPT_SOURCE_ROUTE,		"accept_source_route" },
+	{ NET_IPV6_LINK_DETECT,			"link_detect" },
 	{}
 };
 
--- a/net/ipv6/addrconf.c	2008-06-02 15:30:03.000000000 -0700
+++ b/net/ipv6/addrconf.c	2008-06-02 16:32:00.000000000 -0700
@@ -185,6 +185,7 @@ struct ipv6_devconf ipv6_devconf __read_
 #endif
 	.proxy_ndp		= 0,
 	.accept_source_route	= 0,	/* we do not accept RH0 by default. */
+	.link_detect		= 0,
 };
 
 static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
@@ -217,6 +218,7 @@ static struct ipv6_devconf ipv6_devconf_
 #endif
 	.proxy_ndp		= 0,
 	.accept_source_route	= 0,	/* we do not accept RH0 by default. */
+	.link_detect		= 0,
 };
 
 /* IPv6 Wildcard Address and Loopback Address defined by RFC2553 */
@@ -2413,8 +2415,11 @@ static int addrconf_notify(struct notifi
 				return notifier_from_errno(-ENOMEM);
 		}
 		break;
-	case NETDEV_UP:
+
 	case NETDEV_CHANGE:
+		if (!netif_running(dev))
+			break;
+	case NETDEV_UP:
 		if (dev->flags & IFF_SLAVE)
 			break;
 
@@ -2433,12 +2438,15 @@ static int addrconf_notify(struct notifi
 
 			if (idev)
 				idev->if_flags |= IF_READY;
+		} else if (!netif_carrier_ok(dev)) {
+			/* device is offline */
+			if (idev && idev->cnf.link_detect)
+				addrconf_ifdown(dev, 1);
+			break;
+		} else if (!addrconf_qdisc_ok(dev)) {
+			/* device is still not ready. */
+			break;
 		} else {
-			if (!addrconf_qdisc_ok(dev)) {
-				/* device is still not ready. */
-				break;
-			}
-
 			if (idev) {
 				if (idev->if_flags & IF_READY) {
 					/* device is already configured. */
@@ -3629,6 +3637,7 @@ static inline void ipv6_store_devconf(st
 #ifdef CONFIG_IPV6_MROUTE
 	array[DEVCONF_MC_FORWARDING] = cnf->mc_forwarding;
 #endif
+	array[DEVCONF_LINK_DETECT] = cnf->link_detect;
 }
 
 static inline size_t inet6_if_nlmsg_size(void)
@@ -4098,6 +4107,14 @@ static struct addrconf_sysctl_table
 		},
 #endif
 		{
+			.ctl_name	=	NET_IPV6_LINK_DETECT,
+			.procname	=	"link_detect",
+			.data		=	&ipv6_devconf.link_detect,
+			.maxlen		=	sizeof(int),
+			.mode		=	0644,
+			.proc_handler	=	&proc_dointvec,
+		},
+		{
 			.ctl_name	=	NET_IPV6_MAX_ADDRESSES,
 			.procname	=	"max_addresses",
 			.data		=	&ipv6_devconf.max_addresses,

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-02 23:52 [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost Stephen Hemminger
  2008-06-02 23:53 ` [PATCH 1/2] IPV6: " Stephen Hemminger
@ 2008-06-03 15:05 ` Lennart Sorensen
  2008-06-03 15:57   ` Stephen Hemminger
  2008-06-06 14:56 ` Lennart Sorensen
  2 siblings, 1 reply; 24+ messages in thread
From: Lennart Sorensen @ 2008-06-03 15:05 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev

On Mon, Jun 02, 2008 at 04:52:49PM -0700, Stephen Hemminger wrote:
> This patch adds a new configuration sysctl that causes link loss to clear
> FIB state in the same way as admin down. This allows for routing daemons
> like Quagga which have option to remove routes when carrier is lost.
> 
> This has been a long standing problem with Quagga on Linux with complaints
> on the developers list going back to 2004. Fixing it properly, so the routing 
> daemon manages the RIB, and the kernel manages the FIB, requires changes to
> both parts.

Does this cover only the local route for the interface, or all routes
assigned staticly to the interface too?

Certianly if I do ifconfig eth3 down, it removes all the routes, but
doing ifconfig eth3 up only adds back the directly connected network
based on the IP and netmask, but my static routes are gone which is a
problem.

I have been considering writing a user space daemon to listen on netlink
for link up/down events to remove routes from the routing table and then
add them back when the link is restored since I couldn't find a good way
to make the kernel remember the static routes when the link was brought
down.

-- 
Len Sorensen

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-03 15:05 ` [PATCH 1/2] IPV4: " Lennart Sorensen
@ 2008-06-03 15:57   ` Stephen Hemminger
  2008-06-03 16:03     ` Patrick McHardy
  2008-06-03 21:44     ` Lennart Sorensen
  0 siblings, 2 replies; 24+ messages in thread
From: Stephen Hemminger @ 2008-06-03 15:57 UTC (permalink / raw)
  To: Lennart Sorensen; +Cc: Stephen Hemminger, David Miller, netdev

On Tue, 3 Jun 2008 11:05:24 -0400
lsorense@csclub.uwaterloo.ca (Lennart Sorensen) wrote:

> On Mon, Jun 02, 2008 at 04:52:49PM -0700, Stephen Hemminger wrote:
> > This patch adds a new configuration sysctl that causes link loss to clear
> > FIB state in the same way as admin down. This allows for routing daemons
> > like Quagga which have option to remove routes when carrier is lost.
> > 
> > This has been a long standing problem with Quagga on Linux with complaints
> > on the developers list going back to 2004. Fixing it properly, so the routing 
> > daemon manages the RIB, and the kernel manages the FIB, requires changes to
> > both parts.
> 
> Does this cover only the local route for the interface, or all routes
> assigned staticly to the interface too?

The patch makes carrier down == interface down. So the same
behaviour as doing 'ip link set dev eth3 down'

> Certianly if I do ifconfig eth3 down, it removes all the routes, but
> doing ifconfig eth3 up only adds back the directly connected network
> based on the IP and netmask, but my static routes are gone which is a
> problem.

If you are using a routing daemon like quagga, it will restore your static
routes.


> I have been considering writing a user space daemon to listen on netlink
> for link up/down events to remove routes from the routing table and then
> add them back when the link is restored since I couldn't find a good way
> to make the kernel remember the static routes when the link was brought
> down.

That is pretty much what zebra portion of quagga does.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-03 15:57   ` Stephen Hemminger
@ 2008-06-03 16:03     ` Patrick McHardy
  2008-06-03 16:31       ` Stephen Hemminger
  2008-06-03 21:45       ` Lennart Sorensen
  2008-06-03 21:44     ` Lennart Sorensen
  1 sibling, 2 replies; 24+ messages in thread
From: Patrick McHardy @ 2008-06-03 16:03 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Lennart Sorensen, Stephen Hemminger, David Miller, netdev

Stephen Hemminger wrote:
> On Tue, 3 Jun 2008 11:05:24 -0400
> lsorense@csclub.uwaterloo.ca (Lennart Sorensen) wrote:
> 
>> On Mon, Jun 02, 2008 at 04:52:49PM -0700, Stephen Hemminger wrote:
>>> This patch adds a new configuration sysctl that causes link loss to clear
>>> FIB state in the same way as admin down. This allows for routing daemons
>>> like Quagga which have option to remove routes when carrier is lost.
>>>
>>> This has been a long standing problem with Quagga on Linux with complaints
>>> on the developers list going back to 2004. Fixing it properly, so the routing 
>>> daemon manages the RIB, and the kernel manages the FIB, requires changes to
>>> both parts.
>> Does this cover only the local route for the interface, or all routes
>> assigned staticly to the interface too?
> 
> The patch makes carrier down == interface down. So the same
> behaviour as doing 'ip link set dev eth3 down'


Can't the routing daemon simply ignore routes with a
device that has no carrier?


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-02 23:53 ` [PATCH 1/2] IPV6: " Stephen Hemminger
@ 2008-06-03 16:17   ` YOSHIFUJI Hideaki / 吉藤英明
  2008-06-03 17:21     ` Stephen Hemminger
  0 siblings, 1 reply; 24+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2008-06-03 16:17 UTC (permalink / raw)
  To: shemminger; +Cc: davem, netdev, yoshfuji

In article <20080602165347.5662f602@extreme> (at Mon, 2 Jun 2008 16:53:47 -0700), Stephen Hemminger <shemminger@vyatta.com> says:

> Similar to previous change for IPV4. Allow configuration option to remove
> addresses from FIB when carrier is lost.
> 
> This patch also makes IPV6 ignore changes to carrier state when device
> is admin down state. This prevents address configuration from starting
> up on a buggy device driver that signals carrier changes when offline.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Well, we need to be careful here.
IMHO we should not remove addresses (especially link-local address
based on MAC, or manually assigned addresses, maybe).
We might want to do Optimistic DAD here.

--yoshfuji

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-03 16:03     ` Patrick McHardy
@ 2008-06-03 16:31       ` Stephen Hemminger
  2008-06-03 16:44         ` Patrick McHardy
  2008-06-03 21:45       ` Lennart Sorensen
  1 sibling, 1 reply; 24+ messages in thread
From: Stephen Hemminger @ 2008-06-03 16:31 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Stephen Hemminger, Lennart Sorensen, David Miller, netdev

On Tue, 03 Jun 2008 18:03:46 +0200
Patrick McHardy <kaber@trash.net> wrote:

> Stephen Hemminger wrote:
> > On Tue, 3 Jun 2008 11:05:24 -0400
> > lsorense@csclub.uwaterloo.ca (Lennart Sorensen) wrote:
> > 
> >> On Mon, Jun 02, 2008 at 04:52:49PM -0700, Stephen Hemminger wrote:
> >>> This patch adds a new configuration sysctl that causes link loss to clear
> >>> FIB state in the same way as admin down. This allows for routing daemons
> >>> like Quagga which have option to remove routes when carrier is lost.
> >>>
> >>> This has been a long standing problem with Quagga on Linux with complaints
> >>> on the developers list going back to 2004. Fixing it properly, so the routing 
> >>> daemon manages the RIB, and the kernel manages the FIB, requires changes to
> >>> both parts.
> >> Does this cover only the local route for the interface, or all routes
> >> assigned staticly to the interface too?
> > 
> > The patch makes carrier down == interface down. So the same
> > behaviour as doing 'ip link set dev eth3 down'
> 
> 
> Can't the routing daemon simply ignore routes with a
> device that has no carrier?
> 

It does that, but the problem is that packets get routed in kernel
to interfaces without carrier, rather than being correctly rerouted
over alternate paths.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-03 16:31       ` Stephen Hemminger
@ 2008-06-03 16:44         ` Patrick McHardy
  0 siblings, 0 replies; 24+ messages in thread
From: Patrick McHardy @ 2008-06-03 16:44 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Stephen Hemminger, Lennart Sorensen, David Miller, netdev

Stephen Hemminger wrote:
> On Tue, 03 Jun 2008 18:03:46 +0200
> Patrick McHardy <kaber@trash.net> wrote:
> 
>> Stephen Hemminger wrote:
>>> On Tue, 3 Jun 2008 11:05:24 -0400
>>> lsorense@csclub.uwaterloo.ca (Lennart Sorensen) wrote:
>>>
>>>> On Mon, Jun 02, 2008 at 04:52:49PM -0700, Stephen Hemminger wrote:
>>>>> This patch adds a new configuration sysctl that causes link loss to clear
>>>>> FIB state in the same way as admin down. This allows for routing daemons
>>>>> like Quagga which have option to remove routes when carrier is lost.
>>>>>
>>>>> This has been a long standing problem with Quagga on Linux with complaints
>>>>> on the developers list going back to 2004. Fixing it properly, so the routing 
>>>>> daemon manages the RIB, and the kernel manages the FIB, requires changes to
>>>>> both parts.
>>>> Does this cover only the local route for the interface, or all routes
>>>> assigned staticly to the interface too?
>>> The patch makes carrier down == interface down. So the same
>>> behaviour as doing 'ip link set dev eth3 down'
>>
>> Can't the routing daemon simply ignore routes with a
>> device that has no carrier?
>>
> 
> It does that, but the problem is that packets get routed in kernel
> to interfaces without carrier, rather than being correctly rerouted
> over alternate paths.

I see, thanks for the explanation.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 16:17   ` YOSHIFUJI Hideaki / 吉藤英明
@ 2008-06-03 17:21     ` Stephen Hemminger
  2008-06-03 17:25       ` David Miller
  0 siblings, 1 reply; 24+ messages in thread
From: Stephen Hemminger @ 2008-06-03 17:21 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / 吉藤英明
  Cc: davem, netdev, yoshfuji

On Wed, 04 Jun 2008 01:17:17 +0900 (JST)
YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> wrote:

> In article <20080602165347.5662f602@extreme> (at Mon, 2 Jun 2008 16:53:47 -0700), Stephen Hemminger <shemminger@vyatta.com> says:
> 
> > Similar to previous change for IPV4. Allow configuration option to remove
> > addresses from FIB when carrier is lost.
> > 
> > This patch also makes IPV6 ignore changes to carrier state when device
> > is admin down state. This prevents address configuration from starting
> > up on a buggy device driver that signals carrier changes when offline.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> Well, we need to be careful here.
> IMHO we should not remove addresses (especially link-local address
> based on MAC, or manually assigned addresses, maybe).
> We might want to do Optimistic DAD here.
> 
> --yoshfuji

The patch just makes carrier_off respond the same as doing 'ip link set dev eth0 down'
(or ifconfig eth0 down). A router needs to be able to re-route when link fails.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 17:21     ` Stephen Hemminger
@ 2008-06-03 17:25       ` David Miller
  2008-06-03 17:28         ` Patrick McHardy
  2008-06-03 17:28         ` YOSHIFUJI Hideaki / 吉藤英明
  0 siblings, 2 replies; 24+ messages in thread
From: David Miller @ 2008-06-03 17:25 UTC (permalink / raw)
  To: shemminger; +Cc: yoshfuji, netdev

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 3 Jun 2008 10:21:44 -0700

> The patch just makes carrier_off respond the same as doing 'ip link set dev eth0 down'
> (or ifconfig eth0 down). A router needs to be able to re-route when link fails.

But I can't see how this behavior makes sense for the normal desktop case
and it disagrees with existing practice for many years.

If I pull out my network cable while making some adjustments in my
rack, and then plug it back in, I don't expect to lose my static
routes on that interface.

That doesn't make any sense at all.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 17:25       ` David Miller
@ 2008-06-03 17:28         ` Patrick McHardy
  2008-06-03 17:31           ` David Miller
  2008-06-03 17:28         ` YOSHIFUJI Hideaki / 吉藤英明
  1 sibling, 1 reply; 24+ messages in thread
From: Patrick McHardy @ 2008-06-03 17:28 UTC (permalink / raw)
  To: David Miller; +Cc: shemminger, yoshfuji, netdev

David Miller wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 3 Jun 2008 10:21:44 -0700
> 
>> The patch just makes carrier_off respond the same as doing 'ip link set dev eth0 down'
>> (or ifconfig eth0 down). A router needs to be able to re-route when link fails.
> 
> But I can't see how this behavior makes sense for the normal desktop case
> and it disagrees with existing practice for many years.
> 
> If I pull out my network cable while making some adjustments in my
> rack, and then plug it back in, I don't expect to lose my static
> routes on that interface.
> 
> That doesn't make any sense at all.


Maybe the logic to disable those routes should simply live in
the routing daemons or a seperate daemon.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 17:25       ` David Miller
  2008-06-03 17:28         ` Patrick McHardy
@ 2008-06-03 17:28         ` YOSHIFUJI Hideaki / 吉藤英明
  2008-06-03 17:30           ` David Miller
  1 sibling, 1 reply; 24+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2008-06-03 17:28 UTC (permalink / raw)
  To: davem; +Cc: shemminger, netdev, yoshfuji

In article <20080603.102501.193702820.davem@davemloft.net> (at Tue, 03 Jun 2008 10:25:01 -0700 (PDT)), David Miller <davem@davemloft.net> says:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 3 Jun 2008 10:21:44 -0700
> 
> > The patch just makes carrier_off respond the same as doing 'ip link set dev eth0 down'
> > (or ifconfig eth0 down). A router needs to be able to re-route when link fails.
> 
> But I can't see how this behavior makes sense for the normal desktop case
> and it disagrees with existing practice for many years.
> 
> If I pull out my network cable while making some adjustments in my
> rack, and then plug it back in, I don't expect to lose my static
> routes on that interface.
> 
> That doesn't make any sense at all.

How about ignoring routes via down interface?

--yoshfuji

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 17:28         ` YOSHIFUJI Hideaki / 吉藤英明
@ 2008-06-03 17:30           ` David Miller
  2008-06-03 17:46             ` Stephen Hemminger
  0 siblings, 1 reply; 24+ messages in thread
From: David Miller @ 2008-06-03 17:30 UTC (permalink / raw)
  To: yoshfuji; +Cc: shemminger, netdev

From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Wed, 04 Jun 2008 02:28:36 +0900 (JST)

> In article <20080603.102501.193702820.davem@davemloft.net> (at Tue, 03 Jun 2008 10:25:01 -0700 (PDT)), David Miller <davem@davemloft.net> says:
> 
> > From: Stephen Hemminger <shemminger@vyatta.com>
> > Date: Tue, 3 Jun 2008 10:21:44 -0700
> > 
> > > The patch just makes carrier_off respond the same as doing 'ip link set dev eth0 down'
> > > (or ifconfig eth0 down). A router needs to be able to re-route when link fails.
> > 
> > But I can't see how this behavior makes sense for the normal desktop case
> > and it disagrees with existing practice for many years.
> > 
> > If I pull out my network cable while making some adjustments in my
> > rack, and then plug it back in, I don't expect to lose my static
> > routes on that interface.
> > 
> > That doesn't make any sense at all.
> 
> How about ignoring routes via down interface?

Look at what happens now in my example case.  The packets simply get
queued in the device queue until the carrier comes back up.  Once
it comes back up, the packets go out with zero packet loss.

With your suggestion, the packets will get dropped if there are no
other devices with active routes to the destination, which is a very
poor quality of implementation decision in my opinion, especially for
this case.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 17:28         ` Patrick McHardy
@ 2008-06-03 17:31           ` David Miller
  0 siblings, 0 replies; 24+ messages in thread
From: David Miller @ 2008-06-03 17:31 UTC (permalink / raw)
  To: kaber; +Cc: shemminger, yoshfuji, netdev

From: Patrick McHardy <kaber@trash.net>
Date: Tue, 03 Jun 2008 19:28:21 +0200

> Maybe the logic to disable those routes should simply live in
> the routing daemons or a seperate daemon.

Indeed.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 17:30           ` David Miller
@ 2008-06-03 17:46             ` Stephen Hemminger
  2008-06-03 17:53               ` David Miller
  0 siblings, 1 reply; 24+ messages in thread
From: Stephen Hemminger @ 2008-06-03 17:46 UTC (permalink / raw)
  To: David Miller; +Cc: yoshfuji, netdev

On Tue, 03 Jun 2008 10:30:59 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Date: Wed, 04 Jun 2008 02:28:36 +0900 (JST)
> 
> > In article <20080603.102501.193702820.davem@davemloft.net> (at Tue, 03 Jun 2008 10:25:01 -0700 (PDT)), David Miller <davem@davemloft.net> says:
> > 
> > > From: Stephen Hemminger <shemminger@vyatta.com>
> > > Date: Tue, 3 Jun 2008 10:21:44 -0700
> > > 
> > > > The patch just makes carrier_off respond the same as doing 'ip link set dev eth0 down'
> > > > (or ifconfig eth0 down). A router needs to be able to re-route when link fails.
> > > 
> > > But I can't see how this behavior makes sense for the normal desktop case
> > > and it disagrees with existing practice for many years.
> > > 
> > > If I pull out my network cable while making some adjustments in my
> > > rack, and then plug it back in, I don't expect to lose my static
> > > routes on that interface.
> > > 
> > > That doesn't make any sense at all.
> > 
> > How about ignoring routes via down interface?
> 
> Look at what happens now in my example case.  The packets simply get
> queued in the device queue until the carrier comes back up.  Once
> it comes back up, the packets go out with zero packet loss.
> 
> With your suggestion, the packets will get dropped if there are no
> other devices with active routes to the destination, which is a very
> poor quality of implementation decision in my opinion, especially for
> this case.

It is not what desktop users want, that is why it is a dynamic configuration
option via /proc/sys/net/ipv6/conf/ethX/link_detect.

But it is what a router wants. So why not allow it? Obviously, Vyatta
users expect systems behave same as Cisco.

The problem with doing it all in user space are not impossible just more difficult:
  * links bouncing lead to synchronization problems
  * existing Quagga code avoids messing with "system routes" 
  * Quagga has to be portable to Solaris/BSD etc..

More at:
http://osdir.com/ml/network.quagga.devel/2004-08/msg00009.html

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 17:46             ` Stephen Hemminger
@ 2008-06-03 17:53               ` David Miller
  2008-06-03 18:34                 ` Stephen Hemminger
  0 siblings, 1 reply; 24+ messages in thread
From: David Miller @ 2008-06-03 17:53 UTC (permalink / raw)
  To: shemminger; +Cc: yoshfuji, netdev

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 3 Jun 2008 10:46:40 -0700

> It is not what desktop users want, that is why it is a dynamic configuration
> option via /proc/sys/net/ipv6/conf/ethX/link_detect.
> 
> But it is what a router wants. So why not allow it? Obviously, Vyatta
> users expect systems behave same as Cisco.
> 
> The problem with doing it all in user space are not impossible just more difficult:
>   * links bouncing lead to synchronization problems
>   * existing Quagga code avoids messing with "system routes" 
>   * Quagga has to be portable to Solaris/BSD etc..
> 
> More at:
> http://osdir.com/ml/network.quagga.devel/2004-08/msg00009.html

If it's a route behavioral attribute, make it as such and add
a new rtnetlink route attribute.  If it's not there, existing
behavior is maintained.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 17:53               ` David Miller
@ 2008-06-03 18:34                 ` Stephen Hemminger
  2008-06-03 19:06                   ` David Miller
  0 siblings, 1 reply; 24+ messages in thread
From: Stephen Hemminger @ 2008-06-03 18:34 UTC (permalink / raw)
  To: David Miller; +Cc: yoshfuji, netdev

On Tue, 03 Jun 2008 10:53:08 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 3 Jun 2008 10:46:40 -0700
> 
> > It is not what desktop users want, that is why it is a dynamic configuration
> > option via /proc/sys/net/ipv6/conf/ethX/link_detect.
> > 
> > But it is what a router wants. So why not allow it? Obviously, Vyatta
> > users expect systems behave same as Cisco.
> > 
> > The problem with doing it all in user space are not impossible just more difficult:
> >   * links bouncing lead to synchronization problems
> >   * existing Quagga code avoids messing with "system routes" 
> >   * Quagga has to be portable to Solaris/BSD etc..
> > 
> > More at:
> > http://osdir.com/ml/network.quagga.devel/2004-08/msg00009.html
> 
> If it's a route behavioral attribute, make it as such and add
> a new rtnetlink route attribute.  If it's not there, existing
> behavior is maintained.
> 

How would this work for system generated routes which occur
when address is added to interface?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 18:34                 ` Stephen Hemminger
@ 2008-06-03 19:06                   ` David Miller
  2008-06-03 21:40                     ` Stephen Hemminger
  0 siblings, 1 reply; 24+ messages in thread
From: David Miller @ 2008-06-03 19:06 UTC (permalink / raw)
  To: shemminger; +Cc: yoshfuji, netdev

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 3 Jun 2008 11:34:11 -0700

> On Tue, 03 Jun 2008 10:53:08 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
> > If it's a route behavioral attribute, make it as such and add
> > a new rtnetlink route attribute.  If it's not there, existing
> > behavior is maintained.
> 
> How would this work for system generated routes which occur
> when address is added to interface?

Ok, then this takes us back to making userland take care of it.

You say this is difficult, by how different is this from any
other kind of event response and synchronization that these
routing daemons have to do already?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV6: remove addresses and routes when carrier is lost
  2008-06-03 19:06                   ` David Miller
@ 2008-06-03 21:40                     ` Stephen Hemminger
  0 siblings, 0 replies; 24+ messages in thread
From: Stephen Hemminger @ 2008-06-03 21:40 UTC (permalink / raw)
  To: David Miller; +Cc: yoshfuji, netdev

On Tue, 03 Jun 2008 12:06:40 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 3 Jun 2008 11:34:11 -0700
> 
> > On Tue, 03 Jun 2008 10:53:08 -0700 (PDT)
> > David Miller <davem@davemloft.net> wrote:
> > 
> > > If it's a route behavioral attribute, make it as such and add
> > > a new rtnetlink route attribute.  If it's not there, existing
> > > behavior is maintained.
> > 
> > How would this work for system generated routes which occur
> > when address is added to interface?
> 
> Ok, then this takes us back to making userland take care of it.
> 
> You say this is difficult, by how different is this from any
> other kind of event response and synchronization that these
> routing daemons have to do already?

The problem is more that the zebra code is keeping track of the
RIB and has flags as well. Also, since maintainers are BSD/Solaris
based, it makes life harder. 

Have already fixed a number of bugs in that area, of the route
code where connected and recursive routes aren't getting cleared.
It's not impossible to fix, just a bigger burden.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-03 15:57   ` Stephen Hemminger
  2008-06-03 16:03     ` Patrick McHardy
@ 2008-06-03 21:44     ` Lennart Sorensen
  1 sibling, 0 replies; 24+ messages in thread
From: Lennart Sorensen @ 2008-06-03 21:44 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Stephen Hemminger, David Miller, netdev

On Tue, Jun 03, 2008 at 08:57:07AM -0700, Stephen Hemminger wrote:
> The patch makes carrier down == interface down. So the same
> behaviour as doing 'ip link set dev eth3 down'

That's what I thought from looking at it.

> If you are using a routing daemon like quagga, it will restore your static
> routes.

I never looked at using quagga to manage static routes. Perhaps I
should.

> That is pretty much what zebra portion of quagga does.

Hmm, other than zebra won't delete the route of the interface itself, it
seems to only touch routes it added.  Your patch would solve that
problem though.

-- 
Len Sorensen

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-03 16:03     ` Patrick McHardy
  2008-06-03 16:31       ` Stephen Hemminger
@ 2008-06-03 21:45       ` Lennart Sorensen
  2008-06-03 22:32         ` Patrick McHardy
  1 sibling, 1 reply; 24+ messages in thread
From: Lennart Sorensen @ 2008-06-03 21:45 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Stephen Hemminger, Stephen Hemminger, David Miller, netdev

On Tue, Jun 03, 2008 at 06:03:46PM +0200, Patrick McHardy wrote:
> Can't the routing daemon simply ignore routes with a
> device that has no carrier?

zebra won't add a route that is already provided by the kernel.  The
kernel has to remove it's route when the link is down before zebra can
provide an alternative.

-- 
Len Sorensen

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-03 21:45       ` Lennart Sorensen
@ 2008-06-03 22:32         ` Patrick McHardy
  2008-06-04 13:53           ` Lennart Sorensen
  0 siblings, 1 reply; 24+ messages in thread
From: Patrick McHardy @ 2008-06-03 22:32 UTC (permalink / raw)
  To: Lennart Sorensen
  Cc: Stephen Hemminger, Stephen Hemminger, David Miller, netdev

Lennart Sorensen wrote:
> On Tue, Jun 03, 2008 at 06:03:46PM +0200, Patrick McHardy wrote:
>> Can't the routing daemon simply ignore routes with a
>> device that has no carrier?
> 
> zebra won't add a route that is already provided by the kernel.  The
> kernel has to remove it's route when the link is down before zebra can
> provide an alternative.

That should be easily fixed, the kernel provides all necessary
information to make this decision in userspace.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-03 22:32         ` Patrick McHardy
@ 2008-06-04 13:53           ` Lennart Sorensen
  0 siblings, 0 replies; 24+ messages in thread
From: Lennart Sorensen @ 2008-06-04 13:53 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Stephen Hemminger, Stephen Hemminger, David Miller, netdev

On Wed, Jun 04, 2008 at 12:32:22AM +0200, Patrick McHardy wrote:
> That should be easily fixed, the kernel provides all necessary
> information to make this decision in userspace.

It seems much cleaner for zebra to only ever touch routes that it
created, which would be the ones of type zebra.  Type kernel seems best
only managed by the kernel.

-- 
Len Sorensen

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost
  2008-06-02 23:52 [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost Stephen Hemminger
  2008-06-02 23:53 ` [PATCH 1/2] IPV6: " Stephen Hemminger
  2008-06-03 15:05 ` [PATCH 1/2] IPV4: " Lennart Sorensen
@ 2008-06-06 14:56 ` Lennart Sorensen
  2 siblings, 0 replies; 24+ messages in thread
From: Lennart Sorensen @ 2008-06-06 14:56 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev

On Mon, Jun 02, 2008 at 04:52:49PM -0700, Stephen Hemminger wrote:
> This patch adds a new configuration sysctl that causes link loss to clear
> FIB state in the same way as admin down. This allows for routing daemons
> like Quagga which have option to remove routes when carrier is lost.
> 
> This has been a long standing problem with Quagga on Linux with complaints
> on the developers list going back to 2004. Fixing it properly, so the routing 
> daemon manages the RIB, and the kernel manages the FIB, requires changes to
> both parts.

There is one slight annoyance in the behaviour of this patch.

It only takes effect the first time link is lost, so if you boot with a
port without link, its addresses and routes are still added, and only
after it gains link and looses it again does it get removed.  Makes for
a somewhat inconsistent behaviour unfortunately.

Any idea how to change that?

Otherwise this looks like a very useful patch when combined with zebra
to manage static routes with link-detect (along with vyatta's updates to
zebra to make it actually obey link detect for routes).

-- 
Len Sorensen

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2008-06-06 14:56 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-02 23:52 [PATCH 1/2] IPV4: remove addresses and routes when carrier is lost Stephen Hemminger
2008-06-02 23:53 ` [PATCH 1/2] IPV6: " Stephen Hemminger
2008-06-03 16:17   ` YOSHIFUJI Hideaki / 吉藤英明
2008-06-03 17:21     ` Stephen Hemminger
2008-06-03 17:25       ` David Miller
2008-06-03 17:28         ` Patrick McHardy
2008-06-03 17:31           ` David Miller
2008-06-03 17:28         ` YOSHIFUJI Hideaki / 吉藤英明
2008-06-03 17:30           ` David Miller
2008-06-03 17:46             ` Stephen Hemminger
2008-06-03 17:53               ` David Miller
2008-06-03 18:34                 ` Stephen Hemminger
2008-06-03 19:06                   ` David Miller
2008-06-03 21:40                     ` Stephen Hemminger
2008-06-03 15:05 ` [PATCH 1/2] IPV4: " Lennart Sorensen
2008-06-03 15:57   ` Stephen Hemminger
2008-06-03 16:03     ` Patrick McHardy
2008-06-03 16:31       ` Stephen Hemminger
2008-06-03 16:44         ` Patrick McHardy
2008-06-03 21:45       ` Lennart Sorensen
2008-06-03 22:32         ` Patrick McHardy
2008-06-04 13:53           ` Lennart Sorensen
2008-06-03 21:44     ` Lennart Sorensen
2008-06-06 14:56 ` Lennart Sorensen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).