From b95281666d33dbf568bd3ed960c62c49a9083c20 Mon Sep 17 00:00:00 2001 From: Yakun Xu Date: Wed, 13 Nov 2019 14:36:18 +0800 Subject: [PATCH] [radio] process tx AES without otInstance (#4318) `otPlatRadioFrameUpdated()` is declared not to access any state within OpenThread. However, the current implementation does read the extended address. This commit moves the AES process into `Mac::TxFrame`, which is free of `otInstance` and eliminates the current deep callback stacks when the radio driver wants to process tx AES from interrupt context. --- examples/platforms/nrf528xx/src/radio.c | 9 ++++++- examples/platforms/posix/radio.c | 2 +- examples/platforms/utils/mac_frame.cpp | 5 ++++ examples/platforms/utils/mac_frame.h | 10 +++++++ include/openthread/platform/radio.h | 15 ----------- src/core/mac/mac.cpp | 27 +------------------ src/core/mac/mac.hpp | 10 ------- src/core/mac/mac_frame.cpp | 36 +++++++++++++++++++++++++ src/core/mac/mac_frame.hpp | 8 ++++++ src/core/mac/sub_mac.cpp | 7 ----- src/core/mac/sub_mac.hpp | 17 ------------ src/core/mac/sub_mac_callbacks.cpp | 29 -------------------- src/core/radio/radio.hpp | 16 ----------- src/core/radio/radio_callbacks.cpp | 7 ----- src/core/radio/radio_platform.cpp | 12 --------- 15 files changed, 69 insertions(+), 141 deletions(-) diff --git a/examples/platforms/nrf528xx/src/radio.c b/examples/platforms/nrf528xx/src/radio.c index 34c989211..86423aa51 100644 --- a/examples/platforms/nrf528xx/src/radio.c +++ b/examples/platforms/nrf528xx/src/radio.c @@ -42,6 +42,7 @@ #include #include "utils/code_utils.h" +#include "utils/mac_frame.h" #include #include @@ -84,6 +85,7 @@ enum static bool sDisabled; +static otExtAddress sExtAddress; static otError sReceiveError = OT_ERROR_NONE; static otRadioFrame sReceivedFrames[NRF_802154_RX_BUFFERS]; static otRadioFrame sTransmitFrame; @@ -221,6 +223,11 @@ void otPlatRadioSetExtendedAddress(otInstance *aInstance, const otExtAddress *aE { OT_UNUSED_VARIABLE(aInstance); + for (size_t i = 0; i < sizeof(*aExtAddress); i++) + { + sExtAddress.m8[i] = aExtAddress->m8[sizeof(*aExtAddress) - 1 - i]; + } + nrf_802154_extended_address_set(aExtAddress->m8); } @@ -921,7 +928,7 @@ void nrf_802154_tx_started(const uint8_t *aFrame) if (notifyFrameUpdated) { - otPlatRadioFrameUpdated(sInstance, &sTransmitFrame); + otMacFrameProcessTransmitAesCcm(&sTransmitFrame, &sExtAddress); } } #endif diff --git a/examples/platforms/posix/radio.c b/examples/platforms/posix/radio.c index b93d35873..51e0c9123 100644 --- a/examples/platforms/posix/radio.c +++ b/examples/platforms/posix/radio.c @@ -545,7 +545,7 @@ void radioSendMessage(otInstance *aInstance) if (notifyFrameUpdated) { - otPlatRadioFrameUpdated(aInstance, &sTransmitFrame); + otMacFrameProcessTransmitAesCcm(&sTransmitFrame, &sExtAddress); } #endif // OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT diff --git a/examples/platforms/utils/mac_frame.cpp b/examples/platforms/utils/mac_frame.cpp index 99ddddff7..276e38bf5 100644 --- a/examples/platforms/utils/mac_frame.cpp +++ b/examples/platforms/utils/mac_frame.cpp @@ -115,3 +115,8 @@ uint8_t otMacFrameGetSequence(const otRadioFrame *aFrame) { return static_cast(aFrame)->GetSequence(); } + +void otMacFrameProcessTransmitAesCcm(otRadioFrame *aFrame, const otExtAddress *aExtAddress) +{ + static_cast(aFrame)->ProcessTransmitAesCcm(*static_cast(aExtAddress)); +} diff --git a/examples/platforms/utils/mac_frame.h b/examples/platforms/utils/mac_frame.h index 79343bd4d..429a75339 100644 --- a/examples/platforms/utils/mac_frame.h +++ b/examples/platforms/utils/mac_frame.h @@ -140,6 +140,16 @@ otError otMacFrameGetSrcAddr(const otRadioFrame *aFrame, otMacAddress *aMacAddre */ uint8_t otMacFrameGetSequence(const otRadioFrame *aFrame); +/** + * This function performs AES CCM on the frame which is going to be sent. + * + * @param[in] aFrame A pointer to the MAC frame buffer that is going to be sent. + * @param[in] aExtAddress A pointer to the extended address, which will be used to generate nonce + * for AES CCM computation. + * + */ +void otMacFrameProcessTransmitAesCcm(otRadioFrame *aFrame, const otExtAddress *aExtAddress); + #ifdef __cplusplus } // extern "C" #endif diff --git a/include/openthread/platform/radio.h b/include/openthread/platform/radio.h index a40b0103a..a1e9e01f6 100644 --- a/include/openthread/platform/radio.h +++ b/include/openthread/platform/radio.h @@ -610,21 +610,6 @@ extern void otPlatRadioTxDone(otInstance *aInstance, otRadioFrame *aFrame, otRad */ extern void otPlatDiagRadioTransmitDone(otInstance *aInstance, otRadioFrame *aFrame, otError aError); -/** - * The radio driver calls this method to notify OpenThread to process transmit security for the frame, - * this happens when the frame includes Header IE(s) that were updated before transmission. - * - * This function is used when feature `OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT` is enabled. - * - * @note This function can be called from interrupt context and it would only read/write data passed in - * via @p aFrame, but would not read/write any state within OpenThread. - * - * @param[in] aInstance The OpenThread instance structure. - * @param[in] aFrame The radio frame which needs to process transmit security. - * - */ -extern void otPlatRadioFrameUpdated(otInstance *aInstance, otRadioFrame *aFrame); - /** * Get the most recent RSSI measurement. * diff --git a/src/core/mac/mac.cpp b/src/core/mac/mac.cpp index dd0c4e09a..747ba6067 100644 --- a/src/core/mac/mac.cpp +++ b/src/core/mac/mac.cpp @@ -879,31 +879,6 @@ bool Mac::IsJoinable(void) const return (numUnsecurePorts != 0); } -void Mac::ProcessTransmitAesCcm(TxFrame &aFrame, const ExtAddress *aExtAddress) -{ - uint32_t frameCounter = 0; - uint8_t securityLevel; - uint8_t nonce[KeyManager::kNonceSize]; - uint8_t tagLength; - Crypto::AesCcm aesCcm; - otError error; - - aFrame.GetSecurityLevel(securityLevel); - aFrame.GetFrameCounter(frameCounter); - - KeyManager::GenerateNonce(*aExtAddress, frameCounter, securityLevel, nonce); - - aesCcm.SetKey(aFrame.GetAesKey(), 16); - tagLength = aFrame.GetFooterLength() - Frame::kFcsSize; - - error = aesCcm.Init(aFrame.GetHeaderLength(), aFrame.GetPayloadLength(), tagLength, nonce, sizeof(nonce)); - assert(error == OT_ERROR_NONE); - - aesCcm.Header(aFrame.GetHeader(), aFrame.GetHeaderLength()); - aesCcm.Payload(aFrame.GetPayload(), aFrame.GetPayload(), aFrame.GetPayloadLength(), true); - aesCcm.Finalize(aFrame.GetFooter(), &tagLength); -} - void Mac::ProcessTransmitSecurity(TxFrame &aFrame, bool aProcessAesCcm) { KeyManager & keyManager = Get(); @@ -966,7 +941,7 @@ void Mac::ProcessTransmitSecurity(TxFrame &aFrame, bool aProcessAesCcm) if (aProcessAesCcm) { - ProcessTransmitAesCcm(aFrame, extAddress); + aFrame.ProcessTransmitAesCcm(*extAddress); } exit: diff --git a/src/core/mac/mac.hpp b/src/core/mac/mac.hpp index 968fcf760..5521bb192 100644 --- a/src/core/mac/mac.hpp +++ b/src/core/mac/mac.hpp @@ -613,16 +613,6 @@ public: */ bool IsEnabled(void) const { return mEnabled; } - /** - * This method performs AES CCM on the frame which is going to be sent. - * - * @param[in] aFrame A reference to the MAC frame buffer that is going to be sent. - * @param[in] aExtAddress A pointer to the extended address, which will be used to generate nonce - * for AES CCM computation. - * - */ - static void ProcessTransmitAesCcm(TxFrame &aFrame, const ExtAddress *aExtAddress); - private: enum { diff --git a/src/core/mac/mac_frame.cpp b/src/core/mac/mac_frame.cpp index 8be6cf117..4ff4dea64 100644 --- a/src/core/mac/mac_frame.cpp +++ b/src/core/mac/mac_frame.cpp @@ -37,6 +37,8 @@ #include "common/code_utils.hpp" #include "common/debug.hpp" +#include "crypto/aes_ccm.hpp" +#include "thread/key_manager.hpp" namespace ot { namespace Mac { @@ -998,6 +1000,40 @@ void TxFrame::CopyFrom(const TxFrame &aFromFrame) #endif } +void TxFrame::ProcessTransmitAesCcm(const ExtAddress &aExtAddress) +{ +#if OPENTHREAD_RADIO + OT_UNUSED_VARIABLE(aExtAddress); +#else + uint32_t frameCounter = 0; + uint8_t securityLevel; + uint8_t nonce[KeyManager::kNonceSize]; + uint8_t tagLength; + Crypto::AesCcm aesCcm; + otError error; + + VerifyOrExit(GetSecurityEnabled()); + + SuccessOrExit(error = GetSecurityLevel(securityLevel)); + SuccessOrExit(error = GetFrameCounter(frameCounter)); + + KeyManager::GenerateNonce(aExtAddress, frameCounter, securityLevel, nonce); + + aesCcm.SetKey(GetAesKey(), 16); + tagLength = GetFooterLength() - Frame::kFcsSize; + + error = aesCcm.Init(GetHeaderLength(), GetPayloadLength(), tagLength, nonce, sizeof(nonce)); + assert(error == OT_ERROR_NONE); + + aesCcm.Header(GetHeader(), GetHeaderLength()); + aesCcm.Payload(GetPayload(), GetPayload(), GetPayloadLength(), true); + aesCcm.Finalize(GetFooter(), &tagLength); + +exit: + return; +#endif // OPENTHREAD_MTD || OPENTHREAD_FTD +} + // LCOV_EXCL_START #if (OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_NOTE) && (OPENTHREAD_CONFIG_LOG_MAC == 1) diff --git a/src/core/mac/mac_frame.hpp b/src/core/mac/mac_frame.hpp index ed530fcd2..9b16cd1d2 100644 --- a/src/core/mac/mac_frame.hpp +++ b/src/core/mac/mac_frame.hpp @@ -1095,6 +1095,14 @@ public: */ void CopyFrom(const TxFrame &aFromFrame); + /** + * This method performs AES CCM on the frame which is going to be sent. + * + * @param[in] aExtAddress A reference to the extended address, which will be used to generate nonce + * for AES CCM computation. + * + */ + void ProcessTransmitAesCcm(const ExtAddress &aExtAddress); #if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE /** * This method sets the Time IE offset. diff --git a/src/core/mac/sub_mac.cpp b/src/core/mac/sub_mac.cpp index 547df5ffb..775b73be9 100644 --- a/src/core/mac/sub_mac.cpp +++ b/src/core/mac/sub_mac.cpp @@ -558,13 +558,6 @@ exit: return swEnergyScan; } -#if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT -void SubMac::HandleFrameUpdated(TxFrame &aFrame) -{ - mCallbacks.FrameUpdated(aFrame); -} -#endif - void SubMac::SetState(State aState) { if (mState != aState) diff --git a/src/core/mac/sub_mac.hpp b/src/core/mac/sub_mac.hpp index dd752892a..e406e9dd5 100644 --- a/src/core/mac/sub_mac.hpp +++ b/src/core/mac/sub_mac.hpp @@ -167,20 +167,6 @@ public: * */ void EnergyScanDone(int8_t aMaxRssi); - -#if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT - /** - * The method notifies user of `SubMac` to process transmit security for the frame, which happens when the - * frame includes Header IE(s) that were updated before transmission. - * - * @note This function can be called from interrupt context and it would only read/write data passed in - * via @p aFrame, but would not read/write any state within OpenThread. - * - * @param[in] aFrame The frame which needs to process transmit security. - * - */ - void FrameUpdated(TxFrame &aFrame); -#endif }; /** @@ -393,9 +379,6 @@ private: void HandleTransmitStarted(TxFrame &aFrame); void HandleTransmitDone(TxFrame &aTxFrame, RxFrame *aAckFrame, otError aError); void HandleEnergyScanDone(int8_t aMaxRssi); -#if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT - void HandleFrameUpdated(TxFrame &aFrame); -#endif static void HandleTimer(Timer &aTimer); void HandleTimer(void); diff --git a/src/core/mac/sub_mac_callbacks.cpp b/src/core/mac/sub_mac_callbacks.cpp index f6e3d0994..10145941e 100644 --- a/src/core/mac/sub_mac_callbacks.cpp +++ b/src/core/mac/sub_mac_callbacks.cpp @@ -104,22 +104,6 @@ void SubMac::Callbacks::EnergyScanDone(int8_t aMaxRssi) } } -#if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT -void SubMac::Callbacks::FrameUpdated(TxFrame &aFrame) -{ - /** - * This function will be called from interrupt context, it should only read/write data passed in - * via @p aFrame, but should not read/write any state within OpenThread. - * - */ - - if (aFrame.GetSecurityEnabled()) - { - Get().ProcessTransmitAesCcm(aFrame, &Get().GetExtAddress()); - } -} -#endif - #elif OPENTHREAD_RADIO void SubMac::Callbacks::ReceiveDone(RxFrame *aFrame, otError aError) @@ -150,19 +134,6 @@ void SubMac::Callbacks::EnergyScanDone(int8_t aMaxRssi) Get().InvokeEnergyScanDone(aMaxRssi); } -#if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT -void SubMac::Callbacks::FrameUpdated(TxFrame &) -{ - /** - * This function will be called from interrupt context, it should only read/write data passed in - * via @p aFrame, but should not read/write any state within OpenThread. - * - */ - - // For now this functionality is not supported in Radio Only mode. -} -#endif - #endif // OPENTHREAD_RADIO } // namespace Mac diff --git a/src/core/radio/radio.hpp b/src/core/radio/radio.hpp index 91aca839f..9f99e649e 100644 --- a/src/core/radio/radio.hpp +++ b/src/core/radio/radio.hpp @@ -147,22 +147,6 @@ public: */ void HandleEnergyScanDone(int8_t aMaxRssi); -#if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT - /** - * This callback method handles a "Frame Updated" event from radio platform. - * - * This is called to notify OpenThread to process transmit security for the frame, this happens when the frame - * includes Header IE(s) that were updated before transmission. It is called from `otPlatRadioFrameUpdated()`. - * - * @note This method can be called from interrupt context and it would only read/write data passed in - * via @p aFrame, but would not read/write any state within OpenThread. - * - * @param[in] aFrame The frame which needs to process transmit security. - * - */ - void HandleFrameUpdated(Mac::TxFrame &aFrame); -#endif - #if OPENTHREAD_CONFIG_DIAG_ENABLE /** * This callback method handles a "Receive Done" event from radio platform when diagnostics mode is enabled. diff --git a/src/core/radio/radio_callbacks.cpp b/src/core/radio/radio_callbacks.cpp index 9352be462..c03706109 100644 --- a/src/core/radio/radio_callbacks.cpp +++ b/src/core/radio/radio_callbacks.cpp @@ -58,13 +58,6 @@ void Radio::Callbacks::HandleEnergyScanDone(int8_t aMaxRssi) Get().HandleEnergyScanDone(aMaxRssi); } -#if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT -void Radio::Callbacks::HandleFrameUpdated(Mac::TxFrame &aFrame) -{ - Get().HandleFrameUpdated(aFrame); -} -#endif - #if OPENTHREAD_CONFIG_DIAG_ENABLE void Radio::Callbacks::HandleDiagsReceiveDone(Mac::RxFrame *aFrame, otError aError) { diff --git a/src/core/radio/radio_platform.cpp b/src/core/radio/radio_platform.cpp index 9d978ecf5..f1375afe0 100644 --- a/src/core/radio/radio_platform.cpp +++ b/src/core/radio/radio_platform.cpp @@ -81,18 +81,6 @@ extern "C" void otPlatRadioEnergyScanDone(otInstance *aInstance, int8_t aEnergyS } } -#if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT -extern "C" void otPlatRadioFrameUpdated(otInstance *aInstance, otRadioFrame *aFrame) -{ - Instance *instance = static_cast(aInstance); - - if (instance->IsInitialized()) - { - instance->Get().HandleFrameUpdated(*static_cast(aFrame)); - } -} -#endif - #if OPENTHREAD_CONFIG_DIAG_ENABLE extern "C" void otPlatDiagRadioReceiveDone(otInstance *aInstance, otRadioFrame *aFrame, otError aError) {