gnrc/lorawan: add basic LoRaWAN 1.1 features #17884

Ollrogge · 2022-03-29T17:59:31Z

Contribution description

This PR adds basic support for LoRaWAN version 1.1. This includes joining a network and sending / receiving data.
This PR also modifies the LoRaWAN 1.0 code to use the 1.1 terms. This should not change the behavior of the 1.0 code and everything should still work.

Not implemented are ReJoin-Requests and MAC commands such as RekeyInd and RekeyConf.

For testing purposes I setup a private LoRaWAN network using the Chirpstack and was able to join the network and send data.

Testing procedure

tests/gnrc_lorawan_11
examples/gnrc_lorawan if able to setup a private LoRaWAN network

jia200x · 2022-04-04T09:17:23Z

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan.c

@@ -76,14 +76,34 @@ static void _sleep_radio(gnrc_lorawan_t *mac)
    dev->driver->set(dev, NETOPT_STATE, &state, sizeof(state));
 }

-void gnrc_lorawan_init(gnrc_lorawan_t *mac, uint8_t *nwkskey, uint8_t *appskey)
+#if IS_ACTIVE(CONFIG_LORAWAN_1_1)
+void gnrc_lorawan_init_11x(gnrc_lorawan_t *mac, uint8_t *joineui,


I think at this point it makes sense to use a common gnrc_lorawan_init function but with a "ctx" parameter. E.g

void gnrc_lorawan_init(gnrc_lorawan_t *mac, const gnrc_lorawan_ctx_t *ctx) { mac->nwkskey = ctx->nwskey; .... }

Since this function would be resolved at compile time (e.g user cannot switch between 1.0.3 and 1.1 on runtime), I think it will simplify things a bit

Is there a kind of hard cut between gnrc_netif_lorawan and gnrc_lorawan ? Otherwise I could just pass in a pointer to gnrc_netif_lorawan_t without needing to declare a new ctx struct ?

yes, gnrc_netif_lorawan is the "GNRC Network Interface Implementation" for GNRC LoRaWAN. It could be possible to run GNRC LoRaWAN directly from the API without a network interface. That's why these two structures are decoupled.

If you make it const, you can e.g store that information on ROM.

Okay I see :) I added the gnrc_lorawan_ctx_t struct.

Another question regarding gnrc netif: Right now to configure the NwkSKey for Lorawan 1.0x one has to set CONFIG_LORAMAC_FNWKSINT_KEY_DEFAULT. As I think this is kind of confusing for someone wanting to configure a 1.0x node I think it would be better to use the old 1.0x terms in the netif layer and only switch to the new terms internally ?

hmmm good question... I usually tend to think that's better to reflect the actual key names depending on the version. For example, a user that explicitly wants to use LoRaWAN 1.1 will probably know the key names. Most LoRaWAN users will still use LoRaWAN 1.0.x, so it's probably to keep the original names there. You can always fallback to macro aliases that use the terms internally.

jia200x · 2022-04-05T10:15:59Z

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_crypto.c

-void gnrc_lorawan_calculate_mic(const le_uint32_t *dev_addr, uint32_t fcnt,
-                                uint8_t dir, iolist_t *frame,
-                                const uint8_t *nwkskey, le_uint32_t *out)
+#if IS_USED(CONFIG_LORAWAN_1_1)


Same here. You can merge these two functions and use a common signature. If you have different parameters, consider a structure.
You can also kind of merge the logic here: gnrc_lorawan_calculate_mic_uplink_11x is (almost) a subset of gnrc_lorawan_calculate_mic_uplink_10x.

Also, you can use a regular if with IS_ACTIVEor IS_USED in some parts. The compiler will remove branches that are never reached. Of course, you cannot do that with struct members that don't exist on compile time (those who appear only with LoRaWAN 1.1).

For instance, you could write these functions as:

void gnrc_lorawan_calculate_mic_uplink(const le_uint32_t *dev_addr, uint32_t fcnt, ... lorawan_mic_t *ctx, ... le_uint32_t *out) { orawan_block0_t block0 = { 0 }; block0.fb = MIC_B0_START; block0.dir = 0x00; memcpy(&block0.dev_addr, dev_addr, sizeof(le_uint32_t)); block0.fcnt = byteorder_htoll(fcnt); block0.len = iolist_size(frame); /* cmacF = aes128_cmac(FNwkSIntKey, B0 | msg) */ cmac_init(&CmacContext, fnwksintkey, LORAMAC_FNWKSINTKEY_LEN); cmac_update(&CmacContext, &block0, sizeof(block0)); for (iolist_t *io = frame; io != NULL; io = io->iol_next) { cmac_update(&CmacContext, io->iol_base, io->iol_len); } cmac_final(&CmacContext, digest); if (IS_ACTIVE(GNRC_LORAWAN_1_1)) { /* Do 1.1 specific stuff an get specific keys from `ctx`*/ } }

When exactly can I use if with this macro ? I tried to e.g. use here and it didn't work.
I find it kinda weird to depend on compiler optimizations to make my code compile ...

it probably failed because _appkey exists only when you set LoRaWAN 1.1. Since the "topology" of the file is the same, the variables should always exist.

You can always use if with this macro, since always resolves to 0 or 1.

For example:

/* foo */ if (IS_ACTIVE(MY_MODULE)) { /* bar */ }

In this example the snippets "foo" and "bar" are compiled like regular C code. But in case "MY_MODULE" is not used, the compiler will optimize out "bar". The compiler will still find syntax errors in "bar", since the compiler parses the snippet anyway.

I find it kinda weird to depend on compiler optimizations to make my code compile ...

What do you mean with that? You don't depend on compiler optimizations to compile your code (if you turn off compiler optimizations it will still compile without issues). Could you elaborate more?

it probably failed because _appkey exists only when you set LoRaWAN 1.1. Since the "topology" of the file is the same, the variables should always exist.

You can always use if with this macro, since always resolves to 0 or 1.

For example:

/* foo */ if (IS_ACTIVE(MY_MODULE)) { /* bar */ }

In this example the snippets "foo" and "bar" are compiled like regular C code. But in case "MY_MODULE" is not used, the compiler will optimize out "bar". The compiler will still find syntax errors in "bar", since the compiler parses the snippet anyway.

Having _appkey exist all the time fixed the issue. Thank you !
I misunderstood how using if (IS_USED(...) works. Your example and explanation cleared things up !

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_crypto.c

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_mcps.c

jia200x · 2022-04-05T10:23:23Z

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_region.c

@@ -106,6 +107,8 @@ uint32_t gnrc_lorawan_pick_channel(gnrc_lorawan_t *mac)
    for (int i = 0; i < pos + 1; i++) {
        state = bitarithm_test_and_clear(state, &index);
    }
+
+    mac->last_chan_idx = index;


since this function only picks a random channel, I would be in favor of maintaining "mac->last_chan_idx" outside of this function. It makes it easier to maintain as well

I am now managing it inside gnrc_lorawan_mcps_request by just looping through the channels and looking for the correct one. With this change I had removed the call to gnrc_lorawan_pick_channel from gnrc_lorawan_event_retrans_timeout which I had added because otherwise the last_chan_idx wont be correct.

I am wondering if this is bad since gnrc_lorawan_event_retrans_timeout might wan't to use a new channel ?

I am wondering if this is bad since gnrc_lorawan_event_retrans_timeout might wan't to use a new channel ?

Hmmmm yes, the retransmssion should follow the same rules of any transmission. So it should choose a different channel.

Due to this I now moved the channel picking + the calculation of the uplink mic into the _transmit_pkt function. This is the cleanest solution I could find.

jia200x · 2022-04-05T10:24:52Z

sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c

 static uint8_t _appskey[LORAMAC_APPSKEY_LEN];
+#if IS_USED(CONFIG_LORAWAN_1_1)
 static uint8_t _appkey[LORAMAC_APPKEY_LEN];


why is _appkey not required in LoRaWAN 1.0.3?

It is but its name has changed. The 1.1 spec uses the NwkKey in place of the AppKey.

ok, I see. But maybe you could try to use the same variable names internally. Otherwise you cannot use regular if (IS_ACTIVE()) stuff

sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c

jia200x · 2022-04-05T10:30:14Z

Overall the code looks nice and the changes make sense. I left some comments there and plan to run some tests when they are addressed.

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_mlme.c

Ollrogge · 2022-05-31T13:43:58Z

@jia200x I implement the Rekey MAC command and successfully tested it with TTN. Please test again if you can :)

jia200x · 2022-06-28T09:43:04Z

Hi. It works now!
However, according to this, there should be also backward compatibility between a v1.1 device and a 1.0.3 network server.
When I configure that on TTN, it throws invalid MIC. Does it work for you on Chirpstack?

Ollrogge · 2022-06-29T13:50:31Z

Hi. It works now! However, according to this, there should be also backward compatibility between a v1.1 device and a 1.0.3 network server. When I configure that on TTN, it throws invalid MIC. Does it work for you on Chirpstack?

Should be fixed as well now !

jia200x · 2022-08-08T12:08:34Z

I can confirm this still works as expected when compiling LoRaWAN 1.0.3. I will proceed with all tests for LoRaWAN 1.1

jia200x · 2022-08-23T12:12:29Z

ok, everything seems to work now

jia200x

Tested ACK. Please squash commits to make them more semantic (e.g "add support for GNRC LoRaWAN 1.1", "Adapt test". etc...)

Ollrogge · 2022-08-29T10:18:44Z

@jia200x Tests for some boards fail because they don't implement they flashpage API which I need for LoRaWAN 1.1.
How should I handle this ?

Ollrogge · 2022-09-21T07:09:07Z

@jia200x Can you explain to me the doc and Murdock failures ? Neither seem to be related to my code ...

jia200x · 2022-09-22T17:21:28Z

There seemed to be an unrelated error. I retriggered the CI. Let's see what happens

Ollrogge · 2022-09-23T06:36:40Z

Looks like the same issue :(

jia200x · 2022-09-27T19:23:01Z

Looks like the same issue :(

I just realized that the Murdock problem was because you added NETOPTs without updating tests/unittests/tests-netopt. You have to add the new NETOPTs there to make the test pass.
Let's try to fix that one first and then see what went on with the documentation

Ollrogge · 2022-09-29T18:33:17Z

Looks like the same issue :(

I just realized that the Murdock problem was because you added NETOPTs without updating tests/unittests/tests-netopt. You have to add the new NETOPTs there to make the test pass. Let's try to fix that one first and then see what went on with the documentation

Thanks for this ! I should have checked the error message more closely... All green now !

jia200x · 2022-09-30T09:30:14Z

finally! congrats @Ollrogge 🎉 🎉 🎉

Ollrogge · 2022-09-30T09:53:59Z

finally! congrats @Ollrogge tada tada tada

Yaaayy :) Thank you for all the help !!

Ollrogge requested review from leandrolanzieri, jia200x, cgundogan, miri64, haukepetersen, PeterKietzmann, aabadie, fjmolinas, DipSwitch and vincent-d as code owners March 29, 2022 17:59

jia200x reviewed Apr 4, 2022

View reviewed changes

Ollrogge force-pushed the gnrc_lorawan1.1 branch from b7741c5 to 74caa2c Compare April 5, 2022 06:46

jia200x reviewed Apr 5, 2022

View reviewed changes

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_crypto.c Outdated Show resolved Hide resolved

jia200x reviewed Apr 5, 2022

View reviewed changes

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_mcps.c Outdated Show resolved Hide resolved

jia200x reviewed Apr 5, 2022

View reviewed changes

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_mcps.c Outdated Show resolved Hide resolved

jia200x reviewed Apr 5, 2022

View reviewed changes

sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c Outdated Show resolved Hide resolved

jia200x reviewed Apr 6, 2022

View reviewed changes

sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_mlme.c Outdated Show resolved Hide resolved

Ollrogge force-pushed the gnrc_lorawan1.1 branch from 74caa2c to 9e85b97 Compare April 7, 2022 10:15

jia200x approved these changes Aug 23, 2022

View reviewed changes

Ollrogge force-pushed the gnrc_lorawan1.1 branch from a54092e to cd400ec Compare August 23, 2022 14:12

jia200x added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 24, 2022

Ollrogge force-pushed the gnrc_lorawan1.1 branch from cd400ec to cf4fbac Compare September 16, 2022 13:36

github-actions bot removed Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Sep 16, 2022

Ollrogge force-pushed the gnrc_lorawan1.1 branch from cf4fbac to 5ef2705 Compare September 20, 2022 13:55

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 22, 2022

Ollrogge added 3 commits September 29, 2022 11:25

gnrc/lorawan: add support for GNRC LoRaWAN 1.1

12a4e1c

tests/gnrc_lorawan: Adjust 1.0 tests && add 1.1 tests

5053e3e

gnrc/lorawan: uncrustify files

32cef70

Ollrogge force-pushed the gnrc_lorawan1.1 branch from 5ef2705 to 32cef70 Compare September 29, 2022 09:26

jia200x merged commit f022ac3 into RIOT-OS:master Sep 30, 2022

maribu added this to the Release 2022.10 milestone Oct 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gnrc/lorawan: add basic LoRaWAN 1.1 features #17884

gnrc/lorawan: add basic LoRaWAN 1.1 features #17884

Ollrogge commented Mar 29, 2022 •

edited

Loading

jia200x Apr 4, 2022

Ollrogge Apr 4, 2022 •

edited

Loading

jia200x Apr 4, 2022

Ollrogge Apr 5, 2022

jia200x Apr 5, 2022

jia200x Apr 5, 2022 •

edited

Loading

Ollrogge Apr 5, 2022

jia200x Apr 6, 2022

jia200x Apr 6, 2022

Ollrogge Apr 6, 2022

jia200x Apr 5, 2022

Ollrogge Apr 7, 2022

jia200x Apr 7, 2022

Ollrogge Apr 11, 2022

jia200x Apr 5, 2022

Ollrogge Apr 5, 2022 •

edited

Loading

jia200x Apr 6, 2022

jia200x commented Apr 5, 2022

Ollrogge commented May 31, 2022

jia200x commented Jun 28, 2022

Ollrogge commented Jun 29, 2022

jia200x commented Aug 8, 2022

jia200x commented Aug 23, 2022

jia200x left a comment

Ollrogge commented Aug 29, 2022

Ollrogge commented Sep 21, 2022

jia200x commented Sep 22, 2022

Ollrogge commented Sep 23, 2022

jia200x commented Sep 27, 2022

Ollrogge commented Sep 29, 2022

jia200x commented Sep 30, 2022

Ollrogge commented Sep 30, 2022

gnrc/lorawan: add basic LoRaWAN 1.1 features #17884

gnrc/lorawan: add basic LoRaWAN 1.1 features #17884

Conversation

Ollrogge commented Mar 29, 2022 • edited Loading

Contribution description

Testing procedure

Choose a reason for hiding this comment

Ollrogge Apr 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jia200x Apr 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ollrogge Apr 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jia200x commented Apr 5, 2022

Ollrogge commented May 31, 2022

jia200x commented Jun 28, 2022

Ollrogge commented Jun 29, 2022

jia200x commented Aug 8, 2022

jia200x commented Aug 23, 2022

jia200x left a comment

Choose a reason for hiding this comment

Ollrogge commented Aug 29, 2022

Ollrogge commented Sep 21, 2022

jia200x commented Sep 22, 2022

Ollrogge commented Sep 23, 2022

jia200x commented Sep 27, 2022

Ollrogge commented Sep 29, 2022

jia200x commented Sep 30, 2022

Ollrogge commented Sep 30, 2022

Ollrogge commented Mar 29, 2022 •

edited

Loading

Ollrogge Apr 4, 2022 •

edited

Loading

jia200x Apr 5, 2022 •

edited

Loading

Ollrogge Apr 5, 2022 •

edited

Loading