From f75d03f6cbd77b3d9ca0a630934114f12560b3fc Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 26 May 2022 09:16:49 -0700 Subject: [PATCH] [mle] delay sending multicast Link Request (#7745) A device sends a multicast Link Request message after becoming a router to quickly and efficiently establish links with neighboring routers. However, sending a Link Request too quickly can fail in cases where the neighboring router(s) have not yet received the updated Router ID set from the Leader. This commit delays sending the Link Request to allow time for neighboring routers to receive the updated Router ID from the Leader. --- src/core/thread/mle_router.cpp | 26 ++++++- src/core/thread/mle_router.hpp | 2 + src/core/thread/mle_types.hpp | 1 + .../thread-cert/Cert_5_2_04_REEDUpgrade.py | 70 ++++++++++--------- .../thread-cert/v1_2_test_parent_selection.py | 4 +- tools/otci/tests/test_otci.py | 4 +- 6 files changed, 66 insertions(+), 41 deletions(-) diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 86335e05f..ccd167687 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -85,6 +85,7 @@ MleRouter::MleRouter(Instance &aInstance) , mPreviousPartitionIdTimeout(0) , mRouterSelectionJitter(kRouterSelectionJitter) , mRouterSelectionJitterTimeout(0) + , mLinkRequestDelay(0) , mParentPriority(kParentPriorityUnspecified) #if OPENTHREAD_CONFIG_BACKBONE_ROUTER_ENABLE , mBackboneRouterRegistrationDelay(0) @@ -196,6 +197,7 @@ Error MleRouter::BecomeRouter(ThreadStatusTlv::Status aStatus) Get().SetRxOnWhenIdle(true); mRouterSelectionJitterTimeout = 0; + mLinkRequestDelay = 0; switch (mRole) { @@ -293,7 +295,7 @@ void MleRouter::HandleDetachStart(void) void MleRouter::HandleChildStart(AttachMode aMode) { - // reset `rejected` flag whenever REED becomes child. + mLinkRequestDelay = 0; mAddressSolicitRejected = false; mRouterSelectionJitterTimeout = 1 + Random::NonCrypto::GetUint8InRange(0, mRouterSelectionJitter); @@ -496,6 +498,11 @@ void MleRouter::SendAdvertisement(void) // children to detach. VerifyOrExit(!mAddressSolicitPending); + // Suppress MLE Advertisements before sending multicast Link Request. + // + // Before sending the multicast Link Request message, no links have been established to neighboring routers. + VerifyOrExit(mLinkRequestDelay == 0); + VerifyOrExit((message = NewMleMessage(kCommandAdvertisement)) != nullptr, error = kErrorNoBufs); SuccessOrExit(error = message->AppendSourceAddressTlv()); SuccessOrExit(error = message->AppendLeaderDataTlv()); @@ -535,6 +542,8 @@ Error MleRouter::SendLinkRequest(Neighbor *aNeighbor) TxMessage * message; Ip6::Address destination; + VerifyOrExit(mLinkRequestDelay == 0 && mChallengeTimeout == 0); + destination.Clear(); VerifyOrExit((message = NewMleMessage(kCommandLinkRequest)) != nullptr, error = kErrorNoBufs); @@ -1422,6 +1431,12 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo) ExitNow(); case kRoleRouter: + if (mLinkRequestDelay > 0 && route.IsRouterIdSet(mRouterId)) + { + mLinkRequestDelay = 0; + IgnoreError(SendLinkRequest(nullptr)); + } + router = mRouterTable.GetRouter(routerId); VerifyOrExit(router != nullptr); @@ -1755,6 +1770,11 @@ void MleRouter::HandleTimeTick(void) VerifyOrExit(IsFullThreadDevice(), Get().UnregisterReceiver(TimeTicker::kMleRouter)); + if (mLinkRequestDelay > 0 && --mLinkRequestDelay == 0) + { + IgnoreError(SendLinkRequest(nullptr)); + } + if (mChallengeTimeout > 0) { mChallengeTimeout--; @@ -3783,13 +3803,13 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message * aMessage, leader->SetNextHop(RouterIdFromRloc16(mParent.GetRloc16())); } - IgnoreError(SendLinkRequest(nullptr)); - for (Child &child : Get().Iterate(Child::kInStateChildIdRequest)) { IgnoreError(SendChildIdResponse(child)); } + mLinkRequestDelay = kMulticastLinkRequestDelay; + exit: // Send announce after received address solicit reply if needed InformPreviousChannel(); diff --git a/src/core/thread/mle_router.hpp b/src/core/thread/mle_router.hpp index fc9434a4b..0ae422ed2 100644 --- a/src/core/thread/mle_router.hpp +++ b/src/core/thread/mle_router.hpp @@ -695,6 +695,8 @@ private: uint8_t mRouterSelectionJitter; ///< The variable to save the assigned jitter value. uint8_t mRouterSelectionJitterTimeout; ///< The Timeout prior to request/release Router ID. + uint8_t mLinkRequestDelay; + int8_t mParentPriority; ///< The assigned parent priority value, -2 means not assigned. #if OPENTHREAD_CONFIG_BACKBONE_ROUTER_ENABLE uint8_t mBackboneRouterRegistrationDelay; ///< Delay before registering Backbone Router service. diff --git a/src/core/thread/mle_types.hpp b/src/core/thread/mle_types.hpp index 29fef937f..ad987dbff 100644 --- a/src/core/thread/mle_types.hpp +++ b/src/core/thread/mle_types.hpp @@ -100,6 +100,7 @@ constexpr uint32_t kMaxResponseDelay = 1000; ///< Max response del constexpr uint32_t kMaxChildIdRequestTimeout = 5000; ///< Max delay to rx a Child ID Request (in msec) constexpr uint32_t kMaxChildUpdateResponseTimeout = 2000; ///< Max delay to rx a Child Update Response (in msec) constexpr uint32_t kMaxLinkRequestTimeout = 2000; ///< Max delay to rx a Link Accept +constexpr uint8_t kMulticastLinkRequestDelay = 5; ///< Max delay for sending a mcast Link Request (in sec) constexpr uint32_t kMinTimeoutKeepAlive = (((kMaxChildKeepAliveAttempts + 1) * kUnicastRetransmissionDelay) / 1000); constexpr uint32_t kMinPollPeriod = OPENTHREAD_CONFIG_MAC_MINIMUM_POLL_PERIOD; diff --git a/tests/scripts/thread-cert/Cert_5_2_04_REEDUpgrade.py b/tests/scripts/thread-cert/Cert_5_2_04_REEDUpgrade.py index e5c9b1449..e97bb72ce 100755 --- a/tests/scripts/thread-cert/Cert_5_2_04_REEDUpgrade.py +++ b/tests/scripts/thread-cert/Cert_5_2_04_REEDUpgrade.py @@ -187,7 +187,7 @@ class Cert_5_2_4_REEDUpgrade(thread_cert.TestCase): self.simulator.go(REED_ADVERTISEMENT_INTERVAL + REED_ADVERTISEMENT_MAX_JITTER) self.nodes[MED].start() - self.simulator.go(5) + self.simulator.go(config.ROUTER_STARTUP_DELAY) self.collect_ipaddrs() mleid = self.nodes[LEADER].get_ip6_address(config.ADDRESS_TYPE.ML_EID) @@ -352,43 +352,45 @@ class Cert_5_2_4_REEDUpgrade(thread_cert.TestCase): # - TLV Request TLV: Link Margin # - Version TLV - pkts.filter_wpan_src64(REED).\ - filter_LLARMA().\ - filter_mle_cmd(MLE_LINK_REQUEST).\ - filter(lambda p: { - CHALLENGE_TLV, - LEADER_DATA_TLV, - SOURCE_ADDRESS_TLV, - VERSION_TLV, - TLV_REQUEST_TLV, - LINK_MARGIN_TLV - } <= set(p.mle.tlv.type) - ).\ - must_next() + with pkts.save_index(): + pkts.filter_wpan_src64(REED).\ + filter_LLARMA().\ + filter_mle_cmd(MLE_LINK_REQUEST).\ + filter(lambda p: { + CHALLENGE_TLV, + LEADER_DATA_TLV, + SOURCE_ADDRESS_TLV, + VERSION_TLV, + TLV_REQUEST_TLV, + LINK_MARGIN_TLV + } <= set(p.mle.tlv.type) + ).\ + must_next() # Step 11: The REED MLE Child ID Response MUST be properly # formatted with MED_1’s new 16-bit address. - pkts.filter_wpan_src64(REED).\ - filter_wpan_dst64(MED).\ - filter_mle_cmd(MLE_CHILD_ID_RESPONSE).\ - filter(lambda p: { - ADDRESS16_TLV, - LEADER_DATA_TLV, - NETWORK_DATA_TLV, - SOURCE_ADDRESS_TLV, - ROUTE64_TLV - } <= set(p.mle.tlv.type) or\ - { - ADDRESS16_TLV, - LEADER_DATA_TLV, - NETWORK_DATA_TLV, - SOURCE_ADDRESS_TLV - } <= set(p.mle.tlv.type) and\ - p.mle.tlv.source_addr != REED_RLOC16 and\ - p.mle.tlv.addr16 != MED_RLOC16 - ).\ - must_next() + with pkts.save_index(): + pkts.filter_wpan_src64(REED).\ + filter_wpan_dst64(MED).\ + filter_mle_cmd(MLE_CHILD_ID_RESPONSE).\ + filter(lambda p: { + ADDRESS16_TLV, + LEADER_DATA_TLV, + NETWORK_DATA_TLV, + SOURCE_ADDRESS_TLV, + ROUTE64_TLV + } <= set(p.mle.tlv.type) or\ + { + ADDRESS16_TLV, + LEADER_DATA_TLV, + NETWORK_DATA_TLV, + SOURCE_ADDRESS_TLV + } <= set(p.mle.tlv.type) and\ + p.mle.tlv.source_addr != REED_RLOC16 and\ + p.mle.tlv.addr16 != MED_RLOC16 + ).\ + must_next() # Step 12: The Leader MUST respond with an ICMPv6 Echo Reply diff --git a/tests/scripts/thread-cert/v1_2_test_parent_selection.py b/tests/scripts/thread-cert/v1_2_test_parent_selection.py index 432f984ba..e94571886 100755 --- a/tests/scripts/thread-cert/v1_2_test_parent_selection.py +++ b/tests/scripts/thread-cert/v1_2_test_parent_selection.py @@ -158,7 +158,7 @@ class TestParentSelection(thread_cert.TestCase): self.nodes[REED_1_2].set_link_quality(self.nodes[ROUTER_1_2].get_addr64(), 2) self.nodes[ROUTER_1_2].set_router_selection_jitter(1) self.nodes[ROUTER_1_2].start() - self.simulator.go(5) + self.simulator.go(config.ROUTER_STARTUP_DELAY) self.assertEqual(self.nodes[ROUTER_1_2].get_state(), 'router') # Check Parent Response @@ -220,7 +220,7 @@ class TestParentSelection(thread_cert.TestCase): # MED_1_1 would attach to LEADER_1_2 self.nodes[REED_1_1].set_state('router') - self.simulator.go(5) + self.simulator.go(config.ROUTER_STARTUP_DELAY) self.assertEqual(self.nodes[REED_1_1].get_state(), 'router') # Flush relative message queues diff --git a/tools/otci/tests/test_otci.py b/tools/otci/tests/test_otci.py index c1eacefb5..286e3363c 100644 --- a/tools/otci/tests/test_otci.py +++ b/tools/otci/tests/test_otci.py @@ -533,7 +533,7 @@ class TestOTCI(unittest.TestCase): node2.joiner_start("TEST123") node2.wait(10, expect_line="Join success") node2.thread_start() - node2.wait(5) + node2.wait(10) assert node2.get_state() == "router" def _test_otci_multi_nodes(self, leader, commissioner, child1, child2): @@ -587,7 +587,7 @@ class TestOTCI(unittest.TestCase): commissioner.set_network_key(TEST_NETWORKKEY) commissioner.thread_start() - commissioner.wait(5) + commissioner.wait(10) self.assertEqual('router', commissioner.get_state())