diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 5f91d8165..ab4765afc 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (219) +#define OPENTHREAD_API_VERSION (220) /** * @addtogroup api-instance diff --git a/include/openthread/netdata_publisher.h b/include/openthread/netdata_publisher.h index fa44686be..8b4af3e8d 100644 --- a/include/openthread/netdata_publisher.h +++ b/include/openthread/netdata_publisher.h @@ -192,12 +192,18 @@ void otNetDataUnpublishDnsSrpService(otInstance *aInstance); * * Only stable entries can be published (i.e.,`aConfig.mStable` MUST be TRUE). * + * A subsequent call to this method will replace a previous request for the same prefix. In particular, if the new call + * only changes the flags (e.g., preference level) and the prefix is already added in the Network Data, the change to + * flags is immediately reflected in the Network Data. This ensures that existing entries in the Network Data are not + * abruptly removed. Note that a change in the preference level can potentially later cause the entry to be removed + * from the Network Data after determining there are other nodes that are publishing the same prefix with the same or + * higher preference. + * * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aConfig The on-mesh prefix config to publish (MUST NOT be NULL). * * @retval OT_ERROR_NONE The on-mesh prefix is published successfully. * @retval OT_ERROR_INVALID_ARGS The @p aConfig is not valid (bad prefix, invalid flag combinations, or not stable). - * @retval OT_ERROR_ALREADY An entry with the same prefix is already in the published list. * @retval OT_ERROR_NO_BUFS Could not allocate an entry for the new request. Publisher supports a limited number * of entries (shared between on-mesh prefix and external route) determined by config * `OPENTHREAD_CONFIG_NETDATA_PUBLISHER_MAX_PREFIX_ENTRIES`. @@ -213,12 +219,18 @@ otError otNetDataPublishOnMeshPrefix(otInstance *aInstance, const otBorderRouter * * Only stable entries can be published (i.e.,`aConfig.mStable` MUST be TRUE). * + * A subsequent call to this method will replace a previous request for the same prefix. In particular, if the new call + * only changes the flags (e.g., preference level) and the prefix is already added in the Network Data, the change to + * flags is immediately reflected in the Network Data. This ensures that existing entries in the Network Data are not + * abruptly removed. Note that a change in the preference level can potentially later cause the entry to be removed + * from the Network Data after determining there are other nodes that are publishing the same prefix with the same or + * higher preference. + * * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aConfig The external route config to publish (MUST NOT be NULL). * * @retval OT_ERROR_NONE The external route is published successfully. * @retval OT_ERROR_INVALID_ARGS The @p aConfig is not valid (bad prefix, invalid flag combinations, or not stable). - * @retval OT_ERROR_ALREADY An entry with the same prefix is already in the published list. * @retval OT_ERROR_NO_BUFS Could not allocate an entry for the new request. Publisher supports a limited number * of entries (shared between on-mesh prefix and external route) determined by config * `OPENTHREAD_CONFIG_NETDATA_PUBLISHER_MAX_PREFIX_ENTRIES`. diff --git a/src/core/thread/network_data_publisher.cpp b/src/core/thread/network_data_publisher.cpp index af25ee8f2..8141c760a 100644 --- a/src/core/thread/network_data_publisher.cpp +++ b/src/core/thread/network_data_publisher.cpp @@ -89,13 +89,15 @@ void Publisher::SetPrefixCallback(PrefixCallback aCallback, void *aContext) Error Publisher::PublishOnMeshPrefix(const OnMeshPrefixConfig &aConfig) { - Error error; + Error error = kErrorNone; PrefixEntry *entry; VerifyOrExit(aConfig.IsValid(GetInstance()), error = kErrorInvalidArgs); VerifyOrExit(aConfig.mStable, error = kErrorInvalidArgs); - SuccessOrExit(error = AllocatePrefixEntry(aConfig.GetPrefix(), entry)); + entry = FindOrAllocatePrefixEntry(aConfig.GetPrefix()); + VerifyOrExit(entry != nullptr, error = kErrorNoBufs); + entry->Publish(aConfig); exit: @@ -104,13 +106,15 @@ exit: Error Publisher::PublishExternalRoute(const ExternalRouteConfig &aConfig) { - Error error; + Error error = kErrorNone; PrefixEntry *entry; VerifyOrExit(aConfig.IsValid(GetInstance()), error = kErrorInvalidArgs); VerifyOrExit(aConfig.mStable, error = kErrorInvalidArgs); - SuccessOrExit(error = AllocatePrefixEntry(aConfig.GetPrefix(), entry)); + entry = FindOrAllocatePrefixEntry(aConfig.GetPrefix()); + VerifyOrExit(entry != nullptr, error = kErrorNoBufs); + entry->Publish(aConfig); exit: @@ -145,23 +149,26 @@ exit: return error; } -Error Publisher::AllocatePrefixEntry(const Ip6::Prefix &aPrefix, PrefixEntry *&aEntry) +Publisher::PrefixEntry *Publisher::FindOrAllocatePrefixEntry(const Ip6::Prefix &aPrefix) { - Error error = kErrorNoBufs; + // Returns a matching prefix entry if found, otherwise tries + // to allocate a new entry. - VerifyOrExit(FindMatchingPrefixEntry(aPrefix) == nullptr, error = kErrorAlready); + PrefixEntry *prefixEntry = FindMatchingPrefixEntry(aPrefix); + + VerifyOrExit(prefixEntry == nullptr); for (PrefixEntry &entry : mPrefixEntries) { if (!entry.IsInUse()) { - aEntry = &entry; - ExitNow(error = kErrorNone); + prefixEntry = &entry; + ExitNow(); } } exit: - return error; + return prefixEntry; } Publisher::PrefixEntry *Publisher::FindMatchingPrefixEntry(const Ip6::Prefix &aPrefix) @@ -788,25 +795,50 @@ void Publisher::PrefixEntry::Publish(const OnMeshPrefixConfig &aConfig) { LogInfo("Publishing OnMeshPrefix %s", aConfig.GetPrefix().ToString().AsCString()); - mType = kTypeOnMeshPrefix; - mPrefix = aConfig.GetPrefix(); - mFlags = aConfig.ConvertToTlvFlags(); - - SetState(kToAdd); - - Process(); + Publish(aConfig.GetPrefix(), aConfig.ConvertToTlvFlags(), kTypeOnMeshPrefix); } void Publisher::PrefixEntry::Publish(const ExternalRouteConfig &aConfig) { LogInfo("Publishing ExternalRoute %s", aConfig.GetPrefix().ToString().AsCString()); - mType = kTypeExternalRoute; - mPrefix = aConfig.GetPrefix(); - mFlags = aConfig.ConvertToTlvFlags(); + Publish(aConfig.GetPrefix(), aConfig.ConvertToTlvFlags(), kTypeExternalRoute); +} + +void Publisher::PrefixEntry::Publish(const Ip6::Prefix &aPrefix, uint16_t aNewFlags, Type aNewType) +{ + if (GetState() != kNoEntry) + { + // If this is an existing entry, first we check that there is + // a change in either type or flags. We remove the old entry + // from Network Data if it was added. If the only change is + // to flags (e.g., change to the preference level) and the + // entry was previously added in Network Data, we re-add it + // with the new flags. This ensures that changes to flags are + // immediately reflected in the Network Data. + + State oldState = GetState(); + + VerifyOrExit((mType != aNewType) || (mFlags != aNewFlags)); + + Remove(/* aNextState */ kNoEntry); + + if ((mType == aNewType) && ((oldState == kAdded) || (oldState == kRemoving))) + { + mFlags = aNewFlags; + Add(); + } + } + + VerifyOrExit(GetState() == kNoEntry); + + mType = aNewType; + mPrefix = aPrefix; + mFlags = aNewFlags; SetState(kToAdd); +exit: Process(); } diff --git a/src/core/thread/network_data_publisher.hpp b/src/core/thread/network_data_publisher.hpp index c52283b9b..7d1179095 100644 --- a/src/core/thread/network_data_publisher.hpp +++ b/src/core/thread/network_data_publisher.hpp @@ -214,6 +214,13 @@ public: * * Only stable entries can be published (i.e.,`aConfig.mStable` MUST be `true`). * + * A subsequent call to this method will replace a previous request for the same prefix. In particular if the + * new call only changes the flags (e.g., preference level) and the prefix is already added in the Network Data, + * the change to flags is immediately reflected in the Network Data. This ensures that existing entries in the + * Network Data are not abruptly removed. Note that a change in the preference level can potentially later cause + * the entry to be removed from the Network Data after determining there are other nodes that are publishing the + * same prefix with the same or higher preference. + * * @param[in] aConfig The on-mesh prefix config to publish. * * @retval kErrorNone The on-mesh prefix is published successfully. @@ -232,11 +239,17 @@ public: * * Only stable entries can be published (i.e.,`aConfig.mStable` MUST be `true`). * + * A subsequent call to this method will replace a previous request for the same prefix. In particular if the + * new call only changes the flags (e.g., preference level) and the prefix is already added in the Network Data, + * the change to flags is immediately reflected in the Network Data. This ensures that existing entries in the + * Network Data are not abruptly removed. Note that a change in the preference level can potentially later cause + * the entry to be removed from the Network Data after determining there are other nodes that are publishing the + * same prefix with the same or higher preference. + * * @param[in] aConfig The external route config to publish. * * @retval kErrorNone The external route is published successfully. * @retval kErrorInvalidArgs The @p aConfig is not valid (bad prefix, invalid flag combinations, or not stable). - * @retval kErrorAlready An entry with the same prefix is already in the published list. * @retval kErrorNoBufs Could not allocate an entry for the new request. Publisher supports a limited number * of entries (shared between on-mesh prefix and external route) determined by config * `OPENTHREAD_CONFIG_NETDATA_PUBLISHER_MAX_PREFIX_ENTRIES`. @@ -418,6 +431,7 @@ private: kTypeExternalRoute, }; + void Publish(const Ip6::Prefix &aPrefix, uint16_t aNewFlags, Type aNewType); void Add(void); Error AddOnMeshPrefix(void); Error AddExternalRoute(void); @@ -437,7 +451,7 @@ private: #endif #if OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE - Error AllocatePrefixEntry(const Ip6::Prefix &aPrefix, PrefixEntry *&aEntry); + PrefixEntry * FindOrAllocatePrefixEntry(const Ip6::Prefix &aPrefix); PrefixEntry * FindMatchingPrefixEntry(const Ip6::Prefix &aPrefix); const PrefixEntry *FindMatchingPrefixEntry(const Ip6::Prefix &aPrefix) const; bool IsAPrefixEntry(const Entry &aEntry) const; diff --git a/tests/scripts/thread-cert/test_netdata_publisher.py b/tests/scripts/thread-cert/test_netdata_publisher.py index 3cd2f0c13..98d1ea224 100755 --- a/tests/scripts/thread-cert/test_netdata_publisher.py +++ b/tests/scripts/thread-cert/test_netdata_publisher.py @@ -57,10 +57,10 @@ END_DEV5 = 11 WAIT_TIME = 55 -ON_MESH_PREFIX = 'fd00:1234::/64' +ON_MESH_PREFIX = 'fd00:1234:0:0::/64' ON_MESH_FLAGS = 'paso' -EXTERNAL_ROUTE = 'fd00:abce::/64' +EXTERNAL_ROUTE = 'fd00:abce:0:0::/64' EXTERNAL_FLAGS = 's' ANYCAST_SEQ_NUM = 4 @@ -444,6 +444,8 @@ class NetDataPublisher(thread_cert.TestCase): #--------------------------------------------------------------------------------- # External route + # Publish same external route on all nodes with low preference. + num = 0 for node in nodes: node.netdata_publish_route(EXTERNAL_ROUTE, EXTERNAL_FLAGS, 'low') @@ -452,12 +454,26 @@ class NetDataPublisher(thread_cert.TestCase): routes = leader.get_routes() self.check_num_of_routes(routes, num, 0, 0) - leader.netdata_unpublish_prefix(EXTERNAL_ROUTE) + # Change the preference level of the existing entry on leader to high. + leader.netdata_publish_route(EXTERNAL_ROUTE, EXTERNAL_FLAGS, 'high') self.simulator.go(WAIT_TIME) routes = leader.get_routes() self.check_num_of_routes(routes, num - 1, 0, 1) + # Publish the same prefix on leader as an on-mesh prefix. Make + # sure it is removed from external routes and now seen in the + # prefix list. + + leader.netdata_publish_prefix(EXTERNAL_ROUTE, ON_MESH_FLAGS, 'low') + self.simulator.go(WAIT_TIME) + routes = leader.get_routes() + self.check_num_of_routes(routes, num - 1, 0, 0) + + prefixes = leader.get_prefixes() + print(prefixes) + self.assertIn(EXTERNAL_ROUTE, [prefix.split()[0] for prefix in prefixes]) + if __name__ == '__main__': unittest.main()