ieee802154: Add support for default flag settings #9592

bergzand · 2018-07-18T08:27:18Z

Contribution description

This PR adds a number of configuration flags to ieee802154.h similar to the current default PANID and CHANNEL define. This allows for configuring the AUTOACK, ACK_REQ and CSMA flag at compile time.

Except for Openthread, the current set of supported network stacks are updated to set these flags on interface initialization.

Issues/PRs references

None

Testing

samr21-xpro

Remove L81-L83 from the at86rf2xx driver to prevent it from setting its own default. Also keep in mind that there is the issue piggybacked with #9581 where modifying the AUTOACK flag influences the ACK_REQ flag.

Other radios

The other radios have different defaults enabled in their reset functions which should be removed to effectively test this PR.

Known issues

For some reason the AUTOACK flag is always enabled on the at86rf2xx independent of the settings provided in this PR, I haven't tested yet if the other radios are also affected, but it could be a default setting from the radio hardware itself.
I don't really like the code duplication between the stack initialization functions but at the moment I don't know a nice place to put a common settings init function
@jia200x Does OpenThread require any specific flags from the network interface to function, does it make sense to honor the settings provided at compile time as much as possible except for any flags strictly required?

bergzand · 2018-07-18T10:15:04Z

As a follow up I'd like to extend this to also set the channel and PAN ID this way.

jnohlgard

I agree with the general idea. Only checked the gnrc code change.
Maybe there should be some ifdef for removing the dead code if we are building an application without 802.15.4 (e.g. Ethernet only), to reduce the rom usage

jnohlgard · 2018-07-18T12:22:54Z

sys/net/gnrc/netif/gnrc_netif.c

@@ -1226,6 +1226,29 @@ static void _init_from_device(gnrc_netif_t *netif)
    _update_l2addr_from_dev(netif);
 }

+static void _configure_netdev_802154(netdev_t *dev)
+{
+    netopt_enable_t enable = NETOPT_ENABLE;


Make this static const to reduce stack usage

bergzand · 2018-07-18T12:29:54Z

sys/net/gnrc/netif/gnrc_netif.c

+    switch(dev_type) {
+        case NETDEV_TYPE_IEEE802154:
+            _configure_netdev_802154(dev);
+            break;


Maybe there should be some ifdef for removing the dead code if we are building an application without 802.15.4 (e.g. Ethernet only), to reduce the rom usage

I can put a #ifdef MODULE_NETDEV_IEEE802154 around the case here and the configuration function above.

bergzand · 2018-07-18T13:43:27Z

@gebart fixed! I've also added an ifdef around the config in LwIP. I've used MODULE_LWIP_SIXLOWPAN there as it was already used as guards around 802.15.4 specific code

miri64 · 2018-07-18T13:44:55Z

I actually already dislike the ifdefing in the current gnrc_netif.c and will cause more problems for new device types with similar but not equal options. Maybe this can also be put into the init function of the netdev_data layer?

bergzand · 2018-07-18T13:52:36Z

I actually already dislike the ifdefing in the current gnrc_netif.c

Yeah, I agree with this.

Maybe this can also be put into the init function of the netdev_data layer?

What exactly is the netdev_data layer here?

I strongly prefer to keep the initialization of the device settings outside of netdev. When we have the layered netdev approach, it can be dangerous to set netopt settings halfway the stack. Layers on top of the layer enabling settings might have to be aware of settings that get enabled/disabled or have to mask certain settings for lower layers.

For example a software "AUTOACK" layer would have to be aware of enabling or disabling the NETOPT_AUTOACK flag.

Maybe we can move this to the gnrc_netif_ieee802154.c somehow.

miri64 · 2018-07-18T14:11:17Z

Agreed :-/

bergzand · 2018-07-19T09:03:31Z

@miri64 I've moved the flag setters for gnrc to a new (but already supported) init function for gnrc_netif_ieee802154.

miri64 · 2018-08-24T13:58:50Z

I just tested it. My expectation would be that if I set IEEE802154_DEFAULT_ENABLE_ACK_REQ=0 that ACK_REQ would be disabled on start-up. However, the current behavior seems to be "leave it to the device". Is that intended?

miri64 · 2018-09-25T19:16:32Z

However, the current behavior seems to be "leave it to the device". Is that intended?

@bergzand ping?

bergzand · 2018-09-28T18:43:55Z

I just tested it. My expectation would be that if I set IEEE802154_DEFAULT_ENABLE_ACK_REQ=0 that ACK_REQ would be disabled on start-up. However, the current behavior seems to be "leave it to the device". Is that intended?

It was originally intended. For now it is probably better to enable/disable every setting the way it is defined.

In the long I'd like to remove the mix of flag settings from each device and only enable the flags required/configured by the network stack.

miri64 · 2018-09-29T08:49:07Z

It was originally intended. For now it is probably better to enable/disable every setting the way it is defined.

In the long I'd like to remove the mix of flag settings from each device and only enable the flags required/configured by the network stack.

Ok. That should however be done ASAP after this PR was merged.

miri64 · 2018-10-08T12:45:26Z

Ok. That should however be done ASAP after this PR was merged.

Ping @bergzand?

jia200x · 2018-10-08T13:22:18Z

@jia200x Does OpenThread require any specific flags from the network interface to function, does it make sense to honor the settings provided at compile time as much as possible except for any flags strictly required?

There is this function. It ask for the radio capabilities. So, up to a certain level we can specify some flags at compile time and report them to the stack with that function.

Concerning CHANNEL, PANID, etc we might need to think how to integrate them at compile time. We have our own init for OpenThread, so I guess it's possible ;)

bergzand · 2018-11-08T20:20:17Z

@jia200x Does OpenThread require any specific flags from the network interface to function, does it make sense to honor the settings provided at compile time as much as possible except for any flags strictly required?

There is this function. It ask for the radio capabilities. So, up to a certain level we can specify some flags at compile time and report them to the stack with that function.

@jia200x But from the set of {ACK_REQ, AUTOACK, CSMA}, is there anything that should always be enable if possible for Openthread to function?

jia200x · 2018-11-09T11:05:42Z

@jia200x But from the set of {ACK_REQ, AUTOACK, CSMA}, is there anything that should always be enable if possible for Openthread to function?

I will check this ASAP. At least I know the ACK_REQ and AUTOACK are required with the current implemetation.

Ping me during the next week if I don't reply :)

This adds support to GNRC for enabling radio flags on device initialization

This adds support to LwIP for enabling radio flags on device initialization

This adds support to emb6 for enabling radio flags on device initialization

ACK_REQ and AUTOACK are always forced on independent of the settings as Openthread requires that these are enabled.

bergzand · 2018-11-09T11:28:03Z

Rebased and added openthread based on @jia200x current instructions.

bergzand · 2018-11-11T16:31:00Z

sys/net/gnrc/netif/gnrc_netif_ieee802154.c

@@ -74,6 +76,31 @@ static gnrc_pktsnip_t *_make_netif_hdr(uint8_t *mhr)
    return snip;
 }

+static void _init(gnrc_netif_t *netif)


This should also be called when a NETOPT_STATE_RESET set is done

bergzand · 2018-12-03T09:41:31Z

@jia200x More ping!

jia200x · 2018-12-10T12:23:02Z

@bergzand The current OpenThread port assumes:

Automatic Ack handling (with CSMA)
Ack Timeout event (e.g call to NOACK event)
ACK_REQ is enabled.

However most of these features are also implemented by OpenThread (but not used in the RIOT implementation). So, in theory there's no strict requirement on {ACK_REQ, AUTOACK, CSMA} set to make it work. But ACK_REQ should be enabled.

The thing is, otPlatRadioGetCaps should be implemented with netdev->driver->get calls (NETOPT_CSMA, NETOPT_AUTOACK, etc). I plan do it now-ish :)

TL;DR: Let's assume for now only AUTOACK and ACK_REQ are required (I guess it also calls NOACK if failed, doesn't?)

stale · 2019-08-10T03:08:18Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

bergzand added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: pkg Area: External package ports GNRC labels Jul 18, 2018

bergzand requested review from jnohlgard, miri64 and PeterKietzmann July 18, 2018 08:27

jnohlgard reviewed Jul 18, 2018

View reviewed changes

bergzand commented Jul 18, 2018

View reviewed changes

miri64 self-assigned this Sep 25, 2018

bergzand mentioned this pull request Oct 4, 2018

Ping no NETDEV_IEEE802154_ACK_REQ enabled #9957

Closed

miri64 removed the GNRC label Oct 5, 2018

miri64 mentioned this pull request Oct 8, 2018

pkg/openwsn: re-integrate the network stack as a package #8570

Closed

bergzand added 3 commits November 9, 2018 12:16

ieee802154: Add defines for default device flags

e6f349e

gnrc_netif: Support for ieee802154 init flags

4669a6d

This adds support to GNRC for enabling radio flags on device initialization

lwip: Support for ieee802154 init flags

1093e2a

This adds support to LwIP for enabling radio flags on device initialization

bergzand added 5 commits November 9, 2018 12:17

emb6: Support for ieee802154 init flags

ed32575

This adds support to emb6 for enabling radio flags on device initialization

fixup! gnrc_netif: Support for ieee802154 init flags

0960e1a

fixup! lwip: Support for ieee802154 init flags

68dabb6

fixup! gnrc_netif: Support for ieee802154 init flags

ddd3c48

openthread: Support for ieee802154 init flags

84e9b6d

ACK_REQ and AUTOACK are always forced on independent of the settings as Openthread requires that these are enabled.

bergzand force-pushed the pr/netdev/default_flags branch from 61d5179 to 84e9b6d Compare November 9, 2018 11:27

bergzand mentioned this pull request Nov 9, 2018

radios: Remove default radio flags from init #10355

Closed

5 tasks

bergzand commented Nov 11, 2018

View reviewed changes

bergzand mentioned this pull request Dec 3, 2018

drivers: Add support for KW41Z builtin transceiver #7107

Closed

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

stale bot closed this Sep 10, 2019

ieee802154: Add support for default flag settings #9592

ieee802154: Add support for default flag settings #9592

Uh oh!

Conversation

bergzand commented Jul 18, 2018

Contribution description

Issues/PRs references

Testing

samr21-xpro

Other radios

Known issues

Uh oh!

bergzand commented Jul 18, 2018

Uh oh!

jnohlgard left a comment

Choose a reason for hiding this comment

Uh oh!

jnohlgard Jul 18, 2018

Choose a reason for hiding this comment

Uh oh!

bergzand Jul 18, 2018

Choose a reason for hiding this comment

Uh oh!

bergzand Jul 18, 2018

Choose a reason for hiding this comment

Uh oh!

bergzand commented Jul 18, 2018

Uh oh!

miri64 commented Jul 18, 2018

Uh oh!

bergzand commented Jul 18, 2018

Uh oh!

miri64 commented Jul 18, 2018

Uh oh!

bergzand commented Jul 19, 2018

Uh oh!

miri64 commented Aug 24, 2018

Uh oh!

miri64 commented Sep 25, 2018

Uh oh!

bergzand commented Sep 28, 2018

Uh oh!

miri64 commented Sep 29, 2018

Uh oh!

miri64 commented Oct 8, 2018

Uh oh!

jia200x commented Oct 8, 2018

Uh oh!

bergzand commented Nov 8, 2018

Uh oh!

jia200x commented Nov 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bergzand commented Nov 9, 2018

Uh oh!

bergzand Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

bergzand commented Dec 3, 2018

Uh oh!

jia200x commented Dec 10, 2018

Uh oh!

stale bot commented Aug 10, 2019

Uh oh!

Uh oh!

jia200x commented Nov 9, 2018 •

edited

Loading