[PATCH] [IPV6] Defer IPv6 device initialization until a valid qdisc is specified

[PATCH] [IPV6] Defer IPv6 device initialization until a valid qdisc is specified

[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.

Signed-off-by: Mitsuru Chinen <mitch@linux.vnet.ibm.com>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/addrconf.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6d5c3c2..da8c79c 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -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;
 			}
-- 
1.5.2.2

Re: [PATCH] [IPV6] Defer IPv6 device initialization until a valid qdisc is specified

[Herbert Xu]

Mitsuru Chinen <mitch@linux.vnet.ibm.com> wrote:
> 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,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] [IPV6] Defer IPv6 device initialization until a valid qdisc is specified

[YOSHIFUJI Hideaki / 吉藤英明]

Hello.

In article <E1IfQR9-0006j8-00@gondolin.me.apana.org.au> (at Wed, 10 Oct 2007 09:32:35 +0800), Herbert Xu <herbert@gondor.apana.org.au> says:

> Mitsuru Chinen <mitch@linux.vnet.ibm.com> wrote:
> > 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?

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

Re: [PATCH] [IPV6] Defer IPv6 device initialization until a valid qdisc is specified

[Herbert Xu]

YOSHIFUJI Hideaki / ???????????? <yoshfuji@linux-ipv6.org> wrote:
> 
> 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,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] [IPV6] Defer IPv6 device initialization until a valid qdisc is specified

[David Miller]

From: Mitsuru Chinen <mitch@linux.vnet.ibm.com>
Date: Tue, 9 Oct 2007 16:21:58 +0900

> 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.
> 
> Signed-off-by: Mitsuru Chinen <mitch@linux.vnet.ibm.com>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

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!