Use iptables pipes in BandwidthController startup.
Most of BandwidthController startup is already using
iptables-restore, but some commands (notably listing the costly
chains so they can be flushed by flushCleanTables) still
use iptables. Move these to use execIptablesRestoreWithOutput.
Test: netd_unit_test passes
Bug: 34873832
Change-Id: Ib0741a99a2605cd6934186fd4e5364331a4eab5a
diff --git a/server/BandwidthController.cpp b/server/BandwidthController.cpp
index 09dc483..3df2309 100644
--- a/server/BandwidthController.cpp
+++ b/server/BandwidthController.cpp
@@ -66,7 +66,7 @@
auto BandwidthController::execFunction = android_fork_execvp;
auto BandwidthController::popenFunction = popen;
-auto BandwidthController::iptablesRestoreFunction = execIptablesRestore;
+auto BandwidthController::iptablesRestoreFunction = execIptablesRestoreWithOutput;
namespace {
@@ -75,6 +75,7 @@
const int MAX_CMD_LEN = 1024;
const int MAX_IFACENAME_LEN = 64;
const int MAX_IPT_OUTPUT_LINE_LEN = 256;
+const std::string NEW_CHAIN_COMMAND = "-N ";
/**
* Some comments about the rules:
@@ -138,7 +139,7 @@
* iptables -R 1 bw_data_saver --jump RETURN
*/
-const std::string COMMIT_AND_CLOSE = "COMMIT\n\x04";
+const std::string COMMIT_AND_CLOSE = "COMMIT\n";
const std::string DATA_SAVER_ENABLE_COMMAND = "-R bw_data_saver 1";
const std::string HAPPY_BOX_WHITELIST_COMMAND = android::base::StringPrintf(
"-I bw_happy_box -m owner --uid-owner %d-%d --jump RETURN", 0, MAX_SYSTEM_UID);
@@ -270,7 +271,7 @@
flushExistingCostlyTables(doClean);
std::string commands = android::base::Join(IPT_FLUSH_COMMANDS, '\n');
- iptablesRestoreFunction(V4V6, commands);
+ iptablesRestoreFunction(V4V6, commands, nullptr);
}
int BandwidthController::setupIptablesHooks(void) {
@@ -297,7 +298,7 @@
flushCleanTables(false);
std::string commands = android::base::Join(IPT_BASIC_ACCOUNTING_COMMANDS, '\n');
- return iptablesRestoreFunction(V4V6, commands);
+ return iptablesRestoreFunction(V4V6, commands, nullptr);
}
int BandwidthController::disableBandwidthControl(void) {
@@ -1273,47 +1274,45 @@
}
void BandwidthController::flushExistingCostlyTables(bool doClean) {
- std::string fullCmd;
- FILE *iptOutput;
+ std::string fullCmd = "*filter\n-S\nCOMMIT\n";
+ std::string ruleList;
/* Only lookup ip4 table names as ip6 will have the same tables ... */
- fullCmd = IPTABLES_PATH;
- fullCmd += " -w -S";
- iptOutput = popenFunction(fullCmd.c_str(), "r");
- if (!iptOutput) {
- ALOGE("Failed to run %s err=%s", fullCmd.c_str(), strerror(errno));
+ if (int ret = iptablesRestoreFunction(V4, fullCmd, &ruleList)) {
+ ALOGE("Failed to list existing costly tables ret=%d", ret);
return;
}
/* ... then flush/clean both ip4 and ip6 iptables. */
- parseAndFlushCostlyTables(iptOutput, doClean);
- pclose(iptOutput);
+ parseAndFlushCostlyTables(ruleList, doClean);
}
-void BandwidthController::parseAndFlushCostlyTables(FILE *fp, bool doRemove) {
- int res;
- char lineBuffer[MAX_IPT_OUTPUT_LINE_LEN];
- char costlyIfaceName[MAX_IPT_OUTPUT_LINE_LEN];
- char cmd[MAX_CMD_LEN];
- char *buffPtr;
+void BandwidthController::parseAndFlushCostlyTables(const std::string& ruleList, bool doRemove) {
+ std::stringstream stream(ruleList);
+ std::string rule;
+ std::vector<std::string> clearCommands = { "*filter" };
+ std::string chainName;
- while (NULL != (buffPtr = fgets(lineBuffer, MAX_IPT_OUTPUT_LINE_LEN, fp))) {
- costlyIfaceName[0] = '\0'; /* So that debugging output always works */
- res = sscanf(buffPtr, "-N bw_costly_%s", costlyIfaceName);
- ALOGV("parse res=%d costly=<%s> orig line=<%s>", res,
- costlyIfaceName, buffPtr);
- if (res != 1) {
- continue;
- }
- /* Exclusions: "shared" is not an ifacename */
- if (!strcmp(costlyIfaceName, "shared")) {
+ // Find and flush all rules starting with "-N bw_costly_<iface>" except "-N bw_costly_shared".
+ while (std::getline(stream, rule, '\n')) {
+ if (rule.find(NEW_CHAIN_COMMAND) != 0) continue;
+ chainName = rule.substr(NEW_CHAIN_COMMAND.size());
+ ALOGV("parse chainName=<%s> orig line=<%s>", chainName.c_str(), rule.c_str());
+
+ if (chainName.find("bw_costly_") != 0 || chainName == std::string("bw_costly_shared")) {
continue;
}
- snprintf(cmd, sizeof(cmd), "-F bw_costly_%s", costlyIfaceName);
- runIpxtablesCmd(cmd, IptJumpNoAdd, IptFailHide);
+ clearCommands.push_back(android::base::StringPrintf(":%s -", chainName.c_str()));
if (doRemove) {
- snprintf(cmd, sizeof(cmd), "-X bw_costly_%s", costlyIfaceName);
- runIpxtablesCmd(cmd, IptJumpNoAdd, IptFailHide);
+ clearCommands.push_back(android::base::StringPrintf("-X %s", chainName.c_str()));
}
}
+
+ if (clearCommands.size() == 1) {
+ // No rules found.
+ return;
+ }
+
+ clearCommands.push_back("COMMIT\n");
+ iptablesRestoreFunction(V4V6, android::base::Join(clearCommands, '\n'), nullptr);
}
diff --git a/server/BandwidthController.h b/server/BandwidthController.h
index 53385d1..66fdeee 100644
--- a/server/BandwidthController.h
+++ b/server/BandwidthController.h
@@ -195,7 +195,7 @@
* Deals with both ip4 and ip6 tables.
*/
void flushExistingCostlyTables(bool doClean);
- static void parseAndFlushCostlyTables(FILE *fp, bool doRemove);
+ static void parseAndFlushCostlyTables(const std::string& ruleList, bool doRemove);
/*
* Attempt to flush our tables.
@@ -226,7 +226,7 @@
friend class BandwidthControllerTest;
static int (*execFunction)(int, char **, int *, bool, bool);
static FILE *(*popenFunction)(const char *, const char *);
- static int (*iptablesRestoreFunction)(IptablesTarget, const std::string&);
+ static int (*iptablesRestoreFunction)(IptablesTarget, const std::string&, std::string *);
};
#endif
diff --git a/server/BandwidthControllerTest.cpp b/server/BandwidthControllerTest.cpp
index 3e11596..2b2ce6e 100644
--- a/server/BandwidthControllerTest.cpp
+++ b/server/BandwidthControllerTest.cpp
@@ -27,6 +27,7 @@
#include <gtest/gtest.h>
#include <android-base/strings.h>
+#include <android-base/stringprintf.h>
#include "BandwidthController.h"
#include "IptablesBaseTest.h"
@@ -36,7 +37,7 @@
BandwidthControllerTest() {
BandwidthController::execFunction = fake_android_fork_exec;
BandwidthController::popenFunction = fake_popen;
- BandwidthController::iptablesRestoreFunction = fakeExecIptablesRestore;
+ BandwidthController::iptablesRestoreFunction = fakeExecIptablesRestoreWithOutput;
}
BandwidthController mBw;
@@ -52,49 +53,84 @@
void clearPopenContents() {
sPopenContents.clear();
}
+
+ void addIptablesRestoreOutput(std::string contents) {
+ sIptablesRestoreOutput.push_back(contents);
+ }
+
+ void clearIptablesRestoreOutput() {
+ sIptablesRestoreOutput.clear();
+ }
+
+ void expectSetupCommands(const std::string& expectedClean, std::string expectedAccounting) {
+ std::string expectedList =
+ "*filter\n"
+ "-S\n"
+ "COMMIT\n";
+
+ std::string expectedFlush =
+ "*filter\n"
+ ":bw_INPUT -\n"
+ ":bw_OUTPUT -\n"
+ ":bw_FORWARD -\n"
+ ":bw_happy_box -\n"
+ ":bw_penalty_box -\n"
+ ":bw_data_saver -\n"
+ ":bw_costly_shared -\n"
+ "COMMIT\n"
+ "*raw\n"
+ ":bw_raw_PREROUTING -\n"
+ "COMMIT\n"
+ "*mangle\n"
+ ":bw_mangle_POSTROUTING -\n"
+ "COMMIT\n";
+
+ ExpectedIptablesCommands expected = {{ V4, expectedList }};
+ if (expectedClean.size()) {
+ expected.push_back({ V4V6, expectedClean });
+ }
+ expected.push_back({ V4V6, expectedFlush });
+ if (expectedAccounting.size()) {
+ expected.push_back({ V4V6, expectedAccounting });
+ }
+
+ expectIptablesRestoreCommands(expected);
+ }
};
TEST_F(BandwidthControllerTest, TestSetupIptablesHooks) {
- mBw.setupIptablesHooks();
- std::vector<std::string> expected = {
+ // Pretend some bw_costly_shared_<iface> rules already exist...
+ addIptablesRestoreOutput(
+ "-P OUTPUT ACCEPT\n"
+ "-N bw_costly_rmnet_data0\n"
+ "-N bw_costly_shared\n"
+ "-N unrelated\n"
+ "-N bw_costly_rmnet_data7\n");
+
+ // ... and expect that they be flushed and deleted.
+ std::string expectedCleanCmds =
"*filter\n"
- ":bw_INPUT -\n"
- ":bw_OUTPUT -\n"
- ":bw_FORWARD -\n"
- ":bw_happy_box -\n"
- ":bw_penalty_box -\n"
- ":bw_data_saver -\n"
- ":bw_costly_shared -\n"
- "COMMIT\n"
- "*raw\n"
- ":bw_raw_PREROUTING -\n"
- "COMMIT\n"
- "*mangle\n"
- ":bw_mangle_POSTROUTING -\n"
- "COMMIT\n\x04"
- };
- expectIptablesRestoreCommands(expected);
+ ":bw_costly_rmnet_data0 -\n"
+ "-X bw_costly_rmnet_data0\n"
+ ":bw_costly_rmnet_data7 -\n"
+ "-X bw_costly_rmnet_data7\n"
+ "COMMIT\n";
+
+ mBw.setupIptablesHooks();
+ expectSetupCommands(expectedCleanCmds, "");
}
TEST_F(BandwidthControllerTest, TestEnableBandwidthControl) {
- mBw.enableBandwidthControl(false);
- std::string expectedFlush =
- "*filter\n"
- ":bw_INPUT -\n"
- ":bw_OUTPUT -\n"
- ":bw_FORWARD -\n"
- ":bw_happy_box -\n"
- ":bw_penalty_box -\n"
- ":bw_data_saver -\n"
- ":bw_costly_shared -\n"
- "COMMIT\n"
- "*raw\n"
- ":bw_raw_PREROUTING -\n"
- "COMMIT\n"
- "*mangle\n"
- ":bw_mangle_POSTROUTING -\n"
- "COMMIT\n\x04";
- std::string expectedAccounting =
+ // Pretend no bw_costly_shared_<iface> rules already exist...
+ addIptablesRestoreOutput(
+ "-P OUTPUT ACCEPT\n"
+ "-N bw_costly_shared\n"
+ "-N unrelated\n");
+
+ // ... so none are flushed or deleted.
+ std::string expectedClean = "";
+
+ std::string expectedAccounting =
"*filter\n"
"-A bw_INPUT -m owner --socket-exists\n"
"-A bw_OUTPUT -m owner --socket-exists\n"
@@ -109,30 +145,30 @@
"COMMIT\n"
"*mangle\n"
"-A bw_mangle_POSTROUTING -m owner --socket-exists\n"
- "COMMIT\n\x04";
+ "COMMIT\n";
- expectIptablesRestoreCommands({ expectedFlush, expectedAccounting });
+ mBw.enableBandwidthControl(false);
+ expectSetupCommands(expectedClean, expectedAccounting);
}
TEST_F(BandwidthControllerTest, TestDisableBandwidthControl) {
- mBw.disableBandwidthControl();
- const std::string expected =
+ // Pretend some bw_costly_shared_<iface> rules already exist...
+ addIptablesRestoreOutput(
+ "-P OUTPUT ACCEPT\n"
+ "-N bw_costly_rmnet_data0\n"
+ "-N bw_costly_shared\n"
+ "-N unrelated\n"
+ "-N bw_costly_rmnet_data7\n");
+
+ // ... and expect that they be flushed.
+ std::string expectedCleanCmds =
"*filter\n"
- ":bw_INPUT -\n"
- ":bw_OUTPUT -\n"
- ":bw_FORWARD -\n"
- ":bw_happy_box -\n"
- ":bw_penalty_box -\n"
- ":bw_data_saver -\n"
- ":bw_costly_shared -\n"
- "COMMIT\n"
- "*raw\n"
- ":bw_raw_PREROUTING -\n"
- "COMMIT\n"
- "*mangle\n"
- ":bw_mangle_POSTROUTING -\n"
- "COMMIT\n\x04";
- expectIptablesRestoreCommands({ expected });
+ ":bw_costly_rmnet_data0 -\n"
+ ":bw_costly_rmnet_data7 -\n"
+ "COMMIT\n";
+
+ mBw.disableBandwidthControl();
+ expectSetupCommands(expectedCleanCmds, "");
}
TEST_F(BandwidthControllerTest, TestEnableDataSaver) {