From 6614d14c34df9218380ddba58bbaa9ab4f6b6aa8 Mon Sep 17 00:00:00 2001 From: Yakun Xu Date: Wed, 4 Mar 2020 14:59:00 +0800 Subject: [PATCH] [coap] reduce code size by avoiding using 64bit division (#4612) - Removed OpenThread's default CoAP configuration macro, because they are fixed values by Thread spec. - Use 32bit multiple only to detect overflow of application CoAP transmission parameters. - Added documentation describing conditions of valid CoAP transmission parameters. --- include/openthread/coap.h | 13 ++++++++++-- src/core/api/coap_api.cpp | 18 ++++++++++++---- src/core/coap/coap.cpp | 33 +++++++++++++++++++++++++++--- src/core/coap/coap.hpp | 18 +++++++++++----- src/core/config/coap.h | 43 --------------------------------------- 5 files changed, 68 insertions(+), 57 deletions(-) diff --git a/include/openthread/coap.h b/include/openthread/coap.h index 8171e535c..dc2a031ac 100644 --- a/include/openthread/coap.h +++ b/include/openthread/coap.h @@ -60,6 +60,10 @@ extern "C" { #define OT_COAP_MAX_TOKEN_LENGTH 8 ///< Max token length as specified (RFC 7252). +#define OT_COAP_MAX_RETRANSMIT 30 ///< Max retransmit supported by OpenThread. + +#define OT_COAP_MIN_ACK_TIMEOUT 1000 ///< Minimal ACK timeout in milliseconds supported by OpenThread. + /** * CoAP Type values. * @@ -777,9 +781,14 @@ otMessage *otCoapNewMessage(otInstance *aInstance, const otMessageSettings *aSet * @param[in] aHandler A function pointer that shall be called on response reception or timeout. * @param[in] aContext A pointer to arbitrary context information. May be NULL if not used. * @param[in] aTxParameters A pointer to transmission parameters for this request. Use NULL for defaults. + * Otherwise, parameters given must meet the following conditions: + * 1. mMaxRetransmit is no more than OT_COAP_MAX_RETRANSMIT. + * 2. mAckRandomFactorNumerator / mAckRandomFactorDenominator must not be below 1.0. + * 3. The calculated exchange life time must not overflow uint32_t. * - * @retval OT_ERROR_NONE Successfully sent CoAP message. - * @retval OT_ERROR_NO_BUFS Failed to allocate retransmission data. + * @retval OT_ERROR_INVALID_ARGS @p aTxParameters is invalid. + * @retval OT_ERROR_NONE Successfully sent CoAP message. + * @retval OT_ERROR_NO_BUFS Failed to allocate retransmission data. * */ otError otCoapSendRequestWithParameters(otInstance * aInstance, diff --git a/src/core/api/coap_api.cpp b/src/core/api/coap_api.cpp index 56abc6a74..a7809120b 100644 --- a/src/core/api/coap_api.cpp +++ b/src/core/api/coap_api.cpp @@ -217,11 +217,21 @@ otError otCoapSendRequestWithParameters(otInstance * aInstance, void * aContext, const otCoapTxParameters *aTxParameters) { - Instance &instance = *static_cast(aInstance); + otError error; + Instance & instance = *static_cast(aInstance); + const Coap::TxParameters &txParameters = Coap::TxParameters::From(aTxParameters); - return instance.GetApplicationCoap().SendMessage(*static_cast(aMessage), - *static_cast(aMessageInfo), - Coap::TxParameters::From(aTxParameters), aHandler, aContext); + if (aTxParameters != NULL) + { + VerifyOrExit(txParameters.IsValid(), error = OT_ERROR_INVALID_ARGS); + } + + error = instance.GetApplicationCoap().SendMessage(*static_cast(aMessage), + *static_cast(aMessageInfo), + txParameters, aHandler, aContext); + +exit: + return error; } otError otCoapStart(otInstance *aInstance, uint16_t aPort) diff --git a/src/core/coap/coap.cpp b/src/core/coap/coap.cpp index 3ab3f1cc2..2bdd55af7 100644 --- a/src/core/coap/coap.cpp +++ b/src/core/coap/coap.cpp @@ -962,6 +962,33 @@ uint32_t ResponsesQueue::ResponseMetadata::GetRemainingTime(void) const return (mDequeueTime > now) ? mDequeueTime - now : 0; } +/// Return product of @p aValueA and @p aValueB if no overflow otherwise 0. +static uint32_t Multiply(uint32_t aValueA, uint32_t aValueB) +{ + uint32_t result = aValueA * aValueB; + + return (result / aValueA == aValueB) ? result : 0; +} + +bool TxParameters::IsValid(void) const +{ + bool rval = false; + + if (mAckRandomFactorNumerator >= mAckRandomFactorDenominator && mAckTimeout >= OT_COAP_MIN_ACK_TIMEOUT && + mMaxRetransmit <= OT_COAP_MAX_RETRANSMIT) + { + // Calulate exchange lifetime step by step and verify no overflow. + uint32_t tmp = Multiply(mAckTimeout, (1U << (mMaxRetransmit + 1)) - 1); + + tmp /= mAckRandomFactorDenominator; + tmp = Multiply(tmp, mAckRandomFactorNumerator); + + rval = (tmp != 0 && (tmp + mAckTimeout + 2 * kDefaultMaxLatency) > tmp); + } + + return rval; +} + uint32_t TxParameters::CalculateInitialRetransmissionTimeout(void) const { return Random::NonCrypto::GetUint32InRange( @@ -979,10 +1006,10 @@ uint32_t TxParameters::CalculateMaxTransmitWait(void) const return CalculateSpan(mMaxRetransmit + 1); } -uint32_t TxParameters::CalculateSpan(uint32_t aMaxRetx) const +uint32_t TxParameters::CalculateSpan(uint8_t aMaxRetx) const { - return static_cast(mAckTimeout * ((1ULL << aMaxRetx) - 1) * mAckRandomFactorNumerator / - mAckRandomFactorDenominator); + return static_cast(mAckTimeout * ((1U << aMaxRetx) - 1) / mAckRandomFactorDenominator * + mAckRandomFactorNumerator); } const otCoapTxParameters TxParameters::kDefaultTxParameters = { diff --git a/src/core/coap/coap.hpp b/src/core/coap/coap.hpp index 259815196..703de08e5 100644 --- a/src/core/coap/coap.hpp +++ b/src/core/coap/coap.hpp @@ -102,6 +102,14 @@ public: return aTxParameters ? *static_cast(aTxParameters) : GetDefault(); } + /** + * This method validates whether the CoAP transmission parameters are valid. + * + * @returns Whether the parameters are valid. + * + */ + bool IsValid(void) const; + /** * This static method returns default CoAP tx parameters. * @@ -113,17 +121,17 @@ public: private: enum { - kDefaultAckTimeout = OPENTHREAD_CONFIG_COAP_ACK_TIMEOUT_MILLIS, - kDefaultAckRandomFactorNumerator = OPENTHREAD_CONFIG_COAP_ACK_RANDOM_FACTOR_NUMERATOR, - kDefaultAckRandomFactorDenominator = OPENTHREAD_CONFIG_COAP_ACK_RANDOM_FACTOR_DENOMINATOR, - kDefaultMaxRetransmit = OPENTHREAD_CONFIG_COAP_MAX_RETRANSMIT, + kDefaultAckTimeout = 2000, // in millisecond + kDefaultAckRandomFactorNumerator = 3, + kDefaultAckRandomFactorDenominator = 2, + kDefaultMaxRetransmit = 4, kDefaultMaxLatency = 100000, // in millisecond }; uint32_t CalculateInitialRetransmissionTimeout(void) const; uint32_t CalculateExchangeLifetime(void) const; uint32_t CalculateMaxTransmitWait(void) const; - uint32_t CalculateSpan(uint32_t aMaxRetx) const; + uint32_t CalculateSpan(uint8_t aMaxRetx) const; static const otCoapTxParameters kDefaultTxParameters; }; diff --git a/src/core/config/coap.h b/src/core/config/coap.h index 5484d02d2..712c57e8d 100644 --- a/src/core/config/coap.h +++ b/src/core/config/coap.h @@ -35,49 +35,6 @@ #ifndef CONFIG_COAP_H_ #define CONFIG_COAP_H_ -/** - * @def OPENTHREAD_CONFIG_COAP_ACK_TIMEOUT_MILLIS - * - * Minimum spacing before first retransmission when ACK is not received, in milliseconds (RFC7252 default value - * is 2000). - * - */ -#ifndef OPENTHREAD_CONFIG_COAP_ACK_TIMEOUT_MILLIS -#define OPENTHREAD_CONFIG_COAP_ACK_TIMEOUT_MILLIS 2000 -#endif - -/** - * @def OPENTHREAD_CONFIG_COAP_ACK_RANDOM_FACTOR_NUMERATOR - * - * Numerator of ACK_RANDOM_FACTOR used to calculate maximum spacing before first retransmission when - * ACK is not received (RFC7252 default value of ACK_RANDOM_FACTOR is 1.5, must not be decreased below 1). - * - */ -#ifndef OPENTHREAD_CONFIG_COAP_ACK_RANDOM_FACTOR_NUMERATOR -#define OPENTHREAD_CONFIG_COAP_ACK_RANDOM_FACTOR_NUMERATOR 3 -#endif - -/** - * @def OPENTHREAD_CONFIG_COAP_ACK_RANDOM_FACTOR_DENOMINATOR - * - * Denominator of ACK_RANDOM_FACTOR used to calculate maximum spacing before first retransmission when - * ACK is not received (RFC7252 default value of ACK_RANDOM_FACTOR is 1.5, must not be decreased below 1). - * - */ -#ifndef OPENTHREAD_CONFIG_COAP_ACK_RANDOM_FACTOR_DENOMINATOR -#define OPENTHREAD_CONFIG_COAP_ACK_RANDOM_FACTOR_DENOMINATOR 2 -#endif - -/** - * @def OPENTHREAD_CONFIG_COAP_MAX_RETRANSMIT - * - * Maximum number of retransmissions for CoAP Confirmable messages (RFC7252 default value is 4). - * - */ -#ifndef OPENTHREAD_CONFIG_COAP_MAX_RETRANSMIT -#define OPENTHREAD_CONFIG_COAP_MAX_RETRANSMIT 4 -#endif - /** * @def OPENTHREAD_CONFIG_COAP_SERVER_MAX_CACHED_RESPONSES *