netd: Idletimer vs Nat vs Bandwidth controllers

* modified iptables users to work in controller specific custom chains.
 - each controller only works withing his own custom chains and not the
  top level ones (INPUT, OUTPUT, FORWARD, POSTROUTING,...)
 - CommandListener now invokes setupIptablesHooks() for each controller
   once. That is the only time they are allowed to access the top-level
   chains.

* Added idletimer controller.
  From https://android-git.corp.google.com/g/#/c/180769/2
 - supported commands
   . ndc idletimer enable
   . ndc idletimer add <iface> <timeout>
   . ndc idletimer remove <iface> <timeout_used_during_add>
 There is a framework change elsewhere that receives netlink messages.

Signed-off-by: Ashish Sharma <ashishsharma@google.com>
Signed-off-by: JP Abgrall <jpa@google.com>
Change-Id: Ia57450c09166ce20f21d1e3b49047ef1e98f2a3d
diff --git a/Android.mk b/Android.mk
index 76f395d..655038d 100644
--- a/Android.mk
+++ b/Android.mk
@@ -7,6 +7,7 @@
                   CommandListener.cpp                  \
                   DnsProxyListener.cpp                 \
                   MDnsSdListener.cpp                   \
+                  IdletimerController.cpp              \
                   NatController.cpp                    \
                   NetdCommand.cpp                      \
                   NetdConstants.cpp                    \
diff --git a/BandwidthController.cpp b/BandwidthController.cpp
index fa94d30..5233f3e 100644
--- a/BandwidthController.cpp
+++ b/BandwidthController.cpp
@@ -44,15 +44,13 @@
 extern "C" int logwrap(int argc, const char **argv);
 extern "C" int system_nosh(const char *command);
 
+#include "NetdConstants.h"
 #include "BandwidthController.h"
-#include "oem_iptables_hook.h"
 
 /* Alphabetical */
 #define ALERT_IPT_TEMPLATE "%s %s %s -m quota2 ! --quota %lld --name %s"
 const int  BandwidthController::ALERT_RULE_POS_IN_COSTLY_CHAIN = 4;
 const char BandwidthController::ALERT_GLOBAL_NAME[] = "globalAlert";
-const char BandwidthController::IP6TABLES_PATH[] = "/system/bin/ip6tables";
-const char BandwidthController::IPTABLES_PATH[] = "/system/bin/iptables";
 const int  BandwidthController::MAX_CMD_ARGS = 32;
 const int  BandwidthController::MAX_CMD_LEN = 1024;
 const int  BandwidthController::MAX_IFACENAME_LEN = 64;
@@ -64,29 +62,29 @@
  * Some comments about the rules:
  *  * Ordering
  *    - when an interface is marked as costly it should be INSERTED into the INPUT/OUTPUT chains.
- *      E.g. "-I INPUT -i rmnet0 --goto costly"
+ *      E.g. "-I INPUT -i rmnet0 --jump costly"
  *    - quota'd rules in the costly chain should be before penalty_box lookups.
  *
  * * global quota vs per interface quota
  *   - global quota for all costly interfaces uses a single costly chain:
  *    . initial rules
  *      iptables -N costly_shared
- *      iptables -I INPUT -i iface0 --goto costly_shared
- *      iptables -I OUTPUT -o iface0 --goto costly_shared
+ *      iptables -I INPUT -i iface0 --jump costly_shared
+ *      iptables -I OUTPUT -o iface0 --jump costly_shared
  *      iptables -I costly_shared -m quota \! --quota 500000 \
  *          --jump REJECT --reject-with icmp-net-prohibited
  *      iptables -A costly_shared --jump penalty_box
  *      iptables -A costly_shared -m owner --socket-exists
  *
  *    . adding a new iface to this, E.g.:
- *      iptables -I INPUT -i iface1 --goto costly_shared
- *      iptables -I OUTPUT -o iface1 --goto costly_shared
+ *      iptables -I INPUT -i iface1 --jump costly_shared
+ *      iptables -I OUTPUT -o iface1 --jump costly_shared
  *
  *   - quota per interface. This is achieve by having "costly" chains per quota.
  *     E.g. adding a new costly interface iface0 with its own quota:
  *      iptables -N costly_iface0
- *      iptables -I INPUT -i iface0 --goto costly_iface0
- *      iptables -I OUTPUT -o iface0 --goto costly_iface0
+ *      iptables -I INPUT -i iface0 --jump costly_iface0
+ *      iptables -I OUTPUT -o iface0 --jump costly_iface0
  *      iptables -A costly_iface0 -m quota \! --quota 500000 \
  *          --jump REJECT --reject-with icmp-net-prohibited
  *      iptables -A costly_iface0 --jump penalty_box
@@ -98,48 +96,61 @@
  *    iptables -A penalty_box -m owner --uid-owner app_3 \
  *        --jump REJECT --reject-with icmp-net-prohibited
  */
-const char *BandwidthController::IPT_CLEANUP_COMMANDS[] = {
-    /* Cleanup rules. */
-    "-F",
-    "-t raw -F",
-    /* TODO: If at some point we need more user chains than here, then we will need
-     * a different cleanup approach.
+const char *BandwidthController::IPT_FLUSH_COMMANDS[] = {
+    /*
+     * Cleanup rules.
+     * Should normally include costly_<iface>, but we rely on the way they are setup
+     * to allow coexistance.
      */
-    "-X",  /* Should normally only be costly_shared, penalty_box, and costly_<iface>  */
+    "-F bw_INPUT",
+    "-F bw_OUTPUT",
+    "-F bw_FORWARD",
+    "-F penalty_box",
+    "-F costly_shared",
+};
+
+/* The cleanup commands assume flushing has been done. */
+const char *BandwidthController::IPT_CLEANUP_COMMANDS[] = {
+    /* Delete hooks to custom chains. */
+    "-D INPUT -j bw_INPUT",
+    "-D OUTPUT -j bw_OUTPUT",
+    "-D FORWARD -j bw_FORWARD",
+    "-X bw_INPUT",
+    "-X bw_OUTPUT",
+    "-X bw_FORWARD",
+    "-X penalty_box",
+    "-X costly_shared",
 };
 
 const char *BandwidthController::IPT_SETUP_COMMANDS[] = {
     /* Created needed chains. */
+    "-N bw_INPUT",
+    "-A INPUT -j bw_INPUT",
+
+    "-N bw_OUTPUT",
+    "-A OUTPUT -j bw_OUTPUT",
+
+    "-N bw_FORWARD",
+    "-I FORWARD -j bw_FORWARD",
+
     "-N costly_shared",
     "-N penalty_box",
 };
 
 const char *BandwidthController::IPT_BASIC_ACCOUNTING_COMMANDS[] = {
-    "-F INPUT",
-    "-A INPUT -i lo --jump ACCEPT",
-    "-A INPUT -m owner --socket-exists", /* This is a tracking rule. */
+    "-A bw_INPUT -i lo --jump RETURN",
+    "-A bw_INPUT -m owner --socket-exists", /* This is a tracking rule. */
 
-    "-F OUTPUT",
-    "-A OUTPUT -o lo --jump ACCEPT",
-    "-A OUTPUT -m owner --socket-exists", /* This is a tracking rule. */
+    "-A bw_OUTPUT -o lo --jump RETURN",
+    "-A bw_OUTPUT -m owner --socket-exists", /* This is a tracking rule. */
 
-    "-F costly_shared",
     "-A costly_shared --jump penalty_box",
     "-A costly_shared -m owner --socket-exists", /* This is a tracking rule. */
-    /* TODO(jpa): Figure out why iptables doesn't correctly return from this
-     * chain. For now, hack the chain exit with an ACCEPT.
-     */
-    "-A costly_shared --jump ACCEPT",
 };
 
 BandwidthController::BandwidthController(void) {
     char value[PROPERTY_VALUE_MAX];
 
-    property_get("persist.bandwidth.enable", value, "0");
-    if (!strcmp(value, "1")) {
-        enableBandwidthControl();
-    }
-
     property_get("persist.bandwidth.uselogwrap", value, "0");
     useLogwrapCall = !strcmp(value, "1");
 }
@@ -212,8 +223,31 @@
     return res;
 }
 
-int BandwidthController::enableBandwidthControl(void) {
+int BandwidthController::setupIptablesHooks(void) {
+
+    /* Some of the initialCommands are allowed to fail */
+    runCommands(sizeof(IPT_FLUSH_COMMANDS) / sizeof(char*),
+            IPT_FLUSH_COMMANDS, RunCmdFailureOk);
+
+    runCommands(sizeof(IPT_CLEANUP_COMMANDS) / sizeof(char*),
+            IPT_CLEANUP_COMMANDS, RunCmdFailureOk);
+
+    runCommands(sizeof(IPT_SETUP_COMMANDS) / sizeof(char*),
+            IPT_SETUP_COMMANDS, RunCmdFailureBad);
+
+    return 0;
+
+}
+
+int BandwidthController::enableBandwidthControl(bool force) {
     int res;
+    char value[PROPERTY_VALUE_MAX];
+
+    if (!force) {
+            property_get("persist.bandwidth.enable", value, "1");
+            if (!strcmp(value, "0"))
+                    return 0;
+    }
 
     /* Let's pretend we started from scratch ... */
     sharedQuotaIfaces.clear();
@@ -223,26 +257,19 @@
     globalAlertTetherCount = 0;
     sharedQuotaBytes = sharedAlertBytes = 0;
 
+    res = runCommands(sizeof(IPT_FLUSH_COMMANDS) / sizeof(char*),
+            IPT_FLUSH_COMMANDS, RunCmdFailureOk);
 
-    /* Some of the initialCommands are allowed to fail */
-    runCommands(sizeof(IPT_CLEANUP_COMMANDS) / sizeof(char*),
-            IPT_CLEANUP_COMMANDS, RunCmdFailureOk);
-    runCommands(sizeof(IPT_SETUP_COMMANDS) / sizeof(char*),
-            IPT_SETUP_COMMANDS, RunCmdFailureOk);
-    res = runCommands(sizeof(IPT_BASIC_ACCOUNTING_COMMANDS) / sizeof(char*),
+    res |= runCommands(sizeof(IPT_BASIC_ACCOUNTING_COMMANDS) / sizeof(char*),
             IPT_BASIC_ACCOUNTING_COMMANDS, RunCmdFailureBad);
 
-    setupOemIptablesHook();
-
     return res;
 
 }
 
 int BandwidthController::disableBandwidthControl(void) {
-    /* The IPT_CLEANUP_COMMANDS are allowed to fail. */
-    runCommands(sizeof(IPT_CLEANUP_COMMANDS) / sizeof(char*),
-            IPT_CLEANUP_COMMANDS, RunCmdFailureOk);
-    setupOemIptablesHook();
+    runCommands(sizeof(IPT_FLUSH_COMMANDS) / sizeof(char*),
+            IPT_FLUSH_COMMANDS, RunCmdFailureOk);
     return 0;
 }
 
@@ -252,10 +279,10 @@
     ALOGV("runCommands(): %d commands", numCommands);
     for (int cmdNum = 0; cmdNum < numCommands; cmdNum++) {
         res = runIpxtablesCmd(commands[cmdNum], IptRejectNoAdd);
-        if (res && cmdErrHandling != RunCmdFailureBad)
+        if (res && cmdErrHandling != RunCmdFailureOk)
             return res;
     }
-    return cmdErrHandling == RunCmdFailureBad ? res : 0;
+    return 0;
 }
 
 std::string BandwidthController::makeIptablesNaughtyCmd(IptOp op, int uid) {
@@ -306,6 +333,9 @@
         op = IptOpDelete;
         failLogTemplate = "Failed to delete app uid %d from penalty box.";
         break;
+    default:
+        ALOGE("Unexpected app Op %d", appOp);
+        return -1;
     }
 
     for (uidNum = 0; uidNum < numUids; uidNum++) {
@@ -363,7 +393,7 @@
 
 int BandwidthController::prepCostlyIface(const char *ifn, QuotaType quotaType) {
     char cmd[MAX_CMD_LEN];
-    int res = 0;
+    int res = 0, res1, res2;
     int ruleInsertPos = 1;
     std::string costString;
     const char *costCString;
@@ -374,30 +404,45 @@
         costString = "costly_";
         costString += ifn;
         costCString = costString.c_str();
+        /*
+         * Flush the costly_<iface> is allowed to fail in case it didn't exist.
+         * Creating a new one is allowed to fail in case it existed.
+         * This helps with netd restarts.
+         */
+        snprintf(cmd, sizeof(cmd), "-F %s", costCString);
+        res1 = runIpxtablesCmd(cmd, IptRejectNoAdd);
         snprintf(cmd, sizeof(cmd), "-N %s", costCString);
-        res |= runIpxtablesCmd(cmd, IptRejectNoAdd);
+        res2 = runIpxtablesCmd(cmd, IptRejectNoAdd);
+        res = (res1 && res2) || (!res1 && !res2);
+
         snprintf(cmd, sizeof(cmd), "-A %s -j penalty_box", costCString);
         res |= runIpxtablesCmd(cmd, IptRejectNoAdd);
         snprintf(cmd, sizeof(cmd), "-A %s -m owner --socket-exists", costCString);
         res |= runIpxtablesCmd(cmd, IptRejectNoAdd);
-        /* TODO(jpa): Figure out why iptables doesn't correctly return from this
-         * chain. For now, hack the chain exit with an ACCEPT.
-         */
-        snprintf(cmd, sizeof(cmd), "-A %s --jump ACCEPT", costCString);
-        res |= runIpxtablesCmd(cmd, IptRejectNoAdd);
         break;
     case QuotaShared:
         costCString = "costly_shared";
         break;
+    default:
+        ALOGE("Unexpected quotatype %d", quotaType);
+        return -1;
     }
 
     if (globalAlertBytes) {
         /* The alert rule comes 1st */
         ruleInsertPos = 2;
     }
-    snprintf(cmd, sizeof(cmd), "-I INPUT %d -i %s --goto %s", ruleInsertPos, ifn, costCString);
+
+    snprintf(cmd, sizeof(cmd), "-D bw_INPUT -i %s --jump %s", ifn, costCString);
+    runIpxtablesCmd(cmd, IptRejectNoAdd);
+
+    snprintf(cmd, sizeof(cmd), "-I bw_INPUT %d -i %s --jump %s", ruleInsertPos, ifn, costCString);
     res |= runIpxtablesCmd(cmd, IptRejectNoAdd);
-    snprintf(cmd, sizeof(cmd), "-I OUTPUT %d -o %s --goto %s", ruleInsertPos, ifn, costCString);
+
+    snprintf(cmd, sizeof(cmd), "-D bw_OUTPUT -o %s --jump %s", ifn, costCString);
+    runIpxtablesCmd(cmd, IptRejectNoAdd);
+
+    snprintf(cmd, sizeof(cmd), "-I bw_OUTPUT %d -o %s --jump %s", ruleInsertPos, ifn, costCString);
     res |= runIpxtablesCmd(cmd, IptRejectNoAdd);
     return res;
 }
@@ -417,11 +462,14 @@
     case QuotaShared:
         costCString = "costly_shared";
         break;
+    default:
+        ALOGE("Unexpected quotatype %d", quotaType);
+        return -1;
     }
 
-    snprintf(cmd, sizeof(cmd), "-D INPUT -i %s --goto %s", ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-D bw_INPUT -i %s --jump %s", ifn, costCString);
     res |= runIpxtablesCmd(cmd, IptRejectNoAdd);
-    snprintf(cmd, sizeof(cmd), "-D OUTPUT -o %s --goto %s", ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-D bw_OUTPUT -o %s --jump %s", ifn, costCString);
     res |= runIpxtablesCmd(cmd, IptRejectNoAdd);
 
     /* The "-N costly_shared" is created upfront, no need to handle it here. */
@@ -693,12 +741,12 @@
     }
 
     ifaceLimiting = "! -i lo+";
-    asprintf(&alertQuotaCmd, ALERT_IPT_TEMPLATE, ifaceLimiting, opFlag, "INPUT",
+    asprintf(&alertQuotaCmd, ALERT_IPT_TEMPLATE, ifaceLimiting, opFlag, "bw_INPUT",
         bytes, alertName);
     res |= runIpxtablesCmd(alertQuotaCmd, IptRejectNoAdd);
     free(alertQuotaCmd);
     ifaceLimiting = "! -o lo+";
-    asprintf(&alertQuotaCmd, ALERT_IPT_TEMPLATE, ifaceLimiting, opFlag, "OUTPUT",
+    asprintf(&alertQuotaCmd, ALERT_IPT_TEMPLATE, ifaceLimiting, opFlag, "bw_OUTPUT",
         bytes, alertName);
     res |= runIpxtablesCmd(alertQuotaCmd, IptRejectNoAdd);
     free(alertQuotaCmd);
@@ -725,7 +773,7 @@
     }
 
     ifaceLimiting = "! -i lo+";
-    asprintf(&alertQuotaCmd, ALERT_IPT_TEMPLATE, ifaceLimiting, opFlag, "FORWARD",
+    asprintf(&alertQuotaCmd, ALERT_IPT_TEMPLATE, ifaceLimiting, opFlag, "bw_FORWARD",
         bytes, alertName);
     res = runIpxtablesCmd(alertQuotaCmd, IptRejectNoAdd);
     free(alertQuotaCmd);
@@ -917,11 +965,11 @@
 
 /*
  * Parse the ptks and bytes out of:
- * Chain FORWARD (policy ACCEPT 0 packets, 0 bytes)
+ * Chain FORWARD (policy RETURN 0 packets, 0 bytes)
  *     pkts      bytes target     prot opt in     out     source               destination
- *        0        0 ACCEPT     all  --  rmnet0 wlan0   0.0.0.0/0            0.0.0.0/0            state RELATED,ESTABLISHED
+ *        0        0 RETURN     all  --  rmnet0 wlan0   0.0.0.0/0            0.0.0.0/0            state RELATED,ESTABLISHED
  *        0        0 DROP       all  --  wlan0  rmnet0  0.0.0.0/0            0.0.0.0/0            state INVALID
- *        0        0 ACCEPT     all  --  wlan0  rmnet0  0.0.0.0/0            0.0.0.0/0
+ *        0        0 RETURN     all  --  wlan0  rmnet0  0.0.0.0/0            0.0.0.0/0
  *
  */
 int BandwidthController::parseForwardChainStats(TetherStats &stats, FILE *fp,
@@ -938,7 +986,7 @@
     while (NULL != (buffPtr = fgets(lineBuffer, MAX_IPT_OUTPUT_LINE_LEN, fp))) {
         /* Clean up, so a failed parse can still print info */
         iface0[0] = iface1[0] = rest[0] = packets = bytes = 0;
-        res = sscanf(buffPtr, "%lld %lld ACCEPT all -- %s %s 0.%s",
+        res = sscanf(buffPtr, "%lld %lld RETURN all -- %s %s 0.%s",
                 &packets, &bytes, iface0, iface1, rest);
         ALOGV("parse res=%d iface0=<%s> iface1=<%s> pkts=%lld bytes=%lld rest=<%s> orig line=<%s>", res,
              iface0, iface1, packets, bytes, rest, buffPtr);
@@ -988,7 +1036,7 @@
      * the wanted info.
      */
     fullCmd = IPTABLES_PATH;
-    fullCmd += " -nvx -L FORWARD";
+    fullCmd += " -nvx -L natctrl_FORWARD";
     iptOutput = popen(fullCmd.c_str(), "r");
     if (!iptOutput) {
             ALOGE("Failed to run %s err=%s", fullCmd.c_str(), strerror(errno));
diff --git a/BandwidthController.h b/BandwidthController.h
index 2b4cecb..a8dc992 100644
--- a/BandwidthController.h
+++ b/BandwidthController.h
@@ -46,7 +46,10 @@
     };
 
     BandwidthController();
-    int enableBandwidthControl(void);
+
+    int setupIptablesHooks(void);
+
+    int enableBandwidthControl(bool force);
     int disableBandwidthControl(void);
 
     int setInterfaceSharedQuota(const char *iface, int64_t bytes);
@@ -125,7 +128,7 @@
      * extraProcessingInfo: contains raw parsed data, and error info.
      */
     static int parseForwardChainStats(TetherStats &stats, FILE *fp,
-				      std::string &extraProcessingInfo);
+                                      std::string &extraProcessingInfo);
 
     /*------------------*/
 
@@ -147,6 +150,7 @@
     std::list<int /*appUid*/> naughtyAppUids;
 
 private:
+    static const char *IPT_FLUSH_COMMANDS[];
     static const char *IPT_CLEANUP_COMMANDS[];
     static const char *IPT_SETUP_COMMANDS[];
     static const char *IPT_BASIC_ACCOUNTING_COMMANDS[];
@@ -154,8 +158,6 @@
     /* Alphabetical */
     static const int  ALERT_RULE_POS_IN_COSTLY_CHAIN;
     static const char ALERT_GLOBAL_NAME[];
-    static const char IP6TABLES_PATH[];
-    static const char IPTABLES_PATH[];
     static const int  MAX_CMD_ARGS;
     static const int  MAX_CMD_LEN;
     static const int  MAX_IFACENAME_LEN;
diff --git a/CommandListener.cpp b/CommandListener.cpp
index 9f92beb..a40d16a 100644
--- a/CommandListener.cpp
+++ b/CommandListener.cpp
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-// #define LOG_NDEBUG 0
+#define LOG_NDEBUG 0
 
 #include <stdlib.h>
 #include <sys/socket.h>
@@ -37,7 +37,9 @@
 #include "ResponseCode.h"
 #include "ThrottleController.h"
 #include "BandwidthController.h"
+#include "IdletimerController.h"
 #include "SecondaryTableController.h"
+#include "oem_iptables_hook.h"
 
 
 TetherController *CommandListener::sTetherCtrl = NULL;
@@ -46,6 +48,7 @@
 PanController *CommandListener::sPanCtrl = NULL;
 SoftapController *CommandListener::sSoftapCtrl = NULL;
 BandwidthController * CommandListener::sBandwidthCtrl = NULL;
+IdletimerController * CommandListener::sIdletimerCtrl = NULL;
 ResolverController *CommandListener::sResolverCtrl = NULL;
 SecondaryTableController *CommandListener::sSecondaryTableCtrl = NULL;
 
@@ -60,6 +63,7 @@
     registerCmd(new PanCmd());
     registerCmd(new SoftapCmd());
     registerCmd(new BandwidthControlCmd());
+    registerCmd(new IdletimerControlCmd());
     registerCmd(new ResolverCmd());
 
     if (!sSecondaryTableCtrl)
@@ -76,8 +80,34 @@
         sSoftapCtrl = new SoftapController();
     if (!sBandwidthCtrl)
         sBandwidthCtrl = new BandwidthController();
+    if (!sIdletimerCtrl)
+        sIdletimerCtrl = new IdletimerController();
     if (!sResolverCtrl)
         sResolverCtrl = new ResolverController();
+
+    /*
+     * This is the only time controllers are allowed to touch
+     * top-level chains in iptables.
+     * Each controller should setup custom chains and hook them into
+     * the top-level ones.
+     * THE ORDER IS IMPORTANT. TRIPPLE CHECK EACH setup function.
+     */
+    /* Does DROP in nat: PREROUTING, FORWARD, OUTPUT */
+    setupOemIptablesHook();
+    /* Does DROPs in FORWARD by default */
+    sNatCtrl->setupIptablesHooks();
+    /*
+     * Does REJECT in INPUT, OUTPUT. Does counting also.
+     * No DROP/REJECT allowed later in netfilter-flow hook order.
+     */
+    sBandwidthCtrl->setupIptablesHooks();
+    /*
+     * Counts in nat: PREROUTING, POSTROUTING.
+     * No DROP/REJECT allowed later in netfilter-flow hook order.
+     */
+    sIdletimerCtrl->setupIptablesHooks();
+
+    sBandwidthCtrl->enableBandwidthControl(false);
 }
 
 CommandListener::InterfaceCmd::InterfaceCmd() :
@@ -908,7 +938,7 @@
     ALOGV("bwctrlcmd: argc=%d %s %s ...", argc, argv[0], argv[1]);
 
     if (!strcmp(argv[1], "enable")) {
-        int rc = sBandwidthCtrl->enableBandwidthControl();
+        int rc = sBandwidthCtrl->enableBandwidthControl(true);
         sendGenericOkFail(cli, rc);
         return 0;
 
@@ -1170,3 +1200,63 @@
     cli->sendMsg(ResponseCode::CommandSyntaxError, "Unknown bandwidth cmd", false);
     return 0;
 }
+
+CommandListener::IdletimerControlCmd::IdletimerControlCmd() :
+    NetdCommand("idletimer") {
+}
+
+int CommandListener::IdletimerControlCmd::runCommand(SocketClient *cli, int argc, char **argv) {
+  // TODO(ashish): Change the error statements
+    if (argc < 2) {
+        cli->sendMsg(ResponseCode::CommandSyntaxError, "Missing argument", false);
+        return 0;
+    }
+
+    ALOGV("idletimerctrlcmd: argc=%d %s %s ...", argc, argv[0], argv[1]);
+
+    if (!strcmp(argv[1], "enable")) {
+      if (0 != sIdletimerCtrl->enableIdletimerControl()) {
+        cli->sendMsg(ResponseCode::CommandSyntaxError, "Missing argument", false);
+      } else {
+        cli->sendMsg(ResponseCode::CommandOkay, "Enable success", false);
+      }
+      return 0;
+
+    }
+    if (!strcmp(argv[1], "disable")) {
+      if (0 != sIdletimerCtrl->disableIdletimerControl()) {
+        cli->sendMsg(ResponseCode::CommandSyntaxError, "Missing argument", false);
+      } else {
+        cli->sendMsg(ResponseCode::CommandOkay, "Disable success", false);
+      }
+      return 0;
+    }
+    if (!strcmp(argv[1], "add")) {
+        if (argc != 4) {
+            cli->sendMsg(ResponseCode::CommandSyntaxError, "Missing argument", false);
+            return 0;
+        }
+        if(0 != sIdletimerCtrl->addInterfaceIdletimer(argv[2], atoi(argv[3]))) {
+          cli->sendMsg(ResponseCode::OperationFailed, "Failed to add interface", false);
+        } else {
+          cli->sendMsg(ResponseCode::CommandOkay,  "Add success", false);
+        }
+        return 0;
+    }
+    if (!strcmp(argv[1], "remove")) {
+        if (argc != 4) {
+            cli->sendMsg(ResponseCode::CommandSyntaxError, "Missing argument", false);
+            return 0;
+        }
+        // ashish: fixme timeout
+        if (0 != sIdletimerCtrl->removeInterfaceIdletimer(argv[2], atoi(argv[3]))) {
+          cli->sendMsg(ResponseCode::OperationFailed, "Failed to remove interface", false);
+        } else {
+          cli->sendMsg(ResponseCode::CommandOkay, "Remove success", false);
+        }
+        return 0;
+    }
+
+    cli->sendMsg(ResponseCode::CommandSyntaxError, "Unknown idletimer cmd", false);
+    return 0;
+}
diff --git a/CommandListener.h b/CommandListener.h
index 0ed600b..a9da6d7 100644
--- a/CommandListener.h
+++ b/CommandListener.h
@@ -26,6 +26,7 @@
 #include "PanController.h"
 #include "SoftapController.h"
 #include "BandwidthController.h"
+#include "IdletimerController.h"
 #include "ResolverController.h"
 #include "SecondaryTableController.h"
 
@@ -36,6 +37,7 @@
     static PanController *sPanCtrl;
     static SoftapController *sSoftapCtrl;
     static BandwidthController *sBandwidthCtrl;
+    static IdletimerController *sIdletimerCtrl;
     static ResolverController *sResolverCtrl;
     static SecondaryTableController *sSecondaryTableCtrl;
 
@@ -116,6 +118,13 @@
         void sendGenericSyntaxError(SocketClient *cli, const char *usageMsg);
     };
 
+    class IdletimerControlCmd : public NetdCommand {
+    public:
+        IdletimerControlCmd();
+        virtual ~IdletimerControlCmd() {}
+        int runCommand(SocketClient *c, int argc, char ** argv);
+    };
+
     class ResolverCmd : public NetdCommand {
     public:
         ResolverCmd();
diff --git a/IdletimerController.cpp b/IdletimerController.cpp
new file mode 100644
index 0000000..efe4f09
--- /dev/null
+++ b/IdletimerController.cpp
@@ -0,0 +1,175 @@
+/*
+ * 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.
+ */
+
+// #define LOG_NDEBUG 0
+
+/*
+ * MODUS OPERANDI
+ * --------------
+ *
+ * IPTABLES command sequence:
+ *
+ * iptables -F
+ *
+ * iptables -t nat -F idletimer_PREROUTING
+ * iptables -t nat -F idletimer_POSTROUTING
+ *
+ *
+ * iptables -t nat -N idletimer_PREROUTING
+ * iptables -t nat -N idletimer_POSTROUTING
+ *
+ * iptables -t nat -D PREROUTING -j idletimer_PREROUTING
+ * iptables -t nat -D POSTROUTING -j idletimer_POSTROUTING
+ *
+ *
+ * iptables -t nat -I PREROUTING -j idletimer_PREROUTING
+ * iptables -t nat -I POSTROUTING -j idletimer_POSTROUTING
+ *
+ * # For notifications to work the lable name must match the name of a valid interface.
+ * # If the label name does match an interface, the rules will be a no-op.
+ *
+ * iptables -t nat -A idletimer_PREROUTING -i rmnet0 -j IDLETIMER  --timeout 5 --label test-chain --send_nl_msg 1
+ * iptables -t nat -A idletimer_POSTROUTING -o rmnet0 -j IDLETIMER  --timeout 5 --label test-chain --send_nl_msg 1
+ *
+ * iptables -nxvL -t nat
+ *
+ * =================
+ *
+ * ndc command sequence
+ * ------------------
+ * ndc idletimer enable
+ * ndc idletimer add <iface> <timeout>
+ * ndc idletimer remove <iface> <timeout>
+ *
+ * Monitor effect on the iptables chains after each step using:
+ *     iptables -nxvL -t nat
+ *
+ * Remember that the timeout value has to be same at the time of the
+ * removal.
+ *
+ * Note that currently if the name of the iface is incorrect, iptables
+ * will setup rules without checking if it is the name of a valid
+ * interface (although no notifications will ever be received).  It is
+ * the responsibility of code in Java land to ensure that the interface name
+ * is correct. The benefit of this, is that idletimers can be setup on
+ * interfaces than come and go.
+ *
+ * A remove should be called for each add command issued during cleanup, as duplicate
+ * entries of the rule may exist and will all have to removed.
+ *
+ */
+
+#include <stdlib.h>
+#include <errno.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+#include <cutils/properties.h>
+
+#define LOG_TAG "IdletimerController"
+#include <cutils/log.h>
+
+#include "IdletimerController.h"
+#include "NetdConstants.h"
+
+extern "C" int system_nosh(const char *command);
+
+IdletimerController::IdletimerController() {
+}
+
+IdletimerController::~IdletimerController() {
+}
+/* return 0 or non-zero */
+int IdletimerController::runIpxtablesCmd(const char *cmd) {
+    char *buffer;
+    size_t len = strnlen(cmd, 255);
+    int res;
+
+    if (len == 255) {
+        ALOGE("command too long");
+        return -1;
+    }
+
+    asprintf(&buffer, "%s %s", IPTABLES_PATH, cmd);
+    res = system_nosh(buffer);
+    ALOGV("%s #%d", buffer, res);
+    free(buffer);
+
+    return res;
+}
+
+bool IdletimerController::setupIptablesHooks() {
+    runIpxtablesCmd("-t nat -D PREROUTING -j idletimer_nat_PREROUTING");
+    runIpxtablesCmd("-t nat -F idletimer_nat_PREROUTING");
+    runIpxtablesCmd("-t nat -N idletimer_nat_PREROUTING");
+
+    runIpxtablesCmd("-t nat -D POSTROUTING -j idletimer_nat_POSTROUTING");
+    runIpxtablesCmd("-t nat -F idletimer_nat_POSTROUTING");
+    runIpxtablesCmd("-t nat -N idletimer_nat_POSTROUTING");
+
+    if (runIpxtablesCmd("-t nat -I PREROUTING -j idletimer_nat_PREROUTING")
+        || runIpxtablesCmd("-t nat -I POSTROUTING -j idletimer_nat_POSTROUTING")) {
+        return false;
+    }
+    return true;
+}
+
+int IdletimerController::setDefaults() {
+  if (runIpxtablesCmd("-t nat -F idletimer_nat_PREROUTING")
+      || runIpxtablesCmd("-t nat -F idletimer_nat_POSTROUTING") )
+      return -1;
+  return 0;
+}
+
+int IdletimerController::enableIdletimerControl() {
+    int res = setDefaults();
+    return res;
+}
+
+int IdletimerController::disableIdletimerControl() {
+    int res = setDefaults();
+    return res;
+}
+
+int IdletimerController::modifyInterfaceIdletimer(IptOp op, const char *iface,
+                                                  uint32_t timeout) {
+  int res;
+  char *buffer;
+  asprintf(&buffer, "-t nat -%c idletimer_nat_PREROUTING -i %s -j IDLETIMER"
+           " --timeout %u --label %s --send_nl_msg 1",
+           (op == IptOpAdd) ? 'A' : 'D', iface, timeout, iface);
+  res = runIpxtablesCmd(buffer);
+  free(buffer);
+
+  asprintf(&buffer, "-t nat -%c idletimer_nat_POSTROUTING -o %s -j IDLETIMER"
+           " --timeout %u --label %s --send_nl_msg 1",
+           (op == IptOpAdd) ? 'A' : 'D', iface, timeout, iface);
+  res |= runIpxtablesCmd(buffer);
+  free(buffer);
+
+  return res;
+}
+
+int IdletimerController::addInterfaceIdletimer(const char *iface, uint32_t timeout) {
+  return modifyInterfaceIdletimer(IptOpAdd, iface, timeout);
+}
+
+int IdletimerController::removeInterfaceIdletimer(const char *iface, uint32_t timeout) {
+  return modifyInterfaceIdletimer(IptOpDelete, iface, timeout);
+}
diff --git a/IdletimerController.h b/IdletimerController.h
new file mode 100644
index 0000000..a55f7af
--- /dev/null
+++ b/IdletimerController.h
@@ -0,0 +1,38 @@
+/*
+ * 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 _IDLETIMER_CONTROLLER_H
+#define _IDLETIMER_CONTROLLER_H
+
+class IdletimerController {
+public:
+
+    IdletimerController();
+    virtual ~IdletimerController();
+
+    int enableIdletimerControl();
+    int disableIdletimerControl();
+    int addInterfaceIdletimer(const char *iface, uint32_t timeout);
+    int removeInterfaceIdletimer(const char *iface, uint32_t timeout);
+    bool setupIptablesHooks();
+
+ private:
+    enum IptOp { IptOpAdd, IptOpDelete };
+    int setDefaults();
+    int runIpxtablesCmd(const char *cmd);
+    int modifyInterfaceIdletimer(IptOp op, const char *iface, uint32_t timeout);
+};
+
+#endif
diff --git a/NatController.cpp b/NatController.cpp
index b68e4bf..db96ed3 100644
--- a/NatController.cpp
+++ b/NatController.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+// #define LOG_NDEBUG 0
+
 #include <stdlib.h>
 #include <errno.h>
 #include <sys/socket.h>
@@ -29,13 +31,14 @@
 
 #include "NatController.h"
 #include "SecondaryTableController.h"
-#include "oem_iptables_hook.h"
 #include "NetdConstants.h"
 
 extern "C" int system_nosh(const char *command);
 
 NatController::NatController(SecondaryTableController *ctrl) {
     secondaryTableCtrl = ctrl;
+
+    setupIptablesHooks();
     setDefaults();
 }
 
@@ -55,21 +58,43 @@
 
     asprintf(&buffer, "%s %s", path, cmd);
     res = system_nosh(buffer);
+    ALOGV("runCmd() buffer='%s' res=%d", buffer, res);
     free(buffer);
     return res;
 }
 
-int NatController::setDefaults() {
-
+int NatController::setupIptablesHooks() {
     if (runCmd(IPTABLES_PATH, "-P INPUT ACCEPT"))
         return -1;
     if (runCmd(IPTABLES_PATH, "-P OUTPUT ACCEPT"))
         return -1;
-    if (runCmd(IPTABLES_PATH, "-P FORWARD DROP"))
+    if (runCmd(IPTABLES_PATH, "-P FORWARD ACCEPT"))
         return -1;
-    if (runCmd(IPTABLES_PATH, "-F FORWARD"))
+
+    // Order is important!
+    // -D to delete any pre-existing jump rule, to prevent dupes (no-op if doesn't exist)
+    // -F to flush the chain (no-op if doesn't exist).
+    // -N to create the chain (no-op if already exist).
+
+    runCmd(IPTABLES_PATH, "-D FORWARD -j natctrl_FORWARD");
+    runCmd(IPTABLES_PATH, "-F natctrl_FORWARD");
+    runCmd(IPTABLES_PATH, "-N natctrl_FORWARD");
+    if (runCmd(IPTABLES_PATH, "-A FORWARD -j natctrl_FORWARD"))
         return -1;
-    if (runCmd(IPTABLES_PATH, "-t nat -F"))
+
+    runCmd(IPTABLES_PATH, "-t nat -D POSTROUTING -j natctrl_nat_POSTROUTING");
+    runCmd(IPTABLES_PATH, "-t nat -F natctrl_nat_POSTROUTING");
+    runCmd(IPTABLES_PATH, "-t nat -N natctrl_nat_POSTROUTING");
+    if (runCmd(IPTABLES_PATH, "-t nat -A POSTROUTING -j natctrl_nat_POSTROUTING"))
+        return -1;
+
+    return 0;
+}
+
+int NatController::setDefaults() {
+    if (runCmd(IPTABLES_PATH, "-F natctrl_FORWARD"))
+        return -1;
+    if (runCmd(IPTABLES_PATH, "-t nat -F natctrl_nat_POSTROUTING"))
         return -1;
 
     runCmd(IP_PATH, "rule flush");
@@ -82,7 +107,6 @@
 
     natCount = 0;
 
-    setupOemIptablesHook();
     return 0;
 }
 
@@ -138,10 +162,17 @@
         return -1;
     }
 
+    /* Always make sure the drop rule is at the end */
+    snprintf(cmd, sizeof(cmd), "-D natctrl_FORWARD -j DROP");
+    runCmd(IPTABLES_PATH, cmd);
+    snprintf(cmd, sizeof(cmd), "-A natctrl_FORWARD -j DROP");
+    runCmd(IPTABLES_PATH, cmd);
+
+
     natCount++;
     // add this if we are the first added nat
     if (natCount == 1) {
-        snprintf(cmd, sizeof(cmd), "-t nat -A POSTROUTING -o %s -j MASQUERADE", extIface);
+        snprintf(cmd, sizeof(cmd), "-t nat -A natctrl_nat_POSTROUTING -o %s -j MASQUERADE", extIface);
         if (runCmd(IPTABLES_PATH, cmd)) {
             ALOGE("Error seting postroute rule: %s", cmd);
             // unwind what's been done, but don't care about success - what more could we do?
@@ -162,7 +193,7 @@
     char cmd[255];
 
     snprintf(cmd, sizeof(cmd),
-             "-%s FORWARD -i %s -o %s -m state --state ESTABLISHED,RELATED -j ACCEPT",
+             "-%s natctrl_FORWARD -i %s -o %s -m state --state ESTABLISHED,RELATED -j RETURN",
              (add ? "A" : "D"),
              extIface, intIface);
     if (runCmd(IPTABLES_PATH, cmd) && add) {
@@ -170,36 +201,41 @@
     }
 
     snprintf(cmd, sizeof(cmd),
-            "-%s FORWARD -i %s -o %s -m state --state INVALID -j DROP",
+            "-%s natctrl_FORWARD -i %s -o %s -m state --state INVALID -j DROP",
             (add ? "A" : "D"),
             intIface, extIface);
     if (runCmd(IPTABLES_PATH, 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",
+                "-%s natctrl_FORWARD -i %s -o %s -m state --state ESTABLISHED,RELATED -j RETURN",
                 (!add ? "A" : "D"),
                 extIface, intIface);
         runCmd(IPTABLES_PATH, cmd);
         return -1;
     }
 
-    snprintf(cmd, sizeof(cmd), "-%s FORWARD -i %s -o %s -j ACCEPT", (add ? "A" : "D"),
+    snprintf(cmd, sizeof(cmd), "-%s natctrl_FORWARD -i %s -o %s -j RETURN", (add ? "A" : "D"),
             intIface, extIface);
     if (runCmd(IPTABLES_PATH, 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",
+                "-%s natctrl_FORWARD -i %s -o %s -m state --state INVALID -j DROP",
                 (!add ? "A" : "D"),
                 intIface, extIface);
         runCmd(IPTABLES_PATH, cmd);
 
         snprintf(cmd, sizeof(cmd),
-                 "-%s FORWARD -i %s -o %s -m state --state ESTABLISHED,RELATED -j ACCEPT",
+                 "-%s natctrl_FORWARD -i %s -o %s -m state --state ESTABLISHED,RELATED -j RETURN",
                  (!add ? "A" : "D"),
                  extIface, intIface);
         runCmd(IPTABLES_PATH, cmd);
         return -1;
     }
+
+    snprintf(cmd, sizeof(cmd), "-%s natctrl_FORWARD -j DROP", (add ? "A" : "D"),
+            intIface, extIface);
+    runCmd(IPTABLES_PATH, cmd);
+
     return 0;
 }
 
diff --git a/NatController.h b/NatController.h
index cef54f6..1d328e5 100644
--- a/NatController.h
+++ b/NatController.h
@@ -29,6 +29,7 @@
 
     int enableNat(const int argc, char **argv);
     int disableNat(const int argc, char **argv);
+    int setupIptablesHooks();
 
 private:
     int natCount;
diff --git a/NetdConstants.cpp b/NetdConstants.cpp
index bac3cbb..e57b483 100644
--- a/NetdConstants.cpp
+++ b/NetdConstants.cpp
@@ -18,6 +18,7 @@
 
 const char * const OEM_SCRIPT_PATH = "/system/bin/oem-iptables-init.sh";
 const char * const IPTABLES_PATH = "/system/bin/iptables";
+const char * const IP6TABLES_PATH = "/system/bin/ip6tables";
 const char * const TC_PATH = "/system/bin/tc";
 const char * const IP_PATH = "/system/bin/ip";
 const char * const ADD = "add";
diff --git a/NetdConstants.h b/NetdConstants.h
index d92c9e4..9943a05 100644
--- a/NetdConstants.h
+++ b/NetdConstants.h
@@ -19,6 +19,7 @@
 
 
 extern const char * const IPTABLES_PATH;
+extern const char * const IP6TABLES_PATH;
 extern const char * const IP_PATH;
 extern const char * const TC_PATH;
 extern const char * const OEM_SCRIPT_PATH;