| Message ID | 1406681743-104844-1-git-send-email-equinox@diac24.net |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi, On Mi, 2014-07-30 at 02:55 +0200, David Lamparter wrote: > GRE devices, for some reason, were coming up with an autoconfigured > address, but no ff00::/8 route in the local table. This breaks any kind > of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only > works at all because there is little need for ND on PtP devices. > > Adding any other IPv6 address on the device would rectify this issue > through inet6_addr_add()/addrconf_add_dev() - and would leave the route > around even if the address was later removed. (This is probably why > this issue was not discovered earlier. AFAICS it has been there from > the beginning, e.g. aee80b5 "generate link local address for GRE > tunnel") Yep, this is poor, but changing this will break user space... > (Note: multicast is supported on GRE devices of all kinds, including PtP > GRE, P-t-Mcast GRE and NBMA-GRE.) > > Fixes: aee80b54b235 (ipv6: generate link local address for GRE tunnel) > Signed-off-by: David Lamparter <equinox@diac24.net> > Cc: Stephen Hemminger <stephen@networkplumber.org> We should install routes before joining LL, I would go with idev = addrconf_add_dev(dev); if (IS_ERR(idev)) return; in addrconf_gre_config etc. Looks like we don't correctly configure ARPHRD_IP6GRE gre tunnels... > --- > net/ipv6/addrconf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index 0b239fc..655a0df 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -2824,6 +2824,7 @@ static void addrconf_gre_config(struct net_device *dev) > } > > addrconf_addr_gen(idev, true); > + addrconf_add_mroute(dev); > } > #endif > 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
(New patch following in separate mail - I haven't touched addrconf_sit_config since I have no clue about ISATAP & the likes.) On Wed, Jul 30, 2014 at 05:14:42PM +0200, Hannes Frederic Sowa wrote: > On Mi, 2014-07-30 at 02:55 +0200, David Lamparter wrote: > > GRE devices, for some reason, were coming up with an autoconfigured > > address, but no ff00::/8 route in the local table. This breaks any kind > > of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only > > works at all because there is little need for ND on PtP devices. > > > > Adding any other IPv6 address on the device would rectify this issue > > through inet6_addr_add()/addrconf_add_dev() - and would leave the route > > around even if the address was later removed. (This is probably why > > this issue was not discovered earlier. AFAICS it has been there from > > the beginning, e.g. aee80b5 "generate link local address for GRE > > tunnel") > > Yep, this is poor, but changing this will break user space... How exactly will this break user space? > > (Note: multicast is supported on GRE devices of all kinds, including PtP > > GRE, P-t-Mcast GRE and NBMA-GRE.) > > > > Fixes: aee80b54b235 (ipv6: generate link local address for GRE tunnel) > > Signed-off-by: David Lamparter <equinox@diac24.net> > > Cc: Stephen Hemminger <stephen@networkplumber.org> > > We should install routes before joining LL, I would go with > > idev = addrconf_add_dev(dev); > if (IS_ERR(idev)) > return; > > in addrconf_gre_config etc. Well, yeah, to be honest I have no clue why these functions are separated as they are. Once fixed up, the only difference between addrconf_gre_config and addrconf_dev_config is the true/false value on prefix_route for addrconf_addr_gen... and what the rationale for that is, I can only consult an oracle for. How about pushing the switch(dev->type) down from addrconf_notify into addrconf_dev_config()? > Looks like we don't correctly configure ARPHRD_IP6GRE gre tunnels... IP6GRE doesn't currently auto-generate a link-local address at all, is that what you meant? (Yeah we could/should add that too...) -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
Hi, On Mi, 2014-07-30 at 17:35 +0200, David Lamparter wrote: > (New patch following in separate mail - I haven't touched > addrconf_sit_config since I have no clue about ISATAP & the likes.) isatap shouldn't use multicast and afaik output path does block multicast tx. sit in non-ptp mainly depends on 2002:: or 6rd prefix and drops others packets. sit-ptp can use multicast. > > On Wed, Jul 30, 2014 at 05:14:42PM +0200, Hannes Frederic Sowa wrote: > > On Mi, 2014-07-30 at 02:55 +0200, David Lamparter wrote: > > > GRE devices, for some reason, were coming up with an autoconfigured > > > address, but no ff00::/8 route in the local table. This breaks any kind > > > of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only > > > works at all because there is little need for ND on PtP devices. > > > > > > Adding any other IPv6 address on the device would rectify this issue > > > through inet6_addr_add()/addrconf_add_dev() - and would leave the route > > > around even if the address was later removed. (This is probably why > > > this issue was not discovered earlier. AFAICS it has been there from > > > the beginning, e.g. aee80b5 "generate link local address for GRE > > > tunnel") > > > > Yep, this is poor, but changing this will break user space... > > How exactly will this break user space? Because the multicast routes will always be restored after e.g. a route flush or manual route deletion. Scripts might depend on this. > > > (Note: multicast is supported on GRE devices of all kinds, including PtP > > > GRE, P-t-Mcast GRE and NBMA-GRE.) > > > > > > Fixes: aee80b54b235 (ipv6: generate link local address for GRE tunnel) > > > Signed-off-by: David Lamparter <equinox@diac24.net> > > > Cc: Stephen Hemminger <stephen@networkplumber.org> > > > > We should install routes before joining LL, I would go with > > > > idev = addrconf_add_dev(dev); > > if (IS_ERR(idev)) > > return; > > > > in addrconf_gre_config etc. > > Well, yeah, to be honest I have no clue why these functions are > separated as they are. Once fixed up, the only difference between > addrconf_gre_config and addrconf_dev_config is the true/false value on > prefix_route for addrconf_addr_gen... and what the rationale for that > is, I can only consult an oracle for. Basically this is a slalom run to keep exisisting behaviour and clean up the code a little bit. > How about pushing the switch(dev->type) down from addrconf_notify into > addrconf_dev_config()? > > > Looks like we don't correctly configure ARPHRD_IP6GRE gre tunnels... > > IP6GRE doesn't currently auto-generate a link-local address at all, is > that what you meant? (Yeah we could/should add that too...) Exactly. 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:09:27PM +0200, Hannes Frederic Sowa wrote: [cut] > > On Wed, Jul 30, 2014 at 05:14:42PM +0200, Hannes Frederic Sowa wrote: > > > On Mi, 2014-07-30 at 02:55 +0200, David Lamparter wrote: > > > > GRE devices, for some reason, were coming up with an autoconfigured > > > > address, but no ff00::/8 route in the local table. This breaks any kind > > > > of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only > > > > works at all because there is little need for ND on PtP devices. > > > > > > > > Adding any other IPv6 address on the device would rectify this issue > > > > through inet6_addr_add()/addrconf_add_dev() - and would leave the route > > > > around even if the address was later removed. (This is probably why > > > > this issue was not discovered earlier. AFAICS it has been there from > > > > the beginning, e.g. aee80b5 "generate link local address for GRE > > > > tunnel") > > > > > > Yep, this is poor, but changing this will break user space... > > > > How exactly will this break user space? > > Because the multicast routes will always be restored after e.g. a route > flush or manual route deletion. Scripts might depend on this. Sorry, I still don't get it. Without this patch you end up in an inconsistent state, where a LL addr exists, but multicast doesn't work (since ff00::/8 is missing from RT6_TABLE_LOCAL). Userspace is not supposed to touch RT6_TABLE_LOCAL in general, and, the kernel will actually refuse installing the ff00::/8 route into the local table from userspace (because there will be other ff00::/8 routes from other interfaces, so you get "File exists"). You can delete the route (and thus break mcast), but not add it. The only way to add it is to add an address. Not changing this behaviour keeps breaking userspace; ospf6d among other things assumes an interface has working IPv6 when a link-local address is present. -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:31 +0200, David Lamparter wrote: > On Wed, Jul 30, 2014 at 06:09:27PM +0200, Hannes Frederic Sowa wrote: > [cut] > > > On Wed, Jul 30, 2014 at 05:14:42PM +0200, Hannes Frederic Sowa wrote: > > > > On Mi, 2014-07-30 at 02:55 +0200, David Lamparter wrote: > > > > > GRE devices, for some reason, were coming up with an autoconfigured > > > > > address, but no ff00::/8 route in the local table. This breaks any kind > > > > > of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only > > > > > works at all because there is little need for ND on PtP devices. > > > > > > > > > > Adding any other IPv6 address on the device would rectify this issue > > > > > through inet6_addr_add()/addrconf_add_dev() - and would leave the route > > > > > around even if the address was later removed. (This is probably why > > > > > this issue was not discovered earlier. AFAICS it has been there from > > > > > the beginning, e.g. aee80b5 "generate link local address for GRE > > > > > tunnel") > > > > > > > > Yep, this is poor, but changing this will break user space... > > > > > > How exactly will this break user space? > > > > Because the multicast routes will always be restored after e.g. a route > > flush or manual route deletion. Scripts might depend on this. > > Sorry, I still don't get it. Without this patch you end up in an > inconsistent state, where a LL addr exists, but multicast doesn't work > (since ff00::/8 is missing from RT6_TABLE_LOCAL). Sure, people can remove addresses and routes at will. > Userspace is not supposed to touch RT6_TABLE_LOCAL in general, and, the > kernel will actually refuse installing the ff00::/8 route into the local > table from userspace (because there will be other ff00::/8 routes from > other interfaces, so you get "File exists"). You can delete the route > (and thus break mcast), but not add it. The only way to add it is to > add an address. People really do flush the routing table. I'll have a look why the addition of the multicast route throws an error. > Not changing this behaviour keeps breaking userspace; ospf6d among > other things assumes an interface has working IPv6 when a link-local > address is present. Yeah, but in the end, people also can drop specific packets and we cannot do anything. Bye, 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:52:21PM +0200, Hannes Frederic Sowa wrote: > On Mi, 2014-07-30 at 18:31 +0200, David Lamparter wrote: > > On Wed, Jul 30, 2014 at 06:09:27PM +0200, Hannes Frederic Sowa wrote: > > [cut] > > > > On Wed, Jul 30, 2014 at 05:14:42PM +0200, Hannes Frederic Sowa wrote: > > > > > On Mi, 2014-07-30 at 02:55 +0200, David Lamparter wrote: > > > > > > GRE devices, for some reason, were coming up with an autoconfigured > > > > > > address, but no ff00::/8 route in the local table. This breaks any kind > > > > > > of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only > > > > > > works at all because there is little need for ND on PtP devices. > > > > > > > > > > > > Adding any other IPv6 address on the device would rectify this issue > > > > > > through inet6_addr_add()/addrconf_add_dev() - and would leave the route > > > > > > around even if the address was later removed. (This is probably why > > > > > > this issue was not discovered earlier. AFAICS it has been there from > > > > > > the beginning, e.g. aee80b5 "generate link local address for GRE > > > > > > tunnel") > > > > > > > > > > Yep, this is poor, but changing this will break user space... > > > > > > > > How exactly will this break user space? > > > > > > Because the multicast routes will always be restored after e.g. a route > > > flush or manual route deletion. Scripts might depend on this. > > > > Sorry, I still don't get it. Without this patch you end up in an > > inconsistent state, where a LL addr exists, but multicast doesn't work > > (since ff00::/8 is missing from RT6_TABLE_LOCAL). > > Sure, people can remove addresses and routes at will. That's not the point. user manually adds address on any if: ff00::/8 added. kernel link-up autoconf addr on !gre: ff00::/8 added. kernel link-up autoconf addr on gre: no ff00::/8. This is about automatic kernel behaviour in device up/change notifications. The user can always shoot himself in the foot. This is the kernel shooting the user in the foot in one particular and quite specific instance and without a request to do so. > > Userspace is not supposed to touch RT6_TABLE_LOCAL in general, and, the > > kernel will actually refuse installing the ff00::/8 route into the local > > table from userspace (because there will be other ff00::/8 routes from > > other interfaces, so you get "File exists"). You can delete the route > > (and thus break mcast), but not add it. The only way to add it is to > > add an address. > > People really do flush the routing table. This is RT6_TABLE_LOCAL. Most people aren't even aware it exists. And even though I can't find a reference for it, my memory tells me that "table local" is supposed to be under the kernel's authority. > I'll have a look why the addition of the multicast route throws an > error. While that may be interesting, it won't fix the issue at hand. (It's probably because the output device is not part of uniqueness considerations for ff00::/8. Might need to be treated as "scope link".) > > Not changing this behaviour keeps breaking userspace; ospf6d among > > other things assumes an interface has working IPv6 when a link-local > > address is present. > > Yeah, but in the end, people also can drop specific packets and we > cannot do anything. See above, this isn't the user screwing himself, this is the kernel doing the wrong thing in one specific corner case. (Which shouldn't even be a corner case.) -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 Wed, Jul 30, 2014, at 19:35, David Lamparter wrote: > On Wed, Jul 30, 2014 at 06:52:21PM +0200, Hannes Frederic Sowa wrote: > > On Mi, 2014-07-30 at 18:31 +0200, David Lamparter wrote: > > > On Wed, Jul 30, 2014 at 06:09:27PM +0200, Hannes Frederic Sowa wrote: > > > [cut] > > > > > On Wed, Jul 30, 2014 at 05:14:42PM +0200, Hannes Frederic Sowa wrote: > > > > > > On Mi, 2014-07-30 at 02:55 +0200, David Lamparter wrote: > > > > > > > GRE devices, for some reason, were coming up with an autoconfigured > > > > > > > address, but no ff00::/8 route in the local table. This breaks any kind > > > > > > > of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only > > > > > > > works at all because there is little need for ND on PtP devices. > > > > > > > > > > > > > > Adding any other IPv6 address on the device would rectify this issue > > > > > > > through inet6_addr_add()/addrconf_add_dev() - and would leave the route > > > > > > > around even if the address was later removed. (This is probably why > > > > > > > this issue was not discovered earlier. AFAICS it has been there from > > > > > > > the beginning, e.g. aee80b5 "generate link local address for GRE > > > > > > > tunnel") > > > > > > > > > > > > Yep, this is poor, but changing this will break user space... > > > > > > > > > > How exactly will this break user space? > > > > > > > > Because the multicast routes will always be restored after e.g. a route > > > > flush or manual route deletion. Scripts might depend on this. > > > > > > Sorry, I still don't get it. Without this patch you end up in an > > > inconsistent state, where a LL addr exists, but multicast doesn't work > > > (since ff00::/8 is missing from RT6_TABLE_LOCAL). > > > > Sure, people can remove addresses and routes at will. > > That's not the point. > > user manually adds address on any if: ff00::/8 added. > kernel link-up autoconf addr on !gre: ff00::/8 added. > kernel link-up autoconf addr on gre: no ff00::/8. Ah sorry, I am ok with your patch that we generate a ff00::/8 multicast route if we set a gre interface up. Getting rid of the logic, that we try to add the ff00::/8 on every addition of an address to an interface would be a change which concerns because of backward compatibility reasons. Especially this also creates multicast routes on sit-non-ptp/isatap routes, which actually don't need one. > This is about automatic kernel behaviour in device up/change > notifications. The user can always shoot himself in the foot. This is > the kernel shooting the user in the foot in one particular and quite > specific instance and without a request to do so. Agreed. > > > Userspace is not supposed to touch RT6_TABLE_LOCAL in general, and, the > > > kernel will actually refuse installing the ff00::/8 route into the local > > > table from userspace (because there will be other ff00::/8 routes from > > > other interfaces, so you get "File exists"). You can delete the route > > > (and thus break mcast), but not add it. The only way to add it is to > > > add an address. > > > > People really do flush the routing table. > > This is RT6_TABLE_LOCAL. Most people aren't even aware it exists. And > even though I can't find a reference for it, my memory tells me that > "table local" is supposed to be under the kernel's authority. Yep, but still provide ip -6 route flush table all, which also clears the local table. And I guess people depend on ff00:: multicast routes being inserted as soon as they add addresses. Bye, 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, 2014-07-30 at 20:03 +0200, Hannes Frederic Sowa wrote: > On Wed, Jul 30, 2014, at 19:35, David Lamparter wrote: > > On Wed, Jul 30, 2014 at 06:52:21PM +0200, Hannes Frederic Sowa wrote: > > > On Mi, 2014-07-30 at 18:31 +0200, David Lamparter wrote: > > > > On Wed, Jul 30, 2014 at 06:09:27PM +0200, Hannes Frederic Sowa wrote: > > > > [cut] > > > > > > On Wed, Jul 30, 2014 at 05:14:42PM +0200, Hannes Frederic Sowa wrote: > > > > > > > On Mi, 2014-07-30 at 02:55 +0200, David Lamparter wrote: > > > > > > > > GRE devices, for some reason, were coming up with an autoconfigured > > > > > > > > address, but no ff00::/8 route in the local table. This breaks any kind > > > > > > > > of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only > > > > > > > > works at all because there is little need for ND on PtP devices. > > > > > > > > > > > > > > > > Adding any other IPv6 address on the device would rectify this issue > > > > > > > > through inet6_addr_add()/addrconf_add_dev() - and would leave the route > > > > > > > > around even if the address was later removed. (This is probably why > > > > > > > > this issue was not discovered earlier. AFAICS it has been there from > > > > > > > > the beginning, e.g. aee80b5 "generate link local address for GRE > > > > > > > > tunnel") > > > > > > > > > > > > > > Yep, this is poor, but changing this will break user space... > > > > > > > > > > > > How exactly will this break user space? > > > > > > > > > > Because the multicast routes will always be restored after e.g. a route > > > > > flush or manual route deletion. Scripts might depend on this. > > > > > > > > Sorry, I still don't get it. Without this patch you end up in an > > > > inconsistent state, where a LL addr exists, but multicast doesn't work > > > > (since ff00::/8 is missing from RT6_TABLE_LOCAL). > > > > > > Sure, people can remove addresses and routes at will. > > > > That's not the point. > > > > user manually adds address on any if: ff00::/8 added. > > kernel link-up autoconf addr on !gre: ff00::/8 added. > > kernel link-up autoconf addr on gre: no ff00::/8. > > Ah sorry, I am ok with your patch that we generate a ff00::/8 multicast > route if we set a gre interface up. Getting rid of the logic, that we > try to add the ff00::/8 on every addition of an address to an interface > would be a change which concerns because of backward compatibility > reasons. With IPv6LL address generation modes (IFLA_INET6_ADDR_GEN_MODE) the kernel might not be handling the LL address, so I'd like to keep addrconf_add_dev() in the inet6_addr_add() codepath. Having the multicast route is useful/correct no matter who adds the LL address to the interface. I think you alluded to this earlier, but I just wanted to bring it up. Dan > Especially this also creates multicast routes on sit-non-ptp/isatap > routes, which actually don't need one. > > > This is about automatic kernel behaviour in device up/change > > notifications. The user can always shoot himself in the foot. This is > > the kernel shooting the user in the foot in one particular and quite > > specific instance and without a request to do so. > > Agreed. > > > > > Userspace is not supposed to touch RT6_TABLE_LOCAL in general, and, the > > > > kernel will actually refuse installing the ff00::/8 route into the local > > > > table from userspace (because there will be other ff00::/8 routes from > > > > other interfaces, so you get "File exists"). You can delete the route > > > > (and thus break mcast), but not add it. The only way to add it is to > > > > add an address. > > > > > > People really do flush the routing table. > > > > This is RT6_TABLE_LOCAL. Most people aren't even aware it exists. And > > even though I can't find a reference for it, my memory tells me that > > "table local" is supposed to be under the kernel's authority. > > Yep, but still provide ip -6 route flush table all, which also clears > the local table. And I guess people depend on ff00:: multicast routes > being inserted as soon as they add addresses. > > Bye, > 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 -- 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
From: David Lamparter <equinox@diac24.net> Date: Wed, 30 Jul 2014 19:35:44 +0200 > This is RT6_TABLE_LOCAL. Most people aren't even aware it exists. And > even though I can't find a reference for it, my memory tells me that > "table local" is supposed to be under the kernel's authority. The origin of local routing tables (both in ipv4 and ipv6) has more to do with internal implementation issues rather than issues pertaining to how the user should or should not manipulate routes. It's basically an optimization for local packet delivery, since the local routing tables get consulted first. Secondly, separation of local and non-local routes into different tables theoretically makes those tables less dense and thus resulting in faster lookups. -- 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..655a0df 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -2824,6 +2824,7 @@ static void addrconf_gre_config(struct net_device *dev) } addrconf_addr_gen(idev, true); + addrconf_add_mroute(dev); } #endif
GRE devices, for some reason, were coming up with an autoconfigured address, but no ff00::/8 route in the local table. This breaks any kind of multicast, in particular OSPFv3, mDNS, - and ND. In fact, IPv6 only works at all because there is little need for ND on PtP devices. Adding any other IPv6 address on the device would rectify this issue through inet6_addr_add()/addrconf_add_dev() - and would leave the route around even if the address was later removed. (This is probably why this issue was not discovered earlier. AFAICS it has been there from the beginning, e.g. aee80b5 "generate link local address for GRE tunnel") (Note: multicast is supported on GRE devices of all kinds, including PtP GRE, P-t-Mcast GRE and NBMA-GRE.) Fixes: aee80b54b235 (ipv6: generate link local address for GRE tunnel) Signed-off-by: David Lamparter <equinox@diac24.net> Cc: Stephen Hemminger <stephen@networkplumber.org> --- net/ipv6/addrconf.c | 1 + 1 file changed, 1 insertion(+)