net: ip: Introduce mesh_local flag #12731

rlubos · 2019-01-25T11:58:00Z

This PR introduces a concept of mesh-local address. Such addresses (used for instance by Thread) are fine to use for on-mesh communication, but should not be used when sending packets off-mesh.

Source address selection algorithm needs to distinguish between addresses that have mesh-local (or Realm-local in IPv6 nomenclature) scope and other addresses. As they all have form of ULA, it cannot be distinguished on the address format itself, which lead to erroneous source address assignment. Therfore, an additional flag was added to the net_if_addr structure that can store this information.

Mesh-local addresses will only be used as a source address for destinations placed within the same subnet (mesh).

A sample interface configuration is shown below. Starred addresses were marked as meshlocal, therefore they'll only be selected for on-mesh communication.

Interface 0x200149a0 (<unknown type>) [0]
=========================================
Link addr : 16:0F:22:72:A6:90:22:3D
MTU       : 1280
IPv6 unicast addresses (max 8):
        *fdde:ad00:beef:0:1bc2:6311:f1e7:31a4 autoconf preferred infinite meshlocal
        fe80::28a6:a531:967d:9660 autoconf preferred infinite
        *fdde:ad00:beef::2 manual preferred infinite meshlocal
        fd5f:a074:3e6e:5:1aec:4f9f:9138:4d39 autoconf preferred infinite
        fdbe:387:ce8d:15:1797:25ec:a7e5:92a3 autoconf preferred infinite
        2a02:f78:1:315:1696:202:12d0:762c autoconf preferred infinite
        fd11:22::2554:c337:1897:ab6f autoconf preferred infinite
IPv6 multicast addresses (max 8):
        ff33:40:fdde:ad00:beef::1
        ff32:40:fdde:ad00:beef::1
        ff02::1
        ff03::1
        ff03::fc
        ff02::2
        ff03::2
IPv6 prefixes (max 2):
        <none>
IPv6 hop limit           : 64
IPv6 base reachable time : 30000
IPv6 reachable time      : 38475
IPv6 retransmit timer    : 0

Resolves #12203

rlubos · 2019-01-25T11:59:39Z

@mike-scott @aurel32 Note that though the concept remained similar in this solution, the implementation is slightly different to what you've tested yesterday. It'd be good if you could give it a try with your use-cases.

tbursztyka · 2019-01-25T12:09:02Z

@rlubos Do you have a reference in an RFC? Could be nice to add it in the commit message then.

codecov-io · 2019-01-25T12:35:07Z

Codecov Report

Merging #12731 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #12731      +/-   ##
==========================================
- Coverage   54.06%   54.06%   -0.01%     
==========================================
  Files         239      239              
  Lines       26922    26924       +2     
  Branches     6505     6506       +1     
==========================================
  Hits        14556    14556              
- Misses       9701     9702       +1     
- Partials     2665     2666       +1

Impacted Files	Coverage Δ
include/net/net_if.h	`57.89% <ø> (ø)`	⬆️
subsys/net/ip/net_if.c	`61.37% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update daa22c4...d79034a. Read the comment docs.

rlubos · 2019-01-25T12:38:31Z

@tbursztyka Not exactly a RFC, the concept was more or less introduced by Thread, which differentiates between mesh-local addresses and global addresses (I'm not aware of RFC covering that). And the former ones should only be used within a mesh.

In Thread specification. ch. 5.2.2.5, we have:

Thread uses mesh-local addresses to reach Thread interfaces within the same Thread
Network Partition.

And later in ch. 9.2.4:

Communication using MLAs [Mesh Local Address] is not routable to the exterior of a Thread 
Network via a Border Router.

jukkar

Overall looks good. Any idea why there is * char in front of the address as the shell is not printing those, or did you add them manually in this example?

 *fdde:ad00:beef:0:1bc2:6311:f1e7:31a4 autoconf preferred infinite meshlocal
 fe80::28a6:a531:967d:9660 autoconf preferred infinite
 *fdde:ad00:beef::2 manual preferred infinite meshlocal

aurel32 · 2019-01-25T18:49:37Z

It seems the mesh-local addresses concept from Thread is a bit similar to the site-local addresses that was deprecated by RFC3879.

aurel32

It looks good to me. I have tested it here on a OT network where I haven't attributed GUA addresses, and it works fine. I am able to use COAP with the nodes using the ML-EID addresses.

rlubos · 2019-01-28T08:21:11Z

@jukkar It was manual addition, sorry for confusion.

rlubos · 2019-01-28T08:24:21Z

@aurel32 I'd say it's a Realm-Local scope, which was introduced in rfc7346 for multicast purposes. It has specialized definition for 15.4 networks:

5.  Definition of Realm-Local Scope for IEEE 802.15.4

   When used in an IP-over-IEEE802.15.4 network, scop 3 is defined to
   include all interfaces sharing a Personal Area Network Identifier
   (PAN ID).

aurel32 · 2019-01-28T08:32:04Z

@rlubos The IP range used for mesh-local addresses by Thread corresponds to the unique local address (ULA) range, as defined by RFC4193.

rlubos · 2019-01-28T08:39:50Z

@aurel32 Yes, you are right. The point is, the same IP range (ULA) is used in prefixes distributed by Border Routers (see unstarred unicast addresses in my example), and these prefixes (and therefore addresses created with them) are allowed to be used outside of the mesh (and to be honest that's why they are assigned). Hence we need more information to differentiate these.

pfalcon

First commit message has "addresss" (3 "s").

But most importantly, let's say again (in commit message I guess), what happens if we add NET_ADDR_MESH_LOCAL instead?

pfalcon · 2019-01-28T13:45:08Z

subsys/net/l2/openthread/openthread_utils.c

+					NET_ADDR_AUTOCONF, 0);
+
+		if (if_addr == NULL) {
+			NET_WARN("Not enough buffers to register OpenThread's"


Why "buffers"?

And this is not NET_WARN I guess.

Why "buffers"?

Because of lack of better word for "IP address placeholder"

And this is not NET_WARN I guess.

And what exactly is wrong with NET_WARN?

Because of lack of better word for "IP address placeholder"

The right word would be "entry", but the whole message is problematic. That message is printed when net_if_ipv6_addr_add() returns NULL. Are you sure that the only condition when it does that is when there're no free address entries? Are you sure it'll stay like that in the future? "Cannot add OpenThread <if possible, type> address" is the message which will never lie. (I'm all for more detailed error messages, but as I said, there're limits to it all.)

And what exactly is wrong with NET_WARN?

Because it's error. Unless that address is not used for anything useful, and we add it just for fun ;-).

Ok, I'll update this piece

pfalcon · 2019-01-28T13:50:41Z

subsys/net/l2/openthread/openthread_utils.c

+		if (if_addr == NULL) {
+			NET_WARN("Not enough buffers to register OpenThread's"
+				 " IP address. Increase "
+				 "CONFIG_NET_IF_UNICAST_IPV6_ADDR_COUNT");


And I absolutely love self-diagnosing software. But as other things I love, I also like to know limits on them. For example, I do it like this (explicitly hint user that by enabling some config var they can get more info) in the shell. But that's because shell is a debugging feature. Speaking of error messages, this one would be first of such nature. (So, do we need such a long error message?)

Sorry I don't follow this comment (beside of the last sentence).

rlubos · 2019-01-28T14:44:27Z

But most importantly, let's say again (in commit message I guess), what happens if we add NET_ADDR_MESH_LOCAL instead?

I'm not convinced about writing "what if" statements in commit message.

But to answer you question, I did not reuse enum net_addr_type addr_type field because it already stores different information (see starred addresses in the sample I posted, one is autoconf another one is manual). If we overwrite this field, we lose this info.

pfalcon · 2019-01-28T14:47:32Z

If we overwrite this field, we lose this info.

Ok, please add a short paragraph describing this situation to the commit message, so it can be found by someone who'll investigate it in the future.

This commit introduces a concept of mesh-local IPv6 addresses. Such addresses should only be used for mesh-local communication, therefore should not be used to communicate with different subnets (i. e. destinations outside the mesh). As `addr_type` field already holds different kind of information (whether address was created automatically/manually) it was not used in this case. Instead a mesh_local flag was added, so that we do not lose information on how address was created. Address with such flag set will only be selected as a source address automatically if the destination address is within the same subnet it belongs to. Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>

Mark automatically and statically configured mesh-local addresses with mesh_local flag, so that they are not used as a source for off-mesh destinations. Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>

rlubos · 2019-01-28T15:24:40Z

Updated the commit message and error message.

mike-scott · 2019-01-28T17:23:40Z

Sorry for the delay, the depreciation changes are making it semi- difficult to swap back and forth between dev trees and master at the moment.

I tested this today using #12781
And found the fix to be quite nice:

IPv6 unicast addresses (max 6):
        fdde:ad00:beef:0:531:74ba:b057:ef7d autoconf preferred infinite meshlocal
        fe80::e7:f30e:3f3e:901e autoconf preferred infinite
        fd11:22::262:b438:3091:e604 autoconf preferred infinite

Works great, thanks @rlubos

pfalcon

Thanks!

mike-scott · 2019-01-28T18:56:32Z

I'm not sure if the test_select sanity failure is due to this commit. Might want to re-run.

rlubos added area: Networking area: OpenThread labels Jan 25, 2019

rlubos requested review from aurel32 and mike-scott January 25, 2019 11:58

rlubos requested review from jukkar, pfalcon and tbursztyka as code owners January 25, 2019 11:58

jukkar reviewed Jan 25, 2019

View reviewed changes

aurel32 approved these changes Jan 25, 2019

View reviewed changes

mike-scott mentioned this pull request Jan 28, 2019

openthread: fix source IP address selection #12343

Closed

jukkar approved these changes Jan 28, 2019

View reviewed changes

pfalcon requested changes Jan 28, 2019

View reviewed changes

pfalcon reviewed Jan 28, 2019

View reviewed changes

rlubos added 2 commits January 28, 2019 16:10

net: openthread: Set mesh_local flag in mesh-local OT addresses

d79034a

Mark automatically and statically configured mesh-local addresses with mesh_local flag, so that they are not used as a source for off-mesh destinations. Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>

rlubos force-pushed the introduce-mesh-local-flag branch from 8054cef to d79034a Compare January 28, 2019 15:23

mike-scott approved these changes Jan 28, 2019

View reviewed changes

pfalcon approved these changes Jan 28, 2019

View reviewed changes

nashif merged commit 43a431e into zephyrproject-rtos:master Jan 29, 2019

rlubos deleted the introduce-mesh-local-flag branch February 18, 2019 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net: ip: Introduce mesh_local flag #12731

net: ip: Introduce mesh_local flag #12731

rlubos commented Jan 25, 2019

rlubos commented Jan 25, 2019

tbursztyka commented Jan 25, 2019

codecov-io commented Jan 25, 2019 •

edited

rlubos commented Jan 25, 2019

jukkar left a comment

aurel32 commented Jan 25, 2019

aurel32 left a comment

rlubos commented Jan 28, 2019

rlubos commented Jan 28, 2019

aurel32 commented Jan 28, 2019

rlubos commented Jan 28, 2019

pfalcon left a comment

pfalcon Jan 28, 2019

pfalcon Jan 28, 2019

rlubos Jan 28, 2019

pfalcon Jan 28, 2019 •

edited

rlubos Jan 28, 2019

pfalcon Jan 28, 2019

rlubos Jan 28, 2019

rlubos commented Jan 28, 2019

pfalcon commented Jan 28, 2019

rlubos commented Jan 28, 2019

mike-scott commented Jan 28, 2019

pfalcon left a comment

mike-scott commented Jan 28, 2019

net: ip: Introduce mesh_local flag #12731

net: ip: Introduce mesh_local flag #12731

Conversation

rlubos commented Jan 25, 2019

rlubos commented Jan 25, 2019

tbursztyka commented Jan 25, 2019

codecov-io commented Jan 25, 2019 • edited

Codecov Report

rlubos commented Jan 25, 2019

jukkar left a comment

Choose a reason for hiding this comment

aurel32 commented Jan 25, 2019

aurel32 left a comment

Choose a reason for hiding this comment

rlubos commented Jan 28, 2019

rlubos commented Jan 28, 2019

aurel32 commented Jan 28, 2019

rlubos commented Jan 28, 2019

pfalcon left a comment

Choose a reason for hiding this comment

pfalcon Jan 28, 2019

Choose a reason for hiding this comment

pfalcon Jan 28, 2019

Choose a reason for hiding this comment

rlubos Jan 28, 2019

Choose a reason for hiding this comment

pfalcon Jan 28, 2019 • edited

Choose a reason for hiding this comment

rlubos Jan 28, 2019

Choose a reason for hiding this comment

pfalcon Jan 28, 2019

Choose a reason for hiding this comment

rlubos Jan 28, 2019

Choose a reason for hiding this comment

rlubos commented Jan 28, 2019

pfalcon commented Jan 28, 2019

rlubos commented Jan 28, 2019

mike-scott commented Jan 28, 2019

pfalcon left a comment

Choose a reason for hiding this comment

mike-scott commented Jan 28, 2019

codecov-io commented Jan 25, 2019 •

edited

pfalcon Jan 28, 2019 •

edited