Remove support for filtering tether stats.
The framework does not use this filtering, so it's just
dead code. It also requires that NatController publicly
expose its list of interface pairs.
Also make the parsing code a bit stricter - for example, return
an error if any of the lines (except the headers) fail to parse.
Bug: 32163131
Bug: 64995262
Test: bullhead builds and boots
Test: netd_{unit,integration}_test pass
Test: output of "adb shell ndc bandwidth gettetherstats" looks correct
Change-Id: Ib7440f935809c59d8b48396764cc63eb95f509b4
diff --git a/server/CommandListener.cpp b/server/CommandListener.cpp
index 610b96b..0ca24b4 100644
--- a/server/CommandListener.cpp
+++ b/server/CommandListener.cpp
@@ -972,21 +972,17 @@
return 0;
}
- if (!strcmp(argv[1], "gettetherstats") || !strcmp(argv[1], "gts")) {
- TetherController::TetherStats tetherStats;
+ if (!strcmp(argv[1], "gettetherstats")) {
std::string extraProcessingInfo = "";
- if (argc < 2 || argc > 4) {
- sendGenericSyntaxError(cli, "gettetherstats [<intInterface> <extInterface>]");
+ if (argc != 2) {
+ sendGenericSyntaxError(cli, "gettetherstats");
return 0;
}
- 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 && gCtls->tetherCtrl.ifacePairList.empty()) {
+ if (gCtls->tetherCtrl.ifacePairList.empty()) {
cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
return 0;
}
- int rc = gCtls->tetherCtrl.getTetherStats(cli, tetherStats, extraProcessingInfo);
+ int rc = gCtls->tetherCtrl.getTetherStats(cli, extraProcessingInfo);
if (rc) {
extraProcessingInfo.insert(0, "Failed to get tethering stats.\n");
sendGenericOpFailed(cli, extraProcessingInfo.c_str());
diff --git a/server/TetherController.cpp b/server/TetherController.cpp
index 094170f..543eaef 100644
--- a/server/TetherController.cpp
+++ b/server/TetherController.cpp
@@ -628,11 +628,8 @@
* 0 0 RETURN all wlan0 rmnet_data0 ::/0 ::/0
* 0 0 RETURN all rmnet_data0 wlan0 ::/0 ::/0
*
- * It results in an error if invoked and no tethering counter rules exist. The constraint
- * helps detect complete parsing failure.
*/
-int TetherController::addForwardChainStats(const TetherStats& filter,
- TetherStatsList& statsList,
+int TetherController::addForwardChainStats(TetherStatsList& statsList,
const std::string& statsOutput,
std::string &extraProcessingInfo) {
int res;
@@ -642,17 +639,23 @@
char rest[MAX_IPT_OUTPUT_LINE_LEN];
TetherStats stats;
+ const TetherStats empty;
const char *buffPtr;
int64_t packets, bytes;
int statsFound = 0;
- bool filterPair = filter.intIface[0] && filter.extIface[0];
-
- ALOGV("filter: %s", filter.getStatsLine().c_str());
-
- stats = filter;
-
std::stringstream stream(statsOutput);
+
+ // Skip headers.
+ for (int i = 0; i < 2; i++) {
+ std::getline(stream, statsLine, '\n');
+ extraProcessingInfo += statsLine + "\n";
+ if (statsLine.empty()) {
+ ALOGE("Empty header while parsing tethering stats");
+ return -1;
+ }
+ }
+
while (std::getline(stream, statsLine, '\n')) {
buffPtr = statsLine.c_str();
@@ -675,66 +678,34 @@
extraProcessingInfo += "\n";
if (res != 5) {
- continue;
+ return -EREMOTEIO;
}
/*
* The following assumes that the 1st rule has in:extIface out:intIface,
* which is what TetherController sets up.
- * If not filtering, the 1st match rx, and sets up the pair for the tx side.
+ * The 1st matches rx, and sets up the pair for the tx side.
*/
- if (filter.intIface[0] && filter.extIface[0]) {
- if (filter.intIface == iface0 && filter.extIface == iface1) {
- ALOGV("2Filter RX iface_in=%s iface_out=%s rx_bytes=%" PRId64" rx_packets=%" PRId64" ", iface0, iface1, bytes, packets);
- stats.rxPackets = packets;
- stats.rxBytes = bytes;
- } else if (filter.intIface == iface1 && filter.extIface == iface0) {
- ALOGV("2Filter TX iface_in=%s iface_out=%s rx_bytes=%" PRId64" rx_packets=%" PRId64" ", iface0, iface1, bytes, packets);
- stats.txPackets = packets;
- stats.txBytes = bytes;
- }
- } else if (filter.intIface[0] || filter.extIface[0]) {
- if (filter.intIface == iface0 || filter.extIface == iface1) {
- ALOGV("1Filter RX iface_in=%s iface_out=%s rx_bytes=%" PRId64" rx_packets=%" PRId64" ", iface0, iface1, bytes, packets);
- stats.intIface = iface0;
- stats.extIface = iface1;
- stats.rxPackets = packets;
- stats.rxBytes = bytes;
- } else if (filter.intIface == iface1 || filter.extIface == iface0) {
- ALOGV("1Filter TX iface_in=%s iface_out=%s rx_bytes=%" PRId64" rx_packets=%" PRId64" ", iface0, iface1, bytes, packets);
- stats.intIface = iface1;
- stats.extIface = iface0;
- stats.txPackets = packets;
- stats.txBytes = bytes;
- }
- } else /* if (!filter.intFace[0] && !filter.extIface[0]) */ {
- if (!stats.intIface[0]) {
- ALOGV("0Filter RX iface_in=%s iface_out=%s rx_bytes=%" PRId64" rx_packets=%" PRId64" ", iface0, iface1, bytes, packets);
- stats.intIface = iface0;
- stats.extIface = iface1;
- stats.rxPackets = packets;
- stats.rxBytes = bytes;
- } else if (stats.intIface == iface1 && stats.extIface == iface0) {
- ALOGV("0Filter TX iface_in=%s iface_out=%s rx_bytes=%" PRId64" rx_packets=%" PRId64" ", iface0, iface1, bytes, packets);
- stats.txPackets = packets;
- stats.txBytes = bytes;
- }
+ if (!stats.intIface[0]) {
+ ALOGV("0Filter RX iface_in=%s iface_out=%s rx_bytes=%" PRId64" rx_packets=%" PRId64" ", iface0, iface1, bytes, packets);
+ stats.intIface = iface0;
+ stats.extIface = iface1;
+ stats.rxPackets = packets;
+ stats.rxBytes = bytes;
+ } else if (stats.intIface == iface1 && stats.extIface == iface0) {
+ ALOGV("0Filter TX iface_in=%s iface_out=%s rx_bytes=%" PRId64" rx_packets=%" PRId64" ", iface0, iface1, bytes, packets);
+ stats.txPackets = packets;
+ stats.txBytes = bytes;
}
if (stats.rxBytes != -1 && stats.txBytes != -1) {
- ALOGV("rx_bytes=%" PRId64" tx_bytes=%" PRId64" filterPair=%d", stats.rxBytes, stats.txBytes, filterPair);
+ ALOGV("rx_bytes=%" PRId64" tx_bytes=%" PRId64, stats.rxBytes, stats.txBytes);
addStats(statsList, stats);
- if (filterPair) {
- return 0;
- } else {
- statsFound++;
- stats = filter;
- }
+ statsFound++;
+ stats = empty;
}
}
/* 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)) {
+ if (((stats.rxBytes == -1) != (stats.txBytes == -1)) || !statsFound) {
return -1;
}
return 0;
@@ -747,40 +718,28 @@
return msg;
}
-int TetherController::getTetherStats(SocketClient *cli, TetherStats& filter,
- std::string &extraProcessingInfo) {
- int res = 0;
-
+int TetherController::getTetherStats(SocketClient *cli, std::string &extraProcessingInfo) {
TetherStatsList statsList;
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);
+ if (int ret = iptablesRestoreFunction(target, GET_TETHER_STATS_COMMAND, &statsString)) {
+ ALOGE("Failed to run %s err=%d", GET_TETHER_STATS_COMMAND.c_str(), ret);
return -1;
}
- res = addForwardChainStats(filter, statsList, statsString, extraProcessingInfo);
- if (res != 0) {
- return res;
+ if (int ret = addForwardChainStats(statsList, statsString, extraProcessingInfo)) {
+ return ret;
}
}
- if (filter.intIface[0] && filter.extIface[0] && statsList.size() == 1) {
- cli->sendMsg(ResponseCode::TetheringStatsResult,
- statsList[0].getStatsLine().c_str(), false);
- } else {
- for (const auto& stats: statsList) {
- cli->sendMsg(ResponseCode::TetheringStatsListResult,
- stats.getStatsLine().c_str(), false);
- }
- if (res == 0) {
- cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
- }
+ for (const auto& stats: statsList) {
+ cli->sendMsg(ResponseCode::TetheringStatsListResult,
+ stats.getStatsLine().c_str(), false);
}
+ cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
- return res;
+ return 0;
}
} // namespace net
diff --git a/server/TetherController.h b/server/TetherController.h
index 4ed94ee..b0da5bf 100644
--- a/server/TetherController.h
+++ b/server/TetherController.h
@@ -104,13 +104,10 @@
};
/*
- * For single pair of ifaces, stats should have ifaceIn and ifaceOut initialized.
- * For all pairs, stats should have ifaceIn=ifaceOut="".
- * Sends out to the cli the single stat (TetheringStatsReluts) or a list of stats
- * (TetheringStatsListResult+CommandOkay).
+ * Sends out to the cli a list of stats TetheringStatsListResult+CommandOkay).
* 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, std::string &extraProcessingInfo);
int getTetherStats(SocketClient *cli, TetherStats &stats, std::string &extraProcessingInfo);
typedef std::vector<TetherStats> TetherStatsList;
@@ -118,6 +115,17 @@
static void addStats(TetherStatsList& statsList, const TetherStats& stats);
/*
+ * output should be a file to the apropriate FORWARD chain of iptables rules.
+ * extraProcessingInfo: contains raw parsed data, and error info.
+ * This strongly requires that setup of the rules is in a specific order:
+ * in:intIface out:extIface
+ * in:extIface out:intIface
+ * and the rules are grouped in pairs when more that one tethering was setup.
+ */
+ static int addForwardChainStats(TetherStatsList& statsList, const std::string& iptOutput,
+ 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.
* extraProcessingInfo: contains raw parsed data, and error info.
diff --git a/server/TetherControllerTest.cpp b/server/TetherControllerTest.cpp
index 887c648..1e44db4 100644
--- a/server/TetherControllerTest.cpp
+++ b/server/TetherControllerTest.cpp
@@ -198,6 +198,11 @@
expectIptablesRestoreCommands(expected);
}
+std::string kTetherCounterHeaders = Join(std::vector<std::string> {
+ "Chain natctrl_tether_counters (4 references)",
+ " pkts bytes target prot opt in out source destination",
+}, '\n');
+
std::string kIPv4TetherCounters = Join(std::vector<std::string> {
"Chain natctrl_tether_counters (4 references)",
" pkts bytes target prot opt in out source destination",
@@ -228,7 +233,7 @@
void expectNoSocketClientResponse(int fd) {
char buf[64];
- EXPECT_EQ(-1, read(fd, buf, sizeof(buf)));
+ EXPECT_EQ(-1, read(fd, buf, sizeof(buf))) << "Unexpected response: " << buf << "\n";
}
TEST_F(TetherControllerTest, TestGetTetherStats) {
@@ -239,74 +244,36 @@
SocketClient cli(socketPair[0], false);
std::string err;
- TetherController::TetherStats filter;
// If no filter is specified, both IPv4 and IPv6 counters must have at least one interface pair.
addIptablesRestoreOutput(kIPv4TetherCounters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, filter, err));
+ ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
expectNoSocketClientResponse(socketPair[1]);
clearIptablesRestoreOutput();
addIptablesRestoreOutput(kIPv6TetherCounters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, filter, err));
+ ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
clearIptablesRestoreOutput();
// IPv4 and IPv6 counters are properly added together.
addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
- filter = TetherController::TetherStats();
std::string expected =
"114 wlan0 rmnet0 10002373 10026 20002002 20027\n"
"114 bt-pan rmnet0 107471 1040 1708806 1450\n"
"200 Tethering stats list completed\n";
- ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, filter, err));
+ ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, err));
ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
expectNoSocketClientResponse(socketPair[1]);
clearIptablesRestoreOutput();
- // Test filtering.
- addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
- filter = TetherController::TetherStats("bt-pan", "rmnet0", -1, -1, -1, -1);
- expected = "221 bt-pan rmnet0 107471 1040 1708806 1450\n";
- ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, filter, err));
- ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
- expectNoSocketClientResponse(socketPair[1]);
- clearIptablesRestoreOutput();
-
- addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
- filter = TetherController::TetherStats("wlan0", "rmnet0", -1, -1, -1, -1);
- expected = "221 wlan0 rmnet0 10002373 10026 20002002 20027\n";
- ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, filter, err));
- ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
- clearIptablesRestoreOutput();
-
- // Select nonexistent interfaces.
- addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
- filter = TetherController::TetherStats("rmnet0", "foo0", -1, -1, -1, -1);
- expected = "200 Tethering stats list completed\n";
- ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, filter, err));
- ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
- clearIptablesRestoreOutput();
-
- // No stats with a filter: no error.
- addIptablesRestoreOutput("", "");
- ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, filter, err));
- ASSERT_EQ("200 Tethering stats list completed\n", readSocketClientResponse(socketPair[1]));
- clearIptablesRestoreOutput();
-
- addIptablesRestoreOutput("foo", "foo");
- ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, filter, err));
- ASSERT_EQ("200 Tethering stats list completed\n", readSocketClientResponse(socketPair[1]));
- clearIptablesRestoreOutput();
-
- // No stats and empty filter: error.
- filter = TetherController::TetherStats();
+ // No stats: error.
addIptablesRestoreOutput("", kIPv6TetherCounters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, filter, err));
+ ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
expectNoSocketClientResponse(socketPair[1]);
clearIptablesRestoreOutput();
addIptablesRestoreOutput(kIPv4TetherCounters, "");
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, filter, err));
+ ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
expectNoSocketClientResponse(socketPair[1]);
clearIptablesRestoreOutput();
@@ -319,7 +286,7 @@
expected =
"114 wlan0 rmnet0 4746 52 4004 54\n"
"200 Tethering stats list completed\n";
- ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, filter, err));
+ ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, err));
ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
clearIptablesRestoreOutput();
@@ -328,7 +295,7 @@
counterLines.resize(3);
counters = Join(counterLines, "\n") + "\n";
addIptablesRestoreOutput(counters, counters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, filter, err));
+ ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
expectNoSocketClientResponse(socketPair[1]);
clearIptablesRestoreOutput();
@@ -336,15 +303,6 @@
// ignores.
std::string expectedError = counters;
EXPECT_EQ(expectedError, err);
-
- addIptablesRestoreOutput(kIPv4TetherCounters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, filter, err));
- expectNoSocketClientResponse(socketPair[1]);
- clearIptablesRestoreOutput();
- addIptablesRestoreOutput(kIPv6TetherCounters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, filter, err));
- expectNoSocketClientResponse(socketPair[1]);
- clearIptablesRestoreOutput();
}
} // namespace net