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
This commit is contained in:
parent
0a04fe1367
commit
8c1a67b7af
15 changed files with 58 additions and 35 deletions
|
@ -32,7 +32,6 @@ using android::hardware::joinRpcThreadpool;
|
|||
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 @@ int main(int /*argc*/, char** argv) {
|
|||
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();
|
||||
|
|
|
@ -29,8 +29,9 @@ namespace implementation {
|
|||
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
|
||||
|
|
|
@ -31,7 +31,8 @@ namespace iface_util {
|
|||
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,
|
||||
|
|
|
@ -62,6 +62,7 @@ class MockWifiLegacyHal : public WifiLegacyHal {
|
|||
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
|
||||
|
|
|
@ -276,7 +276,7 @@ class WifiChipTest : public Test {
|
|||
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>};
|
||||
|
||||
|
|
|
@ -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 @@ class WifiIfaceUtilTest : public Test {
|
|||
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) {
|
||||
|
|
|
@ -122,7 +122,7 @@ class WifiNanIfaceTest : public Test {
|
|||
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) {
|
||||
|
|
|
@ -37,12 +37,10 @@ Wifi::Wifi(
|
|||
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 @@ WifiStatus Wifi::startInternal() {
|
|||
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;
|
||||
|
|
|
@ -46,7 +46,6 @@ class Wifi : public V1_5::IWifi {
|
|||
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 @@ class Wifi : public V1_5::IWifi {
|
|||
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_;
|
||||
|
|
|
@ -353,7 +353,7 @@ WifiChip::WifiChip(
|
|||
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 @@ WifiChip::createBridgedApIfaceInternal() {
|
|||
}
|
||||
}
|
||||
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 @@ WifiStatus WifiChip::removeIfaceInstanceFromBridgedApIfaceInternal(
|
|||
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 @@ WifiChip::createNanIfaceInternal() {
|
|||
}
|
||||
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 @@ std::string WifiChip::getWlanIfaceNameWithType(IfaceType type, unsigned idx) {
|
|||
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 @@ void WifiChip::invalidateAndClearBridgedAp(const std::string& br_name) {
|
|||
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;
|
||||
}
|
||||
|
|
|
@ -54,7 +54,7 @@ class WifiChip : public V1_5::IWifiChip {
|
|||
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 @@ class WifiChip : public V1_5::IWifiChip {
|
|||
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_;
|
||||
|
|
|
@ -41,8 +41,10 @@ namespace implementation {
|
|||
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 @@ bool WifiIfaceUtil::setMacAddress(const std::string& iface_name,
|
|||
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 @@ bool WifiIfaceUtil::setMacAddress(const std::string& iface_name,
|
|||
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() {
|
||||
|
|
|
@ -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 @@ struct IfaceEventHandlers {
|
|||
*/
|
||||
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 @@ class WifiIfaceUtil {
|
|||
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_;
|
||||
};
|
||||
|
|
|
@ -476,6 +476,10 @@ wifi_error WifiLegacyHal::stop(
|
|||
|
||||
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;
|
||||
|
|
|
@ -473,6 +473,7 @@ class WifiLegacyHal {
|
|||
// 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.
|
||||
|
|
Loading…
Reference in a new issue