| Message ID | 1406735921-122830-1-git-send-email-equinox@diac24.net |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mi, 2014-07-30 at 17:58 +0200, David Lamparter wrote: > This realigns addrconf support for the various lower-layer device types, > and removes a little bit of duplicate code. > > For GRE devices, this includes a semantic change in that there is now a > ff00::/8 route installed on address autogeneration. This was previously > missing and broke any kind of IPv6 multicast - unless another address > was configured from userspace (which then added the missing ff00::/8). > > Fixes: aee80b54b235 (ipv6: generate link local address for GRE tunnel) > Signed-off-by: David Lamparter <equinox@diac24.net> > Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> > Cc: Stephen Hemminger <stephen@networkplumber.org> > Cc: Jiri Pirko <jiri@resnulli.us> > --- > > This is an alternate version, yanking the switch() down and removing > dev_config/gre_config duplication. I have no idea what rationale is behind > prefix_route - the result is a fe80::/64 route, but no address, which is not a > functioning configuration. Jiri, you touched this just a few weeks ago, can > you comment? (The "XXX: why is GRE special?") Sure, it is valid. You can still use global addresses to talk to link local addresses on the same link, even from another interface. I prefer this patch. Thanks, Hannes -- 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
On Wed, Jul 30, 2014 at 06:12:35PM +0200, Hannes Frederic Sowa wrote: > On Mi, 2014-07-30 at 17:58 +0200, David Lamparter wrote: [cut] > > This is an alternate version, yanking the switch() down and removing > > dev_config/gre_config duplication. I have no idea what rationale is behind > > prefix_route - the result is a fe80::/64 route, but no address, which is not a > > functioning configuration. Jiri, you touched this just a few weeks ago, can > > you comment? (The "XXX: why is GRE special?") > > Sure, it is valid. You can still use global addresses to talk to link > local addresses on the same link, even from another interface. Okay, well, that may give some purpose to it, but doesn't really explain why GRE is special in this regard... (And it's a violation of RFC4291 section 2.1 - "All interfaces are required to have at least one Link-Local unicast address" - and I'd bet on ndisc doing weird things in a setup like that.) -David -- 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
On Mi, 2014-07-30 at 18:23 +0200, David Lamparter wrote: > On Wed, Jul 30, 2014 at 06:12:35PM +0200, Hannes Frederic Sowa wrote: > > On Mi, 2014-07-30 at 17:58 +0200, David Lamparter wrote: > [cut] > > > This is an alternate version, yanking the switch() down and removing > > > dev_config/gre_config duplication. I have no idea what rationale is behind > > > prefix_route - the result is a fe80::/64 route, but no address, which is not a > > > functioning configuration. Jiri, you touched this just a few weeks ago, can > > > you comment? (The "XXX: why is GRE special?") > > > > Sure, it is valid. You can still use global addresses to talk to link > > local addresses on the same link, even from another interface. > > Okay, well, that may give some purpose to it, but doesn't really explain > why GRE is special in this regard... > > (And it's a violation of RFC4291 section 2.1 - "All interfaces are > required to have at least one Link-Local unicast address" - and I'd bet > on ndisc doing weird things in a setup like that.) Yep - sure. But we also allow someone to remove the ll address manually. And people did do that to transition the interface into an disable_ipv6=1 mode, which had its own problems. We don't care if the user does rfc compliant configurations. :) The settings in the kernel should just be reasonable. -- 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
On Mi, 2014-07-30 at 17:58 +0200, David Lamparter wrote: > This realigns addrconf support for the various lower-layer device types, > and removes a little bit of duplicate code. > > For GRE devices, this includes a semantic change in that there is now a > ff00::/8 route installed on address autogeneration. This was previously > missing and broke any kind of IPv6 multicast - unless another address > was configured from userspace (which then added the missing ff00::/8). > > Fixes: aee80b54b235 (ipv6: generate link local address for GRE tunnel) > Signed-off-by: David Lamparter <equinox@diac24.net> > Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> > Cc: Stephen Hemminger <stephen@networkplumber.org> > Cc: Jiri Pirko <jiri@resnulli.us> > --- > > This is an alternate version, yanking the switch() down and removing > dev_config/gre_config duplication. I have no idea what rationale is behind > prefix_route - the result is a fe80::/64 route, but no address, which is not a > functioning configuration. Jiri, you touched this just a few weeks ago, can > you comment? (The "XXX: why is GRE special?") IIRC some time ago it was decided that randomizing the perm_addr and generate LL addresses based on that is not the way to go and fragile. We have this behaviour for ipv6 tunnels, but (IIRC) should not be expanded and leave user space with this burden. > -#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(); > > - if ((idev = ipv6_find_idev(dev)) == NULL) { > - pr_debug("%s: add_dev failed\n", __func__); > + 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: We can now also add ARPHRD_IP6GRE, maybe in a separate patch. Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> Thanks, Hannes -- 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 --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 0b239fc..45746f7 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,21 +2786,51 @@ 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(); - if ((idev = ipv6_find_idev(dev)) == NULL) { - pr_debug("%s: add_dev failed\n", __func__); + 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: why is GRE special? */ + 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; } - addrconf_addr_gen(idev, true); + idev = addrconf_add_dev(dev); + if (IS_ERR(idev)) + return; + + addrconf_addr_gen(idev, prefix_route); } -#endif static int addrconf_notify(struct notifier_block *this, unsigned long event, void *ptr) @@ -2883,25 +2888,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)
This realigns addrconf support for the various lower-layer device types, and removes a little bit of duplicate code. For GRE devices, this includes a semantic change in that there is now a ff00::/8 route installed on address autogeneration. This was previously missing and broke any kind of IPv6 multicast - unless another address was configured from userspace (which then added the missing ff00::/8). Fixes: aee80b54b235 (ipv6: generate link local address for GRE tunnel) Signed-off-by: David Lamparter <equinox@diac24.net> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> Cc: Stephen Hemminger <stephen@networkplumber.org> Cc: Jiri Pirko <jiri@resnulli.us> --- This is an alternate version, yanking the switch() down and removing dev_config/gre_config duplication. I have no idea what rationale is behind prefix_route - the result is a fe80::/64 route, but no address, which is not a functioning configuration. Jiri, you touched this just a few weeks ago, can you comment? (The "XXX: why is GRE special?") net/ipv6/addrconf.c | 87 +++++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 50 deletions(-)