From a3e5b7fce7206ac2e84a74516ed8baeeae6f8451 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Mon, 25 Mar 2019 13:52:45 -0700 Subject: [PATCH] wifi(implementation): Pin primary STA iface to wlan0 The primary STA iface will always be pinned to wlan0. The primary AP iface will be pinned to wlan0 for devices not supporting STA + AP concurrency & wlan1 for devices supporting STA + AP concurrency. All secondary STA or AP ifaces will be allocated on a first come first service basis (the current logic). Also, refactored/renamed some of the iface combo selection logic methods to help check whether concurrency is allowed in the current mode. Bug: 128946563 Test: ./data/android.hardware.wifi@1.0-service-tests Test: Will send for full regression tests. Test: On crosshatch, ensured that STA always comes up on wlan0 & AP comes up on wlan1 regardless of the sequence of toggle followed. Change-Id: Idca8de42ce819240bf0fac2a9039d15ed4bcaf90 --- .../default/tests/wifi_chip_unit_tests.cpp | 47 +++++-- wifi/1.3/default/wifi_chip.cpp | 117 +++++++++++++++--- wifi/1.3/default/wifi_chip.h | 19 ++- 3 files changed, 149 insertions(+), 34 deletions(-) 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 134563c7ea..7df9705314 100644 --- a/wifi/1.3/default/tests/wifi_chip_unit_tests.cpp +++ b/wifi/1.3/default/tests/wifi_chip_unit_tests.cpp @@ -122,7 +122,7 @@ class WifiChipTest : public Test { void setup_MultiIfaceCombination() { // clang-format off const hidl_vec combinations = { - {{{{IfaceType::STA}, 3}}} + {{{{IfaceType::STA}, 3}, {{IfaceType::AP}, 1}}} }; const std::vector modes = { {feature_flags::chip_mode_ids::kV3, combinations} @@ -272,6 +272,13 @@ class WifiChipTest : public Test { .WillRepeatedly(testing::Return(legacy_hal::WIFI_SUCCESS)); } + void TearDown() override { + // Restore default system iface names (This should ideally be using a + // mock). + property_set("wifi.interface", "wlan0"); + property_set("wifi.concurrent.interface", "wlan1"); + } + private: sp chip_; ChipId chip_id_ = kFakeChipId; @@ -300,7 +307,7 @@ class WifiChipV1IfaceCombinationTest : public WifiChipTest { TEST_F(WifiChipV1IfaceCombinationTest, StaMode_CreateSta_ShouldSucceed) { findModeAndConfigureForIfaceType(IfaceType::STA); - ASSERT_FALSE(createIface(IfaceType::STA).empty()); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); } TEST_F(WifiChipV1IfaceCombinationTest, StaMode_CreateP2p_ShouldSucceed) { @@ -326,7 +333,7 @@ TEST_F(WifiChipV1IfaceCombinationTest, StaMode_CreateStaP2p_ShouldSucceed) { TEST_F(WifiChipV1IfaceCombinationTest, ApMode_CreateAp_ShouldSucceed) { findModeAndConfigureForIfaceType(IfaceType::AP); - ASSERT_FALSE(createIface(IfaceType::AP).empty()); + ASSERT_EQ(createIface(IfaceType::AP), "wlan0"); } TEST_F(WifiChipV1IfaceCombinationTest, ApMode_CreateSta_ShouldFail) { @@ -359,7 +366,7 @@ class WifiChipV1_AwareIfaceCombinationTest : public WifiChipTest { TEST_F(WifiChipV1_AwareIfaceCombinationTest, StaMode_CreateSta_ShouldSucceed) { findModeAndConfigureForIfaceType(IfaceType::STA); - ASSERT_FALSE(createIface(IfaceType::STA).empty()); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); } TEST_F(WifiChipV1_AwareIfaceCombinationTest, StaMode_CreateP2p_ShouldSucceed) { @@ -427,7 +434,7 @@ TEST_F(WifiChipV1_AwareIfaceCombinationTest, TEST_F(WifiChipV1_AwareIfaceCombinationTest, ApMode_CreateAp_ShouldSucceed) { findModeAndConfigureForIfaceType(IfaceType::AP); - ASSERT_FALSE(createIface(IfaceType::AP).empty()); + ASSERT_EQ(createIface(IfaceType::AP), "wlan0"); } TEST_F(WifiChipV1_AwareIfaceCombinationTest, ApMode_CreateSta_ShouldFail) { @@ -483,7 +490,7 @@ class WifiChipV2_AwareIfaceCombinationTest : public WifiChipTest { TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateSta_ShouldSucceed) { findModeAndConfigureForIfaceType(IfaceType::STA); - ASSERT_FALSE(createIface(IfaceType::STA).empty()); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); } TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateP2p_ShouldSucceed) { @@ -498,19 +505,25 @@ TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateNan_ShouldSucceed) { TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateAp_ShouldSucceed) { findModeAndConfigureForIfaceType(IfaceType::STA); - ASSERT_FALSE(createIface(IfaceType::AP).empty()); + ASSERT_EQ(createIface(IfaceType::AP), "wlan1"); } TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateStaSta_ShouldFail) { findModeAndConfigureForIfaceType(IfaceType::AP); - ASSERT_FALSE(createIface(IfaceType::STA).empty()); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); ASSERT_TRUE(createIface(IfaceType::STA).empty()); } TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateStaAp_ShouldSucceed) { findModeAndConfigureForIfaceType(IfaceType::AP); - ASSERT_FALSE(createIface(IfaceType::AP).empty()); - ASSERT_FALSE(createIface(IfaceType::STA).empty()); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); + ASSERT_EQ(createIface(IfaceType::AP), "wlan1"); +} + +TEST_F(WifiChipV2_AwareIfaceCombinationTest, CreateApSta_ShouldSucceed) { + findModeAndConfigureForIfaceType(IfaceType::AP); + ASSERT_EQ(createIface(IfaceType::AP), "wlan1"); + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); } TEST_F(WifiChipV2_AwareIfaceCombinationTest, @@ -707,8 +720,8 @@ TEST_F(WifiChip_MultiIfaceTest, CreateStaWithCustomNames) { property_set("wifi.interface", "bad0"); property_set("wifi.concurrent.interface", "bad1"); findModeAndConfigureForIfaceType(IfaceType::STA); - ASSERT_EQ(createIface(IfaceType::STA), "test0"); - ASSERT_EQ(createIface(IfaceType::STA), "test1"); + ASSERT_EQ(createIface(IfaceType::STA), "bad0"); + ASSERT_EQ(createIface(IfaceType::STA), "bad1"); ASSERT_EQ(createIface(IfaceType::STA), "test2"); } @@ -724,6 +737,16 @@ TEST_F(WifiChip_MultiIfaceTest, CreateStaWithCustomAltNames) { ASSERT_EQ(createIface(IfaceType::STA), "wlan2"); } +TEST_F(WifiChip_MultiIfaceTest, CreateApStartsWithIdx1) { + findModeAndConfigureForIfaceType(IfaceType::STA); + // First AP will be slotted to wlan1. + ASSERT_EQ(createIface(IfaceType::AP), "wlan1"); + // First STA will be slotted to wlan0. + ASSERT_EQ(createIface(IfaceType::STA), "wlan0"); + // All further STA will be slotted to the remaining free indices. + ASSERT_EQ(createIface(IfaceType::STA), "wlan2"); + ASSERT_EQ(createIface(IfaceType::STA), "wlan3"); +} } // namespace implementation } // namespace V1_3 } // namespace wifi diff --git a/wifi/1.3/default/wifi_chip.cpp b/wifi/1.3/default/wifi_chip.cpp index 3697d50b5c..ce5df174a2 100644 --- a/wifi/1.3/default/wifi_chip.cpp +++ b/wifi/1.3/default/wifi_chip.cpp @@ -766,10 +766,10 @@ WifiChip::requestFirmwareDebugDumpInternal() { } std::pair> WifiChip::createApIfaceInternal() { - if (!canCurrentModeSupportIfaceOfType(IfaceType::AP)) { + if (!canCurrentModeSupportIfaceOfTypeWithCurrentIfaces(IfaceType::AP)) { return {createWifiStatus(WifiStatusCode::ERROR_NOT_AVAILABLE), {}}; } - std::string ifname = allocateApOrStaIfaceName(); + std::string ifname = allocateApIfaceName(); sp iface = new WifiApIface(ifname, legacy_hal_, iface_util_, feature_flags_); ap_ifaces_.push_back(iface); @@ -813,7 +813,7 @@ WifiStatus WifiChip::removeApIfaceInternal(const std::string& ifname) { } std::pair> WifiChip::createNanIfaceInternal() { - if (!canCurrentModeSupportIfaceOfType(IfaceType::NAN)) { + if (!canCurrentModeSupportIfaceOfTypeWithCurrentIfaces(IfaceType::NAN)) { return {createWifiStatus(WifiStatusCode::ERROR_NOT_AVAILABLE), {}}; } // These are still assumed to be based on wlan0. @@ -860,7 +860,7 @@ WifiStatus WifiChip::removeNanIfaceInternal(const std::string& ifname) { } std::pair> WifiChip::createP2pIfaceInternal() { - if (!canCurrentModeSupportIfaceOfType(IfaceType::P2P)) { + if (!canCurrentModeSupportIfaceOfTypeWithCurrentIfaces(IfaceType::P2P)) { return {createWifiStatus(WifiStatusCode::ERROR_NOT_AVAILABLE), {}}; } std::string ifname = getP2pIfaceName(); @@ -906,10 +906,10 @@ WifiStatus WifiChip::removeP2pIfaceInternal(const std::string& ifname) { } std::pair> WifiChip::createStaIfaceInternal() { - if (!canCurrentModeSupportIfaceOfType(IfaceType::STA)) { + if (!canCurrentModeSupportIfaceOfTypeWithCurrentIfaces(IfaceType::STA)) { return {createWifiStatus(WifiStatusCode::ERROR_NOT_AVAILABLE), {}}; } - std::string ifname = allocateApOrStaIfaceName(); + std::string ifname = allocateStaIfaceName(); sp iface = new WifiStaIface(ifname, legacy_hal_, iface_util_); sta_ifaces_.push_back(iface); for (const auto& callback : event_cb_handler_.getCallbacks()) { @@ -1298,8 +1298,9 @@ std::vector> WifiChip::expandIfaceCombinations( return expanded_combos; } -bool WifiChip::canExpandedIfaceCombinationSupportIfaceOfType( - const std::map& combo, IfaceType requested_type) { +bool WifiChip::canExpandedIfaceComboSupportIfaceOfTypeWithCurrentIfaces( + const std::map& expanded_combo, + IfaceType requested_type) { const auto current_combo = getCurrentIfaceCombination(); // Check if we have space for 1 more iface of |type| in this combo @@ -1309,7 +1310,7 @@ bool WifiChip::canExpandedIfaceCombinationSupportIfaceOfType( if (type == requested_type) { num_ifaces_needed++; } - size_t num_ifaces_allowed = combo.at(type); + size_t num_ifaces_allowed = expanded_combo.at(type); if (num_ifaces_needed > num_ifaces_allowed) { return false; } @@ -1320,8 +1321,10 @@ bool WifiChip::canExpandedIfaceCombinationSupportIfaceOfType( // This method does the following: // a) Enumerate all possible iface combos by expanding the current // ChipIfaceCombination. -// b) Check if the requested iface type can be added to the current mode. -bool WifiChip::canCurrentModeSupportIfaceOfType(IfaceType type) { +// b) Check if the requested iface type can be added to the current mode +// with the iface combination that is already active. +bool WifiChip::canCurrentModeSupportIfaceOfTypeWithCurrentIfaces( + IfaceType requested_type) { if (!isValidModeId(current_mode_id_)) { LOG(ERROR) << "Chip not configured in a mode yet"; return false; @@ -1330,8 +1333,8 @@ bool WifiChip::canCurrentModeSupportIfaceOfType(IfaceType type) { for (const auto& combination : combinations) { const auto expanded_combos = expandIfaceCombinations(combination); for (const auto& expanded_combo : expanded_combos) { - if (canExpandedIfaceCombinationSupportIfaceOfType(expanded_combo, - type)) { + if (canExpandedIfaceComboSupportIfaceOfTypeWithCurrentIfaces( + expanded_combo, requested_type)) { return true; } } @@ -1339,6 +1342,62 @@ bool WifiChip::canCurrentModeSupportIfaceOfType(IfaceType type) { return false; } +// Note: This does not consider ifaces already active. It only checks if the +// provided expanded iface combination can support the requested combo. +bool WifiChip::canExpandedIfaceComboSupportIfaceCombo( + const std::map& expanded_combo, + const std::map& req_combo) { + // Check if we have space for 1 more iface of |type| in this combo + for (const auto type : + {IfaceType::AP, IfaceType::NAN, IfaceType::P2P, IfaceType::STA}) { + if (req_combo.count(type) == 0) { + // Iface of "type" not in the req_combo. + continue; + } + size_t num_ifaces_needed = req_combo.at(type); + size_t num_ifaces_allowed = expanded_combo.at(type); + if (num_ifaces_needed > num_ifaces_allowed) { + return false; + } + } + return true; +} +// This method does the following: +// a) Enumerate all possible iface combos by expanding the current +// ChipIfaceCombination. +// b) Check if the requested iface combo can be added to the current mode. +// Note: This does not consider ifaces already active. It only checks if the +// current mode can support the requested combo. +bool WifiChip::canCurrentModeSupportIfaceCombo( + const std::map& req_combo) { + if (!isValidModeId(current_mode_id_)) { + LOG(ERROR) << "Chip not configured in a mode yet"; + return false; + } + const auto combinations = getCurrentModeIfaceCombinations(); + for (const auto& combination : combinations) { + const auto expanded_combos = expandIfaceCombinations(combination); + for (const auto& expanded_combo : expanded_combos) { + if (canExpandedIfaceComboSupportIfaceCombo(expanded_combo, + req_combo)) { + return true; + } + } + } + return false; +} + +// This method does the following: +// a) Enumerate all possible iface combos by expanding the current +// ChipIfaceCombination. +// b) Check if the requested iface type can be added to the current mode. +bool WifiChip::canCurrentModeSupportIfaceOfType(IfaceType requested_type) { + // Check if we can support atleast 1 iface of type. + std::map req_iface_combo; + req_iface_combo[requested_type] = 1; + return canCurrentModeSupportIfaceCombo(req_iface_combo); +} + bool WifiChip::isValidModeId(ChipModeId mode_id) { for (const auto& mode : modes_) { if (mode.id == mode_id) { @@ -1348,11 +1407,20 @@ bool WifiChip::isValidModeId(ChipModeId mode_id) { return false; } -// Return the first wlan (wlan0, wlan1 etc.) not already in use. -// This doesn't check the actual presence of these interfaces. -std::string WifiChip::allocateApOrStaIfaceName() { - for (unsigned i = 0; i < kMaxWlanIfaces; i++) { - const auto ifname = getWlanIfaceName(i); +bool WifiChip::isStaApConcurrencyAllowedInCurrentMode() { + // Check if we can support atleast 1 STA & 1 AP concurrently. + std::map req_iface_combo; + req_iface_combo[IfaceType::AP] = 1; + req_iface_combo[IfaceType::STA] = 1; + return canCurrentModeSupportIfaceCombo(req_iface_combo); +} + +// Return the first wlan (wlan0, wlan1 etc.) starting from |start_idx| +// not already in use. +// Note: This doesn't check the actual presence of these interfaces. +std::string WifiChip::allocateApOrStaIfaceName(uint32_t start_idx) { + for (unsigned idx = start_idx; idx < kMaxWlanIfaces; idx++) { + const auto ifname = getWlanIfaceName(idx); if (findUsingName(ap_ifaces_, ifname)) continue; if (findUsingName(sta_ifaces_, ifname)) continue; return ifname; @@ -1362,6 +1430,19 @@ std::string WifiChip::allocateApOrStaIfaceName() { return {}; } +// AP iface names start with idx 1 for modes supporting +// concurrent STA, else start with idx 0. +std::string WifiChip::allocateApIfaceName() { + return allocateApOrStaIfaceName( + isStaApConcurrencyAllowedInCurrentMode() ? 1 : 0); +} + +// STA iface names start with idx 0. +// Primary STA iface will always be 0. +std::string WifiChip::allocateStaIfaceName() { + return allocateApOrStaIfaceName(0); +} + bool WifiChip::writeRingbufferFilesInternal() { if (!removeOldFilesInternal()) { LOG(ERROR) << "Error occurred while deleting old tombstone files"; diff --git a/wifi/1.3/default/wifi_chip.h b/wifi/1.3/default/wifi_chip.h index 3eb0aee471..27db9a487b 100644 --- a/wifi/1.3/default/wifi_chip.h +++ b/wifi/1.3/default/wifi_chip.h @@ -224,11 +224,22 @@ class WifiChip : public V1_3::IWifiChip { std::map getCurrentIfaceCombination(); std::vector> expandIfaceCombinations( const IWifiChip::ChipIfaceCombination& combination); - bool canExpandedIfaceCombinationSupportIfaceOfType( - const std::map& combo, IfaceType type); - bool canCurrentModeSupportIfaceOfType(IfaceType type); + bool canExpandedIfaceComboSupportIfaceOfTypeWithCurrentIfaces( + const std::map& expanded_combo, + IfaceType requested_type); + bool canCurrentModeSupportIfaceOfTypeWithCurrentIfaces( + IfaceType requested_type); + bool canExpandedIfaceComboSupportIfaceCombo( + const std::map& expanded_combo, + const std::map& req_combo); + bool canCurrentModeSupportIfaceCombo( + const std::map& req_combo); + bool canCurrentModeSupportIfaceOfType(IfaceType requested_type); bool isValidModeId(ChipModeId mode_id); - std::string allocateApOrStaIfaceName(); + bool isStaApConcurrencyAllowedInCurrentMode(); + std::string allocateApOrStaIfaceName(uint32_t start_idx); + std::string allocateApIfaceName(); + std::string allocateStaIfaceName(); bool writeRingbufferFilesInternal(); ChipId chip_id_;