From 02bdc692a29c4783141e880e42e48a14081ef7e7 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 15 Aug 2017 09:11:19 -0700 Subject: [PATCH] [ncp] handle `NOT_FOUND` and `ALREADY` errors during `VALUE_INSERT`/`VALUE_REMOVE` commands (#2103) This commit changes the error type returned from `VALUE_IS` and `VALUE_REMOVE` spinel commands. If a `VALUE_REMOVE` is issued where the item being removed is not in the list, instead of passing up `NOT_FOUND` error, a `LAST_STATUS` with `STATUS_OK` is used. Similarly the behavior is changed for the `VALUE_INSERT` operation and error status `ALREADY`. This change helps ensure driver/user considers the operation as a success while still being able to distinguish (if required) whether the value was actually removed by checking if we get a `VALUE_REMOVED` response versus `LAST_STATUS(OK)` response. --- include/openthread/border_router.h | 6 ++++-- include/openthread/ip6.h | 2 ++ include/openthread/thread_ftd.h | 4 +++- src/ncp/ncp_base_ftd.cpp | 5 +++++ src/ncp/ncp_base_mtd.cpp | 33 ++++++++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 3 deletions(-) diff --git a/include/openthread/border_router.h b/include/openthread/border_router.h index 37871f80b..39324d6ed 100644 --- a/include/openthread/border_router.h +++ b/include/openthread/border_router.h @@ -81,7 +81,8 @@ OTAPI otError OTCALL otBorderRouterAddOnMeshPrefix(otInstance *aInstance, const * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aPrefix A pointer to an IPv6 prefix. * - * @retval OT_ERROR_NONE Successfully removed the configuration from the local network data. + * @retval OT_ERROR_NONE Successfully removed the configuration from the local network data. + * @retval OT_ERROR_NOT_FOUND Could not find the Border Router entry. * * @sa otBorderRouterAddOnMeshPrefix * @sa otBorderRouterRegister @@ -124,7 +125,8 @@ OTAPI otError OTCALL otBorderRouterAddRoute(otInstance *aInstance, const otExter * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aPrefix A pointer to an IPv6 prefix. * - * @retval OT_ERROR_NONE Successfully removed the configuration from the local network data. + * @retval OT_ERROR_NONE Successfully removed the configuration from the local network data. + * @retval OT_ERROR_NOT_FOUND Could not find the Border Router entry. * * @sa otBorderRouterAddRoute * @sa otBorderRouterRegister diff --git a/include/openthread/ip6.h b/include/openthread/ip6.h index a464d71ea..9dab4f0bd 100644 --- a/include/openthread/ip6.h +++ b/include/openthread/ip6.h @@ -123,8 +123,10 @@ OTAPI const otNetifAddress *OTCALL otIp6GetUnicastAddresses(otInstance *aInstanc * @param[in] aAddress A pointer to an IP Address. * * @retval OT_ERROR_NONE Successfully subscribed to the Network Interface Multicast Address. + * @retval OT_ERROR_ALREADY The multicast address is already subscribed. * @retval OT_ERROR_INVALID_ARGS The IP Address indicated by @p aAddress is invalid address. * @retval OT_ERROR_NO_BUFS The Network Interface is already storing the maximum allowed external multicast addresses. + * */ otError otIp6SubscribeMulticastAddress(otInstance *aInstance, const otIp6Address *aAddress); diff --git a/include/openthread/thread_ftd.h b/include/openthread/thread_ftd.h index eeffdf573..4ed933b90 100644 --- a/include/openthread/thread_ftd.h +++ b/include/openthread/thread_ftd.h @@ -266,7 +266,9 @@ OTAPI void OTCALL otThreadSetRouterUpgradeThreshold(otInstance *aInstance, uint8 * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aRouterId The Router ID to release. Valid range is [0, 62]. * - * @retval OT_ERROR_NONE Successfully released the Router ID specified by aRouterId. + * @retval OT_ERROR_NONE Successfully released the Router ID specified by aRouterId. + * @retval OT_ERROR_INVALID_STATE The Router ID was not allocated. + * */ OTAPI otError OTCALL otThreadReleaseRouterId(otInstance *aInstance, uint8_t aRouterId); diff --git a/src/ncp/ncp_base_ftd.cpp b/src/ncp/ncp_base_ftd.cpp index 17948513d..50835955b 100644 --- a/src/ncp/ncp_base_ftd.cpp +++ b/src/ncp/ncp_base_ftd.cpp @@ -669,6 +669,11 @@ otError NcpBase::RemovePropertyHandler_THREAD_ACTIVE_ROUTER_IDS(uint8_t aHeader, VerifyOrExit(parsedLength > 0, spinelError = SPINEL_STATUS_PARSE_ERROR); error = otThreadReleaseRouterId(mInstance, routerId); + + // `INVALID_STATE` is returned when router ID was not allocated (i.e. not in the list) + // in such a case, the "remove" operation can be considered successful. + VerifyOrExit(error != OT_ERROR_INVALID_STATE, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( diff --git a/src/ncp/ncp_base_mtd.cpp b/src/ncp/ncp_base_mtd.cpp index 7722c469e..b96318409 100644 --- a/src/ncp/ncp_base_mtd.cpp +++ b/src/ncp/ncp_base_mtd.cpp @@ -1228,6 +1228,10 @@ otError NcpBase::RemovePropertyHandler_THREAD_ON_MESH_NETS(uint8_t aHeader, spin error = otBorderRouterRemoveOnMeshPrefix(mInstance, &ip6Prefix); + // If prefix was not on the list, "remove" command is successful, + // and we respond with a `SPINEL_STATUS_OK` status. + VerifyOrExit(error != OT_ERROR_NOT_FOUND, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( @@ -1573,6 +1577,11 @@ otError NcpBase::RemovePropertyHandler_IPV6_ADDRESS_TABLE(uint8_t aHeader, spine VerifyOrExit(parsedLength > 0, spinelError = SPINEL_STATUS_PARSE_ERROR); error = otIp6RemoveUnicastAddress(mInstance, addrPtr); + + // If address was not on the list, "remove" command is successful, + // and we respond with a `SPINEL_STATUS_OK` status. + VerifyOrExit(error != OT_ERROR_NOT_FOUND, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( @@ -1686,6 +1695,9 @@ otError NcpBase::InsertPropertyHandler_IPV6_MULTICAST_ADDRESS_TABLE(uint8_t aHea VerifyOrExit(parsedLength > 0, spinelError = SPINEL_STATUS_PARSE_ERROR); error = otIp6SubscribeMulticastAddress(mInstance, addrPtr); + + VerifyOrExit(error != OT_ERROR_ALREADY, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( @@ -1726,6 +1738,11 @@ otError NcpBase::RemovePropertyHandler_IPV6_MULTICAST_ADDRESS_TABLE(uint8_t aHea VerifyOrExit(parsedLength > 0, spinelError = SPINEL_STATUS_PARSE_ERROR); error = otIp6UnsubscribeMulticastAddress(mInstance, addrPtr); + + // If the address was not on the list, "remove" command is successful, + // and we respond with a `SPINEL_STATUS_OK` status. + VerifyOrExit(error != OT_ERROR_NOT_FOUND, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( @@ -1962,6 +1979,11 @@ otError NcpBase::RemovePropertyHandler_THREAD_OFF_MESH_ROUTES(uint8_t aHeader, s ip6Prefix.mPrefix = *addrPtr; error = otBorderRouterRemoveRoute(mInstance, &ip6Prefix); + + // If the route prefix was not on the list, "remove" command is successful, + // and we respond with a `SPINEL_STATUS_OK` status. + VerifyOrExit(error != OT_ERROR_NOT_FOUND, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( @@ -3413,6 +3435,11 @@ otError NcpBase::RemovePropertyHandler_THREAD_ASSISTING_PORTS(uint8_t aHeader, s VerifyOrExit(parsedLength > 0, spinelError = SPINEL_STATUS_PARSE_ERROR); error = otIp6RemoveUnsecurePort(mInstance, port); + + // If unsecure port was not on the list, "remove" command is successful, + // and we respond with a `SPINEL_STATUS_OK` status. + VerifyOrExit(error != OT_ERROR_NOT_FOUND, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( @@ -3454,6 +3481,8 @@ otError NcpBase::RemovePropertyHandler_MAC_WHITELIST(uint8_t aHeader, spinel_pro error = otLinkFilterRemoveAddress(mInstance, extAddress); + VerifyOrExit(error != OT_ERROR_NOT_FOUND, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( @@ -3493,6 +3522,8 @@ otError NcpBase::RemovePropertyHandler_MAC_BLACKLIST(uint8_t aHeader, spinel_pro error = otLinkFilterRemoveAddress(mInstance, extAddress); + VerifyOrExit(error != OT_ERROR_NOT_FOUND, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate( @@ -3535,6 +3566,8 @@ otError NcpBase::RemovePropertyHandler_MAC_FIXED_RSS(uint8_t aHeader, spinel_pro error = otLinkFilterRemoveRssIn(mInstance, extAddress); + VerifyOrExit(error != OT_ERROR_NOT_FOUND, error = SendLastStatus(aHeader, SPINEL_STATUS_OK)); + VerifyOrExit(error == OT_ERROR_NONE, spinelError = ThreadErrorToSpinelStatus(error)); error = SendPropertyUpdate(