gnrc_netif: check all required options on initialization #10532

miri64 · 2018-11-30T19:10:37Z

Contribution description

With DEVELHELP activated all required options required by GNRC are now checked at interface initialization, so that developers of new link-layer protocols or device drivers notice as soon as possible that something is missing.

Testing procedure

All already supported L2 protocols up until this point should still work without crashing at the start when DEVELHELP is activated:

Issues/PRs references

Depends on ~~#10524~~ merged.

miri64 · 2018-12-06T18:57:17Z

Rebased to current #10524

maribu · 2019-01-16T15:55:25Z

@miri64: I could test cc110x ;-) Mind to rebase?

miri64 · 2019-01-16T16:07:20Z

@miri64: I could test cc110x ;-) Mind to rebase?

I was about to. #10524 was just merged ;-)

miri64 · 2019-01-16T16:08:46Z

Rebased.

maribu

Looks code-wise good. I think some code was incorrectly merged, so comment ~~above~~ below.

sys/net/gnrc/netif/gnrc_netif_device_type.c

sys/net/gnrc/netif/gnrc_netif.c

miri64 · 2019-01-17T13:38:27Z

Addressed comments

maribu · 2019-01-18T15:58:48Z

Works with cc110x (both current and WIP rewrite version) :-)

miri64 · 2019-01-18T16:06:52Z

@maribu may I squash?

cladmi · 2019-01-18T16:25:21Z

I tested under @miri64's supervision:

I rebased on master and tested running examples/gnrc_networking.

It worked for native:

native

CFLAGS='-DDEBUG_ASSERT_VERBOSE=1' BOARD=native make -C examples/gnrc_networking term
make: Entering directory '/home/harter/work/git/RIOT/examples/gnrc_networking'
/home/harter/work/git/RIOT/examples/gnrc_networking/bin/native/gnrc_networking.elf tap0 
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2018.10-RC1-1106-g0c67c2-pr/riot/10532/check_all_required_options_on_initialization)
RIOT network stack example application
All up, running the shell now
> ifconfig
ifconfig
Iface  6  HWaddr: B6:99:7A:F3:D6:DB 
          MTU:1500  HL:64  RTR  
          RTR_ADV  
          Source address length: 6
          Link type: wired
          inet6 addr: fe80::b499:7aff:fef3:d6db  scope: local  VAL
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:fff3:d6db
          inet6 group: ff02::1a
          
          Statistics for Layer 2
            RX packets 4  bytes 440
            TX packets 1 (Multicast: 1)  bytes 78
            TX succeeded 1 errors 0
          Statistics for IPv6
            RX packets 4  bytes 384
            TX packets 1 (Multicast: 1)  bytes 64
            TX succeeded 1 errors 0

I have crashes for the following

nrf52dk with `nordic_softdevice_ble`

2019-01-18 17:16:31,496 - INFO # gnrc_netif: can't get address length from NDP link-layer address option on interface 7
2019-01-18 17:16:31,502 - INFO # sys/net/gnrc/netif/gnrc_netif_device_type.c:283 => 0x1fe21
2019-01-18 17:16:31,504 - INFO # *** RIOT kernel panic:
2019-01-18 17:16:31,506 - INFO # FAILED ASSERTION.
2019-01-18 17:16:31,506 - INFO # 
2019-01-18 17:16:31,513 - INFO #        pid | name                 | state    Q | pri | stack  ( used) | base addr  | current     
2019-01-18 17:16:31,522 - INFO #          - | isr_stack            | -        - |   - |   8192 (  720) | 0x20002800 | 0x200047c8
2019-01-18 17:16:31,529 - INFO #          1 | idle                 | pending  Q |  15 |    256 (  120) | 0x20004b74 | 0x20004bfc 
2019-01-18 17:16:31,538 - INFO #          2 | main                 | pending  Q |   7 |   1536 (  232) | 0x20004c74 | 0x2000518c 
2019-01-18 17:16:31,546 - INFO #          3 | pktdump              | bl rx    _ |   6 |   1536 (  236) | 0x20007278 | 0x2000778c 
2019-01-18 17:16:31,554 - INFO #          4 | 6lo                  | bl rx    _ |   3 |   1024 (  256) | 0x2000787c | 0x20007b7c 
2019-01-18 17:16:31,562 - INFO #          5 | ipv6                 | bl rx    _ |   4 |   1024 (  276) | 0x20005310 | 0x200055fc 
2019-01-18 17:16:31,570 - INFO #          6 | udp                  | bl rx    _ |   5 |   1024 (  256) | 0x20007e04 | 0x20008104 
2019-01-18 17:16:31,578 - INFO #          7 | ble                  | running  Q |   2 |   1024 (  572) | 0x20008708 | 0x20008a8c 
2019-01-18 17:16:31,583 - INFO #            | SUM                  |            |     |  15616 ( 2668)

nrf52dk with `nrfmin`

2019-01-18 17:17:11,561 - INFO # sys/net/gnrc/netif/gnrc_netif.c:1204 => 0xce5
2019-01-18 17:17:11,563 - INFO # *** RIOT kernel panic:
2019-01-18 17:17:11,565 - INFO # FAILED ASSERTION.
2019-01-18 17:17:11,566 - INFO # 
2019-01-18 17:17:11,573 - INFO #        pid | name                 | state    Q | pri | stack  ( used) | base addr  | current     
2019-01-18 17:17:11,581 - INFO #          - | isr_stack            | -        - |   - |    512 (   84) | 0x20000000 | 0x200001c8
2019-01-18 17:17:11,589 - INFO #          1 | idle                 | pending  Q |  15 |    256 (  124) | 0x20000560 | 0x200005e4 
2019-01-18 17:17:11,597 - INFO #          2 | main                 | pending  Q |   7 |   1536 (  244) | 0x20000660 | 0x20000b6c 
2019-01-18 17:17:11,605 - INFO #          3 | pktdump              | bl rx    _ |   6 |   1536 (  236) | 0x20002e10 | 0x20003324 
2019-01-18 17:17:11,613 - INFO #          4 | 6lo                  | bl rx    _ |   3 |   1024 (  256) | 0x20003a64 | 0x20003d64 
2019-01-18 17:17:11,621 - INFO #          5 | ipv6                 | bl rx    _ |   4 |   1024 (  256) | 0x20000cb4 | 0x20000fb4 
2019-01-18 17:17:11,629 - INFO #          6 | udp                  | bl rx    _ |   5 |   1024 (  252) | 0x20004240 | 0x20004544 
2019-01-18 17:17:11,637 - INFO #          7 | nrfmin               | running  Q |   2 |   1024 (  564) | 0x20004640 | 0x200049c4 
2019-01-18 17:17:11,643 - INFO #            | SUM                  |            |     |   7936 ( 2016)

iotlab-m3

2019-01-18 17:24:32,036 - INFO #  �drivers/netdev_ieee802154/netdev_ieee802154.c:133 => 0x80013cd
2019-01-18 17:24:32,038 - INFO # *** RIOT kernel panic:
2019-01-18 17:24:32,038 - INFO # FAILED ASSERTION.
2019-01-18 17:24:32,039 - INFO # 
2019-01-18 17:24:32,040 - INFO #        pid | name                 | state    Q | pri | stack  ( used) | base addr  | current     
2019-01-18 17:24:32,041 - INFO #          - | isr_stack            | -        - |   - |    512 (  140) | 0x20000000 | 0x200001c8
2019-01-18 17:24:32,042 - INFO #          1 | idle                 | pending  Q |  15 |    256 (  128) | 0x20000594 | 0x20000614 
2019-01-18 17:24:32,043 - INFO #          2 | main                 | bl mutex _ |   7 |   1536 (  256) | 0x20000694 | 0x20000bbc 
2019-01-18 17:24:32,046 - INFO #          3 | pktdump              | bl rx    _ |   6 |   1536 (  240) | 0x2000331c | 0x2000382c 
2019-01-18 17:24:32,047 - INFO #          4 | 6lo                  | bl rx    _ |   3 |   1024 (  256) | 0x20003f6c | 0x2000426c 
2019-01-18 17:24:32,048 - INFO #          5 | ipv6                 | bl rx    _ |   4 |   1024 (  260) | 0x200011c0 | 0x200014bc 
2019-01-18 17:24:32,049 - INFO #          6 | udp                  | bl rx    _ |   5 |   1024 (  252) | 0x20004748 | 0x20004a4c 
2019-01-18 17:24:32,050 - INFO #          7 | at86rf2xx            | running  Q |   2 |   1024 (  760) | 0x20000d64 | 0x20000e6c 
2019-01-18 17:24:32,051 - INFO #          8 | RPL                  | bl rx    _ |   5 |   1024 (  196) | 0x20003970 | 0x20003cac 
2019-01-18 17:24:32,061 - INFO #            | SUM                  |            |     |   8960 ( 2488)
2019-01-18 17:24:32,062 - INFO # 
2019-01-18 17:24:32,062 - INFO # *** halted.

miri64 · 2019-01-18T16:26:16Z

Thanks @cladmi. @kb2ma or @gebart could you maybe check SLIP?

miri64 · 2019-01-18T16:48:22Z

I rebased to master (to include the changes from #10514) and fixed the bugs found by @cladmi.

cladmi · 2019-01-18T17:00:57Z

Does not crash anymore for iotlab-m3, native, nrf52dk with nordic_softdevice_ble.
It crashes somewhere else with nrfmin:

2019-01-18 17:57:59,041 - INFO # cpu/nrf5x_common/radio/nrfmin/nrfmin.c:493 => 0xce5
2019-01-18 17:57:59,043 - INFO # *** RIOT kernel panic:
2019-01-18 17:57:59,044 - INFO # FAILED ASSERTION.
2019-01-18 17:57:59,045 - INFO # 
2019-01-18 17:57:59,053 - INFO #        pid | name                 | state    Q | pri | stack  ( used) | base addr  | current     
2019-01-18 17:57:59,060 - INFO #          - | isr_stack            | -        - |   - |    512 (   84) | 0x20000000 | 0x200001c8
2019-01-18 17:57:59,069 - INFO #          1 | idle                 | pending  Q |  15 |    256 (  124) | 0x20000560 | 0x200005e4 
2019-01-18 17:57:59,076 - INFO #          2 | main                 | pending  Q |   7 |   1536 (  244) | 0x20000660 | 0x20000b6c 
2019-01-18 17:57:59,084 - INFO #          3 | pktdump              | bl rx    _ |   6 |   1536 (  236) | 0x20002e10 | 0x20003324 
2019-01-18 17:57:59,092 - INFO #          4 | 6lo                  | bl rx    _ |   3 |   1024 (  256) | 0x20003a64 | 0x20003d64 
2019-01-18 17:57:59,101 - INFO #          5 | ipv6                 | bl rx    _ |   4 |   1024 (  256) | 0x20000cb4 | 0x20000fb4 
2019-01-18 17:57:59,109 - INFO #          6 | udp                  | bl rx    _ |   5 |   1024 (  252) | 0x20004240 | 0x20004544 
2019-01-18 17:57:59,117 - INFO #          7 | nrfmin               | running  Q |   2 |   1024 (  572) | 0x20004640 | 0x200049c4 
2019-01-18 17:57:59,122 - INFO #            | SUM                  |            |     |   7936 ( 2024)
2019-01-18 17:57:59,123 - INFO # 
2019-01-18 17:57:59,124 - INFO # *** halted.

cladmi · 2019-01-18T17:10:01Z

msba2 is working (but the flasher path is not found automatically, I will remember to fix it one day :D)

cladmi · 2019-01-18T17:36:03Z

nrf52dk using nrfmin is fixed with #10822

cladmi · 2019-01-18T17:43:37Z

I tested slip with examples/gcoap/Makefile.slip

CFLAGS='-DDEBUG_ASSERT_VERBOSE=1' BOARD=samr21-xpro make -C examples/gcoap -f Makefile.slip flash

It does not crash in master but crashes here:

> 2019-01-18 18:42:09,124 - INFO #  sys/net/gnrc/netif/gnrc_netif.c:1242 => 0xbf5
2019-01-18 18:42:09,126 - INFO # *** RIOT kernel panic:
2019-01-18 18:42:09,128 - INFO # FAILED ASSERTION.
2019-01-18 18:42:09,128 - INFO #
2019-01-18 18:42:09,136 - INFO #        pid | name                 | state    Q | pri | stack  ( used) | base addr  | current
2019-01-18 18:42:09,144 - INFO #          - | isr_stack            | -        - |   - |    512 (  144) | 0x20000000 | 0x200001b8
2019-01-18 18:42:09,153 - INFO #          1 | idle                 | pending  Q |  15 |    256 (  124) | 0x200006f8 | 0x2000077c
2019-01-18 18:42:09,161 - INFO #          2 | main                 | pending  Q |   7 |   1536 (  268) | 0x200007f8 | 0x20000cec
2019-01-18 18:42:09,170 - INFO #          3 | 6lo                  | bl rx    _ |   3 |   1024 (  280) | 0x20004b8c | 0x20004e74
2019-01-18 18:42:09,178 - INFO #          4 | ipv6                 | bl rx    _ |   4 |   1024 (  284) | 0x20002438 | 0x2000271c
2019-01-18 18:42:09,186 - INFO #          5 | udp                  | bl rx    _ |   5 |   1024 (  276) | 0x20005368 | 0x20005654
2019-01-18 18:42:09,195 - INFO #          6 | coap                 | bl mbox  _ |   6 |   1108 (  676) | 0x20001f44 | 0x200020f4
2019-01-18 18:42:09,203 - INFO #          7 | at86rf2xx            | bl rx    _ |   2 |   1024 (  436) | 0x20001ae4 | 0x20001dd4
2019-01-18 18:42:09,211 - INFO #          8 | slipdev              | running  Q |   2 |   1024 (  596) | 0x20000ebc | 0x2000123c
2019-01-18 18:42:09,217 - INFO #            | SUM                  |            |     |   8532 ( 3084)
2019-01-18 18:42:09,217 - INFO #
2019-01-18 18:42:09,218 - INFO # *** halted.
2019-01-18 18:42:09,218 - INFO #

miri64 · 2019-01-18T17:49:44Z

I tested slip with examples/gcoap/Makefile.slip

CFLAGS='-DDEBUG_ASSERT_VERBOSE=1' BOARD=samr21-xpro make -C examples/gcoap -f Makefile.slip flash

It does not crash in master but crashes here:

> 2019-01-18 18:42:09,124 - INFO #  sys/net/gnrc/netif/gnrc_netif.c:1242 => 0xbf5
2019-01-18 18:42:09,126 - INFO # *** RIOT kernel panic:
2019-01-18 18:42:09,128 - INFO # FAILED ASSERTION.
2019-01-18 18:42:09,128 - INFO #
2019-01-18 18:42:09,136 - INFO #        pid | name                 | state    Q | pri | stack  ( used) | base addr  | current
2019-01-18 18:42:09,144 - INFO #          - | isr_stack            | -        - |   - |    512 (  144) | 0x20000000 | 0x200001b8
2019-01-18 18:42:09,153 - INFO #          1 | idle                 | pending  Q |  15 |    256 (  124) | 0x200006f8 | 0x2000077c
2019-01-18 18:42:09,161 - INFO #          2 | main                 | pending  Q |   7 |   1536 (  268) | 0x200007f8 | 0x20000cec
2019-01-18 18:42:09,170 - INFO #          3 | 6lo                  | bl rx    _ |   3 |   1024 (  280) | 0x20004b8c | 0x20004e74
2019-01-18 18:42:09,178 - INFO #          4 | ipv6                 | bl rx    _ |   4 |   1024 (  284) | 0x20002438 | 0x2000271c
2019-01-18 18:42:09,186 - INFO #          5 | udp                  | bl rx    _ |   5 |   1024 (  276) | 0x20005368 | 0x20005654
2019-01-18 18:42:09,195 - INFO #          6 | coap                 | bl mbox  _ |   6 |   1108 (  676) | 0x20001f44 | 0x200020f4
2019-01-18 18:42:09,203 - INFO #          7 | at86rf2xx            | bl rx    _ |   2 |   1024 (  436) | 0x20001ae4 | 0x20001dd4
2019-01-18 18:42:09,211 - INFO #          8 | slipdev              | running  Q |   2 |   1024 (  596) | 0x20000ebc | 0x2000123c
2019-01-18 18:42:09,217 - INFO #            | SUM                  |            |     |   8532 ( 3084)
2019-01-18 18:42:09,217 - INFO #
2019-01-18 18:42:09,218 - INFO # *** halted.
2019-01-18 18:42:09,218 - INFO #

Latest commit fixes that.

miri64 · 2019-01-18T17:52:05Z

Rebased to include changes made in #10822.

miri64 · 2019-01-18T17:52:39Z

Thanks for all the testing @cladmi. I really appreciate it. Sadly the rebase means the tests should be run again :-/.

miri64 · 2019-01-19T15:06:52Z

There's some foobar happening due to my fixes to it in tests/gnrc_ndp I don't understand yet. Also the "generic" interfaces in gnrc_netif now make problems so that I'm thinking to replace

RIOT/sys/net/gnrc/netif/gnrc_netif.c

Lines 1238 to 1240 in 59130e6

    
                       /* device type not supported yet, please amend case above when 
        
                        * porting new device type */ 
        
                       assert(false);

just with a LOG_INFO() message. I'll debug further on monday, but now I want to do some other weekend stuff ;-).

miri64 · 2019-01-19T21:24:26Z

Should now work. Note to self: provide some common mock netdevs for the most used device types...

miri64 · 2019-01-19T21:44:10Z

For the fixups I did not add any asserts to gnrc_netif, so all your testing efforts should still be valid. However, now that I see that at least from a testing perspective this PR is more involved, I'd say we wait for after the feature freeze of the current release to merge this.

cladmi

From @miri64
However, now that I see that at least from a testing perspective this PR is more involved, I'd say we wait for after the feature freeze of the current release to merge this.

I just put a request changes to prevent merging too early.
Feel free to dismiss my review after the feature freeze or if you want to merge it anyway.

miri64 · 2019-02-26T14:46:22Z

Rebased to current master. @cladmi I think you can remove your block now.

No need to wait anymore

maribu · 2019-02-26T15:23:54Z

My remarks have been addressed, thus I shall no longer block this PR :-)

@miri64: Squash?

@cladmi: Mind to hit merge later? I'll power off my PC now...

With `DEVELHELP` activated all required options required by GNRC are now checked at interface initialization, so that developers of new link-layer protocols or device drivers notice as soon as possible that something is missing.

miri64 · 2019-02-26T15:46:48Z

In the meantime, I squashed.

maribu · 2019-02-26T16:30:11Z

(Turns out you can press the merge buttons also via phone :-) )

miri64 · 2019-02-26T16:37:08Z

(only if all CIs are happy ;-))

miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking State: waiting for other PR State: The PR requires another PR to be merged first labels Nov 30, 2018

miri64 requested a review from haukepetersen November 30, 2018 19:10

miri64 force-pushed the gnrc_netif/enh/check-opt-at-init branch from 94c0b4d to e8f4bba Compare December 6, 2018 18:57

miri64 force-pushed the gnrc_netif/enh/check-opt-at-init branch from e8f4bba to df34247 Compare January 16, 2019 16:08

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 Jan 16, 2019

maribu requested changes Jan 17, 2019

View reviewed changes

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

maribu reviewed Jan 17, 2019

View reviewed changes

sys/net/gnrc/netif/gnrc_netif.c Outdated Show resolved Hide resolved

miri64 force-pushed the gnrc_netif/enh/check-opt-at-init branch from d7adb74 to 988bbc1 Compare January 18, 2019 16:47

miri64 mentioned this pull request Jan 18, 2019

nrfmin: fix type for NETOPT_PROTO #10822

Merged

miri64 force-pushed the gnrc_netif/enh/check-opt-at-init branch from 722a356 to df16332 Compare January 18, 2019 17:51

cladmi previously requested changes Jan 21, 2019

View reviewed changes

gnrc_netif: provide addr_len_from_l2ao for nordic_softdevice_ble

0f1bfad

miri64 force-pushed the gnrc_netif/enh/check-opt-at-init branch from 45d0db7 to 77a90eb Compare February 26, 2019 14:46

maribu approved these changes Feb 26, 2019

View reviewed changes

miri64 added 3 commits February 26, 2019 16:45

gnrc_netif: check all required options on initialization

7d8f6ab

With `DEVELHELP` activated all required options required by GNRC are now checked at interface initialization, so that developers of new link-layer protocols or device drivers notice as soon as possible that something is missing.

netdev: introduce test device type

2d6a557

tests: adapt tests for gnrc_netif checks

dd1f45d

miri64 force-pushed the gnrc_netif/enh/check-opt-at-init branch from 77a90eb to dd1f45d Compare February 26, 2019 15:46

maribu merged commit cebea3a into RIOT-OS:master Feb 26, 2019

miri64 deleted the gnrc_netif/enh/check-opt-at-init branch February 26, 2019 16:36

miri64 mentioned this pull request Feb 26, 2019

l2util: initial import of a general IPv6 over X helper module #10589

Merged

danpetry added this to the Release 2019.04 milestone Mar 12, 2019

miri64 mentioned this pull request Aug 9, 2019

tests/gnrc_ndp: enhance coverage #10987

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gnrc_netif: check all required options on initialization #10532

gnrc_netif: check all required options on initialization #10532

miri64 commented Nov 30, 2018 •

edited by gschorcht

Loading

miri64 commented Dec 6, 2018

maribu commented Jan 16, 2019

miri64 commented Jan 16, 2019

miri64 commented Jan 16, 2019

maribu left a comment •

edited

Loading

miri64 commented Jan 17, 2019

maribu commented Jan 18, 2019

miri64 commented Jan 18, 2019

cladmi commented Jan 18, 2019

miri64 commented Jan 18, 2019

miri64 commented Jan 18, 2019

cladmi commented Jan 18, 2019

cladmi commented Jan 18, 2019

cladmi commented Jan 18, 2019

cladmi commented Jan 18, 2019

miri64 commented Jan 18, 2019

miri64 commented Jan 18, 2019

miri64 commented Jan 18, 2019

miri64 commented Jan 19, 2019

miri64 commented Jan 19, 2019

miri64 commented Jan 19, 2019 •

edited

Loading

cladmi left a comment

miri64 commented Feb 26, 2019

maribu commented Feb 26, 2019

miri64 commented Feb 26, 2019

maribu commented Feb 26, 2019

miri64 commented Feb 26, 2019

gnrc_netif: check all required options on initialization #10532

gnrc_netif: check all required options on initialization #10532

Conversation

miri64 commented Nov 30, 2018 • edited by gschorcht Loading

Contribution description

Testing procedure

Issues/PRs references

miri64 commented Dec 6, 2018

maribu commented Jan 16, 2019

miri64 commented Jan 16, 2019

miri64 commented Jan 16, 2019

maribu left a comment • edited Loading

Choose a reason for hiding this comment

miri64 commented Jan 17, 2019

maribu commented Jan 18, 2019

miri64 commented Jan 18, 2019

cladmi commented Jan 18, 2019

native

nrf52dk with nordic_softdevice_ble

nrf52dk with nrfmin

iotlab-m3

miri64 commented Jan 18, 2019

miri64 commented Jan 18, 2019

cladmi commented Jan 18, 2019

cladmi commented Jan 18, 2019

cladmi commented Jan 18, 2019

cladmi commented Jan 18, 2019

miri64 commented Jan 18, 2019

miri64 commented Jan 18, 2019

miri64 commented Jan 18, 2019

miri64 commented Jan 19, 2019

miri64 commented Jan 19, 2019

miri64 commented Jan 19, 2019 • edited Loading

cladmi left a comment

Choose a reason for hiding this comment

miri64 commented Feb 26, 2019

maribu commented Feb 26, 2019

miri64 commented Feb 26, 2019

maribu commented Feb 26, 2019

miri64 commented Feb 26, 2019

miri64 commented Nov 30, 2018 •

edited by gschorcht

Loading

maribu left a comment •

edited

Loading

nrf52dk with `nordic_softdevice_ble`

nrf52dk with `nrfmin`

miri64 commented Jan 19, 2019 •

edited

Loading