ieee802154_submac: add initial support for common MAC sub layer #14950

jia200x · 2020-09-04T12:37:56Z

Contribution description

This PR adds initial support for the IEEE 802.15.4 SubMAC (#13376). Although this component was introduced in the draft PR #14787 , this PR should contain the "merge-able" version.

In a nutshell, this module provides a uniform layer to send and receive IEEE 802.15.4 MAC frames with retransmissions and CSMA-CA. The SubMAC emulates in software hardware features (CSMA-CA, Retransmissions, etc) missing on a given radio.
This component also keeps track of certain MAC Information Base parameters like channel settings, TX power, etc. An upper layer might (in the near future) request such information directly from the SubMAC layer instead of fetching it from a network device.

Besides that, I added a a netdev_ieee802154_submac as a transition layer. With this, default and gnrc_networking are indirectly working on top on the Radio HAL (#13943 ).

Since the Radio HAL (#14371 ) is asynchronous, #11263 is required in order to send packets with several 6LowPAN fragments.

This module heavily improves the performance of radios without retransmissions / CSMA-CA handling (see e.g #14787 (comment)).

In the future, the full IEEE 802.15.4 MAC layer (to be implemented) can benefit of this, as well as applications interested in the lower part of the MAC layer (CSMA-CA, retransmissions, etc) like lwip.

Testing procedure

So far, only the CC2538 has an implementation of the Radio HAL API, but this might change depending on the status of #14792.

Test that the old netdev abstraction still works as expected with GNRC.
Use the test application in tests/ieee802154_submac to send and receive packets using the SubMAC.
Try gnrc_networking and default using the netdev_ieee802154_submac module. E.g USEMODULE=netdev_ieee802154_submac BOARD=cc2538dk make -C examples/gnrc_networking all flash term. Try to run some tests and compare the performance for radios without CSMA-CA or retransmissions. I suggest running the Release tests.

Here are some test results for samr21-xpro and remote-revb using this PR (both with CSMA-CA and retransmissions):

Ping payload size: 1024 bytes
Ping interval: 170 ms
Count: 1000

2020-09-04 15:19:16,076 # --- fe80::2068:3123:fe42:2e25 PING statistics ---
2020-09-04 15:19:16,083 # 1000 packets transmitted, 1000 packets received, 0% packet loss
2020-09-04 15:19:16,088 # round-trip min/avg/max = 133.318/145.909/161.423 ms

Disabling retransmissions in cc2538 (similar to what we have in master):

020-09-04 15:29:11,891 # --- fe80::2068:3123:fe42:2e25 PING statistics ---
2020-09-04 15:29:11,897 # 1000 packets transmitted, 626 packets received, 37% packet loss
2020-09-04 15:29:11,901 # round-trip min/avg/max = 129.693/140.842/155.274 ms

Issues/PRs references

#13376
#14787
#11263

jia200x · 2020-09-08T12:29:19Z

Just pushed an implementation of NETOPT_STATE for netdev_ieee802154_submac_t.

benpicco

Looking good!
Some comments

drivers/include/net/netdev/ieee802154_submac.h

drivers/netdev_ieee802154_submac/netdev_ieee802154_submac.c

sys/include/net/ieee802154/submac.h

drivers/netdev_ieee802154_submac/netdev_ieee802154_submac.c

sys/net/link_layer/ieee802154/submac.c

tests/ieee802154_submac/Makefile

benpicco · 2020-09-08T16:20:54Z

tests/ieee802154_submac/main.c

+    return count;
+}
+
+int txtsnd(int argc, char **argv)


Isn't there a gnrc_txtsnd already?

yes, but this test has no GNRC dependencies, so we couldn't use that one :(

Maybe it can be moved elsewhere to be put in common?

we could do that. Ideally it would be cool to have an L2 interface here (this is exactly what txtsnd does)

jia200x · 2020-09-09T09:32:11Z

rebased on top of #14984

jia200x · 2020-09-09T11:29:15Z

rebased on top of master

fjmolinas

Nice work @jia200x, a couple of comments on the interface, I haven't followed the discussion that closely so maybe this had already been agreed upon before.

fjmolinas · 2020-09-10T15:48:01Z

sys/include/net/ieee802154/submac.h

+    bool wait_for_ack;                  /**< SubMAC is waiting for an ACK frame */
+    bool tx;                            /**< SubMAC is currently transmitting a frame */


Whats the difference between this tx and the ieee802154_submac_state_t couldn't a TX_STATE be added there? Could a waiting_for_ack state be added there as well?

The thing is, the ieee802154_submac_state_t states are simply "upper layer states" (they don't reflect the internal state machine).

sys/include/net/ieee802154/submac.h

sys/net/link_layer/ieee802154/submac.c

jia200x · 2020-09-22T08:22:58Z

@benpicco I'm having issues integrating the netdev_register here.
The only "short term" dependency of the SubMAC is a transition layer (netdev_ieee802154_submac) but this layer will be removed at some point.
But even for this short term dependency, netdev_register is attached to a specific device descriptor. Is it possible to:

Make netdev_register agnostic to the device descriptor (e.g make it work with netdev_ieee802154_submac)
Make the netdev_register mechanism agnostic to the netdev interface? Netdev is not the only component that might need a permanent EUI.

sys/net/gnrc/netif/init_devs/auto_init_nrf802154.c

benpicco · 2020-09-30T18:32:05Z

Oh well, Murdock is still not happy, but this time we again ran into the build system limitation that modules can't provide features.

Maybe just whitelist a select few boards for the test? That would be the easiest solution.

fabian18 · 2020-10-01T07:54:14Z

Didin't @fabian18 already post a patch for MAC layer security based on this?

I had MAC layer security implemented in my RIOT fork in a branch for an at86rf233 driver, but I must give it a rebase and test if ping6 still works.
It´s not based on the IEEE802154 HAL or the submac. Do you think IEEE82154 security could benefit from submac?
I thought MAC layer security was not concerning the IEEE802154 HAL #13943 (comment).

benpicco · 2020-10-01T08:11:09Z

Yea since now it world not have to be driver specific and could just work ™️ with any 802.15.4 / sub-MAC driver.

AFAIU the payload would be encrypted / decrypted by the sub-MAC and then handed to the driver.

jia200x · 2020-10-01T08:43:34Z

AFAIU the payload would be encrypted / decrypted by the sub-MAC and then handed to the driver.

This could be indeed analyzed. I was always considering that the encryption was going on top of the SubMAC (by the actual MAC component), but there are some radios that do in-place encryption. This means it could be possible to add security to the SubMAC, if we think it makes sense

jia200x · 2020-10-01T09:00:19Z

done! let's see how it goes now

fabian18 · 2020-10-01T09:00:20Z

The submac emulates radio capabilities, right?
So my first idea of how to use submac in the context of IEEE802154 security is to let submac check if the radio has crypto capabilities (ECB/CBC).
If so, then chose e.g. at86rf2xx_ecb at86rf2xx_cbc. Else chose cipher_encrypt_ecb and cipher_encrypt_cbc.

But the call to decrypt/encrypt a frame still happens in gnrc_netif_ieee802154.c in _recv() / _send().

gnrc_netif_ieee802154::_recv()
- -> submac::_recv()
  - -> driver::_recv()

(then I have the raw data what has been received)
(then it parses the IEEE802154 header and sees that it is an encrypted frame)

-> ieee802154_security::ieee802154_ccm_decrypt_frame()
- -> submac::ieee802154_radio_has_ecb && submac::ieee802154_radio_has_cbc
  (chose right function)
-> return pkt

That´s what I think of. Or do you think encrypt()/decrypt() should be called by submac?

jia200x · 2020-10-01T09:04:50Z

The submac emulates radio capabilities, right?

Yes. It emulates a "super" radio with frame retransmissions, CSMA-CA and any other features.

But the call to decrypt/encrypt a frame still happens in gnrc_netif_ieee802154.c in _recv() / _send().

The idea would be to have this as a stack independent component (otherwise it's attached to GNRC). This could be done properly with a MAC layer, or ismply trying to integrate this feature to the SubMAC (if applies).

-> ieee802154_security::ieee802154_ccm_decrypt_frame()
    -> submac::ieee802154_radio_has_ecb && submac::ieee802154_radio_has_cbc
    (chose right function)
-> return pkt

I think this multiplex here could work OK with the SubMAC. Maybe it could be the right place to put it.

That´s what I think of. Or do you think encrypt()/decrypt() should be called by submac?

I'm not entirely sure how the encryption works here. Do you think it could make sense? If so, I don't see any showstoppers for going in this direction.

jia200x · 2020-10-01T09:30:37Z

green :)

benpicco · 2020-10-01T09:31:46Z

That check-labels thing is weird - what does it actually do?

jia200x · 2020-10-01T09:32:15Z

thanks a lot for the review!!

jia200x · 2020-10-01T09:32:44Z

That check-labels thing is weird - what does it actually do?

+1. I always require to remove and re-add a label

miri64 · 2020-10-01T09:40:32Z

This is indeed weird. It should react to open, reopen, label, unlabel, and reviews but appearently it is also resetted to pending on push. :-/

That check-labels thing is weird - what does it actually do?

Checking labels: https://github.com/RIOT-OS/check-labels-action/

fabian18 · 2020-10-03T08:52:57Z

I'm not entirely sure how the encryption works here. Do you think it could make sense? If so, I don't see any showstoppers for going in this direction.

Sorry for the late reply, but I was thinking about it and I am still not sure if it would make sense. Maybe you can checkout #15150

jia200x requested review from miri64, benpicco and PeterKietzmann September 4, 2020 12:37

jia200x requested review from aabadie, cgundogan, fjmolinas, haukepetersen, leandrolanzieri, MichelRottleuthner and smlng as code owners September 4, 2020 12:37

jia200x added State: waiting for other PR State: The PR requires another PR to be merged first Area: network Area: Networking labels Sep 4, 2020

benpicco removed the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 7, 2020

jia200x force-pushed the pr/ieee802154/submac branch 3 times, most recently from b2dc920 to 83c8676 Compare September 7, 2020 15:53

jia200x added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Sep 8, 2020

jia200x force-pushed the pr/ieee802154/submac branch from 83c8676 to caba86e Compare September 8, 2020 09:27

jia200x mentioned this pull request Sep 8, 2020

openthread: add support for radios compatible with SubMAC #14979

Merged

benpicco reviewed Sep 8, 2020

View reviewed changes

jia200x mentioned this pull request Sep 9, 2020

ieee802154_hal: add missing const qualifiers #14984

Merged

jia200x force-pushed the pr/ieee802154/submac branch from 0ad1e04 to 362d259 Compare September 9, 2020 09:32

jia200x mentioned this pull request Sep 9, 2020

cc2538_rf_hal: fix style issues #14985

Merged

jia200x force-pushed the pr/ieee802154/submac branch from 60e63ac to fbbb882 Compare September 9, 2020 11:29

jia200x mentioned this pull request Sep 9, 2020

Add other radios support for OpenThread pkg #10045

Closed

9 tasks

fjmolinas reviewed Sep 10, 2020

View reviewed changes

benpicco reviewed Sep 30, 2020

View reviewed changes

sys/net/gnrc/netif/init_devs/auto_init_nrf802154.c Outdated Show resolved Hide resolved

jia200x force-pushed the pr/ieee802154/submac branch from 66e7e18 to e93c362 Compare September 30, 2020 17:06

jia200x added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 30, 2020

jia200x added 4 commits October 1, 2020 10:55

tests/ieee802154_submac: add test application

d0a0190

auto_init/cc2538_rf: add support for SubMAC

1de5b34

net/ieee802154: add config for default CCA threshold

7cf6943

auto_init/nrf52840: add support for SubMAC

9ebc3f8

jia200x force-pushed the pr/ieee802154/submac branch from e93c362 to 9ebc3f8 Compare October 1, 2020 08:56

jia200x added Area: network Area: Networking and removed Area: network Area: Networking labels Oct 1, 2020

benpicco added State: waiting for CI update State: The PR requires an Update to CI to be performed first and removed State: waiting for CI update State: The PR requires an Update to CI to be performed first labels Oct 1, 2020

benpicco merged commit 6cb8da7 into RIOT-OS:master Oct 1, 2020

jia200x mentioned this pull request Oct 1, 2020

[RFC] The IEEE802.15.4 SubMAC #13376

Closed

fabian18 mentioned this pull request Oct 3, 2020

Initial implementation of IEEE 802.15.4 security #15150

Merged

benpicco mentioned this pull request Oct 8, 2020

nrf802154: Add ACK handling capabilities #11150

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ieee802154_submac: add initial support for common MAC sub layer #14950

ieee802154_submac: add initial support for common MAC sub layer #14950

jia200x commented Sep 4, 2020 •

edited

Loading

jia200x commented Sep 8, 2020

benpicco left a comment

benpicco Sep 8, 2020

jia200x Sep 9, 2020

fjmolinas Sep 9, 2020

jia200x Sep 9, 2020

jia200x commented Sep 9, 2020

jia200x commented Sep 9, 2020

fjmolinas left a comment

fjmolinas Sep 10, 2020

jia200x Sep 10, 2020

jia200x commented Sep 22, 2020 •

edited

Loading

benpicco commented Sep 30, 2020 •

edited

Loading

fabian18 commented Oct 1, 2020

benpicco commented Oct 1, 2020

jia200x commented Oct 1, 2020

jia200x commented Oct 1, 2020

fabian18 commented Oct 1, 2020

jia200x commented Oct 1, 2020

jia200x commented Oct 1, 2020

benpicco commented Oct 1, 2020

jia200x commented Oct 1, 2020

jia200x commented Oct 1, 2020

miri64 commented Oct 1, 2020

fabian18 commented Oct 3, 2020

		bool wait_for_ack; /*< SubMAC is waiting for an ACK frame /
		bool tx; /*< SubMAC is currently transmitting a frame /

ieee802154_submac: add initial support for common MAC sub layer #14950

ieee802154_submac: add initial support for common MAC sub layer #14950

Conversation

jia200x commented Sep 4, 2020 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

jia200x commented Sep 8, 2020

benpicco left a comment

Choose a reason for hiding this comment

benpicco Sep 8, 2020

Choose a reason for hiding this comment

jia200x Sep 9, 2020

Choose a reason for hiding this comment

fjmolinas Sep 9, 2020

Choose a reason for hiding this comment

jia200x Sep 9, 2020

Choose a reason for hiding this comment

jia200x commented Sep 9, 2020

jia200x commented Sep 9, 2020

fjmolinas left a comment

Choose a reason for hiding this comment

fjmolinas Sep 10, 2020

Choose a reason for hiding this comment

jia200x Sep 10, 2020

Choose a reason for hiding this comment

jia200x commented Sep 22, 2020 • edited Loading

benpicco commented Sep 30, 2020 • edited Loading

fabian18 commented Oct 1, 2020

benpicco commented Oct 1, 2020

jia200x commented Oct 1, 2020

jia200x commented Oct 1, 2020

fabian18 commented Oct 1, 2020

jia200x commented Oct 1, 2020

jia200x commented Oct 1, 2020

benpicco commented Oct 1, 2020

jia200x commented Oct 1, 2020

jia200x commented Oct 1, 2020

miri64 commented Oct 1, 2020

fabian18 commented Oct 3, 2020

jia200x commented Sep 4, 2020 •

edited

Loading

jia200x commented Sep 22, 2020 •

edited

Loading

benpicco commented Sep 30, 2020 •

edited

Loading