diff --git a/examples/chip-tool/commands/discover/Commands.h b/examples/chip-tool/commands/discover/Commands.h index 5f74b42ca82037..e09a837d6f1810 100644 --- a/examples/chip-tool/commands/discover/Commands.h +++ b/examples/chip-tool/commands/discover/Commands.h @@ -36,7 +36,8 @@ class Resolve : public DiscoverCommand, public chip::Dnssd::ResolverDelegate chip::Dnssd::Resolver::Instance().SetResolverDelegate(this); ChipLogProgress(chipTool, "Dnssd: Searching for NodeId: %" PRIx64 " FabricId: %" PRIx64 " ...", remoteId, fabricId); return chip::Dnssd::Resolver::Instance().ResolveNodeId(chip::PeerId().SetNodeId(remoteId).SetCompressedFabricId(fabricId), - chip::Inet::IPAddressType::kAny); + chip::Inet::IPAddressType::kAny, + chip::Dnssd::Resolver::CacheBypass::On); } void OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index b14542272e3414..f5c76239b2a04c 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -69,7 +69,8 @@ void CASESessionManager::ReleaseSession(NodeId nodeId) CHIP_ERROR CASESessionManager::ResolveDeviceAddress(NodeId nodeId) { - return Dnssd::Resolver::Instance().ResolveNodeId(GetFabricInfo()->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny); + return Dnssd::Resolver::Instance().ResolveNodeId(GetFabricInfo()->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny, + Dnssd::Resolver::CacheBypass::On); } void CASESessionManager::OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index fd35d846fe2ec0..c5331f37b53324 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -269,6 +269,13 @@ void DeviceController::OnSessionReleased(SessionHandle session) mCASESessionManager->OnSessionReleased(session); } +void DeviceController::OnFirstMessageDeliveryFailed(const SessionHandle & session) +{ + VerifyOrReturn(mState == State::Initialized, + ChipLogError(Controller, "OnFirstMessageDeliveryFailed was called in incorrect state")); + UpdateDevice(session.GetPeerNodeId()); +} + CHIP_ERROR DeviceController::InitializePairedDeviceList() { CHIP_ERROR err = CHIP_NO_ERROR; @@ -543,6 +550,7 @@ CHIP_ERROR DeviceCommissioner::Init(CommissionerInitParams params) params.systemState->SessionMgr()->RegisterCreationDelegate(*this); params.systemState->SessionMgr()->RegisterReleaseDelegate(*this); + params.systemState->SessionMgr()->RegisterRecoveryDelegate(*this); uint16_t nextKeyID = 0; uint16_t size = sizeof(nextKeyID); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index c3c12300e42b03..598cce01f6362a 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -188,6 +188,7 @@ typedef void (*OnOpenCommissioningWindow)(void * context, NodeId deviceId, CHIP_ * relevant information when the application tries to communicate with the device */ class DLL_EXPORT DeviceController : public SessionReleaseDelegate, + public SessionRecoveryDelegate, #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD public AbstractDnssdDiscoveryController, #endif @@ -379,6 +380,9 @@ class DLL_EXPORT DeviceController : public SessionReleaseDelegate, //////////// SessionReleaseDelegate Implementation /////////////// void OnSessionReleased(SessionHandle session) override; + //////////// SessionRecoveryDelegate Implementation /////////////// + void OnFirstMessageDeliveryFailed(const SessionHandle & session) override; + #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD //////////// ResolverDelegate Implementation /////////////// void OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override; diff --git a/src/controller/tests/TestCommissionableNodeController.cpp b/src/controller/tests/TestCommissionableNodeController.cpp index 1a9fbd8f7dbac8..c1101eca7ebb97 100644 --- a/src/controller/tests/TestCommissionableNodeController.cpp +++ b/src/controller/tests/TestCommissionableNodeController.cpp @@ -34,7 +34,10 @@ class MockResolver : public Resolver CHIP_ERROR Init(chip::Inet::InetLayer * inetLayer) override { return InitStatus; } void Shutdown() override {} void SetResolverDelegate(ResolverDelegate *) override {} - CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override { return ResolveNodeIdStatus; } + CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, Resolver::CacheBypass dnssdCacheBypass) override + { + return ResolveNodeIdStatus; + } CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return FindCommissionersStatus; } CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override { return CHIP_ERROR_NOT_IMPLEMENTED; } diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index 0f78b4c2eef9ae..3d13987061d868 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -2702,6 +2702,15 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; #define CHIP_CONFIG_MAX_SESSION_RELEASE_DELEGATES 2 #endif +/** + * @def CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES + * + * @brief Defines the max number of SessionRecoveryDelegate + */ +#ifndef CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES +#define CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES 3 +#endif + /** * @} */ diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index c6f55c85dc6777..ea94fed4763e47 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -459,17 +459,21 @@ CHIP_ERROR DiscoveryImplPlatform::FinalizeServiceUpdate() return ChipDnssdFinalizeServiceUpdate(); } -CHIP_ERROR DiscoveryImplPlatform::ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) +CHIP_ERROR DiscoveryImplPlatform::ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, + Resolver::CacheBypass dnssdCacheBypass) { ReturnErrorOnFailure(InitImpl()); #if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 - /* see if the entry is cached and use it.... */ - ResolvedNodeData nodeData; - if (sDnssdCache.Lookup(peerId, nodeData) == CHIP_NO_ERROR) + if (dnssdCacheBypass == Resolver::CacheBypass::Off) { - mResolverDelegate->OnNodeIdResolved(nodeData); - return CHIP_NO_ERROR; + /* see if the entry is cached and use it.... */ + ResolvedNodeData nodeData; + if (sDnssdCache.Lookup(peerId, nodeData) == CHIP_NO_ERROR) + { + mResolverDelegate->OnNodeIdResolved(nodeData); + return CHIP_NO_ERROR; + } } #endif diff --git a/src/lib/dnssd/Discovery_ImplPlatform.h b/src/lib/dnssd/Discovery_ImplPlatform.h index 076052ab042bc2..5ac1753493d6ca 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.h +++ b/src/lib/dnssd/Discovery_ImplPlatform.h @@ -49,7 +49,7 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver // Members that implement Resolver interface. void SetResolverDelegate(ResolverDelegate * delegate) override { mResolverDelegate = delegate; } - CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override; + CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, Resolver::CacheBypass dnssdCacheBypass) override; CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index d44c55b9a6fc1b..6512ada84f34f0 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -252,6 +252,12 @@ class ResolverDelegate class Resolver { public: + enum class CacheBypass + { + On, + Off + }; + virtual ~Resolver() {} /** @@ -276,11 +282,15 @@ class Resolver /** * Requests resolution of the given operational node service. * + * If `dnssdCacheBypass` is set to `On` it forces resolution of the given node and bypass option + * of using DNS-SD cache. + * * When the operation succeeds or fails, and a resolver delegate has been registered, * the result of the operation is passed to the delegate's `OnNodeIdResolved` or * `OnNodeIdResolutionFailed` method, respectively. */ - virtual CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) = 0; + virtual CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, + Resolver::CacheBypass dnssdCacheBypass = CacheBypass::Off) = 0; /** * Finds all commissionable nodes matching the given filter. diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 739894d3a55e5f..da09d23722fe82 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -345,7 +345,7 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate CHIP_ERROR Init(chip::Inet::InetLayer * inetLayer) override; void Shutdown() override; void SetResolverDelegate(ResolverDelegate * delegate) override { mDelegate = delegate; } - CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override; + CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, Resolver::CacheBypass dnssdCacheBypass) override; CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; @@ -500,7 +500,7 @@ CHIP_ERROR MinMdnsResolver::BrowseNodes(DiscoveryType type, DiscoveryFilter filt return SendQuery(qname, mdns::Minimal::QType::ANY); } -CHIP_ERROR MinMdnsResolver::ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) +CHIP_ERROR MinMdnsResolver::ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, Resolver::CacheBypass dnssdCacheBypass) { mDiscoveryType = DiscoveryType::kOperational; mActiveResolves.MarkPending(peerId); diff --git a/src/lib/dnssd/Resolver_ImplNone.cpp b/src/lib/dnssd/Resolver_ImplNone.cpp index fc1f0870859ae2..c3c28ac8d138b2 100644 --- a/src/lib/dnssd/Resolver_ImplNone.cpp +++ b/src/lib/dnssd/Resolver_ImplNone.cpp @@ -30,7 +30,7 @@ class NoneResolver : public Resolver void Shutdown() override {} void SetResolverDelegate(ResolverDelegate *) override {} - CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override + CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, Resolver::CacheBypass dnssdCacheBypass) override { ChipLogError(Discovery, "Failed to resolve node ID: dnssd resolving not available"); return CHIP_ERROR_NOT_IMPLEMENTED; diff --git a/src/lib/shell/commands/Dns.cpp b/src/lib/shell/commands/Dns.cpp index 5512b5258f76be..8f8bb31ffeb50d 100644 --- a/src/lib/shell/commands/Dns.cpp +++ b/src/lib/shell/commands/Dns.cpp @@ -124,7 +124,7 @@ CHIP_ERROR ResolveHandler(int argc, char ** argv) peerId.SetCompressedFabricId(strtoull(argv[0], NULL, 10)); peerId.SetNodeId(strtoull(argv[1], NULL, 10)); - return Dnssd::Resolver::Instance().ResolveNodeId(peerId, Inet::IPAddressType::kAny); + return Dnssd::Resolver::Instance().ResolveNodeId(peerId, Inet::IPAddressType::kAny, Dnssd::Resolver::CacheBypass::On); } bool ParseSubType(int argc, char ** argv, Dnssd::DiscoveryFilter & filter) diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index a156f7d489b07b..8594d080bdadda 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -353,6 +354,17 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry) if (err == CHIP_NO_ERROR) { + const ExchangeManager * exchangeMgr = entry->ec->GetExchangeMgr(); + // TODO: investigate why in ReliableMessageMgr::CheckResendApplicationMessageWithPeerExchange unit test released exchange + // context with mExchangeMgr==nullptr is used. + if (exchangeMgr) + { + // After the first failure notify session manager to refresh device data + if (entry->sendCount == 0) + { + exchangeMgr->GetSessionManager()->RefreshSessionOperationalData(entry->ec->GetSessionHandle()); + } + } // Update the counters entry->sendCount++; } diff --git a/src/transport/SessionDelegate.h b/src/transport/SessionDelegate.h index 893b063071ba53..aacb6e7824245b 100644 --- a/src/transport/SessionDelegate.h +++ b/src/transport/SessionDelegate.h @@ -48,4 +48,19 @@ class DLL_EXPORT SessionReleaseDelegate virtual void OnSessionReleased(SessionHandle session) = 0; }; +class DLL_EXPORT SessionRecoveryDelegate +{ +public: + virtual ~SessionRecoveryDelegate() {} + + /** + * @brief + * Called when the first message delivery in a session failed, + * so actions aiming to recover connection can be performed. + * + * @param session The handle to the secure session + */ + virtual void OnFirstMessageDeliveryFailed(const SessionHandle & session) = 0; +}; + } // namespace chip diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 549214193861a7..e2f4ac7d843f1e 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -364,6 +364,38 @@ void SessionManager::OnMessageReceived(const PeerAddress & peerAddress, System:: } } +void SessionManager::RegisterRecoveryDelegate(SessionRecoveryDelegate & cb) +{ +#ifndef NDEBUG + mSessionRecoveryDelegates.ForEachActiveObject([&](std::reference_wrapper * i) { + VerifyOrDie(std::addressof(cb) != std::addressof(i->get())); + return true; + }); +#endif + std::reference_wrapper * slot = mSessionRecoveryDelegates.CreateObject(cb); + VerifyOrDie(slot != nullptr); +} + +void SessionManager::UnregisterRecoveryDelegate(SessionRecoveryDelegate & cb) +{ + mSessionRecoveryDelegates.ForEachActiveObject([&](std::reference_wrapper * i) { + if (std::addressof(cb) == std::addressof(i->get())) + { + mSessionRecoveryDelegates.ReleaseObject(i); + return false; + } + return true; + }); +} + +void SessionManager::RefreshSessionOperationalData(const SessionHandle & sessionHandle) +{ + mSessionRecoveryDelegates.ForEachActiveObject([&](std::reference_wrapper * cb) { + cb->get().OnFirstMessageDeliveryFailed(sessionHandle); + return true; + }); +} + void SessionManager::MessageDispatch(const PacketHeader & packetHeader, const Transport::PeerAddress & peerAddress, System::PacketBufferHandle && msg) { diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 7112542593c784..3cd472ff49f6be 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -198,6 +198,10 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate }); } + void RegisterRecoveryDelegate(SessionRecoveryDelegate & cb); + void UnregisterRecoveryDelegate(SessionRecoveryDelegate & cb); + void RefreshSessionOperationalData(const SessionHandle & sessionHandle); + /** * @brief * Establish a new pairing with a peer node @@ -291,6 +295,9 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RELEASE_DELEGATES> mSessionReleaseDelegates; + BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES> + mSessionRecoveryDelegates; + TransportMgrBase * mTransportMgr = nullptr; Transport::MessageCounterManagerInterface * mMessageCounterManager = nullptr;