Update include/commissioner/network_diag_data.hpp

Co-authored-by: Kangping <wgtdkp@google.com>

include/commissioner/network_diag_data.hpp

@@ -106,7 +106,8 @@ struct Route64
  */
 struct ChildIpv6AddrInfo
 {
-    uint16_t                 mRloc16 = 0;
+    uint16_t                 mRloc16  = 0;
+    uint16_t                 mChildId = 0;
     std::vector<std::string> mAddresses;
 };
 

src/library/CMakeLists.txt

@@ -34,6 +34,7 @@ add_library(commissioner
     coap_secure.hpp
     commissioner_impl.cpp
     commissioner_impl.hpp
+    commissioner_impl_internal.hpp
     commissioner_safe.cpp
     commissioner_safe.hpp
     cose.cpp
@@ -148,6 +149,7 @@ if (OT_COMM_TEST)
         coap.hpp
         coap_test.cpp
         commissioner_impl.hpp
+        commissioner_impl_internal.hpp
         commissioner_impl_test.cpp
         commissioner_safe.hpp
         commissioner_safe_test.cpp

src/library/commissioner_impl.cpp

@@ -52,6 +52,7 @@
 #include "common/utils.hpp"
 #include "event2/event.h"
 #include "library/coap.hpp"
+#include "library/commissioner_impl_internal.hpp"
 #include "library/dtls.hpp"
 #include "library/joiner_session.hpp"
 #include "library/openthread/bloom_filter.hpp"
@@ -73,13 +74,6 @@ static constexpr uint16_t kPrimaryBbrAloc16 = 0xFC38;
 static constexpr uint32_t kMinKeepAliveInterval = 30;
 static constexpr uint32_t kMaxKeepAliveInterval = 45;
 
-static constexpr uint8_t kChildTableEntryBytes = 3;
-static constexpr uint8_t kIpv6AddressBytes     = 16;
-static constexpr uint8_t kLeaderDataBytes      = 8;
-static constexpr uint8_t kMacCountersBytes     = 36;
-static constexpr uint8_t kRloc16Bytes          = 2;
-static constexpr uint8_t kRouterIdMaskBytes    = 8;
-
 Error Commissioner::GeneratePSKc(ByteArray         &aPSKc,
                                  const std::string &aPassphrase,
                                  const std::string &aNetworkName,
@@ -2019,7 +2013,7 @@ ByteArray CommissionerImpl::GetNetDiagTlvTypes(uint64_t aDiagTlvFlags)
     return tlvTypes;
 }
 
-Error CommissionerImpl::DecodeNetDiagData(NetDiagData &aNetDiagData, const ByteArray &aPayload)
+Error internal::DecodeNetDiagData(NetDiagData &aNetDiagData, const ByteArray &aPayload)
 {
     Error        error;
     tlv::TlvSet  tlvSet;
@@ -2045,7 +2039,7 @@ Error CommissionerImpl::DecodeNetDiagData(NetDiagData &aNetDiagData, const ByteA
 
     if (auto mode = tlvSet[tlv::Type::kNetworkDiagMode])
     {
-        SuccessOrExit(error = DecodeModeData(diagData.mMode, mode->GetValue()));
+        SuccessOrExit(error = internal::DecodeModeData(diagData.mMode, mode->GetValue()));
         diagData.mPresentFlags |= NetDiagData::kModeBit;
     }
 
@@ -2109,7 +2103,7 @@ Error CommissionerImpl::DecodeNetDiagData(NetDiagData &aNetDiagData, const ByteA
     return error;
 }
 
-Error CommissionerImpl::DecodeIpv6AddressList(std::vector<std::string> &aAddrs, const ByteArray &aBuf)
+Error internal::DecodeIpv6AddressList(std::vector<std::string> &aAddrs, const ByteArray &aBuf)
 {
     Error  error;
     size_t length = aBuf.size();
@@ -2128,16 +2122,17 @@ Error CommissionerImpl::DecodeIpv6AddressList(std::vector<std::string> &aAddrs,
     return error;
 }
 
-Error CommissionerImpl::DecodeChildIpv6AddressList(std::vector<ChildIpv6AddrInfo> &aChildIpv6AddressInfoList,
-                                                   const ByteArray                &aBuf)
+Error internal::DecodeChildIpv6AddressList(std::vector<ChildIpv6AddrInfo> &aChildIpv6AddressInfoList,
+                                           const ByteArray                &aBuf)
 {
     Error             error;
     size_t            length = aBuf.size();
     ChildIpv6AddrInfo childIpv6AddrsInfo;
 
     VerifyOrExit((length % kIpv6AddressBytes) == kRloc16Bytes,
                  error = ERROR_BAD_FORMAT("premature end of Child IPv6 Address"));
-    childIpv6AddrsInfo.mRloc16 = utils::Decode<uint16_t>(aBuf.data(), kRloc16Bytes);
+    childIpv6AddrsInfo.mRloc16  = utils::Decode<uint16_t>(aBuf.data(), kRloc16Bytes);
+    childIpv6AddrsInfo.mChildId = childIpv6AddrsInfo.mRloc16 & 0x1FF;
 
     SuccessOrExit(error =
                       DecodeIpv6AddressList(childIpv6AddrsInfo.mAddresses, {aBuf.begin() + kRloc16Bytes, aBuf.end()}));
@@ -2147,13 +2142,13 @@ Error CommissionerImpl::DecodeChildIpv6AddressList(std::vector<ChildIpv6AddrInfo
     return error;
 }
 
-Error CommissionerImpl::DecodeModeData(ModeData &aModeData, const ByteArray &aBuf)
+Error internal::DecodeModeData(ModeData &aModeData, const ByteArray &aBuf)
 {
     Error  error;
     size_t length = aBuf.size();
     VerifyOrExit(length == 1, error = ERROR_BAD_FORMAT("invalid Mode value length {}, expect 1", length));
     aModeData.mIsRxOnWhenIdleMode          = (aBuf[0] & 0x01) != 0;
-    aModeData.mIsMtd                       = (aBuf[0] & 0x02) != 0;
+    aModeData.mIsMtd                       = (aBuf[0] & 0x02) == 0;
     aModeData.mIsStableNetworkDataRequired = (aBuf[0] & 0x04) != 0;
 
 exit:
@@ -2177,7 +2172,7 @@ Error CommissionerImpl::DecodeModeData(ModeData &aModeData, const ByteArray &aBu
  * @param aBuf The buffer to be decoded
  * @return Error code
  */
-Error CommissionerImpl::DecodeChildTable(std::vector<ChildTableEntry> &aChildTable, const ByteArray &aBuf)
+Error internal::DecodeChildTable(std::vector<ChildTableEntry> &aChildTable, const ByteArray &aBuf)
 {
     Error  error;
     size_t length = aBuf.size();
@@ -2200,7 +2195,7 @@ Error CommissionerImpl::DecodeChildTable(std::vector<ChildTableEntry> &aChildTab
     return error;
 }
 
-Error CommissionerImpl::DecodeLeaderData(LeaderData &aLeaderData, const ByteArray &aBuf)
+Error internal::DecodeLeaderData(LeaderData &aLeaderData, const ByteArray &aBuf)
 {
     Error  error;
     size_t length = aBuf.size();
@@ -2215,7 +2210,7 @@ Error CommissionerImpl::DecodeLeaderData(LeaderData &aLeaderData, const ByteArra
     return error;
 }
 
-Error CommissionerImpl::DecodeRoute64(Route64 &aRoute64, const ByteArray &aBuf)
+Error internal::DecodeRoute64(Route64 &aRoute64, const ByteArray &aBuf)
 {
     Error     error;
     size_t    length = aBuf.size();
@@ -2242,14 +2237,14 @@ Error CommissionerImpl::DecodeRoute64(Route64 &aRoute64, const ByteArray &aBuf)
     return error;
 }
 
-void CommissionerImpl::DecodeRouteDataEntry(RouteDataEntry &aRouteDataEntry, uint8_t aBuf)
+void internal::DecodeRouteDataEntry(RouteDataEntry &aRouteDataEntry, uint8_t aBuf)
 {
     aRouteDataEntry.mOutgoingLinkQuality = (aBuf >> 6) & 0x03;
     aRouteDataEntry.mIncomingLinkQuality = (aBuf >> 4) & 0x03;
     aRouteDataEntry.mRouteCost           = aBuf & 0x0F;
 }
 
-ByteArray CommissionerImpl::ExtractRouterIds(const ByteArray &aMask)
+ByteArray internal::ExtractRouterIds(const ByteArray &aMask)
 {
     ByteArray routerIdList;
 
@@ -2264,7 +2259,7 @@ ByteArray CommissionerImpl::ExtractRouterIds(const ByteArray &aMask)
     return routerIdList;
 }
 
-Error CommissionerImpl::DecodeMacCounters(MacCounters &aMacCounters, const ByteArray &aBuf)
+Error internal::DecodeMacCounters(MacCounters &aMacCounters, const ByteArray &aBuf)
 {
     Error  error;
     size_t length = aBuf.size();

src/library/commissioner_impl.hpp

@@ -226,18 +226,7 @@ class CommissionerImpl : public Commissioner
     static Error     EncodeCommissionerDataset(coap::Request &aRequest, const CommissionerDataset &aDataset);
     static ByteArray GetCommissionerDatasetTlvs(uint16_t aDatasetFlags);
 
-    static Error     DecodeNetDiagData(NetDiagData &aNetDiagData, const ByteArray &aPayload);
     static ByteArray GetNetDiagTlvTypes(uint64_t aDiagTlvFlags);
-    static Error     DecodeIpv6AddressList(std::vector<std::string> &aAddrs, const ByteArray &aBuf);
-    static Error     DecodeChildIpv6AddressList(std::vector<ChildIpv6AddrInfo> &aChildIpv6AddressInfoList,
-                                                const ByteArray                &aBuf);
-    static Error     DecodeModeData(ModeData &aModeData, const ByteArray &aBuf);
-    static Error     DecodeChildTable(std::vector<ChildTableEntry> &aChildTable, const ByteArray &aBuf);
-    static Error     DecodeLeaderData(LeaderData &aLeaderData, const ByteArray &aBuf);
-    static Error     DecodeMacCounters(MacCounters &aMacCounters, const ByteArray &aBuf);
-    static Error     DecodeRoute64(Route64 &aRoute64, const ByteArray &aBuf);
-    static void      DecodeRouteDataEntry(RouteDataEntry &aRouteDataEntry, uint8_t aBuf);
-    static ByteArray ExtractRouterIds(const ByteArray &aMask);
 
     void SendPetition(PetitionHandler aHandler);
 

src/library/commissioner_impl_internal.hpp

@@ -0,0 +1,68 @@
+/*
+ *    Copyright (c) 2024, The OpenThread Commissioner Authors.
+ *    All rights reserved.
+ *
+ *    Redistribution and use in source and binary forms, with or without
+ *    modification, are permitted provided that the following conditions are met:
+ *    1. Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *    2. Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *    3. Neither the name of the copyright holder nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *    THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ *    AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ *    IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ *    ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ *    LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ *    CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ *    SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ *    INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ *    CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ *    ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ *    POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef OT_COMM_COMMISSIONER_IMPL_INTERNAL_HPP_
+#define OT_COMM_COMMISSIONER_IMPL_INTERNAL_HPP_
+
+#include <string>
+#include <vector>
+
+#include "commissioner/error.hpp"
+#include "commissioner/network_diag_data.hpp"
+
+namespace ot {
+
+namespace commissioner {
+
+static constexpr uint8_t kChildTableEntryBytes = 3;
+static constexpr uint8_t kIpv6AddressBytes     = 16;
+static constexpr uint8_t kLeaderDataBytes      = 8;
+static constexpr uint8_t kMacCountersBytes     = 36;
+static constexpr uint8_t kRloc16Bytes          = 2;
+static constexpr uint8_t kRouterIdMaskBytes    = 8;
+
+namespace internal {
+
+Error     DecodeIpv6AddressList(std::vector<std::string> &aAddrs, const ByteArray &aBuf);
+Error     DecodeChildIpv6AddressList(std::vector<ChildIpv6AddrInfo> &aChildIpv6AddressInfoList, const ByteArray &aBuf);
+Error     DecodeNetDiagData(NetDiagData &aNetDiagData, const ByteArray &aPayload);
+Error     DecodeModeData(ModeData &aModeData, const ByteArray &aBuf);
+Error     DecodeChildTable(std::vector<ChildTableEntry> &aChildTable, const ByteArray &aBuf);
+Error     DecodeLeaderData(LeaderData &aLeaderData, const ByteArray &aBuf);
+Error     DecodeMacCounters(MacCounters &aMacCounters, const ByteArray &aBuf);
+Error     DecodeRoute64(Route64 &aRoute64, const ByteArray &aBuf);
+void      DecodeRouteDataEntry(RouteDataEntry &aRouteDataEntry, uint8_t aBuf);
+ByteArray ExtractRouterIds(const ByteArray &aMask);
+
+} // namespace internal
+
+} // namespace commissioner
+
+} // namespace ot
+
+#endif // OT_COMM_COMMISSIONER_IMPL_INTERNAL_HPP_

src/library/commissioner_impl_test.cpp

@@ -35,9 +35,11 @@
  */
 
 #include "library/commissioner_impl.hpp"
+#include "library/commissioner_impl_internal.hpp"
 
 #include <gtest/gtest.h>
 
+#include "commissioner/defines.hpp"
 #include "common/utils.hpp"
 
 namespace ot {
@@ -143,6 +145,38 @@ TEST(CommissionerImpl, NotImplementedApis)
     event_base_free(eventBase);
 }
 
+TEST(CommissionerImplTest, ValidInput_DecodeNetDiagData)
+{
+    ByteArray   buf;
+    NetDiagData diagData;
+    std::string tlvsHexString =
+        "00086ac6c2de12b212df0102c80002010f0512e7000400204300300af1f1f1f1f101f1f1f10608360bb9f7415c30210840fd9238a3395d"
+        "0001f9043dfeb7b3edf3fd7d604fb88a0000000000fffe00c800fd7d604fb88a0000fe3e5a4c31acb559fe8000000000000068c6c2de12"
+        "b212df1009601804601d046019041e227018fdc31ff45feff4e7e580431c60becfabfd110022000000008df846f3ab0c05551e227002fd"
+        "c31ff45feff4e75257420f1cbd46f5fd1100220000000034e5d9e28d1952c0";
+    utils::Hex(buf, tlvsHexString);
+
+    Error     error = ot::commissioner::internal::DecodeNetDiagData(diagData, buf);
+    ByteArray extMacAddrBytes;
+    uint16_t  macAddr = 0xc800;
+    utils::Hex(extMacAddrBytes, "6ac6c2de12b212df");
+
+    EXPECT_EQ(error, ErrorCode::kNone);
+    EXPECT_EQ(diagData.mPresentFlags, 639);
+    EXPECT_EQ(diagData.mExtMacAddr, extMacAddrBytes);
+    EXPECT_EQ(diagData.mMacAddr, macAddr);
+    EXPECT_EQ(diagData.mMode.mIsMtd, false);
+    EXPECT_EQ(diagData.mRoute64.mRouteData.size(), 9);
+    EXPECT_EQ(diagData.mAddrs.size(), 4);
+    EXPECT_EQ(diagData.mAddrs[0], "fd92:38a3:395d:1:f904:3dfe:b7b3:edf3");
+    EXPECT_EQ(diagData.mChildTable.size(), 3);
+    EXPECT_EQ(diagData.mChildTable[0].mChildId, 24);
+    EXPECT_EQ(diagData.mLeaderData.mRouterId, 33);
+    EXPECT_EQ(diagData.mChildIpv6AddrsInfoList.size(), 2);
+    EXPECT_EQ(diagData.mChildIpv6AddrsInfoList[0].mRloc16, 28696);
+    EXPECT_EQ(diagData.mChildIpv6AddrsInfoList[0].mChildId, 24);
+}
+
 } // namespace commissioner
 
 } // namespace ot

src/library/tlv.cpp

@@ -274,7 +274,7 @@ bool Tlv::IsValid() const
         case Type::kNetworkDiagChild:
             return length >= 43;
         case Type::kNetworkDiagChildIpv6Address:
-            return (length % 16 == 0) && (length / 16 >= 1);
+            return (length % 16 == 2) && (length / 16 >= 1);
         case Type::kNetworkDiagRouterNeighbor:
             return length >= 24;
         case Type::kNetworkDiagAnswer: