pkg/openwsn: add sock_udp [upstream implementation]

[fjmolinas]

Contribution description

This PR adds sock_udp for OpenWSN, this allows easily running modules like gcoap or suit on top of OpenWSN. In a follow up I'll add an example show casing this (an easy test can be done in this poc branch https://github.com/fjmolinas/RIOT/tree/poc/openwsn_sock_examples).

The pkg version has been bumped and the test application updated.

Testing procedure

Run tests/pkg_openwsn according to the readme.

• root

11:25:26 SUCCESS Setting mote 2685 as root
11:25:26 ERROR 2685 [IEEE802154E] wrong state 1 in startSlot, at slotOffset 1
11:25:26 INFO registering DAGroot a6-51-fa-b5-22-0d-a6-85
11:25:39 VERBOSE New node: e6d3:6fec:df36:bbe. Creating new OSCORE context in /home/francisco/workspace/RIOT/bin/oscore_context_e6d36fecdf360bbe.json.
11:25:39 VERBOSE received JRC join request
11:25:39 ERROR 2685 [OPENSERIAL] wrong CRC in input Buffer
11:25:58 VERBOSE New node: e62e:ce92:ca67:b62b. Creating new OSCORE context in /home/francisco/workspace/RIOT/bin/oscore_context_e62ece92ca67b62b.json.
11:25:58 VERBOSE received JRC join request
11:26:38 ERROR Type-error in conversion of o6

11:26:38 VERBOSE received JRC join request
11:27:28 INFO received RPL DAO from bbbb:0:0:0:e62e:ce92:ca67:b62b
        - parents:
           bbbb:0:0:0:a651:fab5:220d:a685
        - children:
11:28:06 ERROR 2685 [IEEE802154E] wdDataDuration overflows while at state 19 in slotOffset 21
11:28:07 INFO received RPL DAO from bbbb:0:0:0:e62e:ce92:ca67:b62b
        - parents:
           bbbb:0:0:0:a651:fab5:220d:a685
        - children:
11:28:24 INFO received RPL DAO from bbbb:0:0:0:e6d3:6fec:df36:bbe
        - parents:
           bbbb:0:0:0:a651:fab5:220d:a685
        - children:
11:29:15 INFO received RPL DAO from bbbb:0:0:0:e6d3:6fec:df36:bbe
        - parents:
           bbbb:0:0:0:a651:fab5:220d:a685
        - children:
11:29:20 INFO received RPL DAO from bbbb:0:0:0:e62e:ce92:ca67:b62b
        - parents:
           bbbb:0:0:0:a651:fab5:220d:a685
        - children:
11:29:26 ERROR 2685 [IEEE802154E] wdDataDuration overflows while at state 19 in slotOffset 0
11:29:30 INFO received RPL DAO from bbbb:0:0:0:e6d3:6fec:df36:bbe
        - parents:
           bbbb:0:0:0:a651:fab5:220d:a685
        - children:

• leafs

ifconfig
Iface  2        HWaddr: 0B:BE  NID: FE:CA

                Long HWaddr: E6:D3:6F:EC:DF:36:0B:BE
                inet6 addr: bbbb::e6d3:6fec:df36:bbe

                IEEE802154E sync: 1
                6TiSCH joined: 1

                RPL rank: 2816
                RPL parent: A6:51:FA:B5:22:0D:A6:85
                RPL children:
                RPL DODAG ID: bbbb::a651:fab5:220d:a685
> udp send bbbb::e62e:ce92:ca67:b62b 7 hello
udp send bbbb::e62e:ce92:ca67:b62b 7 hello
Send 5 byte over UDP to [bbbb::e62e:ce92:ca67:b62b]:7
Send Success

> ifconfig
ifconfig
Iface  2        HWaddr: 36:2B  NID: FE:CA

                Long HWaddr: E6:2E:CE:92:CA:67:B6:2B
                inet6 addr: bbbb::e62e:ce92:ca67:b62b

                IEEE802154E sync: 1
                6TiSCH joined: 1

                RPL rank: 2816
                RPL parent: A6:51:FA:B5:22:0D:A6:85
                RPL children:
                RPL DODAG ID: bbbb::a651:fab5:220d:a685
> Received 5 bytes on port 7
00000000  68  65  6C  6C  6F                                              hello

Issues/PRs references

[aabadie]

Looks good, except 3 minor comments that can be squashed right away.

[fjmolinas]

@aabadie I addressed your changes. I also fixed some more dependencies I found as well as bumped the version of the pkg (new merged changes since.)

[miri64]

Is there a way to provide a lwip_sock_udp/gnrc_sock_udp-style test application as well (maybe as a follow-up) to check if the API implementation is sane?

[fjmolinas]

provide a lwip_sock_udp/gnrc_sock_udp-style test application as well (maybe as a follow-up) to check if the API implementation is sane?

I'll take a look at those applications.

[aabadie]

This seems unrelated

[fjmolinas]

It popped out because netif netif_addr_to_str urtility uses fmt and the dependency is not explicitly made, should I split this commit?

[miri64]

See #14758 (review) for a similar discussion.

[fjmolinas]

Thanks for the pointer, took a quick look your suggestion makes sense.

[miri64]

Where can I find the API implementation? I grep'ed in both repositories, openwsn and RIOT, for the following terms and found nothing useful with regards to openwsn_sock_udp.

• sock_udp_send
• openwsn_sock_udp
• OPENWSN_SOCK_UDP

[miri64]

Ah, found it. Searched on master, not develop.

[fjmolinas]

Ah, found it. Searched on master, not develop.

Yes openwsn master is very old, develop is the actual master.

[fjmolinas]

Where can I find the API implementation? I grep'ed in both repositories, openwsn and RIOT, for the following terms and found nothing useful with regards to openwsn_sock_udp.

* `sock_udp_send`

* `openwsn_sock_udp`

* `OPENWSN_SOCK_UDP`

Note that here I opted by adding the missing functionalities to the sock implementation as a patch, I could have also copied the implementation here.

[fjmolinas]

Where can I find the API implementation? I grep'ed in both repositories, openwsn and RIOT, for the following terms and found nothing useful with regards to openwsn_sock_udp.

* `sock_udp_send`

* `openwsn_sock_udp`

* `OPENWSN_SOCK_UDP`

Thanks for taking a close look BTW :)

[miri64]

Note that here I opted by adding the missing functionalities to the sock implementation as a patch, I could have also copied the implementation here.

I actually was thinking about this. Having a sock header here and there, I see some potential for future conflict. What if there is an API change to sock on either side? That would require quite some effort with regards to openwsn then.

Also there are already some divergences in their implementation and our API:

• sock_udp_get_local() is supposed to return -EADDRNOTAVAIL when sock has no local endpoint, not -EADDRINUSE.
• sock_udp_recv() does not check the local of the sock, and thus never returns -EADDRNOTAVAIL. Likewise the remote is not checked with the actual remote in the packet (should return -EPROTO if wrong).
• sock_udp_recv() truncates the data to max_len instead of returning -ENOBUFS if the buffer is too small (also not sure the memset() is expected). For a truncated return on receive, there is sock_udp_recv_buf() (which is not implemented).
• sock_udp_send() returns -EINVAL when the precondition is wrong instead of running into an assert or something similar, while an actual conditions for -EINVAL (if remote->netif is wrong).
• sock_udp_send() does not check if the range of ephemeral ports might be depleted (returning -EADDRINUSE in that case)[https://doc.riot-os.org/group__net__sock__udp.html#ga89562920ad89fe0e098fc989e3b064ec].
• sock_udp_send() assumes there is a remote endpoint to the sock when no remote parameter is given. The function is supposed to check that and to return -ENOTCON in that case.

Not sure there are more issues (unittests might reveal that), but those are the ones I found so far.

[miri64]

What if there is an API change to sock on either side?

#14704 is a good example for an ongoing change on our side.

[fjmolinas]

Note that here I opted by adding the missing functionalities to the sock implementation as a patch, I could have also copied the implementation here.

I actually was thinking about this. Having a sock header here and there, I see some potential for future conflict. What if there is an API change to sock on either side? That would require quite some effort with regards to openwsn then.

Also there are already some divergences in their implementation and our API:

* [`sock_udp_get_local()` is supposed to return `-EADDRNOTAVAIL`](https://doc.riot-os.org/group__net__sock__udp.html#ga1a0e4c3a85b42cfcdc2f016dd2c96c83) when sock has no local endpoint, [not `-EADDRINUSE`](https://github.com/openwsn-berkeley/openwsn-fw/blob/e2958a031ad50a6f3eeca3a98ad1ae85aa773a48/openstack/04-TRAN/sock.c#L181-L184).

* [`sock_udp_recv()` does not check the local of the `sock`](https://github.com/openwsn-berkeley/openwsn-fw/blob/1b20367fd2fff53b4ae00e0511a5582645c879cb/openstack/04-TRAN/sock.c#L201-L228), and thus never [returns `-EADDRNOTAVAIL`](https://doc.riot-os.org/group__net__sock__udp.html#gad1030df9b925878c2bf678487a9c88f7). Likewise the remote is not checked with the actual remote in the packet (should return `-EPROTO` if wrong).

* [`sock_udp_recv()` truncates the data to `max_len` instead of returning `-ENOBUFS` if the buffer is too small](https://github.com/openwsn-berkeley/openwsn-fw/blob/1b20367fd2fff53b4ae00e0511a5582645c879cb/openstack/04-TRAN/sock.c#L211-L213) (also not sure [the `memset()`](https://github.com/openwsn-berkeley/openwsn-fw/blob/1b20367fd2fff53b4ae00e0511a5582645c879cb/openstack/04-TRAN/sock.c#L222) is expected). For a truncated return on receive, there is [`sock_udp_recv_buf()`](https://doc.riot-os.org/group__net__sock__udp.html#gaabbd7a35b628f79eebef42886dc796e1) (which is not implemented).

* [`sock_udp_send()` returns `-EINVAL`](https://github.com/openwsn-berkeley/openwsn-fw/blob/1b20367fd2fff53b4ae00e0511a5582645c879cb/openstack/04-TRAN/sock.c#L87-L93) when the [precondition](https://doc.riot-os.org/group__net__sock__udp.html#ga89562920ad89fe0e098fc989e3b064ec) is wrong instead of running into an assert or something similar, while an actual conditions for `-EINVAL` (if `remote->netif` is  wrong).

* [`sock_udp_send()` does not check if the range of ephemeral ports might be depleted](https://github.com/openwsn-berkeley/openwsn-fw/blob/1b20367fd2fff53b4ae00e0511a5582645c879cb/openstack/04-TRAN/sock.c#L120) (returning `-EADDRINUSE` in that case)[https://doc.riot-os.org/group__net__sock__udp.html#ga89562920ad89fe0e098fc989e3b064ec].

* [`sock_udp_send()` assumes there is a remote endpoint to the sock](https://github.com/openwsn-berkeley/openwsn-fw/blob/1b20367fd2fff53b4ae00e0511a5582645c879cb/openstack/04-TRAN/sock.c#L124) when no `remote` parameter is given. The function is supposed to check that and [to return `-ENOTCON` in that case](https://github.com/openwsn-berkeley/openwsn-fw/blob/1b20367fd2fff53b4ae00e0511a5582645c879cb/openstack/04-TRAN/sock.c#L124).

Not sure there are more issues (unittests might reveal that), but those are the ones I found so far.

They implemented there sock api against ours, its exactly the same header. But in the future they could diverge that is true.

[fjmolinas]

I actually was thinking about this. Having a sock header here and there, I see some potential for future conflict. What if there is an API change to sock on either side? That would require quite some effort with regards to openwsn then.

So you would prefer a verbatim copy and "enhancement" of their implementation?

[miri64]

I actually was thinking about this. Having a sock header here and there, I see some potential for future conflict. What if there is an API change to sock on either side? That would require quite some effort with regards to openwsn then.

So you would prefer a verbatim copy and "enhancement" of their implementation?

I would prefer our own (to our definition "correct") implementation in pkg/openwsn/contrib or something like that, correct.

[miri64]

They implemented there sock api against ours, its exactly the same header. But in the future they could diverge that is true.

Apparently they took some liberties already, see my list of divergences.

[miri64]

BTW the degrading maintainability of the large number of patches was one of the reasons, why the original OpenWSN port was removed. We should be careful not to repeat the same mistake.

[fjmolinas]

BTW the degrading maintainability of the large number of patches was one of the reasons, why the original OpenWSN port was removed. We should be careful not to repeat the same mistake.

I saw it as a trade-off:

• sock implementation in RIOT: easier to maintain, changes and implementation are visible.
• sock implementation in OpenWSN patched: will force us to keep both api's inline. They are using our api in hopes of making higher layers modules work both on OpenWSN and RIOT.

I'll go with the first one then, I'll add some tests as well. I'm currently on another subject, so it will take me a couple of days to go back to this, but thanks a lot for the feedback @miri64!

[maribu]

This will need a rebase on master due to #14703 being merged now.

I'll point out inline how this PR can be changed to adapt

[maribu]

See inline how to adapt to the API extension of the SOCK API

[fjmolinas]

Closing since #15536 is in.