wifi: Wait for driver ready and bring up the interface when setMacAddress fails
setMacAddress may fail in some scenarios like SSR inprogress. In such
case framework is not bringing up the iface again if it was brought down
to set random MAC address. Due to this subsequent operations like scans
are failing with "Network Down" error and Wi-Fi can't recover until
Wi-Fi restarts. To avoid this bring up the iface irrespective of
setMacAddress status.
Modified the original CL to move the WifiIfaceUtil creation to inside
Wifi object since that is where the legacy HAL instance is created for
the corresponding chip. This helps keeping the setMacAddress logic still
inside WifiIfaceUtil. Modified the iface_util lifetime - no longer a
singleton, one instance created per wifi chip instance.
Bug: 174183763
Test: Wifi can be enabled when back-to-back SSR and wifi on
Change-Id: I926b59f5da126aba222e05d1e570c0c19de739ed
diff --git a/wifi/1.5/default/service.cpp b/wifi/1.5/default/service.cpp
index 23e2b47..3de49b2 100644
--- a/wifi/1.5/default/service.cpp
+++ b/wifi/1.5/default/service.cpp
@@ -32,7 +32,6 @@
using android::hardware::LazyServiceRegistrar;
using android::hardware::wifi::V1_5::implementation::feature_flags::
WifiFeatureFlags;
-using android::hardware::wifi::V1_5::implementation::iface_util::WifiIfaceUtil;
using android::hardware::wifi::V1_5::implementation::legacy_hal::WifiLegacyHal;
using android::hardware::wifi::V1_5::implementation::legacy_hal::
WifiLegacyHalFactory;
@@ -63,7 +62,6 @@
new android::hardware::wifi::V1_5::implementation::Wifi(
iface_tool, legacy_hal_factory,
std::make_shared<WifiModeController>(),
- std::make_shared<WifiIfaceUtil>(iface_tool),
std::make_shared<WifiFeatureFlags>());
if (kLazyService) {
auto registrar = LazyServiceRegistrar::getInstance();
diff --git a/wifi/1.5/default/tests/mock_wifi_iface_util.cpp b/wifi/1.5/default/tests/mock_wifi_iface_util.cpp
index fe6e9e2..b101c4a 100644
--- a/wifi/1.5/default/tests/mock_wifi_iface_util.cpp
+++ b/wifi/1.5/default/tests/mock_wifi_iface_util.cpp
@@ -29,8 +29,9 @@
namespace iface_util {
MockWifiIfaceUtil::MockWifiIfaceUtil(
- const std::weak_ptr<wifi_system::InterfaceTool> iface_tool)
- : WifiIfaceUtil(iface_tool) {}
+ const std::weak_ptr<wifi_system::InterfaceTool> iface_tool,
+ const std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal)
+ : WifiIfaceUtil(iface_tool, legacy_hal) {}
} // namespace iface_util
} // namespace implementation
} // namespace V1_5
diff --git a/wifi/1.5/default/tests/mock_wifi_iface_util.h b/wifi/1.5/default/tests/mock_wifi_iface_util.h
index a719fec..6d5f59c 100644
--- a/wifi/1.5/default/tests/mock_wifi_iface_util.h
+++ b/wifi/1.5/default/tests/mock_wifi_iface_util.h
@@ -31,7 +31,8 @@
class MockWifiIfaceUtil : public WifiIfaceUtil {
public:
MockWifiIfaceUtil(
- const std::weak_ptr<wifi_system::InterfaceTool> iface_tool);
+ const std::weak_ptr<wifi_system::InterfaceTool> iface_tool,
+ const std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal);
MOCK_METHOD1(getFactoryMacAddress,
std::array<uint8_t, 6>(const std::string&));
MOCK_METHOD2(setMacAddress,
diff --git a/wifi/1.5/default/tests/mock_wifi_legacy_hal.h b/wifi/1.5/default/tests/mock_wifi_legacy_hal.h
index 9ab2fd5..826b35f 100644
--- a/wifi/1.5/default/tests/mock_wifi_legacy_hal.h
+++ b/wifi/1.5/default/tests/mock_wifi_legacy_hal.h
@@ -62,6 +62,7 @@
wifi_error(const std::string& ifname,
wifi_interface_type iftype));
MOCK_METHOD1(deleteVirtualInterface, wifi_error(const std::string& ifname));
+ MOCK_METHOD0(waitForDriverReady, wifi_error());
};
} // namespace legacy_hal
} // namespace implementation
diff --git a/wifi/1.5/default/tests/wifi_chip_unit_tests.cpp b/wifi/1.5/default/tests/wifi_chip_unit_tests.cpp
index d99bfbd..0ad6f3e 100644
--- a/wifi/1.5/default/tests/wifi_chip_unit_tests.cpp
+++ b/wifi/1.5/default/tests/wifi_chip_unit_tests.cpp
@@ -276,7 +276,7 @@
std::shared_ptr<NiceMock<mode_controller::MockWifiModeController>>
mode_controller_{new NiceMock<mode_controller::MockWifiModeController>};
std::shared_ptr<NiceMock<iface_util::MockWifiIfaceUtil>> iface_util_{
- new NiceMock<iface_util::MockWifiIfaceUtil>(iface_tool_)};
+ new NiceMock<iface_util::MockWifiIfaceUtil>(iface_tool_, legacy_hal_)};
std::shared_ptr<NiceMock<feature_flags::MockWifiFeatureFlags>>
feature_flags_{new NiceMock<feature_flags::MockWifiFeatureFlags>};
diff --git a/wifi/1.5/default/tests/wifi_iface_util_unit_tests.cpp b/wifi/1.5/default/tests/wifi_iface_util_unit_tests.cpp
index d70e42f..8b67bb8 100644
--- a/wifi/1.5/default/tests/wifi_iface_util_unit_tests.cpp
+++ b/wifi/1.5/default/tests/wifi_iface_util_unit_tests.cpp
@@ -22,6 +22,7 @@
#include "wifi_iface_util.h"
#include "mock_interface_tool.h"
+#include "mock_wifi_legacy_hal.h"
using testing::NiceMock;
using testing::Test;
@@ -48,7 +49,11 @@
protected:
std::shared_ptr<NiceMock<wifi_system::MockInterfaceTool>> iface_tool_{
new NiceMock<wifi_system::MockInterfaceTool>};
- WifiIfaceUtil* iface_util_ = new WifiIfaceUtil(iface_tool_);
+ legacy_hal::wifi_hal_fn fake_func_table_;
+ std::shared_ptr<NiceMock<legacy_hal::MockWifiLegacyHal>> legacy_hal_{
+ new NiceMock<legacy_hal::MockWifiLegacyHal>(iface_tool_,
+ fake_func_table_, true)};
+ WifiIfaceUtil* iface_util_ = new WifiIfaceUtil(iface_tool_, legacy_hal_);
};
TEST_F(WifiIfaceUtilTest, GetOrCreateRandomMacAddress) {
diff --git a/wifi/1.5/default/tests/wifi_nan_iface_unit_tests.cpp b/wifi/1.5/default/tests/wifi_nan_iface_unit_tests.cpp
index 52f0c2b..32da55e 100644
--- a/wifi/1.5/default/tests/wifi_nan_iface_unit_tests.cpp
+++ b/wifi/1.5/default/tests/wifi_nan_iface_unit_tests.cpp
@@ -122,7 +122,7 @@
new NiceMock<legacy_hal::MockWifiLegacyHal>(iface_tool_,
fake_func_table_, true)};
std::shared_ptr<NiceMock<iface_util::MockWifiIfaceUtil>> iface_util_{
- new NiceMock<iface_util::MockWifiIfaceUtil>(iface_tool_)};
+ new NiceMock<iface_util::MockWifiIfaceUtil>(iface_tool_, legacy_hal_)};
};
TEST_F(WifiNanIfaceTest, IfacEventHandlers_OnStateToggleOffOn) {
diff --git a/wifi/1.5/default/wifi.cpp b/wifi/1.5/default/wifi.cpp
index 17db51d..da98db8 100644
--- a/wifi/1.5/default/wifi.cpp
+++ b/wifi/1.5/default/wifi.cpp
@@ -37,12 +37,10 @@
const std::shared_ptr<wifi_system::InterfaceTool> iface_tool,
const std::shared_ptr<legacy_hal::WifiLegacyHalFactory> legacy_hal_factory,
const std::shared_ptr<mode_controller::WifiModeController> mode_controller,
- const std::shared_ptr<iface_util::WifiIfaceUtil> iface_util,
const std::shared_ptr<feature_flags::WifiFeatureFlags> feature_flags)
: iface_tool_(iface_tool),
legacy_hal_factory_(legacy_hal_factory),
mode_controller_(mode_controller),
- iface_util_(iface_util),
feature_flags_(feature_flags),
run_state_(RunState::STOPPED) {}
@@ -130,7 +128,8 @@
for (auto& hal : legacy_hals_) {
chips_.push_back(new WifiChip(
chipId, chipId == kPrimaryChipId, hal, mode_controller_,
- iface_util_, feature_flags_, on_subsystem_restart_callback));
+ std::make_shared<iface_util::WifiIfaceUtil>(iface_tool_, hal),
+ feature_flags_, on_subsystem_restart_callback));
chipId++;
}
run_state_ = RunState::STARTED;
diff --git a/wifi/1.5/default/wifi.h b/wifi/1.5/default/wifi.h
index 9f5a1b0..825c0bc 100644
--- a/wifi/1.5/default/wifi.h
+++ b/wifi/1.5/default/wifi.h
@@ -46,7 +46,6 @@
legacy_hal_factory,
const std::shared_ptr<mode_controller::WifiModeController>
mode_controller,
- const std::shared_ptr<iface_util::WifiIfaceUtil> iface_util,
const std::shared_ptr<feature_flags::WifiFeatureFlags> feature_flags);
bool isValid();
@@ -85,7 +84,6 @@
std::shared_ptr<legacy_hal::WifiLegacyHalFactory> legacy_hal_factory_;
std::shared_ptr<mode_controller::WifiModeController> mode_controller_;
std::vector<std::shared_ptr<legacy_hal::WifiLegacyHal>> legacy_hals_;
- std::shared_ptr<iface_util::WifiIfaceUtil> iface_util_;
std::shared_ptr<feature_flags::WifiFeatureFlags> feature_flags_;
RunState run_state_;
std::vector<sp<WifiChip>> chips_;
diff --git a/wifi/1.5/default/wifi_chip.cpp b/wifi/1.5/default/wifi_chip.cpp
index 0450a7b..0499f45 100644
--- a/wifi/1.5/default/wifi_chip.cpp
+++ b/wifi/1.5/default/wifi_chip.cpp
@@ -353,7 +353,7 @@
ChipId chip_id, bool is_primary,
const std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal,
const std::weak_ptr<mode_controller::WifiModeController> mode_controller,
- const std::weak_ptr<iface_util::WifiIfaceUtil> iface_util,
+ const std::shared_ptr<iface_util::WifiIfaceUtil> iface_util,
const std::weak_ptr<feature_flags::WifiFeatureFlags> feature_flags,
const std::function<void(const std::string&)>& handler)
: chip_id_(chip_id),
@@ -986,14 +986,14 @@
}
}
br_ifaces_ap_instances_[br_ifname] = ap_instances;
- if (!iface_util_.lock()->createBridge(br_ifname)) {
+ if (!iface_util_->createBridge(br_ifname)) {
LOG(ERROR) << "Failed createBridge - br_name=" << br_ifname.c_str();
invalidateAndClearBridgedAp(br_ifname);
return {createWifiStatus(WifiStatusCode::ERROR_NOT_AVAILABLE), {}};
}
for (auto const& instance : ap_instances) {
// Bind ap instance interface to AP bridge
- if (!iface_util_.lock()->addIfaceToBridge(br_ifname, instance)) {
+ if (!iface_util_->addIfaceToBridge(br_ifname, instance)) {
LOG(ERROR) << "Failed add if to Bridge - if_name="
<< instance.c_str();
invalidateAndClearBridgedAp(br_ifname);
@@ -1054,8 +1054,7 @@
if (it.first == ifname) {
for (auto const& iface : it.second) {
if (iface == ifInstanceName) {
- if (!iface_util_.lock()->removeIfaceFromBridge(it.first,
- iface)) {
+ if (!iface_util_->removeIfaceFromBridge(it.first, iface)) {
LOG(ERROR)
<< "Failed to remove interface: " << ifInstanceName
<< " from " << ifname;
@@ -1086,7 +1085,7 @@
}
bool is_dedicated_iface = true;
std::string ifname = getPredefinedNanIfaceName();
- if (ifname.empty() || !iface_util_.lock()->ifNameToIndex(ifname)) {
+ if (ifname.empty() || !iface_util_->ifNameToIndex(ifname)) {
// Use the first shared STA iface (wlan0) if a dedicated aware iface is
// not defined.
ifname = getFirstActiveWlanIfaceName();
@@ -1968,10 +1967,10 @@
void WifiChip::invalidateAndClearBridgedApAll() {
for (auto const& it : br_ifaces_ap_instances_) {
for (auto const& iface : it.second) {
- iface_util_.lock()->removeIfaceFromBridge(it.first, iface);
+ iface_util_->removeIfaceFromBridge(it.first, iface);
legacy_hal_.lock()->deleteVirtualInterface(iface);
}
- iface_util_.lock()->deleteBridge(it.first);
+ iface_util_->deleteBridge(it.first);
}
br_ifaces_ap_instances_.clear();
}
@@ -1982,10 +1981,10 @@
for (auto const& it : br_ifaces_ap_instances_) {
if (it.first == br_name) {
for (auto const& iface : it.second) {
- iface_util_.lock()->removeIfaceFromBridge(br_name, iface);
+ iface_util_->removeIfaceFromBridge(br_name, iface);
legacy_hal_.lock()->deleteVirtualInterface(iface);
}
- iface_util_.lock()->deleteBridge(br_name);
+ iface_util_->deleteBridge(br_name);
br_ifaces_ap_instances_.erase(br_name);
break;
}
diff --git a/wifi/1.5/default/wifi_chip.h b/wifi/1.5/default/wifi_chip.h
index b4ed30e..92d639f 100644
--- a/wifi/1.5/default/wifi_chip.h
+++ b/wifi/1.5/default/wifi_chip.h
@@ -54,7 +54,7 @@
const std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal,
const std::weak_ptr<mode_controller::WifiModeController>
mode_controller,
- const std::weak_ptr<iface_util::WifiIfaceUtil> iface_util,
+ const std::shared_ptr<iface_util::WifiIfaceUtil> iface_util,
const std::weak_ptr<feature_flags::WifiFeatureFlags> feature_flags,
const std::function<void(const std::string&)>&
subsystemCallbackHandler);
@@ -307,7 +307,7 @@
ChipId chip_id_;
std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal_;
std::weak_ptr<mode_controller::WifiModeController> mode_controller_;
- std::weak_ptr<iface_util::WifiIfaceUtil> iface_util_;
+ std::shared_ptr<iface_util::WifiIfaceUtil> iface_util_;
std::vector<sp<WifiApIface>> ap_ifaces_;
std::vector<sp<WifiNanIface>> nan_ifaces_;
std::vector<sp<WifiP2pIface>> p2p_ifaces_;
diff --git a/wifi/1.5/default/wifi_iface_util.cpp b/wifi/1.5/default/wifi_iface_util.cpp
index 2a0aef8..d1434e3 100644
--- a/wifi/1.5/default/wifi_iface_util.cpp
+++ b/wifi/1.5/default/wifi_iface_util.cpp
@@ -41,8 +41,10 @@
namespace iface_util {
WifiIfaceUtil::WifiIfaceUtil(
- const std::weak_ptr<wifi_system::InterfaceTool> iface_tool)
+ const std::weak_ptr<wifi_system::InterfaceTool> iface_tool,
+ const std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal)
: iface_tool_(iface_tool),
+ legacy_hal_(legacy_hal),
random_mac_address_(nullptr),
event_handlers_map_() {}
@@ -59,14 +61,20 @@
return false;
}
#endif
- if (!iface_tool_.lock()->SetMacAddress(iface_name.c_str(), mac)) {
- LOG(ERROR) << "SetMacAddress failed.";
- return false;
- }
+ bool success = iface_tool_.lock()->SetMacAddress(iface_name.c_str(), mac);
#ifndef WIFI_AVOID_IFACE_RESET_MAC_CHANGE
if (!iface_tool_.lock()->SetUpState(iface_name.c_str(), true)) {
- LOG(ERROR) << "SetUpState(true) failed.";
- return false;
+ LOG(ERROR) << "SetUpState(true) failed. Wait for driver ready.";
+ // Wait for driver ready and try to set iface UP again
+ if (legacy_hal_.lock()->waitForDriverReady() !=
+ legacy_hal::WIFI_SUCCESS) {
+ LOG(ERROR) << "SetUpState(true) wait for driver ready failed.";
+ return false;
+ }
+ if (!iface_tool_.lock()->SetUpState(iface_name.c_str(), true)) {
+ LOG(ERROR) << "SetUpState(true) failed after retry.";
+ return false;
+ }
}
#endif
IfaceEventHandlers event_handlers = {};
@@ -77,8 +85,12 @@
if (event_handlers.on_state_toggle_off_on != nullptr) {
event_handlers.on_state_toggle_off_on(iface_name);
}
- LOG(DEBUG) << "Successfully SetMacAddress.";
- return true;
+ if (!success) {
+ LOG(ERROR) << "SetMacAddress failed.";
+ } else {
+ LOG(DEBUG) << "SetMacAddress succeeded.";
+ }
+ return success;
}
std::array<uint8_t, 6> WifiIfaceUtil::getOrCreateRandomMacAddress() {
diff --git a/wifi/1.5/default/wifi_iface_util.h b/wifi/1.5/default/wifi_iface_util.h
index 296d182..b449077 100644
--- a/wifi/1.5/default/wifi_iface_util.h
+++ b/wifi/1.5/default/wifi_iface_util.h
@@ -21,6 +21,8 @@
#include <android/hardware/wifi/1.0/IWifi.h>
+#include "wifi_legacy_hal.h"
+
namespace android {
namespace hardware {
namespace wifi {
@@ -40,7 +42,8 @@
*/
class WifiIfaceUtil {
public:
- WifiIfaceUtil(const std::weak_ptr<wifi_system::InterfaceTool> iface_tool);
+ WifiIfaceUtil(const std::weak_ptr<wifi_system::InterfaceTool> iface_tool,
+ const std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal);
virtual ~WifiIfaceUtil() = default;
virtual std::array<uint8_t, 6> getFactoryMacAddress(
@@ -73,6 +76,7 @@
std::array<uint8_t, 6> createRandomMacAddress();
std::weak_ptr<wifi_system::InterfaceTool> iface_tool_;
+ std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal_;
std::unique_ptr<std::array<uint8_t, 6>> random_mac_address_;
std::map<std::string, IfaceEventHandlers> event_handlers_map_;
};
diff --git a/wifi/1.5/default/wifi_legacy_hal.cpp b/wifi/1.5/default/wifi_legacy_hal.cpp
index f5ca753..260f186 100644
--- a/wifi/1.5/default/wifi_legacy_hal.cpp
+++ b/wifi/1.5/default/wifi_legacy_hal.cpp
@@ -476,6 +476,10 @@
bool WifiLegacyHal::isStarted() { return is_started_; }
+wifi_error WifiLegacyHal::waitForDriverReady() {
+ return global_func_table_.wifi_wait_for_driver_ready();
+}
+
std::pair<wifi_error, std::string> WifiLegacyHal::getDriverVersion(
const std::string& iface_name) {
std::array<char, kMaxVersionStringLength> buffer;
diff --git a/wifi/1.5/default/wifi_legacy_hal.h b/wifi/1.5/default/wifi_legacy_hal.h
index 03ca841..2372535 100644
--- a/wifi/1.5/default/wifi_legacy_hal.h
+++ b/wifi/1.5/default/wifi_legacy_hal.h
@@ -473,6 +473,7 @@
// using a predefined timeout.
virtual wifi_error stop(std::unique_lock<std::recursive_mutex>* lock,
const std::function<void()>& on_complete_callback);
+ virtual wifi_error waitForDriverReady();
// Checks if legacy HAL has successfully started
bool isStarted();
// Wrappers for all the functions in the legacy HAL function table.