Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relay NotifyNewEndDeviceReq payload incorrectly formatted #7334

Open
3 of 6 tasks
StevenCellist opened this issue Oct 9, 2024 · 6 comments
Open
3 of 6 tasks

Relay NotifyNewEndDeviceReq payload incorrectly formatted #7334

StevenCellist opened this issue Oct 9, 2024 · 6 comments
Assignees
Labels
needs/testing This needs to be tested on staging
Milestone

Comments

@StevenCellist
Copy link

StevenCellist commented Oct 9, 2024

Summary

The payload of the Relay MAC command NotifyNewEndDeviceReq is not correctly formatted/parsed on TTS.

Steps to Reproduce

  1. Register two devices on the console on LW v1.0.4.
  2. Set one device as the serving device (relay): ttn-lw-cli relays create <app-id> <device-id> --mode.serving
  3. Set other device as the served device (node): ttn-lw-cli relays create <app-id> <device-id> --mode.served.mode.always --mode.served.serving-device-id <device-id>
  4. Set uplink rule for serving device to allow served device to use the relay: ttn-lw-cli relays uplink-forwarding-rules create <app-id> <device-id> 0 --device-id <device-id>
  5. Let the end-device join the network through a normal gateway.
  6. Send an uplink from the end-device through the relay, such that NotifyNewEndDeviceReq is triggered.

Current Result

NotifyNewEndDeviceReq payload formatted as per TS011 paragraph 10.7

First four bytes DevAddr, last two bytes PowerLevel

Served device DevAddr copied from Console: 260B470F

Relay log:

18:51:49.983 > [MAC] 46
18:51:49.983 > [Pay] 0f 47 0b 26 76 00

Relay console log:

{
  "name": "ns.mac.relay_notify_new_end_device.indication",
  "time": "2024-10-09T16:51:50.829305191Z",
  "identifiers": [
    {
      "device_ids": {
        "device_id": "relay",
        "application_ids": {
          "application_id": "xxx"
        },
        "dev_eui": "750110000008E7A4",
        "join_eui": "CE77157000000000",
        "dev_addr": "260B3F14"
      }
    }
  ],
  "data": {
    "@type": "type.googleapis.com/ttn.lorawan.v3.MACCommand.RelayNotifyNewEndDeviceReq",
    "dev_addr": "0076260B",
    "snr": -5,
    "rssi": -71
  },
  "correlation_ids": [
    "gs:uplink:01J9S33AQ31XBJG162AKNSFDJS"
  ],
  "origin": "ip-10-100-12-21.eu-west-1.compute.internal",
  "context": {
    "tenant-id": "CgN0dG4="
  },
  "visibility": {
    "rights": [
      "RIGHT_APPLICATION_TRAFFIC_READ"
    ]
  },
  "unique_id": "01J9S33AXDX7VPSENRNCGBVJ99"
}

Notice that the dev_addr field of RelayNotifyNewEndDeviceReq is not correct (as well as snr/rssi but those are not so easy to see).


NotifyNewEndDeviceReq payload swapped formatting

First two bytes PowerLevel, last four bytes DevAddr

Served device DevAddr copied from Console: 260B01C7

Relay log:

18:51:49.983 > [MAC] 46
18:51:49.983 > [Pay] 36 00 c7 01 0b 26

Relay console log:

{
  "name": "ns.mac.relay_notify_new_end_device.indication",
  "time": "2024-10-09T16:58:32.273130864Z",
  "identifiers": [
    {
      "device_ids": {
        "device_id": "relay",
        "application_ids": {
          "application_id": "relay-test-radiolib"
        },
        "dev_eui": "750110000008E7A4",
        "join_eui": "CE77157000000000",
        "dev_addr": "260B4801"
      }
    }
  ],
  "data": {
    "@type": "type.googleapis.com/ttn.lorawan.v3.MACCommand.RelayNotifyNewEndDeviceReq",
    "dev_addr": "260B01C7",
    "snr": 2,
    "rssi": -16
  },
  "correlation_ids": [
    "gs:uplink:01J9S3FJR578V4E7QA9CZ3F8F0"
  ],
  "origin": "ip-10-100-5-143.eu-west-1.compute.internal",
  "context": {
    "tenant-id": "CgN0dG4="
  },
  "visibility": {
    "rights": [
      "RIGHT_APPLICATION_TRAFFIC_READ"
    ]
  },
  "unique_id": "01J9S3FJYHNEES6YQCSZZ3BDA5"
}

Notice that this swapped formatting yields the expected result on the console.

Expected Result

The formatting as per TS011 paragraph 10.7 should be the correct formatting (and therefore the decoding on TTS should be corrected).

Relevant Logs

No response

URL

No response

Deployment

The Things Stack Community Edition

The Things Stack Version

3.32.1

Client Name and Version

The Things Network Command-line Interface: ttn-lw-cli
Version:             3.31.1
Build date:          2024-08-01T15:00:26Z
Git commit:          e351ea62e
Go version:          go1.21.12
OS/Arch:             windows/amd64

Other Information

No response

Proposed Fix

Swap the payload fields of NotifyNewEndDeviceReq.

Contributing

  • I can help by doing more research.
  • I can help by implementing a fix after the proposal above is approved.
  • I can help by testing the fix before it's released.

Validation

Code of Conduct

@StevenCellist StevenCellist added the needs/triage We still need to triage this label Oct 9, 2024
@KrishnaIyer
Copy link
Member

This is not a bug. The endianness of the value printed on your relay log is in reverse.

[Pay] 36 00 c7 01 0b 26

The Things Stack Sandbox does not issue a Device Address of c7 01 0b 26 but 26 0b 01 c7 is valid device address from 0x000013 NET ID.

The Power level of 36 00 is also in reverse. If you look at the spec, the the first 4 bits should be RFU (zeroes) but for 36 00, that would be 0011b.

Screenshot 2024-10-22 at 13 24 57

@KrishnaIyer KrishnaIyer closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2024
@StevenCellist
Copy link
Author

@KrishnaIyer I object, your honour. I will supply more information this time around.

The endianness of my device is right, and the endianness of the network is similarly little-endian. Observe the contents of a (random) downlink, dumped in straight byte order as received by the radio:

14:38:35.772 > 00000000: 60 24 21 0b 26 90 01 00 e2 d8 c9 df 0f 0e 86 33  `$!.&..........3
14:38:35.780 > 00000010: e6 50 03 65 14 c8 60 23 f8 21 01 fc 24 66 ca ff  .P.e..`#.!..$f..
14:38:35.787 > 00000020: 73 b2 0e f8 9a 16 f2 5e 21 21 be c6 54 81        s......^!!..T.

Byte 0 is the MHDR, byte 1 through 4 is the DevAddr: 24 21 0b 26 which indeed is a valid DevAddr for NetID 0013, given the little-endianness.

With that out of the way, here is a NotifyNewEndDevice, as seen through the Relay:

14:48:27.857 > DevAddr: 260BF444, RSSI: -18.0, SNR: 10.5
14:48:27.861 > 00000000: 3e 04 44 f4 0b 26                                >.D..&
14:48:27.869 > Uplink MAC payload:
14:48:27.871 > 00000000: 46 3e 04 44 f4 0b 26                             F>.D..&
14:48:27.938 > Uplink (FCntUp = 3) decoded:
14:48:27.949 > 00000000: 40 24 21 0b 26 07 03 00 46 3e 04 44 f4 0b 26 01  @$!.&...F>.D..&.
14:48:27.956 > 00000010: 20 20 20 20

Base64 payload in the console:

QCQhCyYHAwBGPgRE9AsmAX0ukxU=

NwkSEncKey from the console:

C609BFBE338429DF7AE2AD71E5CA6516

Online decoder:

         ( FHDR = DevAddr[4] | FCtrl[1] | FCnt[2] | FOpts[0..15] )
        DevAddr = 260B2124 (Big Endian)
          FCtrl = 07
           FCnt = 0003 (Big Endian)
          FOpts = 463E0444F40B26

Now the corresponding downlink to UpdateUplinkList (in base64):

YCQhCyaAAgAAvTS6taYrpldtxpMCEUliKeBI46hebsllQQ7PNEvS8g==

Which, decoded, reports the following:

  ( MACPayload = FHDR | FPort | FRMPayload )
          FHDR = 24210B26800200
         FPort = 00
    FRMPayload = BD34BAB5A62BA6576DC6930211496229E048E3A85E6EC965410ECF (from packet, encrypted)
               = 43003C44F40B2600000000E8E8C7BDFA679958D0F4F29CB9CBE203 (decrypted)

Notice the same endianness on the DevAddr in bytes 2-5.
image
This matches exactly with the 2 single bytes and then the DevAddr.

Given that the byte ordering is clear from the UpdateUplinkList, here is again the NotifyNewEndDevice:
image

We should see the four bytes for DevAddr first (in little-endianness), and only then the PowerLevel.
And as a result, the PowerLevel encoding is not setting the RFU bits.

I am curious to hear from you @KrishnaIyer. If there are any questions, please let me know, happy to supply more information.

@StevenCellist
Copy link
Author

@KrishnaIyer can you please review this additional information?

@StevenCellist
Copy link
Author

@KrishnaIyer @johanstokking sorry for the repeated ping, but I'd like to investigate this further please.

@johanstokking
Copy link
Member

@StevenCellist thanks for your report. So the issue is that the fields are swapped; we're sending PowerLevel | DevAddr and not DevAddr | PowerLevel, the endianness being correct, yes?

Looping in @halimi

@johanstokking johanstokking reopened this Dec 4, 2024
@StevenCellist
Copy link
Author

Yes, that seems to be the case, to the best of my understanding. Thanks for reopening!

@KrishnaIyer KrishnaIyer added this to the v3.33.1 milestone Dec 5, 2024
@KrishnaIyer KrishnaIyer removed the needs/triage We still need to triage this label Dec 5, 2024
@KrishnaIyer KrishnaIyer added the needs/testing This needs to be tested on staging label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/testing This needs to be tested on staging
Projects
None yet
Development

No branches or pull requests

4 participants