From 4ae80dea9cbf1fe1b33037aeb5feb04daeba8ee0 Mon Sep 17 00:00:00 2001 From: JP Abgrall Date: Thu, 14 Mar 2013 20:06:20 -0700 Subject: [PATCH] NatController: refactor some code wrt sequences of commands This will help when adding/removing commands. Change-Id: I154fb3d7064acddc3e067d60f225ecab6ea57ddb --- NatController.cpp | 198 ++++++++++++++---------------------------------------- NatController.h | 1 + 2 files changed, 52 insertions(+), 147 deletions(-) diff --git a/NatController.cpp b/NatController.cpp index ff35d89..46ac01c 100644 --- a/NatController.cpp +++ b/NatController.cpp @@ -45,6 +45,12 @@ NatController::NatController(SecondaryTableController *ctrl) { NatController::~NatController() { } +struct CommandsAndArgs { + /* The array size doesn't really matter as the compiler will barf if too many initializers are specified. */ + const char *cmd[32]; + bool checkRes; +}; + int NatController::runCmd(int argc, const char **argv) { int res; @@ -59,100 +65,23 @@ int NatController::setupIptablesHooks() { } int NatController::setDefaults() { - const char *cmd1[] = { - IPTABLES_PATH, - "-F", - "natctrl_FORWARD" - }; - if (runCmd(ARRAY_SIZE(cmd1), cmd1)) - return -1; - - const char *cmd2[] = { - IPTABLES_PATH, - "-t", - "nat", - "-F", - "natctrl_nat_POSTROUTING" - }; - if (runCmd(ARRAY_SIZE(cmd2), cmd2)) - return -1; - - const char *cmd3[] = { - IP_PATH, - "rule", - "flush" - }; - runCmd(ARRAY_SIZE(cmd3), cmd3); - - const char *cmd4[] = { - IP_PATH, - "-6", - "rule", - "flush" - }; - runCmd(ARRAY_SIZE(cmd4), cmd4); - - const char *cmd5[] = { - IP_PATH, - "rule", - "add", - "from", - "all", - "lookup", - "default", - "prio", - "32767" - }; - runCmd(ARRAY_SIZE(cmd5), cmd5); - - const char *cmd6[] = { - IP_PATH, - "rule", - "add", - "from", - "all", - "lookup", - "main", - "prio", - "32766" - }; - runCmd(ARRAY_SIZE(cmd6), cmd6); - - const char *cmd7[] = { - IP_PATH, - "-6", - "rule", - "add", - "from", - "all", - "lookup", - "default", - "prio", - "32767" - }; - runCmd(ARRAY_SIZE(cmd7), cmd7); - - const char *cmd8[] = { - IP_PATH, - "-6", - "rule", - "add", - "from", - "all", - "lookup", - "main", - "prio", - "32766" + struct CommandsAndArgs defaultCommands[] = { + {{IPTABLES_PATH, "-F", "natctrl_FORWARD",}, 1}, + {{IPTABLES_PATH, "-t", "nat", "-F", "natctrl_nat_POSTROUTING"}, 1}, + {{IP_PATH, "rule", "flush"}, 0}, + {{IP_PATH, "-6", "rule", "flush"}, 0}, + {{IP_PATH, "rule", "add", "from", "all", "lookup", "default", "prio", "32767"}, 0}, + {{IP_PATH, "rule", "add", "from", "all", "lookup", "main", "prio", "32766"}, 0}, + {{IP_PATH, "-6", "rule", "add", "from", "all", "lookup", "default", "prio", "32767"}, 0}, + {{IP_PATH, "-6", "rule", "add", "from", "all", "lookup", "main", "prio", "32766"}, 0}, + {{IP_PATH, "route", "flush", "cache"}, 0}, }; - runCmd(ARRAY_SIZE(cmd8), cmd8); - - const char *cmd9[] = { - IP_PATH, - "route", - "flush", - "cache" - }; - runCmd(ARRAY_SIZE(cmd9), cmd9); + for (unsigned int cmdNum = 0; cmdNum < ARRAY_SIZE(defaultCommands); cmdNum++) { + if (runCmd(ARRAY_SIZE(defaultCommands[cmdNum].cmd), defaultCommands[cmdNum].cmd) && + defaultCommands[cmdNum].checkRes) { + return -1; + } + } natCount = 0; @@ -164,6 +93,31 @@ bool NatController::checkInterface(const char *iface) { return true; } +int NatController::routesOp(bool add, const char *intIface, const char *extIface, char **argv, int addrCount) { + int tableNumber = secondaryTableCtrl->findTableNumber(extIface); + int ret = 0; + + if (tableNumber != -1) { + for (int i = 0; i < addrCount; i++) { + if (add) { + ret |= secondaryTableCtrl->modifyFromRule(tableNumber, ADD, argv[5+i]); + ret |= secondaryTableCtrl->modifyLocalRoute(tableNumber, ADD, intIface, argv[5+i]); + } else { + ret |= secondaryTableCtrl->modifyLocalRoute(tableNumber, DEL, intIface, argv[5+i]); + ret |= secondaryTableCtrl->modifyFromRule(tableNumber, DEL, argv[5+i]); + } + } + const char *cmd[] = { + IP_PATH, + "route", + "flush", + "cache" + }; + runCmd(ARRAY_SIZE(cmd), cmd); + } + return ret; +} + // 0 1 2 3 4 5 // nat enable intface extface addrcnt nated-ipaddr/prelength int NatController::enableNat(const int argc, char **argv) { @@ -185,39 +139,10 @@ int NatController::enableNat(const int argc, char **argv) { errno = EINVAL; return -1; } - - tableNumber = secondaryTableCtrl->findTableNumber(extIface); - if (tableNumber != -1) { - for(i = 0; i < addrCount; i++) { - ret |= secondaryTableCtrl->modifyFromRule(tableNumber, ADD, argv[5+i]); - - ret |= secondaryTableCtrl->modifyLocalRoute(tableNumber, ADD, intIface, argv[5+i]); - } - const char *cmd[] = { - IP_PATH, - "route", - "flush", - "cache" - }; - runCmd(ARRAY_SIZE(cmd), cmd); - } - + ret = routesOp(true, intIface, extIface, argv, addrCount); if (ret != 0 || setForwardRules(true, intIface, extIface) != 0) { - if (tableNumber != -1) { - for (i = 0; i < addrCount; i++) { - secondaryTableCtrl->modifyLocalRoute(tableNumber, DEL, intIface, argv[5+i]); - - secondaryTableCtrl->modifyFromRule(tableNumber, DEL, argv[5+i]); - } - const char *cmd[] = { - IP_PATH, - "route", - "flush", - "cache" - }; - runCmd(ARRAY_SIZE(cmd), cmd); - } ALOGE("Error setting forward rules"); + routesOp(false, intIface, extIface, argv, addrCount); errno = ENODEV; return -1; } @@ -258,11 +183,7 @@ int NatController::enableNat(const int argc, char **argv) { if (runCmd(ARRAY_SIZE(cmd), cmd)) { ALOGE("Error seting postroute rule: iface=%s", extIface); // unwind what's been done, but don't care about success - what more could we do? - for (i = 0; i < addrCount; i++) { - secondaryTableCtrl->modifyLocalRoute(tableNumber, DEL, intIface, argv[5+i]); - - secondaryTableCtrl->modifyFromRule(tableNumber, DEL, argv[5+i]); - } + routesOp(false, intIface, extIface, argv, addrCount); setDefaults(); return -1; } @@ -367,24 +288,7 @@ int NatController::disableNat(const int argc, char **argv) { } setForwardRules(false, intIface, extIface); - - tableNumber = secondaryTableCtrl->findTableNumber(extIface); - if (tableNumber != -1) { - for (i = 0; i < addrCount; i++) { - secondaryTableCtrl->modifyLocalRoute(tableNumber, DEL, intIface, argv[5+i]); - - secondaryTableCtrl->modifyFromRule(tableNumber, DEL, argv[5+i]); - } - - const char *cmd[] = { - IP_PATH, - "route", - "flush", - "cache" - }; - runCmd(ARRAY_SIZE(cmd), cmd); - } - + routesOp(false, intIface, extIface, argv, addrCount); if (--natCount <= 0) { // handle decrement to 0 case (do reset to defaults) and erroneous dec below 0 setDefaults(); diff --git a/NatController.h b/NatController.h index 4330567..ba7daaa 100644 --- a/NatController.h +++ b/NatController.h @@ -42,6 +42,7 @@ private: int runCmd(int argc, const char **argv); bool checkInterface(const char *iface); int setForwardRules(bool set, const char *intIface, const char *extIface); + int routesOp(bool add, const char *intIface, const char *extIface, char **argv, int addrCount); }; #endif -- 2.11.0