Skip to contentSkip to conversation

ipv6 addrconf.c : wrong handling of non-ipv6 hardware since 2.6.21

PATCH In review Oliver Hartkopp 4 participants 7 messages May 17, 2007 – May 23, 2007

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

PATCH 1/1 ipv6 addrconf.c : wrong handling of non-ipv6 hardware since 2.6.21

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

---
*--- 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) {

*

#1 lore raw

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

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

---

net/ipv6/addrconf.c 2007-05-16 02:01:18.000000000 +0200 renamed from net/ipv6/addrconf.c.orig 2007-05-16 01:43:30.000000000 +0200 +0 -9
@@ -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) {
#2 lore raw
In reply to Oliver Hartkopp

In article <[email protected]> (at Thu, 17 May 2007 18:53:03 +0200), Oliver Hartkopp <[email protected]> says:

Quoted 7 lines
(quoted diff) +7 -0
+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

Review

Quoted 10 lines
(quoted diff) +10 -0
+        (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.

Signature · 2 lines
YOSHIFUJI Hideaki @ USAGI Project  <[email protected]>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA
#3 lore raw
Quoted 25 lines · YOSHIFUJI Hideaki / 吉藤英明

In article <[email protected]> (at Thu, 17 May 2007 18:53:03 +0200), Oliver Hartkopp <[email protected]> says:

Quoted 8 lines
(quoted diff) +7 -0
+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

Review

Quoted 10 lines
(quoted diff) +10 -0
+        (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.)

Quoted 3 lines

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

#4 lore raw

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

In reply to YOSHIFUJI Hideaki / 吉藤英明
Quoted 6 lines · YOSHIFUJI Hideaki / 吉藤英明

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

net/ipv6/addrconf.c 2007-05-18 00:57:11.000000000 +0200 renamed from net/ipv6/addrconf.c.orig 2007-05-18 00:54:54.000000000 +0200 +0 -9
@@ -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) {
#5 lore raw

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

Quoted 2 lines · Oliver Hartkopp

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.

urs

usr/src/linux-2.6.21/net/ipv6/addrconf.c renamed from usr/src/linux-2.6.21/net/ipv6/addrconf.c.orig +21 -10
@@ -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) {
maintainer
#6 lore raw

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

From: Urs Thuermann <[email protected]>
Date: 21 May 2007 20:20:51 +0200

Quoted 5 lines
Quoted 2 lines · Oliver Hartkopp

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

Quoted 8 lines

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.

Patch applied, thank you.