From cf59c5826fdf5d0d86f00f827404e6030d07d676 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 11 May 2017 10:00:29 -0700 Subject: [PATCH] Co-locate settings structs and document rules for updating. (#1754) --- src/core/api/thread_api.cpp | 5 +- src/core/common/settings.hpp | 82 ++++++++++++++++++++++++++---- src/core/meshcop/dataset.cpp | 24 +++++++-- src/core/meshcop/dataset.hpp | 2 + src/core/thread/mle.cpp | 16 +++--- src/core/thread/mle.hpp | 27 ---------- src/core/thread/mle_router.cpp | 16 +++--- src/core/thread/mle_router_ftd.hpp | 12 ----- 8 files changed, 111 insertions(+), 73 deletions(-) diff --git a/src/core/api/thread_api.cpp b/src/core/api/thread_api.cpp index 6049b7d7b..a176438eb 100644 --- a/src/core/api/thread_api.cpp +++ b/src/core/api/thread_api.cpp @@ -489,7 +489,8 @@ bool otThreadGetAutoStart(otInstance *aInstance) uint8_t autoStart = 0; uint16_t autoStartLength = sizeof(autoStart); - if (otPlatSettingsGet(aInstance, kKeyThreadAutoStart, 0, &autoStart, &autoStartLength) != kThreadError_None) + if (otPlatSettingsGet(aInstance, Settings::kKeyThreadAutoStart, 0, &autoStart, &autoStartLength) != + kThreadError_None) { autoStart = 0; } @@ -505,7 +506,7 @@ ThreadError otThreadSetAutoStart(otInstance *aInstance, bool aStartAutomatically { #if OPENTHREAD_CONFIG_ENABLE_AUTO_START_SUPPORT uint8_t autoStart = aStartAutomatically ? 1 : 0; - return otPlatSettingsSet(aInstance, kKeyThreadAutoStart, &autoStart, sizeof(autoStart)); + return otPlatSettingsSet(aInstance, Settings::kKeyThreadAutoStart, &autoStart, sizeof(autoStart)); #else (void)aInstance; (void)aStartAutomatically; diff --git a/src/core/common/settings.hpp b/src/core/common/settings.hpp index 4dd0d22e5..4d64c2e8d 100644 --- a/src/core/common/settings.hpp +++ b/src/core/common/settings.hpp @@ -34,9 +34,32 @@ #ifndef SETTINGS_HPP_ #define SETTINGS_HPP_ -#ifdef __cplusplus -extern "C" { -#endif +#include "thread/mle.hpp" + +namespace ot { +namespace Settings { + +/** + * Rules for updating existing value structures. + * + * 1. Modifying existing otPlatSettings* key value fields MUST only be + * done by appending new fields. Existing fields MUST NOT be + * deleted or modified in any way. + * + * 2. To support backward compatibility (rolling back to an older + * software version), code reading and processing key values MUST + * process key values that have longer length. Additionally, newer + * versions MUST update/maintain values in existing key value + * fields. + * + * 3. To support forward compatibility (rolling forward to a newer + * software version), code reading and processing key values MUST + * process key values that have shorter length. + * + * 4. New Key IDs may be defined in the future with the understanding + * that such key values are not backward compatible. + * + */ /** * This enumeration defines the keys of settings @@ -44,16 +67,53 @@ extern "C" { */ enum { - kKeyActiveDataset = 0x0001, - kKeyPendingDataset = 0x0002, - kKeyNetworkInfo = 0x0003, - kKeyParentInfo = 0x0004, - kKeyChildInfo = 0x0005, - kKeyThreadAutoStart = 0x0006, + kKeyActiveDataset = 0x0001, ///< Active Operational Dataset + kKeyPendingDataset = 0x0002, ///< Pending Operational Dataset + kKeyNetworkInfo = 0x0003, ///< Thread network information + kKeyParentInfo = 0x0004, ///< Parent information + kKeyChildInfo = 0x0005, ///< Child information + kKeyThreadAutoStart = 0x0006, ///< Auto-start information }; -#ifdef __cplusplus +/** + * This structure represents the device's own network information for settings storage. + * + */ +struct NetworkInfo +{ + uint8_t mDeviceState; ///< Current Thread interface state. + uint8_t mDeviceMode; ///< Device mode setting. + uint16_t mRloc16; ///< RLOC16 + uint32_t mKeySequence; ///< Key Sequence + uint32_t mMleFrameCounter; ///< MLE Frame Counter + uint32_t mMacFrameCounter; ///< MAC Frame Counter + uint32_t mPreviousPartitionId; ///< PartitionId + Mac::ExtAddress mExtAddress; ///< Extended Address + uint8_t mMlIid[OT_IP6_IID_SIZE]; ///< IID from ML-EID }; -#endif + +/** + * This structure represents the parent information for settings storage. + * + */ +struct ParentInfo +{ + Mac::ExtAddress mExtAddress; ///< Extended Address +}; + +/** + * This structure represents the child information for settings storage. + * + */ +struct ChildInfo +{ + Mac::ExtAddress mExtAddress; ///< Extended Address + uint32_t mTimeout; ///< Timeout + uint16_t mRloc16; ///< RLOC16 + uint8_t mMode; ///< The MLE device mode +}; + +} // namespace Settings +} // namespace ot #endif // SETTINGS_HPP_ diff --git a/src/core/meshcop/dataset.cpp b/src/core/meshcop/dataset.cpp index 1b759ada9..a46157a69 100644 --- a/src/core/meshcop/dataset.cpp +++ b/src/core/meshcop/dataset.cpp @@ -65,8 +65,7 @@ void Dataset::Clear(bool isLocal) if (isLocal) { - otPlatSettingsDelete(mInstance, static_cast(mType == Tlv::kActiveTimestamp ? kKeyActiveDataset : - kKeyPendingDataset), -1); + otPlatSettingsDelete(mInstance, GetSettingsKey(), -1); } } @@ -441,8 +440,7 @@ ThreadError Dataset::Restore(void) ThreadError error; uint16_t length = sizeof(mTlvs); - error = otPlatSettingsGet(mInstance, static_cast(mType == Tlv::kActiveTimestamp ? kKeyActiveDataset : - kKeyPendingDataset), 0, mTlvs, &length); + error = otPlatSettingsGet(mInstance, GetSettingsKey(), 0, mTlvs, &length); mLength = (error == kThreadError_None) ? length : 0; return error; @@ -450,8 +448,8 @@ ThreadError Dataset::Restore(void) ThreadError Dataset::Store(void) { - uint16_t key = static_cast((mType == Tlv::kActiveTimestamp) ? kKeyActiveDataset : kKeyPendingDataset); ThreadError error; + uint16_t key = GetSettingsKey(); if (mLength == 0) { @@ -538,6 +536,22 @@ exit: return error; } +uint16_t Dataset::GetSettingsKey(void) +{ + uint16_t rval; + + if (mType == Tlv::kActiveTimestamp) + { + rval = static_cast(Settings::kKeyActiveDataset); + } + else + { + rval = static_cast(Settings::kKeyPendingDataset); + } + + return rval; +} + void Dataset::Remove(uint8_t *aStart, uint8_t aLength) { memmove(aStart, aStart + aLength, mLength - (static_cast(aStart - mTlvs) + aLength)); diff --git a/src/core/meshcop/dataset.hpp b/src/core/meshcop/dataset.hpp index c84b415c9..4f46a48f2 100644 --- a/src/core/meshcop/dataset.hpp +++ b/src/core/meshcop/dataset.hpp @@ -186,6 +186,8 @@ public: ThreadError AppendMleDatasetTlv(Message &aMessage); private: + uint16_t GetSettingsKey(void); + void Remove(uint8_t *aStart, uint8_t aLength); Tlv::Type mType; ///< Active or Pending diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 4494a99ba..7f6f4088f 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -276,15 +276,15 @@ ThreadError Mle::Stop(bool aClearNetworkDatasets) ThreadError Mle::Restore(void) { ThreadError error = kThreadError_None; - NetworkInfo networkInfo; - ParentInfo parentInfo; + Settings::NetworkInfo networkInfo; + Settings::ParentInfo parentInfo; uint16_t length; mNetif.GetActiveDataset().Restore(); mNetif.GetPendingDataset().Restore(); length = sizeof(networkInfo); - SuccessOrExit(error = otPlatSettingsGet(mNetif.GetInstance(), kKeyNetworkInfo, 0, + SuccessOrExit(error = otPlatSettingsGet(mNetif.GetInstance(), Settings::kKeyNetworkInfo, 0, reinterpret_cast(&networkInfo), &length)); VerifyOrExit(length == sizeof(networkInfo), error = kThreadError_NotFound); VerifyOrExit(networkInfo.mDeviceState >= kDeviceStateChild, error = kThreadError_NotFound); @@ -304,7 +304,7 @@ ThreadError Mle::Restore(void) if (networkInfo.mDeviceState == kDeviceStateChild) { length = sizeof(parentInfo); - SuccessOrExit(error = otPlatSettingsGet(mNetif.GetInstance(), kKeyParentInfo, 0, + SuccessOrExit(error = otPlatSettingsGet(mNetif.GetInstance(), Settings::kKeyParentInfo, 0, reinterpret_cast(&parentInfo), &length)); VerifyOrExit(length >= sizeof(parentInfo), error = kThreadError_Parse); @@ -345,8 +345,8 @@ exit: ThreadError Mle::Store(void) { ThreadError error = kThreadError_None; - NetworkInfo networkInfo; - ParentInfo parentInfo; + Settings::NetworkInfo networkInfo; + Settings::ParentInfo parentInfo; VerifyOrExit(IsAttached(), error = kThreadError_InvalidState); @@ -378,11 +378,11 @@ ThreadError Mle::Store(void) memset(&parentInfo, 0, sizeof(parentInfo)); memcpy(&parentInfo.mExtAddress, &mParent.GetExtAddress(), sizeof(parentInfo.mExtAddress)); - SuccessOrExit(error = otPlatSettingsSet(mNetif.GetInstance(), kKeyParentInfo, + SuccessOrExit(error = otPlatSettingsSet(mNetif.GetInstance(), Settings::kKeyParentInfo, reinterpret_cast(&parentInfo), sizeof(parentInfo))); } - SuccessOrExit(error = otPlatSettingsSet(mNetif.GetInstance(), kKeyNetworkInfo, + SuccessOrExit(error = otPlatSettingsSet(mNetif.GetInstance(), Settings::kKeyNetworkInfo, reinterpret_cast(&networkInfo), sizeof(networkInfo))); mNetif.GetKeyManager().SetStoredMleFrameCounter(networkInfo.mMleFrameCounter); diff --git a/src/core/thread/mle.hpp b/src/core/thread/mle.hpp index 703e5b58d..d6ea4157e 100644 --- a/src/core/thread/mle.hpp +++ b/src/core/thread/mle.hpp @@ -121,33 +121,6 @@ enum AlocAllocation kAloc16NeighborDiscoveryAgentEnd = 0xfc4e, }; -/** -* This structure represents the device's own network information for persistent storage. -* -*/ -struct NetworkInfo -{ - DeviceState mDeviceState; ///< Current Thread interface state. - - uint8_t mDeviceMode; ///< Device mode setting. - uint16_t mRloc16; ///< RLOC16 - uint32_t mKeySequence; ///< Key Sequence - uint32_t mMleFrameCounter; ///< MLE Frame Counter - uint32_t mMacFrameCounter; ///< MAC Frame Counter - uint32_t mPreviousPartitionId; ///< PartitionId - Mac::ExtAddress mExtAddress; ///< Extended Address - uint8_t mMlIid[OT_IP6_ADDRESS_SIZE - OT_IP6_PREFIX_SIZE]; ///< IID from ML-EID -}; - -/** -* This structure represents the parent information for persistent storage. -* -*/ -struct ParentInfo -{ - Mac::ExtAddress mExtAddress; ///< Extended Address -}; - /** * This class implements MLE Header generation and parsing. * diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 444bcb480..694baea06 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -3463,11 +3463,11 @@ ThreadError MleRouter::RestoreChildren(void) for (uint8_t i = 0; ; i++) { Child *child; - ChildInfo childInfo; + Settings::ChildInfo childInfo; uint16_t length; length = sizeof(childInfo); - SuccessOrExit(error = otPlatSettingsGet(mNetif.GetInstance(), kKeyChildInfo, i, + SuccessOrExit(error = otPlatSettingsGet(mNetif.GetInstance(), Settings::kKeyChildInfo, i, reinterpret_cast(&childInfo), &length)); VerifyOrExit(length >= sizeof(childInfo), error = kThreadError_Parse); @@ -3493,16 +3493,16 @@ ThreadError MleRouter::RemoveStoredChild(uint16_t aChildRloc16) for (uint8_t i = 0; i < kMaxChildren; i++) { - ChildInfo childInfo; + Settings::ChildInfo childInfo; uint16_t length = sizeof(childInfo); - SuccessOrExit(error = otPlatSettingsGet(mNetif.GetInstance(), kKeyChildInfo, i, + SuccessOrExit(error = otPlatSettingsGet(mNetif.GetInstance(), Settings::kKeyChildInfo, i, reinterpret_cast(&childInfo), &length)); VerifyOrExit(length == sizeof(childInfo), error = kThreadError_Parse); if (childInfo.mRloc16 == aChildRloc16) { - error = otPlatSettingsDelete(mNetif.GetInstance(), kKeyChildInfo, i); + error = otPlatSettingsDelete(mNetif.GetInstance(), Settings::kKeyChildInfo, i); ExitNow(); } } @@ -3515,7 +3515,7 @@ ThreadError MleRouter::StoreChild(uint16_t aChildRloc16) { ThreadError error = kThreadError_None; Child *child; - ChildInfo childInfo; + Settings::ChildInfo childInfo; VerifyOrExit((child = FindChild(GetChildId(aChildRloc16))) != NULL, error = kThreadError_NotFound); @@ -3528,7 +3528,7 @@ ThreadError MleRouter::StoreChild(uint16_t aChildRloc16) childInfo.mRloc16 = child->GetRloc16(); childInfo.mMode = child->GetDeviceMode(); - error = otPlatSettingsAdd(mNetif.GetInstance(), kKeyChildInfo, reinterpret_cast(&childInfo), + error = otPlatSettingsAdd(mNetif.GetInstance(), Settings::kKeyChildInfo, reinterpret_cast(&childInfo), sizeof(childInfo)); exit: @@ -3539,7 +3539,7 @@ ThreadError MleRouter::RefreshStoredChildren(void) { ThreadError error = kThreadError_None; - SuccessOrExit(error = otPlatSettingsDelete(mNetif.GetInstance(), kKeyChildInfo, -1)); + SuccessOrExit(error = otPlatSettingsDelete(mNetif.GetInstance(), Settings::kKeyChildInfo, -1)); for (uint8_t i = 0; i < kMaxChildren; i++) { diff --git a/src/core/thread/mle_router_ftd.hpp b/src/core/thread/mle_router_ftd.hpp index 091081a52..a72d1d65d 100644 --- a/src/core/thread/mle_router_ftd.hpp +++ b/src/core/thread/mle_router_ftd.hpp @@ -66,18 +66,6 @@ class NetworkDataLeader; * @{ */ -/** -* This structure represents the child information for persistent storage. -* -*/ -struct ChildInfo -{ - Mac::ExtAddress mExtAddress; ///< Extended Address - uint32_t mTimeout; ///< Timeout - uint16_t mRloc16; ///< RLOC16 - uint8_t mMode; ///< The MLE device mode -}; - /** * This class implements MLE functionality required by the Thread Router and Leader roles. *