gnrc_netif2: provide raw adaption layer (for e.g. SLIP)

[miri64]

Another adaption layer for #7370. This time for SLIP (#7381).

Depends on the mentioned PRs.

This PR is part of the network layer remodelling effort:

[miri64]

Rebased to current master and current dependencies

[miri64]

Rebased to current master and dependencies.

[miri64]

@kaspar030 made some comments about my usage of LOG_ERROR() in #7410. I will remove them here and in the other PRs.

[miri64]

Squashed and rebased. @kYc0o, please review ASAP.

[kYc0o]

With the embargo and the plans to not support any legacy driver belonging to "netif1", is there a reason to keep this called "netif2"?

[miri64]

The embargo is against master. Not against the branch the embargo is up in the first place ;-). And the rename is definitely planned (but only after everything is merged and definitely not in this only marginally related PR) ;-).

[kYc0o]

It just came to my mind because I saw it here, not because this PR would be the one where to do the renaming.

I'll test asap.

[miri64]

Ping @kYc0o?

[miri64]

Rebased to current #7925.

[miri64]

Another day a another rebase.

[bergzand]

The code here doesn't look that complicated. I can give it a try this evening (CET) if nobody picked it up for review until then

[miri64]

That would be great! Until then: rebased again. If you want to test: just connect to boards via their UART, configure that UART to be the SLIP UART and test with tests/slip

[bergzand]

Something like this?

2017-11-14 15:14:52,890 - INFO # PKTDUMP: data received:
2017-11-14 15:14:52,895 - INFO # ~~ SNIP  0 - size: 104 byte, type: NETTYPE_UNDEF (0)
2017-11-14 15:14:52,902 - INFO # 00000000  60  03  90  19  00  40  3A  40  AA  AA  00  00  00  00  00  00
2017-11-14 15:14:52,908 - INFO # 00000010  00  00  00  00  00  00  00  01  AA  AA  00  00  00  00  00  00
2017-11-14 15:14:52,915 - INFO # 00000020  00  00  00  00  00  00  00  02  80  00  FB  D7  36  97  00  4B
2017-11-14 15:14:52,922 - INFO # 00000030  5C  FA  0A  5A  00  00  00  00  C4  4B  0D  00  00  00  00  00
2017-11-14 15:14:52,928 - INFO # 00000040  10  11  12  13  14  15  16  17  18  19  1A  1B  1C  1D  1E  1F
2017-11-14 15:14:52,935 - INFO # 00000050  20  21  22  23  24  25  26  27  28  29  2A  2B  2C  2D  2E  2F
2017-11-14 15:14:52,938 - INFO # 00000060  30  31  32  33  34  35  36  37
2017-11-14 15:14:52,942 - INFO # ~~ PKT    -  1 snips, total size: 104 byte

Tested on a nucleo-f446 with a separate USB to serial converter. I'll have a closer look at the code later, but functionally everything looks working.

[miri64]

If you had been sending this data, yes! :-)

[bergzand]

Two results so far:

1. I can crash the tunslip6 application by using txtsnd on the RIOT side. I guess this is due to me sending a random string which doesn't really parse as correct IPv6.
2. When testing a bit more with the gnrc_border_router example (configured with slipdev obviously), I trigger a kernel panic assertion failure at sys/net/gnrc/netif2/gnrc_netif2.c:1104:

2017-11-14 21:48:33,946 - INFO # slipdev: initializing device 0x2000113c on UART 1 with baudrate 115200
2017-11-14 21:48:33,951 - INFO # sys/net/gnrc/netif2/gnrc_netif2.c:1104 => 0x8000e2d
2017-11-14 21:48:33,953 - INFO # *** RIOT kernel panic:
2017-11-14 21:48:33,955 - INFO # FAILED ASSERTION.
2017-11-14 21:48:33,956 - INFO # 
2017-11-14 21:48:33,962 - INFO #        pid | name                 | state    Q | pri | stack  ( used) | base addr  | current     
2017-11-14 21:48:33,970 - INFO #          - | isr_stack            | -        - |   - |    512 (   88) | 0x20000000 | 0x200001c8
2017-11-14 21:48:33,978 - INFO #          1 | idle                 | pending  Q |  15 |    256 (  124) | 0x20000550 | 0x200005d4 
2017-11-14 21:48:33,986 - INFO #          2 | main                 | pending  Q |   7 |   1536 (  252) | 0x20000650 | 0x20000b54 
2017-11-14 21:48:33,994 - INFO #          3 | 6lo                  | bl rx    _ |   3 |   1024 (  280) | 0x2000392c | 0x20003c14 
2017-11-14 21:48:34,002 - INFO #          4 | ipv6                 | bl rx    _ |   4 |   1024 (  264) | 0x20001794 | 0x20001a8c 
2017-11-14 21:48:34,010 - INFO #          5 | slipdev              | running  Q |   2 |   1024 (  544) | 0x20000d3c | 0x200010c4 
2017-11-14 21:48:34,016 - INFO #            | SUM                  |            |     |   5376 ( 1552)
2017-11-14 21:48:34,016 - INFO # 
2017-11-14 21:48:34,016 - INFO # *** halted.
2017-11-14 21:48:34,016 - INFO # 

[miri64]

1. first, yeah tests/slip doesn't send IP packets (which might be a little counter-intuitive for a SLIP test), like all other applications utilizing txtsnd the "payload" (i.e. in SLIP the whole packet) is the string you provide to that command. So I guess that is messing with tunslip6 a little ;-). The test is more thought to connect two boards via a secondary UART and let them communicate.
2. try it with the latest commit. I asserted the maximum packet size to be provided by the device (which in SLIP obviously doesn't make sense). Adapted gnrc_netif2 accordingly.

[bergzand]

Works better now. with example/gnrc_border_router, the device doesn't seem to respond to ping requests or responses on the link local address, but I can see packet arriving correctly on the tunslip side with tcpdump.

[miri64]

Works better now. with example/gnrc_border_router, the device doesn't seem to respond to ping requests or responses on the link local address, but I can see packet arriving correctly on the tunslip side with tcpdump.

Will investigate tomorrow.

[miri64]

@bergzand the problem is in fact, that Linux doesn't answers to RIOT's neighbor solicitations, so this is actually something we need to fix in the NIB (i.e. don't do address resolution for SLIP). Since that is not really about gnrc_netif2: Should I fix this here or in another follow-up PR?

[bergzand]

I remember vaguely that the old slip implementation also had this issue. Maybe @kYc0o can confirm that. With that in mind, my opinion would be to keep this PR to reproduce the functionality of the old slip module (but with netif2 and such) and build a new PR for the other SLIP related issues.

[miri64]

@bergzand See #8041

[bergzand]

ACK on the code, can't find any issues with it.

Now continuing with the functional testing of slipdev. :)

[miri64]

Now continuing with the functional testing of slipdev. :)

Remember that this PR is about integrating slipdev with gnrc_netif2 not about introducing slipdev (that happened already in #7381). If anything in slipdev is buggy I would argue, that that should go into a separate PR. You should focus on testing the interaction of GNRC and slipdev primarily here.

[bergzand]

You should focus on testing the interaction of GNRC and slipdev primarily here.

Yeah, you're right. I'm going to ACK this one as it appears to be working. I'm experiencing some stability issues with either not receiving any frames anymore or easily being able to crash the tunslip6 application. It appears that these issues are not related to this code, but could be in the lower layers (#7381 or the UART driver).

[bergzand]

Ack

[miri64]

Yeah, you're right. I'm going to ACK this one as it appears to be working. I'm experiencing some stability issues with either not receiving any frames anymore or easily being able to crash the tunslip6 application. It appears that these issues are not related to this code, but could be in the lower layers (#7381 or the UART driver).

Thanks (both for review and in advance for the further testing)!

[miri64]

Please do not merge yet! Murdock reports an error, that is not related to #7414! I investigate.

[miri64]

Fixed! @bergzand tell me when I might squash?

[bergzand]

Fixed! @bergzand tell me when I might squash?

Oh, completely forgot about that again, go ahead with the squash -_-

Edit: Oh, didn't forget it this time, just a last minute fix :)

[miri64]

Oh, completely forgot about that again, go ahead with the squash -_-

No problem. The only squashable commit was the one I just added before my request to squash ;-)

[miri64]

All remaining failures are related to #7414. So let's go!