[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 <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Feb 22, 2022 · c13839a · c13839a
1 parent d28d344
commit c13839a
Show file tree

Hide file tree

Showing 8 changed files with 167 additions and 64 deletions.
diff --git 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
@@ -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
@@ -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<OperationalDeviceProxy, N> mDevicePool;
 };

diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp
@@ -17,11 +17,48 @@
 
 #include <app/clusters/bindings/BindingManager.h>
 #include <app/util/binding-table.h>
+#include <credentials/FabricTable.h>
+
+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<BindingManager *>(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
@@ -53,26 +53,34 @@ using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry * bindin
  */
 class BindingManager
 {
+    friend class PendingNotificationEntry;
+
 public:
     BindingManager() :
         mOnConnectedCallback(HandleDeviceConnected, this), mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this)
     {}
 
     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.
      *
      */
     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
@@ -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 {