[meshcop-tlvs] simplify and enhance ChannelMaskTlv (openthread#9675)

This commit simplifies `ChannelMaskTlv` generation and parsing.
Notably, it ensures that when we read this TLV from a `Message` the
TLV value format is validated. The changes include:
- `Entry` class representing a channel page entry is now defined as
  a `private` nested class within `ChannelMaskTlv`.
- A shared `EntriesData::Parse()` method is added that validates and
  parses the entries in the TLV, whether the TLV value resides in a
  buffer (e.g., within `Dataset`) or in a `Message`.
- `ChannelMaskTlv::AppendTo()` method is added to construct entries
  from a given combined channel mask (for all pages) and append the
  `ChannelMaskTlv` to a given `Message`.
- `Radio::kSupportedChannelPages` is changed to an array
  (containing all supported pages) instead of mask.
- Helper functions added in `Radio`, `SupportsChannelPage()` to
  verify if a channel page is supported by the radio, and
  `ChannelMaskForPage()` to obtain the supported channel mask for
  a given page.

src/core/meshcop/announce_begin_client.cpp

@@ -60,10 +60,9 @@ Error AnnounceBeginClient::SendRequest(uint32_t            aChannelMask,
                                        uint16_t            aPeriod,
                                        const Ip6::Address &aAddress)
 {
-    Error                   error = kErrorNone;
-    MeshCoP::ChannelMaskTlv channelMask;
-    Tmf::MessageInfo        messageInfo(GetInstance());
-    Coap::Message          *message = nullptr;
+    Error            error = kErrorNone;
+    Tmf::MessageInfo messageInfo(GetInstance());
+    Coap::Message   *message = nullptr;
 
     VerifyOrExit(Get<MeshCoP::Commissioner>().IsActive(), error = kErrorInvalidState);
     VerifyOrExit((message = Get<Tmf::Agent>().NewPriorityMessage()) != nullptr, error = kErrorNoBufs);
@@ -74,9 +73,7 @@ Error AnnounceBeginClient::SendRequest(uint32_t            aChannelMask,
     SuccessOrExit(
         error = Tlv::Append<MeshCoP::CommissionerSessionIdTlv>(*message, Get<MeshCoP::Commissioner>().GetSessionId()));
 
-    channelMask.Init();
-    channelMask.SetChannelMask(aChannelMask);
-    SuccessOrExit(error = channelMask.AppendTo(*message));
+    SuccessOrExit(error = MeshCoP::ChannelMaskTlv::AppendTo(*message, aChannelMask));
 
     SuccessOrExit(error = Tlv::Append<MeshCoP::CountTlv>(*message, aCount));
     SuccessOrExit(error = Tlv::Append<MeshCoP::PeriodTlv>(*message, aPeriod));

src/core/meshcop/dataset.cpp

@@ -199,9 +199,9 @@ void Dataset::ConvertTo(Info &aDatasetInfo) const
 
         case Tlv::kChannelMask:
         {
-            uint32_t mask = As<ChannelMaskTlv>(cur)->GetChannelMask();
+            uint32_t mask;
 
-            if (mask != 0)
+            if (As<ChannelMaskTlv>(cur)->ReadChannelMask(mask) == kErrorNone)
             {
                 aDatasetInfo.SetChannelMask(mask);
             }
@@ -312,10 +312,10 @@ Error Dataset::SetFrom(const Info &aDatasetInfo)
 
     if (aDatasetInfo.IsChannelMaskPresent())
     {
-        ChannelMaskTlv tlv;
-        tlv.Init();
-        tlv.SetChannelMask(aDatasetInfo.GetChannelMask());
-        IgnoreError(WriteTlv(tlv));
+        ChannelMaskTlv::Value value;
+
+        ChannelMaskTlv::PrepareValue(value, aDatasetInfo.GetChannelMask());
+        IgnoreError(WriteTlv(Tlv::kChannelMask, value.mData, value.mLength));
     }
 
     if (aDatasetInfo.IsExtendedPanIdPresent())

src/core/meshcop/dataset_manager.cpp

@@ -228,7 +228,7 @@ Error DatasetManager::GetChannelMask(Mac::ChannelMask &aChannelMask) const
 
     channelMaskTlv = As<ChannelMaskTlv>(dataset.FindTlv(Tlv::kChannelMask));
     VerifyOrExit(channelMaskTlv != nullptr, error = kErrorNotFound);
-    VerifyOrExit((mask = channelMaskTlv->GetChannelMask()) != 0);
+    SuccessOrExit(channelMaskTlv->ReadChannelMask(mask));
 
     aChannelMask.SetMask(mask & Get<Mac::Mac>().GetSupportedChannelMask().GetMask());
 

src/core/meshcop/dataset_manager_ftd.cpp

@@ -319,10 +319,10 @@ Error ActiveDatasetManager::GenerateLocal(void)
 
     if (!dataset.Contains<ChannelMaskTlv>())
     {
-        ChannelMaskTlv tlv;
-        tlv.Init();
-        tlv.SetChannelMask(Get<Mac::Mac>().GetSupportedChannelMask().GetMask());
-        IgnoreError(dataset.WriteTlv(tlv));
+        ChannelMaskTlv::Value value;
+
+        ChannelMaskTlv::PrepareValue(value, Get<Mac::Mac>().GetSupportedChannelMask().GetMask());
+        IgnoreError(dataset.WriteTlv(Tlv::kChannelMask, value.mData, value.mLength));
     }
 
     if (!dataset.Contains<ExtendedPanIdTlv>())

src/core/meshcop/energy_scan_client.cpp

@@ -66,10 +66,9 @@ Error EnergyScanClient::SendQuery(uint32_t                           aChannelMas
                                   otCommissionerEnergyReportCallback aCallback,
                                   void                              *aContext)
 {
-    Error                   error = kErrorNone;
-    MeshCoP::ChannelMaskTlv channelMask;
-    Tmf::MessageInfo        messageInfo(GetInstance());
-    Coap::Message          *message = nullptr;
+    Error            error = kErrorNone;
+    Tmf::MessageInfo messageInfo(GetInstance());
+    Coap::Message   *message = nullptr;
 
     VerifyOrExit(Get<MeshCoP::Commissioner>().IsActive(), error = kErrorInvalidState);
     VerifyOrExit((message = Get<Tmf::Agent>().NewPriorityMessage()) != nullptr, error = kErrorNoBufs);
@@ -80,9 +79,7 @@ Error EnergyScanClient::SendQuery(uint32_t                           aChannelMas
     SuccessOrExit(
         error = Tlv::Append<MeshCoP::CommissionerSessionIdTlv>(*message, Get<MeshCoP::Commissioner>().GetSessionId()));
 
-    channelMask.Init();
-    channelMask.SetChannelMask(aChannelMask);
-    SuccessOrExit(error = channelMask.AppendTo(*message));
+    SuccessOrExit(error = MeshCoP::ChannelMaskTlv::AppendTo(*message, aChannelMask));
 
     SuccessOrExit(error = Tlv::Append<MeshCoP::CountTlv>(*message, aCount));
     SuccessOrExit(error = Tlv::Append<MeshCoP::PeriodTlv>(*message, aPeriod));
@@ -110,7 +107,7 @@ void EnergyScanClient::HandleTmf<kUriEnergyReport>(Coap::Message &aMessage, cons
 
     LogInfo("Received %s", UriToString<kUriEnergyReport>());
 
-    VerifyOrExit((mask = MeshCoP::ChannelMaskTlv::GetChannelMask(aMessage)) != 0);
+    SuccessOrExit(MeshCoP::ChannelMaskTlv::FindIn(aMessage, mask));
 
     SuccessOrExit(MeshCoP::Tlv::FindTlv(aMessage, MeshCoP::Tlv::kEnergyList, sizeof(energyListTlv), energyListTlv));
 

src/core/meshcop/meshcop_tlvs.cpp

@@ -158,191 +158,140 @@ const char *StateTlv::StateToString(State aState)
     return aState == kReject ? kStateStrings[2] : kStateStrings[aState];
 }
 
-bool ChannelMaskBaseTlv::IsValid(void) const
+bool ChannelMaskTlv::IsValid(void) const
 {
-    const ChannelMaskEntryBase *cur = GetFirstEntry();
-    const ChannelMaskEntryBase *end = reinterpret_cast<const ChannelMaskEntryBase *>(GetNext());
-    bool                        ret = false;
+    uint32_t channelMask;
 
-    VerifyOrExit(cur != nullptr);
-
-    while (cur < end)
-    {
-        uint8_t channelPage;
-
-        VerifyOrExit((cur + 1) <= end && cur->GetNext() <= end);
-
-        channelPage = cur->GetChannelPage();
-
-#if OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_SUPPORT
-        if (channelPage == OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_CHANNEL_PAGE)
-#else
-        if ((channelPage == OT_RADIO_CHANNEL_PAGE_0) || (channelPage == OT_RADIO_CHANNEL_PAGE_2))
-#endif
-        {
-            VerifyOrExit(static_cast<const ChannelMaskEntry *>(cur)->IsValid());
-        }
+    return (ReadChannelMask(channelMask) == kErrorNone);
+}
 
-        cur = cur->GetNext();
-    }
+Error ChannelMaskTlv::ReadChannelMask(uint32_t &aChannelMask) const
+{
+    EntriesData entriesData;
 
-    ret = true;
+    entriesData.Clear();
+    entriesData.mData   = &mEntriesStart;
+    entriesData.mLength = GetLength();
 
-exit:
-    return ret;
+    return entriesData.Parse(aChannelMask);
 }
 
-const ChannelMaskEntryBase *ChannelMaskBaseTlv::GetFirstEntry(void) const
+Error ChannelMaskTlv::FindIn(const Message &aMessage, uint32_t &aChannelMask)
 {
-    const ChannelMaskEntryBase *entry = nullptr;
+    Error       error;
+    EntriesData entriesData;
 
-    VerifyOrExit(GetLength() >= sizeof(ChannelMaskEntryBase));
+    entriesData.Clear();
+    entriesData.mMessage = &aMessage;
 
-    entry = reinterpret_cast<const ChannelMaskEntryBase *>(GetValue());
-    VerifyOrExit(GetLength() >= entry->GetEntrySize(), entry = nullptr);
+    SuccessOrExit(error = FindTlvValueOffset(aMessage, Tlv::kChannelMask, entriesData.mOffset, entriesData.mLength));
+    error = entriesData.Parse(aChannelMask);
 
 exit:
-    return entry;
+    return error;
 }
 
-ChannelMaskEntryBase *ChannelMaskBaseTlv::GetFirstEntry(void) { return AsNonConst(AsConst(this)->GetFirstEntry()); }
-
-void ChannelMaskTlv::SetChannelMask(uint32_t aChannelMask)
+Error ChannelMaskTlv::EntriesData::Parse(uint32_t &aChannelMask)
 {
-    uint8_t           length = 0;
-    ChannelMaskEntry *entry;
-
-    entry = static_cast<ChannelMaskEntry *>(GetFirstEntry());
-
-#if OPENTHREAD_CONFIG_RADIO_915MHZ_OQPSK_SUPPORT
-    if (aChannelMask & OT_RADIO_915MHZ_OQPSK_CHANNEL_MASK)
-    {
-        OT_ASSERT(entry != nullptr);
-        entry->Init();
-        entry->SetChannelPage(OT_RADIO_CHANNEL_PAGE_2);
-        entry->SetMask(aChannelMask & OT_RADIO_915MHZ_OQPSK_CHANNEL_MASK);
+    // Validates and parses the Channel Mask TLV entries for each
+    // channel page and if successful updates `aChannelMask` to
+    // return the combined mask for all channel pages supported by
+    // radio. The entries can be either contained in `mMessage` from
+    // `mOffset` (when `mMessage` is non-null) or be in a buffer
+    // `mData`. `mLength` gives the number of bytes for all entries.
 
-        length += sizeof(ChannelMaskEntry);
+    Error        error = kErrorParse;
+    Entry        readEntry;
+    const Entry *entry;
+    uint16_t     size;
 
-        entry = static_cast<ChannelMaskEntry *>(entry->GetNext());
-    }
-#endif
+    aChannelMask = 0;
 
-#if OPENTHREAD_CONFIG_RADIO_2P4GHZ_OQPSK_SUPPORT
-    if (aChannelMask & OT_RADIO_2P4GHZ_OQPSK_CHANNEL_MASK)
-    {
-        OT_ASSERT(entry != nullptr);
-        entry->Init();
-        entry->SetChannelPage(OT_RADIO_CHANNEL_PAGE_0);
-        entry->SetMask(aChannelMask & OT_RADIO_2P4GHZ_OQPSK_CHANNEL_MASK);
+    VerifyOrExit(mLength > 0); // At least one entry.
 
-        length += sizeof(ChannelMaskEntry);
-    }
-#endif
-
-#if OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_SUPPORT
-    if (aChannelMask & OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_CHANNEL_MASK)
+    while (mLength > 0)
     {
-        OT_ASSERT(entry != nullptr);
-        entry->Init();
-        entry->SetChannelPage(OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_CHANNEL_PAGE);
-        entry->SetMask(aChannelMask & OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_CHANNEL_MASK);
+        VerifyOrExit(mLength > kEntryHeaderSize);
 
-        length += sizeof(ChannelMaskEntry);
-    }
-#endif
+        if (mMessage != nullptr)
+        {
+            // We first read the entry's header only and after
+            // validating the entry and that the entry's channel page
+            // is supported by radio, we read the full `Entry`.
 
-    SetLength(length);
-}
+            mMessage->ReadBytes(mOffset, &readEntry, kEntryHeaderSize);
+            entry = &readEntry;
+        }
+        else
+        {
+            entry = reinterpret_cast<const Entry *>(mData);
+        }
 
-uint32_t ChannelMaskTlv::GetChannelMask(void) const
-{
-    const ChannelMaskEntryBase *cur  = GetFirstEntry();
-    const ChannelMaskEntryBase *end  = reinterpret_cast<const ChannelMaskEntryBase *>(GetNext());
-    uint32_t                    mask = 0;
+        size = kEntryHeaderSize + entry->GetMaskLength();
 
-    VerifyOrExit(cur != nullptr);
+        VerifyOrExit(size <= mLength);
 
-    while (cur < end)
-    {
-        uint8_t channelPage;
+        if (Radio::SupportsChannelPage(entry->GetChannelPage()))
+        {
+            // Currently supported channel pages all use `uint32_t`
+            // channel mask.
 
-        VerifyOrExit((cur + 1) <= end && cur->GetNext() <= end);
+            VerifyOrExit(entry->GetMaskLength() == kMaskLength);
 
-        channelPage = cur->GetChannelPage();
+            if (mMessage != nullptr)
+            {
+                IgnoreError(mMessage->Read(mOffset, readEntry));
+            }
 
-#if OPENTHREAD_CONFIG_RADIO_915MHZ_OQPSK_SUPPORT
-        if (channelPage == OT_RADIO_CHANNEL_PAGE_2)
-        {
-            mask |= static_cast<const ChannelMaskEntry *>(cur)->GetMask() & OT_RADIO_915MHZ_OQPSK_CHANNEL_MASK;
+            aChannelMask |= (entry->GetMask() & Radio::ChannelMaskForPage(entry->GetChannelPage()));
         }
-#endif
 
-#if OPENTHREAD_CONFIG_RADIO_2P4GHZ_OQPSK_SUPPORT
-        if (channelPage == OT_RADIO_CHANNEL_PAGE_0)
+        mLength -= size;
+
+        if (mMessage != nullptr)
         {
-            mask |= static_cast<const ChannelMaskEntry *>(cur)->GetMask() & OT_RADIO_2P4GHZ_OQPSK_CHANNEL_MASK;
+            mOffset += size;
         }
-#endif
-
-#if OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_SUPPORT
-        if (channelPage == OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_CHANNEL_PAGE)
+        else
         {
-            mask |= static_cast<const ChannelMaskEntry *>(cur)->GetMask() &
-                    OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_CHANNEL_MASK;
+            mData += size;
         }
-#endif
-
-        cur = cur->GetNext();
     }
 
+    error = kErrorNone;
+
 exit:
-    return mask;
+    return error;
 }
 
-uint32_t ChannelMaskTlv::GetChannelMask(const Message &aMessage)
+void ChannelMaskTlv::PrepareValue(Value &aValue, uint32_t aChannelMask)
 {
-    uint32_t mask = 0;
-    uint16_t offset;
-    uint16_t end;
+    Entry *entry = reinterpret_cast<Entry *>(aValue.mData);
 
-    SuccessOrExit(FindTlvValueStartEndOffsets(aMessage, kChannelMask, offset, end));
+    aValue.mLength = 0;
 
-    while (offset + sizeof(ChannelMaskEntryBase) <= end)
+    for (uint8_t page : Radio::kSupportedChannelPages)
     {
-        ChannelMaskEntry entry;
+        uint32_t mask = (Radio::ChannelMaskForPage(page) & aChannelMask);
 
-        IgnoreError(aMessage.Read(offset, entry));
-        VerifyOrExit(offset + entry.GetEntrySize() <= end);
-
-        switch (entry.GetChannelPage())
+        if (mask != 0)
         {
-#if OPENTHREAD_CONFIG_RADIO_2P4GHZ_OQPSK_SUPPORT
-        case OT_RADIO_CHANNEL_PAGE_0:
-            IgnoreError(aMessage.Read(offset, entry));
-            mask |= entry.GetMask() & OT_RADIO_2P4GHZ_OQPSK_CHANNEL_MASK;
-            break;
-#endif
-
-#if OPENTHREAD_CONFIG_RADIO_915MHZ_OQPSK_SUPPORT
-        case OT_RADIO_CHANNEL_PAGE_2:
-            IgnoreError(aMessage.Read(offset, entry));
-            mask |= entry.GetMask() & OT_RADIO_915MHZ_OQPSK_CHANNEL_MASK;
-            break;
-#endif
-
-#if OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_SUPPORT
-        case OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_CHANNEL_PAGE:
-            IgnoreError(aMessage.Read(offset, entry));
-            mask |= entry.GetMask() & OPENTHREAD_CONFIG_PLATFORM_RADIO_PROPRIETARY_CHANNEL_MASK;
-            break;
-#endif
+            entry->SetChannelPage(page);
+            entry->SetMaskLength(kMaskLength);
+            entry->SetMask(mask);
+
+            aValue.mLength += sizeof(Entry);
+            entry++;
         }
-        offset += entry.GetEntrySize();
     }
+}
 
-exit:
-    return mask;
+Error ChannelMaskTlv::AppendTo(Message &aMessage, uint32_t aChannelMask)
+{
+    Value value;
+
+    PrepareValue(value, aChannelMask);
+    return Tlv::Append<ChannelMaskTlv>(aMessage, value.mData, value.mLength);
 }
 
 } // namespace MeshCoP