diff mbox

[2/2] ipv6: addrconf: clean up device type handling

Message ID 1406840034-140848-3-git-send-email-equinox@diac24.net
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

David Lamparter July 31, 2014, 8:53 p.m. UTC
This realigns addrconf support for the various lower-layer device types,
and removes a little bit of duplicate code.  Behaviour should be exactly
identical before and after the patch.

(Also adds a comment to explain the weird fe80::/64 on GRE.)

Signed-off-by: David Lamparter <equinox@diac24.net>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv6/addrconf.c | 87 ++++++++++++++++++++++++-----------------------------
 1 file changed, 39 insertions(+), 48 deletions(-)

Comments

Hannes Frederic Sowa July 31, 2014, 10:13 p.m. UTC | #1
On Do, 2014-07-31 at 22:53 +0200, David Lamparter wrote:
> This realigns addrconf support for the various lower-layer device types,
> and removes a little bit of duplicate code.  Behaviour should be exactly
> identical before and after the patch.
> 
> (Also adds a comment to explain the weird fe80::/64 on GRE.)
> 
> Signed-off-by: David Lamparter <equinox@diac24.net>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>  net/ipv6/addrconf.c | 87 ++++++++++++++++++++++++-----------------------------
>  1 file changed, 39 insertions(+), 48 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 7540a25..aa17ee6 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2756,31 +2756,6 @@ static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
>  	}
>  }
>  
> -static void addrconf_dev_config(struct net_device *dev)
> -{
> -	struct inet6_dev *idev;
> -
> -	ASSERT_RTNL();
> -
> -	if ((dev->type != ARPHRD_ETHER) &&
> -	    (dev->type != ARPHRD_FDDI) &&
> -	    (dev->type != ARPHRD_ARCNET) &&
> -	    (dev->type != ARPHRD_INFINIBAND) &&
> -	    (dev->type != ARPHRD_IEEE802154) &&
> -	    (dev->type != ARPHRD_IEEE1394) &&
> -	    (dev->type != ARPHRD_TUNNEL6) &&
> -	    (dev->type != ARPHRD_6LOWPAN)) {
> -		/* Alas, we support only Ethernet autoconfiguration. */
> -		return;
> -	}
> -
> -	idev = addrconf_add_dev(dev);
> -	if (IS_ERR(idev))
> -		return;
> -
> -	addrconf_addr_gen(idev, false);
> -}
> -
>  #if IS_ENABLED(CONFIG_IPV6_SIT)
>  static void addrconf_sit_config(struct net_device *dev)
>  {
> @@ -2811,20 +2786,54 @@ static void addrconf_sit_config(struct net_device *dev)
>  }
>  #endif
>  
> -#if IS_ENABLED(CONFIG_NET_IPGRE)
> -static void addrconf_gre_config(struct net_device *dev)
> +static void addrconf_dev_config(struct net_device *dev)
>  {
>  	struct inet6_dev *idev;
> +	bool prefix_route;
>  
>  	ASSERT_RTNL();
>  
> +	switch (dev->type) {
> +	case ARPHRD_LOOPBACK:
> +		init_loopback(dev);
> +		return;
> +
> +	case ARPHRD_ETHER:
> +	case ARPHRD_FDDI:
> +	case ARPHRD_ARCNET:
> +	case ARPHRD_INFINIBAND:
> +	case ARPHRD_IEEE802154:
> +	case ARPHRD_IEEE1394:
> +	case ARPHRD_TUNNEL6:
> +	case ARPHRD_6LOWPAN:
> +		prefix_route = false;
> +		break;
> +
> +#if IS_ENABLED(CONFIG_NET_IPGRE)
> +	case ARPHRD_IPGRE:
> +		/* XXX: This special-case behaviour is there to have a
> +		 * fe80::/64 route around even if no link-local address gets
> +		 * generated, which happens when gre's local == 0.0.0.0.
> +		 * See commit 08b4465 for more information. */
> +		prefix_route = true;
> +		break;
> +#endif

I don't see a need for the macro protection any more. We can drop this
now.

David, can you respin this series without the macros and maybe add the

"case ARPHRD_IP6GRE:" stuff

as a separate patch (which should also go into net/stable). If you don't
want to do this, I also can take care of this, no problem.

You can then repost this one separate and state in the comment section
that this cleanup patch depends on the other two, so Dave can merge it
accordingly to net and net-next?

Otherwise this patch is fine, thanks!


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Aug. 1, 2014, 5:31 a.m. UTC | #2
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 01 Aug 2014 00:13:35 +0200

> David, can you respin this series without the macros and maybe add the
> 
> "case ARPHRD_IP6GRE:" stuff
> 
> as a separate patch (which should also go into net/stable). If you don't
> want to do this, I also can take care of this, no problem.
> 
> You can then repost this one separate and state in the comment section
> that this cleanup patch depends on the other two, so Dave can merge it
> accordingly to net and net-next?

Indeed, please respin this series making the target tree(s) more
explicit, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 7540a25..aa17ee6 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2756,31 +2756,6 @@  static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
 	}
 }
 
-static void addrconf_dev_config(struct net_device *dev)
-{
-	struct inet6_dev *idev;
-
-	ASSERT_RTNL();
-
-	if ((dev->type != ARPHRD_ETHER) &&
-	    (dev->type != ARPHRD_FDDI) &&
-	    (dev->type != ARPHRD_ARCNET) &&
-	    (dev->type != ARPHRD_INFINIBAND) &&
-	    (dev->type != ARPHRD_IEEE802154) &&
-	    (dev->type != ARPHRD_IEEE1394) &&
-	    (dev->type != ARPHRD_TUNNEL6) &&
-	    (dev->type != ARPHRD_6LOWPAN)) {
-		/* Alas, we support only Ethernet autoconfiguration. */
-		return;
-	}
-
-	idev = addrconf_add_dev(dev);
-	if (IS_ERR(idev))
-		return;
-
-	addrconf_addr_gen(idev, false);
-}
-
 #if IS_ENABLED(CONFIG_IPV6_SIT)
 static void addrconf_sit_config(struct net_device *dev)
 {
@@ -2811,20 +2786,54 @@  static void addrconf_sit_config(struct net_device *dev)
 }
 #endif
 
-#if IS_ENABLED(CONFIG_NET_IPGRE)
-static void addrconf_gre_config(struct net_device *dev)
+static void addrconf_dev_config(struct net_device *dev)
 {
 	struct inet6_dev *idev;
+	bool prefix_route;
 
 	ASSERT_RTNL();
 
+	switch (dev->type) {
+	case ARPHRD_LOOPBACK:
+		init_loopback(dev);
+		return;
+
+	case ARPHRD_ETHER:
+	case ARPHRD_FDDI:
+	case ARPHRD_ARCNET:
+	case ARPHRD_INFINIBAND:
+	case ARPHRD_IEEE802154:
+	case ARPHRD_IEEE1394:
+	case ARPHRD_TUNNEL6:
+	case ARPHRD_6LOWPAN:
+		prefix_route = false;
+		break;
+
+#if IS_ENABLED(CONFIG_NET_IPGRE)
+	case ARPHRD_IPGRE:
+		/* XXX: This special-case behaviour is there to have a
+		 * fe80::/64 route around even if no link-local address gets
+		 * generated, which happens when gre's local == 0.0.0.0.
+		 * See commit 08b4465 for more information. */
+		prefix_route = true;
+		break;
+#endif
+#if IS_ENABLED(CONFIG_IPV6_SIT)
+	case ARPHRD_SIT:
+		addrconf_sit_config(dev);
+		return;
+#endif
+	default:
+		/* No support autoconfiguration on this type */
+		return;
+	}
+
 	idev = addrconf_add_dev(dev);
 	if (IS_ERR(idev))
 		return;
 
-	addrconf_addr_gen(idev, true);
+	addrconf_addr_gen(idev, prefix_route);
 }
-#endif
 
 static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			   void *ptr)
@@ -2882,25 +2891,7 @@  static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			run_pending = 1;
 		}
 
-		switch (dev->type) {
-#if IS_ENABLED(CONFIG_IPV6_SIT)
-		case ARPHRD_SIT:
-			addrconf_sit_config(dev);
-			break;
-#endif
-#if IS_ENABLED(CONFIG_NET_IPGRE)
-		case ARPHRD_IPGRE:
-			addrconf_gre_config(dev);
-			break;
-#endif
-		case ARPHRD_LOOPBACK:
-			init_loopback(dev);
-			break;
-
-		default:
-			addrconf_dev_config(dev);
-			break;
-		}
+		addrconf_dev_config(dev);
 
 		if (!IS_ERR_OR_NULL(idev)) {
 			if (run_pending)