Remove the gettetherstats command.
Bug: 32163131
Bug: 64995262
Test: netd_{unit,integration}_test pass
Test: tethering data usage UI reflects actual data usage
Change-Id: I1722f64cf775e73d71df997f6bae4820133e67bf
diff --git a/server/CommandListener.cpp b/server/CommandListener.cpp
index 10f069e..1382160 100644
--- a/server/CommandListener.cpp
+++ b/server/CommandListener.cpp
@@ -971,25 +971,6 @@
return 0;
}
- if (!strcmp(argv[1], "gettetherstats")) {
- std::string extraProcessingInfo = "";
- if (argc != 2) {
- sendGenericSyntaxError(cli, "gettetherstats");
- return 0;
- }
- if (gCtls->tetherCtrl.ifacePairList.empty()) {
- cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
- return 0;
- }
- int rc = gCtls->tetherCtrl.getTetherStats(cli, extraProcessingInfo);
- if (rc) {
- extraProcessingInfo.insert(0, "Failed to get tethering stats.\n");
- sendGenericOpFailed(cli, extraProcessingInfo.c_str());
- return 0;
- }
- return 0;
-
- }
cli->sendMsg(ResponseCode::CommandSyntaxError, "Unknown bandwidth cmd", false);
return 0;
diff --git a/server/NetdNativeService.cpp b/server/NetdNativeService.cpp
index 3f10d7c..ec4360a 100644
--- a/server/NetdNativeService.cpp
+++ b/server/NetdNativeService.cpp
@@ -323,8 +323,7 @@
binder::Status NetdNativeService::tetherGetStats(PersistableBundle *bundle) {
NETD_LOCKING_RPC(NETWORK_STACK, gCtls->tetherCtrl.lock)
- std::string extraProcessingInfo;
- const auto& statsList = gCtls->tetherCtrl.getTetherStats(extraProcessingInfo);
+ const auto& statsList = gCtls->tetherCtrl.getTetherStats();
if (!isOk(statsList)) {
return toBinderStatus(statsList);
}
diff --git a/server/TetherController.cpp b/server/TetherController.cpp
index 334da2e..3f6efe9 100644
--- a/server/TetherController.cpp
+++ b/server/TetherController.cpp
@@ -714,16 +714,9 @@
return 0;
}
-std::string TetherController::TetherStats::getStatsLine() const {
- std::string msg;
- StringAppendF(&msg, "%s %s %" PRId64" %" PRId64" %" PRId64" %" PRId64, intIface.c_str(),
- extIface.c_str(), rxBytes, rxPackets, txBytes, txPackets);
- return msg;
-}
-
-StatusOr<TetherController::TetherStatsList> TetherController::getTetherStats(
- std::string& extraProcessingInfo) {
+StatusOr<TetherController::TetherStatsList> TetherController::getTetherStats() {
TetherStatsList statsList;
+ std::string parsedIptablesOutput;
for (const IptablesTarget target : {V4, V6}) {
std::string statsString;
@@ -732,30 +725,15 @@
target, ret));
}
- if (int ret = addForwardChainStats(statsList, statsString, extraProcessingInfo)) {
+ if (int ret = addForwardChainStats(statsList, statsString, parsedIptablesOutput)) {
return statusFromErrno(-ret, StringPrintf("failed to parse %s tether stats:\n%s",
target == V4 ? "IPv4": "IPv6",
- extraProcessingInfo.c_str()));
+ parsedIptablesOutput.c_str()));
}
}
return statsList;
}
-int TetherController::getTetherStats(SocketClient *cli, std::string &extraProcessingInfo) {
- StatusOr<TetherStatsList> statsList = getTetherStats(extraProcessingInfo);
- if (!isOk(statsList)) {
- return -1;
- }
-
- for (const auto& stats: statsList.value()) {
- cli->sendMsg(ResponseCode::TetheringStatsListResult,
- stats.getStatsLine().c_str(), false);
- }
- cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
-
- return 0;
-}
-
} // namespace net
} // namespace android
diff --git a/server/TetherController.h b/server/TetherController.h
index e715a41..df43a7b 100644
--- a/server/TetherController.h
+++ b/server/TetherController.h
@@ -88,11 +88,6 @@
int64_t rxPackets = -1;
int64_t txBytes = -1;
int64_t txPackets = -1;
- /*
- * Returns a new string representing this:
- * intIface extIface rx_bytes rx_packets tx_bytes tx_packets
- */
- std::string getStatsLine() const;
bool addStatsIfMatch(const TetherStats& other) {
if (intIface == other.intIface && extIface == other.extIface) {
@@ -108,13 +103,7 @@
typedef std::vector<TetherStats> TetherStatsList;
- StatusOr<TetherStatsList> getTetherStats(std::string &extraProcessingInfo);
-
- /*
- * Sends out to the cli a list of stats TetheringStatsListResult+CommandOkay).
- * Error is to be handled on the outside.
- */
- int getTetherStats(SocketClient *cli, std::string &extraProcessingInfo);
+ StatusOr<TetherStatsList> getTetherStats();
/*
* extraProcessingInfo: contains raw parsed data, and error info.
diff --git a/server/TetherControllerTest.cpp b/server/TetherControllerTest.cpp
index c13758c..5043d12 100644
--- a/server/TetherControllerTest.cpp
+++ b/server/TetherControllerTest.cpp
@@ -28,12 +28,16 @@
#include <android-base/stringprintf.h>
#include <android-base/strings.h>
+#include <netdutils/StatusOr.h>
#include "TetherController.h"
#include "IptablesBaseTest.h"
using android::base::Join;
using android::base::StringPrintf;
+using android::netdutils::StatusOr;
+using TetherStats = android::net::TetherController::TetherStats;
+using TetherStatsList = android::net::TetherController::TetherStatsList;
namespace android {
namespace net {
@@ -219,62 +223,45 @@
" 20000 20000000 RETURN all rmnet0 wlan0 ::/0 ::/0",
}, '\n');
-std::string readSocketClientResponse(int fd) {
- char buf[32768];
- ssize_t bytesRead = read(fd, buf, sizeof(buf));
- if (bytesRead < 0) {
- return "";
- }
- for (int i = 0; i < bytesRead; i++) {
- if (buf[i] == '\0') buf[i] = '\n';
- }
- return std::string(buf, bytesRead);
-}
-
-void expectNoSocketClientResponse(int fd) {
- char buf[64];
- EXPECT_EQ(-1, read(fd, buf, sizeof(buf))) << "Unexpected response: " << buf << "\n";
+void expectTetherStatsEqual(const TetherController::TetherStats& expected,
+ const TetherController::TetherStats& actual) {
+ EXPECT_EQ(expected.intIface, actual.intIface);
+ EXPECT_EQ(expected.extIface, actual.extIface);
+ EXPECT_EQ(expected.rxBytes, actual.rxBytes);
+ EXPECT_EQ(expected.txBytes, actual.txBytes);
+ EXPECT_EQ(expected.rxPackets, actual.rxPackets);
+ EXPECT_EQ(expected.txPackets, actual.txPackets);
}
TEST_F(TetherControllerTest, TestGetTetherStats) {
- int socketPair[2];
- ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_STREAM, 0, socketPair));
- ASSERT_EQ(0, fcntl(socketPair[0], F_SETFL, O_NONBLOCK | fcntl(socketPair[0], F_GETFL)));
- ASSERT_EQ(0, fcntl(socketPair[1], F_SETFL, O_NONBLOCK | fcntl(socketPair[1], F_GETFL)));
- SocketClient cli(socketPair[0], false);
-
- std::string err;
-
// 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, err));
- expectNoSocketClientResponse(socketPair[1]);
+ ASSERT_FALSE(isOk(mTetherCtrl.getTetherStats()));
clearIptablesRestoreOutput();
addIptablesRestoreOutput(kIPv6TetherCounters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
+ ASSERT_FALSE(isOk(mTetherCtrl.getTetherStats()));
clearIptablesRestoreOutput();
// IPv4 and IPv6 counters are properly added together.
addIptablesRestoreOutput(kIPv4TetherCounters, kIPv6TetherCounters);
- 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, err));
- ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
- expectNoSocketClientResponse(socketPair[1]);
+ TetherStats expected0("wlan0", "rmnet0", 10002373, 10026, 20002002, 20027);
+ TetherStats expected1("bt-pan", "rmnet0", 107471, 1040, 1708806, 1450);
+ StatusOr<TetherStatsList> result = mTetherCtrl.getTetherStats();
+ ASSERT_TRUE(isOk(result));
+ TetherStatsList actual = result.value();
+ ASSERT_EQ(2U, actual.size());
+ expectTetherStatsEqual(expected0, result.value()[0]);
+ expectTetherStatsEqual(expected1, result.value()[1]);
clearIptablesRestoreOutput();
// No stats: error.
addIptablesRestoreOutput("", kIPv6TetherCounters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
- expectNoSocketClientResponse(socketPair[1]);
+ ASSERT_FALSE(isOk(mTetherCtrl.getTetherStats()));
clearIptablesRestoreOutput();
addIptablesRestoreOutput(kIPv4TetherCounters, "");
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
- expectNoSocketClientResponse(socketPair[1]);
+ ASSERT_FALSE(isOk(mTetherCtrl.getTetherStats()));
clearIptablesRestoreOutput();
// Include only one pair of interfaces and things are fine.
@@ -283,24 +270,26 @@
counterLines.resize(4);
std::string counters = Join(counterLines, "\n") + "\n";
addIptablesRestoreOutput(counters, counters);
- expected =
- "114 wlan0 rmnet0 4746 52 4004 54\n"
- "200 Tethering stats list completed\n";
- ASSERT_EQ(0, mTetherCtrl.getTetherStats(&cli, err));
- ASSERT_EQ(expected, readSocketClientResponse(socketPair[1]));
+ TetherStats expected1_0("wlan0", "rmnet0", 4746, 52, 4004, 54);
+ result = mTetherCtrl.getTetherStats();
+ ASSERT_TRUE(isOk(result));
+ actual = result.value();
+ ASSERT_EQ(1U, actual.size());
+ expectTetherStatsEqual(expected1_0, actual[0]);
clearIptablesRestoreOutput();
// But if interfaces aren't paired, it's always an error.
counterLines.resize(3);
counters = Join(counterLines, "\n") + "\n";
addIptablesRestoreOutput(counters, counters);
- ASSERT_EQ(-1, mTetherCtrl.getTetherStats(&cli, err));
- expectNoSocketClientResponse(socketPair[1]);
+ result = mTetherCtrl.getTetherStats();
+ ASSERT_FALSE(isOk(result));
clearIptablesRestoreOutput();
// Token unit test of the fact that we return the stats in the error message which the caller
// ignores.
std::string expectedError = counters;
+ std::string err = result.status().msg();
ASSERT_LE(expectedError.size(), err.size());
EXPECT_TRUE(std::equal(expectedError.rbegin(), expectedError.rend(), err.rbegin()));
}