From ae34357023b6d0b4f847627d642e914ed242065e Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 1 Jul 2020 14:24:03 -0700 Subject: [PATCH] [netif] misc enhancement (#5168) This commit contains some smaller enhancements in netif: - Add method to check if unicast/multicast address is external or internal (e.g., `IsUnicastAddressExternal`) - Simplify iteration over (external) multicast addresses and remove `GetNextExternalMulticast()`. - Use range-based `for` loop for iteration over external address arrays. - Rename `IsUnicastAddress()` to `HasUnicastAddress()`. - Clarify method documentation specially between internal and external addresses (how the passed-in address parameter is used, i.e., copied into another entry from an address pool for external vs. directly used for internal addresses). --- src/core/meshcop/dtls.cpp | 2 +- src/core/net/ip6.cpp | 2 +- src/core/net/netif.cpp | 81 +++++++---------------- src/core/net/netif.hpp | 99 ++++++++++++++++++++-------- src/core/thread/address_resolver.cpp | 2 +- src/core/thread/dua_manager.cpp | 4 +- src/core/thread/mle.cpp | 43 +++++++----- src/core/thread/mle_router.cpp | 2 +- 8 files changed, 128 insertions(+), 107 deletions(-) diff --git a/src/core/meshcop/dtls.cpp b/src/core/meshcop/dtls.cpp index 0b88f0ca2..ef09d4c2a 100644 --- a/src/core/meshcop/dtls.cpp +++ b/src/core/meshcop/dtls.cpp @@ -180,7 +180,7 @@ void Dtls::HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessageI mPeerAddress.SetPeerPort(aMessageInfo.GetPeerPort()); mPeerAddress.SetIsHostInterface(aMessageInfo.IsHostInterface()); - if (Get().IsUnicastAddress(aMessageInfo.GetSockAddr())) + if (Get().HasUnicastAddress(aMessageInfo.GetSockAddr())) { mPeerAddress.SetSockAddr(aMessageInfo.GetSockAddr()); } diff --git a/src/core/net/ip6.cpp b/src/core/net/ip6.cpp index 11b178ed7..74982e4c5 100644 --- a/src/core/net/ip6.cpp +++ b/src/core/net/ip6.cpp @@ -1206,7 +1206,7 @@ otError Ip6::HandleDatagram(Message &aMessage, Netif *aNetif, const void *aLinkM } else { - if (Get().IsUnicastAddress(header.GetDestination())) + if (Get().HasUnicastAddress(header.GetDestination())) { receive = true; } diff --git a/src/core/net/netif.cpp b/src/core/net/netif.cpp index 901284a4f..23f20ad40 100644 --- a/src/core/net/netif.cpp +++ b/src/core/net/netif.cpp @@ -33,7 +33,6 @@ #include "netif.hpp" -#include "common/code_utils.hpp" #include "common/debug.hpp" #include "common/instance.hpp" #include "common/locator-getters.hpp" @@ -88,15 +87,14 @@ Netif::Netif(Instance &aInstance) , mAddressCallback(nullptr) , mAddressCallbackContext(nullptr) { - for (NetifUnicastAddress *entry = &mExtUnicastAddresses[0]; entry < OT_ARRAY_END(mExtUnicastAddresses); entry++) + for (NetifUnicastAddress &entry : mExtUnicastAddresses) { - entry->MarkAsNotInUse(); + entry.MarkAsNotInUse(); } - for (NetifMulticastAddress *entry = &mExtMulticastAddresses[0]; entry < OT_ARRAY_END(mExtMulticastAddresses); - entry++) + for (NetifMulticastAddress &entry : mExtMulticastAddresses) { - entry->MarkAsNotInUse(); + entry.MarkAsNotInUse(); } } @@ -327,33 +325,9 @@ exit: return; } -otError Netif::GetNextExternalMulticast(uint8_t &aIterator, Address &aAddress) const -{ - otError error = OT_ERROR_NOT_FOUND; - size_t num = OT_ARRAY_LENGTH(mExtMulticastAddresses); - - VerifyOrExit(aIterator < num, OT_NOOP); - - for (uint8_t i = aIterator; i < num; i++) - { - const NetifMulticastAddress &entry = mExtMulticastAddresses[i]; - - if (entry.IsInUse()) - { - aAddress = entry.GetAddress(); - aIterator = i + 1; - ExitNow(error = OT_ERROR_NONE); - } - } - -exit: - return error; -} - otError Netif::SubscribeExternalMulticast(const Address &aAddress) { - otError error = OT_ERROR_NONE; - NetifMulticastAddress *entry; + otError error = OT_ERROR_NONE; NetifMulticastAddress &linkLocalAllRoutersAddress = static_cast( const_cast(kLinkLocalAllRoutersMulticastAddress)); @@ -368,12 +342,12 @@ otError Netif::SubscribeExternalMulticast(const Address &aAddress) VerifyOrExit(cur->GetAddress() != aAddress, error = OT_ERROR_INVALID_ARGS); } - for (entry = &mExtMulticastAddresses[0]; entry < OT_ARRAY_END(mExtMulticastAddresses); entry++) + for (NetifMulticastAddress &entry : mExtMulticastAddresses) { - if (!entry->IsInUse()) + if (!entry.IsInUse()) { - entry->mAddress = aAddress; - mMulticastAddresses.Push(*entry); + entry.mAddress = aAddress; + mMulticastAddresses.Push(entry); Get().Signal(kEventIp6MulticastSubscribed); ExitNow(); } @@ -395,8 +369,7 @@ otError Netif::UnsubscribeExternalMulticast(const Address &aAddress) { if (entry->GetAddress() == aAddress) { - VerifyOrExit((entry >= &mExtMulticastAddresses[0]) && (entry < OT_ARRAY_END(mExtMulticastAddresses)), - error = OT_ERROR_INVALID_ARGS); + VerifyOrExit(IsMulticastAddressExternal(*entry), error = OT_ERROR_INVALID_ARGS); mMulticastAddresses.PopAfter(last); break; @@ -417,12 +390,11 @@ exit: void Netif::UnsubscribeAllExternalMulticastAddresses(void) { - for (NetifMulticastAddress *entry = &mExtMulticastAddresses[0]; entry < OT_ARRAY_END(mExtMulticastAddresses); - entry++) + for (NetifMulticastAddress &entry : mExtMulticastAddresses) { - if (entry->IsInUse()) + if (entry.IsInUse()) { - IgnoreError(UnsubscribeExternalMulticast(entry->GetAddress())); + IgnoreError(UnsubscribeExternalMulticast(entry.GetAddress())); } } } @@ -461,15 +433,13 @@ exit: otError Netif::AddExternalUnicastAddress(const NetifUnicastAddress &aAddress) { - otError error = OT_ERROR_NONE; - NetifUnicastAddress *entry; + otError error = OT_ERROR_NONE; - for (entry = mUnicastAddresses.GetHead(); entry; entry = entry->GetNext()) + for (NetifUnicastAddress *entry = mUnicastAddresses.GetHead(); entry; entry = entry->GetNext()) { if (entry->GetAddress() == aAddress.GetAddress()) { - VerifyOrExit((entry >= &mExtUnicastAddresses[0]) && (entry < OT_ARRAY_END(mExtUnicastAddresses)), - error = OT_ERROR_ALREADY); + VerifyOrExit(IsUnicastAddressExternal(*entry), error = OT_ERROR_ALREADY); entry->mPrefixLength = aAddress.mPrefixLength; entry->mAddressOrigin = aAddress.mAddressOrigin; @@ -481,12 +451,12 @@ otError Netif::AddExternalUnicastAddress(const NetifUnicastAddress &aAddress) VerifyOrExit(!aAddress.GetAddress().IsLinkLocal(), error = OT_ERROR_INVALID_ARGS); - for (entry = &mExtUnicastAddresses[0]; entry < OT_ARRAY_END(mExtUnicastAddresses); entry++) + for (NetifUnicastAddress &entry : mExtUnicastAddresses) { - if (!entry->IsInUse()) + if (!entry.IsInUse()) { - *entry = aAddress; - mUnicastAddresses.Push(*entry); + entry = aAddress; + mUnicastAddresses.Push(entry); Get().Signal(kEventIp6AddressAdded); ExitNow(); } @@ -508,8 +478,7 @@ otError Netif::RemoveExternalUnicastAddress(const Address &aAddress) { if (entry->GetAddress() == aAddress) { - VerifyOrExit((entry >= &mExtUnicastAddresses[0]) && (entry < OT_ARRAY_END(mExtUnicastAddresses)), - error = OT_ERROR_INVALID_ARGS); + VerifyOrExit(IsUnicastAddressExternal(*entry), error = OT_ERROR_INVALID_ARGS); mUnicastAddresses.PopAfter(last); break; @@ -530,16 +499,16 @@ exit: void Netif::RemoveAllExternalUnicastAddresses(void) { - for (NetifUnicastAddress *entry = &mExtUnicastAddresses[0]; entry < OT_ARRAY_END(mExtUnicastAddresses); entry++) + for (NetifUnicastAddress &entry : mExtUnicastAddresses) { - if (entry->IsInUse()) + if (entry.IsInUse()) { - IgnoreError(RemoveExternalUnicastAddress(entry->GetAddress())); + IgnoreError(RemoveExternalUnicastAddress(entry.GetAddress())); } } } -bool Netif::IsUnicastAddress(const Address &aAddress) const +bool Netif::HasUnicastAddress(const Address &aAddress) const { bool rval = false; diff --git a/src/core/net/netif.hpp b/src/core/net/netif.hpp index 3414396db..35613585b 100644 --- a/src/core/net/netif.hpp +++ b/src/core/net/netif.hpp @@ -37,6 +37,7 @@ #include "openthread-core-config.h" #include "common/clearable.hpp" +#include "common/code_utils.hpp" #include "common/linked_list.hpp" #include "common/locator.hpp" #include "common/message.hpp" @@ -187,9 +188,9 @@ public: void SetAddressCallback(otIp6AddressCallback aCallback, void *aCallbackContext); /** - * This method returns a pointer to the list of unicast addresses. + * This method returns a pointer to the head of the linked list of unicast addresses. * - * @returns A pointer to the list of unicast addresses. + * @returns A pointer to the head of the linked list of unicast addresses. * */ const NetifUnicastAddress *GetUnicastAddresses(void) const { return mUnicastAddresses.GetHead(); } @@ -197,6 +198,9 @@ public: /** * This method adds a unicast address to the network interface. * + * This method is intended for addresses internal to OpenThread. The @p aAddress instance is directly added in the + * unicast address linked list. + * * @param[in] aAddress A reference to the unicast address. * */ @@ -205,22 +209,57 @@ public: /** * This method removes a unicast address from the network interface. * + * This method is intended for addresses internal to OpenThread. The @p aAddress instance is removed from the + * unicast address linked list. + * * @param[in] aAddress A reference to the unicast address. * */ void RemoveUnicastAddress(const NetifUnicastAddress &aAddress); /** - * This method indicates whether a unicast address is added to the network interface. + * This method indicates whether or not an address is assigned to the interface. * * @param[in] aAddress A reference to the unicast address. * + * @retval TRUE If @p aAddress is assigned to the network interface, + * @retval FALSE If @p aAddress is not assigned to the network interface. + * + */ + bool HasUnicastAddress(const Address &aAddress) const; + + /** + * This method indicates whether or not a unicast address is assigned to the network interface. + * + * @param[in] aAddress A reference to the unicast address. + * + * @retval TRUE If @p aAddress is assigned to the network interface, + * @retval FALSE If @p aAddress is not assigned to the network interface. + * */ bool HasUnicastAddress(const NetifUnicastAddress &aAddress) const { return mUnicastAddresses.Contains(aAddress); } + /** + * This method indicates whether a unicast address is an external or internal address. + * + * @param[in] aAddress A reference to the unicast address. + * + * @retval TRUE The address is an external address. + * @retval FALSE The address is not an external address (it is an OpenThread internal address). + * + */ + bool IsUnicastAddressExternal(const NetifUnicastAddress &aAddress) const + { + return (&mExtUnicastAddresses[0] <= &aAddress) && (&aAddress < OT_ARRAY_END(mExtUnicastAddresses)); + } + /** * This method adds an external (to OpenThread) unicast address to the network interface. * + * For external address, the @p aAddress instance is not directly used (i.e., it can be temporary). It is copied + * into a local entry (allocated from an internal pool) before being added in the unicast address linked list. + * The maximum number of external addresses is specified by `OPENTHREAD_CONFIG_IP6_MAX_EXT_UCAST_ADDRS`. + * * @param[in] aAddress A reference to the unicast address. * * @retval OT_ERROR_NONE Successfully added (or updated) the unicast address. @@ -249,16 +288,6 @@ public: */ void RemoveAllExternalUnicastAddresses(void); - /** - * This method indicates whether or not an address is assigned to this interface. - * - * @param[in] aAddress A reference to the unicast address. - * - * @returns TRUE if @p aAddress is assigned to this interface, FALSE otherwise. - * - */ - bool IsUnicastAddress(const Address &aAddress) const; - /** * This method indicates whether or not the network interface is subscribed to a multicast address. * @@ -285,16 +314,33 @@ public: void UnsubscribeAllRoutersMulticast(void); /** - * This method returns a pointer to the list of multicast addresses. + * This method returns a pointer to the head of the linked list of multicast addresses. * - * @returns A pointer to the list of multicast addresses. + * @returns A pointer to the head of the linked list of multicast addresses. * */ const NetifMulticastAddress *GetMulticastAddresses(void) const { return mMulticastAddresses.GetHead(); } + /** + * This method indicates whether a multicast address is an external or internal address. + * + * @param[in] aAddress A reference to the multicast address. + * + * @retval TRUE The address is an external address. + * @retval FALSE The address is not an external address (it is an OpenThread internal address). + * + */ + bool IsMulticastAddressExternal(const NetifMulticastAddress &aAddress) const + { + return (&mExtMulticastAddresses[0] <= &aAddress) && (&aAddress < OT_ARRAY_END(mExtMulticastAddresses)); + } + /** * This method subscribes the network interface to a multicast address. * + * This method is intended for addresses internal to OpenThread. The @p aAddress instance is directly added in the + * multicast address linked list. + * * @param[in] aAddress A reference to the multicast address. * */ @@ -303,28 +349,21 @@ public: /** * This method unsubscribes the network interface to a multicast address. * + * This method is intended for addresses internal to OpenThread. The @p aAddress instance is directly removed from + * the multicast address linked list. + * * @param[in] aAddress A reference to the multicast address. * */ void UnsubscribeMulticast(const NetifMulticastAddress &aAddress); - /** - * This method provides the next external multicast address that the network interface subscribed. - * It is used to iterate through the entries of the external multicast address table. - * - * @param[inout] aIterator A reference to the iterator context. To get the first - * external multicast address, it should be set to 0. - * @param[out] aAddress A reference where to place the external multicast address. - * - * @retval OT_ERROR_NONE Successfully found the next external multicast address. - * @retval OT_ERROR_NOT_FOUND No subsequent external multicast address. - * - */ - otError GetNextExternalMulticast(uint8_t &aIterator, Address &aAddress) const; - /** * This method subscribes the network interface to the external (to OpenThread) multicast address. * + * For external address, the @p aAddress instance is not directly used (i.e., it can be temporary). It is copied + * into a local entry (allocated from an internal pool) before being added in the multicast address linked list. + * The maximum number of external addresses is specified by `OPENTHREAD_CONFIG_IP6_MAX_EXT_MCAST_ADDRS`. + * * @param[in] aAddress A reference to the multicast address. * * @retval OT_ERROR_NONE Successfully subscribed to @p aAddress. @@ -350,6 +389,7 @@ public: /** * This method unsubscribes the network interface from all previously added external (to OpenThread) multicast * addresses. + * */ void UnsubscribeAllExternalMulticastAddresses(void); @@ -358,6 +398,7 @@ public: * * @retval TRUE If the multicast promiscuous mode is enabled. * @retval FALSE If the multicast promiscuous mode is disabled. + * */ bool IsMulticastPromiscuousEnabled(void) const { return mMulticastPromiscuous; } diff --git a/src/core/thread/address_resolver.cpp b/src/core/thread/address_resolver.cpp index 4681cc99e..1df61d396 100644 --- a/src/core/thread/address_resolver.cpp +++ b/src/core/thread/address_resolver.cpp @@ -800,7 +800,7 @@ void AddressResolver::HandleAddressQuery(Coap::Message &aMessage, const Ip6::Mes otLogInfoArp("Received address query from 0x%04x for target %s", aMessageInfo.GetPeerAddr().GetLocator(), target.ToString().AsCString()); - if (Get().IsUnicastAddress(target)) + if (Get().HasUnicastAddress(target)) { SendAddressQueryResponse(target, Get().GetMeshLocal64().GetIid(), nullptr, aMessageInfo.GetPeerAddr()); diff --git a/src/core/thread/dua_manager.cpp b/src/core/thread/dua_manager.cpp index 6be910bed..3f814b5cb 100644 --- a/src/core/thread/dua_manager.cpp +++ b/src/core/thread/dua_manager.cpp @@ -131,7 +131,7 @@ otError DuaManager::SetFixedDuaInterfaceIdentifier(const Ip6::InterfaceIdentifie mFixedDuaInterfaceIdentifier = aIid; otLogInfoIp6("Set DUA IID: %s", mFixedDuaInterfaceIdentifier.ToString().AsCString()); - if (Get().IsUnicastAddress(GetDomainUnicastAddress())) + if (Get().HasUnicastAddress(GetDomainUnicastAddress())) { Get().RemoveUnicastAddress(mDomainUnicastAddress); mDomainUnicastAddress.GetAddress().SetIid(mFixedDuaInterfaceIdentifier); @@ -148,7 +148,7 @@ void DuaManager::ClearFixedDuaInterfaceIdentifier(void) VerifyOrExit(IsFixedDuaInterfaceIdentifierSet(), OT_NOOP); if (GetDomainUnicastAddress().HasIid(mFixedDuaInterfaceIdentifier) && - Get().IsUnicastAddress(GetDomainUnicastAddress())) + Get().HasUnicastAddress(GetDomainUnicastAddress())) { Get().RemoveUnicastAddress(mDomainUnicastAddress); diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index b43c248fd..deb2e0c00 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -1226,13 +1226,17 @@ bool Mle::HasUnregisteredAddress(void) if (!IsRxOnWhenIdle()) { - uint8_t iterator = 0; - Ip6::Address address; - // For sleepy end-device, we register any external multicast // addresses. - retval = (Get().GetNextExternalMulticast(iterator, address) == OT_ERROR_NONE); + for (const Ip6::NetifMulticastAddress *address = Get().GetMulticastAddresses(); address != nullptr; + address = address->GetNext()) + { + if (Get().IsMulticastAddressExternal(*address)) + { + ExitNow(retval = true); + } + } } exit: @@ -1248,8 +1252,9 @@ otError Mle::AppendAddressRegistration(Message &aMessage, AddressRegistrationMod uint8_t length = 0; uint8_t counter = 0; uint16_t startOffset = aMessage.GetLength(); - uint8_t iterator = 0; // used to iterate external multicast addresses. - Ip6::Address address; +#if OPENTHREAD_CONFIG_DUA_ENABLE + Ip6::Address domainUnicastAddress; +#endif tlv.SetType(Tlv::kAddressRegistration); SuccessOrExit(error = aMessage.Append(&tlv, sizeof(tlv))); @@ -1266,17 +1271,17 @@ otError Mle::AppendAddressRegistration(Message &aMessage, AddressRegistrationMod #if OPENTHREAD_CONFIG_DUA_ENABLE // Cache Domain Unicast Address. - address = Get().GetDomainUnicastAddress(); + domainUnicastAddress = Get().GetDomainUnicastAddress(); - if (Get().IsUnicastAddress(address)) + if (Get().HasUnicastAddress(domainUnicastAddress)) { - error = Get().GetContext(address, context); + error = Get().GetContext(domainUnicastAddress, context); OT_ASSERT(error == OT_ERROR_NONE); // Prioritize DUA, compressed entry entry.SetContextId(context.mContextId); - entry.SetIid(address.GetIid()); + entry.SetIid(domainUnicastAddress.GetIid()); SuccessOrExit(error = aMessage.Append(&entry, entry.GetLength())); length += entry.GetLength(); counter++; @@ -1293,7 +1298,7 @@ otError Mle::AppendAddressRegistration(Message &aMessage, AddressRegistrationMod #if OPENTHREAD_CONFIG_DUA_ENABLE // Skip DUA that was already appended above. - if (addr->GetAddress() == address) + if (addr->GetAddress() == domainUnicastAddress) { continue; } @@ -1330,19 +1335,25 @@ otError Mle::AppendAddressRegistration(Message &aMessage, AddressRegistrationMod #endif ) { - while (Get().GetNextExternalMulticast(iterator, address) == OT_ERROR_NONE) + for (const Ip6::NetifMulticastAddress *addr = Get().GetMulticastAddresses(); addr != nullptr; + addr = addr->GetNext()) { + if (!Get().IsMulticastAddressExternal(*addr)) + { + continue; + } + #if (OPENTHREAD_CONFIG_THREAD_VERSION >= OT_THREAD_VERSION_1_2) // For Thread 1.2 MED, skip multicast address with scope not // larger than realm local when registering. - if (IsRxOnWhenIdle() && !address.IsMulticastLargerThanRealmLocal()) + if (IsRxOnWhenIdle() && !addr->GetAddress().IsMulticastLargerThanRealmLocal()) { continue; } #endif entry.SetUncompressed(); - entry.SetIp6Address(address); + entry.SetIp6Address(addr->GetAddress()); SuccessOrExit(error = aMessage.Append(&entry, entry.GetLength())); length += entry.GetLength(); @@ -1450,7 +1461,7 @@ void Mle::HandleNotifierEvents(Events aEvents) if (aEvents.ContainsAny(kEventIp6AddressAdded | kEventIp6AddressRemoved)) { - if (!Get().IsUnicastAddress(mMeshLocal64.GetAddress())) + if (!Get().HasUnicastAddress(mMeshLocal64.GetAddress())) { // Mesh Local EID was removed, choose a new one and add it back IgnoreError(Random::Crypto::FillBuffer(mMeshLocal64.GetAddress().mFields.m8 + OT_IP6_PREFIX_SIZE, @@ -3847,7 +3858,7 @@ otError Mle::CheckReachability(uint16_t aMeshDest, Ip6::Header &aIp6Header) { otError error; - if ((aMeshDest != GetRloc16()) || Get().IsUnicastAddress(aIp6Header.GetDestination())) + if ((aMeshDest != GetRloc16()) || Get().HasUnicastAddress(aIp6Header.GetDestination())) { error = OT_ERROR_NONE; } diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 4c2e96a54..4df5a0846 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -3901,7 +3901,7 @@ otError MleRouter::CheckReachability(uint16_t aMeshDest, Ip6::Header &aIp6Header if (aMeshDest == Get().GetShortAddress()) { // mesh destination is this device - if (Get().IsUnicastAddress(aIp6Header.GetDestination())) + if (Get().HasUnicastAddress(aIp6Header.GetDestination())) { // IPv6 destination is this device ExitNow();