Skip to content

Commit 4e943d0

Browse files
committed
For thread devices throttle the response to BlockQuery by an interval specified in kBdxThrottleIntervalInMsecs so that we don't overload the network with frequent BDX messages
1 parent 63f9782 commit 4e943d0

4 files changed

+73
-13
lines changed

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h

+5
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,11 @@ NS_ASSUME_NONNULL_BEGIN
223223
queue:(dispatch_queue_t)queue
224224
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion;
225225

226+
/**
227+
* Returns YES if the MTRDevice corrresponding to the given node ID is a thread device, NO otherwise.
228+
*/
229+
- (BOOL)usesThreadForDevice:(chip::NodeId)nodeID;
230+
226231
/**
227232
* Will return chip::kUndefinedFabricIndex if we do not have a fabric index.
228233
*/

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm

+22-12
Original file line numberDiff line numberDiff line change
@@ -1429,33 +1429,43 @@ - (BOOL)checkIsRunning:(NSError * __autoreleasing *)error
14291429
return NO;
14301430
}
14311431

1432-
- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion
1432+
- (BOOL)usesThreadForDevice:(chip::NodeId)nodeID
14331433
{
1434-
// TODO: Figure out whether the synchronization here makes sense. What
1435-
// happens if this call happens mid-suspend or mid-resume?
1436-
if (self.suspended) {
1437-
MTR_LOG_ERROR("%@ suspended: can't get session for node %016llX-%016llx (%llu)", self, self.compressedFabricID.unsignedLongLongValue, nodeID, nodeID);
1438-
// TODO: Can we do a better error here?
1439-
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
1440-
return;
1434+
if (nodeID == chip::kUndefinedNodeId)
1435+
{
1436+
return NO;
14411437
}
14421438

1443-
// Get the corresponding MTRDevice object to determine if the case/subscription pool is to be used
1439+
// Get the corresponding MTRDevice object for the node id
14441440
MTRDevice * device = [self deviceForNodeID:@(nodeID)];
14451441

14461442
// TODO: Can we not just assume this isKindOfClass test is true? Would be
14471443
// really nice if we had compile-time checking for this somehow...
14481444
if (![device isKindOfClass:MTRDevice_Concrete.class]) {
14491445
MTR_LOG_ERROR("%@ somehow has %@ instead of MTRDevice_Concrete for node ID 0x%016llX (%llu)", self, device, nodeID, nodeID);
1450-
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
1451-
return;
1446+
return NO;
14521447
}
14531448

14541449
auto * concreteDevice = static_cast<MTRDevice_Concrete *>(device);
14551450

1451+
BOOL usesThread = [concreteDevice deviceUsesThread];
1452+
return usesThread;
1453+
}
1454+
1455+
- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion
1456+
{
1457+
// TODO: Figure out whether the synchronization here makes sense. What
1458+
// happens if this call happens mid-suspend or mid-resume?
1459+
if (self.suspended) {
1460+
MTR_LOG_ERROR("%@ suspended: can't get session for node %016llX-%016llx (%llu)", self, self.compressedFabricID.unsignedLongLongValue, nodeID, nodeID);
1461+
// TODO: Can we do a better error here?
1462+
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
1463+
return;
1464+
}
1465+
14561466
// In the case that this device is known to use thread, queue this with subscription attempts as well, to
14571467
// help with throttling Thread traffic.
1458-
if ([concreteDevice deviceUsesThread]) {
1468+
if ([self usesThreadForDevice:nodeID]) {
14591469
MTRAsyncWorkItem * workItem = [[MTRAsyncWorkItem alloc] initWithQueue:dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)];
14601470
[workItem setReadyHandler:^(id _Nonnull context, NSInteger retryCount, MTRAsyncWorkCompletionBlock _Nonnull workItemCompletion) {
14611471
MTRInternalDeviceConnectionCallback completionWrapper = ^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,

src/darwin/Framework/CHIP/MTROTAImageTransferHandler.h

+2
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ class MTROTAImageTransferHandler : public chip::bdx::AsyncResponder {
7474
MTROTAImageTransferHandlerWrapper * mOTAImageTransferHandlerWrapper;
7575

7676
bool mNeedToCallTransferSessionEnd = false;
77+
78+
bool mIsPeerNodeAThreadDevice = NO;
7779
};
7880

7981
NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm

+44-1
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,16 @@
2828

2929
constexpr uint32_t kMaxBdxBlockSize = 1024;
3030

31+
constexpr double kMilliSecondsInSecond = 1000.0;
32+
3133
// Timeout for the BDX transfer session. The OTA Spec mandates this should be >= 5 minutes.
3234
constexpr System::Clock::Timeout kBdxTimeout = System::Clock::Seconds16(5 * 60);
3335

36+
// For thread devices, we need to throttle sending Blocks in response to BlockQuery messages
37+
// to avoid spamming the network with too many BDX messages. We are going to match the polling
38+
// interval of 50 ms as the time to wait before sending a Block in response to a BlockQuery.
39+
constexpr System::Clock::Timeout kBdxThrottleIntervalInMsecs = System::Clock::Milliseconds32(50);
40+
3441
constexpr bdx::TransferRole kBdxRole = bdx::TransferRole::kSender;
3542

3643
// An ARC-managed object that lets us do weak references to a MTROTAImageTransferHandler
@@ -78,6 +85,8 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *
7885
VerifyOrReturnError(mDelegate != nil, CHIP_ERROR_INCORRECT_STATE);
7986
VerifyOrReturnError(mDelegateNotificationQueue != nil, CHIP_ERROR_INCORRECT_STATE);
8087

88+
mIsPeerNodeAThreadDevice = [controller usesThreadForDevice:mPeer.GetNodeId()];
89+
8190
BitFlags<bdx::TransferControlFlags> flags(bdx::TransferControlFlags::kReceiverDrive);
8291

8392
return AsyncResponder::Init(mSystemLayer, exchangeCtx, kBdxRole, flags, kMaxBdxBlockSize, kBdxTimeout);
@@ -233,6 +242,11 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *
233242
{
234243
assertChipStackLockedByCurrentThread();
235244

245+
// For thread devices, we need to throttle sending the response to BlockQuery, if the query is processed, before kBdxThrottleIntervalInMsecs
246+
// has elapsed to prevent the BDX messages spamming up the network. Get the timestamp at which we start processing the BlockQuery message.
247+
248+
__block uint64_t startBlockQueryHandlingTimestamp = chip::System::SystemClock().GetMonotonicMilliseconds64().count();
249+
236250
auto blockSize = @(mTransfer.GetTransferBlockSize());
237251
auto blockIndex = @(mTransfer.GetNextBlockNum());
238252

@@ -241,7 +255,7 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *
241255

242256
MTROTAImageTransferHandlerWrapper * __weak weakWrapper = mOTAImageTransferHandlerWrapper;
243257

244-
auto completionHandler = ^(NSData * _Nullable data, BOOL isEOF) {
258+
auto respondWithBlock = ^(NSData * _Nullable data, BOOL isEOF) {
245259
[controller
246260
asyncDispatchToMatterQueue:^() {
247261
assertChipStackLockedByCurrentThread();
@@ -272,6 +286,35 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *
272286
}];
273287
};
274288

289+
__block void (^completionHandler)(NSData * _Nullable data, BOOL isEOF) = nil;
290+
291+
// If the peer node is a Thread device, check how much time has elapsed since we started processing the BlockQuery.
292+
// If the time elapsed is greater than kBdxThrottleIntervalInMsecs, call the completion handler to respond with a Block right away.
293+
// If time elapsed is less than kBdxThrottleIntervalInMsecs, dispatch the completion handler to respond with a Block after kBdxThrottleIntervalInMsecs has elapsed.
294+
295+
if (mIsPeerNodeAThreadDevice)
296+
{
297+
completionHandler = ^(NSData * _Nullable data, BOOL isEOF) {
298+
uint64_t timeElapsed = chip::System::SystemClock().GetMonotonicMilliseconds64().count() - startBlockQueryHandlingTimestamp;
299+
if (timeElapsed >= kBdxThrottleIntervalInMsecs.count())
300+
{
301+
completionHandler = respondWithBlock;
302+
}
303+
else
304+
{
305+
double timeRemainingInSecs = (kBdxThrottleIntervalInMsecs.count() - timeElapsed) / kMilliSecondsInSecond;
306+
dispatch_time_t time = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(timeRemainingInSecs * NSEC_PER_SEC));
307+
dispatch_after(time, dispatch_get_main_queue(), ^{
308+
respondWithBlock(data, isEOF);
309+
});
310+
}
311+
};
312+
}
313+
else
314+
{
315+
completionHandler = respondWithBlock;
316+
}
317+
275318
// TODO Handle MaxLength
276319

277320
auto nodeId = @(mPeer.GetNodeId());

0 commit comments

Comments
 (0)