From 9ce57b239e7460f1ac9ce68b6ab9e1c759817e63 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 5 Dec 2017 18:33:53 -0800 Subject: [PATCH] [ncp] enhance handling of outbound Spinel responses (to host) (#2302) This commit adds a new feature in `NcpBase` to allow spinel responses for property "get/set" commands to be saved/queued by NCP. This in turn helps to ensure that the responses are never dropped/missed due to NCP buffer space not being available at the time of processing of the command. The responses are sent when NCP buffer space becomes available. --- src/core/openthread-core-default-config.h | 18 + src/ncp/ncp_base.cpp | 453 ++++++++++++++-------- src/ncp/ncp_base.hpp | 61 ++- src/ncp/ncp_base_ftd.cpp | 31 +- src/ncp/ncp_base_mtd.cpp | 33 +- src/ncp/ncp_base_radio.cpp | 13 +- 6 files changed, 408 insertions(+), 201 deletions(-) diff --git a/src/core/openthread-core-default-config.h b/src/core/openthread-core-default-config.h index 7402885b7..ede4d938e 100644 --- a/src/core/openthread-core-default-config.h +++ b/src/core/openthread-core-default-config.h @@ -1040,6 +1040,24 @@ #define OPENTHREAD_CONFIG_NCP_ENABLE_PEEK_POKE 0 #endif +/** + * @def OPENTHREAD_CONFIG_NCP_SPINEL_RESPONSE_QUEUE_SIZE + * + * Size of NCP Spinel command response queue. + * + * NCP guarantees that it can respond up to `OPENTHREAD_CONFIG_NCP_SPINEL_RESPONSE_QUEUE_SIZE` spinel commands at the + * same time. The spinel protocol defines a Transaction ID (TID) as part of spinel command frame (the TID can be + * a value 0-15 where TID 0 is used for frames which require no response). Spinel protocol can support at most support + * 15 simultaneous commands. + * + * The host driver implementation may further limit the number of simultaneous Spinel command frames (e.g., wpantund + * limits this to two). This configuration option can be used to reduce the response queue size. + * + */ +#ifndef OPENTHREAD_CONFIG_NCP_SPINEL_RESPONSE_QUEUE_SIZE +#define OPENTHREAD_CONFIG_NCP_SPINEL_RESPONSE_QUEUE_SIZE 15 +#endif + /** * @def OPENTHREAD_CONFIG_STAY_AWAKE_BETWEEN_FRAGMENTS * diff --git a/src/ncp/ncp_base.cpp b/src/ncp/ncp_base.cpp index 322569f36..17cdab635 100644 --- a/src/ncp/ncp_base.cpp +++ b/src/ncp/ncp_base.cpp @@ -220,13 +220,21 @@ const NcpBase::PropertyHandlerEntry NcpBase::mGetPropertyHandlerTable[] = NCP_GET_PROP_HANDLER_ENTRY(THREAD_CONTEXT_REUSE_DELAY), NCP_GET_PROP_HANDLER_ENTRY(THREAD_NETWORK_ID_TIMEOUT), NCP_GET_PROP_HANDLER_ENTRY(THREAD_ROUTER_SELECTION_JITTER), + NCP_GET_PROP_HANDLER_ENTRY(THREAD_PREFERRED_ROUTER_ID), #if OPENTHREAD_ENABLE_COMMISSIONER NCP_GET_PROP_HANDLER_ENTRY(THREAD_COMMISSIONER_ENABLED), #endif #if OPENTHREAD_ENABLE_TMF_PROXY NCP_GET_PROP_HANDLER_ENTRY(THREAD_TMF_PROXY_ENABLED), #endif +#if OPENTHREAD_CONFIG_ENABLE_STEERING_DATA_SET_OOB + NCP_GET_PROP_HANDLER_ENTRY(THREAD_STEERING_DATA), +#endif #endif // OPENTHREAD_FTD + +#if OPENTHREAD_ENABLE_RAW_LINK_API + NCP_GET_PROP_HANDLER_ENTRY(MAC_SRC_MATCH_ENABLED), +#endif }; #define NCP_SET_PROP_HANDLER_ENTRY(name) { SPINEL_PROP_##name, &NcpBase::SetPropertyHandler_##name } @@ -528,18 +536,22 @@ NcpBase::NcpBase(Instance *aInstance): mAllowPeekDelegate(NULL), mAllowPokeDelegate(NULL), #endif - mDroppedReplyTid(0), - mDroppedReplyTidBitSet(0), mNextExpectedTid(0), + mResponseQueueHead(0), + mResponseQueueTail(0), mAllowLocalNetworkDataChange(false), mRequireJoinExistingNetwork(false), mIsRawStreamEnabled(false), mDisableStreamWrite(false), mShouldEmitChildTableUpdate(false), +#if OPENTHREAD_FTD + mPreferredRouteId(0), +#endif #if OPENTHREAD_ENABLE_RAW_LINK_API mCurTransmitTID(0), mCurReceiveChannel(OPENTHREAD_CONFIG_DEFAULT_CHANNEL), mCurScanChannel(kInvalidScanChannel), + mSrcMatchEnabled(false), #endif // OPENTHREAD_ENABLE_RAW_LINK_API #if OPENTHREAD_MTD || OPENTHREAD_FTD mInboundSecureIpFrameCounter(0), @@ -560,6 +572,8 @@ NcpBase::NcpBase(Instance *aInstance): mTxFrameBuffer.SetFrameRemovedCallback(&NcpBase::HandleFrameRemovedFromNcpBuffer, this); + memset(&mResponseQueue, 0, sizeof(mResponseQueue)); + #if OPENTHREAD_MTD || OPENTHREAD_FTD otMessageQueueInit(&mMessageQueue); otSetStateChangedCallback(mInstance, &NcpBase::HandleNetifStateChanged, this); @@ -569,7 +583,10 @@ NcpBase::NcpBase(Instance *aInstance): otIcmp6SetEchoEnabled(mInstance, false); #if OPENTHREAD_FTD otThreadSetChildTableCallback(mInstance, &NcpBase::HandleChildTableChanged); +#if OPENTHREAD_CONFIG_ENABLE_STEERING_DATA_SET_OOB + memset(&mSteeringDataAddress, 0, sizeof(mSteeringDataAddress)); #endif +#endif // OPENTHREAD_FTD #if OPENTHREAD_ENABLE_LEGACY mLegacyNodeDidJoin = false; mLegacyHandlers = NULL; @@ -597,12 +614,12 @@ NcpFrameBuffer::FrameTag NcpBase::GetLastOutboundFrameTag(void) void NcpBase::HandleReceive(const uint8_t *aBuf, uint16_t aBufLength) { - otError parseError = OT_ERROR_NONE; - otError responseError = OT_ERROR_NONE; + otError error = OT_ERROR_NONE; uint8_t header = 0; - unsigned int command = 0; spinel_tid_t tid = 0; + mDisableStreamWrite = true; + // Initialize the decoder with the newly received spinel frame. mDecoder.Init(aBuf, aBufLength); @@ -610,14 +627,43 @@ void NcpBase::HandleReceive(const uint8_t *aBuf, uint16_t aBufLength) mHostPowerState = SPINEL_HOST_POWER_STATE_ONLINE; mHostPowerStateInProgress = false; - SuccessOrExit(parseError = mDecoder.ReadUint8(header)); - SuccessOrExit(parseError = mDecoder.ReadUintPacked(command)); + // Skip if there is no header byte to read or this isn't a spinel frame. - responseError = HandleCommand(header, command); + SuccessOrExit(mDecoder.ReadUint8(header)); + VerifyOrExit((SPINEL_HEADER_FLAG & header) == SPINEL_HEADER_FLAG); + + mRxSpinelFrameCounter++; + + // We only support IID zero for now. + if (SPINEL_HEADER_GET_IID(header) != 0) + { + WriteLastStatusFrame(header, SPINEL_STATUS_INVALID_INTERFACE); + ExitNow(); + } + + error = HandleCommand(header); + + if (error != OT_ERROR_NONE) + { + PrepareLastStatusResponse(header, ThreadErrorToSpinelStatus(error)); + } + + if (!IsResponseQueueEmpty()) + { + // A response may have been prepared and queued for this command, + // so we attempt to send/write any queued responses. Note that + // if the response was prepared but cannot be sent now (not + // enough buffer space available), it will be attempted again + // from `HandleFrameRemovedFromNcpBuffer()` when buffer space + // becomes available. + + IgnoreReturnValue(SendQueuedResponses()); + } + + // Check for out of sequence TIDs and update `mNextExpectedTid`, tid = SPINEL_HEADER_GET_TID(header); - // Check if we may have missed a `tid` in the sequence. if ((mNextExpectedTid != 0) && (tid != mNextExpectedTid)) { mRxSpinelOutOfOrderTidCounter++; @@ -626,38 +672,7 @@ void NcpBase::HandleReceive(const uint8_t *aBuf, uint16_t aBufLength) mNextExpectedTid = SPINEL_GET_NEXT_TID(tid); exit: - - if (parseError != OT_ERROR_NONE) - { - responseError = SendLastStatus(header, SPINEL_STATUS_PARSE_ERROR); - } - - if (responseError == OT_ERROR_NO_BUFS) - { - // If we cannot send a response due to buffer space not being - // available, we remember the TID of command so to send an - // error status when buffer space becomes available later. - - // Valid TID range is 1-15 (zero being used as special case - // where no reply is expected). TIDs for dropped reply are - // stored in two variables: `mDroppedReplyTidBitSet` which - // is a bit set (bits 1-15 correspond to TID values 1-15). - // The first/next dropped TID value in the set is stored in - // `mDroppedReplyTid` (with value zero indicating that there - // is no dropped reply). - - if (tid != 0) - { - if (mDroppedReplyTid == 0) - { - mDroppedReplyTid = tid; - } - - mDroppedReplyTidBitSet |= (1 << tid); - } - } - - mRxSpinelFrameCounter++; + mDisableStreamWrite = false; } void NcpBase::HandleFrameRemovedFromNcpBuffer(void *aContext, NcpFrameBuffer::FrameTag aFrameTag, @@ -679,39 +694,21 @@ void NcpBase::HandleFrameRemovedFromNcpBuffer(NcpFrameBuffer::FrameTag aFrameTag } } - // Space is now available in ncp tx frame buffer. + // A frame was removed from NCP TX buffer, so more space is now available. + // We attempt to write/send any pending frames. Order of the checks + // below is important: First any queued command responses, then + // any queued IPv6 datagram messages, then any asynchronous property updates. + // If a frame still can not fit in the available buffer, we exit immediately + // and wait for next time this callback is invoked (when another frame is + // removed and more buffer space becomes available). - while (mDroppedReplyTid != 0) - { - SuccessOrExit( - SendLastStatus( - SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0 | mDroppedReplyTid, - SPINEL_STATUS_NOMEM - )); + SuccessOrExit(SendQueuedResponses()); - mDroppedReplyTidBitSet &= ~(1 << mDroppedReplyTid); - - if (mDroppedReplyTidBitSet == 0) - { - mDroppedReplyTid = 0; - - break; - } - - do - { - mDroppedReplyTid = SPINEL_GET_NEXT_TID(mDroppedReplyTid); - } - while ((mDroppedReplyTidBitSet & (1 << mDroppedReplyTid)) == 0); - } + // Check if `HOST_POWER_STATE` property update is required. if (mHostPowerStateHeader) { - SuccessOrExit( - HandleCommandPropertyGet( - mHostPowerStateHeader, - SPINEL_PROP_HOST_POWER_STATE - )); + SuccessOrExit(WritePropertyValueIsFrame(mHostPowerStateHeader, SPINEL_PROP_HOST_POWER_STATE)); mHostPowerStateHeader = 0; @@ -722,10 +719,16 @@ void NcpBase::HandleFrameRemovedFromNcpBuffer(NcpFrameBuffer::FrameTag aFrameTag } } + #if OPENTHREAD_MTD || OPENTHREAD_FTD + + // Send any queued IPv6 datagram message. + SuccessOrExit(SendQueuedDatagramMessages()); #endif + // Send any unsolicited event-triggered property updates. + UpdateChangedProps(); exit: @@ -765,6 +768,13 @@ otError NcpBase::StreamWrite(int aStreamId, const uint8_t *aDataPtr, int aDataLe VerifyOrExit(!mDisableStreamWrite, error = OT_ERROR_INVALID_STATE); VerifyOrExit(!mChangedPropsSet.IsPropertyFiltered(streamPropKey), error = OT_ERROR_INVALID_STATE); + // If there is a pending queued response or unsolicited property update, + // we do not allow any new log stream writes. This is to ensure that log + // messages can not continue to use the NCP buffer space and block other + // spinel frames. + + VerifyOrExit(IsResponseQueueEmpty() && mChangedPropsSet.IsEmpty(), error = OT_ERROR_NO_BUFS); + SuccessOrExit(error = mEncoder.BeginFrame(header, SPINEL_CMD_PROP_VALUE_IS, streamPropKey)); SuccessOrExit(error = mEncoder.WriteData(aDataPtr, static_cast(aDataLen))); SuccessOrExit(error = mEncoder.EndFrame()); @@ -841,6 +851,139 @@ exit: return; } +// ---------------------------------------------------------------------------- +// MARK: Spinel Response Handling +// ---------------------------------------------------------------------------- + +uint8_t NcpBase::GetWrappedResponseQueueIndex(uint8_t aPosition) +{ + while (aPosition >= kResponseQueueSize) + { + aPosition -= kResponseQueueSize; + } + + return aPosition; +} + +otError NcpBase::PrepareResponse(uint8_t aHeader, bool aIsLastStatus, bool aIsGetResponse, unsigned int aKeyOrStatus) +{ + otError error = OT_ERROR_NONE; + spinel_tid_t tid = SPINEL_HEADER_GET_TID(aHeader); + ResponseEntry *entry; + + if (tid == 0) + { + // No response is required for TID zero. But we may emit a + // `LAST_STATUS` error status (if not filtered) for TID + // zero (e.g., for a dropped `STREAM_NET` set command). + + if (aIsLastStatus) + { + mChangedPropsSet.AddLastStatus(static_cast(aKeyOrStatus)); + } + + ExitNow(); + } + + if ((mResponseQueueTail - mResponseQueueHead) >= kResponseQueueSize) + { + // If there is no room a for a response, emit an unsolicited + // `DROPPED` error status to indicate a spinel response was + // dropped. + + mChangedPropsSet.AddLastStatus(SPINEL_STATUS_DROPPED); + + ExitNow(error = OT_ERROR_NO_BUFS); + } + + // Transaction IDs are expected to come in sequence, if however, we + // get an out of sequence TID, check if we already have a response + // queued for this TID and if so mark the old entry as deleted. + + if (tid != mNextExpectedTid) + { + for (uint8_t cur = mResponseQueueHead; cur < mResponseQueueTail; cur++) + { + entry = &mResponseQueue[GetWrappedResponseQueueIndex(cur)]; + + if (entry->mIsInUse && (entry->mTid == tid)) + { + // Entry is just marked here and will be removed + // from `SendQueuedResponses()`. + + entry->mIsInUse = false; + break; + } + } + } + + // Add the new entry in the queue at tail. + + entry = &mResponseQueue[GetWrappedResponseQueueIndex(mResponseQueueTail)]; + + entry->mTid = tid; + entry->mIsInUse = true; + entry->mIsLastStatus = aIsLastStatus; + entry->mIsGetResponse = aIsGetResponse; + entry->mPropKeyOrStatus = aKeyOrStatus; + + mResponseQueueTail++; + +exit: + return error; +} + +otError NcpBase::SendQueuedResponses(void) +{ + otError error = OT_ERROR_NONE; + + while (mResponseQueueHead != mResponseQueueTail) + { + ResponseEntry &entry = mResponseQueue[mResponseQueueHead]; + + if (entry.mIsInUse) + { + uint8_t header = SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0; + + header |= static_cast(entry.mTid << SPINEL_HEADER_TID_SHIFT); + + if (entry.mIsLastStatus) + { + spinel_status_t status = static_cast(entry.mPropKeyOrStatus); + + SuccessOrExit(error = WriteLastStatusFrame(header, status)); + } + else + { + spinel_prop_key_t propKey = static_cast(entry.mPropKeyOrStatus); + + SuccessOrExit(error = WritePropertyValueIsFrame(header, propKey, entry.mIsGetResponse)); + } + } + + // Remove the response entry. + + entry.mIsInUse = false; + + mResponseQueueHead++; + + if (mResponseQueueHead == kResponseQueueSize) + { + // Only when `head` wraps, the `tail` will be wrapped as well. + // + // This ensures that `tail` is always bigger than `head` and + // `(tail - head)` to correctly give the number of items in + // the queue. + + mResponseQueueHead = 0; + mResponseQueueTail = GetWrappedResponseQueueIndex(mResponseQueueTail); + } + } + +exit: + return error; +} + // ---------------------------------------------------------------------------- // MARK: Property/Status Changed // ---------------------------------------------------------------------------- @@ -883,11 +1026,11 @@ void NcpBase::UpdateChangedProps(void) status = ResetReasonToSpinelStatus(otPlatGetResetReason(mInstance)); } - SuccessOrExit(SendLastStatus(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, status)); + SuccessOrExit(WriteLastStatusFrame(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, status)); } else { - SuccessOrExit(HandleCommandPropertyGet(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, propKey)); + SuccessOrExit(WritePropertyValueIsFrame(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, propKey)); } mChangedPropsSet.RemoveEntry(index); @@ -902,19 +1045,14 @@ exit: // MARK: Inbound Command Handler // ---------------------------------------------------------------------------- -otError NcpBase::HandleCommand(uint8_t aHeader, unsigned int aCommand) +otError NcpBase::HandleCommand(uint8_t aHeader) { otError error = OT_ERROR_NONE; + unsigned int command; - // Skip if this isn't a spinel frame - VerifyOrExit((SPINEL_HEADER_FLAG & aHeader) == SPINEL_HEADER_FLAG, error = OT_ERROR_INVALID_ARGS); + SuccessOrExit(error = mDecoder.ReadUintPacked(command)); - mDisableStreamWrite = true; - - // We only support IID zero for now. - VerifyOrExit(SPINEL_HEADER_GET_IID(aHeader) == 0, error = SendLastStatus(aHeader, SPINEL_STATUS_INVALID_INTERFACE)); - - switch (aCommand) + switch (command) { case SPINEL_CMD_NOOP: error = CommandHandler_NOOP(aHeader); @@ -928,7 +1066,7 @@ otError NcpBase::HandleCommand(uint8_t aHeader, unsigned int aCommand) case SPINEL_CMD_PROP_VALUE_SET: case SPINEL_CMD_PROP_VALUE_INSERT: case SPINEL_CMD_PROP_VALUE_REMOVE: - error = CommandHandler_PROP_VALUE_update(aHeader, aCommand); + error = CommandHandler_PROP_VALUE_update(aHeader, command); break; #if OPENTHREAD_CONFIG_NCP_ENABLE_PEEK_POKE @@ -958,19 +1096,18 @@ otError NcpBase::HandleCommand(uint8_t aHeader, unsigned int aCommand) default: #if OPENTHREAD_ENABLE_NCP_VENDOR_HOOK - if (aCommand >= SPINEL_CMD_VENDOR__BEGIN && aCommand < SPINEL_CMD_VENDOR__END) + if (command >= SPINEL_CMD_VENDOR__BEGIN && command < SPINEL_CMD_VENDOR__END) { - error = VendorCommandHandler(aHeader, aCommand); + error = VendorCommandHandler(aHeader, command); break; } #endif - error = SendLastStatus(aHeader, SPINEL_STATUS_INVALID_COMMAND); + error = PrepareLastStatusResponse(aHeader, SPINEL_STATUS_INVALID_COMMAND); break; } exit: - mDisableStreamWrite = false; return error; } @@ -1034,29 +1171,14 @@ NcpBase::PropertyHandler NcpBase::FindRemovePropertyHandler(spinel_prop_key_t aK ); } -otError NcpBase::HandleCommandPropertyGet(uint8_t aHeader, spinel_prop_key_t aKey) -{ - otError error = OT_ERROR_NONE; - PropertyHandler handler = FindGetPropertyHandler(aKey); - - VerifyOrExit(handler != NULL, error = SendLastStatus(aHeader, SPINEL_STATUS_PROP_NOT_FOUND)); - - SuccessOrExit(error = mEncoder.BeginFrame(aHeader, SPINEL_CMD_PROP_VALUE_IS, aKey)); - SuccessOrExit(error = (this->*handler)()); - SuccessOrExit(error = mEncoder.EndFrame()); - -exit: - return error; -} - // Returns `true` and updates the `aError` on success. bool NcpBase::HandlePropertySetForSpecialProperties(uint8_t aHeader, spinel_prop_key_t aKey, otError &aError) { bool didHandle = true; // Here the properties that require special treatment are handled. - // These properties are expected to form the response from their - // set handler directly. + // These properties are expected to form/write the response from + // their set handler directly. switch (aKey) { @@ -1091,8 +1213,6 @@ otError NcpBase::HandleCommandPropertySet(uint8_t aHeader, spinel_prop_key_t aKe { otError error = OT_ERROR_NONE; PropertyHandler handler = FindSetPropertyHandler(aKey); - const uint8_t *valuePtr; - uint16_t valueLen; if (handler == NULL) { @@ -1103,56 +1223,18 @@ otError NcpBase::HandleCommandPropertySet(uint8_t aHeader, spinel_prop_key_t aKe VerifyOrExit(!didHandle); - ExitNow(error = SendLastStatus(aHeader, SPINEL_STATUS_PROP_NOT_FOUND)); + ExitNow(error = PrepareLastStatusResponse(aHeader, SPINEL_STATUS_PROP_NOT_FOUND)); } - // Save current read position in the decoder. Read the entire - // content as a data blob (which can be used in forming the response - // if there is no associated get handler), then reset the read - // position back so that the handler method can parse the - // content. - - mDecoder.SavePosition(); - mDecoder.ReadData(valuePtr, valueLen); - mDecoder.ResetToSaved(); - error = (this->*handler)(); - VerifyOrExit(error == OT_ERROR_NONE, error = SendLastStatus(aHeader, ThreadErrorToSpinelStatus(error))); - - // Prepare the response. - - if ((handler = FindGetPropertyHandler(aKey)) != NULL) + if (error == OT_ERROR_NONE) { - SuccessOrExit(error = mEncoder.BeginFrame(aHeader, SPINEL_CMD_PROP_VALUE_IS, aKey)); - SuccessOrExit(error = (this->*handler)()); - SuccessOrExit(error = mEncoder.EndFrame()); + error = PrepareSetResponse(aHeader, aKey); } else { - if ((aKey == SPINEL_PROP_STREAM_NET) || (aKey == SPINEL_PROP_STREAM_NET_INSECURE) -#if OPENTHREAD_FTD && OPENTHREAD_ENABLE_TMF_PROXY - || (aKey == SPINEL_PROP_THREAD_TMF_PROXY_STREAM) -#endif - ) - { - // Only send a successful status for `STREAM` properties - // if the transaction id (TID) is non-zero. - - if (SPINEL_HEADER_GET_TID(aHeader) != 0) - { - error = SendLastStatus(aHeader, SPINEL_STATUS_OK); - } - - ExitNow(); - } - - // If there is no get handler for this property, echo the same - // value from input frame in the response. - - SuccessOrExit(error = mEncoder.BeginFrame(aHeader, SPINEL_CMD_PROP_VALUE_IS, aKey)); - SuccessOrExit(error = mEncoder.WriteData(valuePtr, valueLen)); - SuccessOrExit(error = mEncoder.EndFrame()); + error = PrepareLastStatusResponse(aHeader, ThreadErrorToSpinelStatus(error)); } exit: @@ -1184,7 +1266,7 @@ otError NcpBase::HandleCommandPropertyInsertRemove(uint8_t aHeader, spinel_prop_ break; } - VerifyOrExit(handler != NULL, error = SendLastStatus(aHeader, SPINEL_STATUS_PROP_NOT_FOUND)); + VerifyOrExit(handler != NULL, error = PrepareLastStatusResponse(aHeader, SPINEL_STATUS_PROP_NOT_FOUND)); // Save current read position in the decoder. Read the entire // content as a data blob (which is used in forming the response @@ -1197,12 +1279,17 @@ otError NcpBase::HandleCommandPropertyInsertRemove(uint8_t aHeader, spinel_prop_ error = (this->*handler)(); - VerifyOrExit(error == OT_ERROR_NONE, error = SendLastStatus(aHeader, ThreadErrorToSpinelStatus(error))); + VerifyOrExit(error == OT_ERROR_NONE, error = PrepareLastStatusResponse(aHeader, ThreadErrorToSpinelStatus(error))); - // Prepare the response - SuccessOrExit(error = mEncoder.BeginFrame(aHeader, responseCommand, aKey)); - SuccessOrExit(error = mEncoder.WriteData(valuePtr, valueLen)); - SuccessOrExit(error = mEncoder.EndFrame()); + error = WritePropertyValueInsertedRemovedFrame(aHeader, responseCommand, aKey, valuePtr, valueLen); + + // If the full response cannot be written now, instead prepare + // a `LAST_STATUS(STATUS_OK)` update as response. + + if (error != OT_ERROR_NONE) + { + error = PrepareLastStatusResponse(aHeader, SPINEL_STATUS_OK); + } exit: return error; @@ -1212,7 +1299,7 @@ exit: // MARK: Outbound Frame Methods // ---------------------------------------------------------------------------- -otError NcpBase::SendLastStatus(uint8_t aHeader, spinel_status_t aLastStatus) +otError NcpBase::WriteLastStatusFrame(uint8_t aHeader, spinel_status_t aLastStatus) { otError error = OT_ERROR_NONE; @@ -1229,13 +1316,55 @@ exit: return error; } +otError NcpBase::WritePropertyValueIsFrame(uint8_t aHeader, spinel_prop_key_t aPropKey, bool aIsGetResponse) +{ + otError error = OT_ERROR_NONE; + PropertyHandler handler = FindGetPropertyHandler(aPropKey); + + if (handler != NULL) + { + SuccessOrExit(error = mEncoder.BeginFrame(aHeader, SPINEL_CMD_PROP_VALUE_IS, aPropKey)); + SuccessOrExit(error = (this->*handler)()); + ExitNow(error = mEncoder.EndFrame()); + } + + if (aIsGetResponse) + { + SuccessOrExit(error = WriteLastStatusFrame(aHeader, SPINEL_STATUS_PROP_NOT_FOUND)); + } + else + { + // Send a STATUS_OK for "set" response to a property that + // has no corresponding get handler. + + SuccessOrExit(error = WriteLastStatusFrame(aHeader, SPINEL_STATUS_OK)); + } + +exit: + return error; +} + +otError NcpBase::WritePropertyValueInsertedRemovedFrame(uint8_t aHeader, unsigned int aResponseCommand, + spinel_prop_key_t aPropKey, const uint8_t *aValuePtr, + uint16_t aValueLen) +{ + otError error = OT_ERROR_NONE; + + SuccessOrExit(error = mEncoder.BeginFrame(aHeader, aResponseCommand, aPropKey)); + SuccessOrExit(error = mEncoder.WriteData(aValuePtr, aValueLen)); + SuccessOrExit(error = mEncoder.EndFrame()); + +exit: + return error; +} + // ---------------------------------------------------------------------------- // MARK: Individual Command Handlers // ---------------------------------------------------------------------------- otError NcpBase::CommandHandler_NOOP(uint8_t aHeader) { - return SendLastStatus(aHeader, SPINEL_STATUS_OK); + return PrepareLastStatusResponse(aHeader, SPINEL_STATUS_OK); } otError NcpBase::CommandHandler_RESET(uint8_t aHeader) @@ -1257,7 +1386,7 @@ otError NcpBase::CommandHandler_RESET(uint8_t aHeader) otIp6SetEnabled(mInstance, false); #endif - error = SendLastStatus(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_STATUS_RESET_SOFTWARE); + error = WriteLastStatusFrame(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_STATUS_RESET_SOFTWARE); if (error != OT_ERROR_NONE) { @@ -1275,12 +1404,12 @@ otError NcpBase::CommandHandler_PROP_VALUE_update(uint8_t aHeader, unsigned int error = mDecoder.ReadUintPacked(propKey); - VerifyOrExit(error == OT_ERROR_NONE, error = SendLastStatus(aHeader, ThreadErrorToSpinelStatus(error))); + VerifyOrExit(error == OT_ERROR_NONE, error = PrepareLastStatusResponse(aHeader, ThreadErrorToSpinelStatus(error))); switch (aCommand) { case SPINEL_CMD_PROP_VALUE_GET: - error = HandleCommandPropertyGet(aHeader, static_cast(propKey)); + error = PrepareGetResponse(aHeader, static_cast(propKey)); break; case SPINEL_CMD_PROP_VALUE_SET: @@ -1328,7 +1457,7 @@ otError NcpBase::CommandHandler_PEEK(uint8_t aHeader) exit: if (parseError != OT_ERROR_NONE) { - responseError = SendLastStatus(aHeader, ThreadErrorToSpinelStatus(parseError)); + responseError = PrepareLastStatusResponse(aHeader, ThreadErrorToSpinelStatus(parseError)); } return responseError; @@ -1357,7 +1486,7 @@ otError NcpBase::CommandHandler_POKE(uint8_t aHeader) memcpy(reinterpret_cast(address), dataPtr, count); exit: - return SendLastStatus(aHeader, ThreadErrorToSpinelStatus(parseError)); + return PrepareLastStatusResponse(aHeader, ThreadErrorToSpinelStatus(parseError)); } #endif // OPENTHREAD_CONFIG_NCP_ENABLE_PEEK_POKE @@ -1567,7 +1696,7 @@ exit: if (error != OT_ERROR_NONE) { - HandleCommandPropertyGet(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_UNSOL_UPDATE_FILTER); + WritePropertyValueIsFrame(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_UNSOL_UPDATE_FILTER); } return error; @@ -1755,7 +1884,7 @@ otError NcpBase::SetPropertyHandler_HOST_POWER_STATE(uint8_t aHeader) mHostPowerStateHeader = 0; - error = HandleCommandPropertyGet(aHeader, SPINEL_PROP_HOST_POWER_STATE); + error = WritePropertyValueIsFrame(aHeader, SPINEL_PROP_HOST_POWER_STATE); if (mHostPowerState != SPINEL_HOST_POWER_STATE_ONLINE) { @@ -1784,7 +1913,7 @@ otError NcpBase::SetPropertyHandler_HOST_POWER_STATE(uint8_t aHeader) } else { - error = SendLastStatus(aHeader, ThreadErrorToSpinelStatus(error)); + error = WriteLastStatusFrame(aHeader, ThreadErrorToSpinelStatus(error)); } return error; diff --git a/src/ncp/ncp_base.hpp b/src/ncp/ncp_base.hpp index 86385a8db..7018d10a7 100644 --- a/src/ncp/ncp_base.hpp +++ b/src/ncp/ncp_base.hpp @@ -176,9 +176,25 @@ private: PropertyHandler mHandler; }; + /** + * This struct represents a spinel response entry. + * + * It can be either a `VALUE_IS` property update for a specific property, or a `VALUE_IS(LAST_STATUS)` with a given + * spinel status error. + * + */ + struct ResponseEntry + { + uint8_t mTid : 4; ///< Spinel transaction id. + bool mIsInUse : 1; ///< `true` if this entry is in use, `false` otherwise. + bool mIsLastStatus : 1; ///< `true` if entry is a `LAST_STATUS`, otherwise it is a property update. + bool mIsGetResponse : 1; ///< `true` if response is for `VALUE_GET`, 'false` otherwise. + uint32_t mPropKeyOrStatus : 24; ///< 3 bytes for either property key or spinel status. + }; + NcpFrameBuffer::FrameTag GetLastOutboundFrameTag(void); - otError HandleCommand(uint8_t aHeader, unsigned int aCommand); + otError HandleCommand(uint8_t aHeader); PropertyHandler FindPropertyHandler(spinel_prop_key_t aKey, const PropertyHandlerEntry *aTable, size_t aTableLen); PropertyHandler FindGetPropertyHandler(spinel_prop_key_t aKey); @@ -186,12 +202,29 @@ private: PropertyHandler FindInsertPropertyHandler(spinel_prop_key_t aKey); PropertyHandler FindRemovePropertyHandler(spinel_prop_key_t aKey); - otError HandleCommandPropertyGet(uint8_t aHeader, spinel_prop_key_t aKey); bool HandlePropertySetForSpecialProperties(uint8_t aHeader, spinel_prop_key_t aKey, otError &aError); otError HandleCommandPropertySet(uint8_t aHeader, spinel_prop_key_t aKey); otError HandleCommandPropertyInsertRemove(uint8_t aHeader, spinel_prop_key_t aKey, unsigned int aCommand); - otError SendLastStatus(uint8_t aHeader, spinel_status_t aLastStatus); + otError WriteLastStatusFrame(uint8_t aHeader, spinel_status_t aLastStatus); + otError WritePropertyValueIsFrame(uint8_t aHeader, spinel_prop_key_t aPropKey, bool aIsGetResponse = true); + otError WritePropertyValueInsertedRemovedFrame(uint8_t aHeader, unsigned int aResponseCommand, + spinel_prop_key_t aPropKey, const uint8_t *aValuePtr, + uint16_t aValueLen); + + otError SendQueuedResponses(void); + bool IsResponseQueueEmpty(void) { return (mResponseQueueHead == mResponseQueueTail); } + otError PrepareResponse(uint8_t aHeader, bool aIsLastStatus, bool aIsGetResponse, unsigned int aPropKeyOrStatus); + otError PrepareGetResponse(uint8_t aHeader, spinel_prop_key_t aPropKey) { + return PrepareResponse(aHeader, false /* aIsLastStatus */, true /* aIsGetResponse*/, aPropKey); + } + otError PrepareSetResponse(uint8_t aHeader, spinel_prop_key_t aPropKey) { + return PrepareResponse(aHeader, false /* aIsLastStatus */, false /* aIsGetResponse*/, aPropKey); + } + otError PrepareLastStatusResponse(uint8_t aHeader, spinel_status_t aStatus) { + return PrepareResponse(aHeader, true /*aIsLastStatus */, false, aStatus); + } + static uint8_t GetWrappedResponseQueueIndex(uint8_t aPosition); static void UpdateChangedProps(Tasklet &aTasklet); void UpdateChangedProps(void); @@ -364,6 +397,7 @@ private: NCP_SET_PROP_HANDLER(PHY_ENABLED); NCP_SET_PROP_HANDLER(MAC_15_4_SADDR); + NCP_GET_PROP_HANDLER(MAC_SRC_MATCH_ENABLED); NCP_SET_PROP_HANDLER(MAC_SRC_MATCH_ENABLED); NCP_SET_PROP_HANDLER(MAC_SRC_MATCH_SHORT_ADDRESSES); NCP_INSERT_PROP_HANDLER(MAC_SRC_MATCH_SHORT_ADDRESSES); @@ -589,8 +623,10 @@ private: NCP_SET_PROP_HANDLER(THREAD_ROUTER_DOWNGRADE_THRESHOLD); NCP_GET_PROP_HANDLER(THREAD_ROUTER_SELECTION_JITTER); NCP_SET_PROP_HANDLER(THREAD_ROUTER_SELECTION_JITTER); + NCP_GET_PROP_HANDLER(THREAD_PREFERRED_ROUTER_ID); NCP_SET_PROP_HANDLER(THREAD_PREFERRED_ROUTER_ID); #if OPENTHREAD_CONFIG_ENABLE_STEERING_DATA_SET_OOB + NCP_GET_PROP_HANDLER(THREAD_STEERING_DATA); NCP_SET_PROP_HANDLER(THREAD_STEERING_DATA); #endif #if OPENTHREAD_ENABLE_COMMISSIONER @@ -642,6 +678,7 @@ private: enum { kTxBufferSize = OPENTHREAD_CONFIG_NCP_TX_BUFFER_SIZE, // Tx Buffer size (used by mTxFrameBuffer). + kResponseQueueSize = OPENTHREAD_CONFIG_NCP_SPINEL_RESPONSE_QUEUE_SIZE, kInvalidScanChannel = -1, // Invalid scan channel. }; @@ -672,21 +709,32 @@ private: otNcpDelegateAllowPeekPoke mAllowPokeDelegate; #endif - spinel_tid_t mDroppedReplyTid; - uint16_t mDroppedReplyTidBitSet; - spinel_tid_t mNextExpectedTid; uint8_t mTxBuffer[kTxBufferSize]; + spinel_tid_t mNextExpectedTid; + + uint8_t mResponseQueueHead; + uint8_t mResponseQueueTail; + ResponseEntry mResponseQueue[kResponseQueueSize]; + bool mAllowLocalNetworkDataChange; bool mRequireJoinExistingNetwork; bool mIsRawStreamEnabled; bool mDisableStreamWrite; bool mShouldEmitChildTableUpdate; +#if OPENTHREAD_FTD +#if OPENTHREAD_CONFIG_ENABLE_STEERING_DATA_SET_OOB + otExtAddress mSteeringDataAddress; +#endif + uint8_t mPreferredRouteId; +#endif + #if OPENTHREAD_ENABLE_RAW_LINK_API uint8_t mCurTransmitTID; uint8_t mCurReceiveChannel; int8_t mCurScanChannel; + bool mSrcMatchEnabled; #endif // OPENTHREAD_ENABLE_RAW_LINK_API #if OPENTHREAD_MTD || OPENTHREAD_FTD @@ -704,7 +752,6 @@ private: uint32_t mRxSpinelOutOfOrderTidCounter; // Number of out of order received spinel frames (tid increase > 1). uint32_t mTxSpinelFrameCounter; // Number of sent (outbound) spinel frames. - #if OPENTHREAD_ENABLE_LEGACY const otNcpLegacyHandlers *mLegacyHandlers; uint8_t mLegacyUlaPrefix[OT_NCP_LEGACY_ULA_PREFIX_LENGTH]; diff --git a/src/ncp/ncp_base_ftd.cpp b/src/ncp/ncp_base_ftd.cpp index 1220938ff..90e1cd909 100644 --- a/src/ncp/ncp_base_ftd.cpp +++ b/src/ncp/ncp_base_ftd.cpp @@ -380,7 +380,7 @@ otError NcpBase::SetPropertyHandler_THREAD_COMMISSIONER_ENABLED(uint8_t aHeader) } exit: - return SendLastStatus(aHeader, ThreadErrorToSpinelStatus(error)); + return PrepareLastStatusResponse(aHeader, ThreadErrorToSpinelStatus(error)); } otError NcpBase::InsertPropertyHandler_THREAD_JOINERS(void) @@ -423,18 +423,19 @@ exit: } #if OPENTHREAD_CONFIG_ENABLE_STEERING_DATA_SET_OOB + +otError NcpBase::GetPropertyHandler_THREAD_STEERING_DATA(void) +{ + return mEncoder.WriteEui64(mSteeringDataAddress); +} + otError NcpBase::SetPropertyHandler_THREAD_STEERING_DATA(void) { - const otExtAddress *extAddress; otError error = OT_ERROR_NONE; - SuccessOrExit(error = mDecoder.ReadEui64(extAddress)); + SuccessOrExit(error = mDecoder.ReadEui64(mSteeringDataAddress)); - SuccessOrExit(error = otThreadSetSteeringData(mInstance, extAddress)); - - // Note that there is no get handler for this property - // so the response becomes `VALUE_IS` echo of the - // received content. + SuccessOrExit(error = otThreadSetSteeringData(mInstance, &mSteeringDataAddress)); exit: return error; @@ -454,18 +455,18 @@ exit: return error; } +otError NcpBase::GetPropertyHandler_THREAD_PREFERRED_ROUTER_ID(void) +{ + return mEncoder.WriteUint8(mPreferredRouteId); +} + otError NcpBase::SetPropertyHandler_THREAD_PREFERRED_ROUTER_ID(void) { - uint8_t routerId = 0; otError error = OT_ERROR_NONE; - SuccessOrExit(error = mDecoder.ReadUint8(routerId)); + SuccessOrExit(error = mDecoder.ReadUint8(mPreferredRouteId)); - SuccessOrExit(error = otThreadSetPreferredRouterId(mInstance, routerId)); - - // Note that there is no get handler for this property - // so the response becomes `VALUE_IS` echo of the - // received content. + SuccessOrExit(error = otThreadSetPreferredRouterId(mInstance, mPreferredRouteId)); exit: return error; diff --git a/src/ncp/ncp_base_mtd.cpp b/src/ncp/ncp_base_mtd.cpp index a6371cc0f..51c7f4834 100644 --- a/src/ncp/ncp_base_mtd.cpp +++ b/src/ncp/ncp_base_mtd.cpp @@ -227,17 +227,17 @@ otError NcpBase::GetPropertyHandler_PHY_RSSI(void) otError NcpBase::CommandHandler_NET_SAVE(uint8_t aHeader) { - return SendLastStatus(aHeader, SPINEL_STATUS_UNIMPLEMENTED); + return PrepareLastStatusResponse(aHeader, SPINEL_STATUS_UNIMPLEMENTED); } otError NcpBase::CommandHandler_NET_CLEAR(uint8_t aHeader) { - return SendLastStatus(aHeader, ThreadErrorToSpinelStatus(otInstanceErasePersistentInfo(mInstance))); + return PrepareLastStatusResponse(aHeader, ThreadErrorToSpinelStatus(otInstanceErasePersistentInfo(mInstance))); } otError NcpBase::CommandHandler_NET_RECALL(uint8_t aHeader) { - return SendLastStatus(aHeader, SPINEL_STATUS_UNIMPLEMENTED); + return PrepareLastStatusResponse(aHeader, SPINEL_STATUS_UNIMPLEMENTED); } otError NcpBase::GetPropertyHandler_NET_SAVED(void) @@ -668,7 +668,7 @@ exit: // the state of these ports, so we need to report // those incomplete changes via an asynchronous // change event. - HandleCommandPropertyGet(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_THREAD_ASSISTING_PORTS); + WritePropertyValueIsFrame(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_THREAD_ASSISTING_PORTS); } return error; @@ -1854,7 +1854,7 @@ exit: if (error != OT_ERROR_NONE) { - HandleCommandPropertyGet(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_MAC_WHITELIST); + WritePropertyValueIsFrame(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_MAC_WHITELIST); } return error; @@ -1912,7 +1912,7 @@ exit: if (error != OT_ERROR_NONE) { - HandleCommandPropertyGet(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_MAC_BLACKLIST); + WritePropertyValueIsFrame(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_MAC_BLACKLIST); } return error; @@ -1975,7 +1975,7 @@ exit: if (error != OT_ERROR_NONE) { - HandleCommandPropertyGet(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_MAC_FIXED_RSS); + WritePropertyValueIsFrame(SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0, SPINEL_PROP_MAC_FIXED_RSS); } return error; @@ -2115,7 +2115,7 @@ otError NcpBase::SetPropertyHandler_NEST_STREAM_MFG(uint8_t aHeader) error = mDecoder.ReadUtf8(string); - VerifyOrExit(error == OT_ERROR_NONE, error = SendLastStatus(aHeader, ThreadErrorToSpinelStatus(error))); + VerifyOrExit(error == OT_ERROR_NONE, error = WriteLastStatusFrame(aHeader, ThreadErrorToSpinelStatus(error))); // All diagnostics related features are processed within diagnostics module output = otDiagProcessCmdLine(const_cast(string)); @@ -2695,13 +2695,20 @@ void NcpBase::HandleDatagramFromStack(otMessage *aMessage) VerifyOrExit(aMessage != NULL); SuccessOrExit(otMessageQueueEnqueue(&mMessageQueue, aMessage)); - SuccessOrExit(SendQueuedDatagramMessages()); + + // If there is no queued spinel command response, try to write/send + // the datagram message immediately. If there is a queued response + // or if currently out of buffer space, the IPv6 datagram message + // will be sent from `HandleFrameRemovedFromNcpBuffer()` when buffer + // space becomes available and after any pending spinel command + // response. + + if (IsResponseQueueEmpty()) + { + IgnoreReturnValue(SendQueuedDatagramMessages()); + } exit: - // If the queued message can not be sent now (out of buffer), - // it will be sent once spinel buffer becomes available from - // `HandleFrameRemovedFromNcpBuffer()` callback. - return; } diff --git a/src/ncp/ncp_base_radio.cpp b/src/ncp/ncp_base_radio.cpp index 948c44a54..124149114 100644 --- a/src/ncp/ncp_base_radio.cpp +++ b/src/ncp/ncp_base_radio.cpp @@ -191,14 +191,19 @@ exit: return; } +otError NcpBase::GetPropertyHandler_MAC_SRC_MATCH_ENABLED(void) +{ + // TODO: Would be good to add an `otLinkRaw` API to give the the status of source match. + return mEncoder.WriteBool(mSrcMatchEnabled); +} + otError NcpBase::SetPropertyHandler_MAC_SRC_MATCH_ENABLED(void) { - bool enabled; otError error = OT_ERROR_NONE; - SuccessOrExit(error = mDecoder.ReadBool(enabled)); + SuccessOrExit(error = mDecoder.ReadBool(mSrcMatchEnabled)); - error = otLinkRawSrcMatchEnable(mInstance, enabled); + error = otLinkRawSrcMatchEnable(mInstance, mSrcMatchEnabled); exit: return error; @@ -381,7 +386,7 @@ exit: } else { - error = SendLastStatus(aHeader, ThreadErrorToSpinelStatus(error)); + error = WriteLastStatusFrame(aHeader, ThreadErrorToSpinelStatus(error)); } return error;