search

TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
980 stars 309 forks source link

Maximum downlink payload size exceeded when dwell time is activated #4971

Closed johanstokking closed 2 years ago

johanstokking commented 2 years ago

Summary

Maximum downlink payload size is exceeded when the end device has dwell time enabled, but the Network Server's frequency plan does not.

Steps to Reproduce

  1. Register device with AS_920_923_TTN_AU (dwell time is deactivated)
  2. Configure device with AS923 band, which boots with dwell time activated
  3. Let end device join; this works successfully
  4. Observe first downlink sent by Network Server

What do you see now?

That downlink message can be too long for the end device, which assumes that dwell time is activated. Consequently, the end device may reject the downlink, leading to an invalid state and potential loss of the downlink path.

For instance, it may carry FOpts 06 07 07 a0 af 8c 50 03 31 ff 00 01 09 05, i.e. TxParamSetupReq, DevStatusReq, LinkADRReq and NewChannelReq. This TxParamSetupReq deactivates dwell time, but this downlink frame is discarded.

What do you want to see instead?

Network Server should not exceed the maximum payload size for downlink, even when dwell time is deactivated in the frequency plan, because it may still be activated in the end device.

In RP1 1.0.2 Rev. B band AS923, there is no default value specified for end device boot parameters. This means that NS cannot assume that dwell time is activated or not when the device joins. Wrongly using dwell time activated breaks the RX1 DR offset; wrongly using dwell time deactivated breaks the maximum payload size.

Environment

TTS 3.16.1, AS923 band, AS_920_923_TTN_AU frequency plan, LoRaWAN 1.0.2, RP1 1.0.2 rev. B.

How do you propose to implement this?

NS always considers the dwell time setting from the frequency plan: https://github.com/TheThingsNetwork/lorawan-stack/blob/v3.16/pkg/networkserver/downlink.go#L1243

To support AS923 RP1 1.0.2 Rev. B end devices with dwell time activated and deactivated on boot, we need a new MACSettings field (BoolValue). This should take precedence when configuring current parameters on MAC state reset.

Since LoRaMac-node has dwell time activated in this band and version, NS should assume that dwell time is activated when the new MAC settings field is not set and false.

In short:

  1. Add BoolValue {uplink,downlink}_dwell_time to MACSettings
  2. On MAC state reset, for configuring the current parameters, use the value from the new MAC setting (if set). If unset, use the new band version assumed value, which will be true for RP1 1.0.2 Rev. B band AS923. It should be commented that the assumed value comes from LoRaMac-node
  3. Make sure that frequency plan's dwell time settings are only used on MAC state reset to configure desired parameters, not in other places
  4. The maximum payload size is always the most restrictive between: MAC settings boot value, band version default and frequency plan. This is basically the most restrictive between the current and desired parameters, even if the desired parameters have not been conveyed yet to the end device.

How do you propose to test this?

Unit testing

Can you do this yourself and submit a Pull Request?

Can review

johanstokking commented 2 years ago

I updated the original comment; we should assume that dwell time is activated in this band and version, but it should be possible to disable it explicitly via a new field in MAC settings. This is because the boot value is undefined in this band and version (which is fortunately fixed in RP2).

johanstokking commented 2 years ago

@NicolasMrad let's triage this based on @adriansmares assessment.

NicolasMrad commented 2 years ago

https://github.com/TheThingsNetwork/lorawan-stack/issues/725

johanstokking commented 2 years ago

725

That is since 1.0.3. The behavior in this issue applies specifically to 1.0.2 Rev. B, where the end device's boot parameters in AS923 aren't defined.

725 is an optimization; avoid sending a redundant TxParamSetupReq to deactivate downlink dwell time while it was already deactivated.

This issue and #725 can probably be resolved in one go, once the MAC state reset configure the current parameters correctly. In case of 1.0.2 Rev. B, downlink dwell time should be activated (unless overridden via MAC settings), while in 1.0.3 and higher it should be deactivated.

htdvisser commented 2 years ago

Doesn't look like this is going to get finished for v3.17.0, so pushing to v3.17.1.