[FIX][PATCH] ipv6 addrconf.c : wrong handling of non-ipv6 hardware since 2.6.21

[FIX][PATCH] ipv6 addrconf.c : wrong handling of non-ipv6 hardware since 2.6.21

[Oliver Hartkopp]

Hello Hideaki, hello David,

since 2.6.21 the new case NETDEV_REGISTER has been added to the big 
'switch' in addrconf_notify().

But within this "case NETDEV_REGISTER"-code no check about valid ipv6 
networking hardware is done, as it is done in NETDEV_UP, NETDEV_CHANGE 
and others.

Please apply the following patch to handle only valid ipv6 networking 
hardware inside the addrconf-notifier. This omits the invocation of 
ipv6_add_dev(dev) for non ipv6 capable hardware (e.g. with MTUs < 
IPV6_MIN_MTU) which leads to an ugly //"IPv6: add_dev failed for %s\n" 
kernel warning.//

The patch applies since 2.6.21 and also to the latest 2.6.22-rc1-git5 
(with an offset of 78 lines).

Best regards,
Oliver

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

---
*--- linux-2.6.21/net/ipv6/addrconf.c.orig    2007-05-16 
01:43:30.000000000 +0200
+++ linux-2.6.21/net/ipv6/addrconf.c    2007-05-16 02:01:18.000000000 +0200
@@ -2076,15 +2076,6 @@ static void addrconf_dev_config(struct n
 
     ASSERT_RTNL();
 
-    if ((dev->type != ARPHRD_ETHER) &&
-        (dev->type != ARPHRD_FDDI) &&
-        (dev->type != ARPHRD_IEEE802_TR) &&
-        (dev->type != ARPHRD_ARCNET) &&
-        (dev->type != ARPHRD_INFINIBAND)) {
-        /* Alas, we support only Ethernet autoconfiguration. */
-        return;
-    }
-
     idev = addrconf_add_dev(dev);
     if (idev == NULL)
         return;
@@ -2172,13 +2163,35 @@ static void addrconf_ip6_tnl_config(stru
     ip6_tnl_add_linklocal(idev);
 }
 
+static int ipv6_hwtype(struct net_device *dev)
+{
+    if ((dev->type == ARPHRD_ETHER) ||
+        (dev->type == ARPHRD_LOOPBACK) ||
+#if defined(CONFIG_IPV6_SIT) || defined(CONFIG_IPV6_SIT_MODULE)
+        (dev->type == ARPHRD_SIT) ||
+#endif
+        (dev->type == ARPHRD_TUNNEL6) ||
+        (dev->type == ARPHRD_FDDI) ||
+        (dev->type == ARPHRD_IEEE802_TR) ||
+        (dev->type == ARPHRD_ARCNET) ||
+        (dev->type == ARPHRD_INFINIBAND))
+        return 1;
+
+    return 0;
+}
+
 static int addrconf_notify(struct notifier_block *this, unsigned long 
event,
                void * data)
 {
     struct net_device *dev = (struct net_device *) data;
-    struct inet6_dev *idev = __in6_dev_get(dev);
+    struct inet6_dev *idev;
     int run_pending = 0;
 
+    if (!ipv6_hwtype(dev))
+        return NOTIFY_OK;
+
+    idev = __in6_dev_get(dev);
+
     switch(event) {
     case NETDEV_REGISTER:
         if (!idev) {
*

Resend: [FIX][PATCH] ipv6 addrconf.c : wrong handling of non-ipv6 hardware since 2.6.21

[Oliver Hartkopp]

Sorry. I had a problem with my mailer that disrupted the patch ...

--------------------------

Hello Hideaki, hello David,

since 2.6.21 the new case NETDEV_REGISTER has been added to the big 
'switch' in addrconf_notify().

But within this "case NETDEV_REGISTER"-code no check about valid ipv6 
networking hardware is done, as it is done in NETDEV_UP, NETDEV_CHANGE 
and others.

Please apply the following patch to handle only valid ipv6 networking 
hardware inside the addrconf-notifier. This omits the invocation of 
ipv6_add_dev(dev) for non ipv6 capable hardware (e.g. with MTUs < 
IPV6_MIN_MTU) which leads to an ugly "IPv6: add_dev failed for %s\n" 
kernel warning.

The patch applies since 2.6.21 and also to the latest 2.6.22-rc1-git5 
(with an offset of 78 lines).

Best regards,
Oliver

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

---
--- linux-2.6.21/net/ipv6/addrconf.c.orig    2007-05-16 
01:43:30.000000000 +0200
+++ linux-2.6.21/net/ipv6/addrconf.c    2007-05-16 02:01:18.000000000 +0200
@@ -2076,15 +2076,6 @@ static void addrconf_dev_config(struct n

    ASSERT_RTNL();

-    if ((dev->type != ARPHRD_ETHER) &&
-        (dev->type != ARPHRD_FDDI) &&
-        (dev->type != ARPHRD_IEEE802_TR) &&
-        (dev->type != ARPHRD_ARCNET) &&
-        (dev->type != ARPHRD_INFINIBAND)) {
-        /* Alas, we support only Ethernet autoconfiguration. */
-        return;
-    }
-
    idev = addrconf_add_dev(dev);
    if (idev == NULL)
        return;
@@ -2172,13 +2163,35 @@ static void addrconf_ip6_tnl_config(stru
    ip6_tnl_add_linklocal(idev);
}

+static int ipv6_hwtype(struct net_device *dev)
+{
+    if ((dev->type == ARPHRD_ETHER) ||
+        (dev->type == ARPHRD_LOOPBACK) ||
+#if defined(CONFIG_IPV6_SIT) || defined(CONFIG_IPV6_SIT_MODULE)
+        (dev->type == ARPHRD_SIT) ||
+#endif
+        (dev->type == ARPHRD_TUNNEL6) ||
+        (dev->type == ARPHRD_FDDI) ||
+        (dev->type == ARPHRD_IEEE802_TR) ||
+        (dev->type == ARPHRD_ARCNET) ||
+        (dev->type == ARPHRD_INFINIBAND))
+        return 1;
+
+    return 0;
+}
+
static int addrconf_notify(struct notifier_block *this, unsigned long 
event,
               void * data)
{
    struct net_device *dev = (struct net_device *) data;
-    struct inet6_dev *idev = __in6_dev_get(dev);
+    struct inet6_dev *idev;
    int run_pending = 0;

+    if (!ipv6_hwtype(dev))
+        return NOTIFY_OK;
+
+    idev = __in6_dev_get(dev);
+
    switch(event) {
    case NETDEV_REGISTER:
        if (!idev) {

Re: [FIX][PATCH] ipv6 addrconf.c : wrong handling of non-ipv6 hardware since 2.6.21

[YOSHIFUJI Hideaki / 吉藤英明]

In article <464C886F.50703@hartkopp.net> (at Thu, 17 May 2007 18:53:03 +0200), Oliver Hartkopp <socketcan@hartkopp.net> says:

> +static int ipv6_hwtype(struct net_device *dev)
> +{
> +    if ((dev->type == ARPHRD_ETHER) ||
> +        (dev->type == ARPHRD_LOOPBACK) ||
> +#if defined(CONFIG_IPV6_SIT) || defined(CONFIG_IPV6_SIT_MODULE)
> +        (dev->type == ARPHRD_SIT) ||
> +#endif
> +        (dev->type == ARPHRD_TUNNEL6) ||
> +        (dev->type == ARPHRD_FDDI) ||
> +        (dev->type == ARPHRD_IEEE802_TR) ||
> +        (dev->type == ARPHRD_ARCNET) ||
> +        (dev->type == ARPHRD_INFINIBAND))
> +        return 1;
> +
> +    return 0;
> +}
> +

Please remove #if, or please provide for other
dependencies as well (e.g., CONFIG_IPV6_TUNNEL etc.)

Otherwise, I would agree.

-- 
YOSHIFUJI Hideaki @ USAGI Project  <yoshfuji@linux-ipv6.org>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [FIX][PATCH] ipv6 addrconf.c : wrong handling of non-ipv6 hardware since 2.6.21

[Oliver Hartkopp]

YOSHIFUJI Hideaki / 吉藤英明 schrieb:
> In article <464C886F.50703@hartkopp.net> (at Thu, 17 May 2007 18:53:03 +0200), Oliver Hartkopp <socketcan@hartkopp.net> says:
>
>   
>> +static int ipv6_hwtype(struct net_device *dev)
>> +{
>> +    if ((dev->type == ARPHRD_ETHER) ||
>> +        (dev->type == ARPHRD_LOOPBACK) ||
>> +#if defined(CONFIG_IPV6_SIT) || defined(CONFIG_IPV6_SIT_MODULE)
>> +        (dev->type == ARPHRD_SIT) ||
>> +#endif
>> +        (dev->type == ARPHRD_TUNNEL6) ||
>> +        (dev->type == ARPHRD_FDDI) ||
>> +        (dev->type == ARPHRD_IEEE802_TR) ||
>> +        (dev->type == ARPHRD_ARCNET) ||
>> +        (dev->type == ARPHRD_INFINIBAND))
>> +        return 1;
>> +
>> +    return 0;
>> +}
>> +
>>     
>
> Please remove #if, or please provide for other
> dependencies as well (e.g., CONFIG_IPV6_TUNNEL etc.)
>
> Otherwise, I would agree.
>
>   
Hello Hideaki,

thanks for your (positive) feedback.
Indeed ARPHRD_SIT is the only hardware type that's explicitely #if 'ed 
in the addrconf.c code. Any other ARPHRD_* is not conditional to any 
CONFIGs. E.g. there is no #ifdef CONFIG_IPV6_TUNNEL arround the other 
ARPHRD_TUNNEL6 stuff in addrconf.c.

So the current patch meets the exact functional extension to the other 
code, where only ARPHRD_SIT is in #if 's.
Of course it is no problem to remove the #if defined(CONFIG_IPV6_SIT)  
... but this would be the only place in the code then.

Are you sure, that you really want have a divergent codestyle only here 
in ipv6_hwtype()?

If so i can post it of course.

Best Regards,
Oliver

Re: [FIX][PATCH] ipv6 addrconf.c REMOVED #if patch

[Oliver Hartkopp]

YOSHIFUJI Hideaki / 吉藤英明 wrote:
> Please remove #if, or please provide for other
> dependencies as well (e.g., CONFIG_IPV6_TUNNEL etc.)
>
> Otherwise, I would agree.
>
>   
Yep - you were right. It looks better like this as ARPHRD_SIT is defined 
in if_arp.h in any CONFIG_* case ...

Best regards & tnx,
Oliver


--- linux-2.6.21/net/ipv6/addrconf.c.orig    2007-05-18 
00:54:54.000000000 +0200
+++ linux-2.6.21/net/ipv6/addrconf.c    2007-05-18 00:57:11.000000000 +0200
@@ -2076,15 +2076,6 @@ static void addrconf_dev_config(struct n
 
     ASSERT_RTNL();
 
-    if ((dev->type != ARPHRD_ETHER) &&
-        (dev->type != ARPHRD_FDDI) &&
-        (dev->type != ARPHRD_IEEE802_TR) &&
-        (dev->type != ARPHRD_ARCNET) &&
-        (dev->type != ARPHRD_INFINIBAND)) {
-        /* Alas, we support only Ethernet autoconfiguration. */
-        return;
-    }
-
     idev = addrconf_add_dev(dev);
     if (idev == NULL)
         return;
@@ -2172,13 +2163,33 @@ static void addrconf_ip6_tnl_config(stru
     ip6_tnl_add_linklocal(idev);
 }
 
+static int ipv6_hwtype(struct net_device *dev)
+{
+    if ((dev->type == ARPHRD_ETHER) ||
+        (dev->type == ARPHRD_LOOPBACK) ||
+        (dev->type == ARPHRD_SIT) ||
+        (dev->type == ARPHRD_TUNNEL6) ||
+        (dev->type == ARPHRD_FDDI) ||
+        (dev->type == ARPHRD_IEEE802_TR) ||
+        (dev->type == ARPHRD_ARCNET) ||
+        (dev->type == ARPHRD_INFINIBAND))
+        return 1;
+
+    return 0;
+}
+
 static int addrconf_notify(struct notifier_block *this, unsigned long 
event,
                void * data)
 {
     struct net_device *dev = (struct net_device *) data;
-    struct inet6_dev *idev = __in6_dev_get(dev);
+    struct inet6_dev *idev;
     int run_pending = 0;
 
+    if (!ipv6_hwtype(dev))
+        return NOTIFY_OK;
+
+    idev = __in6_dev_get(dev);
+
     switch(event) {
     case NETDEV_REGISTER:
         if (!idev) {

Re: [FIX][PATCH] ipv6 addrconf.c REMOVED #if patch

[Urs Thuermann]

Oliver Hartkopp <socketcan@hartkopp.net> writes:

> Yep - you were right. It looks better like this as ARPHRD_SIT is
> defined in if_arp.h in any CONFIG_* case ...

Oliver asked me to resend the patch with a sane mailer that doesn't
mangle white-space and line breaks.  So here it is.

This patch makes the netdevice notifier for IPv6 ignore all events on
non-IPv6 capable devices.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>

urs



--- /usr/src/linux-2.6.21/net/ipv6/addrconf.c.orig	2007-05-21 19:47:36.000000000 +0200
+++ /usr/src/linux-2.6.21/net/ipv6/addrconf.c	2007-05-21 19:54:30.000000000 +0200
@@ -2076,15 +2076,6 @@
 
 	ASSERT_RTNL();
 
-	if ((dev->type != ARPHRD_ETHER) &&
-	    (dev->type != ARPHRD_FDDI) &&
-	    (dev->type != ARPHRD_IEEE802_TR) &&
-	    (dev->type != ARPHRD_ARCNET) &&
-	    (dev->type != ARPHRD_INFINIBAND)) {
-		/* Alas, we support only Ethernet autoconfiguration. */
-		return;
-	}
-
 	idev = addrconf_add_dev(dev);
 	if (idev == NULL)
 		return;
@@ -2172,13 +2163,33 @@
 	ip6_tnl_add_linklocal(idev);
 }
 
+static int ipv6_hwtype(struct net_device *dev)
+{
+	if ((dev->type == ARPHRD_ETHER) ||
+	    (dev->type == ARPHRD_LOOPBACK) ||
+	    (dev->type == ARPHRD_SIT) ||
+	    (dev->type == ARPHRD_TUNNEL6) ||
+	    (dev->type == ARPHRD_FDDI) ||
+	    (dev->type == ARPHRD_IEEE802_TR) ||
+	    (dev->type == ARPHRD_ARCNET) ||
+	    (dev->type == ARPHRD_INFINIBAND))
+		return 1;
+
+	return 0;
+}
+
 static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			   void * data)
 {
 	struct net_device *dev = (struct net_device *) data;
-	struct inet6_dev *idev = __in6_dev_get(dev);
+	struct inet6_dev *idev;
 	int run_pending = 0;
 
+	if (!ipv6_hwtype(dev))
+		return NOTIFY_OK;
+
+	idev = __in6_dev_get(dev);
+
 	switch(event) {
 	case NETDEV_REGISTER:
 		if (!idev) {

Re: [FIX][PATCH] ipv6 addrconf.c REMOVED #if patch

[David Miller]

From: Urs Thuermann <urs@isnogud.escape.de>
Date: 21 May 2007 20:20:51 +0200

> Oliver Hartkopp <socketcan@hartkopp.net> writes:
> 
> > Yep - you were right. It looks better like this as ARPHRD_SIT is
> > defined in if_arp.h in any CONFIG_* case ...
> 
> Oliver asked me to resend the patch with a sane mailer that doesn't
> mangle white-space and line breaks.  So here it is.
> 
> This patch makes the netdevice notifier for IPv6 ignore all events on
> non-IPv6 capable devices.
> 
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>

Patch applied, thank you.