From d2d9aff01274663d673c3cdceddeac16c1380a5d Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 6 Mar 2023 21:15:32 -0800 Subject: [PATCH] [mle] send MLE Avd on router promo & accept former child as router (#8307) This commit updates `MleRouter to send an immediate MLE Advertisement on promotion to `router` role and assignment of new Router ID. This helps inform our former parent of our newly allocated Router ID and cause it to reset it own advertisement trickle timer. This can help speed up the dissemination of the new Router ID to other routers. It can also help with quicker link establishment with our former parent and other routers. It also removes tx of multicast Link Request (including its delay mechanism) upon promotion to `router` role. It also updates `Mle` so if we receive an MLE advertisement from a former child (which is recently get promoted to router) to accept it immediately and copy the info from `Child` entry to `Router`. Finally this commit updates the following test-cases which are impacted by the changes above (e.g., skipping the checks for multicast Link Request): - `v1_2_router_5_1_1` - `Cert_5_1_01_RouterAttach` - `Cert_5_2_04_REEDUpgrade` - `Cert_5_5_02_LeaderReboot` - `Cert_5_5_05_SplitMergeREED` --- src/core/thread/mle_router.cpp | 54 ++++++++----------- src/core/thread/mle_router.hpp | 2 - .../thread-cert/Cert_5_1_01_RouterAttach.py | 49 +---------------- .../thread-cert/Cert_5_2_04_REEDUpgrade.py | 24 +-------- .../thread-cert/Cert_5_3_06_RouterIdMask.py | 8 +-- .../thread-cert/Cert_5_5_02_LeaderReboot.py | 9 +--- .../thread-cert/Cert_5_5_05_SplitMergeREED.py | 5 +- .../Cert_7_1_07_BorderRouterAsLeader.py | 2 +- .../test_router_multicast_link_request.py | 32 +++-------- .../scripts/thread-cert/v1_2_router_5_1_1.py | 28 +--------- 10 files changed, 41 insertions(+), 172 deletions(-) diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 4a43c90bb..fe5c0cefb 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -86,7 +86,6 @@ MleRouter::MleRouter(Instance &aInstance) , mRouterSelectionJitter(kRouterSelectionJitter) , mRouterSelectionJitterTimeout(0) , mChildRouterLinks(kChildRouterLinks) - , mLinkRequestDelay(0) , mParentPriority(kParentPriorityUnspecified) #if OPENTHREAD_CONFIG_BACKBONE_ROUTER_ENABLE , mBackboneRouterRegistrationDelay(0) @@ -196,7 +195,6 @@ Error MleRouter::BecomeRouter(ThreadStatusTlv::Status aStatus) Get().SetRxOnWhenIdle(true); mRouterSelectionJitterTimeout = 0; - mLinkRequestDelay = 0; switch (mRole) { @@ -290,7 +288,6 @@ void MleRouter::HandleDetachStart(void) void MleRouter::HandleChildStart(AttachMode aMode) { - mLinkRequestDelay = 0; mAddressSolicitRejected = false; mRouterSelectionJitterTimeout = 1 + Random::NonCrypto::GetUint8InRange(0, mRouterSelectionJitter); @@ -478,11 +475,6 @@ 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()); @@ -522,7 +514,7 @@ Error MleRouter::SendLinkRequest(Neighbor *aNeighbor) TxMessage *message = nullptr; Ip6::Address destination; - VerifyOrExit(mLinkRequestDelay == 0 && mChallengeTimeout == 0); + VerifyOrExit(mChallengeTimeout == 0); destination.Clear(); @@ -1296,12 +1288,6 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo, uint16_t aSourceAddress, c if (IsRouter()) { - if (mLinkRequestDelay > 0 && routeTlv.IsRouterIdSet(mRouterId)) - { - mLinkRequestDelay = 0; - IgnoreError(SendLinkRequest(nullptr)); - } - if (ShouldDowngrade(routerId, routeTlv)) { mRouterSelectionJitterTimeout = 1 + Random::NonCrypto::GetUint8InRange(0, mRouterSelectionJitter); @@ -1311,6 +1297,19 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo, uint16_t aSourceAddress, c router = mRouterTable.FindRouterById(routerId); VerifyOrExit(router != nullptr); + if (!router->IsStateValid() && aRxInfo.IsNeighborStateValid() && Get().Contains(*aRxInfo.mNeighbor)) + { + // The Adv is from a former child that is now acting as a router, + // we copy the info from child entry and update the RLOC16. + + *static_cast(router) = *aRxInfo.mNeighbor; + router->SetRloc16(Rloc16FromRouterId(routerId)); + router->SetDeviceMode(DeviceMode(DeviceMode::kModeFullThreadDevice | DeviceMode::kModeRxOnWhenIdle | + DeviceMode::kModeFullNetworkData)); + + mNeighborTable.Signal(NeighborTable::kRouterAdded, *router); + } + // Send unicast link request if no link to router and no unicast/multicast link request in progress if (!router->IsStateValid() && !router->IsStateLinkRequest() && (mChallengeTimeout == 0) && (linkMargin >= kLinkRequestMinMargin)) @@ -1484,11 +1483,6 @@ void MleRouter::HandleTimeTick(void) VerifyOrExit(IsFullThreadDevice(), Get().UnregisterReceiver(TimeTicker::kMleRouter)); - if (mLinkRequestDelay > 0 && --mLinkRequestDelay == 0) - { - IgnoreError(SendLinkRequest(nullptr)); - } - if (mChallengeTimeout > 0) { mChallengeTimeout--; @@ -3427,7 +3421,6 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message *aMessage, ThreadRouterMaskTlv routerMaskTlv; uint8_t routerId; Router *router; - bool isRouterIdAllocated; mAddressSolicitPending = false; @@ -3459,8 +3452,6 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message *aMessage, SuccessOrExit(Tlv::Find(*aMessage, rloc16)); routerId = RouterIdFromRloc16(rloc16); - isRouterIdAllocated = mRouterTable.IsAllocated(routerId); - SuccessOrExit(Tlv::FindTlv(*aMessage, routerMaskTlv)); VerifyOrExit(routerMaskTlv.IsValid()); @@ -3503,16 +3494,13 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message *aMessage, leader->SetNextHopAndCost(RouterIdFromRloc16(mParent.GetRloc16()), mParent.GetLeaderCost()); } - if (isRouterIdAllocated) - { - // send Link Request immediately if Router ID was previously allocated - IgnoreError(SendLinkRequest(nullptr)); - } - else - { - // wait to send Link Request until new Router ID has been disseminated from the Leader - mLinkRequestDelay = kMulticastLinkRequestDelay; - } + // We send an Advertisement to inform our former parent of our + // newly allocated Router ID. This will cause the parent to + // reset its advertisement trickle timer which can help speed + // up the dissemination of the new Router ID to other routers. + // This can also help with quicker link establishment with our + // former parent and other routers. + SendAdvertisement(); for (Child &child : Get().Iterate(Child::kInStateChildIdRequest)) { diff --git a/src/core/thread/mle_router.hpp b/src/core/thread/mle_router.hpp index f97885e46..7ac7d2677 100644 --- a/src/core/thread/mle_router.hpp +++ b/src/core/thread/mle_router.hpp @@ -668,8 +668,6 @@ private: uint8_t mChildRouterLinks; - 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/tests/scripts/thread-cert/Cert_5_1_01_RouterAttach.py b/tests/scripts/thread-cert/Cert_5_1_01_RouterAttach.py index 1e4c3269e..0c8739948 100755 --- a/tests/scripts/thread-cert/Cert_5_1_01_RouterAttach.py +++ b/tests/scripts/thread-cert/Cert_5_1_01_RouterAttach.py @@ -271,53 +271,8 @@ class Cert_5_1_01_RouterAttach(thread_cert.TestCase): ).\ must_next() - # Step 8: Router Sends a Link Request Message. - # The Link Request Message MUST be multicast and contain - # the following TLVs: - # - Challenge TLV - # - Leader Data TLV - # - Source Address TLV - # - Version TLV - # - TLV Request TLV: Link Margin - - pkts.filter_wpan_src64(ROUTER).\ - 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 9: Leader sends a Unicast Link Accept and Request Message. - # The Message MUST be unicast to Router - # The Message MUST contain the following TLVs: - # - Leader Data TLV - # - Link-layer Frame Counter TLV - # - Link Margin TLV - # - Response TLV - # - Source Address TLV - # - Version TLV - # - Challenge TLV (optional) - # - MLE Frame Counter TLV (optional) - - pkts.filter_wpan_src64(LEADER).\ - filter_wpan_dst64(ROUTER).\ - filter_mle_cmd(MLE_LINK_ACCEPT_AND_REQUEST).\ - filter(lambda p: { - LEADER_DATA_TLV, - LINK_LAYER_FRAME_COUNTER_TLV, - LINK_MARGIN_TLV, - RESPONSE_TLV, - SOURCE_ADDRESS_TLV, - VERSION_TLV - } <= set(p.mle.tlv.type)).\ - must_next() + # Steps 8 and 9 are skipped due to change the Link establishment + # process (no multicast MLE Link Request by new router). # Step 10: Router is sending properly formatted MLE Advertisements. # MLE Advertisements MUST be sent with an IP Hop Limit of 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 241c9bb36..5a0506837 100755 --- a/tests/scripts/thread-cert/Cert_5_2_04_REEDUpgrade.py +++ b/tests/scripts/thread-cert/Cert_5_2_04_REEDUpgrade.py @@ -344,28 +344,8 @@ class Cert_5_2_4_REEDUpgrade(thread_cert.TestCase): must_next() # Step 10: REED Sends a Link Request Message. - # The Link Request Message MUST be multicast and contain - # the following TLVs: - # - Challenge TLV - # - Leader Data TLV - # - Source Address TLV - # - TLV Request TLV: Link Margin - # - Version TLV - - 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() + # This step is skipped due to change that new router no + # longer send multicast Link Request. # Step 11: The REED MLE Child ID Response MUST be properly # formatted with MED_1’s new 16-bit address. diff --git a/tests/scripts/thread-cert/Cert_5_3_06_RouterIdMask.py b/tests/scripts/thread-cert/Cert_5_3_06_RouterIdMask.py index 2f227f9f7..97455af05 100755 --- a/tests/scripts/thread-cert/Cert_5_3_06_RouterIdMask.py +++ b/tests/scripts/thread-cert/Cert_5_3_06_RouterIdMask.py @@ -190,12 +190,8 @@ class Cert_5_3_6_RouterIdMask(thread_cert.TestCase): filter_mle_cmd(MLE_ADVERTISEMENT).\ filter(lambda p: p.sniff_timestamp - _pkt.sniff_timestamp <= 4).\ must_next() - # check router cost before and after the re-attach - pkts.filter_wpan_src64(LEADER).\ - filter_LLANMA().\ - filter_mle_cmd(MLE_ADVERTISEMENT).\ - filter(lambda p: {1,0,1} == set(p.mle.tlv.route64.cost)).\ - must_next() + + # check router cost after the re-attach pkts.filter_wpan_src64(LEADER).\ filter_LLANMA().\ filter_mle_cmd(MLE_ADVERTISEMENT).\ diff --git a/tests/scripts/thread-cert/Cert_5_5_02_LeaderReboot.py b/tests/scripts/thread-cert/Cert_5_5_02_LeaderReboot.py index bdbf3c5b8..80aba8774 100755 --- a/tests/scripts/thread-cert/Cert_5_5_02_LeaderReboot.py +++ b/tests/scripts/thread-cert/Cert_5_5_02_LeaderReboot.py @@ -176,14 +176,9 @@ class Cert_5_5_2_LeaderReboot(thread_cert.TestCase): thread_address.tlv.status == 0) #Step 15: Leader Send a Multicast Link Request - _lpkts.filter_mle_cmd(MLE_LINK_REQUEST).must_next().must_verify( - lambda p: {VERSION_TLV, TLV_REQUEST_TLV, SOURCE_ADDRESS_TLV, LEADER_DATA_TLV, CHALLENGE_TLV} < set( - p.mle.tlv.type)) - #Step 16: Router_1 send a Unicast Link Accept - _rpkts.filter_mle_cmd(MLE_LINK_ACCEPT_AND_REQUEST).must_next().must_verify(lambda p: { - VERSION_TLV, SOURCE_ADDRESS_TLV, RESPONSE_TLV, MLE_FRAME_COUNTER_TLV, LINK_MARGIN_TLV, LEADER_DATA_TLV - } < set(p.mle.tlv.type)) + # Steps 15 and 16 are skipped since new router no longer + # send multicast Link Request. #Step 17: Router_1 MUST respond with an ICMPv6 Echo Reply _rpkts.filter_ping_request().filter_wpan_dst64(MED).must_next() diff --git a/tests/scripts/thread-cert/Cert_5_5_05_SplitMergeREED.py b/tests/scripts/thread-cert/Cert_5_5_05_SplitMergeREED.py index abfab5149..0cd99f5cd 100755 --- a/tests/scripts/thread-cert/Cert_5_5_05_SplitMergeREED.py +++ b/tests/scripts/thread-cert/Cert_5_5_05_SplitMergeREED.py @@ -197,9 +197,8 @@ class Cert_5_5_5_SplitMergeREED(thread_cert.TestCase): lambda p: {NL_MAC_EXTENDED_ADDRESS_TLV, NL_STATUS_TLV} <= set(p.coap.tlv.type)) # Step 7: DUT send a Multicast Link Request - pkts.filter_wpan_src64(REED).filter_mle_cmd(MLE_LINK_REQUEST).must_next().must_verify(lambda p: { - VERSION_TLV, TLV_REQUEST_TLV, SOURCE_ADDRESS_TLV, LEADER_DATA_TLV, CHALLENGE_TLV, LINK_MARGIN_TLV - } <= set(p.mle.tlv.type)) + # Step 7 is skipped since new router no longer sends + # multicast Link Request. # Step 8: DUT send Child ID Response to Router_1 reed_pkts.filter_mle_cmd(MLE_CHILD_ID_RESPONSE).must_next().must_verify(lambda p: p.wpan.dst64 == ROUTER_1) diff --git a/tests/scripts/thread-cert/Cert_7_1_07_BorderRouterAsLeader.py b/tests/scripts/thread-cert/Cert_7_1_07_BorderRouterAsLeader.py index 617fb58b7..49e000bc7 100755 --- a/tests/scripts/thread-cert/Cert_7_1_07_BorderRouterAsLeader.py +++ b/tests/scripts/thread-cert/Cert_7_1_07_BorderRouterAsLeader.py @@ -158,7 +158,7 @@ class Cert_7_1_7_BorderRouterAsLeader(thread_cert.TestCase): # Wait for Router_2 reattachment and network data propagation # ADVERTISEMENT_I_MAX + DEFAULT_CHILD_TIMEOUT + ATTACH_DELAY + Extra - self.simulator.go(60) + self.simulator.go(120) self.assertEqual(self.nodes[ROUTER_2].get_state(), 'router') self.collect_ipaddrs() self.collect_rloc16s() diff --git a/tests/scripts/thread-cert/test_router_multicast_link_request.py b/tests/scripts/thread-cert/test_router_multicast_link_request.py index 38e768131..fb366053e 100755 --- a/tests/scripts/thread-cert/test_router_multicast_link_request.py +++ b/tests/scripts/thread-cert/test_router_multicast_link_request.py @@ -31,7 +31,7 @@ import unittest import config import thread_cert -from pktverify.consts import MLE_LINK_REQUEST, MLE_LINK_ACCEPT +from pktverify.consts import MLE_LINK_REQUEST from pktverify.packet_verifier import PacketVerifier LEADER = 1 @@ -110,30 +110,12 @@ class TestRouterMulticastLinkRequest(thread_cert.TestCase): self.assertEqual(self.nodes[REED].get_state(), 'router') self.simulator.go(LINK_ESTABLISH_DELAY_THRESHOLD + 3) - def verify(self, pv: PacketVerifier): - pkts = pv.pkts - print(pv.vars) - pv.summary.show() - - REED = pv.vars['REED'] - as_pkt = pkts.filter_wpan_src64(REED).filter_coap_request('/a/as', confirmable=True).must_next() - parent_rloc16 = as_pkt.wpan.dst16 - as_ack_pkt = pkts.filter_wpan_src16(parent_rloc16).filter_coap_ack('/a/as').must_next() - become_router_timestamp = as_ack_pkt.sniff_timestamp - - # REED has just received `/a/as` and become a Router - # REED should send Multicast Link Request after becoming Router - link_request_pkt = pkts.filter_wpan_src64(REED).filter_mle_cmd(MLE_LINK_REQUEST).must_next() - link_request_pkt.must_verify('ipv6.dst == "ff02::2"') - - # REED should send Link Accept to the three Routers - for router in ('ROUTER1', 'ROUTER2', 'ROUTER3'): - with pkts.save_index(): - pkt = pkts.filter_wpan_src64(REED).filter_wpan_dst64( - pv.vars[router]).filter_mle_cmd(MLE_LINK_ACCEPT).must_next() - link_establish_delay = pkt.sniff_timestamp - become_router_timestamp - logging.info("Link to %s established in %.3f seconds", router, link_establish_delay) - self.assertLess(link_establish_delay, LINK_ESTABLISH_DELAY_THRESHOLD) + # Verify that REED has established link with all routers + reed_table = self.nodes[REED].router_table() + reed_id = self.nodes[REED].get_router_id() + for router in [self.nodes[ROUTER1], self.nodes[ROUTER2], self.nodes[ROUTER3]]: + self.assertEqual(reed_table[router.get_router_id()]['link'], 1) + self.assertEqual(router.router_table()[reed_id]['link'], 1) if __name__ == '__main__': diff --git a/tests/scripts/thread-cert/v1_2_router_5_1_1.py b/tests/scripts/thread-cert/v1_2_router_5_1_1.py index 20173f5ba..6f8b01c02 100755 --- a/tests/scripts/thread-cert/v1_2_router_5_1_1.py +++ b/tests/scripts/thread-cert/v1_2_router_5_1_1.py @@ -136,31 +136,7 @@ class Router_5_1_01(thread_cert.TestCase): status_tlv = msg.get_coap_message_tlv(network_layer.Status) self.assertEqual(network_layer.StatusValues.SUCCESS, status_tlv.status) - # 8 - Router_1 sends a multicast Link Request Message - msg = router_messages.next_mle_message(mle.CommandType.LINK_REQUEST) - msg.assertMleMessageContainsTlv(mle.SourceAddress) - msg.assertMleMessageContainsTlv(mle.LeaderData) - msg.assertMleMessageContainsTlv(mle.Challenge) - msg.assertMleMessageContainsTlv(mle.Version) - msg.assertMleMessageContainsTlv(mle.TlvRequest) - assert msg.get_mle_message_tlv(mle.Version).version >= 3 - - tlv_request = msg.get_mle_message_tlv(mle.TlvRequest) - self.assertIn(mle.TlvType.LINK_MARGIN, tlv_request.tlvs) - - # 9 - Leader sends a Unicast Link Accept - msg = leader_messages.next_mle_message(mle.CommandType.LINK_ACCEPT_AND_REQUEST) - msg.assertMleMessageContainsTlv(mle.SourceAddress) - msg.assertMleMessageContainsTlv(mle.LeaderData) - msg.assertMleMessageContainsTlv(mle.Response) - msg.assertMleMessageContainsTlv(mle.LinkLayerFrameCounter) - msg.assertMleMessageContainsTlv(mle.Version) - msg.assertMleMessageContainsTlv(mle.LinkMargin) - msg.assertMleMessageContainsOptionalTlv(mle.MleFrameCounter) - msg.assertMleMessageContainsOptionalTlv(mle.Challenge) - assert msg.get_mle_message_tlv(mle.Version).version >= 3 - - # 10 - Router_1 Transmit MLE advertisements + # 8 - Router_1 Transmit MLE advertisements msg = router_messages.next_mle_message(mle.CommandType.ADVERTISEMENT) msg.assertSentWithHopLimit(255) msg.assertSentToDestinationAddress('ff02::1') @@ -168,7 +144,7 @@ class Router_5_1_01(thread_cert.TestCase): msg.assertMleMessageContainsTlv(mle.LeaderData) msg.assertMleMessageContainsTlv(mle.Route64) - # 11 - Verify connectivity by sending an ICMPv6 Echo Request to the DUT link local address + # 9 - Verify connectivity by sending an ICMPv6 Echo Request to the DUT link local address self.assertTrue(self.nodes[LEADER].ping(self.nodes[ROUTER_1].get_linklocal())) self.assertTrue(self.nodes[ROUTER_1].ping(self.nodes[LEADER].get_linklocal()))