From fd7284b0e9cb6cfe273b9906ea3d63635ee3fac0 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Wed, 14 Aug 2024 12:58:04 -0700
Subject: [PATCH] [address-resolver] freshness timeout mechanism for cache
 entries (#10575)

This commit introduces a freshness timeout mechanism for address cache
entries which is used to decide when removing stale entries when the
associated RLOC16 is unreachable.

In `AddressResolver`, when resolving an EID from an existing cache
entry, if the target RLOC16 is unreachable, the entry is typically
considered stale and removed to allow a new address query to be sent.
This commit adds a mechanism to skip this removal step if the entry
has been recently updated, i.e., an `AddressNotify` has been received
for it and its `FreshnessTimeout` has not yet expired.

The `FreshnessTimeout` check prevents repeated address query
transmissions when mesh routes are not yet discovered (e.g., after
initial attach) or if there is a temporary link issue.
---
 src/core/thread/address_resolver.cpp | 39 +++++++++++++++++++++++-----
 src/core/thread/address_resolver.hpp | 12 +++++++--
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/src/core/thread/address_resolver.cpp b/src/core/thread/address_resolver.cpp
index 930bfbe05..b3f2b0336 100644
--- a/src/core/thread/address_resolver.cpp
+++ b/src/core/thread/address_resolver.cpp
@@ -493,18 +493,31 @@ Error AddressResolver::Resolve(const Ip6::Address &aEid, uint16_t &aRloc16, bool
 
     if ((entry != nullptr) && ((list == &mCachedList) || (list == &mSnoopedList)))
     {
+        bool isFresh;
+
         list->PopAfter(prev);
 
-        if (Get<RouterTable>().GetNextHop(entry->GetRloc16()) == Mle::kInvalidRloc16)
-        {
-            // If the `entry->GetRloc16()` is unreachable (there is no valid
-            // next hop towards it), we clear the entry so to start a new
-            // address query.
+        // If the `entry->GetRloc16()` is unreachable (there is no
+        // valid next hop towards it), it may be a stale entry. We
+        // clear the entry to allow new address query to be sent for
+        // it, unless the entry has been recently updated, i.e., we
+        // have recently received an `AddressNotify` for it and its
+        // `FreshnessTimeout` has not expired yet.
+        //
+        // The `FreshnessTimeout` check prevents repeated address
+        // query transmissions when mesh routes are not yet
+        // discovered (e.g., after initial attach) or if there is a
+        // temporary link issue.
 
+        isFresh = (list == &mCachedList) && !entry->IsFreshnessTimeoutZero();
+
+        if (!isFresh && (Get<RouterTable>().GetNextHop(entry->GetRloc16()) == Mle::kInvalidRloc16))
+        {
             mCacheEntryPool.Free(*entry);
             entry = nullptr;
         }
-        else
+
+        if (entry != nullptr)
         {
             // Push the entry at the head of cached list.
 
@@ -701,6 +714,8 @@ void AddressResolver::HandleTmf<kUriAddressNotify>(Coap::Message &aMessage, cons
     entry->SetRloc16(rloc16);
     entry->SetMeshLocalIid(meshLocalIid);
     entry->SetLastTransactionTime(lastTransactionTime);
+    entry->ResetFreshnessTimeout();
+    Get<TimeTicker>().RegisterReceiver(TimeTicker::kAddressResolver);
 
     list->PopAfter(prev);
     mCachedList.Push(*entry);
@@ -926,6 +941,15 @@ void AddressResolver::HandleTimeTick(void)
 {
     bool continueRxingTicks = false;
 
+    for (CacheEntry &entry : mCachedList)
+    {
+        if (!entry.IsFreshnessTimeoutZero())
+        {
+            entry.DecrementFreshnessTimeout();
+            continueRxingTicks = true;
+        }
+    }
+
     for (CacheEntry &entry : mSnoopedList)
     {
         if (entry.IsTimeoutZero())
@@ -1128,7 +1152,8 @@ void AddressResolver::LogCacheEntryChange(EntryChange, Reason, const CacheEntry
 void AddressResolver::CacheEntry::Init(Instance &aInstance)
 {
     InstanceLocatorInit::Init(aInstance);
-    mNextIndex = kNoNextIndex;
+    mNextIndex        = kNoNextIndex;
+    mFreshnessTimeout = 0;
 }
 
 AddressResolver::CacheEntry *AddressResolver::CacheEntry::GetNext(void)
diff --git a/src/core/thread/address_resolver.hpp b/src/core/thread/address_resolver.hpp
index 9e9799f19..84edeef5c 100644
--- a/src/core/thread/address_resolver.hpp
+++ b/src/core/thread/address_resolver.hpp
@@ -283,6 +283,10 @@ private:
         uint16_t GetTimeout(void) const { return mInfo.mOther.mTimeout; }
         void     SetTimeout(uint16_t aTimeout) { mInfo.mOther.mTimeout = aTimeout; }
 
+        void DecrementFreshnessTimeout(void) { mFreshnessTimeout--; }
+        bool IsFreshnessTimeoutZero(void) const { return mFreshnessTimeout == 0; }
+        void ResetFreshnessTimeout(void) { mFreshnessTimeout = kFreshnessTimeout; }
+
         uint16_t GetRetryDelay(void) const { return mInfo.mOther.mRetryDelay; }
         void     SetRetryDelay(uint16_t aDelay) { mInfo.mOther.mRetryDelay = aDelay; }
 
@@ -295,12 +299,16 @@ private:
         bool Matches(const Ip6::Address &aEid) const { return GetTarget() == aEid; }
 
     private:
-        static constexpr uint16_t kNoNextIndex          = 0xffff;     // `mNextIndex` value when at end of list.
+        static constexpr uint16_t kNoNextIndex          = 0x3fff;     // `mNextIndex` value when at end of list.
         static constexpr uint32_t kInvalidLastTransTime = 0xffffffff; // Value when `mLastTransactionTime` is invalid.
+        static constexpr uint8_t  kFreshnessTimeout     = 3;
+
+        static_assert(kCacheEntries < kNoNextIndex, "kCacheEntries is too large and does not fit in 14 bit index");
 
         Ip6::Address mTarget;
         uint16_t     mRloc16;
-        uint16_t     mNextIndex;
+        uint16_t     mNextIndex : 14;
+        uint8_t      mFreshnessTimeout : 2;
 
         union
         {