Simplify enums in BandwidthController.
1. Ensure that the code always uses all enum values. This
provides a clear compile-time error if a passed-in enum value
is not handled, and allows us to remove several default
case labels and unreachable error logging code.
2. Factor out to common functions the code that converts enum
values to parts of iptables command lines.
Bug: 32073253
Test: netd_{unit,integration}_test pass
Change-Id: I7136055100dc312fa7cb8bba5506fe86412b1f4d
diff --git a/server/BandwidthController.cpp b/server/BandwidthController.cpp
index 30a048a..828ef3a 100644
--- a/server/BandwidthController.cpp
+++ b/server/BandwidthController.cpp
@@ -229,22 +229,7 @@
int status = 0;
std::string fullCmd = cmd;
-
- switch (jumpHandling) {
- case IptJumpReject:
- /*
- * Must be carefull what one rejects with, as uper layer protocols will just
- * keep on hammering the device until the number of retries are done.
- * For port-unreachable (default), TCP should consider as an abort (RFC1122).
- */
- fullCmd += " --jump REJECT";
- break;
- case IptJumpReturn:
- fullCmd += " --jump RETURN";
- break;
- case IptJumpNoAdd:
- break;
- }
+ fullCmd += jumpToString(jumpHandling);
fullCmd.insert(0, " -w ");
fullCmd.insert(0, iptVer == IptIpV4 ? IPTABLES_PATH : IP6TABLES_PATH);
@@ -345,15 +330,6 @@
case IptOpInsert:
opFlag = "-I";
break;
- case IptOpAppend:
- ALOGE("Append op not supported for %s uids", chain);
- res = "";
- return res;
- break;
- case IptOpReplace:
- opFlag = "-R";
- break;
- default:
case IptOpDelete:
opFlag = "-D";
break;
@@ -365,52 +341,46 @@
}
int BandwidthController::addNaughtyApps(int numUids, char *appUids[]) {
- return manipulateNaughtyApps(numUids, appUids, SpecialAppOpAdd);
+ return manipulateNaughtyApps(numUids, appUids, IptOpInsert);
}
int BandwidthController::removeNaughtyApps(int numUids, char *appUids[]) {
- return manipulateNaughtyApps(numUids, appUids, SpecialAppOpRemove);
+ return manipulateNaughtyApps(numUids, appUids, IptOpDelete);
}
int BandwidthController::addNiceApps(int numUids, char *appUids[]) {
- return manipulateNiceApps(numUids, appUids, SpecialAppOpAdd);
+ return manipulateNiceApps(numUids, appUids, IptOpInsert);
}
int BandwidthController::removeNiceApps(int numUids, char *appUids[]) {
- return manipulateNiceApps(numUids, appUids, SpecialAppOpRemove);
+ return manipulateNiceApps(numUids, appUids, IptOpDelete);
}
-int BandwidthController::manipulateNaughtyApps(int numUids, char *appStrUids[], SpecialAppOp appOp) {
- return manipulateSpecialApps(numUids, appStrUids, "bw_penalty_box", IptJumpReject, appOp);
+int BandwidthController::manipulateNaughtyApps(int numUids, char *appStrUids[], IptOp op) {
+ return manipulateSpecialApps(numUids, appStrUids, "bw_penalty_box", IptJumpReject, op);
}
-int BandwidthController::manipulateNiceApps(int numUids, char *appStrUids[], SpecialAppOp appOp) {
- return manipulateSpecialApps(numUids, appStrUids, "bw_happy_box", IptJumpReturn, appOp);
+int BandwidthController::manipulateNiceApps(int numUids, char *appStrUids[], IptOp op) {
+ return manipulateSpecialApps(numUids, appStrUids, "bw_happy_box", IptJumpReturn, op);
}
int BandwidthController::manipulateSpecialApps(int numUids, char *appStrUids[],
const char *chain,
- IptJumpOp jumpHandling, SpecialAppOp appOp) {
+ IptJumpOp jumpHandling, IptOp op) {
int uidNum;
const char *failLogTemplate;
- IptOp op;
int appUids[numUids];
std::string iptCmd;
- switch (appOp) {
- case SpecialAppOpAdd:
- op = IptOpInsert;
+ switch (op) {
+ case IptOpInsert:
failLogTemplate = "Failed to add app uid %s(%d) to %s.";
break;
- case SpecialAppOpRemove:
- op = IptOpDelete;
+ case IptOpDelete:
failLogTemplate = "Failed to delete app uid %s(%d) from %s box.";
break;
- default:
- ALOGE("Unexpected app Op %d", appOp);
- return -1;
}
for (uidNum = 0; uidNum < numUids; uidNum++) {
@@ -441,7 +411,7 @@
return -1;
}
-std::string BandwidthController::makeIptablesQuotaCmd(IptOp op, const char *costName, int64_t quota) {
+std::string BandwidthController::makeIptablesQuotaCmd(IptFullOp op, const char *costName, int64_t quota) {
std::string res;
char *buff;
const char *opFlag;
@@ -449,17 +419,13 @@
ALOGV("makeIptablesQuotaCmd(%d, %" PRId64")", op, quota);
switch (op) {
- case IptOpInsert:
+ case IptFullOpInsert:
opFlag = "-I";
break;
- case IptOpAppend:
+ case IptFullOpAppend:
opFlag = "-A";
break;
- case IptOpReplace:
- opFlag = "-R";
- break;
- default:
- case IptOpDelete:
+ case IptFullOpDelete:
opFlag = "-D";
break;
}
@@ -502,9 +468,6 @@
case QuotaShared:
costCString = "bw_costly_shared";
break;
- default:
- ALOGE("Unexpected quotatype %d", quotaType);
- return -1;
}
if (globalAlertBytes) {
@@ -547,9 +510,6 @@
case QuotaShared:
costCString = "bw_costly_shared";
break;
- default:
- ALOGE("Unexpected quotatype %d", quotaType);
- return -1;
}
snprintf(cmd, sizeof(cmd), "-D bw_INPUT -i %s --jump %s", ifn, costCString);
@@ -604,7 +564,7 @@
if (it == sharedQuotaIfaces.end()) {
res |= prepCostlyIface(ifn, QuotaShared);
if (sharedQuotaIfaces.empty()) {
- quotaCmd = makeIptablesQuotaCmd(IptOpInsert, costName, maxBytes);
+ quotaCmd = makeIptablesQuotaCmd(IptFullOpInsert, costName, maxBytes);
res |= runIpxtablesCmd(quotaCmd.c_str(), IptJumpReject);
if (res) {
ALOGE("Failed set quota rule");
@@ -667,7 +627,7 @@
if (sharedQuotaIfaces.empty()) {
std::string quotaCmd;
- quotaCmd = makeIptablesQuotaCmd(IptOpDelete, costName, sharedQuotaBytes);
+ quotaCmd = makeIptablesQuotaCmd(IptFullOpDelete, costName, sharedQuotaBytes);
res |= runIpxtablesCmd(quotaCmd.c_str(), IptJumpReject);
sharedQuotaBytes = 0;
if (sharedAlertBytes) {
@@ -719,7 +679,7 @@
* or else a naughty app could just eat up the quota.
* So we append here.
*/
- quotaCmd = makeIptablesQuotaCmd(IptOpAppend, costName, maxBytes);
+ quotaCmd = makeIptablesQuotaCmd(IptFullOpAppend, costName, maxBytes);
res |= runIpxtablesCmd(quotaCmd.c_str(), IptJumpReject);
if (res) {
ALOGE("Failed set quota rule");
@@ -829,19 +789,9 @@
}
int BandwidthController::runIptablesAlertCmd(IptOp op, const char *alertName, int64_t bytes) {
- const char *opFlag;
+ const char *opFlag = opToString(op);
std::string alertQuotaCmd = "*filter\n";
- switch (op) {
- case IptOpInsert:
- opFlag = "-I";
- break;
- default:
- case IptOpDelete:
- opFlag = "-D";
- break;
- }
-
// TODO: consider using an alternate template for the delete that does not include the --quota
// value. This code works because the --quota value is ignored by deletes
StringAppendF(&alertQuotaCmd, ALERT_IPT_TEMPLATE, opFlag, "bw_INPUT", bytes, alertName);
@@ -852,19 +802,8 @@
}
int BandwidthController::runIptablesAlertFwdCmd(IptOp op, const char *alertName, int64_t bytes) {
- const char *opFlag;
+ const char *opFlag = opToString(op);
std::string alertQuotaCmd = "*filter\n";
-
- switch (op) {
- case IptOpInsert:
- opFlag = "-I";
- break;
- default:
- case IptOpDelete:
- opFlag = "-D";
- break;
- }
-
StringAppendF(&alertQuotaCmd, ALERT_IPT_TEMPLATE, opFlag, "bw_FORWARD", bytes, alertName);
StringAppendF(&alertQuotaCmd, "COMMIT\n");
@@ -1296,3 +1235,28 @@
clearCommands.push_back("COMMIT\n");
iptablesRestoreFunction(V4V6, android::base::Join(clearCommands, '\n'), nullptr);
}
+
+inline const char *BandwidthController::opToString(IptOp op) {
+ switch (op) {
+ case IptOpInsert:
+ return "-I";
+ case IptOpDelete:
+ return "-D";
+ }
+}
+
+inline const char *BandwidthController::jumpToString(IptJumpOp jumpHandling) {
+ /*
+ * Must be careful what one rejects with, as upper layer protocols will just
+ * keep on hammering the device until the number of retries are done.
+ * For port-unreachable (default), TCP should consider as an abort (RFC1122).
+ */
+ switch (jumpHandling) {
+ case IptJumpNoAdd:
+ return "";
+ case IptJumpReject:
+ return " --jump REJECT";
+ case IptJumpReturn:
+ return " --jump RETURN";
+ }
+}
diff --git a/server/BandwidthController.h b/server/BandwidthController.h
index 0398ce9..0a51346 100644
--- a/server/BandwidthController.h
+++ b/server/BandwidthController.h
@@ -124,9 +124,9 @@
};
enum IptIpVer { IptIpV4, IptIpV6 };
- enum IptOp { IptOpInsert, IptOpReplace, IptOpDelete, IptOpAppend };
+ enum IptFullOp { IptFullOpInsert, IptFullOpDelete, IptFullOpAppend };
enum IptJumpOp { IptJumpReject, IptJumpReturn, IptJumpNoAdd };
- enum SpecialAppOp { SpecialAppOpAdd, SpecialAppOpRemove };
+ enum IptOp { IptOpInsert, IptOpDelete };
enum QuotaType { QuotaUnique, QuotaShared };
enum RunCmdErrHandling { RunCmdFailureBad, RunCmdFailureOk };
#if LOG_NDEBUG
@@ -137,15 +137,15 @@
int manipulateSpecialApps(int numUids, char *appStrUids[],
const char *chain,
- IptJumpOp jumpHandling, SpecialAppOp appOp);
- int manipulateNaughtyApps(int numUids, char *appStrUids[], SpecialAppOp appOp);
- int manipulateNiceApps(int numUids, char *appStrUids[], SpecialAppOp appOp);
+ IptJumpOp jumpHandling, IptOp appOp);
+ int manipulateNaughtyApps(int numUids, char *appStrUids[], IptOp appOp);
+ int manipulateNiceApps(int numUids, char *appStrUids[], IptOp appOp);
int prepCostlyIface(const char *ifn, QuotaType quotaType);
int cleanupCostlyIface(const char *ifn, QuotaType quotaType);
std::string makeIptablesSpecialAppCmd(IptOp op, int uid, const char *chain);
- std::string makeIptablesQuotaCmd(IptOp op, const char *costName, int64_t quota);
+ std::string makeIptablesQuotaCmd(IptFullOp op, const char *costName, int64_t quota);
int runIptablesAlertCmd(IptOp op, const char *alertName, int64_t bytes);
int runIptablesAlertFwdCmd(IptOp op, const char *alertName, int64_t bytes);
@@ -223,6 +223,10 @@
static int (*execFunction)(int, char **, int *, bool, bool);
static FILE *(*popenFunction)(const char *, const char *);
static int (*iptablesRestoreFunction)(IptablesTarget, const std::string&, std::string *);
+
+private:
+ static const char *opToString(IptOp op);
+ static const char *jumpToString(IptJumpOp jumpHandling);
};
#endif
