[mle] enhance `ProcessRouteTlv()` to update neighbor pointer (#7667)
This commit updates `MleRouter::ProcessRouteTlv()` to not only read
and process Route TLV from a received MLE message but also to update
the `mNeighbor` pointer (in `RxInfo`) which indicates the neighbor
from which the MLE message was received.
During processing of Route TLV, the entries in the router table may
shuffle. If the `mNeighbor` was pointing to an entry in `RouterTable`
it may be invalid afterwards. The new implementation ensures that
`mNeighbor` is correctly updated to point the same entry.
This change addresses a scenario where in `Mle::HandleAdvertisement()`
a potentially invalid (or set to null) `mNeighbor` may be checked
after Route TLV is processed from `MleRouter::HandleAdvertisement()`
call.
diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp
index b5c18d6..3e5c3c0 100644
--- a/src/core/thread/mle.cpp
+++ b/src/core/thread/mle.cpp
@@ -3153,12 +3153,13 @@
#if OPENTHREAD_FTD
if (IsFullThreadDevice())
{
- RouteTlv route;
-
- if ((Tlv::FindTlv(aRxInfo.mMessage, route) == kErrorNone) && route.IsValid())
+ switch (Get<MleRouter>().ProcessRouteTlv(aRxInfo))
{
- // Overwrite Route Data
- IgnoreError(Get<MleRouter>().ProcessRouteTlv(route));
+ case kErrorNone:
+ case kErrorNotFound:
+ break;
+ default:
+ ExitNow(error = kErrorParse);
}
}
#endif
@@ -3795,11 +3796,13 @@
#if OPENTHREAD_FTD
if (IsFullThreadDevice())
{
- RouteTlv route;
-
- if (Tlv::FindTlv(aRxInfo.mMessage, route) == kErrorNone)
+ switch (Get<MleRouter>().ProcessRouteTlv(aRxInfo))
{
- SuccessOrExit(error = Get<MleRouter>().ProcessRouteTlv(route));
+ case kErrorNone:
+ case kErrorNotFound:
+ break;
+ default:
+ ExitNow(error = kErrorParse);
}
}
#endif
diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp
index 1a90d47..42446ee 100644
--- a/src/core/thread/mle_router.cpp
+++ b/src/core/thread/mle_router.cpp
@@ -837,7 +837,7 @@
uint32_t mleFrameCounter;
uint8_t routerId;
uint16_t address16;
- RouteTlv route;
+ RouteTlv routeTlv;
LeaderData leaderData;
uint8_t linkMargin;
@@ -919,10 +919,8 @@
SetLeaderData(leaderData.GetPartitionId(), leaderData.GetWeighting(), leaderData.GetLeaderRouterId());
// Route
- SuccessOrExit(error = Tlv::FindTlv(aRxInfo.mMessage, Tlv::kRoute, sizeof(route), route));
- VerifyOrExit(route.IsValid(), error = kErrorParse);
mRouterTable.Clear();
- SuccessOrExit(error = ProcessRouteTlv(route));
+ SuccessOrExit(error = ProcessRouteTlv(aRxInfo));
router = mRouterTable.GetRouter(routerId);
VerifyOrExit(router != nullptr);
@@ -964,14 +962,21 @@
}
// Route (optional)
- if (Tlv::FindTlv(aRxInfo.mMessage, route) == kErrorNone)
+ switch (error = ProcessRouteTlv(aRxInfo, routeTlv))
{
- VerifyOrExit(route.IsValid(), error = kErrorParse);
- SuccessOrExit(error = ProcessRouteTlv(route));
- UpdateRoutes(route, routerId);
- // need to update router after ProcessRouteTlv
+ case kErrorNone:
+ UpdateRoutes(routeTlv, routerId);
+ // Need to update router after ProcessRouteTlv
router = mRouterTable.GetRouter(routerId);
OT_ASSERT(router != nullptr);
+ break;
+
+ case kErrorNotFound:
+ error = kErrorNone;
+ break;
+
+ default:
+ ExitNow();
}
// update routing table
@@ -1084,18 +1089,54 @@
return error;
}
-Error MleRouter::ProcessRouteTlv(const RouteTlv &aRoute)
+Error MleRouter::ProcessRouteTlv(RxInfo &aRxInfo)
{
- Error error = kErrorNone;
+ RouteTlv routeTlv;
- mRouterTable.UpdateRouterIdSet(aRoute.GetRouterIdSequence(), aRoute.GetRouterIdMask());
+ return ProcessRouteTlv(aRxInfo, routeTlv);
+}
- if (IsRouter() && !mRouterTable.IsAllocated(mRouterId))
+Error MleRouter::ProcessRouteTlv(RxInfo &aRxInfo, RouteTlv &aRouteTlv)
+{
+ // This method processes Route TLV in a received MLE message
+ // (from `RxInfo`). In case of success, `aRouteTlv` is updated
+ // to return the read/processed route TLV from the message.
+ // If the message contains no Route TLV, `kErrorNotFound` is
+ // returned.
+ //
+ // During processing of Route TLV, the entries in the router table
+ // may shuffle. This method ensures that the `aRxInfo.mNeighbor`
+ // (which indicates the neighbor from which the MLE message was
+ // received) is correctly updated to point to the same neighbor
+ // (in case `mNeighbor` was pointing to a router entry from the
+ // `RouterTable`).
+
+ Error error;
+ uint16_t neighborRloc16 = Mac::kShortAddrInvalid;
+
+ if ((aRxInfo.mNeighbor != nullptr) && Get<RouterTable>().Contains(*aRxInfo.mNeighbor))
+ {
+ neighborRloc16 = aRxInfo.mNeighbor->GetRloc16();
+ }
+
+ SuccessOrExit(error = Tlv::FindTlv(aRxInfo.mMessage, aRouteTlv));
+
+ VerifyOrExit(aRouteTlv.IsValid(), error = kErrorParse);
+
+ Get<RouterTable>().UpdateRouterIdSet(aRouteTlv.GetRouterIdSequence(), aRouteTlv.GetRouterIdMask());
+
+ if (IsRouter() && !Get<RouterTable>().IsAllocated(mRouterId))
{
IgnoreError(BecomeDetached());
error = kErrorNoRoute;
}
+ if (neighborRloc16 != Mac::kShortAddrInvalid)
+ {
+ aRxInfo.mNeighbor = Get<RouterTable>().GetNeighbor(neighborRloc16);
+ }
+
+exit:
return error;
}
@@ -1289,11 +1330,7 @@
if (processRouteTlv)
{
- SuccessOrExit(error = ProcessRouteTlv(route));
- if (Get<RouterTable>().Contains(*aRxInfo.mNeighbor))
- {
- aRxInfo.mNeighbor = nullptr; // aRxInfo.mNeighbor is no longer valid after `ProcessRouteTlv`
- }
+ SuccessOrExit(error = ProcessRouteTlv(aRxInfo));
}
}
diff --git a/src/core/thread/mle_router.hpp b/src/core/thread/mle_router.hpp
index 1a2df96..6645498 100644
--- a/src/core/thread/mle_router.hpp
+++ b/src/core/thread/mle_router.hpp
@@ -603,7 +603,8 @@
void HandleTimeSync(RxInfo &aRxInfo);
#endif
- Error ProcessRouteTlv(const RouteTlv &aRoute);
+ Error ProcessRouteTlv(RxInfo &aRxInfo);
+ Error ProcessRouteTlv(RxInfo &aRxInfo, RouteTlv &aRouteTlv);
void StopAdvertiseTrickleTimer(void);
Error SendAddressSolicit(ThreadStatusTlv::Status aStatus);
void SendAddressRelease(void);
