From 5ba0a90cac5172b5b406971aaf3280afd06c3c03 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 14 Apr 2020 11:55:42 -0700 Subject: [PATCH] Use additional interface for the WiFi Aware Discovery operations. NAN operations are currently done on the STA interface.This resulted in the NAN sessions getting terminated in all the instances where the STA interface is restarted (IFF DOWN/UP), due to connected MAC randomization. Hence, remove the dependency of NAN over STA interface. This change expects the driver to create a dedicated interface for the NAN operations. The iface name needs to be set in "wifi.aware.interface" property. Bug: 1636219 Test: HAL unit tests Test: Device boots up and connects to wifi network. Change-Id: I1b6d64eb94334172a8cd621d0b15ed8c8dc87f91 --- wifi/1.4/default/Android.mk | 12 +++++----- wifi/1.4/default/tests/mock_wifi_iface_util.h | 1 + wifi/1.4/default/tests/mock_wifi_legacy_hal.h | 4 ++++ .../default/tests/wifi_chip_unit_tests.cpp | 23 +++++++++++++++++++ .../tests/wifi_nan_iface_unit_tests.cpp | 2 +- wifi/1.4/default/wifi_chip.cpp | 22 +++++++++++++++--- wifi/1.4/default/wifi_iface_util.cpp | 8 +++++++ wifi/1.4/default/wifi_iface_util.h | 1 + wifi/1.4/default/wifi_legacy_hal.h | 6 ++--- wifi/1.4/default/wifi_nan_iface.cpp | 15 +++++++++++- wifi/1.4/default/wifi_nan_iface.h | 3 ++- 11 files changed, 82 insertions(+), 15 deletions(-) diff --git a/wifi/1.4/default/Android.mk b/wifi/1.4/default/Android.mk index ab76ff66e2..8573e8e1f5 100644 --- a/wifi/1.4/default/Android.mk +++ b/wifi/1.4/default/Android.mk @@ -156,6 +156,11 @@ LOCAL_SRC_FILES := \ LOCAL_STATIC_LIBRARIES := \ libgmock \ libgtest \ + android.hardware.wifi@1.0 \ + android.hardware.wifi@1.1 \ + android.hardware.wifi@1.2 \ + android.hardware.wifi@1.3 \ + android.hardware.wifi@1.4 \ android.hardware.wifi@1.0-service-lib LOCAL_SHARED_LIBRARIES := \ libbase \ @@ -165,10 +170,5 @@ LOCAL_SHARED_LIBRARIES := \ libnl \ libutils \ libwifi-hal \ - libwifi-system-iface \ - android.hardware.wifi@1.0 \ - android.hardware.wifi@1.1 \ - android.hardware.wifi@1.2 \ - android.hardware.wifi@1.3 \ - android.hardware.wifi@1.4 + libwifi-system-iface include $(BUILD_NATIVE_TEST) diff --git a/wifi/1.4/default/tests/mock_wifi_iface_util.h b/wifi/1.4/default/tests/mock_wifi_iface_util.h index 6cc81e4083..3b36f13b37 100644 --- a/wifi/1.4/default/tests/mock_wifi_iface_util.h +++ b/wifi/1.4/default/tests/mock_wifi_iface_util.h @@ -40,6 +40,7 @@ class MockWifiIfaceUtil : public WifiIfaceUtil { MOCK_METHOD2(registerIfaceEventHandlers, void(const std::string&, IfaceEventHandlers)); MOCK_METHOD1(unregisterIfaceEventHandlers, void(const std::string&)); + MOCK_METHOD2(setUpState, bool(const std::string&, bool)); }; } // namespace iface_util } // namespace implementation diff --git a/wifi/1.4/default/tests/mock_wifi_legacy_hal.h b/wifi/1.4/default/tests/mock_wifi_legacy_hal.h index 6942c1e4e3..3bb7b54760 100644 --- a/wifi/1.4/default/tests/mock_wifi_legacy_hal.h +++ b/wifi/1.4/default/tests/mock_wifi_legacy_hal.h @@ -57,6 +57,10 @@ class MockWifiLegacyHal : public WifiLegacyHal { MOCK_METHOD3(nanDataInterfaceDelete, wifi_error(const std::string&, transaction_id, const std::string&)); + MOCK_METHOD2(createVirtualInterface, + wifi_error(const std::string& ifname, + wifi_interface_type iftype)); + MOCK_METHOD1(deleteVirtualInterface, wifi_error(const std::string& ifname)); }; } // namespace legacy_hal } // namespace implementation diff --git a/wifi/1.4/default/tests/wifi_chip_unit_tests.cpp b/wifi/1.4/default/tests/wifi_chip_unit_tests.cpp index d35adbc125..d5b1a50ca6 100644 --- a/wifi/1.4/default/tests/wifi_chip_unit_tests.cpp +++ b/wifi/1.4/default/tests/wifi_chip_unit_tests.cpp @@ -292,6 +292,7 @@ class WifiChipTest : public Test { // mock). property_set("wifi.interface", "wlan0"); property_set("wifi.concurrent.interface", "wlan1"); + property_set("wifi.aware.interface", nullptr); } }; @@ -773,6 +774,28 @@ TEST_F(WifiChipV2_AwareIfaceCombinationTest, }); } +TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateNanWithSharedNanIface) { + property_set("wifi.aware.interface", nullptr); + findModeAndConfigureForIfaceType(IfaceType::STA); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); + ASSERT_EQ(createIface(IfaceType::NAN), "wlan0"); + removeIface(IfaceType::NAN, "wlan0"); + EXPECT_CALL(*iface_util_, setUpState(testing::_, testing::_)).Times(0); +} + +TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateNanWithDedicatedNanIface) { + property_set("wifi.aware.interface", "aware0"); + findModeAndConfigureForIfaceType(IfaceType::STA); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); + EXPECT_CALL(*iface_util_, setUpState("aware0", true)) + .WillOnce(testing::Return(true)); + ASSERT_EQ(createIface(IfaceType::NAN), "aware0"); + + EXPECT_CALL(*iface_util_, setUpState("aware0", false)) + .WillOnce(testing::Return(true)); + removeIface(IfaceType::NAN, "aware0"); +} + ////////// V1 Iface Combinations when AP creation is disabled ////////// class WifiChipV1_AwareDisabledApIfaceCombinationTest : public WifiChipTest { public: diff --git a/wifi/1.4/default/tests/wifi_nan_iface_unit_tests.cpp b/wifi/1.4/default/tests/wifi_nan_iface_unit_tests.cpp index 90227925dd..70424db815 100644 --- a/wifi/1.4/default/tests/wifi_nan_iface_unit_tests.cpp +++ b/wifi/1.4/default/tests/wifi_nan_iface_unit_tests.cpp @@ -131,7 +131,7 @@ TEST_F(WifiNanIfaceTest, IfacEventHandlers_OnStateToggleOffOn) { bind(CaptureIfaceEventHandlers, std::placeholders::_1, std::placeholders::_2, &captured_iface_event_handlers))); sp nan_iface = - new WifiNanIface(kIfaceName, legacy_hal_, iface_util_); + new WifiNanIface(kIfaceName, false, legacy_hal_, iface_util_); // Register a mock nan event callback. sp> mock_event_callback{ diff --git a/wifi/1.4/default/wifi_chip.cpp b/wifi/1.4/default/wifi_chip.cpp index 4c9fad17bb..d64dfbfd60 100644 --- a/wifi/1.4/default/wifi_chip.cpp +++ b/wifi/1.4/default/wifi_chip.cpp @@ -107,6 +107,15 @@ std::string getP2pIfaceName() { return buffer.data(); } +// Returns the dedicated iface name if one is defined. +std::string getNanIfaceName() { + std::array buffer; + if (property_get("wifi.aware.interface", buffer.data(), nullptr) == 0) { + return {}; + } + return buffer.data(); +} + void setActiveWlanIfaceNameProperty(const std::string& ifname) { auto res = property_set(kActiveWlanIfaceNameProperty, ifname.data()); if (res != 0) { @@ -864,9 +873,16 @@ std::pair> WifiChip::createNanIfaceInternal() { if (!canCurrentModeSupportIfaceOfTypeWithCurrentIfaces(IfaceType::NAN)) { return {createWifiStatus(WifiStatusCode::ERROR_NOT_AVAILABLE), {}}; } - // These are still assumed to be based on wlan0. - std::string ifname = getFirstActiveWlanIfaceName(); - sp iface = new WifiNanIface(ifname, legacy_hal_, iface_util_); + bool is_dedicated_iface = true; + std::string ifname = getNanIfaceName(); + if (ifname.empty()) { + // Use the first shared STA iface (wlan0) if a dedicated aware iface is + // not defined. + ifname = getFirstActiveWlanIfaceName(); + is_dedicated_iface = false; + } + sp iface = + new WifiNanIface(ifname, is_dedicated_iface, legacy_hal_, iface_util_); nan_ifaces_.push_back(iface); for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceAdded(IfaceType::NAN, ifname).isOk()) { diff --git a/wifi/1.4/default/wifi_iface_util.cpp b/wifi/1.4/default/wifi_iface_util.cpp index 2883b4627b..036c97bcba 100644 --- a/wifi/1.4/default/wifi_iface_util.cpp +++ b/wifi/1.4/default/wifi_iface_util.cpp @@ -110,6 +110,14 @@ std::array WifiIfaceUtil::createRandomMacAddress() { address[0] &= ~kMacAddressMulticastMask; return address; } + +bool WifiIfaceUtil::setUpState(const std::string& iface_name, bool request_up) { + if (!iface_tool_.lock()->SetUpState(iface_name.c_str(), request_up)) { + LOG(ERROR) << "SetUpState to " << request_up << " failed"; + return false; + } + return true; +} } // namespace iface_util } // namespace implementation } // namespace V1_4 diff --git a/wifi/1.4/default/wifi_iface_util.h b/wifi/1.4/default/wifi_iface_util.h index 35edff68bd..f83d717bf6 100644 --- a/wifi/1.4/default/wifi_iface_util.h +++ b/wifi/1.4/default/wifi_iface_util.h @@ -56,6 +56,7 @@ class WifiIfaceUtil { virtual void registerIfaceEventHandlers(const std::string& iface_name, IfaceEventHandlers handlers); virtual void unregisterIfaceEventHandlers(const std::string& iface_name); + virtual bool setUpState(const std::string& iface_name, bool request_up); private: std::array createRandomMacAddress(); diff --git a/wifi/1.4/default/wifi_legacy_hal.h b/wifi/1.4/default/wifi_legacy_hal.h index c21563ee92..c697ff9682 100644 --- a/wifi/1.4/default/wifi_legacy_hal.h +++ b/wifi/1.4/default/wifi_legacy_hal.h @@ -373,9 +373,9 @@ class WifiLegacyHal { std::array code); // interface functions. - wifi_error createVirtualInterface(const std::string& ifname, - wifi_interface_type iftype); - wifi_error deleteVirtualInterface(const std::string& ifname); + virtual wifi_error createVirtualInterface(const std::string& ifname, + wifi_interface_type iftype); + virtual wifi_error deleteVirtualInterface(const std::string& ifname); private: // Retrieve interface handles for all the available interfaces. diff --git a/wifi/1.4/default/wifi_nan_iface.cpp b/wifi/1.4/default/wifi_nan_iface.cpp index 073101cbd5..5764d35ec4 100644 --- a/wifi/1.4/default/wifi_nan_iface.cpp +++ b/wifi/1.4/default/wifi_nan_iface.cpp @@ -29,13 +29,22 @@ namespace implementation { using hidl_return_util::validateAndCall; WifiNanIface::WifiNanIface( - const std::string& ifname, + const std::string& ifname, bool is_dedicated_iface, const std::weak_ptr legacy_hal, const std::weak_ptr iface_util) : ifname_(ifname), + is_dedicated_iface_(is_dedicated_iface), legacy_hal_(legacy_hal), iface_util_(iface_util), is_valid_(true) { + if (is_dedicated_iface_) { + // If using a dedicated iface, set the iface up first. + if (!iface_util_.lock()->setUpState(ifname_, true)) { + // Fatal failure, invalidate the iface object. + invalidate(); + return; + } + } // Register all the callbacks here. these should be valid for the lifetime // of the object. Whenever the mode changes legacy HAL will remove // all of these callbacks. @@ -534,6 +543,10 @@ void WifiNanIface::invalidate() { event_cb_handler_.invalidate(); event_cb_handler_1_2_.invalidate(); is_valid_ = false; + if (is_dedicated_iface_) { + // If using a dedicated iface, set the iface down. + iface_util_.lock()->setUpState(ifname_, false); + } } bool WifiNanIface::isValid() { return is_valid_; } diff --git a/wifi/1.4/default/wifi_nan_iface.h b/wifi/1.4/default/wifi_nan_iface.h index c16628bbf6..06edbf2609 100644 --- a/wifi/1.4/default/wifi_nan_iface.h +++ b/wifi/1.4/default/wifi_nan_iface.h @@ -38,7 +38,7 @@ using namespace android::hardware::wifi::V1_2; */ class WifiNanIface : public V1_4::IWifiNanIface { public: - WifiNanIface(const std::string& ifname, + WifiNanIface(const std::string& ifname, bool is_dedicated_iface, const std::weak_ptr legacy_hal, const std::weak_ptr iface_util); // Refer to |WifiChip::invalidate()|. @@ -165,6 +165,7 @@ class WifiNanIface : public V1_4::IWifiNanIface { std::set> getEventCallbacks_1_2(); std::string ifname_; + bool is_dedicated_iface_; std::weak_ptr legacy_hal_; std::weak_ptr iface_util_; bool is_valid_;