NatController: Fix tethering-counting iptables rules handling

Fix duplicate tethering-pair detection. It was broken because the
underlying mechanism used quota2 which has a 15char limit and
the two combined interface names were longer than that.

Fix parsing or tether-counting rules when no interfaces are available
vs when parsing is broken.
Now the parser is not invoked if no tether-counting rules were created.

Bug: 10710027
Change-Id: I37899d113a37cd84255d439efa6e2ed3ce712ec0

diff --git a/BandwidthController.cpp b/BandwidthController.cpp
index 664211d..c76f00a 100644 (file)
--- a/BandwidthController.cpp
+++ b/BandwidthController.cpp
@@ -1082,10 +1082,12 @@ int BandwidthController::removeCostlyAlert(const char *costName, int64_t *alertB
  * Parse the ptks and bytes out of:
  *   Chain natctrl_tether_counters (4 references)
  *       pkts      bytes target     prot opt in     out     source               destination
- *         26     2373 RETURN     all  --  wlan0  rmnet0  0.0.0.0/0            0.0.0.0/0            counter wlan0_rmnet0: 0 bytes
- *         27     2002 RETURN     all  --  rmnet0 wlan0   0.0.0.0/0            0.0.0.0/0            counter rmnet0_wlan0: 0 bytes
- *       1040   107471 RETURN     all  --  bt-pan rmnet0  0.0.0.0/0            0.0.0.0/0            counter bt-pan_rmnet0: 0 bytes
- *       1450  1708806 RETURN     all  --  rmnet0 bt-pan  0.0.0.0/0            0.0.0.0/0            counter rmnet0_bt-pan: 0 bytes
+ *         26     2373 RETURN     all  --  wlan0  rmnet0  0.0.0.0/0            0.0.0.0/0
+ *         27     2002 RETURN     all  --  rmnet0 wlan0   0.0.0.0/0            0.0.0.0/0
+ *       1040   107471 RETURN     all  --  bt-pan rmnet0  0.0.0.0/0            0.0.0.0/0
+ *       1450  1708806 RETURN     all  --  rmnet0 bt-pan  0.0.0.0/0            0.0.0.0/0
+ * It results in an error if invoked and no tethering counter rules exist. The constraint
+ * helps detect complete parsing failure.
  */
 int BandwidthController::parseForwardChainStats(SocketClient *cli, const TetherStats filter,
                                                 FILE *fp, std::string &extraProcessingInfo) {
@@ -1098,6 +1100,7 @@ int BandwidthController::parseForwardChainStats(SocketClient *cli, const TetherS
     TetherStats stats;
     char *buffPtr;
     int64_t packets, bytes;
+    int statsFound = 0;
 
     bool filterPair = filter.intIface[0] && filter.extIface[0];
 
@@ -1173,14 +1176,18 @@ int BandwidthController::parseForwardChainStats(SocketClient *cli, const TetherS
                 stats = filter;
             }
             free(msg);
+            statsFound++;
         }
     }
-    /* Successful if the last stats entry wasn't partial. */
-    if ((stats.rxBytes == -1) == (stats.txBytes == -1)) {
-        cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
-        return 0;
+
+    /* It is always an error to find only one side of the stats. */
+    /* It is an error to find nothing when not filtering. */
+    if (((stats.rxBytes == -1) != (stats.txBytes == -1)) ||
+        (!statsFound && !filterPair)) {
+        return -1;
     }
-    return -1;
+    cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
+    return 0;
 }
 
 char *BandwidthController::TetherStats::getStatsLine(void) const {

diff --git a/BandwidthController.h b/BandwidthController.h
index 74e595c..2aca2cd 100644 (file)
--- a/BandwidthController.h
+++ b/BandwidthController.h
@@ -87,7 +87,8 @@ public:
      * For all pairs, stats should have ifaceIn=ifaceOut="".
      * Sends out to the cli the single stat (TetheringStatsReluts) or a list of stats
      * (TetheringStatsListResult+CommandOkay).
-     * Error is to be handled on the outside
+     * Error is to be handled on the outside.
+     * It results in an error if invoked and no tethering counter rules exist.
      */
     int getTetherStats(SocketClient *cli, TetherStats &stats, std::string &extraProcessingInfo);
 

diff --git a/CommandListener.cpp b/CommandListener.cpp
index f1bec15..382349a 100644 (file)
--- a/CommandListener.cpp
+++ b/CommandListener.cpp
@@ -1355,6 +1355,11 @@ int CommandListener::BandwidthControlCmd::runCommand(SocketClient *cli, int argc
         }
         tetherStats.intIface = argc > 2 ? argv[2] : "";
         tetherStats.extIface = argc > 3 ? argv[3] : "";
+        // No filtering requested and there are no interface pairs to lookup.
+        if (argc <= 2 && sNatCtrl->ifacePairList.empty()) {
+            cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
+            return 0;
+        }
         int rc = sBandwidthCtrl->getTetherStats(cli, tetherStats, extraProcessingInfo);
         if (rc) {
                 extraProcessingInfo.insert(0, "Failed to get tethering stats.\n");

diff --git a/NatController.cpp b/NatController.cpp
index b2a0e64..90335c2 100644 (file)
--- a/NatController.cpp
+++ b/NatController.cpp
@@ -97,6 +97,7 @@ int NatController::setupIptablesHooks() {
                 return -1;
         }
     }
+    ifacePairList.clear();
 
     return 0;
 }
@@ -253,27 +254,32 @@ int NatController::enableNat(const int argc, char **argv) {
     return 0;
 }
 
+bool NatController::checkTetherCountingRuleExist(const char *pair_name) {
+    std::list<std::string>::iterator it;
+
+    for (it = ifacePairList.begin(); it != ifacePairList.end(); it++) {
+        if (*it == pair_name) {
+            /* We already have this counter */
+            return true;
+        }
+    }
+    return false;
+}
+
 int NatController::setTetherCountingRules(bool add, const char *intIface, const char *extIface) {
 
     /* We only ever add tethering quota rules so that they stick. */
     if (!add) {
         return 0;
     }
-    char *quota_name, *proc_path;
+    char *pair_name, *proc_path;
     int quota_fd;
-    asprintf(&quota_name, "%s_%s", intIface, extIface);
-
-    asprintf(&proc_path, "/proc/net/xt_quota/%s", quota_name);
-    quota_fd = open(proc_path, O_RDONLY);
-    if (quota_fd >= 0) {
-        /* quota for iface pair already exists */
-        free(proc_path);
-        free(quota_name);
+    asprintf(&pair_name, "%s_%s", intIface, extIface);
+
+    if (checkTetherCountingRuleExist(pair_name)) {
+        free(pair_name);
         return 0;
     }
-    close(quota_fd);
-    free(proc_path);
-
     const char *cmd2b[] = {
             IPTABLES_PATH,
             "-A",
@@ -282,32 +288,22 @@ int NatController::setTetherCountingRules(bool add, const char *intIface, const
             intIface,
             "-o",
             extIface,
-            "-m",
-            "quota2",
-            "--name",
-            quota_name,
-            "--grow",
             "-j",
           "RETURN"
     };
 
     if (runCmd(ARRAY_SIZE(cmd2b), cmd2b) && add) {
-        free(quota_name);
+        free(pair_name);
         return -1;
     }
-    free(quota_name);
-
-    asprintf(&quota_name, "%s_%s", extIface, intIface);
-    asprintf(&proc_path, "/proc/net/xt_quota/%s", quota_name);
-    quota_fd = open(proc_path, O_RDONLY);
-    if (quota_fd >= 0) {
-        /* quota for iface pair already exists */
-        free(proc_path);
-        free(quota_name);
+    ifacePairList.push_front(pair_name);
+    free(pair_name);
+
+    asprintf(&pair_name, "%s_%s", extIface, intIface);
+    if (checkTetherCountingRuleExist(pair_name)) {
+        free(pair_name);
         return 0;
     }
-    close(quota_fd);
-    free(proc_path);
 
     const char *cmd3b[] = {
             IPTABLES_PATH,
@@ -317,21 +313,17 @@ int NatController::setTetherCountingRules(bool add, const char *intIface, const
             extIface,
             "-o",
             intIface,
-            "-m",
-            "quota2",
-            "--name",
-            quota_name,
-            "--grow",
             "-j",
             "RETURN"
     };
 
     if (runCmd(ARRAY_SIZE(cmd3b), cmd3b) && add) {
         // unwind what's been done, but don't care about success - what more could we do?
-        free(quota_name);
+        free(pair_name);
         return -1;
     }
-    free(quota_name);
+    ifacePairList.push_front(pair_name);
+    free(pair_name);
     return 0;
 }
 

diff --git a/NatController.h b/NatController.h
index 525ca02..52a1627 100644 (file)
--- a/NatController.h
+++ b/NatController.h
@@ -18,6 +18,7 @@
 #define _NAT_CONTROLLER_H
 
 #include <linux/in.h>
+#include <list>
 
 #include "SecondaryTableController.h"
 
@@ -35,10 +36,15 @@ public:
     static const char* LOCAL_NAT_POSTROUTING;
     static const char* LOCAL_TETHER_COUNTERS_CHAIN;
 
+    // List of strings of interface pairs.
+    std::list<std::string> ifacePairList;
+
 private:
     int natCount;
     SecondaryTableController *secondaryTableCtrl;
 
+    bool checkTetherCountingRuleExist(const char *pair_name);
+
     int setDefaults();
     int runCmd(int argc, const char **argv);
     bool checkInterface(const char *iface);