Keep better tabs on secondary tables.
We had some places (NatController) where routes were being set
but not accounted for in the number-of-routes talley so we
could end up thinking the table was empty and not clean up
after ourselves properly.
Also consolidated constants.
bug:5917475
Change-Id: I98a41d433e1d4b4ca6692fb2328e2c9afc828145
diff --git a/Android.mk b/Android.mk
index 4e5bd5b..612a2cf 100644
--- a/Android.mk
+++ b/Android.mk
@@ -8,6 +8,7 @@
DnsProxyListener.cpp \
NatController.cpp \
NetdCommand.cpp \
+ NetdConstants.cpp \
NetlinkHandler.cpp \
NetlinkManager.cpp \
PanController.cpp \
diff --git a/NatController.cpp b/NatController.cpp
index ef1f343..bdbc429 100644
--- a/NatController.cpp
+++ b/NatController.cpp
@@ -30,12 +30,10 @@
#include "NatController.h"
#include "SecondaryTableController.h"
#include "oem_iptables_hook.h"
+#include "NetdConstants.h"
extern "C" int system_nosh(const char *command);
-static char IPTABLES_PATH[] = "/system/bin/iptables";
-static char IP_PATH[] = "/system/bin/ip";
-
NatController::NatController(SecondaryTableController *ctrl) {
secondaryTableCtrl = ctrl;
setDefaults();
@@ -93,14 +91,6 @@
return true;
}
-const char *NatController::getVersion(const char *addr) {
- if (strchr(addr, ':') != NULL) {
- return "-6";
- } else {
- return "-4";
- }
-}
-
// 0 1 2 3 4 5
// nat enable intface extface addrcnt nated-ipaddr/prelength
int NatController::enableNat(const int argc, char **argv) {
@@ -126,16 +116,10 @@
tableNumber = secondaryTableCtrl->findTableNumber(extIface);
if (tableNumber != -1) {
- for(i = 0; i < addrCount && ret == 0; i++) {
- snprintf(cmd, sizeof(cmd), "%s rule add from %s table %d", getVersion(argv[5+i]),
- argv[5+i], tableNumber + BASE_TABLE_NUMBER);
- ret |= runCmd(IP_PATH, cmd);
- if (ret) ALOGE("IP rule %s got %d", cmd, ret);
+ for(i = 0; i < addrCount; i++) {
+ ret |= secondaryTableCtrl->modifyFromRule(tableNumber, ADD, argv[5+i]);
- snprintf(cmd, sizeof(cmd), "route add %s dev %s table %d", argv[5+i], intIface,
- tableNumber + BASE_TABLE_NUMBER);
- ret |= runCmd(IP_PATH, cmd);
- if (ret) ALOGE("IP route %s got %d", cmd, ret);
+ ret |= secondaryTableCtrl->modifyLocalRoute(tableNumber, ADD, intIface, argv[5+i]);
}
runCmd(IP_PATH, "route flush cache");
}
@@ -143,13 +127,9 @@
if (ret != 0 || setForwardRules(true, intIface, extIface) != 0) {
if (tableNumber != -1) {
for (i = 0; i < addrCount; i++) {
- snprintf(cmd, sizeof(cmd), "route del %s dev %s table %d", argv[5+i], intIface,
- tableNumber + BASE_TABLE_NUMBER);
- runCmd(IP_PATH, cmd);
+ secondaryTableCtrl->modifyLocalRoute(tableNumber, DEL, intIface, argv[5+i]);
- snprintf(cmd, sizeof(cmd), "%s rule del from %s table %d", getVersion(argv[5+i]),
- argv[5+i], tableNumber + BASE_TABLE_NUMBER);
- runCmd(IP_PATH, cmd);
+ secondaryTableCtrl->modifyFromRule(tableNumber, DEL, argv[5+i]);
}
runCmd(IP_PATH, "route flush cache");
}
@@ -166,9 +146,9 @@
ALOGE("Error seting postroute rule: %s", cmd);
// unwind what's been done, but don't care about success - what more could we do?
for (i = 0; i < addrCount; i++) {
- snprintf(cmd, sizeof(cmd), "route del %s dev %s table %d", argv[5+i], intIface,
- tableNumber + BASE_TABLE_NUMBER);
- runCmd(IP_PATH, cmd);
+ secondaryTableCtrl->modifyLocalRoute(tableNumber, DEL, intIface, argv[5+i]);
+
+ secondaryTableCtrl->modifyFromRule(tableNumber, DEL, argv[5+i]);
}
setDefaults();
return -1;
@@ -251,15 +231,9 @@
tableNumber = secondaryTableCtrl->findTableNumber(extIface);
if (tableNumber != -1) {
for (i = 0; i < addrCount; i++) {
- snprintf(cmd, sizeof(cmd), "route del %s dev %s table %d", argv[5+i], intIface,
- tableNumber + BASE_TABLE_NUMBER);
- // if the interface has gone down these will be gone already and give errors
- // ignore them.
- runCmd(IP_PATH, cmd);
+ secondaryTableCtrl->modifyLocalRoute(tableNumber, DEL, intIface, argv[5+i]);
- snprintf(cmd, sizeof(cmd), "%s rule del from %s table %d", getVersion(argv[5+i]),
- argv[5+i], tableNumber + BASE_TABLE_NUMBER);
- runCmd(IP_PATH, cmd);
+ secondaryTableCtrl->modifyFromRule(tableNumber, DEL, argv[5+i]);
}
runCmd(IP_PATH, "route flush cache");
diff --git a/NatController.h b/NatController.h
index eae32b4..d10cbd9 100644
--- a/NatController.h
+++ b/NatController.h
@@ -40,7 +40,6 @@
int runCmd(const char *path, const char *cmd);
bool checkInterface(const char *iface);
int setForwardRules(bool set, const char *intIface, const char *extIface);
- const char *getVersion(const char *addr);
};
#endif
diff --git a/NetdConstants.cpp b/NetdConstants.cpp
new file mode 100644
index 0000000..bac3cbb
--- /dev/null
+++ b/NetdConstants.cpp
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2012 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.
+ */
+
+#include "NetdConstants.h"
+
+const char * const OEM_SCRIPT_PATH = "/system/bin/oem-iptables-init.sh";
+const char * const IPTABLES_PATH = "/system/bin/iptables";
+const char * const TC_PATH = "/system/bin/tc";
+const char * const IP_PATH = "/system/bin/ip";
+const char * const ADD = "add";
+const char * const DEL = "del";
diff --git a/NetdConstants.h b/NetdConstants.h
new file mode 100644
index 0000000..d92c9e4
--- /dev/null
+++ b/NetdConstants.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2012 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.
+ */
+
+#ifndef _NETD_CONSTANTS_H
+#define _NETD_CONSTANTS_H
+
+
+extern const char * const IPTABLES_PATH;
+extern const char * const IP_PATH;
+extern const char * const TC_PATH;
+extern const char * const OEM_SCRIPT_PATH;
+extern const char * const ADD;
+extern const char * const DEL;
+
+#endif
diff --git a/SecondaryTableController.cpp b/SecondaryTableController.cpp
index 5d79f86..5939924 100644
--- a/SecondaryTableController.cpp
+++ b/SecondaryTableController.cpp
@@ -34,12 +34,9 @@
extern "C" int system_nosh(const char *command);
#include "ResponseCode.h"
+#include "NetdConstants.h"
#include "SecondaryTableController.h"
-static char IP_PATH[] = "/system/bin/ip";
-static char ADD[] = "add";
-static char DEL[] = "del";
-
SecondaryTableController::SecondaryTableController() {
int i;
for (i=0; i < INTERFACES_TRACKED; i++) {
@@ -79,8 +76,8 @@
return modifyRoute(cli, ADD, iface, dest, prefix, gateway, tableIndex);
}
-int SecondaryTableController::modifyRoute(SocketClient *cli, char *action, char *iface, char *dest,
- int prefix, char *gateway, int tableIndex) {
+int SecondaryTableController::modifyRoute(SocketClient *cli, const char *action, char *iface,
+ char *dest, int prefix, char *gateway, int tableIndex) {
char *cmd;
if (strcmp("::", gateway) == 0) {
@@ -108,10 +105,40 @@
mInterfaceTable[tableIndex][0] = 0;
}
}
+ modifyRuleCount(tableIndex, action);
cli->sendMsg(ResponseCode::CommandOkay, "Route modified", false);
return 0;
}
+void SecondaryTableController::modifyRuleCount(int tableIndex, const char *action) {
+ if (strcmp(action, ADD) == 0) {
+ mInterfaceRuleCount[tableIndex]++;
+ } else {
+ if (--mInterfaceRuleCount[tableIndex] < 1) {
+ mInterfaceRuleCount[tableIndex] = 0;
+ mInterfaceTable[tableIndex][0] = 0;
+ }
+ }
+}
+
+int SecondaryTableController::verifyTableIndex(int tableIndex) {
+ if ((tableIndex < 0) ||
+ (tableIndex >= INTERFACES_TRACKED) ||
+ (mInterfaceTable[tableIndex][0] == 0)) {
+ return -1;
+ } else {
+ return 0;
+ }
+}
+
+const char *SecondaryTableController::getVersion(const char *addr) {
+ if (strchr(addr, ':') != NULL) {
+ return "-6";
+ } else {
+ return "-4";
+ }
+}
+
int SecondaryTableController::removeRoute(SocketClient *cli, char *iface, char *dest, int prefix,
char *gateway) {
int tableIndex = findTableNumber(iface);
@@ -125,12 +152,46 @@
return modifyRoute(cli, DEL, iface, dest, prefix, gateway, tableIndex);
}
+int SecondaryTableController::modifyFromRule(int tableIndex, const char *action,
+ const char *addr) {
+ char *cmd;
+
+ if (verifyTableIndex(tableIndex)) {
+ return -1;
+ }
+ asprintf(&cmd, "%s %s rule %s from %s table %d", IP_PATH, getVersion(addr),
+ action, addr, tableIndex + BASE_TABLE_NUMBER);
+ if (runAndFree(NULL, cmd)) {
+ return -1;
+ }
+
+ modifyRuleCount(tableIndex, action);
+ return 0;
+}
+
+int SecondaryTableController::modifyLocalRoute(int tableIndex, const char *action,
+ const char *iface, const char *addr) {
+ char *cmd;
+
+ if (verifyTableIndex(tableIndex)) {
+ return -1;
+ }
+
+ modifyRuleCount(tableIndex, action); // some del's will fail as the iface is already gone.
+
+ asprintf(&cmd, "%s route %s %s dev %s table %d", IP_PATH, action, addr, iface,
+ tableIndex+BASE_TABLE_NUMBER);
+ return runAndFree(NULL, cmd);
+}
+
int SecondaryTableController::runAndFree(SocketClient *cli, char *cmd) {
int ret = 0;
if (strlen(cmd) >= 255) {
- ALOGE("ip command (%s) too long", cmd);
- errno = E2BIG;
- cli->sendMsg(ResponseCode::CommandSyntaxError, "Too long", true);
+ if (cli != NULL) {
+ ALOGE("ip command (%s) too long", cmd);
+ errno = E2BIG;
+ cli->sendMsg(ResponseCode::CommandSyntaxError, "Too long", true);
+ }
free(cmd);
return -1;
}
diff --git a/SecondaryTableController.h b/SecondaryTableController.h
index 0051f7f..259ab43 100644
--- a/SecondaryTableController.h
+++ b/SecondaryTableController.h
@@ -33,13 +33,18 @@
int addRoute(SocketClient *cli, char *iface, char *dest, int prefixLen, char *gateway);
int removeRoute(SocketClient *cli, char *iface, char *dest, int prefixLen, char *gateway);
int findTableNumber(const char *iface);
+ int modifyFromRule(int tableIndex, const char *action, const char *addr);
+ int modifyLocalRoute(int tableIndex, const char *action, const char *iface, const char *addr);
private:
- int modifyRoute(SocketClient *cli, char *action, char *iface, char *dest, int prefix,
+ int modifyRoute(SocketClient *cli, const char *action, char *iface, char *dest, int prefix,
char *gateway, int tableIndex);
char mInterfaceTable[INTERFACES_TRACKED][MAX_IFACE_LENGTH];
int mInterfaceRuleCount[INTERFACES_TRACKED];
+ void modifyRuleCount(int tableIndex, const char *action);
+ int verifyTableIndex(int tableIndex);
+ const char *getVersion(const char *addr);
int runAndFree(SocketClient *cli, char *cmd);
};
diff --git a/ThrottleController.cpp b/ThrottleController.cpp
index b4720b5..5bd7ad5 100644
--- a/ThrottleController.cpp
+++ b/ThrottleController.cpp
@@ -33,8 +33,7 @@
#include "ThrottleController.h"
-
-static char TC_PATH[] = "/system/bin/tc";
+#include "NetdConstants.h"
extern "C" int system_nosh(const char *command);
extern "C" int ifc_init(void);
diff --git a/oem_iptables_hook.cpp b/oem_iptables_hook.cpp
index 321c5f4..d3026a9 100644
--- a/oem_iptables_hook.cpp
+++ b/oem_iptables_hook.cpp
@@ -24,11 +24,10 @@
#define LOG_TAG "OemIptablesHook"
#include <cutils/log.h>
+#include "NetdConstants.h"
extern "C" int system_nosh(const char *command);
-static char IPTABLES_PATH[] = "/system/bin/iptables";
-static char OEM_SCRIPT_PATH[] = "/system/bin/oem-iptables-init.sh";
static int runIptablesCmd(const char *cmd) {
char *buffer;