From 130936579e522a62f458e48ba968c5349727e7fc Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 25 Jan 2018 10:05:45 -0800 Subject: [PATCH] [child] enhance how mesh-local IPv6 address is stored in a child entry (#2504) This commit changes how a mesh-local IPv6 address is stored in `Child` class. Instead of storing the entire IPv6 addresses (16 bytes), only the Interface Identifier (8 bytes) is saved. The full address is derived (when needed) using the mesh-local prefix from `Mle`. This reduces the memory required for storing registered IPv6 addresses in the child table. This commit also updates the unit test `test_child` to use mesh-local IPv6 address as part of the test and verify the behavior of new implementation. --- src/core/net/ip6_address.cpp | 1 - src/core/net/ip6_address.hpp | 1 + src/core/thread/address_resolver.cpp | 4 +- src/core/thread/mle_router.cpp | 10 +-- src/core/thread/topology.cpp | 91 ++++++++++++++++++++++++---- src/core/thread/topology.hpp | 59 +++++++++++------- tests/unit/test_child.cpp | 51 ++++++++++------ 7 files changed, 154 insertions(+), 63 deletions(-) diff --git a/src/core/net/ip6_address.cpp b/src/core/net/ip6_address.cpp index 131e9141f..a60e0082a 100644 --- a/src/core/net/ip6_address.cpp +++ b/src/core/net/ip6_address.cpp @@ -141,7 +141,6 @@ bool Address::IsIidReserved(void) const return IsSubnetRouterAnycast() || IsReservedSubnetAnycast() || IsAnycastRoutingLocator(); } - const uint8_t *Address::GetIid(void) const { return mFields.m8 + kInterfaceIdentifierOffset; diff --git a/src/core/net/ip6_address.hpp b/src/core/net/ip6_address.hpp index c12dd516f..2333f6dbd 100644 --- a/src/core/net/ip6_address.hpp +++ b/src/core/net/ip6_address.hpp @@ -79,6 +79,7 @@ public: kInterfaceIdentifierSize = 8, ///< Interface Identifier size in bytes. kIp6AddressStringSize = 40, ///< Max buffer size in bytes to store an IPv6 address in string format. kMeshLocalPrefixLength = 64, ///< Length of Thread mesh local prefix. + kMeshLocalPrefixSize = 8, ///< Mesh local prefix size in bytes. }; /** diff --git a/src/core/thread/address_resolver.cpp b/src/core/thread/address_resolver.cpp index 58097e581..fd85a9cd7 100644 --- a/src/core/thread/address_resolver.cpp +++ b/src/core/thread/address_resolver.cpp @@ -593,7 +593,7 @@ void AddressResolver::HandleAddressError(Coap::Header &aHeader, Message &aMessag // Mesh Local EID differs, so check whether Target EID // matches a child address and if so remove it. - if (child.RemoveIp6Address(targetTlv.GetTarget()) == OT_ERROR_NONE) + if (child.RemoveIp6Address(GetInstance(), targetTlv.GetTarget()) == OT_ERROR_NONE) { memset(&destination, 0, sizeof(destination)); destination.mFields.m16[0] = HostSwap16(0xfe80); @@ -669,7 +669,7 @@ void AddressResolver::HandleAddressQuery(Coap::Header &aHeader, Message &aMessag continue; } - if (child.HasIp6Address(targetTlv.GetTarget())) + if (child.HasIp6Address(GetInstance(), targetTlv.GetTarget())) { mlIidTlv.SetIid(child.GetExtAddress()); lastTransactionTimeTlv.SetTime(TimerMilli::GetNow() - child.GetLastHeard()); diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 7922522f7..cb9aadea6 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -2116,7 +2116,7 @@ otError MleRouter::UpdateChildAddresses(const AddressRegistrationTlv &aTlv, Chil // "Child ID/Update Response" will indicate the accepted // addresses. - error = aChild.AddIp6Address(address); + error = aChild.AddIp6Address(GetInstance(), address); if (error == OT_ERROR_NONE) { @@ -2149,7 +2149,7 @@ otError MleRouter::UpdateChildAddresses(const AddressRegistrationTlv &aTlv, Chil continue; } - IgnoreReturnValue(child.RemoveIp6Address(address)); + IgnoreReturnValue(child.RemoveIp6Address(GetInstance(), address)); } } @@ -3545,7 +3545,7 @@ Neighbor *MleRouter::GetNeighbor(const Ip6::Address &aAddress) ExitNow(rval = child); } - if (child->HasIp6Address(aAddress)) + if (child->HasIp6Address(GetInstance(), aAddress)) { ExitNow(rval = child); } @@ -3788,7 +3788,7 @@ otError MleRouter::GetChildNextIp6Address(uint8_t aChildIndex, Child::Ip6Address VerifyOrExit(aChildIndex < mMaxChildrenAllowed, error = OT_ERROR_INVALID_ARGS); VerifyOrExit(mChildren[aChildIndex].IsStateValidOrRestoring(), error = OT_ERROR_INVALID_ARGS); - error = mChildren[aChildIndex].GetNextIp6Address(aIterator, aAddress); + error = mChildren[aChildIndex].GetNextIp6Address(GetInstance(), aIterator, aAddress); exit: return error; @@ -4694,7 +4694,7 @@ otError MleRouter::AppendChildAddresses(Message &aMessage, Child &aChild) tlv.SetType(Tlv::kAddressRegistration); SuccessOrExit(error = aMessage.Append(&tlv, sizeof(tlv))); - while (aChild.GetNextIp6Address(iterator, address) == OT_ERROR_NONE) + while (aChild.GetNextIp6Address(GetInstance(), iterator, address) == OT_ERROR_NONE) { if (netif.GetNetworkDataLeader().GetContext(address, context) == OT_ERROR_NONE) { diff --git a/src/core/thread/topology.cpp b/src/core/thread/topology.cpp index dd297f43c..7ddbc398a 100644 --- a/src/core/thread/topology.cpp +++ b/src/core/thread/topology.cpp @@ -37,6 +37,7 @@ #include "common/code_utils.hpp" #include "common/debug.hpp" +#include "common/instance.hpp" #include "common/logging.hpp" namespace ot { @@ -51,29 +52,78 @@ void Neighbor::GenerateChallenge(void) void Child::ClearIp6Addresses(void) { + memset(mMeshLocalIid, 0, sizeof(mMeshLocalIid)); memset(mIp6Address, 0, sizeof(mIp6Address)); } -otError Child::GetNextIp6Address(Ip6AddressIterator &aIterator, Ip6::Address &aAddress) const +/** + * Determines if all elements in an array are zero. + * + * @param[in] aArray A pointer to an array of bytes. + * @param[in] aLength Array length (number of bytes). + * + * @returns TRUE if all bytes in the array are zero, FALSE otherwise. + * + */ +static bool IsAllZero(const uint8_t *aArray, uint8_t aLength) +{ + bool retval = true; + + for (; aLength != 0; aArray++, aLength--) + { + VerifyOrExit(*aArray == 0, retval = false); + } + +exit: + return retval; +} + +otError Child::GetNextIp6Address(Instance &aInstance, Ip6AddressIterator &aIterator, Ip6::Address &aAddress) const { otError error = OT_ERROR_NONE; + otChildIp6AddressIterator index; - VerifyOrExit(aIterator.Get() < kMaxIp6AddressPerChild, error = OT_ERROR_NOT_FOUND); - VerifyOrExit(!mIp6Address[aIterator.Get()].IsUnspecified(), error = OT_ERROR_NOT_FOUND); - aAddress = mIp6Address[aIterator.Get()]; + // Index zero corresponds to the Mesh Local IPv6 address (if any). + + if (aIterator.Get() == 0) + { + aIterator.Increment(); + + if (!IsAllZero(mMeshLocalIid, sizeof(mMeshLocalIid))) + { + memcpy(aAddress.mFields.m8, aInstance.GetThreadNetif().GetMle().GetMeshLocalPrefix(), + Ip6::Address::kMeshLocalPrefixSize); + aAddress.SetIid(mMeshLocalIid); + ExitNow(); + } + } + + index = aIterator.Get() - 1; + + VerifyOrExit(index < kNumIp6Addresses, error = OT_ERROR_NOT_FOUND); + + VerifyOrExit(!mIp6Address[index].IsUnspecified(), error = OT_ERROR_NOT_FOUND); + aAddress = mIp6Address[index]; aIterator.Increment(); exit: return error; } -otError Child::AddIp6Address(const Ip6::Address &aAddress) +otError Child::AddIp6Address(Instance &aInstance, const Ip6::Address &aAddress) { otError error = OT_ERROR_NONE; VerifyOrExit(!aAddress.IsUnspecified(), error = OT_ERROR_INVALID_ARGS); - for (uint16_t index = 0; index < kMaxIp6AddressPerChild; index++) + if (aInstance.GetThreadNetif().GetMle().IsMeshLocalAddress(aAddress)) + { + VerifyOrExit(IsAllZero(mMeshLocalIid, sizeof(mMeshLocalIid)), error = OT_ERROR_ALREADY); + memcpy(mMeshLocalIid, aAddress.GetIid(), Ip6::Address::kInterfaceIdentifierSize); + ExitNow(); + } + + for (uint16_t index = 0; index < kNumIp6Addresses; index++) { if (mIp6Address[index].IsUnspecified()) { @@ -90,14 +140,25 @@ exit: return error; } -otError Child::RemoveIp6Address(const Ip6::Address &aAddress) +otError Child::RemoveIp6Address(Instance &aInstance, const Ip6::Address &aAddress) { otError error = OT_ERROR_NOT_FOUND; uint16_t index; VerifyOrExit(!aAddress.IsUnspecified(), error = OT_ERROR_INVALID_ARGS); - for (index = 0; index < kMaxIp6AddressPerChild; index++) + if (aInstance.GetThreadNetif().GetMle().IsMeshLocalAddress(aAddress)) + { + if (memcmp(aAddress.GetIid(), mMeshLocalIid, Ip6::Address::kInterfaceIdentifierSize) == 0) + { + memset(mMeshLocalIid, 0, sizeof(mMeshLocalIid)); + error = OT_ERROR_NONE; + } + + ExitNow(); + } + + for (index = 0; index < kNumIp6Addresses; index++) { VerifyOrExit(!mIp6Address[index].IsUnspecified()); @@ -110,24 +171,30 @@ otError Child::RemoveIp6Address(const Ip6::Address &aAddress) SuccessOrExit(error); - for (; index < kMaxIp6AddressPerChild - 1; index++) + for (; index < kNumIp6Addresses - 1; index++) { mIp6Address[index] = mIp6Address[index + 1]; } - mIp6Address[kMaxIp6AddressPerChild - 1].Clear(); + mIp6Address[kNumIp6Addresses - 1].Clear(); exit: return error; } -bool Child::HasIp6Address(const Ip6::Address &aAddress) const +bool Child::HasIp6Address(Instance &aInstance, const Ip6::Address &aAddress) const { bool retval = false; VerifyOrExit(!aAddress.IsUnspecified()); - for (uint16_t index = 0; index < kMaxIp6AddressPerChild; index++) + if (aInstance.GetThreadNetif().GetMle().IsMeshLocalAddress(aAddress)) + { + retval = (memcmp(aAddress.GetIid(), mMeshLocalIid, Ip6::Address::kInterfaceIdentifierSize) == 0); + ExitNow(); + } + + for (uint16_t index = 0; index < kNumIp6Addresses; index++) { VerifyOrExit(!mIp6Address[index].IsUnspecified()); diff --git a/src/core/thread/topology.hpp b/src/core/thread/topology.hpp index 5ed5b14ed..386df70ab 100644 --- a/src/core/thread/topology.hpp +++ b/src/core/thread/topology.hpp @@ -46,6 +46,8 @@ namespace ot { +class Instance; + /** * This class represents a Thread neighbor. * @@ -421,50 +423,54 @@ public: /** * This method gets the next IPv6 address in the list. * - * @param[inout] aIterator A reference to an IPv6 address iterator. - * @param[out] aAddress A reference to an IPv6 address to provide the next address (if any). + * @param[in] aInstance A reference to the OpenThread instance. + * @param[inout] aIterator A reference to an IPv6 address iterator. + * @param[out] aAddress A reference to an IPv6 address to provide the next address (if any). * - * @retval OT_ERROR_NONE Successfully found the next address and updated @p aAddress and @p aIterator. - * @retval OT_ERROR_NOT_FOUND No subsequent IPv6 address exists in the IPv6 address list. + * @retval OT_ERROR_NONE Successfully found the next address and updated @p aAddress and @p aIterator. + * @retval OT_ERROR_NOT_FOUND No subsequent IPv6 address exists in the IPv6 address list. * */ - otError GetNextIp6Address(Ip6AddressIterator &aIterator, Ip6::Address &aAddress) const; + otError GetNextIp6Address(Instance &aInstance, Ip6AddressIterator &aIterator, Ip6::Address &aAddress) const; /** * This method adds an IPv6 address to the list. * - * @param[in] aAddress A reference to IPv6 address to be added. + * @param[in] aInstance A reference to the OpenThread instance. + * @param[in] aAddress A reference to IPv6 address to be added. * - * @retval OT_ERROR_NONE Successfully added the new address. - * @retval OT_ERROR_ALREADY Address is already in the list. - * @retval OT_ERROR_NO_BUFS Already at maximum number of addresses. No entry available to add the new address. - * @retval OT_ERROR_INVALID_ARGS Address is invalid (it is the Unspecified Address). + * @retval OT_ERROR_NONE Successfully added the new address. + * @retval OT_ERROR_ALREADY Address is already in the list. + * @retval OT_ERROR_NO_BUFS Already at maximum number of addresses. No entry available to add the new address. + * @retval OT_ERROR_INVALID_ARGS Address is invalid (it is the Unspecified Address). * */ - otError AddIp6Address(const Ip6::Address &aAddress); + otError AddIp6Address(Instance &aInstance, const Ip6::Address &aAddress); /** * This method removes an IPv6 address from the list. * - * @param[in] aAddress A reference to IPv6 address to be removed. + * @param[in] aInstance A reference to the OpenThread instance. + * @param[in] aAddress A reference to IPv6 address to be removed. * - * @retval OT_ERROR_NONE Successfully removed the address. - * @retval OT_ERROR_NOT_FOUND Address was not found in the list. - * @retval OT_ERROR_INVALID_ARGS Address is invalid (it is the Unspecified Address). + * @retval OT_ERROR_NONE Successfully removed the address. + * @retval OT_ERROR_NOT_FOUND Address was not found in the list. + * @retval OT_ERROR_INVALID_ARGS Address is invalid (it is the Unspecified Address). * */ - otError RemoveIp6Address(const Ip6::Address &aAddress); + otError RemoveIp6Address(Instance &aInstance, const Ip6::Address &aAddress); /** * This method indicates whether an IPv6 address is in the list of IPv6 addresses of the child. * - * @param[in] aAddress A reference to IPv6 address. + * @param[in] aInstance A reference to the OpenThread instance. + * @param[in] aAddress A reference to IPv6 address. * - * @retval TRUE The address exists on the list. - * @retval FALSE Address was not found in the list. + * @retval TRUE The address exists on the list. + * @retval FALSE Address was not found in the list. * */ - bool HasIp6Address(const Ip6::Address &aAddress) const; + bool HasIp6Address(Instance &aInstance, const Ip6::Address &aAddress) const; /** * This method gets the child timeout. @@ -738,13 +744,20 @@ public: #endif // #if OPENTHREAD_ENABLE_CHILD_SUPERVISION private: + +#if OPENTHREAD_CONFIG_IP_ADDRS_PER_CHILD < 2 +#error OPENTHREAD_CONFIG_IP_ADDRS_PER_CHILD should be at least set to 2. +#endif + enum { - kMaxIp6AddressPerChild = OPENTHREAD_CONFIG_IP_ADDRS_PER_CHILD, + kNumIp6Addresses = OPENTHREAD_CONFIG_IP_ADDRS_PER_CHILD - 1, }; - Ip6::Address mIp6Address[kMaxIp6AddressPerChild]; ///< Registered IPv6 addresses - uint32_t mTimeout; ///< Child timeout + uint8_t mMeshLocalIid[Ip6::Address::kInterfaceIdentifierSize]; ///< IPv6 address IID for mesh-local address + Ip6::Address mIp6Address[kNumIp6Addresses]; ///< Registered IPv6 addresses + + uint32_t mTimeout; ///< Child timeout union { diff --git a/tests/unit/test_child.cpp b/tests/unit/test_child.cpp index 04ae4dc88..3de48cecf 100644 --- a/tests/unit/test_child.cpp +++ b/tests/unit/test_child.cpp @@ -41,7 +41,7 @@ static ot::Instance *sInstance; enum { - kMaxChildIp6Addresses = OPENTHREAD_CONFIG_IP_ADDRS_PER_CHILD + kMaxChildIp6Addresses = OPENTHREAD_CONFIG_IP_ADDRS_PER_CHILD, }; void VerifyChildIp6Addresses(const Child &aChild, uint8_t aAddressListLength, const Ip6::Address aAddressList[]) @@ -52,12 +52,12 @@ void VerifyChildIp6Addresses(const Child &aChild, uint8_t aAddressListLength, co for (uint8_t index = 0; index < aAddressListLength; index++) { - VerifyOrQuit(aChild.HasIp6Address(aAddressList[index]), "HasIp6Address() failed\n"); + VerifyOrQuit(aChild.HasIp6Address(*sInstance, aAddressList[index]), "HasIp6Address() failed\n"); } memset(addressObserved, 0, sizeof(addressObserved)); - while (aChild.GetNextIp6Address(iterator, address) == OT_ERROR_NONE) + while (aChild.GetNextIp6Address(*sInstance, iterator, address) == OT_ERROR_NONE) { bool addressIsInList = false; @@ -84,15 +84,18 @@ void TestChildIp6Address(void) { Child child; Ip6::Address addresses[kMaxChildIp6Addresses]; + uint8_t numAddresses; const char *ip6Addresses[] = { - "fe80::1234", + "fd00:1234::1234", "fd6b:e251:52fb:0:12e6:b94c:1c28:c56a", "fd00:1234::204c:3d7c:98f6:9a1b", - "fd00:cafe::204c:3d7c:98f6:9a1b", }; - uint8_t numAddresses = static_cast(sizeof(ip6Addresses) / sizeof(ip6Addresses[0])); + const uint8_t meshLocalIid[] = + { + 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88 + }; sInstance = testInitInstance(); VerifyOrQuit(sInstance != NULL, "Null instance"); @@ -101,10 +104,18 @@ void TestChildIp6Address(void) printf("\nConverting IPv6 addresses from string"); - for (uint8_t index = 0; index < numAddresses; index++) + numAddresses = 0; + + // First addresses uses the mesh local prefix (mesh-local address). + addresses[numAddresses] = sInstance->GetThreadNetif().GetMle().GetMeshLocal64(); + addresses[numAddresses].SetIid(meshLocalIid); + + numAddresses++; + + for (uint8_t index = 0; index < static_cast(sizeof(ip6Addresses) / sizeof(ip6Addresses[0])); index++) { - VerifyOrQuit(index < kMaxChildIp6Addresses, "Too many IPv6 addresses in the unit test"); - SuccessOrQuit(addresses[index].FromString(ip6Addresses[index]), + VerifyOrQuit(numAddresses < kMaxChildIp6Addresses, "Too many IPv6 addresses in the unit test"); + SuccessOrQuit(addresses[numAddresses++].FromString(ip6Addresses[index]), "could not convert IPv6 address from string"); } @@ -121,7 +132,7 @@ void TestChildIp6Address(void) for (uint8_t index = 0; index < numAddresses; index++) { - SuccessOrQuit(child.AddIp6Address(addresses[index]), "AddIp6Address() failed"); + SuccessOrQuit(child.AddIp6Address(*sInstance, addresses[index]), "AddIp6Address() failed"); VerifyChildIp6Addresses(child, 1, &addresses[index]); child.ClearIp6Addresses(); @@ -135,7 +146,7 @@ void TestChildIp6Address(void) for (uint8_t index = 0; index < numAddresses; index++) { - SuccessOrQuit(child.AddIp6Address(addresses[index]), "AddIp6Address() failed"); + SuccessOrQuit(child.AddIp6Address(*sInstance, addresses[index]), "AddIp6Address() failed"); VerifyChildIp6Addresses(child, index + 1, addresses); } @@ -146,7 +157,7 @@ void TestChildIp6Address(void) for (uint8_t index = 0; index < numAddresses; index++) { - VerifyOrQuit(child.AddIp6Address(addresses[index]) == OT_ERROR_ALREADY, + VerifyOrQuit(child.AddIp6Address(*sInstance, addresses[index]) == OT_ERROR_ALREADY, "AddIp6Address() did not fail when adding same address"); VerifyChildIp6Addresses(child, numAddresses, addresses); } @@ -158,10 +169,10 @@ void TestChildIp6Address(void) for (uint8_t index = 0; index < numAddresses; index++) { - SuccessOrQuit(child.RemoveIp6Address(addresses[index]), "RemoveIp6Address() failed"); + SuccessOrQuit(child.RemoveIp6Address(*sInstance, addresses[index]), "RemoveIp6Address() failed"); VerifyChildIp6Addresses(child, numAddresses - 1 - index, &addresses[index + 1]); - VerifyOrQuit(child.RemoveIp6Address(addresses[index]) == OT_ERROR_NOT_FOUND, + VerifyOrQuit(child.RemoveIp6Address(*sInstance, addresses[index]) == OT_ERROR_NOT_FOUND, "RemoveIp6Address() did not fail when removing an address not on the list"); } @@ -173,15 +184,15 @@ void TestChildIp6Address(void) for (uint8_t index = 0; index < numAddresses; index++) { - SuccessOrQuit(child.AddIp6Address(addresses[index]), "AddIp6Address() failed"); + SuccessOrQuit(child.AddIp6Address(*sInstance, addresses[index]), "AddIp6Address() failed"); } for (uint8_t index = numAddresses - 1; index > 0; index--) { - SuccessOrQuit(child.RemoveIp6Address(addresses[index]), "RemoveIp6Address() failed"); + SuccessOrQuit(child.RemoveIp6Address(*sInstance, addresses[index]), "RemoveIp6Address() failed"); VerifyChildIp6Addresses(child, index, &addresses[0]); - VerifyOrQuit(child.RemoveIp6Address(addresses[index]) == OT_ERROR_NOT_FOUND, + VerifyOrQuit(child.RemoveIp6Address(*sInstance, addresses[index]) == OT_ERROR_NOT_FOUND, "RemoveIp6Address() did not fail when removing an address not on the list"); } @@ -196,12 +207,12 @@ void TestChildIp6Address(void) for (uint8_t index = 0; index < numAddresses; index++) { - SuccessOrQuit(child.AddIp6Address(addresses[index]), "AddIp6Address() failed"); + SuccessOrQuit(child.AddIp6Address(*sInstance, addresses[index]), "AddIp6Address() failed"); } - SuccessOrQuit(child.RemoveIp6Address(addresses[indexToRemove]), "RemoveIp6Address() failed"); + SuccessOrQuit(child.RemoveIp6Address(*sInstance, addresses[indexToRemove]), "RemoveIp6Address() failed"); - VerifyOrQuit(child.RemoveIp6Address(addresses[indexToRemove]) == OT_ERROR_NOT_FOUND, + VerifyOrQuit(child.RemoveIp6Address(*sInstance, addresses[indexToRemove]) == OT_ERROR_NOT_FOUND, "RemoveIp6Address() did not fail when removing an address not on the list"); {