Message ID | 307b4d32099f606d0fbe0ce9fecd3a039b796361.1460123261.git.daniel@iogearbox.net |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On 08.04.2016 15:55, Daniel Borkmann wrote: > The original tokenized iid support implemented via f53adae4eae5 ("net: ipv6: > add tokenized interface identifier support") didn't allow for clearing a > device token as it was intended that this addressing mode was the only one > active for globally scoped IPv6 addresses. Later we relaxed that restriction > via 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses"), > and we should also allow for clearing tokens as there's no good reason why > it shouldn't be allowed. > > Fixes: 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses") > Reported-by: Robin H. Johnson <robbat2@gentoo.org> > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> Thanks, Hannes
Daniel Borkmann <daniel@iogearbox.net> writes: > > if (!token) > return -EINVAL; > - if (ipv6_addr_any(token)) > - return -EINVAL; > if (dev->flags & (IFF_LOOPBACK | IFF_NOARP)) > return -EINVAL; Not directly related to the patch in question. It just made me aware of this restriction... I realize that I'm a few years late here, but what's with the IFF_NOARP? Is that just because we can't do DAD for the token based addresses? How is that different from manually configuring the whole address? Bjørn
On Fri, Apr 8, 2016, at 16:18, Bjørn Mork wrote: > Daniel Borkmann <daniel@iogearbox.net> writes: > > > > > if (!token) > > return -EINVAL; > > - if (ipv6_addr_any(token)) > > - return -EINVAL; > > if (dev->flags & (IFF_LOOPBACK | IFF_NOARP)) > > return -EINVAL; > > Not directly related to the patch in question. It just made me aware of > this restriction... > > I realize that I'm a few years late here, but what's with the IFF_NOARP? > Is that just because we can't do DAD for the token based addresses? How > is that different from manually configuring the whole address? IFF_NOARP is kind of the equivalent to no neighbor discovery. If you set a token and never get in a router advertisement you never create a tokenized ip address, thus the feature is useless. Bye, Hannes
Hannes Frederic Sowa <hannes@stressinduktion.org> writes: > On Fri, Apr 8, 2016, at 16:18, Bjørn Mork wrote: >> Daniel Borkmann <daniel@iogearbox.net> writes: >> >> > >> > if (!token) >> > return -EINVAL; >> > - if (ipv6_addr_any(token)) >> > - return -EINVAL; >> > if (dev->flags & (IFF_LOOPBACK | IFF_NOARP)) >> > return -EINVAL; >> >> Not directly related to the patch in question. It just made me aware of >> this restriction... >> >> I realize that I'm a few years late here, but what's with the IFF_NOARP? >> Is that just because we can't do DAD for the token based addresses? How >> is that different from manually configuring the whole address? > > IFF_NOARP is kind of the equivalent to no neighbor discovery. If you set > a token and never get in a router advertisement you never create a > tokenized ip address, thus the feature is useless. You can get router advertisements with IFF_NOARP. You cannot lookup L2 addresses, but the L3 prefix info is still as useful as with any other interface. Doing tshark -nVi any -f ip6 while bringing up the POINTOPOINT NOARP interface shown at the end shows the expected RS and RA: Frame 1: 64 bytes on wire (512 bits), 64 bytes captured (512 bits) on interface 0 Interface id: 0 (any) Encapsulation type: Linux cooked-mode capture (25) Arrival Time: Apr 8, 2016 17:18:36.094554456 CEST [Time shift for this packet: 0.000000000 seconds] Epoch Time: 1460128716.094554456 seconds [Time delta from previous captured frame: 0.000000000 seconds] [Time delta from previous displayed frame: 0.000000000 seconds] [Time since reference or first frame: 0.000000000 seconds] Frame Number: 1 Frame Length: 64 bytes (512 bits) Capture Length: 64 bytes (512 bits) [Frame is marked: False] [Frame is ignored: False] [Protocols in frame: sll:ethertype:ipv6:icmpv6] Linux cooked capture Packet type: Sent by us (4) Link-layer address type: 65534 Link-layer address length: 0 Protocol: IPv6 (0x86dd) Internet Protocol Version 6, Src: fe80::8019:47ef:17a1:8c38, Dst: ff02::2 0110 .... = Version: 6 .... 0000 0000 .... .... .... .... .... = Traffic class: 0x00 (DSCP: CS0, ECN: Not-ECT) .... 0000 00.. .... .... .... .... .... = Differentiated Services Codepoint: Default (0) .... .... ..00 .... .... .... .... .... = Explicit Congestion Notification: Not ECN-Capable Transport (0) .... .... .... 0000 0000 0000 0000 0000 = Flowlabel: 0x00000000 Payload length: 8 Next header: ICMPv6 (58) Hop limit: 255 Source: fe80::8019:47ef:17a1:8c38 Destination: ff02::2 [Source GeoIP: Unknown] [Destination GeoIP: Unknown] Internet Control Message Protocol v6 Type: Router Solicitation (133) Code: 0 Checksum: 0x1155 [correct] Reserved: 00000000 Frame 2: 120 bytes on wire (960 bits), 120 bytes captured (960 bits) on interface 0 Interface id: 0 (any) Encapsulation type: Linux cooked-mode capture (25) Arrival Time: Apr 8, 2016 17:18:36.101806992 CEST [Time shift for this packet: 0.000000000 seconds] Epoch Time: 1460128716.101806992 seconds [Time delta from previous captured frame: 0.007252536 seconds] [Time delta from previous displayed frame: 0.007252536 seconds] [Time since reference or first frame: 0.007252536 seconds] Frame Number: 2 Frame Length: 120 bytes (960 bits) Capture Length: 120 bytes (960 bits) [Frame is marked: False] [Frame is ignored: False] [Protocols in frame: sll:ethertype:ipv6:icmpv6] Linux cooked capture Packet type: Unicast to us (0) Link-layer address type: 65534 Link-layer address length: 0 Protocol: IPv6 (0x86dd) Internet Protocol Version 6, Src: fe80::a5a6:793:6bfe:ea1c, Dst: fe80::8019:47ef:17a1:8c38 0110 .... = Version: 6 .... 0000 0000 .... .... .... .... .... = Traffic class: 0x00 (DSCP: CS0, ECN: Not-ECT) .... 0000 00.. .... .... .... .... .... = Differentiated Services Codepoint: Default (0) .... .... ..00 .... .... .... .... .... = Explicit Congestion Notification: Not ECN-Capable Transport (0) .... .... .... 0000 0000 0000 0000 0000 = Flowlabel: 0x00000000 Payload length: 64 Next header: ICMPv6 (58) Hop limit: 255 Source: fe80::a5a6:793:6bfe:ea1c Destination: fe80::8019:47ef:17a1:8c38 [Source GeoIP: Unknown] [Destination GeoIP: Unknown] Internet Control Message Protocol v6 Type: Router Advertisement (134) Code: 0 Checksum: 0x96fa [correct] Cur hop limit: 255 Flags: 0x40 0... .... = Managed address configuration: Not set .1.. .... = Other configuration: Set ..0. .... = Home Agent: Not set ...0 0... = Prf (Default Router Preference): Medium (0) .... .0.. = Proxy: Not set .... ..0. = Reserved: 0 Router lifetime (s): 65535 Reachable time (ms): 0 Retrans timer (ms): 0 ICMPv6 Option (Source link-layer address : 02:50:f3:00:01:00) Type: Source link-layer address (1) Length: 1 (8 bytes) Link-layer address: 02:50:f3:00:01:00 ICMPv6 Option (MTU : 1500) Type: MTU (5) Length: 1 (8 bytes) Reserved MTU: 1500 ICMPv6 Option (Prefix information : 2a02:2121:81:e578::/64) Type: Prefix information (3) Length: 4 (32 bytes) Prefix Length: 64 Flag: 0xc0 1... .... = On-link flag(L): Set .1.. .... = Autonomous address-configuration flag(A): Set ..0. .... = Router address flag(R): Not set ...0 0000 = Reserved: 0 Valid Lifetime: 4294967295 (Infinity) Preferred Lifetime: 4294967295 (Infinity) Reserved Prefix: 2a02:2121:81:e578:: nemi:/tmp# ifconfig wwan0 wwan0 Link encap:UNSPEC HWaddr 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 inet addr:10.135.186.66 P-t-P:10.135.186.66 Mask:255.255.255.252 inet6 addr: 2a02:2121:81:e578:bce:7da1:24a5:af48/64 Scope:Global inet6 addr: fe80::8019:47ef:17a1:8c38/64 Scope:Link UP POINTOPOINT RUNNING NOARP MULTICAST MTU:1500 Metric:1 RX packets:3 errors:0 dropped:0 overruns:0 frame:0 TX packets:3 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:716 (716.0 B) TX bytes:704 (704.0 B) (the other end is an LTE modem here) Bjørn
On 08.04.2016 17:25, Bjørn Mork wrote: > Hannes Frederic Sowa <hannes@stressinduktion.org> writes: > >> On Fri, Apr 8, 2016, at 16:18, Bjørn Mork wrote: >>> Daniel Borkmann <daniel@iogearbox.net> writes: >>> >>>> >>>> if (!token) >>>> return -EINVAL; >>>> - if (ipv6_addr_any(token)) >>>> - return -EINVAL; >>>> if (dev->flags & (IFF_LOOPBACK | IFF_NOARP)) >>>> return -EINVAL; >>> >>> Not directly related to the patch in question. It just made me aware of >>> this restriction... >>> >>> I realize that I'm a few years late here, but what's with the IFF_NOARP? >>> Is that just because we can't do DAD for the token based addresses? How >>> is that different from manually configuring the whole address? >> >> IFF_NOARP is kind of the equivalent to no neighbor discovery. If you set >> a token and never get in a router advertisement you never create a >> tokenized ip address, thus the feature is useless. > > You can get router advertisements with IFF_NOARP. You cannot lookup L2 > addresses, but the L3 prefix info is still as useful as with any other > interface. Of course router advertisements can be send and received with IFF_NOARP and probably we act on them as usual, as you showed. Looking in the source we don't really specify what those flags mean/do for IPv6. So I think you can assume that it is in there because of history. I would absolutely not mind if you remove the limitation for IFF_ARP. Bye, Hannes
On 04/08/2016 05:36 PM, Hannes Frederic Sowa wrote: > On 08.04.2016 17:25, Bjørn Mork wrote: >> Hannes Frederic Sowa <hannes@stressinduktion.org> writes: >>> On Fri, Apr 8, 2016, at 16:18, Bjørn Mork wrote: >>>> Daniel Borkmann <daniel@iogearbox.net> writes: >>>> >>>>> if (!token) >>>>> return -EINVAL; >>>>> - if (ipv6_addr_any(token)) >>>>> - return -EINVAL; >>>>> if (dev->flags & (IFF_LOOPBACK | IFF_NOARP)) >>>>> return -EINVAL; >>>> >>>> Not directly related to the patch in question. It just made me aware of >>>> this restriction... >>>> >>>> I realize that I'm a few years late here, but what's with the IFF_NOARP? >>>> Is that just because we can't do DAD for the token based addresses? How >>>> is that different from manually configuring the whole address? >>> >>> IFF_NOARP is kind of the equivalent to no neighbor discovery. If you set >>> a token and never get in a router advertisement you never create a >>> tokenized ip address, thus the feature is useless. >> >> You can get router advertisements with IFF_NOARP. You cannot lookup L2 >> addresses, but the L3 prefix info is still as useful as with any other >> interface. > > Of course router advertisements can be send and received with IFF_NOARP and probably we act on them as usual, as you showed. Looking in the source we don't really specify what those flags mean/do for IPv6. So I think you can assume that it is in there because of history. > > I would absolutely not mind if you remove the limitation for IFF_ARP. Agreed me neither, the code should be able to handle it as far as I see. Thanks, Daniel
From: Daniel Borkmann <daniel@iogearbox.net> Date: Fri, 8 Apr 2016 15:55:00 +0200 > The original tokenized iid support implemented via f53adae4eae5 ("net: ipv6: > add tokenized interface identifier support") didn't allow for clearing a > device token as it was intended that this addressing mode was the only one > active for globally scoped IPv6 addresses. Later we relaxed that restriction > via 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses"), > and we should also allow for clearing tokens as there's no good reason why > it shouldn't be allowed. > > Fixes: 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses") > Reported-by: Robin H. Johnson <robbat2@gentoo.org> > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> Applied to net-next, thanks.
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 27aed1a..a6c9927 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -4995,15 +4995,13 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token) { struct inet6_ifaddr *ifp; struct net_device *dev = idev->dev; - bool update_rs = false; + bool clear_token, update_rs = false; struct in6_addr ll_addr; ASSERT_RTNL(); if (!token) return -EINVAL; - if (ipv6_addr_any(token)) - return -EINVAL; if (dev->flags & (IFF_LOOPBACK | IFF_NOARP)) return -EINVAL; if (!ipv6_accept_ra(idev)) @@ -5018,10 +5016,13 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token) write_unlock_bh(&idev->lock); + clear_token = ipv6_addr_any(token); + if (clear_token) + goto update_lft; + if (!idev->dead && (idev->if_flags & IF_READY) && !ipv6_get_lladdr(dev, &ll_addr, IFA_F_TENTATIVE | IFA_F_OPTIMISTIC)) { - /* If we're not ready, then normal ifup will take care * of this. Otherwise, we need to request our rs here. */ @@ -5029,6 +5030,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token) update_rs = true; } +update_lft: write_lock_bh(&idev->lock); if (update_rs) {
The original tokenized iid support implemented via f53adae4eae5 ("net: ipv6: add tokenized interface identifier support") didn't allow for clearing a device token as it was intended that this addressing mode was the only one active for globally scoped IPv6 addresses. Later we relaxed that restriction via 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses"), and we should also allow for clearing tokens as there's no good reason why it shouldn't be allowed. Fixes: 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses") Reported-by: Robin H. Johnson <robbat2@gentoo.org> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> --- Dave, I think net-next is fine, but don't have any objections if you rather want to put it into net. Thanks! net/ipv6/addrconf.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)