[WiFi] Fix Vendor HAL wifi_get_link_stats
Emulator has its own implementation of wifi_get_link_stats which
reports link layer stats received from kernel.
There are 2 bugs in its implementation which caused the CTS test case
to fail.
1, FindAttribute function doesn't correctly parse nlmsg received from
kernel. Therefore, we use the API from libnl instead.
2, Based on the wifi_legacy_hal implementation in framework,
wifi_get_link_stats should be blocking and must invoke the
wifi_stats_result_handler within the function itself. Otherwise,
the result handler might never be called again because
wifi_legacy_hal set it to nullptr after calling wifi_get_link_stats.
Therefore, we make the API to wait until onLinkStatsReply is called
in order to avoid the race condition.
BUG: 158503396
TEST: run cts -m CtsWifiTestCases -t
android.net.wifi.cts.WifiManagerTest#testTrafficStateCallback
Signed-off-by: Weilun Du <wdu@google.com>
Change-Id: If50d6bbccd142b0d05c0e7e4ed1eecc3be2b3c5d
(cherry picked from commit 08c3e77e9bb5416c876f7169769c9aa744a66f23)
diff --git a/wifi/wifi_hal/Android.bp b/wifi/wifi_hal/Android.bp
index 88c1e30..6431003 100644
--- a/wifi/wifi_hal/Android.bp
+++ b/wifi/wifi_hal/Android.bp
@@ -28,6 +28,7 @@
"wifi_hal.cpp",
],
shared_libs: [
+ "libnl",
"liblog",
"libcutils",
"libhardware_legacy",
diff --git a/wifi/wifi_hal/interface.cpp b/wifi/wifi_hal/interface.cpp
index 7c683e5..47d8154 100644
--- a/wifi/wifi_hal/interface.cpp
+++ b/wifi/wifi_hal/interface.cpp
@@ -86,6 +86,9 @@
return WIFI_SUCCESS;
}
+// Wifi legacy HAL implicitly assumes getLinkStats is blocking and
+// handler will be set to nullptr immediately after invocation.
+// Therefore, this function will wait until onLinkStatsReply is called.
wifi_error Interface::getLinkStats(wifi_request_id requestId,
wifi_stats_result_handler handler) {
NetlinkMessage message(RTM_GETLINK, mNetlink.getSequenceNumber());
@@ -97,12 +100,21 @@
info->ifi_flags = 0;
info->ifi_change = 0xFFFFFFFF;
- bool success = mNetlink.sendMessage(message,
- std::bind(&Interface::onLinkStatsReply,
- this,
- requestId,
- handler,
- std::placeholders::_1));
+ std::condition_variable condition;
+ std::mutex mutex;
+ std::unique_lock<std::mutex> lock(mutex);
+ bool stopped = false;
+ auto callback = [this, requestId, &handler,
+ &mutex, &condition, &stopped] (const NetlinkMessage& message) {
+ stopped = true;
+ std::unique_lock<std::mutex> lock(mutex);
+ onLinkStatsReply(requestId, handler, message);
+ condition.notify_all();
+ };
+ bool success = mNetlink.sendMessage(message, callback);
+ while (!stopped) {
+ condition.wait(lock);
+ }
return success ? WIFI_SUCCESS : WIFI_ERROR_UNKNOWN;
}
diff --git a/wifi/wifi_hal/netlinkmessage.cpp b/wifi/wifi_hal/netlinkmessage.cpp
index baf5800..06bb743 100644
--- a/wifi/wifi_hal/netlinkmessage.cpp
+++ b/wifi/wifi_hal/netlinkmessage.cpp
@@ -22,6 +22,7 @@
#include <linux/rtnetlink.h>
#include <sys/types.h>
#include <unistd.h>
+#include <netlink/msg.h>
size_t getSpaceForMessageType(uint16_t type) {
switch (type) {
@@ -51,13 +52,12 @@
bool NetlinkMessage::getAttribute(int attributeId, void* data, size_t size) const {
const void* value = nullptr;
- uint16_t attrSize = 0;
- if (!findAttribute(attributeId, &value, &attrSize)) {
+ const auto attr = nlmsg_find_attr((struct nlmsghdr*)mData.data(), sizeof(ifinfomsg), attributeId);
+ if (!attr) {
return false;
}
- if (size > attrSize) {
- return false;
- }
+ value = (const uint8_t*) attr + NLA_HDRLEN;
+ size = attr->nla_len;
memcpy(data, value, size);
return true;
}
@@ -71,30 +71,3 @@
auto header = reinterpret_cast<const nlmsghdr*>(mData.data());
return header->nlmsg_seq;
}
-
-bool NetlinkMessage::findAttribute(int attributeId,
- const void** value,
- uint16_t* size) const {
- const uint8_t* end = mData.data() + mData.size();
- size_t attrOffset = getSpaceForMessageType(type());
- if (attrOffset == 0) {
- return false;
- }
- const uint8_t* attribute = mData.data() + attrOffset;
- while (attribute < end) {
- auto header = reinterpret_cast<const nlattr*>(attribute);
- if (header->nla_len == 0) {
- // The length should include the header so the length should always
- // be greater than zero. If it doesn't we're going to end up looping
- // forever so ignore this.
- return false;
- }
- if (header->nla_type == attributeId) {
- *value = attribute + NLA_HDRLEN;
- *size = header->nla_len;
- return true;
- }
- attribute += header->nla_len;
- }
- return false;
-}
diff --git a/wifi/wifi_hal/netlinkmessage.h b/wifi/wifi_hal/netlinkmessage.h
index 45fd7cd..5e1a3b8 100644
--- a/wifi/wifi_hal/netlinkmessage.h
+++ b/wifi/wifi_hal/netlinkmessage.h
@@ -60,9 +60,6 @@
NetlinkMessage& operator=(const NetlinkMessage&) = delete;
bool getAttribute(int attributeId, void* data, size_t size) const;
- bool findAttribute(int attributeId,
- const void** value,
- uint16_t* size) const;
std::vector<uint8_t> mData;
};