native: provide socket-based ZEP device by miri64 · Pull Request #6121 · RIOT-OS/RIOT

miri64 · 2016-11-15T15:41:47Z

This is aiming to replace GNRC ZEP and allow platform independent IEEE 802.15.4 traffic on native (as opposed to #5582).

I tested it already with zep_mesh, a very simple network simulator I specifically wrote for ZEP (and RIOT, though I tried to keep it generalized):

This is more or less done, but there are still some minor stuff I might add/change on a later point:

allow for multiple ZEP interfaces (see netdev2_tap: make driver capable to handle multiple TAP interfaces #2195, I'm planning to adopt this PR in the near future)
filter channels, wrong FCS, etc.

Depends on ~~#6112~~ (merged).

miri64 · 2016-11-15T15:55:22Z

(Just in case you are wondering why the EUI-64 is gradually growing with the nodes: it's generated from the sockets sockaddr. I took great care that the local port part is in the region were the last two bytes of the EUI-64 are and also used those two bytes for the short address).

miri64 · 2016-11-15T16:01:08Z

I implemented this primarily to test my new NDP implementation I'm going to finally start in a few days.

LudwigKnuepfer · 2016-11-15T17:54:31Z

cpu/native/Makefile


 ifneq (,$(filter netdev2_tap,$(USEMODULE)))
-	DIRS += netdev2_tap
+  DIRS += netdev2_tap


there seem to spaces instead of tabs now ..

Yes, this is the quasi convention for a while now. Since I touched this file anyway I allowed myself to change that ;-).

LudwigKnuepfer · 2016-11-15T17:55:40Z

cpu/native/include/socket_zep.h

+ */
+void socket_zep_setup(socket_zep_t *dev, const socket_zep_params_t *params);
+
+void socket_zep_cleanup(socket_zep_t *dev);


Doxygen missing?

LudwigKnuepfer · 2016-11-15T17:59:47Z

cpu/native/socket_zep/socket_zep.c

+{
+    struct timeval tv;
+
+    gettimeofday(&tv, NULL);


I have the feeling gettimeofday should be added to syscalls...

LudwigKnuepfer · 2016-11-15T18:02:09Z

cpu/native/socket_zep/socket_zep.c

+            return (memcmp(dst_addr, ieee802154_addr_bcast, dst_len) != 0) &&
+                   (memcmp(dst_addr, dev->netdev.short_addr, dst_len) != 0);
+        default:
+            return false;    /* better save than sorry ;-) */


s/save/safe/ I guess

LudwigKnuepfer · 2016-11-15T18:05:02Z

cpu/native/socket_zep/socket_zep.c

+            if (max_len != sizeof(uint16_t)) {
+                return -EOVERFLOW;
+            }
+            *v = SOCKET_ZEP_FRAME_PAYLOAD_LEN;


you might want to check for v != NULL before...

I think we don't do that anywhere else, but will do.

Done in https://github.com/RIOT-OS/RIOT/pull/6121/files#diff-01bcdc6a0bf2aa346059e749a7178abaR309 btw ;-)

miri64 · 2016-11-16T16:45:08Z

Addressed comments

miri64 · 2016-11-16T17:05:14Z

I found the following patch useful for testing BTW ;-)

diff --git a/boards/native/Makefile.dep b/boards/native/Makefile.dep
index 88151e9..0991015 100644
--- a/boards/native/Makefile.dep
+++ b/boards/native/Makefile.dep
@@ -1,5 +1,5 @@
 ifneq (,$(filter netdev_default gnrc_netdev_default,$(USEMODULE)))
-  USEMODULE += netdev2_tap
+  USEMODULE += socket_zep
 endif

 ifneq (,$(filter socket_zep,$(USEMODULE)))
diff --git a/boards/native/Makefile.include b/boards/native/Makefile.include
index 01fc119..232dc26 100644
--- a/boards/native/Makefile.include
+++ b/boards/native/Makefile.include
@@ -88,11 +88,11 @@ endif
 export LINKFLAGS += -ffunction-sections

 # set the tap interface for term/valgrind
-ifneq (,$(filter netdev_default gnrc_netdev_default,$(USEMODULE)))
-       export PORT ?= tap0
-else
-       export PORT =
-endif
+# ifneq (,$(filter netdev_default gnrc_netdev_default,$(USEMODULE)))
+#      export PORT ?= tap0
+# else
+#      export PORT =
+# endif

 export TERMFLAGS := $(PORT) $(TERMFLAGS)

(will provide a proper test application ASAP)

kaspar030 · 2016-11-18T11:10:46Z

Very nice! I won't find time to test, though...

kaspar030 · 2016-11-18T11:15:25Z

cpu/native/socket_zep/socket_zep.c

+
+    if (getpeername(dev->sock_fd, (struct sockaddr *)&ss_array, &ss_len) < 0) {
+        perror("ZEP: Unable to retrieve remote address");
+        return;


Here and below: the socket is connected at this point, should it be closed?

I think that's a remnant from when I played around with unbound/unconnected sockets. Will do.

miri64 · 2016-11-22T13:54:45Z

Provided tests.

When working on RIOT-OS#6121 I noticed that the documentation of the `netdev2` driver part is not really precise. This fixes that.

cgundogan · 2016-12-15T08:57:42Z

this looks like a neat feature. Will try to find time on the weekend to give it a shot

miri64 · 2016-12-27T10:22:43Z

Rebased to current master, since #6112 got merged => Removed Waiting for other PR label. Also removed WIP label. Those missing features can well be done in a follow-up.

PeterKietzmann · 2017-01-14T13:42:06Z

@miri64 can you give a hint about how to use this, e.g. with gnrc_networking? I applied your patch and indeed, a gnrc_socket_zep thread appeared but unfortunately the gnrc_netdev2_tap interface is still predominant. Or am I mistaken in using gnrc_networking for testing this PR?

> ps  
ps
	pid | name                 | state    Q | pri | stack ( used) | base       | current    
	  - | isr_stack            | -        - |   - |  8192 (   -1) |  0x808a480 |  0x808a480
	  1 | idle                 | pending  Q |  15 |  8192 (  420) |  0x8088200 |  0x808a074 
	  2 | main                 | running  Q |   7 | 12288 ( 3124) |  0x8085200 |  0x8088074 
	  3 | pktdump              | bl rx    _ |   6 | 12288 (  976) |  0x8092500 |  0x8095374 
	  4 | 6lo                  | bl rx    _ |   3 |  8192 (  976) |  0x80a0ac0 |  0x80a2934 
	  5 | ipv6                 | bl rx    _ |   4 |  8192 ( 1424) |  0x809be40 |  0x809dcb4 
	  6 | udp                  | bl rx    _ |   5 |  8192 (  976) |  0x8095540 |  0x80973b4 
	  7 | gnrc_netdev2_tap     | bl rx    _ |   2 |  8192 ( 2356) |  0x8097c40 |  0x8099ab4 
	  8 | gnrc_socket_zep      | bl rx    _ |   2 |  8192 (  992) |  0x8099dc0 |  0x809bc34 
	  9 | RPL                  | bl rx    _ |   5 |  8192 ( 1080) |  0x809e7c0 |  0x80a0634 
	    | SUM                  |            |     | 90112 (12324)
> ifconfig
ifconfig
Iface  7   HWaddr: 3e:90:48:a8:59:e6 
           
           MTU:1500  HL:64  
           Source address length: 6
           Link type: wired
           inet6 addr: ff02::1/128  scope: local [multicast]
           inet6 addr: fe80::3c90:48ff:fea8:59e6/64  scope: local
           inet6 addr: ff02::1:ffa8:59e6/128  scope: local [multicast]
           inet6 addr: ff02::1a/128  scope: local [multicast]
           
           Statistics for Layer 2
            RX packets 0  bytes 0
            TX packets 1 (Multicast: 1)  bytes 64
            TX succeeded 1 errors 0
           Statistics for IPv6
            RX packets 0  bytes 0
            TX packets 1 (Multicast: 1)  bytes 50
            TX succeeded 1 errors 0

It would be great to have this for the release!

miri64 · 2017-01-14T14:52:17Z

Have you cleaned between builds?

PeterKietzmann · 2017-01-14T16:37:09Z

Sorry, my bad. I didn't apply your patch but just added USEMODULE += socket_zep to the Makefile.dep and forget to take out netdev2_tap. However, it seems there are some parameters needed to run a networking application with sock_zep, right? Could you provide a hint about how to run it? Would make testing easier for others.

smlng · 2018-01-12T12:58:56Z

murdock isn't all that happy, can you have look?

miri64 · 2018-01-16T14:44:16Z

@smlng Murdock should now be happy.

smlng · 2018-01-16T14:47:24Z

Nope

miri64 · 2018-01-16T14:48:02Z

That's a new one :D

miri64 · 2018-01-16T14:56:54Z

Rebased (to include gnrc_netif to merge-base) and fixed.

miri64 · 2018-01-17T16:33:25Z

@smlng now finally both Travis and Murdock are happy (except for squashing notice). Do you think we can include this into the release? Would be awesome for testing and debugging to have IEEE 802.15.4 for native.

BTW, @aabadie @kaspar030 @smlng Murdock doesn't seem to have flake8 to be installed.

smlng · 2018-01-17T17:26:22Z

BTW, @aabadie @kaspar030 @smlng Murdock doesn't seem to have flake8 to be installed.

yep, workers need an update but not before release testing is done.

Otherwise, yes this should go into the release.

smlng

some minor comments on documentation (non blocking). But I also tried the test app which failed with an assertion as follows:

Socket ZEP device driver test
Initializing socket ZEP with (local: [::]:17754, remote: [::1]:17754)
(Hwaddrs: 455b, 005a45501c1e455b)
Send zero-length packet
Send 'Hello\0World\0'
Waiting for an incoming message (use `make test`)
tests/socket_zep/main.c:126 => 0x4d5c
*** RIOT kernel panic:
FAILED ASSERTION.

*** halted.


native: exiting

smlng · 2018-01-17T17:28:18Z

cpu/native/include/socket_zep_params.h

+ * @{
+ *
+ * @file
+ * @brief       Configuration parameters for the @ref netdev_socket_zep driver


indention off? One tab too much?

smlng · 2018-01-17T17:30:02Z

cpu/native/include/socket_zep.h

+/**
+ * @defgroup    netdev_socket_zep  ZEP-based IEEE 802.15.4 device for native
+ * @ingroup     netdev
+ * @brief


mhm, a bit sad this is empty 😢 Maybe put the (rather long) defgroup title here, and shorten the above?

smlng · 2018-01-17T17:32:14Z

sys/auto_init/netif/auto_init_socket_zep.c

+#include "debug.h"
+
+/**
+ * @brief   Define stack parameters for the MAC layer thread


/brief/name/ for group

Isn't generated anyways and the other devices use the same notation (also the brace doesn't seem to be closed... I just remove the brace, because this just refers to the stack, not the prio.

smlng · 2018-01-17T17:32:42Z

sys/include/net/zep.h

+/**
+ * @defgroup    net_zep  ZEP header definitions
+ * @ingroup     net
+ * @brief


group description empty again 😢

smlng · 2018-01-17T17:38:11Z

while the error above was with make term the following comes from make test:

Socket ZEP device driver test
Initializing socket ZEP with (local: [::]:12345, remote: [::1]:17754)
(Hwaddrs: 3038, 005a45501c1e3038)
Send zero-length packet
Send 'Hello\0World\0'
Waiting for an incoming message (use `make test`)
Received invalid packet
Timeout in expect script at "child.expect(r"RSSI: \d+, LQI: \d+, Data:")" (tests/socket_zep/tests/01-run.py:50)
  File "/RIOT/dist/tools/testrunner/testrunner.py", line 55, in run
    testfunc(child)
  File "./tests/01-run.py", line 50, in testfunc
    child.expect(r"RSSI: \d+, LQI: \d+, Data:")
  File "/usr/local/lib/python3.6/site-packages/pexpect/spawnbase.py", line 321, in expect
    timeout, searchwindowsize, async)
  File "/usr/local/lib/python3.6/site-packages/pexpect/spawnbase.py", line 345, in expect_list
    return exp.expect_loop(timeout)
  File "/usr/local/lib/python3.6/site-packages/pexpect/expect.py", line 107, in expect_loop
    return self.timeout(e)
  File "/usr/local/lib/python3.6/site-packages/pexpect/expect.py", line 70, in timeout
    raise TIMEOUT(msg)

Run tests failed
make: *** [test] Error 1

miri64 · 2018-01-17T17:52:17Z

Did you used the most current version? I literally just fixed that in the latest batch and at least on my machine it was working with that.

smlng · 2018-01-17T19:01:02Z

Did you used the most current version

okay, I'll quickly test again

smlng · 2018-01-17T19:52:24Z

I'll verified and I'm on the latest commit, but still get the errors as reported above. Though, I've to admit I test on macOS ...

smlng · 2018-01-17T20:27:48Z

retested on linux: works! So issues reported are likely macOS specific, still would be great to have it compatible in the future, if possible.

smlng · 2018-01-18T08:01:07Z

please squash in a timely manner, the feature freeze is approaching slowly 😉

smlng · 2018-01-18T09:03:03Z

ACK and GO!

miri64 · 2018-01-18T09:23:13Z

Awesome! 🎉

miri64 added Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Nov 15, 2016

miri64 added this to the Release 2017.01 milestone Nov 15, 2016

miri64 assigned OlegHahm, LudwigKnuepfer and kaspar030 Nov 15, 2016

miri64 added Platform: native Platform: This PR/issue effects the native platform Area: network Area: Networking labels Nov 15, 2016

miri64 mentioned this pull request Nov 15, 2016

netdev2_tap: return actual byte number on read request #6122

Merged

LudwigKnuepfer reviewed Nov 15, 2016

View reviewed changes

kaspar030 reviewed Nov 18, 2016

View reviewed changes

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 22, 2016

netdev2: make doc for driver more precise

32c19d6

When working on RIOT-OS#6121 I noticed that the documentation of the `netdev2` driver part is not really precise. This fixes that.

This was referenced Nov 22, 2016

netdev2: make doc for driver more precise #6154

Merged

ucrc16: provide lightweight CRC16 implementation #6112

Merged

miri64 force-pushed the native/feat/zep_socket branch from ec2f8fb to 3af0d87 Compare December 27, 2016 10:20

miri64 removed State: waiting for other PR State: The PR requires another PR to be merged first State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Dec 27, 2016

miri64 mentioned this pull request Jan 12, 2017

native: prettify argument parsing #6337

Merged

smlng modified the milestones: Release 2017.10, Release 2018.01 Nov 16, 2017

PeterKietzmann mentioned this pull request Nov 27, 2017

netdev2_tap: make driver capable to handle multiple TAP interfaces #2195

Closed

smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 12, 2018

miri64 force-pushed the native/feat/zep_socket branch from ca1a23f to 4704c81 Compare January 16, 2018 14:56

smlng suggested changes Jan 17, 2018

View reviewed changes

smlng approved these changes Jan 18, 2018

View reviewed changes

miri64 and others added 3 commits January 18, 2018 09:15

native: provide socket-based ZEP device

81ef152

tests: add tests for socket_zep

28aa90d

examples: adapt border router for ZEP

585e9a2

miri64 force-pushed the native/feat/zep_socket branch from db4ac82 to 585e9a2 Compare January 18, 2018 08:15

smlng merged commit 5c0b058 into RIOT-OS:master Jan 18, 2018

miri64 deleted the native/feat/zep_socket branch January 18, 2018 09:22

bergzand mentioned this pull request Feb 15, 2018

net: Introduce iolists #8351

Merged

22 tasks

benpicco mentioned this pull request Jun 7, 2020

RFC 6775 multi-hop NDP test in native board #14156

Closed

Conversation

miri64 commented Nov 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Nov 15, 2016

Uh oh!

miri64 commented Nov 15, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Nov 16, 2016

Uh oh!

miri64 commented Nov 16, 2016

Uh oh!

kaspar030 commented Nov 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Nov 22, 2016

Uh oh!

cgundogan commented Dec 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Dec 27, 2016

Uh oh!

PeterKietzmann commented Jan 14, 2017

Uh oh!

miri64 commented Jan 14, 2017

Uh oh!

PeterKietzmann commented Jan 14, 2017 • edited by OlegHahm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smlng commented Jan 12, 2018

Uh oh!

miri64 commented Jan 16, 2018

Uh oh!

smlng commented Jan 16, 2018

Uh oh!

miri64 commented Jan 16, 2018

Uh oh!

miri64 commented Jan 16, 2018

Uh oh!

miri64 commented Jan 17, 2018

Uh oh!

smlng commented Jan 17, 2018

Uh oh!

smlng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Jan 17, 2018

Uh oh!

miri64 commented Jan 17, 2018

miri64 commented Nov 15, 2016 •

edited

Loading

cgundogan commented Dec 15, 2016 •

edited

Loading

PeterKietzmann commented Jan 14, 2017 •

edited by OlegHahm

Loading