refine test TestIpfwdEnableDisableStatusForwarding and some minor changes on test
1. fix flaky test for TestIpfwdEnableDisableStatusForwarding
2. fix time unit error in binder_test, set retry default
interval as 50ms
3. refine naming in binder_test
Bug: 128533612
Test: built, flashed, booted
system/netd/tests/runtests.sh pass
Change-Id: Ia03d0c30b49cdd233b627cce8bd2f3f05548798e
diff --git a/server/CommandListener.cpp b/server/CommandListener.cpp
index 6a79704..d5f93f2 100644
--- a/server/CommandListener.cpp
+++ b/server/CommandListener.cpp
@@ -364,7 +364,8 @@
char *tmp = nullptr;
asprintf(&tmp, "Forwarding %s",
- ((gCtls->tetherCtrl.forwardingRequestCount() > 0) ? "enabled" : "disabled"));
+ ((gCtls->tetherCtrl.getIpfwdRequesterList().size() > 0) ? "enabled"
+ : "disabled"));
cli->sendMsg(ResponseCode::IpFwdStatusResult, tmp, false);
free(tmp);
return 0;
diff --git a/server/NetdNativeService.cpp b/server/NetdNativeService.cpp
index 7728e0e..1b537eb 100644
--- a/server/NetdNativeService.cpp
+++ b/server/NetdNativeService.cpp
@@ -886,7 +886,15 @@
binder::Status NetdNativeService::ipfwdEnabled(bool* status) {
NETD_LOCKING_RPC(gCtls->tetherCtrl.lock, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
- *status = (gCtls->tetherCtrl.forwardingRequestCount() > 0) ? true : false;
+ *status = (gCtls->tetherCtrl.getIpfwdRequesterList().size() > 0) ? true : false;
+ return binder::Status::ok();
+}
+
+binder::Status NetdNativeService::ipfwdGetRequesterList(std::vector<std::string>* requesterList) {
+ NETD_LOCKING_RPC(gCtls->tetherCtrl.lock, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
+ for (const auto& requester : gCtls->tetherCtrl.getIpfwdRequesterList()) {
+ requesterList->push_back(requester);
+ }
return binder::Status::ok();
}
diff --git a/server/NetdNativeService.h b/server/NetdNativeService.h
index 09bf712..1bed73d 100644
--- a/server/NetdNativeService.h
+++ b/server/NetdNativeService.h
@@ -235,6 +235,7 @@
// Ipfw-related commands
binder::Status ipfwdEnabled(bool* status) override;
+ binder::Status ipfwdGetRequesterList(std::vector<std::string>* requesterList) override;
binder::Status ipfwdEnableForwarding(const std::string& requester) override;
binder::Status ipfwdDisableForwarding(const std::string& requester) override;
binder::Status ipfwdAddInterfaceForward(const std::string& fromIface,
diff --git a/server/TetherController.cpp b/server/TetherController.cpp
index bf1d177..6ce617d 100644
--- a/server/TetherController.cpp
+++ b/server/TetherController.cpp
@@ -202,8 +202,8 @@
return setIpFwdEnabled();
}
-size_t TetherController::forwardingRequestCount() {
- return mForwardingRequests.size();
+const std::set<std::string>& TetherController::getIpfwdRequesterList() const {
+ return mForwardingRequests;
}
int TetherController::startTethering(int num_addrs, char **dhcp_ranges) {
diff --git a/server/TetherController.h b/server/TetherController.h
index 49c2339..0a04874 100644
--- a/server/TetherController.h
+++ b/server/TetherController.h
@@ -71,7 +71,7 @@
bool enableForwarding(const char* requester);
bool disableForwarding(const char* requester);
- size_t forwardingRequestCount();
+ const std::set<std::string>& getIpfwdRequesterList() const;
int startTethering(int num_addrs, char **dhcp_ranges);
int startTethering(const std::vector<std::string>& dhcpRanges);
diff --git a/server/binder/android/net/INetd.aidl b/server/binder/android/net/INetd.aidl
index 9e8dcb4..4dfa722 100644
--- a/server/binder/android/net/INetd.aidl
+++ b/server/binder/android/net/INetd.aidl
@@ -653,6 +653,13 @@
boolean ipfwdEnabled();
/**
+ * Get requester list of IP forwarding
+ *
+ * @return An array of strings containing requester list of IP forwarding
+ */
+ @utf8InCpp String[] ipfwdGetRequesterList();
+
+ /**
* Enable IP forwarding for specific requester
*
* @param requester requester name to enable IP forwarding. It is a unique name which will be
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index 2e425db..bb569e9 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -1283,7 +1283,7 @@
namespace {
std::vector<std::string> tryToFindProcesses(const std::string& processName, uint32_t maxTries = 1,
- uint32_t intervalMs = 0) {
+ uint32_t intervalMs = 50) {
// Output looks like:(clatd)
// clat 4963 850 1 12:16:51 ? 00:00:00 clatd-netd10a88 -i netd10a88 ...
// ...
@@ -1302,16 +1302,16 @@
break;
}
- usleep(intervalMs);
+ usleep(intervalMs * 1000);
}
return result;
}
void expectProcessExists(const std::string& processName) {
- EXPECT_EQ(1U, tryToFindProcesses(processName, 5 /*maxTries*/, 50 /*intervalMs*/).size());
+ EXPECT_EQ(1U, tryToFindProcesses(processName, 5 /*maxTries*/).size());
}
-void expectProcessDoesNotExists(const std::string& processName) {
+void expectProcessDoesNotExist(const std::string& processName) {
EXPECT_FALSE(tryToFindProcesses(processName).size());
}
@@ -1370,13 +1370,13 @@
// Expect clatd to stop successfully.
status = mNetd->clatdStop(sTun.name());
EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
- expectProcessDoesNotExists(clatdName);
+ expectProcessDoesNotExist(clatdName);
// Stopping a clatd that doesn't exist returns ENODEV.
status = mNetd->clatdStop(sTun.name());
EXPECT_FALSE(status.isOk());
EXPECT_EQ(ENODEV, status.serviceSpecificErrorCode());
- expectProcessDoesNotExists(clatdName);
+ expectProcessDoesNotExist(clatdName);
// Clean up.
EXPECT_TRUE(mNetd->networkRemoveRoute(TEST_NETID1, sTun.name(), "::/0", "").isOk());
@@ -1438,25 +1438,56 @@
} // namespace
TEST_F(BinderTest, TestIpfwdEnableDisableStatusForwarding) {
- // Netd default enable Ipfwd with requester NetdHwService
- const std::string defaultRequester = "NetdHwService";
-
- binder::Status status = mNetd->ipfwdDisableForwarding(defaultRequester);
+ // Get ipfwd requester list from Netd
+ std::vector<std::string> requesterList;
+ binder::Status status = mNetd->ipfwdGetRequesterList(&requesterList);
EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
- expectIpfwdEnable(false);
bool ipfwdEnabled;
- status = mNetd->ipfwdEnabled(&ipfwdEnabled);
- EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
- EXPECT_FALSE(ipfwdEnabled);
+ if (requesterList.size() == 0) {
+ // No requester in Netd, ipfwd should be disabled
+ // So add one test requester and verify
+ status = mNetd->ipfwdEnableForwarding("TestRequester");
+ EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
- status = mNetd->ipfwdEnableForwarding(defaultRequester);
- EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
- expectIpfwdEnable(true);
+ expectIpfwdEnable(true);
+ status = mNetd->ipfwdEnabled(&ipfwdEnabled);
+ EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+ EXPECT_TRUE(ipfwdEnabled);
- status = mNetd->ipfwdEnabled(&ipfwdEnabled);
- EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
- EXPECT_TRUE(ipfwdEnabled);
+ // Remove test one, verify again
+ status = mNetd->ipfwdDisableForwarding("TestRequester");
+ EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+
+ expectIpfwdEnable(false);
+ status = mNetd->ipfwdEnabled(&ipfwdEnabled);
+ EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+ EXPECT_FALSE(ipfwdEnabled);
+ } else {
+ // Disable all requesters
+ for (const auto& requester : requesterList) {
+ status = mNetd->ipfwdDisableForwarding(requester);
+ EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+ }
+
+ // After disable all requester, ipfwd should be disabled
+ expectIpfwdEnable(false);
+ status = mNetd->ipfwdEnabled(&ipfwdEnabled);
+ EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+ EXPECT_FALSE(ipfwdEnabled);
+
+ // Enable them back
+ for (const auto& requester : requesterList) {
+ status = mNetd->ipfwdEnableForwarding(requester);
+ EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+ }
+
+ // ipfwd should be enabled
+ expectIpfwdEnable(true);
+ status = mNetd->ipfwdEnabled(&ipfwdEnabled);
+ EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+ EXPECT_TRUE(ipfwdEnabled);
+ }
}
TEST_F(BinderTest, TestIpfwdAddRemoveInterfaceForward) {
@@ -2088,7 +2119,7 @@
status = mNetd->tetherStop();
EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
- expectProcessDoesNotExists(dnsdName);
+ expectProcessDoesNotExist(dnsdName);
status = mNetd->tetherIsEnabled(&tetherEnabled);
EXPECT_TRUE(status.isOk()) << status.exceptionMessage();