Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tlv] add new tlvs for network diagnostic #266

Closed
wants to merge 13 commits into from
Closed

[tlv] add new tlvs for network diagnostic #266

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f0bbe33
[tlv] add new tlvs for network diagnostic
ZhangLe2016 May 6, 2024
1cc8322
[tlv] add new tlvs for network diagnostic
ZhangLe2016 May 6, 2024
af7f86f
[tlv] add new tlvs for network diagnostic
ZhangLe2016 May 6, 2024
8ada05f
[build] output test files to ./tmp (#265)
wgtdkp May 6, 2024
57bd5f1
[api] remove the `XpanId` and `PanId` structs (#264)
wgtdkp May 8, 2024
6561ae7
[android] cleanup Android sample app (#277)
wgtdkp May 16, 2024
397114d
[log] add a new logging region for network diagnostic functionality (…
ZhangLe2016 May 21, 2024
1913f47
[uri] add DIAG_GET URIs for disgnostic APIs (#267)
ZhangLe2016 May 23, 2024
62a1b82
[udp-proxy] set local port to dedicated port number(:mm) for udp prox…
ZhangLe2016 Jun 19, 2024
384ec58
[thread] make Joiner Provisioning URL independently optional (#285)
wgtdkp Jun 24, 2024
af15906
[coap] handle zero coap token length (#286)
wgtdkp Jul 9, 2024
e0820a3
[swig] fix the SWIG ignores for java (#287)
wgtdkp Jul 15, 2024
70c626d
[tlv] add new tlvs for network diagnostic
ZhangLe2016 May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions src/library/tlv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,71 @@ bool Tlv::IsValid() const
{
return false;
}
else if (mScope == Scope::kNetworkDiag)
{
switch (mType)
{
// Network disgnostic layer TLVs
case Type::kNetworkDiagExtMacAddress:
return length == 8;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally consider a TLV valid if its length is at least as long as the currently defined value length. This accommodates potential future extensions of the TLV format.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abtink Thank you for your review. I have clarified the length of TLVs as per the definition in the spec. For instance, the length of the extended MAC address TLV in spec 5.19.2 is 8. Could you please kindly indicate any other inappropriate definitions in this PR?

Copy link
Member

@wgtdkp wgtdkp May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @abtink wants you to change the check to return length >= 8; so that the extended MAC address can be extended to maybe 16 bytes in Thread 2.0 or whatever later versions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, Thanks @wgtdkp clarification. Let me update check condition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @wgtdkp :). Yes. Exactly.
Thank @ZhangLe2016 for changing.

case Type::kNetworkDiagMacAddress:
return length == 2;
case Type::kNetworkDiagMode:
return length == 1;
case Type::kNetworkDiagTimeout:
return length == 4;
case Type::kNetworkDiagRoute64:
return length >= 4;
case Type::kNetworkDiagLeaderData:
return length == 8;
case Type::kNetworkDiagNetworkData:
return true;
case Type::kNetworkDiagIpv6Address:
return (length % 16 == 0) && (length / 16 >= 1 && length / 16 <= 15);
case Type::kNetworkDiagMacCounters:
return length <= 36;
case Type::kNetworkDiagBatteryLevel:
return length == 1;
case Type::kNetworkDiagSupplyVoltage:
return length == 2;
case Type::kNetworkDiagChildTable:
return true; // list of 0 or more child entry data
case Type::kNetworkDiagChannelPages:
return length >= 1; // 1 or more 8-bit integers
case Type::kNetworkDiagTypeList:
return length >= 1; // 1 or more 8-bit integers
case Type::kNetworkDiagMaxChildTimeout:
return length == 4;
case Type::kNetworkDiagLDevIDSubjectPubKeyInfo:
return true;
case Type::kNetworkDiagIDevIDCert:
return true;
case Type::kNetworkDiagEui64:
return length == 8;
case Type::kNetworkDiagVersion:
return length == 2;
case Type::kNetworkDiagVendorName:
return length <= 4;
case Type::kNetworkDiagVendorModel:
return length <= 4;
case Type::kNetworkDiagVendorSWVersion:
return length <= 2;
case Type::kNetworkDiagChild:
return length <= 43;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these also use >=?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, the TLV will only have content when the queried device has children. If the queried device has no children, the value of TLV is empty, so the length should be 0. If we check the length to be >= 43, then when the value of TLV is empty, the TLV will be considered invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLV kNetworkDiagVendorSWVersion is similar. If the device itself does not have a sw version content, then the value of TLV is empty. When the commissioner gets an empty TLV, it should not be judged as invalid tlv. Please correct me~~ Thanks!

case Type::kNetworkDiagChildIpv6Address:
return (length % 16 == 0) && (length / 16 >= 1 && length / 16 <= 15);
case Type::kNetworkDiagRouterNeighbor:
return length <= 24;
case Type::kNetworkDiagAnswer:
return length == 2;
case Type::kNetworkDiagQueryID:
return length == 2;
case Type::kNetworkDiagMleCounters:
return length <= 66;
default:
return false;
}
}

switch (mType)
{
Expand Down
33 changes: 33 additions & 0 deletions src/library/tlv.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ enum class Scope : uint8_t
kMeshCoP = 0,
kThread,
kMeshLink,
kNetworkDiag,
};

enum class Type : uint8_t
Expand Down Expand Up @@ -139,6 +140,38 @@ enum class Type : uint8_t
kCommissionerPenSignature = 70,
kDiscoveryRequest = 128,
kDiscoveryResponse = 129,

// TMF Network Diagnositcs TLVs
kNetworkDiagExtMacAddress = 0,
kNetworkDiagMacAddress = 1,
kNetworkDiagMode = 2,
kNetworkDiagTimeout = 3,
kNetworkDiagConnectivity = 4,
kNetworkDiagRoute64 = 5,
kNetworkDiagLeaderData = 6,
kNetworkDiagNetworkData = 7,
kNetworkDiagIpv6Address = 8,
kNetworkDiagMacCounters = 9,
kNetworkDiagBatteryLevel = 14,
kNetworkDiagSupplyVoltage = 15,
kNetworkDiagChildTable = 16,
kNetworkDiagChannelPages = 17,
kNetworkDiagTypeList = 18,
kNetworkDiagMaxChildTimeout = 19,
kNetworkDiagLDevIDSubjectPubKeyInfo = 20,
kNetworkDiagIDevIDCert = 21,
kNetworkDiagEui64 = 23,
kNetworkDiagVersion = 24,
kNetworkDiagVendorName = 25,
kNetworkDiagVendorModel = 26,
kNetworkDiagVendorSWVersion = 27,
kNetworkDiagThreadStackVersion = 28,
kNetworkDiagChild = 29,
kNetworkDiagChildIpv6Address = 30,
kNetworkDiagRouterNeighbor = 31,
kNetworkDiagAnswer = 32,
kNetworkDiagQueryID = 33,
kNetworkDiagMleCounters = 34,
};

using TlvPtr = std::shared_ptr<Tlv>;
Expand Down
Loading