gnrc_netif: add general IID to/from l2addr conversion functions by miri64 · Pull Request #10513 · RIOT-OS/RIOT

miri64 · 2018-11-29T13:37:27Z

Contribution description

They are required to make GNRC's 6Lo IPHC implementation ready for hardware addresses other than IEEE 802.15.4's.

Testing procedure

Pinging a link-local address with gnrc_networking should still work for

Issues/PRs references

Depends on ~~#10509~~ (merged), ~~#10514~~ (invalid dependency), and ~~#10515~~ (merged).

miri64 · 2018-11-29T14:48:01Z

Rebased to current master and new dependencies.

…c_netif_hdr/enh/conv-iid-getter

miri64 · 2018-11-30T08:37:29Z

Rebased to and adapted for current master (since the #10514 dependency actually requires a PR downstream from this PR - #10514 (comment)). Ready for review and testing.

maribu

Looks good to me code-wise :-)

maribu · 2018-12-04T19:51:00Z

I hope I can test it tomorrow with the cc110x (both current and future) and with an enc28j60.

sys/net/gnrc/netif/gnrc_netif_device_type.c

maribu · 2018-12-05T09:16:19Z

To me it looks like the current issue Murdock uncovered seems to be unrelated.

@cladmi: As far as I know you are a linker magician. Care to have a look at the murdock build log?

/usr/lib/gcc/avr/5.4.0/../../../avr/bin/ld: address 0x80220a of /tmp/dwq.0.8781428163408961/ce7b5c92c43c4dec218e57c9939ac8c2/build/tests_gnrc_sock_udp.elf section `.bss' is not within region `data'
/usr/lib/gcc/avr/5.4.0/../../../avr/bin/ld: address 0x80220c of /tmp/dwq.0.8781428163408961/ce7b5c92c43c4dec218e57c9939ac8c2/build/tests_gnrc_sock_udp.elf section `.noinit' is not within region `data'
/usr/lib/gcc/avr/5.4.0/../../../avr/bin/ld: address 0x80220a of /tmp/dwq.0.8781428163408961/ce7b5c92c43c4dec218e57c9939ac8c2/build/tests_gnrc_sock_udp.elf section `.bss' is not within region `data'
/usr/lib/gcc/avr/5.4.0/../../../avr/bin/ld: address 0x80220c of /tmp/dwq.0.8781428163408961/ce7b5c92c43c4dec218e57c9939ac8c2/build/tests_gnrc_sock_udp.elf section `.noinit' is not within region `data'
collect2: error: ld returned 1 exit status

This does not seem to be an overflow, as in case of an overflow I get a message like this:

/usr/lib/gcc/arm-none-eabi/6.3.1/../../../../arm-none-eabi/bin/ld: /home/maribu/Repos/software/RIOT/examples/gnrc_networking/bin/bluepill/gnrc_networking.elf section `.text' will not fit in region `rom'
/usr/lib/gcc/arm-none-eabi/6.3.1/../../../../arm-none-eabi/bin/ld: region `rom' overflowed by 17640 bytes

(I cannot recall how the message looks like in case of a RAM overflow, but I assume it will also use the phrase "overflowed by xy bytes".)

haukepetersen · 2018-12-05T09:35:10Z

would you mind to rebase? That would simplify the review a bit... Thx

miri64 · 2018-12-05T15:54:01Z

Back on track: @maribu were you able to test by now?

cladmi · 2018-12-05T16:00:11Z

To show the output for the arduino-mega2560:

On my ubuntu-bionic machine:

With this PR:

RIOT_CI_BUILD=1 BOARD=arduino-mega2560 make -C tests/gnrc_sock_udp/
make: Entering directory '/home/harter/work/git/RIOT/tests/gnrc_sock_udp'
Building application "tests_gnrc_sock_udp" for "arduino-mega2560" with MCU "atmega2560".

/usr/lib/gcc/avr/5.4.0/../../../avr/bin/ld: address 0x80220a of /home/harter/work/git/RIOT/tests/gnrc_sock_udp/bin/arduino-mega2560/tests_gnrc_sock_udp.elf section `.bss' is not within region `data'
/usr/lib/gcc/avr/5.4.0/../../../avr/bin/ld: address 0x80220c of /home/harter/work/git/RIOT/tests/gnrc_sock_udp/bin/arduino-mega2560/tests_gnrc_sock_udp.elf section `.noinit' is not within region `data'
/usr/lib/gcc/avr/5.4.0/../../../avr/bin/ld: address 0x80220a of /home/harter/work/git/RIOT/tests/gnrc_sock_udp/bin/arduino-mega2560/tests_gnrc_sock_udp.elf section `.bss' is not within region `data'
/usr/lib/gcc/avr/5.4.0/../../../avr/bin/ld: address 0x80220c of /home/harter/work/git/RIOT/tests/gnrc_sock_udp/bin/arduino-mega2560/tests_gnrc_sock_udp.elf section `.noinit' is not within region `data'
collect2: error: ld returned 1 exit status
/home/harter/work/git/RIOT/tests/gnrc_sock_udp/../../Makefile.include:446: recipe for target '/home/harter/work/git/RIOT/tests/gnrc_sock_udp/bin/arduino-mega2560/tests_gnrc_sock_udp.elf' failed
make: *** [/home/harter/work/git/RIOT/tests/gnrc_sock_udp/bin/arduino-mega2560/tests_gnrc_sock_udp.elf] Error 1
make: Leaving directory '/home/harter/work/git/RIOT/tests/gnrc_sock_udp'

With a "bigger" RAM, 16K instead of 8K linking works properly:

RIOT_CI_BUILD=1 BOARD=arduino-mega2560 make -C tests/gnrc_sock_udp/ RAM_LEN=16K
make: Entering directory '/home/harter/work/git/RIOT/tests/gnrc_sock_udp'
Building application "tests_gnrc_sock_udp" for "arduino-mega2560" with MCU "atmega2560".

   text    data     bss     dec     hex filename
  46690    4362    3842   54894    d66e /home/harter/work/git/RIOT/tests/gnrc_sock_udp/bin/arduino-mega2560/tests_gnrc_sock_udp.elf
make: Leaving directory '/home/harter/work/git/RIOT/tests/gnrc_sock_udp'

So the board just needs to be put in BOARD_INSUFFICIENT_MEMORY.

maribu · 2018-12-05T16:13:07Z

Works fine with the CC1100/MSB-A2 using the current cc110x driver and works fine with the CC1101/MSB-IoT using PR #10340

miri64 · 2018-12-05T17:27:43Z

@gschorcht can you test with ESP32?

gschorcht · 2018-12-05T21:19:12Z

Works fine with the ESP32

miri64 · 2018-12-05T23:04:04Z

Thanks for testing @maribu and @gschorcht!

maribu · 2018-12-06T09:56:16Z

I just found my enc28j60. I will test as soon as I have time

miri64 · 2018-12-06T10:03:32Z

I just found my enc28j60. I will test as soon as I have time

You don't have to if you don't have the time. native can also be used for testing Ethernet.

gschorcht · 2018-12-06T10:06:52Z

@miri64 I forgot to mention, I tested ESP32 only with esp_now netdev driver.

I have also an ESP32 board with EMAC and LAN8270 ethernet interface. So, I tested it with esp_eth netdev driver too and it works fine.

TimoRoth · 2018-12-06T11:27:15Z

What I'm wondering about ESP-NOW is that for examples its _send function expects the data to be a pointer to an ipv6_hdr_t:

https://github.com/RIOT-OS/RIOT/blob/master/cpu/esp32/esp-now/esp_now_netdev.c#L526

But with 6Lo, wouldn't something along those lines be needed, where the first byte of the data indicates what actually follows:

https://github.com/RIOT-OS/RIOT/blob/master/sys/net/network_layer/sixlowpan/sixlowpan_print.c#L23

Or is that unpacked for ESP-NOW at an earlier point, and it only gets to see properly fragmented actual IPv6 packages?

miri64 · 2018-12-06T11:30:13Z

@TimoRoth can we please discuss 6Lo over ESP-Now in #10531 rather than this unrelated issue?

miri64 · 2018-12-06T11:31:06Z

But to answer your question: yes ESP-now needs then to assume, that the first header is a 6Lo dispatch instead of a IPv6 header.

miri64 · 2018-12-06T11:34:47Z

(this whole file looks rather hacked I might add oO)

haukepetersen · 2018-12-06T13:32:58Z

tested all remaining link layers successfully:

nrfmin using two nrf52dk boards no my desk -> ping works both ways
ieee802.15.4 using two iotlab-m3 boards in the iot-lab -> ping works both ways
ethernet using native -> ping works two ways
softdevice -> ping to/from linux does not work, but the same is true for master. But the IID generation (verified using ifconfig) seems to do the trick, so I consider this PR not doing any harm here...

haukepetersen

ACK

They are required to make GNRC's 6Lo IPHC implementation ready for hardware addresses other than IEEE 802.15.4's.

miri64 · 2018-12-06T13:41:42Z

Squashed

miri64 · 2018-12-06T14:21:03Z

Marked arduino-mega2560 in tests/gnrc_sock_udp for insufficient memory.

haukepetersen · 2018-12-06T14:56:59Z

all green -> go!

miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. labels Nov 29, 2018

miri64 requested review from bergzand and haukepetersen November 29, 2018 13:37

miri64 changed the title ~~Gnrc netif/enh/iid conversion~~ gnrc_netif: add general IID to/from l2addr conversion functions Nov 29, 2018

miri64 requested review from gschorcht and maribu November 29, 2018 13:43

miri64 force-pushed the gnrc_netif/enh/iid-conversion branch from b20cdb4 to 80258ef Compare November 29, 2018 14:46

miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 29, 2018

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 29, 2018

Merge branch 'gnrc_netif/enh/iid-conversion' (RIOT-OS#10513) into gnr…

42b680c

…c_netif_hdr/enh/conv-iid-getter

This was referenced Nov 29, 2018

gnrc_netif_hdr: provide wrappers for gnrc_netif_ipv6_iid_from_addr() #10520

Merged

ethernet: make input parameter const #10522

Merged

gnrc_netif: centralize device-type-specific functions #10524

Merged

miri64 force-pushed the gnrc_netif/enh/iid-conversion branch from c618ccd to b2a05a5 Compare November 30, 2018 08:35

miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Nov 30, 2018

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 30, 2018

Merge branch 'gnrc_netif/enh/iid-conversion' (RIOT-OS#10513) into HEAD

3eabe3e

miri64 mentioned this pull request Dec 3, 2018

gnrc_ipv6_nib: use correct reverse-translate for ARSM-less NCE #10536

Merged

maribu reviewed Dec 4, 2018

View reviewed changes

sys/net/gnrc/netif/gnrc_netif_device_type.c Show resolved Hide resolved

miri64 added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Dec 6, 2018

haukepetersen approved these changes Dec 6, 2018

View reviewed changes

miri64 added 3 commits December 6, 2018 14:41

gnrc_netif: add general IID to/from l2addr conversion functions

5ca30b9

They are required to make GNRC's 6Lo IPHC implementation ready for hardware addresses other than IEEE 802.15.4's.

gnrc_netif: use general IID conversion function for netif-based

a44f7f7

gnrc_ipv6_nib: gnrc_netif_iid_to_addr() instead of _reverse_iid()

bc1b490

miri64 force-pushed the gnrc_netif/enh/iid-conversion branch from 0535402 to bc1b490 Compare December 6, 2018 13:41

tests/gnrc_sock_udp: mark arduino-atmega2560 for insufficient memory

f757376

miri64 force-pushed the gnrc_netif/enh/iid-conversion branch from c7d146d to f757376 Compare December 6, 2018 14:39

haukepetersen merged commit 92fcb4d into RIOT-OS:master Dec 6, 2018

miri64 deleted the gnrc_netif/enh/iid-conversion branch December 6, 2018 14:57

This was referenced Dec 6, 2018

gnrc/netif/internal: fix copy-paste error #10565

Merged

net: add eui48 type including IPv6 IID conversion funcs #10567

Merged

aabadie added this to the Release 2019.01 milestone Dec 19, 2018

Conversation

miri64 commented Nov 29, 2018 • edited by haukepetersen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

miri64 commented Nov 29, 2018

Uh oh!

miri64 commented Nov 30, 2018

Uh oh!

maribu left a comment

Choose a reason for hiding this comment

Uh oh!

maribu commented Dec 4, 2018

Uh oh!

Uh oh!

maribu commented Dec 5, 2018

Uh oh!

haukepetersen commented Dec 5, 2018

Uh oh!

miri64 commented Dec 5, 2018

Uh oh!

cladmi commented Dec 5, 2018

Uh oh!

maribu commented Dec 5, 2018

Uh oh!

miri64 commented Dec 5, 2018

Uh oh!

gschorcht commented Dec 5, 2018

Uh oh!

miri64 commented Dec 5, 2018

Uh oh!

maribu commented Dec 6, 2018

Uh oh!

miri64 commented Dec 6, 2018

Uh oh!

gschorcht commented Dec 6, 2018

Uh oh!

TimoRoth commented Dec 6, 2018

Uh oh!

miri64 commented Dec 6, 2018

Uh oh!

miri64 commented Dec 6, 2018

Uh oh!

miri64 commented Dec 6, 2018

Uh oh!

haukepetersen commented Dec 6, 2018

Uh oh!

haukepetersen left a comment

Choose a reason for hiding this comment

Uh oh!

miri64 commented Dec 6, 2018

Uh oh!

miri64 commented Dec 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haukepetersen commented Dec 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

miri64 commented Nov 29, 2018 •

edited by haukepetersen

Loading

miri64 commented Dec 6, 2018 •

edited

Loading