Ensure spinel messages are not overwritten by log messages. (#1195)

This commit addresses the following issue: while `NcpBase` is in
middle of forming a spinel message, an OT api could trigger a
call to `otNcpWriteStream()` (writing a log message through
spinel) which would then overwrite the spinel message being
formed.

This commit contains two changes to address this issue:

- In many cases (e.g., get handler for `THREAD_NETWORK_DATA`)
  we call the OT api first and then form the entire spinel message
  afterwards.

- In cases where the above approach is not possible (e.g,, get
  handler for `CHILD_TABLE` where we need to iteratively call an
  OT api to get info about the next child and append it to the
  current spinel message), a new flag variable is used to
  temporarily disable the `otNcpWriteStream`.
This commit is contained in:
Abtin Keshavarzian
2017-01-23 19:40:16 -08:00
committed by Jonathan Hui
parent 7c177492f3
commit ce5e73bb9d
2 changed files with 60 additions and 40 deletions
+56 -39
View File
@@ -480,6 +480,7 @@ NcpBase::NcpBase(otInstance *aInstance):
mAllowLocalNetworkDataChange(false),
mRequireJoinExistingNetwork(false),
mIsRawStreamEnabled(false),
mDisableStreamWrite(false),
mFramingErrorCounter(0),
mRxSpinelFrameCounter(0),
@@ -522,6 +523,7 @@ void NcpBase::HandleDatagramFromStack(otMessage aMessage)
{
ThreadError errorCode = kThreadError_None;
bool isSecure = otIsMessageLinkSecurityEnabled(aMessage);
uint16_t length = otGetMessageLength(aMessage);
SuccessOrExit(errorCode = OutboundFrameBegin());
@@ -533,7 +535,7 @@ void NcpBase::HandleDatagramFromStack(otMessage aMessage)
isSecure
? SPINEL_PROP_STREAM_NET
: SPINEL_PROP_STREAM_NET_INSECURE,
otGetMessageLength(aMessage)
length
));
SuccessOrExit(errorCode = OutboundFrameFeedMessage(aMessage));
@@ -593,7 +595,6 @@ void NcpBase::HandleRawFrame(const RadioPacket *aFrame)
SuccessOrExit(errorCode = OutboundFrameBegin());
if (aFrame->mDidTX)
{
flags |= SPINEL_MD_FLAG_TX;
@@ -736,7 +737,7 @@ void NcpBase::HandleEnergyScanResult(otEnergyScanResult *aResult)
SPINEL_SCAN_STATE_IDLE
);
// If we could not send the end of scan inidciator message now (no
// If we could not send the end of scan indicator message now (no
// buffer space), we set `mShouldSignalEndOfScan` to true to send
// it out when buffer space becomes available.
if (errorCode != kThreadError_None)
@@ -1510,15 +1511,12 @@ ThreadError NcpBase::GetPropertyHandler_CAPS(uint8_t header, spinel_prop_key_t k
ThreadError errorCode = kThreadError_None;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
// Begin adding capabilities //////////////////////////////////////////////
SuccessOrExit(errorCode = OutboundFrameFeedPacked(SPINEL_DATATYPE_UINT_PACKED_S, SPINEL_CAP_NET_THREAD_1_0));
SuccessOrExit(errorCode = OutboundFrameFeedPacked(SPINEL_DATATYPE_UINT_PACKED_S, SPINEL_CAP_COUNTERS));
SuccessOrExit(errorCode = OutboundFrameFeedPacked(SPINEL_DATATYPE_UINT_PACKED_S, SPINEL_CAP_MAC_WHITELIST));
#if OPENTHREAD_ENABLE_JAM_DETECTION
@@ -1717,7 +1715,6 @@ ThreadError NcpBase::GetPropertyHandler_ChannelMaskHelper(uint8_t header, spinel
ThreadError errorCode = kThreadError_None;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
for (int i = 0; i < 32; i++)
@@ -1953,14 +1950,15 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_NETWORK_DATA(uint8_t header, spin
uint8_t network_data[255];
uint8_t network_data_len = 255;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
otGetNetworkDataLocal(
mInstance,
false, // Stable?
network_data,
&network_data_len
);
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
SuccessOrExit(errorCode = OutboundFrameFeedData(network_data, network_data_len));
SuccessOrExit(errorCode = OutboundFrameSend());
@@ -1974,10 +1972,6 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_STABLE_NETWORK_DATA(uint8_t heade
uint8_t network_data[255];
uint8_t network_data_len = 255;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
otGetNetworkDataLocal(
mInstance,
true, // Stable?
@@ -1985,6 +1979,8 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_STABLE_NETWORK_DATA(uint8_t heade
&network_data_len
);
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
SuccessOrExit(errorCode = OutboundFrameFeedData(network_data, network_data_len));
SuccessOrExit(errorCode = OutboundFrameSend());
@@ -1998,14 +1994,15 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_LEADER_NETWORK_DATA(uint8_t heade
uint8_t network_data[255];
uint8_t network_data_len = 255;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
otGetNetworkDataLeader(
mInstance,
false, // Stable?
network_data,
&network_data_len
);
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
SuccessOrExit(errorCode = OutboundFrameFeedData(network_data, network_data_len));
SuccessOrExit(errorCode = OutboundFrameSend());
@@ -2019,10 +2016,6 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_STABLE_LEADER_NETWORK_DATA(uint8_
uint8_t network_data[255];
uint8_t network_data_len = 255;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
otGetNetworkDataLeader(
mInstance,
true, // Stable?
@@ -2030,6 +2023,8 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_STABLE_LEADER_NETWORK_DATA(uint8_
&network_data_len
);
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
SuccessOrExit(errorCode = OutboundFrameFeedData(network_data, network_data_len));
SuccessOrExit(errorCode = OutboundFrameSend());
@@ -2128,6 +2123,8 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_CHILD_TABLE(uint8_t header, spine
uint8_t index;
uint8_t modeFlags;
mDisableStreamWrite = true;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
@@ -2188,6 +2185,7 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_CHILD_TABLE(uint8_t header, spine
SuccessOrExit(errorCode = OutboundFrameSend());
exit:
mDisableStreamWrite = false;
return errorCode;
}
@@ -2198,6 +2196,8 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_NEIGHBOR_TABLE(uint8_t header, sp
otNeighborInfo neighInfo;
uint8_t modeFlags;
mDisableStreamWrite = true;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
@@ -2253,6 +2253,7 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_NEIGHBOR_TABLE(uint8_t header, sp
SuccessOrExit(errorCode = OutboundFrameSend());
exit:
mDisableStreamWrite = false;
return errorCode;
}
@@ -2263,7 +2264,6 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_ASSISTING_PORTS(uint8_t header, s
const uint16_t *ports = otGetUnsecurePorts(mInstance, &num_entries);
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
for (; num_entries != 0; ports++, num_entries--)
@@ -2305,8 +2305,9 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_ON_MESH_NETS(uint8_t header, spin
otBorderRouterConfig border_router_config;
uint8_t flags;
SuccessOrExit(errorCode = OutboundFrameBegin());
mDisableStreamWrite = true;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
// Fill from non-local network data first
@@ -2368,6 +2369,7 @@ ThreadError NcpBase::GetPropertyHandler_THREAD_ON_MESH_NETS(uint8_t header, spin
SuccessOrExit(errorCode = OutboundFrameSend());
exit:
mDisableStreamWrite = false;
return errorCode;
}
@@ -2448,9 +2450,9 @@ ThreadError NcpBase::GetPropertyHandler_IPV6_ADDRESS_TABLE(uint8_t header, spine
{
ThreadError errorCode = kThreadError_None;
mDisableStreamWrite = true;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
for (const otNetifAddress *address = otGetUnicastAddresses(mInstance); address; address = address->mNext)
@@ -2468,6 +2470,7 @@ ThreadError NcpBase::GetPropertyHandler_IPV6_ADDRESS_TABLE(uint8_t header, spine
SuccessOrExit(errorCode = OutboundFrameSend());
exit:
mDisableStreamWrite = false;
return errorCode;
}
@@ -2723,7 +2726,6 @@ ThreadError NcpBase::GetPropertyHandler_MAC_CNTR(uint8_t header, spinel_prop_key
value = macCounters->mRxErrOther;
break;
default:
errorCode = SendLastStatus(header, SPINEL_STATUS_INTERNAL_ERROR);
goto bail;
@@ -2811,9 +2813,7 @@ ThreadError NcpBase::GetPropertyHandler_MSG_BUFFER_COUNTERS(uint8_t header, spin
otGetMessageBufferInfo(mInstance, &bufferInfo);
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
SuccessOrExit(errorCode = OutboundFrameFeedPacked("T(SSSSSSSSSSSSSSSS)",
bufferInfo.mTotalBuffers,
bufferInfo.mFreeBuffers,
@@ -2832,7 +2832,6 @@ ThreadError NcpBase::GetPropertyHandler_MSG_BUFFER_COUNTERS(uint8_t header, spin
bufferInfo.mCoapClientMessages,
bufferInfo.mCoapClientBuffers
));
SuccessOrExit(errorCode = OutboundFrameSend());
exit:
@@ -2844,8 +2843,9 @@ ThreadError NcpBase::GetPropertyHandler_MAC_WHITELIST(uint8_t header, spinel_pro
otMacWhitelistEntry entry;
ThreadError errorCode = kThreadError_None;
SuccessOrExit(errorCode = OutboundFrameBegin());
mDisableStreamWrite = true;
SuccessOrExit(errorCode = OutboundFrameBegin());
SuccessOrExit(errorCode = OutboundFrameFeedPacked("Cii", header, SPINEL_CMD_PROP_VALUE_IS, key));
for (uint8_t i = 0; (i != 255) && (errorCode == kThreadError_None); i++)
@@ -2871,6 +2871,7 @@ ThreadError NcpBase::GetPropertyHandler_MAC_WHITELIST(uint8_t header, spinel_pro
SuccessOrExit(errorCode = OutboundFrameSend());
exit:
mDisableStreamWrite = false;
return errorCode;
}
@@ -5452,23 +5453,18 @@ exit:
#endif // OPENTHREAD_ENABLE_LEGACY
} // namespace Thread
// ----------------------------------------------------------------------------
// MARK: Virtual Datastream I/O (Public API)
// ----------------------------------------------------------------------------
ThreadError otNcpStreamWrite(int aStreamId, const uint8_t* aDataPtr, int aDataLen)
ThreadError NcpBase::StreamWrite(int aStreamId, const uint8_t *aDataPtr, int aDataLen)
{
ThreadError errorCode = kThreadError_None;
if (aStreamId == 0)
{
aStreamId = SPINEL_PROP_STREAM_DEBUG;
}
if (Thread::sNcpContext)
if (!mDisableStreamWrite)
{
return Thread::sNcpContext->SendPropertyUpdate(
errorCode = SendPropertyUpdate(
SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0,
SPINEL_CMD_PROP_VALUE_IS,
static_cast<spinel_prop_key_t>(aStreamId),
@@ -5478,8 +5474,29 @@ ThreadError otNcpStreamWrite(int aStreamId, const uint8_t* aDataPtr, int aDataLe
}
else
{
return kThreadError_InvalidState;
errorCode = kThreadError_InvalidState;
}
return errorCode;
}
} // namespace Thread
// ----------------------------------------------------------------------------
// MARK: Virtual Datastream I/O (Public API)
// ----------------------------------------------------------------------------
ThreadError otNcpStreamWrite(int aStreamId, const uint8_t* aDataPtr, int aDataLen)
{
ThreadError errorCode = kThreadError_InvalidState;
if (Thread::sNcpContext)
{
errorCode = Thread::sNcpContext->StreamWrite(aStreamId, aDataPtr, aDataLen);
}
return errorCode;
}
// ----------------------------------------------------------------------------
+4 -1
View File
@@ -213,7 +213,7 @@ private:
ThreadError SendLastStatus(uint8_t header, spinel_status_t lastStatus);
public:
private:
ThreadError SendPropertyUpdate(uint8_t header, uint8_t command, spinel_prop_key_t key, const uint8_t *value_ptr,
uint16_t value_len);
@@ -484,6 +484,8 @@ private:
ThreadError RemovePropertyHandler_MAC_WHITELIST(uint8_t header, spinel_prop_key_t key, const uint8_t *value_ptr, uint16_t value_len);
ThreadError RemovePropertyHandler_THREAD_ACTIVE_ROUTER_IDS(uint8_t header, spinel_prop_key_t key, const uint8_t *value_ptr,
uint16_t value_len);
public:
ThreadError StreamWrite(int aStreamId, const uint8_t *aDataPtr, int aDataLen);
#if OPENTHREAD_ENABLE_LEGACY
public:
@@ -519,6 +521,7 @@ private:
bool mAllowLocalNetworkDataChange;
bool mRequireJoinExistingNetwork;
bool mIsRawStreamEnabled;
bool mDisableStreamWrite;
uint32_t mFramingErrorCounter; // Number of improperly formed received spinel frames.
uint32_t mRxSpinelFrameCounter; // Number of received (inbound) spinel frames.