When un-natting try to do all we canandroid-4.0.1_r1.2android-4.0.1_r1.1android-4.0.1_r1ics-factoryrom-2-release

Regardless of errors we should try to do as much as possible.  Sometimes
some steps may fail if interfaces are taken down before we can un-nat them.

bug:5536516
Change-Id: I9c9b0123198dba890565e0a6e4e15add16b369c2

2 files changed, 10 insertions, 7 deletions

diff --git a/CommandListener.cpp b/CommandListener.cpp
index 21efaddc..0d14d133 100644
--- a/CommandListener.cpp
+++ b/CommandListener.cpp
@@ -588,10 +588,8 @@ int CommandListener::NatCmd::runCommand(SocketClient *cli,
         }
     } else if (!strcmp(argv[1], "disable")) {
         rc = sNatCtrl->disableNat(argv[2], argv[3]);
-        if(!rc) {
-            /* Ignore ifaces for now. */
-            rc = sBandwidthCtrl->removeGlobalAlertInForwardChain();
-        }
+        /* Ignore ifaces for now. */
+        rc |= sBandwidthCtrl->removeGlobalAlertInForwardChain();
     } else {
         cli->sendMsg(ResponseCode::CommandSyntaxError, "Unknown nat cmd", false);
         return 0;
diff --git a/NatController.cpp b/NatController.cpp
index c05aa7b0..49984f8b 100644
--- a/NatController.cpp
+++ b/NatController.cpp
@@ -77,6 +77,8 @@ bool NatController::interfaceExists(const char *iface) {
     return true;
 }
 
+// when un-doing NAT, we should report errors, but also try to do as much cleanup
+// as we can - don't short circuit on error.
 int NatController::doNatCommands(const char *intIface, const char *extIface, bool add) {
     char cmd[255];
 
@@ -90,6 +92,7 @@ int NatController::doNatCommands(const char *intIface, const char *extIface, boo
                 if (ret == 0) {
                     natCount=0;
                 }
+                LOGE("setDefaults returned %d", ret);
                 return ret;
             }
         }
@@ -105,7 +108,8 @@ int NatController::doNatCommands(const char *intIface, const char *extIface, boo
              "-%s FORWARD -i %s -o %s -m state --state ESTABLISHED,RELATED -j ACCEPT",
              (add ? "A" : "D"),
              extIface, intIface);
-    if (runIptablesCmd(cmd)) {
+    if (runIptablesCmd(cmd) && add) {
+        // only bail out if we are adding, not removing nat rules
         return -1;
     }
 
@@ -113,7 +117,8 @@ int NatController::doNatCommands(const char *intIface, const char *extIface, boo
             "-%s FORWARD -i %s -o %s -m state --state INVALID -j DROP",
             (add ? "A" : "D"),
             intIface, extIface);
-    if (runIptablesCmd(cmd)) {
+    if (runIptablesCmd(cmd) && add) {
+        // bail on error, but only if adding
         snprintf(cmd, sizeof(cmd),
                 "-%s FORWARD -i %s -o %s -m state --state ESTABLISHED,RELATED -j ACCEPT",
                 (!add ? "A" : "D"),
@@ -124,7 +129,7 @@ int NatController::doNatCommands(const char *intIface, const char *extIface, boo
 
     snprintf(cmd, sizeof(cmd), "-%s FORWARD -i %s -o %s -j ACCEPT", (add ? "A" : "D"),
             intIface, extIface);
-    if (runIptablesCmd(cmd)) {
+    if (runIptablesCmd(cmd) && add) {
         // unwind what's been done, but don't care about success - what more could we do?
         snprintf(cmd, sizeof(cmd),
                 "-%s FORWARD -i %s -o %s -m state --state INVALID -j DROP",