Use iptables pipes when fetching tether counters.

Tested using:

adb shell ndc bandwidth gettetherstats
adb shell iptables -nvx -L natctrl_tether_counters
adb shell ip6tables -nvx -L natctrl_tether_counters

Results:

114 0 wlan0 rmnet_data0 272883 2976 8624804 6032
200 0 Tethering stats list completed
Chain natctrl_tether_counters (2 references)
    pkts      bytes target     prot opt in     out     source destination
    2688   179096 RETURN     all  --  wlan0  rmnet_data0  0.0.0.0/0 0.0.0.0/0
    5713  8351999 RETURN     all  --  rmnet_data0 wlan0   0.0.0.0/0 0.0.0.0/0
Chain natctrl_tether_counters (1 references)
    pkts      bytes target     prot opt in     out     source destination
     288    93787 RETURN     all      wlan0  rmnet_data0  ::/0 ::/0
     319   272805 RETURN     all      rmnet_data0 wlan0   ::/0 ::/0

Test: manual test described above
Test: data usage increases by 10MB when downloading 10MB file
Test: netd_unit_test passes
Bug: 34873832
Change-Id: I32c4e750a4d3c379074cc13ab1302d51421860d2
diff --git a/server/BandwidthController.cpp b/server/BandwidthController.cpp
index 3df2309..c8e8d14 100644
--- a/server/BandwidthController.cpp
+++ b/server/BandwidthController.cpp
@@ -76,6 +76,11 @@
 const int  MAX_IFACENAME_LEN = 64;
 const int  MAX_IPT_OUTPUT_LINE_LEN = 256;
 const std::string NEW_CHAIN_COMMAND = "-N ";
+const std::string GET_TETHER_STATS_COMMAND = android::base::StringPrintf(
+    "*filter\n"
+    "-nvx -L %s\n"
+    "COMMIT\n", NatController::LOCAL_TETHER_COUNTERS_CHAIN);
+
 
 /**
  * Some comments about the rules:
@@ -1111,16 +1116,17 @@
  * helps detect complete parsing failure.
  */
 int BandwidthController::addForwardChainStats(const TetherStats& filter,
-                                              TetherStatsList& statsList, FILE *fp,
+                                              TetherStatsList& statsList,
+                                              const std::string& statsOutput,
                                               std::string &extraProcessingInfo) {
     int res;
-    char lineBuffer[MAX_IPT_OUTPUT_LINE_LEN];
+    std::string statsLine;
     char iface0[MAX_IPT_OUTPUT_LINE_LEN];
     char iface1[MAX_IPT_OUTPUT_LINE_LEN];
     char rest[MAX_IPT_OUTPUT_LINE_LEN];
 
     TetherStats stats;
-    char *buffPtr;
+    const char *buffPtr;
     int64_t packets, bytes;
     int statsFound = 0;
 
@@ -1132,7 +1138,10 @@
 
     stats = filter;
 
-    while (NULL != (buffPtr = fgets(lineBuffer, MAX_IPT_OUTPUT_LINE_LEN, fp))) {
+    std::stringstream stream(statsOutput);
+    while (std::getline(stream, statsLine, '\n')) {
+        buffPtr = statsLine.c_str();
+
         /* Clean up, so a failed parse can still print info */
         iface0[0] = iface1[0] = rest[0] = packets = bytes = 0;
         if (strstr(buffPtr, "0.0.0.0")) {
@@ -1149,6 +1158,7 @@
         ALOGV("parse res=%d iface0=<%s> iface1=<%s> pkts=%" PRId64" bytes=%" PRId64" rest=<%s> orig line=<%s>", res,
              iface0, iface1, packets, bytes, rest, buffPtr);
         extraProcessingInfo += buffPtr;
+        extraProcessingInfo += "\n";
 
         if (res != 5) {
             continue;
@@ -1223,37 +1233,21 @@
     return msg;
 }
 
-std::string getTetherStatsCommand(const char *binary) {
-    /*
-     * Why not use some kind of lib to talk to iptables?
-     * Because the only libs are libiptc and libip6tc in iptables, and they are
-     * not easy to use. They require the known iptables match modules to be
-     * preloaded/linked, and require apparently a lot of wrapper code to get
-     * the wanted info.
-     */
-    return android::base::StringPrintf("%s -nvx -w -L %s", binary,
-                                       NatController::LOCAL_TETHER_COUNTERS_CHAIN);
-}
-
 int BandwidthController::getTetherStats(SocketClient *cli, TetherStats& filter,
                                         std::string &extraProcessingInfo) {
     int res = 0;
-    std::string fullCmd;
-    FILE *iptOutput;
 
     TetherStatsList statsList;
 
-    for (const auto binary : {IPTABLES_PATH, IP6TABLES_PATH}) {
-        fullCmd = getTetherStatsCommand(binary);
-        iptOutput = popenFunction(fullCmd.c_str(), "r");
-        if (!iptOutput) {
-                ALOGE("Failed to run %s err=%s", fullCmd.c_str(), strerror(errno));
-                extraProcessingInfo += "Failed to run iptables.";
+    for (const IptablesTarget target : {V4, V6}) {
+        std::string statsString;
+        res = iptablesRestoreFunction(target, GET_TETHER_STATS_COMMAND, &statsString);
+        if (res != 0) {
+            ALOGE("Failed to run %s err=%d", GET_TETHER_STATS_COMMAND.c_str(), res);
             return -1;
         }
 
-        res = addForwardChainStats(filter, statsList, iptOutput, extraProcessingInfo);
-        pclose(iptOutput);
+        res = addForwardChainStats(filter, statsList, statsString, extraProcessingInfo);
         if (res != 0) {
             return res;
         }
diff --git a/server/BandwidthController.h b/server/BandwidthController.h
index 66fdeee..0398ce9 100644
--- a/server/BandwidthController.h
+++ b/server/BandwidthController.h
@@ -171,11 +171,6 @@
 
     static void addStats(TetherStatsList& statsList, const TetherStats& stats);
 
-    static int addForwardChainStats(const TetherStats& filter,
-                                    TetherStatsList& statsList, FILE *fp,
-                                    std::string &extraProcessingInfo);
-
-
     /*
      * stats should never have only intIface initialized. Other 3 combos are ok.
      * fp should be a file to the apropriate FORWARD chain of iptables rules.
@@ -185,8 +180,9 @@
      *  in:extIface out:intIface
      * and the rules are grouped in pairs when more that one tethering was setup.
      */
-    static int parseForwardChainStats(SocketClient *cli, const TetherStats filter, FILE *fp,
-                                      std::string &extraProcessingInfo);
+    static int addForwardChainStats(const TetherStats& filter,
+                                    TetherStatsList& statsList, const std::string& iptOutput,
+                                    std::string &extraProcessingInfo);
 
     /*
      * Attempt to find the bw_costly_* tables that need flushing,
diff --git a/server/BandwidthControllerTest.cpp b/server/BandwidthControllerTest.cpp
index 2b2ce6e..959ad9a 100644
--- a/server/BandwidthControllerTest.cpp
+++ b/server/BandwidthControllerTest.cpp
@@ -41,23 +41,15 @@
     }
     BandwidthController mBw;
 
-    void addPopenContents(std::string contents) {
-        sPopenContents.push_back(contents);
-    }
-
-    void addPopenContents(std::string contents1, std::string contents2) {
-        sPopenContents.push_back(contents1);
-        sPopenContents.push_back(contents2);
-    }
-
-    void clearPopenContents() {
-        sPopenContents.clear();
-    }
-
     void addIptablesRestoreOutput(std::string contents) {
         sIptablesRestoreOutput.push_back(contents);
     }
 
+    void addIptablesRestoreOutput(std::string contents1, std::string contents2) {
+        sIptablesRestoreOutput.push_back(contents1);
+        sIptablesRestoreOutput.push_back(contents2);
+    }
+
     void clearIptablesRestoreOutput() {
         sIptablesRestoreOutput.clear();
     }
@@ -229,17 +221,17 @@
     BandwidthController::TetherStats filter;
 
     // If no filter is specified, both IPv4 and IPv6 counters must have at least one interface pair.
-    addPopenContents(kIPv4TetherCounters, "");
+    addIptablesRestoreOutput(kIPv4TetherCounters);
     ASSERT_EQ(-1, mBw.getTetherStats(&cli, filter, err));
     expectNoSocketClientResponse(socketPair[1]);
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
-    addPopenContents("", kIPv6TetherCounters);
+    addIptablesRestoreOutput(kIPv6TetherCounters);
     ASSERT_EQ(-1, mBw.getTetherStats(&cli, filter, err));
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
     // IPv4 and IPv6 counters are properly added together.
-    addPopenContents(kIPv4TetherCounters, kIPv6TetherCounters);
+    addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
     filter = BandwidthController::TetherStats();
     std::string expected =
             "114 wlan0 rmnet0 10002373 10026 20002002 20027\n"
@@ -248,83 +240,89 @@
     ASSERT_EQ(0, mBw.getTetherStats(&cli, filter, err));
     ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
     expectNoSocketClientResponse(socketPair[1]);
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
     // Test filtering.
-    addPopenContents(kIPv4TetherCounters, kIPv6TetherCounters);
+    addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
     filter = BandwidthController::TetherStats("bt-pan", "rmnet0", -1, -1, -1, -1);
     expected = "221 bt-pan rmnet0 107471 1040 1708806 1450\n";
     ASSERT_EQ(0, mBw.getTetherStats(&cli, filter, err));
     ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
     expectNoSocketClientResponse(socketPair[1]);
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
-    addPopenContents(kIPv4TetherCounters, kIPv6TetherCounters);
+    addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
     filter = BandwidthController::TetherStats("wlan0", "rmnet0", -1, -1, -1, -1);
     expected = "221 wlan0 rmnet0 10002373 10026 20002002 20027\n";
     ASSERT_EQ(0, mBw.getTetherStats(&cli, filter, err));
     ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
     // Select nonexistent interfaces.
-    addPopenContents(kIPv4TetherCounters, kIPv6TetherCounters);
+    addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
     filter = BandwidthController::TetherStats("rmnet0", "foo0", -1, -1, -1, -1);
     expected = "200 Tethering stats list completed\n";
     ASSERT_EQ(0, mBw.getTetherStats(&cli, filter, err));
     ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
     // No stats with a filter: no error.
-    addPopenContents("", "");
+    addIptablesRestoreOutput("", "");
     ASSERT_EQ(0, mBw.getTetherStats(&cli, filter, err));
     ASSERT_EQ("200 Tethering stats list completed\n", readSocketClientResponse(socketPair[1]));
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
-    addPopenContents("foo", "foo");
+    addIptablesRestoreOutput("foo", "foo");
     ASSERT_EQ(0, mBw.getTetherStats(&cli, filter, err));
     ASSERT_EQ("200 Tethering stats list completed\n", readSocketClientResponse(socketPair[1]));
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
     // No stats and empty filter: error.
     filter = BandwidthController::TetherStats();
-    addPopenContents("", kIPv6TetherCounters);
+    addIptablesRestoreOutput("", kIPv6TetherCounters);
     ASSERT_EQ(-1, mBw.getTetherStats(&cli, filter, err));
     expectNoSocketClientResponse(socketPair[1]);
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
-    addPopenContents(kIPv4TetherCounters, "");
+    addIptablesRestoreOutput(kIPv4TetherCounters, "");
     ASSERT_EQ(-1, mBw.getTetherStats(&cli, filter, err));
     expectNoSocketClientResponse(socketPair[1]);
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
     // Include only one pair of interfaces and things are fine.
     std::vector<std::string> counterLines = android::base::Split(kIPv4TetherCounters, "\n");
     std::vector<std::string> brokenCounterLines = counterLines;
     counterLines.resize(4);
     std::string counters = android::base::Join(counterLines, "\n") + "\n";
-    addPopenContents(counters, counters);
+    addIptablesRestoreOutput(counters, counters);
     expected =
             "114 wlan0 rmnet0 4746 52 4004 54\n"
             "200 Tethering stats list completed\n";
     ASSERT_EQ(0, mBw.getTetherStats(&cli, filter, err));
     ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 
     // But if interfaces aren't paired, it's always an error.
+    err = "";
     counterLines.resize(3);
     counters = android::base::Join(counterLines, "\n") + "\n";
-    addPopenContents(counters, counters);
+    addIptablesRestoreOutput(counters, counters);
     ASSERT_EQ(-1, mBw.getTetherStats(&cli, filter, err));
     expectNoSocketClientResponse(socketPair[1]);
-    clearPopenContents();
+    clearIptablesRestoreOutput();
+
+    // Token unit test of the fact that we return the stats in the error message which the caller
+    // ignores.
+    std::string expectedError = counters;
+    EXPECT_EQ(expectedError, err);
 
     // popen() failing is always an error.
-    addPopenContents(kIPv4TetherCounters);
+    addIptablesRestoreOutput(kIPv4TetherCounters);
     ASSERT_EQ(-1, mBw.getTetherStats(&cli, filter, err));
     expectNoSocketClientResponse(socketPair[1]);
-    clearPopenContents();
-    addPopenContents(kIPv6TetherCounters);
+    clearIptablesRestoreOutput();
+    addIptablesRestoreOutput(kIPv6TetherCounters);
     ASSERT_EQ(-1, mBw.getTetherStats(&cli, filter, err));
     expectNoSocketClientResponse(socketPair[1]);
-    clearPopenContents();
+    clearIptablesRestoreOutput();
 }