From 8236850bc0cde5a870109b93421ad5d74eb79fd4 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Thu, 16 May 2019 12:53:02 -0700 Subject: wifi(implementation): Invalidate NAN iface on STA iface removal NAN iface/RTT controllers are sharing the STA iface. When a STA iface is removed, we should remove these dependent modules as well to ensure proper cleanup. Bug: 132837537 Test: ./hardware/interfaces/wifi/1.3/default/tests/runtests.sh Test: Will send for regression tests. Change-Id: Ia2da1dcf66b388f54e916ae69e2f4e26f20ecfad --- wifi/1.3/default/tests/wifi_chip_unit_tests.cpp | 67 +++++++++++++++++++++++++ wifi/1.3/default/wifi_chip.cpp | 28 +++++++++++ wifi/1.3/default/wifi_chip.h | 3 ++ wifi/1.3/default/wifi_rtt_controller.cpp | 2 + wifi/1.3/default/wifi_rtt_controller.h | 1 + 5 files changed, 101 insertions(+) (limited to 'wifi/1.3') diff --git a/wifi/1.3/default/tests/wifi_chip_unit_tests.cpp b/wifi/1.3/default/tests/wifi_chip_unit_tests.cpp index 7928328489..22359e956c 100644 --- a/wifi/1.3/default/tests/wifi_chip_unit_tests.cpp +++ b/wifi/1.3/default/tests/wifi_chip_unit_tests.cpp @@ -700,6 +700,72 @@ TEST_F(WifiChipV2_AwareIfaceCombinationTest, SelectTxScenarioWithOnlyAp) { }); } +TEST_F(WifiChipV2_AwareIfaceCombinationTest, + InvalidateAndRemoveNanOnStaRemove) { + findModeAndConfigureForIfaceType(IfaceType::STA); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); + + // Create NAN iface + ASSERT_EQ(createIface(IfaceType::NAN), "wlan0"); + + // We should have 1 nan iface. + chip_->getNanIfaceNames( + [](const WifiStatus& status, const hidl_vec& iface_names) { + ASSERT_EQ(WifiStatusCode::SUCCESS, status.code); + ASSERT_EQ(iface_names.size(), 1u); + ASSERT_EQ(iface_names[0], "wlan0"); + }); + // Retrieve the exact iface object. + sp nan_iface; + chip_->getNanIface("wlan0", [&nan_iface](const WifiStatus& status, + const sp& iface) { + ASSERT_EQ(WifiStatusCode::SUCCESS, status.code); + ASSERT_NE(iface.get(), nullptr); + nan_iface = iface; + }); + + // Remove the STA iface. + removeIface(IfaceType::STA, "wlan0"); + // We should have 0 nan iface now. + chip_->getNanIfaceNames( + [](const WifiStatus& status, const hidl_vec& iface_names) { + ASSERT_EQ(WifiStatusCode::SUCCESS, status.code); + ASSERT_EQ(iface_names.size(), 0u); + }); + // Any operation on the nan iface object should return error now. + nan_iface->getName( + [](const WifiStatus& status, const std::string& /* iface_name */) { + ASSERT_EQ(WifiStatusCode::ERROR_WIFI_IFACE_INVALID, status.code); + }); +} + +TEST_F(WifiChipV2_AwareIfaceCombinationTest, + InvalidateAndRemoveRttControllerOnStaRemove) { + findModeAndConfigureForIfaceType(IfaceType::STA); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); + + // Create RTT controller + sp rtt_controller; + chip_->createRttController( + NULL, [&rtt_controller](const WifiStatus& status, + const sp& rtt) { + if (WifiStatusCode::SUCCESS == status.code) { + ASSERT_NE(rtt.get(), nullptr); + rtt_controller = rtt; + } + }); + + // Remove the STA iface. + removeIface(IfaceType::STA, "wlan0"); + + // Any operation on the rtt controller object should return error now. + rtt_controller->getBoundIface( + [](const WifiStatus& status, const sp& /* iface */) { + ASSERT_EQ(WifiStatusCode::ERROR_WIFI_RTT_CONTROLLER_INVALID, + status.code); + }); +} + ////////// V1 Iface Combinations when AP creation is disabled ////////// class WifiChipV1_AwareDisabledApIfaceCombinationTest : public WifiChipTest { public: @@ -732,6 +798,7 @@ TEST_F(WifiChipV2_AwareDisabledApIfaceCombinationTest, ASSERT_TRUE(createIface(IfaceType::AP).empty()); } +////////// Hypothetical Iface Combination with multiple ifaces ////////// class WifiChip_MultiIfaceTest : public WifiChipTest { public: void SetUp() override { diff --git a/wifi/1.3/default/wifi_chip.cpp b/wifi/1.3/default/wifi_chip.cpp index b768959f2a..27320c407b 100644 --- a/wifi/1.3/default/wifi_chip.cpp +++ b/wifi/1.3/default/wifi_chip.cpp @@ -634,6 +634,27 @@ void WifiChip::invalidateAndRemoveAllIfaces() { rtt_controllers_.clear(); } +void WifiChip::invalidateAndRemoveDependencies( + const std::string& removed_iface_name) { + for (const auto& nan_iface : nan_ifaces_) { + if (nan_iface->getName() == removed_iface_name) { + invalidateAndClear(nan_ifaces_, nan_iface); + for (const auto& callback : event_cb_handler_.getCallbacks()) { + if (!callback + ->onIfaceRemoved(IfaceType::NAN, removed_iface_name) + .isOk()) { + LOG(ERROR) << "Failed to invoke onIfaceRemoved callback"; + } + } + } + } + for (const auto& rtt : rtt_controllers_) { + if (rtt->getIfaceName() == removed_iface_name) { + invalidateAndClear(rtt_controllers_, rtt); + } + } +} + std::pair WifiChip::getIdInternal() { return {createWifiStatus(WifiStatusCode::SUCCESS), chip_id_}; } @@ -819,6 +840,11 @@ WifiStatus WifiChip::removeApIfaceInternal(const std::string& ifname) { if (!iface.get()) { return createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS); } + // Invalidate & remove any dependent objects first. + // Note: This is probably not required because we never create + // nan/rtt objects over AP iface. But, there is no harm to do it + // here and not make that assumption all over the place. + invalidateAndRemoveDependencies(ifname); invalidateAndClear(ap_ifaces_, iface); for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceRemoved(IfaceType::AP, ifname).isOk()) { @@ -960,6 +986,8 @@ WifiStatus WifiChip::removeStaIfaceInternal(const std::string& ifname) { if (!iface.get()) { return createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS); } + // Invalidate & remove any dependent objects first. + invalidateAndRemoveDependencies(ifname); invalidateAndClear(sta_ifaces_, iface); for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceRemoved(IfaceType::STA, ifname).isOk()) { diff --git a/wifi/1.3/default/wifi_chip.h b/wifi/1.3/default/wifi_chip.h index 3d12f4c9b7..d5ae900247 100644 --- a/wifi/1.3/default/wifi_chip.h +++ b/wifi/1.3/default/wifi_chip.h @@ -155,6 +155,9 @@ class WifiChip : public V1_3::IWifiChip { private: void invalidateAndRemoveAllIfaces(); + // When a STA iface is removed any dependent NAN-ifaces/RTT-controllers are + // invalidated & removed. + void invalidateAndRemoveDependencies(const std::string& removed_iface_name); // Corresponding worker functions for the HIDL methods. std::pair getIdInternal(); diff --git a/wifi/1.3/default/wifi_rtt_controller.cpp b/wifi/1.3/default/wifi_rtt_controller.cpp index fa317e3aeb..3dcbee687b 100644 --- a/wifi/1.3/default/wifi_rtt_controller.cpp +++ b/wifi/1.3/default/wifi_rtt_controller.cpp @@ -49,6 +49,8 @@ WifiRttController::getEventCallbacks() { return event_callbacks_; } +std::string WifiRttController::getIfaceName() { return ifname_; } + Return WifiRttController::getBoundIface(getBoundIface_cb hidl_status_cb) { return validateAndCall( this, WifiStatusCode::ERROR_WIFI_RTT_CONTROLLER_INVALID, diff --git a/wifi/1.3/default/wifi_rtt_controller.h b/wifi/1.3/default/wifi_rtt_controller.h index 9798b79f8d..eedd22aeef 100644 --- a/wifi/1.3/default/wifi_rtt_controller.h +++ b/wifi/1.3/default/wifi_rtt_controller.h @@ -42,6 +42,7 @@ class WifiRttController : public V1_0::IWifiRttController { void invalidate(); bool isValid(); std::vector> getEventCallbacks(); + std::string getIfaceName(); // HIDL methods exposed. Return getBoundIface(getBoundIface_cb hidl_status_cb) override; -- cgit v1.2.3