Merge changes I24d83b88,I2ab9269d
* changes:
Start the iptables and ip6tables processes in parallel.
Add test coverage for setting up initial iptables rules.
diff --git a/server/Android.mk b/server/Android.mk
index 9cb4062..d924c84 100644
--- a/server/Android.mk
+++ b/server/Android.mk
@@ -162,7 +162,7 @@
LOCAL_SRC_FILES := \
InterfaceController.cpp InterfaceControllerTest.cpp \
- Controllers.cpp \
+ Controllers.cpp ControllersTest.cpp \
NetdConstants.cpp IptablesBaseTest.cpp \
IptablesRestoreController.cpp IptablesRestoreControllerTest.cpp \
BandwidthController.cpp BandwidthControllerTest.cpp \
diff --git a/server/Controllers.cpp b/server/Controllers.cpp
index ad77ee1..a25e05a 100644
--- a/server/Controllers.cpp
+++ b/server/Controllers.cpp
@@ -29,6 +29,9 @@
namespace android {
namespace net {
+auto Controllers::execIptablesRestore = ::execIptablesRestore;
+auto Controllers::execIptablesSilently = ::execIptablesSilently;
+
namespace {
/**
* List of module chains to be created, along with explicit ordering. ORDERING
@@ -93,8 +96,13 @@
NULL,
};
-static void createChildChains(IptablesTarget target, const char* table, const char* parentChain,
- const char** childChains, bool exclusive) {
+} // namespace
+
+/* static */
+void Controllers::createChildChains(IptablesTarget target, const char* table,
+ const char* parentChain,
+ const char** childChains,
+ bool exclusive) {
std::string command = android::base::StringPrintf("*%s\n", table);
// If we're the exclusive owner of this chain, clear it entirely. This saves us from having to
@@ -116,12 +124,10 @@
command += android::base::StringPrintf(":%s -\n", *childChain);
command += android::base::StringPrintf("-A %s -j %s\n", parentChain, *childChain);
} while (*(++childChain) != NULL);
- command += "COMMIT\n\n";
+ command += "COMMIT\n";
execIptablesRestore(target, command);
}
-} // namespace
-
Controllers::Controllers()
: clatdCtrl(&netCtrl),
wakeupCtrl(
@@ -137,7 +143,7 @@
InterfaceController::initializeAll();
}
-void Controllers::initIptablesRules() {
+void Controllers::initChildChains() {
/*
* This is the only time we touch top-level chains in iptables; controllers
* should only mutate rules inside of their children chains, as created by
@@ -149,18 +155,23 @@
*/
// Create chains for child modules.
- // We cannot use createChildChainsFast for all chains because vendor code modifies filter OUTPUT
- // and mangle POSTROUTING directly.
- Stopwatch s;
createChildChains(V4V6, "filter", "INPUT", FILTER_INPUT, true);
createChildChains(V4V6, "filter", "FORWARD", FILTER_FORWARD, true);
- createChildChains(V4V6, "filter", "OUTPUT", FILTER_OUTPUT, false);
createChildChains(V4V6, "raw", "PREROUTING", RAW_PREROUTING, true);
- createChildChains(V4V6, "mangle", "POSTROUTING", MANGLE_POSTROUTING, false);
createChildChains(V4V6, "mangle", "FORWARD", MANGLE_FORWARD, true);
createChildChains(V4V6, "mangle", "INPUT", MANGLE_INPUT, true);
createChildChains(V4, "nat", "PREROUTING", NAT_PREROUTING, true);
createChildChains(V4, "nat", "POSTROUTING", NAT_POSTROUTING, true);
+
+ // We cannot use createChildChainsFast for all chains because vendor code modifies filter OUTPUT
+ // and mangle POSTROUTING directly.
+ createChildChains(V4V6, "filter", "OUTPUT", FILTER_OUTPUT, false);
+ createChildChains(V4V6, "mangle", "POSTROUTING", MANGLE_POSTROUTING, false);
+}
+
+void Controllers::initIptablesRules() {
+ Stopwatch s;
+ initChildChains();
ALOGI("Creating child chains: %.1fms", s.getTimeAndReset());
// Let each module setup their child chains
diff --git a/server/Controllers.h b/server/Controllers.h
index 0bfa0e7..0754932 100644
--- a/server/Controllers.h
+++ b/server/Controllers.h
@@ -60,7 +60,13 @@
void init();
private:
+ friend class ControllersTest;
void initIptablesRules();
+ static void initChildChains();
+ static void createChildChains(IptablesTarget target, const char* table, const char* parentChain,
+ const char** childChains, bool exclusive);
+ static int (*execIptablesSilently)(IptablesTarget target, ...);
+ static int (*execIptablesRestore)(IptablesTarget, const std::string&);
};
extern Controllers* gCtls;
diff --git a/server/ControllersTest.cpp b/server/ControllersTest.cpp
new file mode 100644
index 0000000..6f41798
--- /dev/null
+++ b/server/ControllersTest.cpp
@@ -0,0 +1,150 @@
+/*
+ * Copyright 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * ControllersTest.cpp - unit tests for Controllers.cpp
+ */
+
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include "Controllers.h"
+#include "IptablesBaseTest.h"
+
+namespace android {
+namespace net {
+
+class ControllersTest : public IptablesBaseTest {
+ public:
+ ControllersTest() {
+ Controllers::execIptablesSilently = fakeExecIptables;
+ Controllers::execIptablesRestore = fakeExecIptablesRestore;
+ }
+
+ protected:
+ void initChildChains() { Controllers::initChildChains(); };
+};
+
+TEST_F(ControllersTest, TestInitIptablesRules) {
+ ExpectedIptablesCommands expectedRestoreCommands = {
+ { V4V6, "*filter\n"
+ ":INPUT -\n"
+ "-F INPUT\n"
+ ":bw_INPUT -\n"
+ "-A INPUT -j bw_INPUT\n"
+ ":fw_INPUT -\n"
+ "-A INPUT -j fw_INPUT\n"
+ "COMMIT\n"
+ },
+ { V4V6, "*filter\n"
+ ":FORWARD -\n"
+ "-F FORWARD\n"
+ ":oem_fwd -\n"
+ "-A FORWARD -j oem_fwd\n"
+ ":fw_FORWARD -\n"
+ "-A FORWARD -j fw_FORWARD\n"
+ ":bw_FORWARD -\n"
+ "-A FORWARD -j bw_FORWARD\n"
+ ":natctrl_FORWARD -\n"
+ "-A FORWARD -j natctrl_FORWARD\n"
+ "COMMIT\n"
+ },
+ { V4V6, "*raw\n"
+ ":PREROUTING -\n"
+ "-F PREROUTING\n"
+ ":bw_raw_PREROUTING -\n"
+ "-A PREROUTING -j bw_raw_PREROUTING\n"
+ ":idletimer_raw_PREROUTING -\n"
+ "-A PREROUTING -j idletimer_raw_PREROUTING\n"
+ ":natctrl_raw_PREROUTING -\n"
+ "-A PREROUTING -j natctrl_raw_PREROUTING\n"
+ "COMMIT\n"
+ },
+ { V4V6, "*mangle\n"
+ ":FORWARD -\n"
+ "-F FORWARD\n"
+ ":natctrl_mangle_FORWARD -\n"
+ "-A FORWARD -j natctrl_mangle_FORWARD\n"
+ "COMMIT\n"
+ },
+ { V4V6, "*mangle\n"
+ ":INPUT -\n"
+ "-F INPUT\n"
+ ":wakeupctrl_mangle_INPUT -\n"
+ "-A INPUT -j wakeupctrl_mangle_INPUT\n"
+ ":routectrl_mangle_INPUT -\n"
+ "-A INPUT -j routectrl_mangle_INPUT\n"
+ "COMMIT\n"
+ },
+ { V4, "*nat\n"
+ ":PREROUTING -\n"
+ "-F PREROUTING\n"
+ ":oem_nat_pre -\n"
+ "-A PREROUTING -j oem_nat_pre\n"
+ "COMMIT\n"
+ },
+ { V4, "*nat\n"
+ ":POSTROUTING -\n"
+ "-F POSTROUTING\n"
+ ":natctrl_nat_POSTROUTING -\n"
+ "-A POSTROUTING -j natctrl_nat_POSTROUTING\n"
+ "COMMIT\n"
+ },
+ { V4V6, "*filter\n"
+ ":oem_out -\n"
+ "-A OUTPUT -j oem_out\n"
+ ":fw_OUTPUT -\n"
+ "-A OUTPUT -j fw_OUTPUT\n"
+ ":st_OUTPUT -\n"
+ "-A OUTPUT -j st_OUTPUT\n"
+ ":bw_OUTPUT -\n"
+ "-A OUTPUT -j bw_OUTPUT\n"
+ "COMMIT\n"
+ },
+ { V4V6, "*mangle\n"
+ ":oem_mangle_post -\n"
+ "-A POSTROUTING -j oem_mangle_post\n"
+ ":bw_mangle_POSTROUTING -\n"
+ "-A POSTROUTING -j bw_mangle_POSTROUTING\n"
+ ":idletimer_mangle_POSTROUTING -\n"
+ "-A POSTROUTING -j idletimer_mangle_POSTROUTING\n"
+ "COMMIT\n"
+ },
+ };
+ initChildChains();
+ expectIptablesRestoreCommands(expectedRestoreCommands);
+
+ std::vector<std::string> expectedIptablesCommands = {
+ "-t filter -D OUTPUT -j oem_out",
+ "-t filter -D OUTPUT -j fw_OUTPUT",
+ "-t filter -D OUTPUT -j st_OUTPUT",
+ "-t filter -D OUTPUT -j bw_OUTPUT",
+ "-t mangle -D POSTROUTING -j oem_mangle_post",
+ "-t mangle -D POSTROUTING -j bw_mangle_POSTROUTING",
+ "-t mangle -D POSTROUTING -j idletimer_mangle_POSTROUTING",
+ };
+ expectIptablesCommands(expectedIptablesCommands);
+
+ // ... and nothing more.
+ expectedRestoreCommands = {};
+ expectIptablesRestoreCommands(expectedRestoreCommands);
+
+ expectedIptablesCommands = {};
+ expectIptablesCommands(expectedIptablesCommands);
+}
+
+} // namespace net
+} // namespace android
diff --git a/server/IptablesRestoreController.cpp b/server/IptablesRestoreController.cpp
index 406983a..e346b82 100644
--- a/server/IptablesRestoreController.cpp
+++ b/server/IptablesRestoreController.cpp
@@ -112,9 +112,12 @@
static constexpr size_t STDERR_IDX = 1;
};
-IptablesRestoreController::IptablesRestoreController() :
- mIpRestore(nullptr),
- mIp6Restore(nullptr) {
+IptablesRestoreController::IptablesRestoreController() {
+ // Start the IPv4 and IPv6 processes in parallel, since each one takes 20-30ms.
+ std::thread v4([this] () { mIpRestore.reset(forkAndExec(IPTABLES_PROCESS)); });
+ std::thread v6([this] () { mIp6Restore.reset(forkAndExec(IP6TABLES_PROCESS)); });
+ v4.join();
+ v6.join();
}
IptablesRestoreController::~IptablesRestoreController() {
