[link-metrics] simplify HandleReport() (#8054)

This commit updates `LinkMetrics::HandleReport()` method:
- Fixes issue where `Status` sub-TLV would not be read.
- Uses `Tlv` helper method to read the sub-TLVs.
- Reads `ReportSubTlv` directly from message and uses its methods
  to get the metrics values.
- Fixes issue where we would not skip over unknown sub-TLVs
  correctly.
This commit is contained in:
Abtin Keshavarzian
2022-08-23 14:34:09 -07:00
committed by GitHub
parent 3d146f3f60
commit 9d70832b85
3 changed files with 58 additions and 65 deletions
+47 -56
View File
@@ -49,8 +49,6 @@ namespace LinkMetrics {
RegisterLogModule("LinkMetrics");
using ot::Encoding::BigEndian::HostSwap32;
LinkMetrics::LinkMetrics(Instance &aInstance)
: InstanceLocator(aInstance)
, mReportCallback(nullptr)
@@ -418,16 +416,15 @@ void LinkMetrics::HandleReport(const Message & aMessage,
uint16_t aLength,
const Ip6::Address &aAddress)
{
Error error = kErrorNone;
MetricsValues values;
uint8_t rawValue;
uint16_t pos = aOffset;
uint16_t endPos = aOffset + aLength;
Tlv tlv;
TypeIdFlags typeIdFlags;
Error error = kErrorNone;
uint16_t offset = aOffset;
uint16_t endOffset = aOffset + aLength;
bool hasStatus = false;
bool hasReport = false;
Status status;
Tlv tlv;
ReportSubTlv reportTlv;
MetricsValues values;
uint8_t status;
OT_UNUSED_VARIABLE(error);
@@ -435,90 +432,84 @@ void LinkMetrics::HandleReport(const Message & aMessage,
values.Clear();
while (pos < endPos)
while (offset < endOffset)
{
SuccessOrExit(aMessage.Read(pos, tlv));
VerifyOrExit(tlv.GetType() == SubTlv::kReport);
pos += sizeof(Tlv);
SuccessOrExit(error = aMessage.Read(offset, tlv));
VerifyOrExit(pos + tlv.GetLength() <= endPos, error = kErrorParse);
VerifyOrExit(offset + sizeof(Tlv) + tlv.GetLength() <= endOffset, error = kErrorParse);
// The report must contain either:
// - One or more Report Sub-TLVs (in case of success), or
// - A single Status Sub-TLV (in case of failure).
switch (tlv.GetType())
{
case SubTlv::kStatus:
// There should be either: one Status TLV or some Report-Sub TLVs
case StatusSubTlv::kType:
VerifyOrExit(!hasStatus && !hasReport, error = kErrorDrop);
VerifyOrExit(tlv.GetLength() == sizeof(status), error = kErrorParse);
SuccessOrExit(aMessage.Read(pos, status));
SuccessOrExit(error = Tlv::Read<StatusSubTlv>(aMessage, offset, status));
hasStatus = true;
pos += sizeof(status);
break;
case SubTlv::kReport:
// There shouldn't be any Report-Sub TLV when there's a Status TLV
case ReportSubTlv::kType:
VerifyOrExit(!hasStatus, error = kErrorDrop);
VerifyOrExit(tlv.GetLength() > sizeof(typeIdFlags), error = kErrorParse);
SuccessOrExit(aMessage.Read(pos, typeIdFlags));
if (typeIdFlags.IsExtendedFlagSet())
// Read the report sub-TLV assuming minimum length
SuccessOrExit(error = Tlv::ReadTlv(aMessage, offset, &reportTlv, ReportSubTlv::kMinLength));
VerifyOrExit(reportTlv.IsValid(), error = kErrorParse);
hasReport = true;
if (reportTlv.GetMetricsTypeId().IsExtendedFlagSet())
{
pos += tlv.GetLength(); // Skip the whole sub-TLV if `E` flag is set
continue;
// Skip the sub-TLV if `E` flag is set.
break;
}
hasReport = true;
pos += sizeof(TypeIdFlags);
if (reportTlv.GetMetricsTypeId().IsLengthFlagSet())
{
// If Type ID indicates metric value has 4 bytes length, we
// read the full `reportTlv`.
SuccessOrExit(error = aMessage.Read(offset, reportTlv));
}
switch (typeIdFlags.GetRawValue())
switch (reportTlv.GetMetricsTypeId().GetRawValue())
{
case TypeIdFlags::kPdu:
values.GetMetrics().mPduCount = true;
SuccessOrExit(aMessage.Read(pos, values.mPduCountValue));
values.mPduCountValue = HostSwap32(values.mPduCountValue);
pos += sizeof(uint32_t);
values.mMetrics.mPduCount = true;
values.mPduCountValue = reportTlv.GetMetricsValue32();
LogDebg(" - PDU Counter: %d (Count/Summation)", values.mPduCountValue);
break;
case TypeIdFlags::kLqi:
values.GetMetrics().mLqi = true;
SuccessOrExit(aMessage.Read(pos, values.mLqiValue));
pos += sizeof(uint8_t);
values.mMetrics.mLqi = true;
values.mLqiValue = reportTlv.GetMetricsValue8();
LogDebg(" - LQI: %d (Exponential Moving Average)", values.mLqiValue);
break;
case TypeIdFlags::kLinkMargin:
values.GetMetrics().mLinkMargin = true;
SuccessOrExit(aMessage.Read(pos, rawValue));
values.mMetrics.mLinkMargin = true;
// Reverse operation for linear scale, map from [0, 255] to [0, 130]
values.mLinkMarginValue = rawValue * 130 / 255;
pos += sizeof(uint8_t);
values.mLinkMarginValue = reportTlv.GetMetricsValue8() * 130 / 255;
LogDebg(" - Margin: %d (dB) (Exponential Moving Average)", values.mLinkMarginValue);
break;
case TypeIdFlags::kRssi:
values.GetMetrics().mRssi = true;
SuccessOrExit(aMessage.Read(pos, rawValue));
values.mMetrics.mRssi = true;
// Reverse operation for linear scale, map from [0, 255] to [-130, 0]
values.mRssiValue = rawValue * 130 / 255 - 130;
pos += sizeof(uint8_t);
values.mRssiValue = reportTlv.GetMetricsValue8() * 130 / 255 - 130;
LogDebg(" - RSSI: %d (dBm) (Exponential Moving Average)", values.mRssiValue);
break;
default:
break;
}
break;
}
offset += sizeof(Tlv) + tlv.GetLength();
}
if (hasStatus)
{
mReportCallback(&aAddress, nullptr, status, mReportCallbackContext);
}
else if (hasReport)
{
mReportCallback(&aAddress, &values, OT_LINK_METRICS_STATUS_SUCCESS, mReportCallbackContext);
}
VerifyOrExit(hasStatus || hasReport);
mReportCallback(&aAddress, hasStatus ? nullptr : &values, hasStatus ? static_cast<Status>(status) : kStatusSuccess,
mReportCallbackContext);
exit:
LogDebg("HandleReport, error:%s", ErrorToString(error));
+3 -1
View File
@@ -94,6 +94,8 @@ OT_TOOL_PACKED_BEGIN
class ReportSubTlv : public Tlv, public TlvInfo<SubTlv::kReport>
{
public:
static constexpr uint8_t kMinLength = sizeof(TypeIdFlags) + sizeof(uint8_t); ///< Minimum expected TLV length.
/**
* This method initializes the TLV.
*
@@ -111,7 +113,7 @@ public:
* @retval false The TLV does not appear to be well-formed.
*
*/
bool IsValid(void) const { return GetLength() >= sizeof(TypeIdFlags) + sizeof(uint8_t); }
bool IsValid(void) const { return GetLength() >= kMinLength; }
/**
* This method returns the Link Metrics Type ID.
+8 -8
View File
@@ -2769,21 +2769,21 @@ exit:
void Mle::HandleDataResponse(RxInfo &aRxInfo)
{
Error error;
#if OPENTHREAD_CONFIG_MLE_LINK_METRICS_INITIATOR_ENABLE
uint16_t metricsReportValueOffset;
uint16_t length;
#endif
Log(kMessageReceive, kTypeDataResponse, aRxInfo.mMessageInfo.GetPeerAddr());
VerifyOrExit(aRxInfo.mNeighbor && aRxInfo.mNeighbor->IsStateValid(), error = kErrorDrop);
#if OPENTHREAD_CONFIG_MLE_LINK_METRICS_INITIATOR_ENABLE
if (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kLinkMetricsReport, metricsReportValueOffset, length) ==
kErrorNone)
{
Get<LinkMetrics::LinkMetrics>().HandleReport(aRxInfo.mMessage, metricsReportValueOffset, length,
aRxInfo.mMessageInfo.GetPeerAddr());
uint16_t offset;
uint16_t length;
if (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kLinkMetricsReport, offset, length) == kErrorNone)
{
Get<LinkMetrics::LinkMetrics>().HandleReport(aRxInfo.mMessage, offset, length,
aRxInfo.mMessageInfo.GetPeerAddr());
}
}
#endif