Speed up FirewallController startup.
FirewallController::createChain runs iptables commands to remove
the newly-created chain from fw_INPUT. This is not necessary,
because createChain is only called from setupIptablesHooks, which
is only called immediately after initIptablesRules, which clears
fw_INPUT. So there is nothing to delete.
Removing these unnecessary commands speeds up netd startup by
~150ms. Before:
02-03 18:51:40.075 492 492 I Netd : Setting up FirewallController hooks: 159.9ms
After:
02-03 18:45:22.005 489 489 I Netd : Setting up FirewallController hooks: 11.3ms
Bug: 34873832
Test: unit tests continue to pass
Change-Id: I651d96a71c98d6aba989927cd23036d5cc371dd7
diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index 826cf75..9cab90a 100644
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -63,16 +63,9 @@
int FirewallController::setupIptablesHooks(void) {
int res = 0;
- // child chains are created but not attached, they will be attached explicitly.
- FirewallType firewallType = getFirewallType(DOZABLE);
- res |= createChain(LOCAL_DOZABLE, LOCAL_INPUT, firewallType);
-
- firewallType = getFirewallType(STANDBY);
- res |= createChain(LOCAL_STANDBY, LOCAL_INPUT, firewallType);
-
- firewallType = getFirewallType(POWERSAVE);
- res |= createChain(LOCAL_POWERSAVE, LOCAL_INPUT, firewallType);
-
+ res |= createChain(LOCAL_DOZABLE, getFirewallType(DOZABLE));
+ res |= createChain(LOCAL_STANDBY, getFirewallType(STANDBY));
+ res |= createChain(LOCAL_POWERSAVE, getFirewallType(POWERSAVE));
return res;
}
@@ -288,11 +281,9 @@
return execIptables(V4V6, "-t", TABLE, "-D", parentChain, "-j", childChain, NULL);
}
-int FirewallController::createChain(const char* childChain,
- const char* parentChain, FirewallType type) {
- execIptablesSilently(V4V6, "-t", TABLE, "-D", parentChain, "-j", childChain, NULL);
- std::vector<int32_t> uids;
- return replaceUidChain(childChain, type == WHITELIST, uids);
+int FirewallController::createChain(const char* chain, FirewallType type) {
+ static const std::vector<int32_t> NO_UIDS;
+ return replaceUidChain(chain, type == WHITELIST, NO_UIDS);
}
std::string FirewallController::makeUidRules(IptablesTarget target, const char *name,
@@ -333,7 +324,7 @@
StringAppendF(&commands, "-A %s -j DROP\n", name);
}
- StringAppendF(&commands, "COMMIT\n\x04"); // EOT.
+ StringAppendF(&commands, "COMMIT\n");
return commands;
}
diff --git a/server/FirewallController.h b/server/FirewallController.h
index d78b461..67d632c 100644
--- a/server/FirewallController.h
+++ b/server/FirewallController.h
@@ -93,7 +93,7 @@
FirewallType mFirewallType;
int attachChain(const char*, const char*);
int detachChain(const char*, const char*);
- int createChain(const char*, const char*, FirewallType);
+ int createChain(const char*, FirewallType);
FirewallType getFirewallType(ChildChain);
};
diff --git a/server/FirewallControllerTest.cpp b/server/FirewallControllerTest.cpp
index 7d96c61..9d43636 100644
--- a/server/FirewallControllerTest.cpp
+++ b/server/FirewallControllerTest.cpp
@@ -42,17 +42,13 @@
return mFw.makeUidRules(a, b, c, d);
}
- int createChain(const char* a, const char* b , FirewallType c) {
- return mFw.createChain(a, b, c);
+ int createChain(const char* a, FirewallType b) {
+ return mFw.createChain(a, b);
}
};
TEST_F(FirewallControllerTest, TestCreateWhitelistChain) {
- ExpectedIptablesCommands expectedCommands = {
- { V4V6, "-t filter -D INPUT -j fw_whitelist" },
- };
-
std::vector<std::string> expectedRestore4 = {
"*filter",
":fw_whitelist -",
@@ -60,7 +56,7 @@
"-A fw_whitelist -p tcp --tcp-flags RST RST -j RETURN",
"-A fw_whitelist -m owner --uid-owner 0-9999 -j RETURN",
"-A fw_whitelist -j DROP",
- "COMMIT\n\x04"
+ "COMMIT\n"
};
std::vector<std::string> expectedRestore6 = {
"*filter",
@@ -75,37 +71,31 @@
"-A fw_whitelist -p icmpv6 --icmpv6-type redirect -j RETURN",
"-A fw_whitelist -m owner --uid-owner 0-9999 -j RETURN",
"-A fw_whitelist -j DROP",
- "COMMIT\n\x04"
+ "COMMIT\n"
};
std::vector<std::pair<IptablesTarget, std::string>> expectedRestoreCommands = {
{ V4, android::base::Join(expectedRestore4, '\n') },
{ V6, android::base::Join(expectedRestore6, '\n') },
};
- createChain("fw_whitelist", "INPUT", WHITELIST);
- expectIptablesCommands(expectedCommands);
+ createChain("fw_whitelist", WHITELIST);
expectIptablesRestoreCommands(expectedRestoreCommands);
}
TEST_F(FirewallControllerTest, TestCreateBlacklistChain) {
- ExpectedIptablesCommands expectedCommands = {
- { V4V6, "-t filter -D INPUT -j fw_blacklist" },
- };
-
std::vector<std::string> expectedRestore = {
"*filter",
":fw_blacklist -",
"-A fw_blacklist -i lo -o lo -j RETURN",
"-A fw_blacklist -p tcp --tcp-flags RST RST -j RETURN",
- "COMMIT\n\x04"
+ "COMMIT\n"
};
std::vector<std::pair<IptablesTarget, std::string>> expectedRestoreCommands = {
{ V4, android::base::Join(expectedRestore, '\n') },
{ V6, android::base::Join(expectedRestore, '\n') },
};
- createChain("fw_blacklist", "INPUT", BLACKLIST);
- expectIptablesCommands(expectedCommands);
+ createChain("fw_blacklist", BLACKLIST);
expectIptablesRestoreCommands(expectedRestoreCommands);
}
@@ -158,7 +148,7 @@
"-A FW_whitechain -m owner --uid-owner 210153 -j RETURN\n"
"-A FW_whitechain -m owner --uid-owner 210024 -j RETURN\n"
"-A FW_whitechain -j DROP\n"
- "COMMIT\n\x04";
+ "COMMIT\n";
std::vector<int32_t> uids = { 10023, 10059, 10124, 10111, 110122, 210153, 210024 };
EXPECT_EQ(expected, makeUidRules(V6, "FW_whitechain", true, uids));
@@ -173,7 +163,7 @@
"-A FW_blackchain -m owner --uid-owner 10023 -j DROP\n"
"-A FW_blackchain -m owner --uid-owner 10059 -j DROP\n"
"-A FW_blackchain -m owner --uid-owner 10124 -j DROP\n"
- "COMMIT\n\x04";
+ "COMMIT\n";
std::vector<int32_t> uids = { 10023, 10059, 10124 };
EXPECT_EQ(expected, makeUidRules(V4 ,"FW_blackchain", false, uids));