Skip to content

drivers/at86rf2xx: use luid_get_eui64() / luid_get_short()#13362

Merged
aabadie merged 1 commit intoRIOT-OS:masterfrom
benpicco:at86rf2xx-luid_get
Feb 13, 2020
Merged

drivers/at86rf2xx: use luid_get_eui64() / luid_get_short()#13362
aabadie merged 1 commit intoRIOT-OS:masterfrom
benpicco:at86rf2xx-luid_get

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Feb 13, 2020 •
edited
Loading

Contribution description

Use dedicated helper functions to generate long and short address.
luid_get_short() will use the 'next' pseudo-random luid instead of re-using the previous one, so this might alleviate the address collisions observed in #13358

Testing procedure

Issues/PRs references

Maybe fixes #13358

@benpicco
drivers/at86rf2xx: use luid_get_eui64() / luid_get_short()
63f6d6a 
Use dedicated helper functions to generate long and short address.

Maybe fixes RIOT-OS#13358
@benpicco benpicco added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 13, 2020
@benpicco benpicco mentioned this pull request Feb 13, 2020
Closed
@maribu maribu added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Feb 13, 2020
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Feb 13, 2020

I'm trying this PR on several samr21-xpro and iotlab-m3.

On samr21-xpro, it doesn't really help, there are still collisions:

samr21-1.saclay:           Long HWaddr: 1A:30:3B:84:5D:00:64:78 
samr21-2.saclay:           Long HWaddr: 46:E6:75:EC:4E:0C:65:78 
samr21-5.saclay:           Long HWaddr: 2A:62:E2:F7:72:08:67:78 
samr21-3.saclay:           Long HWaddr: CE:BC:1A:14:7F:04:67:78 
samr21-9.saclay:           Long HWaddr: 26:9A:F8:DE:59:09:63:78 
samr21-4.saclay:           Long HWaddr: 22:9E:46:3E:57:37:64:78 
samr21-6.saclay:           Long HWaddr: 96:B9:AF:02:60:33:67:78 
samr21-8.saclay:           Long HWaddr: 2E:60:4D:64:5C:3C:65:78 
samr21-7.saclay:           Long HWaddr: 9A:CD:86:F3:7C:0F:67:78 
samr21-10.saclay:           Long HWaddr: E6:3D:A0:0E:64:33:62:78

On the M3, the problem remains the same, the numbers are just different:

m3-1.saclay:           Long HWaddr: 32:8F:F7:65:10:6B:11:14 
m3-2.saclay:           Long HWaddr: 36:BD:FD:65:10:6B:11:14 
m3-10.saclay:           Long HWaddr: 52:BD:FC:65:10:6B:11:14 
m3-9.saclay:           Long HWaddr: 36:BC:FD:65:10:6B:11:14 
m3-5.saclay:           Long HWaddr: 52:A8:FA:65:10:6B:11:14 
m3-6.saclay:           Long HWaddr: 36:BC:F8:65:10:6B:11:14 
m3-8.saclay:           Long HWaddr: 32:BD:F2:65:10:6B:11:14 
m3-7.saclay:           Long HWaddr: 36:8F:F9:65:10:6B:11:14 
m3-3.saclay:           Long HWaddr: 02:BC:F6:65:10:6B:11:14 
m3-4.saclay:           Long HWaddr: 32:8F:F8:65:10:6B:11:14

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 13, 2020

@aabadie have you looked at the short address? The last part of the long address isn't a problem, if the short address is not derived from it.

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 13, 2020

Those are just the long addresses and I don't see any collisions - what about the short addresses?

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Feb 13, 2020

I think I don't know what a short address is then ;)

I'm using examples/default but ifconfig doesn't return the short address, only the long address. And I always thought that txtsnd could only be used with short address, which is not the case apparently...

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 13, 2020

I'm using examples/default but ifconfig doesn't return the short address

2020-02-13 13:41:43,486 # Iface  4  HWaddr: 7D:79  Channel: 26  Page: 0  NID: 0x23
                                            ^^^^^
2020-02-13 13:41:43,498 #           Long HWaddr: 5A:06:59:C9:2C:36:7D:79 
2020-02-13 13:41:43,504 #            TX-Power: 0dBm  State: IDLE  max. Retrans.: 3  CSMA Retries: 4 
2020-02-13 13:41:43,518 #           AUTOACK  ACK_REQ  CSMA  L2-PDU:102 Source address length: 2

It's right there.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Feb 13, 2020

It's right there.

oups!

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Feb 13, 2020

Ok, it's much better with this PR on the M3 (on master the problem is really there):

m3-3.saclay: Iface  4  HWaddr: 54:86  Channel: 26  Page: 0  NID: 0x23
m3-3.saclay:           Long HWaddr: 02:BC:F6:65:10:6B:11:14 
m3-9.saclay: Iface  4  HWaddr: 6A:86  Channel: 26  Page: 0  NID: 0x23
m3-9.saclay:           Long HWaddr: 36:BC:FD:65:10:6B:11:14 
m3-7.saclay: Iface  4  HWaddr: 6E:B5  Channel: 26  Page: 0  NID: 0x23
m3-7.saclay:           Long HWaddr: 36:8F:F9:65:10:6B:11:14 
m3-2.saclay: Iface  4  HWaddr: 68:87  Channel: 26  Page: 0  NID: 0x23
m3-2.saclay:           Long HWaddr: 36:BD:FD:65:10:6B:11:14 
m3-1.saclay: Iface  4  HWaddr: 64:B5  Channel: 26  Page: 0  NID: 0x23
m3-1.saclay:           Long HWaddr: 32:8F:F7:65:10:6B:11:14 
m3-8.saclay: Iface  4  HWaddr: 61:87  Channel: 26  Page: 0  NID: 0x23
m3-8.saclay:           Long HWaddr: 32:BD:F2:65:10:6B:11:14 
m3-4.saclay: Iface  4  HWaddr: 68:B5  Channel: 26  Page: 0  NID: 0x23
m3-4.saclay:           Long HWaddr: 32:8F:F8:65:10:6B:11:14 
m3-6.saclay: Iface  4  HWaddr: 6C:86  Channel: 26  Page: 0  NID: 0x23
m3-6.saclay:           Long HWaddr: 36:BC:F8:65:10:6B:11:14 
m3-10.saclay: Iface  4  HWaddr: 0F:87  Channel: 26  Page: 0  NID: 0x23
m3-10.saclay:           Long HWaddr: 52:BD:FC:65:10:6B:11:14 
m3-5.saclay: Iface  4  HWaddr: 0A:92  Channel: 26  Page: 0  NID: 0x23
m3-5.saclay:           Long HWaddr: 52:A8:FA:65:10:6B:11:14

So I'd say this PR improves the situation.

@maribu
Copy link
Copy Markdown
Member

maribu commented Feb 13, 2020

Does it solve the issue for you?

@benpicco also has an open PR that allows to do proper management of L2 addresses. This could be used to e.g. load the L2 addresses from EEPROM. E.g. this is what we plan to do in our testbed to guarantee that all L2 addresses (including the short IEEE 802.15.4 addresses) are unique within the testbed. That PR could also be of interest for you.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 13, 2020

Still not quite sure why it only "improves" the situation and not fixes your issue though. :-/

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 13, 2020 •
edited
Loading

I'm still wondering why 44bit of the long address are still the same across devices.
My guess would be the CPU ID on STM32 is the same across large stretches as it also encodes the part ID.
I suppose luid_base() should do some real hashing (sponge function?) instead of just repeatedly XORing the CPU ID.
Maybe @maribu knows something lightweight? Doesn't have to be strong crypto, just sufficiently pseudo-random.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Feb 13, 2020

why it only "improves" the situation and not fixes your issue though.

It indeed fixes my problem: I couldn't see any collision with the short address (only tested on 10 M3 though).

@maribu
Copy link
Copy Markdown
Member

maribu commented Feb 13, 2020

I guess that counts as tested? So ACK?

aabadie
aabadie approved these changes Feb 13, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that counts as tested? So ACK?

ACK

@aabadie aabadie merged commit a055a6c into RIOT-OS:master Feb 13, 2020
@benpicco benpicco deleted the at86rf2xx-luid_get branch February 13, 2020 14:34
@fabian18 fabian18 mentioned this pull request Feb 17, 2020
Closed
@miri64 miri64 mentioned this pull request Feb 19, 2020
Merged
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Feb 20, 2020
@maribu maribu mentioned this pull request Jul 2, 2020
Closed
@aabadie aabadie mentioned this pull request Sep 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aabadie aabadie aabadie approved these changes

@maribu maribu Awaiting requested review from maribu

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2020.04

Development

Successfully merging this pull request may close these issues.

iotlab-m3: long HW address is in reversed order

5 participants

@benpicco @aabadie @miri64 @maribu @leandrolanzieri