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

Fix handling of FCnt resets and 32-bit FCnt rollovers #3738

Merged
merged 9 commits into from Feb 9, 2021

Conversation

rvolosatovs
Copy link
Contributor

@rvolosatovs rvolosatovs commented Feb 4, 2021

Summary

Closes #3728
Refs #3743

Changes

  • Fix msgpack encoding of sessions
  • Fix FCnt check on reset for devices with 32-bit frame counters
  • Migrate sessions created on 3.11 snapshot to new format

Testing

Unit tests, integration testing

ses=$(cli dev get test-app2 test-dev-c --session)
dev_addr=$(echo "${ses}" | jq -r '.session.dev_addr')
f_nwk_s_int_key=$(echo "${ses}" | jq -r '.session.keys.f_nwk_s_int_key.key')
echo "${ses}" | jq
cli simulate gateway-uplink --lorawan-phy-version "1.0.3-a" --lorawan-version "1.0.3" --gateway-id test-gtw --dev-addr "${dev_addr}" --f_nwk_s_int_key "${f_nwk_s_int_key}" --f_cnt 0x42
cli simulate gateway-uplink --lorawan-phy-version "1.0.3-a" --lorawan-version "1.0.3" --gateway-id test-gtw --dev-addr "${dev_addr}" --f_nwk_s_int_key "${f_nwk_s_int_key}" --f_cnt "${1:-"0x02"}"

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@rvolosatovs rvolosatovs added bug Something isn't working c/network server This is related to the Network Server compat/db This could affect Database compatibility blocking release This is blocking a release labels Feb 4, 2021
@rvolosatovs rvolosatovs added this to the February 2021 milestone Feb 4, 2021
@rvolosatovs rvolosatovs changed the title ns,util: Lay groundwork for msgpack encoding fix Fix handling of FCnt resets Feb 4, 2021
@rvolosatovs rvolosatovs marked this pull request as draft February 4, 2021 11:53
@rvolosatovs rvolosatovs force-pushed the fix/fcnt-reset branch 2 times, most recently from b2ebd98 to b5f9dc6 Compare February 4, 2021 21:35
@KrishnaIyer
Copy link
Member

@rvolosatovs: This only affects v3.11 and doesn't need a backport right?

@rvolosatovs
Copy link
Contributor Author

yes

Copy link
Contributor

@htdvisser htdvisser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why GitHub decided to request code-owner review on a draft PR. Only quickly glanced over it.

pkg/networkserver/redis/registry.go Outdated Show resolved Hide resolved
pkg/networkserver/redis/registry.go Show resolved Hide resolved
@rvolosatovs rvolosatovs mentioned this pull request Feb 5, 2021
5 tasks
@rvolosatovs rvolosatovs changed the title Fix handling of FCnt resets Fix msgpack encoding of sessions Feb 5, 2021
@rvolosatovs rvolosatovs removed the blocking release This is blocking a release label Feb 5, 2021
@rvolosatovs rvolosatovs force-pushed the fix/fcnt-reset branch 3 times, most recently from ef1b3cd to c09bcad Compare February 5, 2021 20:10
Roman Volosatovs and others added 2 commits February 8, 2021 14:06
Co-authored-by: Erik van Bennekum <erik@thethingsindustries.com>
@rvolosatovs rvolosatovs changed the title Fix msgpack encoding of sessions Fix handling of FCnt resets and 32-bit FCnt rollovers Feb 8, 2021
@rvolosatovs rvolosatovs marked this pull request as ready for review February 8, 2021 14:27
@rvolosatovs rvolosatovs added the blocking release This is blocking a release label Feb 8, 2021
Copy link
Contributor

@htdvisser htdvisser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks okay to me. We're definitely going to need to verify that this all works in a new rc, so let's get that going as soon as this is merged.

@johanstokking johanstokking merged commit c96db1b into v3.11 Feb 9, 2021
@johanstokking johanstokking deleted the fix/fcnt-reset branch February 9, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking release This is blocking a release bug Something isn't working c/network server This is related to the Network Server compat/db This could affect Database compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frame Counter Resets setting has no effect (Store error)
6 participants