Skip to contentSkip to conversation

Defer IPv6 device initialization until a valid qdisc is specified

PATCH In review Mitsuru Chinen 4 participants 5 messages Oct 9, 2007 – Oct 10, 2007

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Defer IPv6 device initialization until a valid qdisc is specified

To judge the timing for DAD, netif_carrier_ok() is used. However, there is a possibility that dev->qdisc stays noop_qdisc even if netif_carrier_ok() returns true. In that case, DAD NS is not sent out. We need to defer the IPv6 device initialization until a valid qdisc is specified.

---
 net/ipv6/addrconf.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
net/ipv6/addrconf.c +10 -3
index 6d5c3c2..da8c79c 100644
@@ -74,6 +74,7 @@
 #include <net/tcp.h>
 #include <net/ip.h>
 #include <net/netlink.h>
+#include <net/pkt_sched.h>
 #include <linux/if_tunnel.h>
 #include <linux/rtnetlink.h>
 
@@ -213,6 +214,12 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 const struct in6_addr in6addr_any = IN6ADDR_ANY_INIT;
 const struct in6_addr in6addr_loopback = IN6ADDR_LOOPBACK_INIT;
 
+/* Check if a valid qdisc is available */
+static inline int addrconf_qdisc_ok(struct net_device *dev)
+{
+	return (dev->qdisc != &noop_qdisc);
+}
+
 static void addrconf_del_timer(struct inet6_ifaddr *ifp)
 {
 	if (del_timer(&ifp->timer))
@@ -384,7 +391,7 @@ static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
 	}
 #endif
 
-	if (netif_running(dev) && netif_carrier_ok(dev))
+	if (netif_running(dev) && addrconf_qdisc_ok(dev))
 		ndev->if_flags |= IF_READY;
 
 	ipv6_mc_init_dev(ndev);
@@ -2283,7 +2290,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			break;
 
 		if (event == NETDEV_UP) {
-			if (!netif_carrier_ok(dev)) {
+			if (!addrconf_qdisc_ok(dev)) {
 				/* device is not ready yet. */
 				printk(KERN_INFO
 					"ADDRCONF(NETDEV_UP): %s: "
@@ -2295,7 +2302,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			if (idev)
 				idev->if_flags |= IF_READY;
 		} else {
-			if (!netif_carrier_ok(dev)) {
+			if (!addrconf_qdisc_ok(dev)) {
 				/* device is still not ready. */
 				break;
 			}
Signature · 1 line
1.5.2.2
#1 lore raw
Quoted 5 lines · Mitsuru Chinen

To judge the timing for DAD, netif_carrier_ok() is used. However, there is a possibility that dev->qdisc stays noop_qdisc even if netif_carrier_ok() returns true. In that case, DAD NS is not sent out. We need to defer the IPv6 device initialization until a valid qdisc is specified.

Is this really necessary?

What if this is plugged into a switch that has nothing else connected? Should you also wait for something to be connected to the switch?

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#2 lore raw

Hello.

In article <[email protected]> (at Wed, 10 Oct 2007 09:32:35 +0800), Herbert Xu <[email protected]> says:

Quoted 7 lines
Quoted 5 lines · Mitsuru Chinen

To judge the timing for DAD, netif_carrier_ok() is used. However, there is a possibility that dev->qdisc stays noop_qdisc even if netif_carrier_ok() returns true. In that case, DAD NS is not sent out. We need to defer the IPv6 device initialization until a valid qdisc is specified.

Quoted 5 lines

Is this really necessary?

What if this is plugged into a switch that has nothing else connected? Should you also wait for something to be connected to the switch?

While MLD, DAD (and RS) packets must be observed on the wire outside the box, we've been observing random "failures" with our test system due to this bug.
The bug results in failures of detecting duplicate address (our side and the other side afterwards), and of having valid global address(es).

Even in practical system, a box sometimes fail to have valid global address for 10 minutes or so, which is not good. if MLD snooping is supported and enabled on the switch, we may fail to insist our address is already in-use against DAD from other node.

We'd really like to do our best to avoid such random failures even if it may/can not be perfect.

Regards,

--yoshfuji

#3 lore raw
Quoted 3 lines · YOSHIFUJI Hideaki / ????????????

We'd really like to do our best to avoid such random failures even if it may/can not be perfect.

OK I see your point of view but there's got to be a better way than this.

As it is your tests will fail randomly anyway if you do it in virtual guest because the carrier is always on since the switch in that case is the host OS.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#4 lore raw
In reply to Mitsuru Chinen

From: Mitsuru Chinen <[email protected]>
Date: Tue, 9 Oct 2007 16:21:58 +0900

Quoted 8 lines

To judge the timing for DAD, netif_carrier_ok() is used. However, there is a possibility that dev->qdisc stays noop_qdisc even if netif_carrier_ok() returns true. In that case, DAD NS is not sent out. We need to defer the IPv6 device initialization until a valid qdisc is specified.

Thanks for submitting the fix.

Although Herbert is right that this does not fix the problem universally, it does make things better, so I will apply this patch.

Thanks!