Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commissioning fails on a slow CPU #12680

Closed
ktaylor-meus opened this issue Dec 7, 2021 · 6 comments
Closed

Commissioning fails on a slow CPU #12680

ktaylor-meus opened this issue Dec 7, 2021 · 6 comments

Comments

@ktaylor-meus
Copy link

Because our CPU is slow, we are failing the commissioning process (timeouts), unless we can stretch the timeouts further.
This happens primarily on the routines to do ECC functions, and the wrappers around those.
Once commissioned, everything works fine - it's just getting through the certificate validation.

architecture
STM32F @24 MHz
Wifi/Bluetooth/no Thread
Thermostat Server
FreeRTOS 9.0, WICED SDK 3.3

The following changes solved it for us. The problem, of course, is interactions with 3rd party devices which use the original timing.
I don't know if this counts as a bug or a feature request...
Commented lines are thr original values, uncommented are our changes.

src/messaging/ReliableMessageProtocolConfig.h
#define CHIP_CONFIG_MRP_DEFAULT_INITIAL_RETRY_INTERVAL (15000)
//#define CHIP_CONFIG_MRP_DEFAULT_INITIAL_RETRY_INTERVAL (5000)

src/protocols/secure_channel/CASESession.cpp
static constexpr ExchangeContext::Timeout kSigma_Response_Timeout = System::Clock::Seconds16(30);
//static constexpr ExchangeContext::Timeout kSigma_Response_Timeout = System::Clock::Seconds16(10);

src/protocols/secure_channel/PASESession.cpp
// no change required
static constexpr ExchangeContext::Timeout kSpake2p_Response_Timeout = System::Clock::Seconds16(30);

src/controller/CHIPDeviceController.cpp
constexpr uint32_t kSessionEstablishmentTimeout = 500 * kMillisecondsPerSecond;
//constexpr uint32_t kSessionEstablishmentTimeout = 30 * kMillisecondsPerSecond;

src/include/platform/CHIPDeviceConfig.h
#define CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS 15 * 60
//#define CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS 3 * 60

@ktaylor-meus ktaylor-meus changed the title Commissiong fails on a slow CPU Commissioning fails on a slow CPU Dec 7, 2021
@woody-apple
Copy link
Contributor

These seem like reasonable extensions to the minimum time?

@msandstedt
Copy link
Contributor

With regard to MRP, it appears the sdk isn't actually honoring the idle retry interval at all. So tweaks there won't do much. The sdk also isn't adding the random, exponential backoff the spec requires.

Do we have a ticket specifically for the MRP component of this?

@ktaylor-meus
Copy link
Author

ktaylor-meus commented Mar 1, 2022 via email

@bzbarsky-apple
Copy link
Contributor

So the current state of things here, compared to when the issue was filed, is:

  1. The MRP params used will be whatever the device advertises. Backoff is now implemented.
  2. kSigma_Response_Timeout is set to 30s, since Increase CASE timeout to fix CI failures #11435.
  3. kSessionEstablishmentTimeout is now 40s.
  4. Important: Per-command timeouts during commissioning are now at least 30s, so the crypto that happens there has time to happen.
  5. CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS is now 15*60 by default. In any case, this is configurable by the app when it builds.

that looks like all the initial proposals except for setting kSessionEstablishmentTimeout to "500s" have happened now. And that one was raised to 40s.

@ktaylor-meus You said above that things are working on your end now. Can this just be closed?

@ktaylor-meus
Copy link
Author

ktaylor-meus commented Jun 13, 2022 via email

@bzbarsky-apple
Copy link
Contributor

@ktaylor-meus Thank you for following up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants