From c13839a73815b7988c77df0445ee2da56f4ceea3 Mon Sep 17 00:00:00 2001 From: Jiacheng Guo Date: Fri, 4 Feb 2022 06:14:26 +0800 Subject: [PATCH] [binding] remove binding when corresponding fabric removed (#14487) * [binding] remove binding when corresponding fabric removed * Update src/credentials/FabricTable.h Co-authored-by: Boris Zbarsky Co-authored-by: Boris Zbarsky --- src/app/CASESessionManager.cpp | 5 ++ src/app/CASESessionManager.h | 2 + src/app/OperationalDeviceProxyPool.h | 13 +++ src/app/clusters/bindings/BindingManager.cpp | 83 ++++++++++++++----- src/app/clusters/bindings/BindingManager.h | 44 +++++++--- .../operational-credentials-server.cpp | 5 +- src/credentials/FabricTable.cpp | 69 +++++++++------ src/credentials/FabricTable.h | 10 ++- 8 files changed, 167 insertions(+), 64 deletions(-) diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index a183dd424d95a5..047562fabb03f9 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -67,6 +67,11 @@ void CASESessionManager::ReleaseSession(PeerId peerId) ReleaseSession(FindExistingSession(peerId)); } +void CASESessionManager::ReleaseSessionForFabric(CompressedFabricId compressedFabricId) +{ + mConfig.devicePool->ReleaseDeviceForFabric(compressedFabricId); +} + CHIP_ERROR CASESessionManager::ResolveDeviceAddress(FabricInfo * fabric, NodeId nodeId) { VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INCORRECT_STATE); diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index f22bf998736dac..9455646448b807 100644 --- a/src/app/CASESessionManager.h +++ b/src/app/CASESessionManager.h @@ -86,6 +86,8 @@ class CASESessionManager : public Dnssd::ResolverDelegate void ReleaseSession(PeerId peerId); + void ReleaseSessionForFabric(CompressedFabricId compressedFabricId); + /** * This API triggers the DNS-SD resolution for the given node ID. The node ID will be looked up * on the fabric that was configured for the CASESessionManager object. diff --git a/src/app/OperationalDeviceProxyPool.h b/src/app/OperationalDeviceProxyPool.h index bfdc53d72a47ec..aaa46bbf5773c8 100644 --- a/src/app/OperationalDeviceProxyPool.h +++ b/src/app/OperationalDeviceProxyPool.h @@ -37,6 +37,8 @@ class OperationalDeviceProxyPoolDelegate virtual OperationalDeviceProxy * FindDevice(PeerId peerId) = 0; + virtual void ReleaseDeviceForFabric(CompressedFabricId compressedFabricId) = 0; + virtual ~OperationalDeviceProxyPoolDelegate() {} }; @@ -89,6 +91,17 @@ class OperationalDeviceProxyPool : public OperationalDeviceProxyPoolDelegate return foundDevice; } + void ReleaseDeviceForFabric(CompressedFabricId compressedFabricId) override + { + mDevicePool.ForEachActiveObject([&](auto * activeDevice) { + if (activeDevice->GetPeerId().GetCompressedFabricId() == compressedFabricId) + { + Release(activeDevice); + } + return Loop::Continue; + }); + } + private: ObjectPool mDevicePool; }; diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index 8198f77bfc39d9..74ff018c7a8111 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -17,11 +17,48 @@ #include #include +#include + +namespace { + +class BindingFabricTableDelegate : public chip::FabricTableDelegate +{ + void OnFabricDeletedFromStorage(chip::CompressedFabricId compressedFabricId, chip::FabricIndex fabricIndex) + { + for (uint8_t i = 0; i < EMBER_BINDING_TABLE_SIZE; i++) + { + EmberBindingTableEntry entry; + emberGetBinding(i, &entry); + if (entry.fabricIndex == fabricIndex) + { + ChipLogProgress(Zcl, "Remove binding for fabric %d\n", entry.fabricIndex); + entry.type = EMBER_UNUSED_BINDING; + } + } + chip::BindingManager::GetInstance().FabricRemoved(compressedFabricId, fabricIndex); + } + + // Intentionally left blank + void OnFabricRetrievedFromStorage(chip::FabricInfo * fabricInfo) {} + + // Intentionally left blank + void OnFabricPersistedToStorage(chip::FabricInfo * fabricInfo) {} +}; + +BindingFabricTableDelegate gFabricTableDelegate; + +} // namespace namespace chip { BindingManager BindingManager::sBindingManager; +void BindingManager::SetAppServer(Server * appServer) +{ + mAppServer = appServer; + mAppServer->GetFabricTable().AddFabricDelegate(&gFabricTableDelegate); +} + CHIP_ERROR BindingManager::EstablishConnection(FabricIndex fabric, NodeId node) { VerifyOrReturnError(mAppServer != nullptr, CHIP_ERROR_INCORRECT_STATE); @@ -49,17 +86,6 @@ CHIP_ERROR BindingManager::EstablishConnection(FabricIndex fabric, NodeId node) return error; } -CHIP_ERROR BindingManager::EnqueueUnicastNotification(FabricIndex fabric, NodeId node, EndpointId endpoint, ClusterId cluster, - void * context) -{ - VerifyOrReturnError(mAppServer != nullptr, CHIP_ERROR_INCORRECT_STATE); - - FabricInfo * fabricInfo = mAppServer->GetFabricTable().FindFabricWithIndex(fabric); - VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_NOT_FOUND); - PeerId peer = fabricInfo->GetPeerIdForNode(node); - return mPendingNotificationMap.AddPendingNotification(peer, endpoint, cluster, context); -} - void BindingManager::HandleDeviceConnected(void * context, OperationalDeviceProxy * device) { BindingManager * manager = static_cast(context); @@ -110,14 +136,27 @@ void BindingManager::HandleDeviceConnectionFailure(PeerId peerId, CHIP_ERROR err mAppServer->GetCASESessionManager()->ReleaseSession(peerId); } -CHIP_ERROR BindingManager::LastUnicastBindingRemoved(FabricIndex fabric, NodeId node) +void BindingManager::FabricRemoved(CompressedFabricId compressedFabricId, FabricIndex fabricIndex) +{ + mPendingNotificationMap.ForEachActiveObject([&](PendingNotificationEntry * entry) { + if (entry->GetFabricIndex() == fabricIndex) + { + mPendingNotificationMap.RemoveEntry(entry); + return Loop::Break; + } + return Loop::Continue; + }); + mAppServer->GetCASESessionManager()->ReleaseSessionForFabric(compressedFabricId); +} + +CHIP_ERROR BindingManager::LastUnicastBindingRemoved(FabricIndex fabricIndex, NodeId node) { VerifyOrReturnError(mAppServer != nullptr, CHIP_ERROR_INCORRECT_STATE); - FabricInfo * fabricInfo = mAppServer->GetFabricTable().FindFabricWithIndex(fabric); + FabricInfo * fabricInfo = mAppServer->GetFabricTable().FindFabricWithIndex(fabricIndex); VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_NOT_FOUND); PeerId peer = fabricInfo->GetPeerIdForNode(node); - PendingNotificationEntry * entry = mPendingNotificationMap.FindEntry(peer); + PendingNotificationEntry * entry = mPendingNotificationMap.FindEntry(fabricIndex, node); if (entry) { mPendingNotificationMap.RemoveEntry(entry); @@ -152,8 +191,8 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste else { // Enqueue pending cluster and establish connection - ReturnErrorOnFailure( - EnqueueUnicastNotification(entry.fabricIndex, entry.nodeId, entry.local, entry.clusterId, context)); + ReturnErrorOnFailure(mPendingNotificationMap.AddPendingNotification(entry.fabricIndex, entry.nodeId, endpoint, + cluster, context)); ReturnErrorOnFailure(EstablishConnection(entry.fabricIndex, entry.nodeId)); } } @@ -179,11 +218,11 @@ BindingManager::PendingNotificationEntry * BindingManager::PendingNotificationMa return lruEntry; } -BindingManager::PendingNotificationEntry * BindingManager::PendingNotificationMap::FindEntry(PeerId peerId) +BindingManager::PendingNotificationEntry * BindingManager::PendingNotificationMap::FindEntry(FabricIndex fabricIndex, NodeId node) { PendingNotificationEntry * foundEntry = nullptr; mPendingNotificationMap.ForEachActiveObject([&](PendingNotificationEntry * entry) { - if (entry->GetPeerId() == peerId) + if (entry->GetFabricIndex() == fabricIndex && entry->GetNodeId() == node) { foundEntry = entry; return Loop::Break; @@ -193,14 +232,14 @@ BindingManager::PendingNotificationEntry * BindingManager::PendingNotificationMa return foundEntry; } -CHIP_ERROR BindingManager::PendingNotificationMap::AddPendingNotification(PeerId peer, EndpointId endpoint, ClusterId cluster, - void * context) +CHIP_ERROR BindingManager::PendingNotificationMap::AddPendingNotification(FabricIndex fabric, NodeId node, EndpointId endpoint, + ClusterId cluster, void * context) { - PendingNotificationEntry * entry = FindEntry(peer); + PendingNotificationEntry * entry = FindEntry(fabric, node); if (entry == nullptr) { - entry = mPendingNotificationMap.CreateObject(peer); + entry = mPendingNotificationMap.CreateObject(fabric, node); VerifyOrReturnError(entry != nullptr, CHIP_ERROR_NO_MEMORY); } entry->AddPendingNotification(endpoint, cluster, context); diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h index d4b9acd0bafc35..fd0ade9d1f5772 100644 --- a/src/app/clusters/bindings/BindingManager.h +++ b/src/app/clusters/bindings/BindingManager.h @@ -53,6 +53,8 @@ using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry * bindin */ class BindingManager { + friend class PendingNotificationEntry; + public: BindingManager() : mOnConnectedCallback(HandleDeviceConnected, this), mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this) @@ -60,7 +62,7 @@ class BindingManager void RegisterBoundDeviceChangedHandler(BoundDeviceChangedHandler handler) { mBoundDeviceChangedHandler = handler; } - void SetAppServer(Server * appServer) { mAppServer = appServer; } + void SetAppServer(Server * appServer); /* * Notifies the BindingManager that a new unicast binding is created. @@ -68,11 +70,17 @@ class BindingManager */ CHIP_ERROR UnicastBindingCreated(FabricIndex fabric, NodeId node) { return EstablishConnection(fabric, node); } + /* + * Notifies the BindingManager that a fabric is removed from the device + * + */ + void FabricRemoved(CompressedFabricId compressedId, FabricIndex fabricIndex); + /* * Notfies the BindingManager that the **last** unicast binding to a device has been removed. * */ - CHIP_ERROR LastUnicastBindingRemoved(FabricIndex fabric, NodeId node); + CHIP_ERROR LastUnicastBindingRemoved(FabricIndex fabricIndex, NodeId node); /* * Notify a cluster change to **all** bound devices associated with the (endpoint, cluster) tuple. @@ -104,9 +112,26 @@ class BindingManager class PendingNotificationEntry { public: - PendingNotificationEntry(PeerId peerId) : mPeerId(peerId) {} + PendingNotificationEntry(FabricIndex fabricIndex, NodeId node) : mNodeId(node), mFabricIndex(fabricIndex) {} + + PeerId GetPeerId() + { + PeerId peer; + if (BindingManager::GetInstance().mAppServer == nullptr) + { + return peer; + } + FabricInfo * fabric = BindingManager::GetInstance().mAppServer->GetFabricTable().FindFabricWithIndex(mFabricIndex); + if (fabric == nullptr) + { + return peer; + } + return fabric->GetPeerIdForNode(mNodeId); + } + + NodeId GetNodeId() { return mNodeId; } - PeerId GetPeerId() { return mPeerId; } + FabricIndex GetFabricIndex() { return mFabricIndex; } System::Clock::Timestamp GetLastUpdateTime() { return mLastUpdateTime; } void Touch() { mLastUpdateTime = System::SystemClock().GetMonotonicTimestamp(); } @@ -139,13 +164,14 @@ class BindingManager } private: - PeerId mPeerId; System::Clock::Timestamp mLastUpdateTime; // TODO: Make the pending notifications list of binding table indecies and list of contexts ClusterPath mPendingNotifications[kMaxPendingNotifications]; + NodeId mNodeId; uint8_t mNumPendingNotifications = 0; uint8_t mNextToOverride = 0; + FabricIndex mFabricIndex; }; // The pool for all the pending comands. @@ -154,9 +180,10 @@ class BindingManager public: PendingNotificationEntry * FindLRUEntry(); - PendingNotificationEntry * FindEntry(PeerId peerId); + PendingNotificationEntry * FindEntry(FabricIndex fabricIndex, NodeId node); - CHIP_ERROR AddPendingNotification(PeerId peer, EndpointId endpoint, ClusterId cluster, void * context); + CHIP_ERROR AddPendingNotification(FabricIndex fabricIndex, NodeId node, EndpointId endpoint, ClusterId cluster, + void * context); void RemoveEntry(PendingNotificationEntry * entry) { mPendingNotificationMap.ReleaseObject(entry); } @@ -181,9 +208,6 @@ class BindingManager // Called when CASE session is established to a peer device. Will send all the pending commands to the peer. void SyncPendingNotificationsToPeer(OperationalDeviceProxy * device, PendingNotificationEntry * pendingClusters); - // Called when CASE session is not established to a peer device. Will enqueue the command and initialize connection. - CHIP_ERROR EnqueueUnicastNotification(FabricIndex fabric, NodeId node, EndpointId endpoint, ClusterId cluster, void * context); - PendingNotificationMap mPendingNotificationMap; BoundDeviceChangedHandler mBoundDeviceChangedHandler; Server * mAppServer = nullptr; diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp index c113b39ad90f2d..c52a92ed5ffe98 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -284,8 +284,9 @@ class OpCredsFabricTableDelegate : public FabricTableDelegate { // Gets called when a fabric is deleted from KVS store - void OnFabricDeletedFromStorage(FabricIndex fabricId) override + void OnFabricDeletedFromStorage(CompressedFabricId compressedFabricId, FabricIndex fabricId) override { + printf("OpCredsFabricTableDelegate::OnFabricDeletedFromStorage\n"); emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: Fabric 0x%" PRIu8 " was deleted from fabric storage.", fabricId); fabricListChanged(); @@ -335,7 +336,7 @@ void MatterOperationalCredentialsPluginServerInitCallback(void) registerAttributeAccessOverride(&gAttrAccess); - Server::GetInstance().GetFabricTable().SetFabricDelegate(&gFabricDelegate); + Server::GetInstance().GetFabricTable().AddFabricDelegate(&gFabricDelegate); } namespace { diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp index d53d87471daacf..d9c0fcd345efd7 100644 --- a/src/credentials/FabricTable.cpp +++ b/src/credentials/FabricTable.cpp @@ -486,7 +486,12 @@ CHIP_ERROR FabricTable::Store(FabricIndex index) if (err == CHIP_NO_ERROR && mDelegate != nullptr) { ChipLogProgress(Discovery, "Fabric (%d) persisted to storage. Calling OnFabricPersistedToStorage", index); - mDelegate->OnFabricPersistedToStorage(fabric); + FabricTableDelegate * delegate = mDelegate; + while (delegate) + { + delegate->OnFabricPersistedToStorage(fabric); + delegate = delegate->mNext; + } } return err; } @@ -500,11 +505,13 @@ CHIP_ERROR FabricTable::LoadFromStorage(FabricInfo * fabric) ReturnErrorOnFailure(fabric->LoadFromStorage(mStorage)); } - if (mDelegate != nullptr) + FabricTableDelegate * delegate = mDelegate; + while (delegate) { ChipLogProgress(Discovery, "Fabric (%d) loaded from storage. Calling OnFabricRetrievedFromStorage", fabric->GetFabricIndex()); - mDelegate->OnFabricRetrievedFromStorage(fabric); + delegate->OnFabricRetrievedFromStorage(fabric); + delegate = delegate->mNext; } return CHIP_NO_ERROR; } @@ -588,31 +595,31 @@ CHIP_ERROR FabricTable::AddNewFabric(FabricInfo & newFabric, FabricIndex * outpu CHIP_ERROR FabricTable::Delete(FabricIndex index) { - FabricInfo * fabric = nullptr; - CHIP_ERROR err = CHIP_NO_ERROR; - bool fabricIsInitialized = false; - VerifyOrExit(mStorage != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - fabric = FindFabricWithIndex(index); - fabricIsInitialized = fabric != nullptr && fabric->IsInitialized(); - err = FabricInfo::DeleteFromStorage(mStorage, index); // Delete from storage regardless + FabricInfo * fabric = FindFabricWithIndex(index); + bool fabricIsInitialized = fabric != nullptr && fabric->IsInitialized(); + CompressedFabricId compressedFabricId = + fabricIsInitialized ? fabric->GetPeerId().GetCompressedFabricId() : kUndefinedCompressedFabricId; + ReturnErrorOnFailure(FabricInfo::DeleteFromStorage(mStorage, index)); // Delete from storage regardless -exit: - if (err == CHIP_NO_ERROR) + ReleaseFabricIndex(index); + if (mDelegate != nullptr && fabricIsInitialized) { - ReleaseFabricIndex(index); - if (mDelegate != nullptr && fabricIsInitialized) + if (mFabricCount == 0) + { + ChipLogError(Discovery, "!!Trying to delete a fabric, but the current fabric count is already 0"); + } + else { - if (mFabricCount == 0) - { - ChipLogError(Discovery, "!!Trying to delete a fabric, but the current fabric count is already 0"); - } - else - { - mFabricCount--; - } - ChipLogProgress(Discovery, "Fabric (%d) deleted. Calling OnFabricDeletedFromStorage", index); - mDelegate->OnFabricDeletedFromStorage(index); + mFabricCount--; + } + ChipLogProgress(Discovery, "Fabric (%d) deleted. Calling OnFabricDeletedFromStorage", index); + FabricTableDelegate * delegate = mDelegate; + while (delegate) + { + delegate->OnFabricDeletedFromStorage(compressedFabricId, index); + delegate = delegate->mNext; } } if (!fabricIsInitialized) @@ -653,11 +660,19 @@ CHIP_ERROR FabricTable::Init(FabricStorage * storage) return CHIP_NO_ERROR; } -CHIP_ERROR FabricTable::SetFabricDelegate(FabricTableDelegate * delegate) +CHIP_ERROR FabricTable::AddFabricDelegate(FabricTableDelegate * delegate) { VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - mDelegate = delegate; - ChipLogDetail(Discovery, "Set the fabric pairing table delegate"); + for (FabricTableDelegate * iter = mDelegate; iter != nullptr; iter = iter->mNext) + { + if (iter == delegate) + { + return CHIP_NO_ERROR; + } + } + delegate->mNext = mDelegate; + mDelegate = delegate; + ChipLogDetail(Discovery, "Add fabric pairing table delegate"); return CHIP_NO_ERROR; } diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h index 532b25a28ff76f..ee59c580bc3cc8 100644 --- a/src/credentials/FabricTable.h +++ b/src/credentials/FabricTable.h @@ -313,12 +313,14 @@ class DLL_EXPORT FabricInfo // TODO: Reimplement FabricTable to only have one backing store. class DLL_EXPORT FabricTableDelegate { + friend class FabricTable; + public: virtual ~FabricTableDelegate() {} /** * Gets called when a fabric is deleted from KVS store. **/ - virtual void OnFabricDeletedFromStorage(FabricIndex fabricIndex) = 0; + virtual void OnFabricDeletedFromStorage(CompressedFabricId compressedId, FabricIndex fabricIndex) = 0; /** * Gets called when a fabric is loaded into Fabric Table from KVS store. @@ -329,6 +331,9 @@ class DLL_EXPORT FabricTableDelegate * Gets called when a fabric in Fabric Table is persisted to KVS store. **/ virtual void OnFabricPersistedToStorage(FabricInfo * fabricInfo) = 0; + +private: + FabricTableDelegate * mNext = nullptr; }; /** @@ -432,7 +437,7 @@ class DLL_EXPORT FabricTable void Reset(); CHIP_ERROR Init(FabricStorage * storage); - CHIP_ERROR SetFabricDelegate(FabricTableDelegate * delegate); + CHIP_ERROR AddFabricDelegate(FabricTableDelegate * delegate); uint8_t FabricCount() const { return mFabricCount; } @@ -448,7 +453,6 @@ class DLL_EXPORT FabricTable FabricInfo mStates[CHIP_CONFIG_MAX_DEVICE_ADMINS]; FabricStorage * mStorage = nullptr; - // TODO: Fabric table should be backed by a single backing store (attribute store), remove delegate callbacks #6419 FabricTableDelegate * mDelegate = nullptr; FabricIndex mNextAvailableFabricIndex = kMinValidFabricIndex;