diff options
author | Nathan Harold <nharold@google.com> | 2018-04-19 13:10:19 -0700 |
---|---|---|
committer | Nathan Harold <nharold@google.com> | 2018-04-23 12:58:37 -0700 |
commit | dfe2a6f43de4aba2780c861c750db8e4f1fb22e3 (patch) | |
tree | a9d2a7582d825cd6a4bf1e20a51ea245587c14c3 | |
parent | 601c3a04a576b7c84677fa2a68197d0bd3bd6f99 (diff) | |
download | netd-dfe2a6f43de4aba2780c861c750db8e4f1fb22e3.tar.gz |
Factor getIfaceNames() from getIfaceList()
getIfaceList first walks the list of interfaces
from the sysfs, then it calls individually for
each of those interfaces to get the ifindex for
them. Because each of the calls to retrieve the
ifindex means a netlink call, this could possibly
cause performance problems (unconfirmed) on the
netlink interface. Since the names are independently
useful and are quick to fetch, this function is
now factored in to 2 parts: one which fetches the
names and a separate function which performs the
original operation of fetching the names and mapping
them to if_indices.
Bug: 74560705
Test: netd_integration_test - GetIfaceListTest
Change-Id: I1f888c31e992c8f7d51f3c67434ffef6d75b065d
-rw-r--r-- | server/InterfaceController.cpp | 24 | ||||
-rw-r--r-- | server/InterfaceController.h | 1 | ||||
-rw-r--r-- | server/InterfaceControllerTest.cpp | 13 |
3 files changed, 32 insertions, 6 deletions
diff --git a/server/InterfaceController.cpp b/server/InterfaceController.cpp index 5a630255..c0210d7b 100644 --- a/server/InterfaceController.cpp +++ b/server/InterfaceController.cpp @@ -43,6 +43,7 @@ using android::base::StringPrintf; using android::base::WriteStringToFile; using android::net::INetd; using android::net::RouteController; +using android::netdutils::isOk; using android::netdutils::Status; using android::netdutils::StatusOr; using android::netdutils::makeSlice; @@ -388,8 +389,8 @@ void InterfaceController::setIPv6OptimisticMode(const char *value) { setOnAllInterfaces(ipv6_proc_path, "use_optimistic", value); } -StatusOr<std::map<std::string, uint32_t>> InterfaceController::getIfaceList() { - std::map<std::string, uint32_t> ifacePairs; +StatusOr<std::vector<std::string>> InterfaceController::getIfaceNames() { + std::vector<std::string> ifaceNames; DIR* d; struct dirent* de; @@ -398,11 +399,22 @@ StatusOr<std::map<std::string, uint32_t>> InterfaceController::getIfaceList() { } while ((de = readdir(d))) { if (de->d_name[0] == '.') continue; - uint32_t ifaceIndex = if_nametoindex(de->d_name); - if (ifaceIndex) { - ifacePairs.insert(std::pair<std::string, uint32_t>(de->d_name, ifaceIndex)); - } + ifaceNames.push_back(std::string(de->d_name)); } closedir(d); + return ifaceNames; +} + +StatusOr<std::map<std::string, uint32_t>> InterfaceController::getIfaceList() { + std::map<std::string, uint32_t> ifacePairs; + + ASSIGN_OR_RETURN(auto ifaceNames, getIfaceNames()); + + for (const auto& name : ifaceNames) { + uint32_t ifaceIndex = if_nametoindex(name.c_str()); + if (ifaceIndex) { + ifacePairs.insert(std::pair<std::string, uint32_t>(name, ifaceIndex)); + } + } return ifacePairs; } diff --git a/server/InterfaceController.h b/server/InterfaceController.h index 9a2e5e44..cd6f0eb2 100644 --- a/server/InterfaceController.h +++ b/server/InterfaceController.h @@ -53,6 +53,7 @@ public: const char *family, const char *which, const char *interface, const char *parameter, const char *value); + static android::netdutils::StatusOr<std::vector<std::string>> getIfaceNames(); static android::netdutils::StatusOr<std::map<std::string, uint32_t>> getIfaceList(); private: diff --git a/server/InterfaceControllerTest.cpp b/server/InterfaceControllerTest.cpp index 0cf7cfc1..fc5dce12 100644 --- a/server/InterfaceControllerTest.cpp +++ b/server/InterfaceControllerTest.cpp @@ -178,6 +178,19 @@ TEST_F(StablePrivacyTest, ExistingPropertyWriteFail) { class GetIfaceListTest : public testing::Test {}; +TEST_F(GetIfaceListTest, IfaceNames) { + StatusOr<std::vector<std::string>> ifaceNames = InterfaceController::getIfaceNames(); + EXPECT_EQ(ok, ifaceNames.status()); + struct ifaddrs *ifaddr, *ifa; + EXPECT_EQ(0, getifaddrs(&ifaddr)); + for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) { + const auto val = std::find( + ifaceNames.value().begin(), ifaceNames.value().end(), ifa->ifa_name); + EXPECT_NE(ifaceNames.value().end(), val); + } + freeifaddrs(ifaddr); +} + TEST_F(GetIfaceListTest, IfaceExist) { StatusOr<std::map<std::string, uint32_t>> ifaceMap = InterfaceController::getIfaceList(); EXPECT_EQ(ok, ifaceMap.status()); |