From 11459245cf60d944a49219ef4da47ffa7557599b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 11 Mar 2022 16:23:36 -0500 Subject: [PATCH] Use AddressResolve in CASE (#15934) * Squash merge: use address resolve for case * Add dependencies to compile on darwin (using xcode) * Updates for ESP32 compile: ensure the address resolve impl header is defined everywhere * Working through fixing Cirque. Fixes: - add proper CloseSession instead of a 'establish then close' since address resolve is not instant anymore - added retries and timeouts during the resolve test (since resolve may take a while) - Made GetPeerAddress to NOT rely exclusively on the dns cache and added a TODO to fix it - removed noisy logging for node resolution (address resolver also logs) - added extra logging to figure out what we connect to during CASE session establishment. Still not fully passing, but progress * Restyle * Fix compile, address code review comments * Add back node id resolve logging on progress * Add comment that CloseSession is TEMPORARY * Restyle * Fix double-free during CASE session cleanup (cleanup methods do case cleanup) * Add ordering issue on freeing commissionee to fix use after free error * Update comment: I did test both negative test cases under valgrind * Apply suggestions from code review * Apply suggestions from code review * Fix potential use-after-free issue in OperationalDeviceProxy::HandleCASEConnected. Co-authored-by: Boris Zbarsky --- .github/workflows/examples-cyw30739.yaml | 6 +- .../esp32/main/CMakeLists.txt | 3 + examples/bridge-app/esp32/main/CMakeLists.txt | 5 +- .../lighting-app/esp32/main/CMakeLists.txt | 4 + examples/lighting-app/telink/CMakeLists.txt | 5 + examples/lock-app/esp32/main/CMakeLists.txt | 3 + .../esp32/main/CMakeLists.txt | 11 +- .../esp32/main/CMakeLists.txt | 13 +- .../esp32/main/CMakeLists.txt | 2 +- .../esp32/main/CMakeLists.txt | 3 + scripts/build/build/targets.py | 3 +- scripts/build/builders/cyw30739.py | 15 ++ .../testdata/all_targets_except_host.txt | 3 +- .../build/testdata/build_all_except_host.txt | 6 + .../glob_star_targets_except_host.txt | 2 +- src/app/BUILD.gn | 1 + src/app/CASESessionManager.cpp | 53 +----- src/app/CASESessionManager.h | 18 +-- src/app/OperationalDeviceProxy.cpp | 60 +++++-- src/app/OperationalDeviceProxy.h | 36 ++++- src/app/chip_data_model.cmake | 4 + src/app/server/BUILD.gn | 1 + src/app/server/Server.cpp | 3 +- src/controller/BUILD.gn | 2 + src/controller/CHIPDeviceController.cpp | 153 ++++++++++-------- src/controller/CHIPDeviceController.h | 54 ++++--- .../ChipDeviceController-ScriptBinding.cpp | 17 +- .../python/test/test_scripts/base.py | 20 ++- .../TestCommissionableNodeController.cpp | 1 - .../Framework/CHIP.xcodeproj/project.pbxproj | 3 + src/darwin/Framework/CHIP/BUILD.gn | 1 + src/lib/address_resolve/AddressResolve.h | 10 +- .../AddressResolve_DefaultImpl.cpp | 76 +++++++-- .../AddressResolve_DefaultImpl.h | 67 +++++++- src/lib/dnssd/Discovery_ImplPlatform.cpp | 34 ---- src/lib/dnssd/Discovery_ImplPlatform.h | 1 - src/lib/dnssd/Resolver.h | 11 -- src/lib/dnssd/ResolverProxy.h | 1 - src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 12 -- src/lib/dnssd/Resolver_ImplNone.cpp | 6 - src/platform/CYW30739/BUILD.gn | 3 + src/platform/EFR32/BUILD.gn | 4 + src/platform/nxp/k32w/k32w0/BUILD.gn | 3 + src/platform/qpg/BUILD.gn | 1 + src/protocols/secure_channel/CASESession.cpp | 6 + src/test_driver/esp32/main/CMakeLists.txt | 3 + 46 files changed, 448 insertions(+), 301 deletions(-) diff --git a/.github/workflows/examples-cyw30739.yaml b/.github/workflows/examples-cyw30739.yaml index 184bcaa6a1df24..979d6c66db12e1 100644 --- a/.github/workflows/examples-cyw30739.yaml +++ b/.github/workflows/examples-cyw30739.yaml @@ -67,7 +67,7 @@ jobs: run: | ./scripts/run_in_build_env.sh \ "./scripts/build/build_examples.py \ - --target-glob 'cyw30739-cyw930739m2evb_01-{light,lock,ota-requestor}' \ + --target-glob 'cyw30739-cyw930739m2evb_01-{light,lock,ota-requestor-no-progress-logging}' \ build \ --copy-artifacts-to out/artifacts \ " @@ -89,8 +89,8 @@ jobs: timeout-minutes: 5 run: | .environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \ - cyw30739 cyw930739m2evb_01 ota-requestor \ - out/artifacts/cyw30739-cyw930739m2evb_01-ota-requestor/chip-cyw30739-ota-requestor-example.elf \ + cyw30739 cyw930739m2evb_01 ota-requestor-no-progress-logging \ + out/artifacts/cyw30739-cyw930739m2evb_01-ota-requestor-no-progress-logging/chip-cyw30739-ota-requestor-example.elf \ /tmp/bloat_reports/ - name: Uploading Size Reports uses: actions/upload-artifact@v2 diff --git a/examples/all-clusters-app/esp32/main/CMakeLists.txt b/examples/all-clusters-app/esp32/main/CMakeLists.txt index 6ac5bfd997ef55..11935f85c0ce31 100644 --- a/examples/all-clusters-app/esp32/main/CMakeLists.txt +++ b/examples/all-clusters-app/esp32/main/CMakeLists.txt @@ -126,6 +126,9 @@ idf_component_register(PRIV_INCLUDE_DIRS ${PRIV_INCLUDE_DIRS_LIST} set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 17) target_compile_options(${COMPONENT_LIB} PRIVATE "-DLWIP_IPV6_SCOPES=0" "-DCHIP_HAVE_CONFIG_H") +target_compile_options(${COMPONENT_LIB} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +) if (CONFIG_ENABLE_PW_RPC) diff --git a/examples/bridge-app/esp32/main/CMakeLists.txt b/examples/bridge-app/esp32/main/CMakeLists.txt index a2af85946b9b1f..f253e8de664868 100644 --- a/examples/bridge-app/esp32/main/CMakeLists.txt +++ b/examples/bridge-app/esp32/main/CMakeLists.txt @@ -35,7 +35,7 @@ idf_component_register(PRIV_INCLUDE_DIRS "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/localization-configuration-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/time-format-localization-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/fixed-label-server" - "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/user-label-server" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/user-label-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/thread-network-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/wifi-network-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/software-diagnostics-server" @@ -52,3 +52,6 @@ idf_component_register(PRIV_INCLUDE_DIRS set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 14) target_compile_options(${COMPONENT_LIB} PRIVATE "-DLWIP_IPV6_SCOPES=0" "-DCHIP_HAVE_CONFIG_H") +target_compile_options(${COMPONENT_LIB} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +) diff --git a/examples/lighting-app/esp32/main/CMakeLists.txt b/examples/lighting-app/esp32/main/CMakeLists.txt index 0190a95d18078c..bfac85429a0444 100644 --- a/examples/lighting-app/esp32/main/CMakeLists.txt +++ b/examples/lighting-app/esp32/main/CMakeLists.txt @@ -64,3 +64,7 @@ idf_component_register(PRIV_INCLUDE_DIRS set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 17) target_compile_options(${COMPONENT_LIB} PRIVATE "-DLWIP_IPV6_SCOPES=0" "-DCHIP_HAVE_CONFIG_H") +target_compile_options(${COMPONENT_LIB} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +) + diff --git a/examples/lighting-app/telink/CMakeLists.txt b/examples/lighting-app/telink/CMakeLists.txt index ff95953c7d52ec..1b9861c474f15e 100644 --- a/examples/lighting-app/telink/CMakeLists.txt +++ b/examples/lighting-app/telink/CMakeLists.txt @@ -39,6 +39,11 @@ target_include_directories(app PRIVATE ${NLIO_ROOT} ${TELINK_COMMON}/util/include ${TELINK_COMMON}/app/include) + +add_definitions( + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +) + # TODO - re-use chip_data_model.cmake to add cluster implementations. target_sources(app PRIVATE src/AppTask.cpp diff --git a/examples/lock-app/esp32/main/CMakeLists.txt b/examples/lock-app/esp32/main/CMakeLists.txt index 43562c24d416b2..5f27725dfb9328 100644 --- a/examples/lock-app/esp32/main/CMakeLists.txt +++ b/examples/lock-app/esp32/main/CMakeLists.txt @@ -176,5 +176,8 @@ idf_component_register(PRIV_INCLUDE_DIRS set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 17) target_compile_options(${COMPONENT_LIB} PRIVATE "-DLWIP_IPV6_SCOPES=0" "-DCHIP_HAVE_CONFIG_H") +target_compile_options(${COMPONENT_LIB} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +) endif (CONFIG_ENABLE_PW_RPC) diff --git a/examples/ota-provider-app/esp32/main/CMakeLists.txt b/examples/ota-provider-app/esp32/main/CMakeLists.txt index 34353da1390aee..eebb110857af21 100644 --- a/examples/ota-provider-app/esp32/main/CMakeLists.txt +++ b/examples/ota-provider-app/esp32/main/CMakeLists.txt @@ -38,13 +38,13 @@ idf_component_register(PRIV_INCLUDE_DIRS "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/localization-configuration-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/time-format-localization-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/fixed-label-server" - "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/user-label-server" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/user-label-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/thread-network-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/wifi-network-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/software-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/switch-server" - "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/general-diagnostics-server" - "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/group-key-mgmt-server" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/general-diagnostics-server" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/group-key-mgmt-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/general-commissioning-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/network-commissioning" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/operational-credentials-server" @@ -56,5 +56,8 @@ idf_component_register(PRIV_INCLUDE_DIRS PRIV_REQUIRES chip QRCode bt console spiffs) spiffs_create_partition_image(img_storage ../spiffs_image FLASH_IN_PROJECT) -set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 14) +set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 14) target_compile_options(${COMPONENT_LIB} PRIVATE "-DLWIP_IPV6_SCOPES=0" "-DCHIP_HAVE_CONFIG_H") +target_compile_options(${COMPONENT_LIB} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +) diff --git a/examples/ota-requestor-app/esp32/main/CMakeLists.txt b/examples/ota-requestor-app/esp32/main/CMakeLists.txt index 05ddfa4e6afabc..08f878c56b683f 100644 --- a/examples/ota-requestor-app/esp32/main/CMakeLists.txt +++ b/examples/ota-requestor-app/esp32/main/CMakeLists.txt @@ -36,20 +36,23 @@ idf_component_register(PRIV_INCLUDE_DIRS "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/diagnostic-logs-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/ethernet-network-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/localization-configuration-server" - "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/time-format-localization-server" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/time-format-localization-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/fixed-label-server" - "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/user-label-server" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/user-label-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/thread-network-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/wifi-network-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/software-diagnostics-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/switch-server" - "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/general-diagnostics-server" - "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/group-key-mgmt-server" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/general-diagnostics-server" + "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/group-key-mgmt-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/general-commissioning-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/network-commissioning" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/operational-credentials-server" "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/ota-requestor" PRIV_REQUIRES chip QRCode bt console app_update) -set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 14) +set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 14) target_compile_options(${COMPONENT_LIB} PRIVATE "-DLWIP_IPV6_SCOPES=0" "-DCHIP_HAVE_CONFIG_H") +target_compile_options(${COMPONENT_LIB} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +) diff --git a/examples/persistent-storage/esp32/main/CMakeLists.txt b/examples/persistent-storage/esp32/main/CMakeLists.txt index 2a9ad004a2a36c..6037ae4b38dd2e 100644 --- a/examples/persistent-storage/esp32/main/CMakeLists.txt +++ b/examples/persistent-storage/esp32/main/CMakeLists.txt @@ -15,7 +15,7 @@ # limitations under the License. # -idf_component_register(PRIV_INCLUDE_DIRS +idf_component_register(PRIV_INCLUDE_DIRS "${CMAKE_CURRENT_LIST_DIR}" "${CMAKE_SOURCE_DIR}/.." SRC_DIRS diff --git a/examples/temperature-measurement-app/esp32/main/CMakeLists.txt b/examples/temperature-measurement-app/esp32/main/CMakeLists.txt index f0ad92edea54d7..e458d7e9407186 100644 --- a/examples/temperature-measurement-app/esp32/main/CMakeLists.txt +++ b/examples/temperature-measurement-app/esp32/main/CMakeLists.txt @@ -77,6 +77,9 @@ idf_component_register(PRIV_INCLUDE_DIRS ${PRIV_INCLUDE_DIRS_LIST} set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 17) target_compile_options(${COMPONENT_LIB} PRIVATE "-DLWIP_IPV6_SCOPES=0" "-DCHIP_HAVE_CONFIG_H") +target_compile_options(${COMPONENT_LIB} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +) if (CONFIG_ENABLE_PW_RPC) diff --git a/scripts/build/build/targets.py b/scripts/build/build/targets.py index 7d292c59286d5f..cf6b7bb3e8df4b 100644 --- a/scripts/build/build/targets.py +++ b/scripts/build/build/targets.py @@ -376,7 +376,8 @@ def K32WTargets(): def Cyw30739Targets(): yield Target('cyw30739-cyw930739m2evb_01-light', Cyw30739Builder, board=Cyw30739Board.CYW930739M2EVB_01, app=Cyw30739App.LIGHT) yield Target('cyw30739-cyw930739m2evb_01-lock', Cyw30739Builder, board=Cyw30739Board.CYW930739M2EVB_01, app=Cyw30739App.LOCK) - yield Target('cyw30739-cyw930739m2evb_01-ota-requestor', Cyw30739Builder, board=Cyw30739Board.CYW930739M2EVB_01, app=Cyw30739App.OTA_REQUESTOR) + yield Target('cyw30739-cyw930739m2evb_01-ota-requestor', Cyw30739Builder, board=Cyw30739Board.CYW930739M2EVB_01, app=Cyw30739App.OTA_REQUESTOR).GlobBlacklist("Running out of XIP flash space") + yield Target('cyw30739-cyw930739m2evb_01-ota-requestor-no-progress-logging', Cyw30739Builder, board=Cyw30739Board.CYW930739M2EVB_01, app=Cyw30739App.OTA_REQUESTOR, progress_logging=False) def QorvoTargets(): diff --git a/scripts/build/builders/cyw30739.py b/scripts/build/builders/cyw30739.py index 07ee65ca2ff7a2..5d4470fef17535 100644 --- a/scripts/build/builders/cyw30739.py +++ b/scripts/build/builders/cyw30739.py @@ -64,11 +64,26 @@ def __init__( runner, app: Cyw30739App = Cyw30739App.LIGHT, board: Cyw30739Board = Cyw30739Board.CYW930739M2EVB_01, + release: bool = False, + progress_logging: bool = True ): super(Cyw30739Builder, self).__init__( root=app.BuildRoot(root), runner=runner) self.app = app self.board = board + self.release = release + self.progress_logging = progress_logging + + def GnBuildArgs(self): + args = [] + + if not self.progress_logging: + args.append('chip_progress_logging=false') + + if self.release: + args.append('is_debug=false') + + return args def build_outputs(self): items = {} diff --git a/scripts/build/testdata/all_targets_except_host.txt b/scripts/build/testdata/all_targets_except_host.txt index 110641c772a859..7b04b75ea5761e 100644 --- a/scripts/build/testdata/all_targets_except_host.txt +++ b/scripts/build/testdata/all_targets_except_host.txt @@ -16,7 +16,8 @@ android-x64-chip-tool android-x86-chip-tool cyw30739-cyw930739m2evb_01-light cyw30739-cyw930739m2evb_01-lock -cyw30739-cyw930739m2evb_01-ota-requestor +cyw30739-cyw930739m2evb_01-ota-requestor (NOGLOB: Running out of XIP flash space) +cyw30739-cyw930739m2evb_01-ota-requestor-no-progress-logging efr32-brd4161a-light efr32-brd4161a-light-rpc efr32-brd4161a-lock diff --git a/scripts/build/testdata/build_all_except_host.txt b/scripts/build/testdata/build_all_except_host.txt index 9220c6ab526a07..94c40f22911a5b 100644 --- a/scripts/build/testdata/build_all_except_host.txt +++ b/scripts/build/testdata/build_all_except_host.txt @@ -175,6 +175,9 @@ gn gen --check --fail-on-unused-args --export-compile-commands --root={root}/exa # Generating cyw30739-cyw930739m2evb_01-ota-requestor gn gen --check --fail-on-unused-args --export-compile-commands --root={root}/examples/ota-requestor-app/cyw30739 {out}/cyw30739-cyw930739m2evb_01-ota-requestor +# Generating cyw30739-cyw930739m2evb_01-ota-requestor-no-progress-logging +gn gen --check --fail-on-unused-args --export-compile-commands --root={root}/examples/ota-requestor-app/cyw30739 --args=chip_progress_logging=false {out}/cyw30739-cyw930739m2evb_01-ota-requestor-no-progress-logging + # Generating efr32-brd4161a-light gn gen --check --fail-on-unused-args --export-compile-commands --root={root}/examples/lighting-app/efr32 '--args=efr32_board="BRD4161A"' {out}/efr32-brd4161a-light @@ -938,6 +941,9 @@ ninja -C {out}/cyw30739-cyw930739m2evb_01-lock # Building cyw30739-cyw930739m2evb_01-ota-requestor ninja -C {out}/cyw30739-cyw930739m2evb_01-ota-requestor +# Building cyw30739-cyw930739m2evb_01-ota-requestor-no-progress-logging +ninja -C {out}/cyw30739-cyw930739m2evb_01-ota-requestor-no-progress-logging + # Building efr32-brd4161a-light ninja -C {out}/efr32-brd4161a-light diff --git a/scripts/build/testdata/glob_star_targets_except_host.txt b/scripts/build/testdata/glob_star_targets_except_host.txt index 6362e825f04c70..d64479847523e0 100644 --- a/scripts/build/testdata/glob_star_targets_except_host.txt +++ b/scripts/build/testdata/glob_star_targets_except_host.txt @@ -16,7 +16,7 @@ android-x64-chip-tool android-x86-chip-tool cyw30739-cyw930739m2evb_01-light cyw30739-cyw930739m2evb_01-lock -cyw30739-cyw930739m2evb_01-ota-requestor +cyw30739-cyw930739m2evb_01-ota-requestor-no-progress-logging efr32-brd4161a-light efr32-brd4161a-light-rpc efr32-brd4161a-lock diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index ecaae7a7db0e27..a04cd60e57ba18 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -156,6 +156,7 @@ static_library("app") { public_deps = [ ":app_buildconfig", "${chip_root}/src/access", + "${chip_root}/src/lib/address_resolve", "${chip_root}/src/lib/support", "${chip_root}/src/messaging", "${chip_root}/src/protocols/secure_channel", diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index e87d857c8ab229..58ec144a8f3fa6 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -17,19 +17,13 @@ */ #include -#include +#include namespace chip { -CHIP_ERROR CASESessionManager::Init() +CHIP_ERROR CASESessionManager::Init(chip::System::Layer * systemLayer) { - if (mConfig.dnsResolver == nullptr) - { - ReturnErrorOnFailure(mDNSResolver.Init(DeviceLayer::UDPEndPointManager())); - mDNSResolver.SetOperationalDelegate(this); - mConfig.dnsResolver = &mDNSResolver; - } - return CHIP_NO_ERROR; + return AddressResolve::Resolver::Instance().Init(systemLayer); } CHIP_ERROR CASESessionManager::FindOrEstablishSession(PeerId peerId, Callback::Callback * onConnection, @@ -68,7 +62,7 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(PeerId peerId, Callback::C session->OnNodeIdResolved(resolutionData); } - CHIP_ERROR err = session->Connect(onConnection, onFailure, mConfig.dnsResolver); + CHIP_ERROR err = session->Connect(onConnection, onFailure); if (err != CHIP_NO_ERROR) { // Release the peer rather than the pointer in case the failure handler has already released the session. @@ -93,49 +87,14 @@ void CASESessionManager::ReleaseAllSessions() mConfig.devicePool->ReleaseAllDevices(); } -CHIP_ERROR CASESessionManager::ResolveDeviceAddress(FabricInfo * fabric, NodeId nodeId) -{ - VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INCORRECT_STATE); - return mConfig.dnsResolver->ResolveNodeId(fabric->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny); -} - -void CASESessionManager::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeData) -{ - ChipLogProgress(Controller, "Address resolved for node: 0x" ChipLogFormatX64, ChipLogValueX64(nodeData.mPeerId.GetNodeId())); - -#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 - if (mConfig.dnsCache != nullptr) - { - CHIP_ERROR err = mConfig.dnsCache->Insert(nodeData); - if (err != CHIP_NO_ERROR) - { - ChipLogError(Controller, "DNS Cache insert: %" CHIP_ERROR_FORMAT, err.Format()); - } - } -#endif - - OperationalDeviceProxy * session = FindExistingSession(nodeData.mPeerId); - VerifyOrReturn(session != nullptr, - ChipLogDetail(Controller, "OnNodeIdResolved was called for a device with no active sessions, ignoring it.")); - - CHIP_ERROR err = session->UpdateDeviceData(OperationalDeviceProxy::ToPeerAddress(nodeData), nodeData.GetMRPConfig()); - if (err != CHIP_NO_ERROR) - { - ChipLogError(Controller, "Update Service Data: %" CHIP_ERROR_FORMAT, err.Format()); - } -} - -void CASESessionManager::OnOperationalNodeResolutionFailed(const PeerId & peer, CHIP_ERROR error) -{ - ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error)); -} - CHIP_ERROR CASESessionManager::GetPeerAddress(PeerId peerId, Transport::PeerAddress & addr) { #if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 if (mConfig.dnsCache != nullptr) { Dnssd::ResolvedNodeData resolutionData; + // TODO(andy31415): DNS caching is generally not populated, need to move + // caching into a the address resolve layer and not have a global one anymore. if (mConfig.dnsCache->Lookup(peerId, resolutionData) == CHIP_NO_ERROR) { addr = OperationalDeviceProxy::ToPeerAddress(resolutionData); diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index 2f129e9f71b08d..2f698c3f4e58ce 100644 --- a/src/app/CASESessionManager.h +++ b/src/app/CASESessionManager.h @@ -39,7 +39,6 @@ struct CASESessionManagerConfig Dnssd::DnssdCache * dnsCache = nullptr; #endif OperationalDeviceProxyPoolDelegate * devicePool = nullptr; - Dnssd::ResolverProxy * dnsResolver = nullptr; }; /** @@ -50,7 +49,7 @@ struct CASESessionManagerConfig * 4. During session establishment, trigger node ID resolution (if needed), and update the DNS-SD cache (if resolution is * successful) */ -class CASESessionManager : public Dnssd::OperationalResolveDelegate +class CASESessionManager { public: CASESessionManager() = delete; @@ -64,7 +63,7 @@ class CASESessionManager : public Dnssd::OperationalResolveDelegate virtual ~CASESessionManager() { mDNSResolver.Shutdown(); } - CHIP_ERROR Init(); + CHIP_ERROR Init(chip::System::Layer * systemLayer); void Shutdown() { mDNSResolver.Shutdown(); } /** @@ -84,15 +83,6 @@ class CASESessionManager : public Dnssd::OperationalResolveDelegate void ReleaseAllSessions(); - /** - * 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. - * - * The results of the DNS-SD resolution request is provided to the class via `OperationalResolveDelegate` - * implementation of CASESessionManager. - */ - CHIP_ERROR ResolveDeviceAddress(FabricInfo * fabric, NodeId nodeId); - /** * This API returns the address for the given node ID. * If the CASESessionManager is configured with a DNS-SD cache, the cache is looked up @@ -103,10 +93,6 @@ class CASESessionManager : public Dnssd::OperationalResolveDelegate */ CHIP_ERROR GetPeerAddress(PeerId peerId, Transport::PeerAddress & addr); - //////////// OperationalResolveDelegate Implementation /////////////// - void OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeData) override; - void OnOperationalNodeResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override; - private: OperationalDeviceProxy * FindSession(const SessionHandle & session); void ReleaseSession(OperationalDeviceProxy * device); diff --git a/src/app/OperationalDeviceProxy.cpp b/src/app/OperationalDeviceProxy.cpp index ac800bc14d6821..2ecd1287def89f 100644 --- a/src/app/OperationalDeviceProxy.cpp +++ b/src/app/OperationalDeviceProxy.cpp @@ -30,6 +30,7 @@ #include "CommandSender.h" #include "ReadPrepareParams.h" +#include #include #include #include @@ -43,8 +44,7 @@ using namespace chip::Callback; namespace chip { CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback * onConnection, - Callback::Callback * onFailure, - Dnssd::ResolverProxy * resolver) + Callback::Callback * onFailure) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -55,15 +55,7 @@ CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback break; case State::NeedsAddress: - VerifyOrReturnError(resolver != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - if (resolver->ResolveNodeIdFromInternalCache(mPeerId, Inet::IPAddressType::kAny)) - { - err = CHIP_NO_ERROR; - } - else - { - err = resolver->ResolveNodeId(mPeerId, Inet::IPAddressType::kAny); - } + err = LookupPeerAddress(); EnqueueConnectionCallbacks(onConnection, onFailure); break; @@ -87,7 +79,7 @@ CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback default: err = CHIP_ERROR_INCORRECT_STATE; - }; + } if (err != CHIP_NO_ERROR && onFailure != nullptr) { @@ -102,6 +94,13 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress { VerifyOrReturnLogError(mState != State::Uninitialized, CHIP_ERROR_INCORRECT_STATE); +#if CHIP_DETAIL_LOGGING + char peerAddrBuff[Transport::PeerAddress::kMaxToStringSize]; + addr.ToString(peerAddrBuff); + + ChipLogDetail(Controller, "Updating device address to %s while in state %d", peerAddrBuff, static_cast(mState)); +#endif + CHIP_ERROR err = CHIP_NO_ERROR; mDeviceAddress = addr; @@ -222,9 +221,11 @@ void OperationalDeviceProxy::HandleCASEConnectionFailure(void * context, CASECli device->mState = State::Initialized; + device->CloseCASESession(); device->DequeueConnectionSuccessCallbacks(/* executeCallback */ false); device->DequeueConnectionFailureCallbacks(error, /* executeCallback */ true); - device->CloseCASESession(); + // Do not touch device anymore; it might have been destroyed by a failure + // callback. } void OperationalDeviceProxy::HandleCASEConnected(void * context, CASEClient * client) @@ -238,14 +239,18 @@ void OperationalDeviceProxy::HandleCASEConnected(void * context, CASEClient * cl if (err != CHIP_NO_ERROR) { device->HandleCASEConnectionFailure(context, client, err); + // Do not touch device anymore; it might have been destroyed by a + // HandleCASEConnectionFailure. } else { device->mState = State::SecureConnected; + device->CloseCASESession(); device->DequeueConnectionFailureCallbacks(CHIP_NO_ERROR, /* executeCallback */ false); device->DequeueConnectionSuccessCallbacks(/* executeCallback */ true); - device->CloseCASESession(); + // Do not touch device anymore; it might have been destroyed by a + // success callback. } } @@ -311,4 +316,31 @@ OperationalDeviceProxy::~OperationalDeviceProxy() } } +CHIP_ERROR OperationalDeviceProxy::LookupPeerAddress() +{ + if (mAddressLookupHandle.IsInList()) + { + ChipLogProgress(Discovery, "Operational node lookup already in progress. Will NOT start a new one."); + return CHIP_NO_ERROR; + } + + AddressResolve::NodeLookupRequest request(mPeerId); + + return AddressResolve::Resolver::Instance().LookupNode(request, mAddressLookupHandle); +} + +void OperationalDeviceProxy::OnNodeAddressResolved(const PeerId & peerId, const AddressResolve::ResolveResult & result) +{ + UpdateDeviceData(result.address, result.mrpConfig); +} + +void OperationalDeviceProxy::OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP_ERROR reason) +{ + ChipLogError(Discovery, "Operational discovery failed for 0x" ChipLogFormatX64 ": %" CHIP_ERROR_FORMAT, + ChipLogValueX64(peerId.GetNodeId()), reason.Format()); + + DequeueConnectionSuccessCallbacks(/* executeCallback */ false); + DequeueConnectionFailureCallbacks(reason, /* executeCallback */ true); +} + } // namespace chip diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index 6bc9798a0979f5..3083fc68408f9f 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -43,8 +44,6 @@ #include #include -#include - namespace chip { struct DeviceProxyInitParams @@ -74,7 +73,19 @@ class OperationalDeviceProxy; typedef void (*OnDeviceConnected)(void * context, OperationalDeviceProxy * device); typedef void (*OnDeviceConnectionFailure)(void * context, PeerId peerId, CHIP_ERROR error); -class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDelegate, public SessionEstablishmentDelegate +/** + * Represents a connection path to a device that is in an operational state. + * + * Handles the lifetime of communicating with such a device: + * - Discover the device using DNSSD (find out what IP address to use and what + * communication parameters are appropriate for it) + * - Establish a secure channel to it via CASE + * - Expose to consumers the secure session for talking to the device. + */ +class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, + SessionReleaseDelegate, + public SessionEstablishmentDelegate, + public AddressResolve::NodeListener { public: virtual ~OperationalDeviceProxy(); @@ -86,13 +97,14 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele mInitParams = params; mPeerId = peerId; mFabricInfo = params.fabricTable->FindFabricWithCompressedId(peerId.GetCompressedFabricId()); - - mState = State::NeedsAddress; + mState = State::NeedsAddress; + mAddressLookupHandle.SetListener(this); } OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId, const Dnssd::ResolvedNodeData & nodeResolutionData) : OperationalDeviceProxy(params, peerId) { + mAddressLookupHandle.SetListener(this); OnNodeIdResolved(nodeResolutionData); } @@ -112,7 +124,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele * returned. */ CHIP_ERROR Connect(Callback::Callback * onConnection, - Callback::Callback * onFailure, Dnssd::ResolverProxy * resolver); + Callback::Callback * onFailure); bool IsConnected() const { return mState == State::SecureConnected; } @@ -205,6 +217,15 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele return kUndefinedFabricIndex; } + /** + * Triggers a DNSSD lookup to find a usable peer address for this operational device. + */ + CHIP_ERROR LookupPeerAddress(); + + // AddressResolve::NodeListener - notifications when dnssd finds a node IP address + void OnNodeAddressResolved(const PeerId & peerId, const AddressResolve::ResolveResult & result) override; + void OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP_ERROR reason) override; + private: enum class State { @@ -234,6 +255,9 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele Callback::CallbackDeque mConnectionSuccess; Callback::CallbackDeque mConnectionFailure; + /// This is used when a node address is required. + chip::AddressResolve::NodeLookupHandle mAddressLookupHandle; + CHIP_ERROR EstablishConnection(); bool IsSecureConnected() const override { return mState == State::SecureConnected; } diff --git a/src/app/chip_data_model.cmake b/src/app/chip_data_model.cmake index df2c52335af0bd..3a5b591018b3bf 100644 --- a/src/app/chip_data_model.cmake +++ b/src/app/chip_data_model.cmake @@ -64,6 +64,10 @@ function(chip_configure_data_model APP_TARGET) ${CHIP_APP_BASE_DIR}/server/Server.cpp ${CHIP_APP_BASE_DIR}/server/CommissioningWindowManager.cpp ) + + target_compile_options(${APP_TARGET} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" + ) endif() if (ARG_ZAP_FILE) diff --git a/src/app/server/BUILD.gn b/src/app/server/BUILD.gn index 5c83eb3c9dd896..2d456ea27a6aa7 100644 --- a/src/app/server/BUILD.gn +++ b/src/app/server/BUILD.gn @@ -56,6 +56,7 @@ static_library("server") { public_deps = [ "${chip_root}/src/app", + "${chip_root}/src/lib/address_resolve", "${chip_root}/src/lib/dnssd", "${chip_root}/src/messaging", "${chip_root}/src/platform", diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 0c755679559874..a6deeabc43ecbb 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -105,7 +105,6 @@ Server::Server() : .dnsCache = nullptr, #endif .devicePool = &mDevicePool, - .dnsResolver = nullptr, }), mGroupsProvider(mDeviceStorage) {} @@ -258,7 +257,7 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint &mSessions, &mFabrics); SuccessOrExit(err); - err = mCASESessionManager.Init(); + err = mCASESessionManager.Init(&DeviceLayer::SystemLayer()); // This code is necessary to restart listening to existing groups after a reboot // Each manufacturer needs to validate that they can rejoin groups by placing this code at the appropriate location for them diff --git a/src/controller/BUILD.gn b/src/controller/BUILD.gn index 111b1573962082..ece90d3921420d 100644 --- a/src/controller/BUILD.gn +++ b/src/controller/BUILD.gn @@ -70,6 +70,8 @@ static_library("controller") { "${chip_root}/src/transport", ] + deps = [ "${chip_root}/src/lib/address_resolve" ] + public_configs = [ ":config" ] defines = [] diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index f18f0f29e3f232..e094c55875226f 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -127,7 +127,6 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) VerifyOrReturnError(params.systemState->TransportMgr() != nullptr, CHIP_ERROR_INVALID_ARGUMENT); ReturnErrorOnFailure(mDNSResolver.Init(params.systemState->UDPEndPointManager())); - mDNSResolver.SetOperationalDelegate(this); mDNSResolver.SetCommissioningDelegate(this); RegisterDeviceDiscoveryDelegate(params.deviceDiscoveryDelegate); @@ -163,13 +162,14 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) #if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 .dnsCache = &mDNSCache, #endif - .devicePool = &mDevicePool, - .dnsResolver = &mDNSResolver, + .devicePool = &mDevicePool, }; mCASESessionManager = chip::Platform::New(sessionManagerConfig); VerifyOrReturnError(mCASESessionManager != nullptr, CHIP_ERROR_NO_MEMORY); + ReturnErrorOnFailure(mCASESessionManager->Init(params.systemState->SystemLayer())); + mSystemState = params.systemState->Retain(); mState = State::Initialized; return CHIP_NO_ERROR; @@ -291,6 +291,34 @@ void DeviceController::ReleaseOperationalDevice(NodeId remoteDeviceId) mCASESessionManager->ReleaseSession(mFabricInfo->GetPeerIdForNode(remoteDeviceId)); } +CHIP_ERROR DeviceController::DisconnectDevice(NodeId nodeId) +{ + ChipLogProgress(Controller, "Force close session for node 0x%" PRIx64, nodeId); + + OperationalDeviceProxy * proxy = mCASESessionManager->FindExistingSession(mFabricInfo->GetPeerIdForNode(nodeId)); + if (proxy == nullptr) + { + ChipLogProgress(Controller, "Attempted to close a session that does not exist."); + return CHIP_NO_ERROR; + } + + if (proxy->IsConnected()) + { + return proxy->Disconnect(); + } + + if (proxy->IsConnecting()) + { + ChipLogError(Controller, "Attempting to disconnect while connection in progress"); + return CHIP_ERROR_INCORRECT_STATE; + } + + // TODO: logic here is unclear. Possible states are "uninitialized, needs address, initialized" + // and disconnecting in those states is unclear (especially for needds-address). + ChipLogProgress(Controller, "Disconnect attempt while not in connected/connecting state"); + return CHIP_NO_ERROR; +} + void DeviceController::OnFirstMessageDeliveryFailed(const SessionHandle & session) { VerifyOrReturn(mState == State::Initialized, @@ -559,21 +587,6 @@ CHIP_ERROR DeviceController::OpenCommissioningWindowInternal() return CHIP_NO_ERROR; } -void DeviceController::OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) -{ - VerifyOrReturn(mState == State::Initialized, - ChipLogError(Controller, "OnOperationalNodeResolved was called in incorrect state")); - mCASESessionManager->OnOperationalNodeResolved(nodeData); -} - -void DeviceController::OnOperationalNodeResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error) -{ - ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error)); - VerifyOrReturn(mState == State::Initialized, - ChipLogError(Controller, "OnOperationalNodeResolutionFailed was called in incorrect state")); - mCASESessionManager->OnOperationalNodeResolutionFailed(peer, error); -} - ControllerDeviceInitParams DeviceController::GetControllerDeviceInitParams() { return ControllerDeviceInitParams{ @@ -617,12 +630,12 @@ CHIP_ERROR DeviceCommissioner::Init(CommissionerInitParams params) mUdcTransportMgr = chip::Platform::New(); ReturnErrorOnFailure(mUdcTransportMgr->Init(Transport::UdpListenParameters(mSystemState->UDPEndPointManager()) .SetAddressType(Inet::IPAddressType::kIPv6) - .SetListenPort((uint16_t)(mUdcListenPort)) + .SetListenPort(static_cast(mUdcListenPort)) #if INET_CONFIG_ENABLE_IPV4 , Transport::UdpListenParameters(mSystemState->UDPEndPointManager()) .SetAddressType(Inet::IPAddressType::kIPv4) - .SetListenPort((uint16_t)(mUdcListenPort)) + .SetListenPort(static_cast(mUdcListenPort)) #endif // INET_CONFIG_ENABLE_IPV4 )); @@ -719,12 +732,6 @@ CHIP_ERROR DeviceCommissioner::GetDeviceBeingCommissioned(NodeId deviceId, Commi return CHIP_NO_ERROR; } -CHIP_ERROR DeviceCommissioner::GetConnectedDevice(NodeId deviceId, chip::Callback::Callback * onConnection, - chip::Callback::Callback * onFailure) -{ - return DeviceController::GetConnectedDevice(deviceId, onConnection, onFailure); -} - CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, const char * setUpCode, const CommissioningParameters & params) { ReturnErrorOnFailure(mAutoCommissioner.SetCommissioningParameters(params)); @@ -1441,43 +1448,6 @@ void DeviceCommissioner::CommissioningStageComplete(CHIP_ERROR err, Commissionin } } -void DeviceCommissioner::OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) -{ - ChipLogProgress(Controller, "OperationalDiscoveryComplete for device ID 0x" ChipLogFormatX64, - ChipLogValueX64(nodeData.mPeerId.GetNodeId())); - VerifyOrReturn(mState == State::Initialized); - - // TODO: minimal mdns is buggy and violates the API contract for the - // resolver proxy by handing us results for all sorts of things we did not - // ask it to resolve, including results that don't even match our fabric. - // Reject at least those mis-matching results, since we can detect those - // easily. - if (nodeData.mPeerId.GetCompressedFabricId() != GetCompressedFabricId()) - { - return; - } - -#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 - mDNSCache.Insert(nodeData); -#endif - - mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); - DeviceController::OnOperationalNodeResolved(nodeData); -} - -void DeviceCommissioner::OnOperationalNodeResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error) -{ - if (mDeviceBeingCommissioned != nullptr) - { - CommissioneeDeviceProxy * device = mDeviceBeingCommissioned; - if (device->GetDeviceId() == peer.GetNodeId() && mCommissioningStage == CommissioningStage::kFindOperational) - { - CommissioningStageComplete(error); - } - } - DeviceController::OnOperationalNodeResolutionFailed(peer, error); -} - void DeviceCommissioner::OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device) { // CASE session established. @@ -1520,14 +1490,11 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peer VerifyOrReturn(commissioner->mPairingDelegate != nullptr, ChipLogProgress(Controller, "Device connection failure callback with null pairing delegate. Ignoring")); - // - // If a device is being commissioned currently and it is the very same device that we just failed to establish CASE with, - // we need to clean it up to prevent a dangling CommissioneeDeviceProxy object. - // - if (commissioner->mDeviceBeingCommissioned != nullptr && commissioner->mDeviceBeingCommissioned->GetPeerId() == peerId) + // Ensure that commissioning stage advancement is done based on seeing an error. + if (error == CHIP_NO_ERROR) { - commissioner->ReleaseCommissioneeDevice(commissioner->mDeviceBeingCommissioned); - commissioner->mDeviceBeingCommissioned = nullptr; + ChipLogError(Controller, "Device connection failed without a valid error code. Making one up."); + error = CHIP_ERROR_INTERNAL; } commissioner->mCASESessionManager->ReleaseSession(peerId); @@ -1540,6 +1507,20 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peer { commissioner->mPairingDelegate->OnPairingComplete(error); } + + if (commissioner->mDeviceBeingCommissioned != nullptr && commissioner->mDeviceBeingCommissioned->GetPeerId() == peerId) + { + // This prevents a leak when commissioning fails in the middle. + // Can use one of the following to simulate a failure: + // - comment out SendSigma2 on the device side (chip-tool will timeout in 1m or so) + // - set kMinLookupTimeMsDefault (and max) in AddressResolve.h + // to a very low value to quickly fail (e.g. 10 ms and 11ms), not enough time for the device + // to actually become operational. Chiptool should fail fast after PASE + // + // Run the above cases under valgrind/asan to validate no additional leaks. + commissioner->ReleaseCommissioneeDevice(commissioner->mDeviceBeingCommissioned); + commissioner->mDeviceBeingCommissioned = nullptr; + } } void DeviceCommissioner::SetupCluster(ClusterBase & base, DeviceProxy * proxy, EndpointId endpoint, @@ -2093,7 +2074,37 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio case CommissioningStage::kSecurePairing: break; } -} // namespace Controller +} + +CHIP_ERROR DeviceController::UpdateDevice(NodeId deviceId) +{ + VerifyOrReturnError(mState == State::Initialized && mFabricInfo != nullptr, CHIP_ERROR_INCORRECT_STATE); + + OperationalDeviceProxy * proxy = GetDeviceSession(mFabricInfo->GetPeerIdForNode(deviceId)); + VerifyOrReturnError(proxy != nullptr, CHIP_ERROR_NOT_FOUND); + + return proxy->LookupPeerAddress(); +} + +OperationalDeviceProxy * DeviceController::GetDeviceSession(const PeerId & peerId) +{ + return mCASESessionManager->FindExistingSession(peerId); +} + +OperationalDeviceProxy * DeviceCommissioner::GetDeviceSession(const PeerId & peerId) +{ + CHIP_ERROR err = + mCASESessionManager->FindOrEstablishSession(peerId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); + + if (err != CHIP_NO_ERROR) + { + ChipLogError(Controller, "Failed to establish new session: %" CHIP_ERROR_FORMAT, err.Format()); + return nullptr; + } + + // session should have been created now, expect this to return non-null + return DeviceController::GetDeviceSession(peerId); +} } // namespace Controller } // namespace chip diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 6b025c8fe4bb58..6a81f061cf0eb7 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -170,9 +170,7 @@ typedef void (*OnOpenCommissioningWindow)(void * context, NodeId deviceId, CHIP_ * and device pairing information for individual devices). Alternatively, this class can retrieve the * relevant information when the application tries to communicate with the device */ -class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, - public AbstractDnssdDiscoveryController, - public Dnssd::OperationalResolveDelegate +class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, public AbstractDnssdDiscoveryController { public: DeviceController(); @@ -211,26 +209,20 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, * Once the connection is successfully establishes (or if it's already connected), it calls `onConnectedDevice` * callback. If it fails to establish the connection, it calls `onError` callback. */ - virtual CHIP_ERROR GetConnectedDevice(NodeId deviceId, Callback::Callback * onConnection, - chip::Callback::Callback * onFailure) + CHIP_ERROR GetConnectedDevice(NodeId deviceId, Callback::Callback * onConnection, + chip::Callback::Callback * onFailure) { VerifyOrReturnError(mState == State::Initialized && mFabricInfo != nullptr, CHIP_ERROR_INCORRECT_STATE); return mCASESessionManager->FindOrEstablishSession(mFabricInfo->GetPeerIdForNode(deviceId), onConnection, onFailure); } /** - * @brief - * This function update the device informations asynchronously using dnssd. - * - * @param[in] deviceId Node ID for the CHIP device + * DEPRECATED - to be removed * - * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error code. + * Forces a DNSSD lookup for the specified device. It finds the corresponding session + * for the given nodeID and initiates a DNSSD lookup to find/update the node address */ - CHIP_ERROR UpdateDevice(NodeId deviceId) - { - VerifyOrReturnError(mState == State::Initialized && mFabricInfo != nullptr, CHIP_ERROR_INCORRECT_STATE); - return mCASESessionManager->ResolveDeviceAddress(mFabricInfo, deviceId); - } + CHIP_ERROR UpdateDevice(NodeId deviceId); /** * @brief @@ -335,6 +327,22 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, OperationalCredentialsDelegate * GetOperationalCredentialsDelegate() { return mOperationalCredentialsDelegate; } + /** + * TEMPORARY - DO NOT USE or if you use please request review on why/how to + * officially support such an API. + * + * This was added to support the 'reuse session' logic in cirque integration + * tests however since that is the only client, the correct update is to + * use 'ConnectDevice' and wait for connect success/failure inside the CI + * logic. The current code does not do that because python was not set up + * to wait for timeouts on success/fail, hence this temporary method. + * + * TODO(andy31415): update cirque test and remove this method. + * + * Returns success if a session with the given peer does not exist yet. + */ + CHIP_ERROR DisconnectDevice(NodeId nodeId); + protected: enum class State { @@ -375,12 +383,13 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, uint16_t mVendorId; + /// Fetches the session to use for the current device. Allows overriding + /// in case subclasses want to create the session if it does not yet exist + virtual OperationalDeviceProxy * GetDeviceSession(const PeerId & peerId); + //////////// SessionRecoveryDelegate Implementation /////////////// void OnFirstMessageDeliveryFailed(const SessionHandle & session) override; - //////////// OperationalResolveDelegate Implementation /////////////// - void OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override; - void OnOperationalNodeResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override; DiscoveredNodeList GetDiscoveredNodes() override { return DiscoveredNodeList(mCommissionableNodes); } private: @@ -548,9 +557,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, CHIP_ERROR GetDeviceBeingCommissioned(NodeId deviceId, CommissioneeDeviceProxy ** device); - CHIP_ERROR GetConnectedDevice(NodeId deviceId, chip::Callback::Callback * onConnection, - chip::Callback::Callback * onFailure) override; - /** * @brief * This function returns the attestation challenge for the secure session of the device being commissioned. @@ -652,9 +658,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, */ int GetMaxCommissionableNodesSupported() { return kMaxCommissionableNodes; } - void OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override; - void OnOperationalNodeResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override; - #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY // make this commissioner discoverable /** * @brief @@ -689,6 +692,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, // AttributeCache::Callback impl void OnDone() override; + // Commissioner will establish new device connections after PASE. + OperationalDeviceProxy * GetDeviceSession(const PeerId & peerId) override; + private: DevicePairingDelegate * mPairingDelegate; diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index c74d90f59f16cf..04276868b47abb 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -342,22 +342,11 @@ ChipError::StorageType pychip_DeviceController_SetWiFiCredentials(const char * s return CHIP_NO_ERROR.AsInteger(); } -void CloseSessionCallback(DeviceProxy * device, ChipError::StorageType err) -{ - if (device != nullptr) - { - device->Disconnect(); - } - if (!ChipError::IsSuccess(err)) - { - ChipLogError(Controller, "Close session callback was called with an error: %d", err); - } -} - ChipError::StorageType pychip_DeviceController_CloseSession(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid) { - return pychip_GetConnectedDeviceByNodeId(devCtrl, nodeid, CloseSessionCallback); + return devCtrl->DisconnectDevice(nodeid).AsInteger(); } + ChipError::StorageType pychip_DeviceController_EstablishPASESessionIP(chip::Controller::DeviceCommissioner * devCtrl, const char * peerAddrStr, uint32_t setupPINCode, chip::NodeId nodeid) @@ -591,9 +580,7 @@ ChipError::StorageType pychip_GetConnectedDeviceByNodeId(chip::Controller::Devic { VerifyOrReturnError(devCtrl != nullptr, CHIP_ERROR_INVALID_ARGUMENT.AsInteger()); auto * callbacks = new GetDeviceCallbacks(callback); - // callback(nullptr, 0); return devCtrl->GetConnectedDevice(nodeId, &callbacks->mOnSuccess, &callbacks->mOnFailure).AsInteger(); - // return CHIP_NO_ERROR.AsInteger(); } ChipError::StorageType pychip_GetDeviceBeingCommissioned(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId, diff --git a/src/controller/python/test/test_scripts/base.py b/src/controller/python/test/test_scripts/base.py index 2430ffeeba7343..4094b2e65c13f9 100644 --- a/src/controller/python/test/test_scripts/base.py +++ b/src/controller/python/test/test_scripts/base.py @@ -378,7 +378,11 @@ def CompareUnfilteredData(accessingFabric, otherFabric, expectedData): def TestCloseSession(self, nodeid: int): self.logger.info(f"Closing sessions with device {nodeid}") try: - self.devCtrl.CloseSession(nodeid) + err = self.devCtrl.CloseSession(nodeid) + if err != 0: + self.logger.exception( + f"Failed to close sessions with device {nodeid}: {err}") + return False return True except Exception as ex: self.logger.exception( @@ -445,8 +449,20 @@ def TestResolve(self, nodeid): "Resolve: node id = {:08x}".format(nodeid)) try: self.devCtrl.ResolveNode(nodeid=nodeid) - addr = self.devCtrl.GetAddressAndPort(nodeid) + addr = None + + start = time.time() + while not addr: + addr = self.devCtrl.GetAddressAndPort(nodeid) + if time.time() - start > 10: + self.logger.exception(f"Timeout waiting for address...") + break + + if not addr: + time.sleep(0.2) + if not addr: + self.logger.exception(f"Addr is missing...") return False self.logger.info(f"Resolved address: {addr[0]}:{addr[1]}") return True diff --git a/src/controller/tests/TestCommissionableNodeController.cpp b/src/controller/tests/TestCommissionableNodeController.cpp index a1ccca9bbf8d67..f5a1283592f869 100644 --- a/src/controller/tests/TestCommissionableNodeController.cpp +++ b/src/controller/tests/TestCommissionableNodeController.cpp @@ -38,7 +38,6 @@ class MockResolver : public Resolver CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override { return ResolveNodeIdStatus; } CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return FindCommissionersStatus; } CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override { return CHIP_ERROR_NOT_IMPLEMENTED; } - bool ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) override { return false; } CHIP_ERROR InitStatus = CHIP_NO_ERROR; CHIP_ERROR ResolveNodeIdStatus = CHIP_NO_ERROR; diff --git a/src/darwin/Framework/CHIP.xcodeproj/project.pbxproj b/src/darwin/Framework/CHIP.xcodeproj/project.pbxproj index ff8ad2524db4ca..cc9f004573bff8 100644 --- a/src/darwin/Framework/CHIP.xcodeproj/project.pbxproj +++ b/src/darwin/Framework/CHIP.xcodeproj/project.pbxproj @@ -612,6 +612,7 @@ GCC_OPTIMIZATION_LEVEL = 0; GCC_PREPROCESSOR_DEFINITIONS = ( "DEBUG=1", + "CHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=", "$(inherited)", ); GCC_WARN_64_TO_32_BIT_CONVERSION = YES; @@ -646,6 +647,7 @@ DYLIB_INSTALL_NAME_BASE = "@rpath"; GCC_PREPROCESSOR_DEFINITIONS = ( CHIP_HAVE_CONFIG_H, + "CHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=", "$(inherited)", ); HEADER_SEARCH_PATHS = ( @@ -788,6 +790,7 @@ DYLIB_INSTALL_NAME_BASE = "@rpath"; GCC_PREPROCESSOR_DEFINITIONS = ( CHIP_HAVE_CONFIG_H, + "CHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=", "$(inherited)", ); HEADER_SEARCH_PATHS = ( diff --git a/src/darwin/Framework/CHIP/BUILD.gn b/src/darwin/Framework/CHIP/BUILD.gn index 540b36129ff1ed..d823e741f78a6b 100644 --- a/src/darwin/Framework/CHIP/BUILD.gn +++ b/src/darwin/Framework/CHIP/BUILD.gn @@ -69,6 +69,7 @@ static_library("framework") { "${chip_root}/src/controller", "${chip_root}/src/controller/data_model", "${chip_root}/src/credentials:default_attestation_verifier", + "${chip_root}/src/lib/address_resolve", "${chip_root}/src/lib/core", "${chip_root}/src/lib/support", ] diff --git a/src/lib/address_resolve/AddressResolve.h b/src/lib/address_resolve/AddressResolve.h index 78713f6f099cb7..4a30b8876ecf91 100644 --- a/src/lib/address_resolve/AddressResolve.h +++ b/src/lib/address_resolve/AddressResolve.h @@ -139,7 +139,7 @@ class NodeLookupRequest private: static constexpr uint32_t kMinLookupTimeMsDefault = 200; - static constexpr uint32_t kMaxLookupTimeMsDefault = 3000; + static constexpr uint32_t kMaxLookupTimeMsDefault = 10000; PeerId mPeerId; System::Clock::Milliseconds32 mMinLookupTimeMs{ kMinLookupTimeMsDefault }; @@ -170,8 +170,14 @@ class Resolver public: virtual ~Resolver(); - /// Expected to be called exactly once before the resolver is ever + /// Expected to be called at least once before the resolver is ever /// used. + /// + /// Expected to override global setting of DNSSD callback for addres resolution + /// and may use the underlying system layer for timers and other functionality. + /// + /// If called multiple times, it is expected that the input systemLayer does + /// not change. virtual CHIP_ERROR Init(System::Layer * systemLayer) = 0; /// Initiate a node lookup for a particular node and use the specified diff --git a/src/lib/address_resolve/AddressResolve_DefaultImpl.cpp b/src/lib/address_resolve/AddressResolve_DefaultImpl.cpp index 2d38fbde8b5b23..f98d1439e97d5b 100644 --- a/src/lib/address_resolve/AddressResolve_DefaultImpl.cpp +++ b/src/lib/address_resolve/AddressResolve_DefaultImpl.cpp @@ -163,24 +163,22 @@ NodeLookupAction NodeLookupHandle::NextAction(System::Clock::Timestamp now) if (elapsed < mRequest.GetMinLookupTime()) { ChipLogProgress(Discovery, "Keeping DNSSD lookup active"); - return NodeLookupAction::kKeepSearching; + return NodeLookupAction::KeepSearching(); } // Minimal time to search reached. If any IP available, ready to return it. if (mBestAddressScore > ScoreValue(IpScore::kInvalid)) { - GetListener()->OnNodeAddressResolved(GetRequest().GetPeerId(), mBestResult); - return NodeLookupAction::kStopSearching; + return NodeLookupAction::Success(mBestResult); } // Give up if the maximum search time has been reached if (elapsed >= mRequest.GetMaxLookupTime()) { - GetListener()->OnNodeAddressResolutionFailed(GetRequest().GetPeerId(), CHIP_ERROR_TIMEOUT); - return NodeLookupAction::kStopSearching; + return NodeLookupAction::Error(CHIP_ERROR_TIMEOUT); } - return NodeLookupAction::kKeepSearching; + return NodeLookupAction::KeepSearching(); } CHIP_ERROR Resolver::LookupNode(const NodeLookupRequest & request, Impl::NodeLookupHandle & handle) @@ -220,19 +218,55 @@ void Resolver::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeDat for (size_t i = 0; i < nodeData.mNumIPs; i++) { +#if !INET_CONFIG_ENABLE_IPV4 + if (!nodeData.mAddress[i].IsIPv6()) + { + ChipLogError(Discovery, "Skipping IPv4 address during operational resolve."); + continue; + } +#endif result.address.SetIPAddress(nodeData.mAddress[i]); current->LookupResult(result); } - if (current->NextAction(mTimeSource.GetMonotonicTimestamp()) == NodeLookupAction::kStopSearching) - { - mActiveLookups.Erase(current); - } + HandleAction(current); } ReArmTimer(); } +void Resolver::HandleAction(IntrusiveList::Iterator & current) +{ + const NodeLookupAction action = current->NextAction(mTimeSource.GetMonotonicTimestamp()); + + if (action.Type() == NodeLookupResult::kKeepSearching) + { + // No change in iterator + return; + } + + // final result, handle either success or failure + const PeerId peerId = current->GetRequest().GetPeerId(); + NodeListener * listener = current->GetListener(); + mActiveLookups.Erase(current); + + // ensure action is taken AFTER the current current lookup is marked complete + // This allows failure handlers to deallocate structures that may + // contain the active lookup data as a member (intrusive lists members) + switch (action.Type()) + { + case NodeLookupResult::kLookupError: + listener->OnNodeAddressResolutionFailed(peerId, action.ErrorResult()); + break; + case NodeLookupResult::kLookupSuccess: + listener->OnNodeAddressResolved(peerId, action.ResolveResult()); + break; + default: + ChipLogError(Discovery, "Unexpected lookup state (not success or fail)."); + break; + } +} + void Resolver::HandleTimer() { auto it = mActiveLookups.begin(); @@ -240,10 +274,8 @@ void Resolver::HandleTimer() { auto current = it; it++; - if (current->NextAction(mTimeSource.GetMonotonicTimestamp()) == NodeLookupAction::kStopSearching) - { - mActiveLookups.Erase(current); - } + + HandleAction(current); } ReArmTimer(); @@ -261,8 +293,13 @@ void Resolver::OnOperationalNodeResolutionFailed(const PeerId & peerId, CHIP_ERR continue; } - current->GetListener()->OnNodeAddressResolutionFailed(peerId, error); + NodeListener * listener = current->GetListener(); mActiveLookups.Erase(current); + + // Failure callback only called after iterator was cleared: + // This allows failure handlers to deallocate structures that may + // contain the active lookup data as a member (intrusive lists members) + listener->OnNodeAddressResolutionFailed(peerId, error); } ReArmTimer(); } @@ -300,9 +337,16 @@ void Resolver::ReArmTimer() auto it = mActiveLookups.begin(); while (it != mActiveLookups.end()) { - it->GetListener()->OnNodeAddressResolutionFailed(it->GetRequest().GetPeerId(), err); + const PeerId peerId = it->GetRequest().GetPeerId(); + NodeListener * listener = it->GetListener(); + mActiveLookups.Erase(it); it = mActiveLookups.begin(); + + // Callback only called after active lookup is cleared + // This allows failure handlers to deallocate structures that may + // contain the active lookup data as a member (intrusive lists members) + listener->OnNodeAddressResolutionFailed(peerId, err); } } } diff --git a/src/lib/address_resolve/AddressResolve_DefaultImpl.h b/src/lib/address_resolve/AddressResolve_DefaultImpl.h index 3f90c96973e2e9..57d6e14c812d67 100644 --- a/src/lib/address_resolve/AddressResolve_DefaultImpl.h +++ b/src/lib/address_resolve/AddressResolve_DefaultImpl.h @@ -25,12 +25,67 @@ namespace chip { namespace AddressResolve { namespace Impl { +enum class NodeLookupResult +{ + kKeepSearching, // keep the current search active + kLookupError, // final status: error + kLookupSuccess, // final status: success +}; + /// Action to take when some resolve data /// has been received by an active lookup -enum class NodeLookupAction +class NodeLookupAction { - kKeepSearching, // Keep the lookup alive - kStopSearching, // Lookup complete +public: + NodeLookupAction(const NodeLookupAction & other) { *this = other; } + + NodeLookupAction & operator=(const NodeLookupAction & other) + { + mResultType = other.mResultType; + switch (mResultType) + { + case NodeLookupResult::kLookupError: + mResult.error = other.mResult.error; + break; + case NodeLookupResult::kLookupSuccess: + mResult.result = other.mResult.result; + break; + case NodeLookupResult::kKeepSearching: + // no data is important here + break; + } + return *this; + } + + static NodeLookupAction KeepSearching() { return NodeLookupAction(NodeLookupResult::kKeepSearching); } + + static NodeLookupAction Error(CHIP_ERROR err) + { + NodeLookupAction value(NodeLookupResult::kLookupError); + value.mResult.error = err; + return value; + } + + static NodeLookupAction Success(const AddressResolve::ResolveResult & result) + { + NodeLookupAction value(NodeLookupResult::kLookupSuccess); + value.mResult.result = result; + return value; + } + + NodeLookupResult Type() const { return mResultType; } + CHIP_ERROR ErrorResult() const { return mResult.error; } + const AddressResolve::ResolveResult & ResolveResult() const { return mResult.result; } + +private: + NodeLookupAction(NodeLookupResult result) : mResultType(result) {} + + union + { + CHIP_ERROR error; + AddressResolve::ResolveResult result; + } mResult = {}; + NodeLookupResult mResultType; }; /// An implementation of a node lookup handle @@ -99,6 +154,12 @@ class Resolver : public ::chip::AddressResolve::Resolver, public Dnssd::Operatio /// on the closest event required for an active resolve. void ReArmTimer(); + /// Handles the 'NextAction' on the given iterator + /// + /// NOTE: may remove `current` from the internal list. Current MUST NOT + /// be used after calling this method. + void HandleAction(IntrusiveList::Iterator & current); + System::Layer * mSystemLayer = nullptr; Time::TimeSource mTimeSource; IntrusiveList mActiveLookups; diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index efd3d81c62f117..a6d5e1273f995b 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -38,10 +38,6 @@ namespace Dnssd { namespace { -#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 -static DnssdCache sDnssdCache; -#endif - static void HandleNodeResolve(void * context, DnssdService * result, const Span & extraIPs, CHIP_ERROR error) { ResolverDelegateProxy * proxy = static_cast(context); @@ -162,9 +158,6 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, const Spa nodeData.LogNodeIdResolved(); nodeData.PrioritizeAddresses(); -#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 - LogErrorOnFailure(sDnssdCache.Insert(nodeData)); -#endif proxy->OnOperationalNodeResolved(nodeData); proxy->Release(); } @@ -604,15 +597,6 @@ CHIP_ERROR DiscoveryImplPlatform::ResolveNodeId(const PeerId & peerId, Inet::IPA return mResolverProxy.ResolveNodeId(peerId, type); } -bool DiscoveryImplPlatform::ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) -{ - if (InitImpl() != CHIP_NO_ERROR) - { - return false; - } - return mResolverProxy.ResolveNodeIdFromInternalCache(peerId, type); -} - CHIP_ERROR DiscoveryImplPlatform::FindCommissionableNodes(DiscoveryFilter filter) { ReturnErrorOnFailure(InitImpl()); @@ -654,24 +638,6 @@ CHIP_ERROR ResolverProxy::ResolveNodeId(const PeerId & peerId, Inet::IPAddressTy return ChipDnssdResolve(&service, Inet::InterfaceId::Null(), HandleNodeIdResolve, mDelegate); } -bool ResolverProxy::ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) -{ -#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 - if (mDelegate != nullptr) - { - /* see if the entry is cached and use it.... */ - ResolvedNodeData nodeData; - if (sDnssdCache.Lookup(peerId, nodeData) == CHIP_NO_ERROR) - { - mDelegate->OnOperationalNodeResolved(nodeData); - mDelegate->Release(); - return true; - } - } -#endif - return false; -} - CHIP_ERROR ResolverProxy::FindCommissionableNodes(DiscoveryFilter filter) { VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); diff --git a/src/lib/dnssd/Discovery_ImplPlatform.h b/src/lib/dnssd/Discovery_ImplPlatform.h index c0c548dea7c790..c070acc9681a1b 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.h +++ b/src/lib/dnssd/Discovery_ImplPlatform.h @@ -57,7 +57,6 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override; CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; - bool ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) override; static DiscoveryImplPlatform & GetInstance(); diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 84c1f0b5d1eec9..aeb8f01d04277a 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -412,17 +412,6 @@ class Resolver */ virtual CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) = 0; - /** - * Explicit attempt to resolve a NodeID without performing network operations. - * - * If the required entry exists in an internal cache, this will call the - * underlying delegate `OnNodeIdResolved` and will return true; - * - * Returns false if the corresponding entry does not exist in the internal cache. - * This will NEVER call `OnNodeIdResolutionFailed` and this method does not block. - */ - virtual bool ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) = 0; - /** * Finds all commissionable nodes matching the given filter. * diff --git a/src/lib/dnssd/ResolverProxy.h b/src/lib/dnssd/ResolverProxy.h index adac1935cd0c14..00bb397a9cd4f0 100644 --- a/src/lib/dnssd/ResolverProxy.h +++ b/src/lib/dnssd/ResolverProxy.h @@ -125,7 +125,6 @@ class ResolverProxy : public Resolver CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override; CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; - bool ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) override; private: ResolverDelegateProxy * mDelegate = nullptr; diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 8336122f5be7a4..40e8037c527dfc 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -376,7 +376,6 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override; CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; - bool ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) override; private: OperationalResolveDelegate * mOperationalDelegate = nullptr; @@ -582,12 +581,6 @@ CHIP_ERROR MinMdnsResolver::ResolveNodeId(const PeerId & peerId, Inet::IPAddress return SendPendingResolveQueries(); } -bool MinMdnsResolver::ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) -{ - // MinMDNS does not do cache node address resolutions. - return false; -} - CHIP_ERROR MinMdnsResolver::ScheduleRetries() { ReturnErrorCodeIf(mSystemLayer == nullptr, CHIP_ERROR_INCORRECT_STATE); @@ -709,10 +702,5 @@ CHIP_ERROR ResolverProxy::FindCommissioners(DiscoveryFilter filter) return chip::Dnssd::Resolver::Instance().FindCommissioners(filter); } -bool ResolverProxy::ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) -{ - return chip::Dnssd::Resolver::Instance().ResolveNodeIdFromInternalCache(peerId, type); -} - } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/Resolver_ImplNone.cpp b/src/lib/dnssd/Resolver_ImplNone.cpp index 8b0999d2fcc908..bf4de55100bf45 100644 --- a/src/lib/dnssd/Resolver_ImplNone.cpp +++ b/src/lib/dnssd/Resolver_ImplNone.cpp @@ -39,7 +39,6 @@ class NoneResolver : public Resolver } CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return CHIP_ERROR_NOT_IMPLEMENTED; } - bool ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) override { return false; } }; NoneResolver gResolver; @@ -66,10 +65,5 @@ CHIP_ERROR ResolverProxy::FindCommissioners(DiscoveryFilter filter) return CHIP_ERROR_NOT_IMPLEMENTED; } -bool ResolverProxy::ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) -{ - return false; -} - } // namespace Dnssd } // namespace chip diff --git a/src/platform/CYW30739/BUILD.gn b/src/platform/CYW30739/BUILD.gn index bbe503441865f7..50d3f464057e74 100644 --- a/src/platform/CYW30739/BUILD.gn +++ b/src/platform/CYW30739/BUILD.gn @@ -71,6 +71,9 @@ static_library("CYW30739") { "ThreadStackManagerImpl.cpp", "ThreadStackManagerImpl.h", ] + public_configs = [ + "${chip_root}/src/lib/address_resolve:default_address_resolve_config", + ] deps += [ "${chip_root}/third_party/openthread/platforms:libopenthread-platform", diff --git a/src/platform/EFR32/BUILD.gn b/src/platform/EFR32/BUILD.gn index 14fbb0013f8ff7..a50db957df5e48 100644 --- a/src/platform/EFR32/BUILD.gn +++ b/src/platform/EFR32/BUILD.gn @@ -95,6 +95,10 @@ static_library("EFR32") { sources += [ "../OpenThread/DnssdImpl.cpp" ] deps += [ "${chip_root}/src/lib/dnssd:platform_header" ] } + + public_configs = [ + "${chip_root}/src/lib/address_resolve:default_address_resolve_config", + ] } if (chip_enable_wifi) { diff --git a/src/platform/nxp/k32w/k32w0/BUILD.gn b/src/platform/nxp/k32w/k32w0/BUILD.gn index eb3e52b0becaf0..e813d8253c48bd 100644 --- a/src/platform/nxp/k32w/k32w0/BUILD.gn +++ b/src/platform/nxp/k32w/k32w0/BUILD.gn @@ -75,4 +75,7 @@ static_library("k32w0") { } public_deps += [ "${chip_root}/src/crypto" ] + + public_configs = + [ "${chip_root}/src/lib/address_resolve:default_address_resolve_config" ] } diff --git a/src/platform/qpg/BUILD.gn b/src/platform/qpg/BUILD.gn index 419f73030ed61a..6abc13b4513754 100644 --- a/src/platform/qpg/BUILD.gn +++ b/src/platform/qpg/BUILD.gn @@ -70,6 +70,7 @@ static_library("qpg") { public_configs += [ "${chip_root}/third_party/openthread/platforms/qpg:openthread_qpg_config", + "${chip_root}/src/lib/address_resolve:default_address_resolve_config", ] sources += [ diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 3c0cd1c012de5c..e88d7db8881ba3 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -232,6 +232,12 @@ CHIP_ERROR CASESession::EstablishSession(const Transport::PeerAddress peerAddres TRACE_EVENT_SCOPE("EstablishSession", "CASESession"); CHIP_ERROR err = CHIP_NO_ERROR; +#if CHIP_PROGRESS_LOGGING + char peerAddrBuff[Transport::PeerAddress::kMaxToStringSize]; + peerAddress.ToString(peerAddrBuff); + ChipLogProgress(SecureChannel, "Establishing CASE session to %s", peerAddrBuff); +#endif + // Return early on error here, as we have not initialized any state yet ReturnErrorCodeIf(exchangeCtxt == nullptr, CHIP_ERROR_INVALID_ARGUMENT); ReturnErrorCodeIf(fabric == nullptr, CHIP_ERROR_INVALID_ARGUMENT); diff --git a/src/test_driver/esp32/main/CMakeLists.txt b/src/test_driver/esp32/main/CMakeLists.txt index ded51eb78bee8c..e452f571f55787 100644 --- a/src/test_driver/esp32/main/CMakeLists.txt +++ b/src/test_driver/esp32/main/CMakeLists.txt @@ -40,3 +40,6 @@ idf_component_register(PRIV_INCLUDE_DIRS ${PRIV_INCLUDE_DIRS_LIST} set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 17) target_compile_options(${COMPONENT_LIB} PRIVATE "-DLWIP_IPV6_SCOPES=0" "-DCHIP_HAVE_CONFIG_H") +target_compile_options(${COMPONENT_LIB} PUBLIC + "-DCHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=" +)