bpf network stats - remove dead code

only the test code ever passes in anything that isn't
a limit {UID_ALL, INTERFACES_ALL, TAG_ALL} (ie. no limit)

Test: TreeHugger, atest libnetworkstats_test FrameworksNetTests
Bug: 276296921
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ida489f25c4da4b12541c6001b41d9e4b30804eff
This commit is contained in:
Maciej Żenczykowski
2023-04-11 07:28:24 +00:00
parent 8d2391a6aa
commit b548e191de
6 changed files with 27 additions and 116 deletions

View File

@@ -170,23 +170,10 @@ static int statsLinesToNetworkStats(JNIEnv* env, jclass clazz, jobject stats,
return 0;
}
static int readNetworkStatsDetail(JNIEnv* env, jclass clazz, jobject stats, jint limitUid,
jobjectArray limitIfacesObj, jint limitTag) {
std::vector<std::string> limitIfaces;
if (limitIfacesObj != NULL && env->GetArrayLength(limitIfacesObj) > 0) {
int num = env->GetArrayLength(limitIfacesObj);
for (int i = 0; i < num; i++) {
jstring string = (jstring)env->GetObjectArrayElement(limitIfacesObj, i);
ScopedUtfChars string8(env, string);
if (string8.c_str() != NULL) {
limitIfaces.push_back(std::string(string8.c_str()));
}
}
}
static int readNetworkStatsDetail(JNIEnv* env, jclass clazz, jobject stats) {
std::vector<stats_line> lines;
if (parseBpfNetworkStatsDetail(&lines, limitIfaces, limitTag, limitUid) < 0)
if (parseBpfNetworkStatsDetail(&lines) < 0)
return -1;
return statsLinesToNetworkStats(env, clazz, stats, lines);
@@ -202,8 +189,7 @@ static int readNetworkStatsDev(JNIEnv* env, jclass clazz, jobject stats) {
}
static const JNINativeMethod gMethods[] = {
{ "nativeReadNetworkStatsDetail",
"(Landroid/net/NetworkStats;I[Ljava/lang/String;I)I",
{ "nativeReadNetworkStatsDetail", "(Landroid/net/NetworkStats;)I",
(void*) readNetworkStatsDetail },
{ "nativeReadNetworkStatsDev", "(Landroid/net/NetworkStats;)I",
(void*) readNetworkStatsDev },

View File

@@ -110,12 +110,11 @@ stats_line populateStatsEntry(const StatsKey& statsKey, const StatsValue& statsE
}
int parseBpfNetworkStatsDetailInternal(std::vector<stats_line>* lines,
const std::vector<std::string>& limitIfaces, int limitTag,
int limitUid, const BpfMap<StatsKey, StatsValue>& statsMap,
const BpfMap<StatsKey, StatsValue>& statsMap,
const BpfMap<uint32_t, IfaceValue>& ifaceMap) {
int64_t unknownIfaceBytesTotal = 0;
const auto processDetailUidStats =
[lines, &limitIfaces, &limitTag, &limitUid, &unknownIfaceBytesTotal, &ifaceMap](
[lines, &unknownIfaceBytesTotal, &ifaceMap](
const StatsKey& key,
const BpfMap<StatsKey, StatsValue>& statsMap) -> Result<void> {
char ifname[IFNAMSIZ];
@@ -123,18 +122,6 @@ int parseBpfNetworkStatsDetailInternal(std::vector<stats_line>* lines,
&unknownIfaceBytesTotal)) {
return Result<void>();
}
std::string ifnameStr(ifname);
if (limitIfaces.size() > 0 &&
std::find(limitIfaces.begin(), limitIfaces.end(), ifnameStr) == limitIfaces.end()) {
// Nothing matched; skip this line.
return Result<void>();
}
if (limitTag != TAG_ALL && uint32_t(limitTag) != key.tag) {
return Result<void>();
}
if (limitUid != UID_ALL && uint32_t(limitUid) != key.uid) {
return Result<void>();
}
Result<StatsValue> statsEntry = statsMap.readValue(key);
if (!statsEntry.ok()) {
return base::ResultError(statsEntry.error().message(), statsEntry.error().code());
@@ -162,9 +149,7 @@ int parseBpfNetworkStatsDetailInternal(std::vector<stats_line>* lines,
return 0;
}
int parseBpfNetworkStatsDetail(std::vector<stats_line>* lines,
const std::vector<std::string>& limitIfaces, int limitTag,
int limitUid) {
int parseBpfNetworkStatsDetail(std::vector<stats_line>* lines) {
static BpfMapRO<uint32_t, IfaceValue> ifaceIndexNameMap(IFACE_INDEX_NAME_MAP_PATH);
static BpfMapRO<uint32_t, uint32_t> configurationMap(CONFIGURATION_MAP_PATH);
static BpfMap<StatsKey, StatsValue> statsMapA(STATS_MAP_A_PATH);
@@ -195,8 +180,7 @@ int parseBpfNetworkStatsDetail(std::vector<stats_line>* lines,
// TODO: the above comment feels like it may be obsolete / out of date,
// since we no longer swap the map via netd binder rpc - though we do
// still swap it.
int ret = parseBpfNetworkStatsDetailInternal(lines, limitIfaces, limitTag, limitUid,
*inactiveStatsMap, ifaceIndexNameMap);
int ret = parseBpfNetworkStatsDetailInternal(lines, *inactiveStatsMap, ifaceIndexNameMap);
if (ret) {
ALOGE("parse detail network stats failed: %s", strerror(errno));
return ret;

View File

@@ -225,18 +225,11 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetUidStatsTotal) {
ASSERT_EQ(0, bpfGetUidStatsInternal(TEST_UID2, &result2, mFakeAppUidStatsMap));
expectStatsEqual(value2, result2);
std::vector<stats_line> lines;
std::vector<std::string> ifaces;
populateFakeStats(TEST_UID1, 0, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
populateFakeStats(TEST_UID1, 0, IFACE_INDEX2, TEST_COUNTERSET1, value1, mFakeStatsMap);
populateFakeStats(TEST_UID2, 0, IFACE_INDEX3, TEST_COUNTERSET1, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, TEST_UID1,
mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)2, lines.size());
lines.clear();
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, TEST_UID2,
mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)1, lines.size());
expectStatsLineEqual(value1, IFACE_NAME3, TEST_UID2, TEST_COUNTERSET1, 0, lines.front());
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)3, lines.size());
}
TEST_F(BpfNetworkStatsHelperTest, TestGetIfaceStatsInternal) {
@@ -297,24 +290,8 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsDetail) {
mFakeStatsMap);
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
std::vector<stats_line> lines;
std::vector<std::string> ifaces;
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, UID_ALL, mFakeStatsMap,
mFakeIfaceIndexNameMap));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)4, lines.size());
lines.clear();
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, TEST_UID1,
mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)3, lines.size());
lines.clear();
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TEST_TAG, TEST_UID1,
mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)2, lines.size());
lines.clear();
ifaces.push_back(std::string(IFACE_NAME1));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TEST_TAG, TEST_UID1,
mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)1, lines.size());
expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines.front());
}
TEST_F(BpfNetworkStatsHelperTest, TestGetStatsWithSkippedIface) {
@@ -333,24 +310,8 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsWithSkippedIface) {
populateFakeStats(TEST_UID1, 0, IFACE_INDEX1, TEST_COUNTERSET1, value1, mFakeStatsMap);
populateFakeStats(TEST_UID2, 0, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
std::vector<stats_line> lines;
std::vector<std::string> ifaces;
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, UID_ALL, mFakeStatsMap,
mFakeIfaceIndexNameMap));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)4, lines.size());
lines.clear();
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, TEST_UID1,
mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)3, lines.size());
lines.clear();
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, TEST_UID2,
mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)1, lines.size());
expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID2, TEST_COUNTERSET0, 0, lines.front());
lines.clear();
ifaces.push_back(std::string(IFACE_NAME1));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, TEST_UID1,
mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)2, lines.size());
}
TEST_F(BpfNetworkStatsHelperTest, TestUnknownIfaceError) {
@@ -387,10 +348,8 @@ TEST_F(BpfNetworkStatsHelperTest, TestUnknownIfaceError) {
ifname, curKey, &unknownIfaceBytesTotal));
ASSERT_EQ(-1, unknownIfaceBytesTotal);
std::vector<stats_line> lines;
std::vector<std::string> ifaces;
// TODO: find a way to test the total of unknown Iface Bytes go above limit.
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, UID_ALL, mFakeStatsMap,
mFakeIfaceIndexNameMap));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((unsigned long)1, lines.size());
expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, 0, lines.front());
}
@@ -458,18 +417,15 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortedAndGrouped) {
};
std::vector<stats_line> lines;
std::vector<std::string> ifaces;
// Test empty stats.
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, UID_ALL, mFakeStatsMap,
mFakeIfaceIndexNameMap));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 0, lines.size());
lines.clear();
// Test 1 line stats.
populateFakeStats(TEST_UID1, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, UID_ALL, mFakeStatsMap,
mFakeIfaceIndexNameMap));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 1, lines.size());
expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[0]);
lines.clear();
@@ -480,8 +436,7 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortedAndGrouped) {
populateFakeStats(TEST_UID1, TEST_TAG + 1, IFACE_INDEX1, TEST_COUNTERSET0, value2,
mFakeStatsMap);
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, UID_ALL, mFakeStatsMap,
mFakeIfaceIndexNameMap));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 5, lines.size());
lines.clear();
@@ -489,8 +444,7 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortedAndGrouped) {
populateFakeStats(TEST_UID1, TEST_TAG, IFACE_INDEX3, TEST_COUNTERSET0, value1, mFakeStatsMap);
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX3, TEST_COUNTERSET0, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, UID_ALL, mFakeStatsMap,
mFakeIfaceIndexNameMap));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 5, lines.size());
// Verify Sorted & Grouped.
@@ -547,9 +501,7 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortAndOverflow) {
// TODO: Mutate counterSet and enlarge TEST_MAP_SIZE if overflow on counterSet is possible.
std::vector<stats_line> lines;
std::vector<std::string> ifaces;
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, ifaces, TAG_ALL, UID_ALL, mFakeStatsMap,
mFakeIfaceIndexNameMap));
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 8, lines.size());
// Uid 0 first

View File

@@ -26,7 +26,7 @@ namespace bpf {
// TODO: set this to a proper value based on the map size;
constexpr int TAG_STATS_MAP_SOFT_LIMIT = 3;
constexpr int UID_ALL = -1;
constexpr int TAG_ALL = -1;
//constexpr int TAG_ALL = -1;
constexpr int TAG_NONE = 0;
constexpr int SET_ALL = -1;
constexpr int SET_DEFAULT = 0;
@@ -64,8 +64,7 @@ int bpfGetIfaceStatsInternal(const char* iface, Stats* stats,
const BpfMap<uint32_t, IfaceValue>& ifaceNameMap);
// For test only
int parseBpfNetworkStatsDetailInternal(std::vector<stats_line>* lines,
const std::vector<std::string>& limitIfaces, int limitTag,
int limitUid, const BpfMap<StatsKey, StatsValue>& statsMap,
const BpfMap<StatsKey, StatsValue>& statsMap,
const BpfMap<uint32_t, IfaceValue>& ifaceMap);
// For test only
int cleanStatsMapInternal(const base::unique_fd& cookieTagMap, const base::unique_fd& tagStatsMap);
@@ -113,9 +112,7 @@ int parseBpfNetworkStatsDevInternal(std::vector<stats_line>* lines,
int bpfGetUidStats(uid_t uid, Stats* stats);
int bpfGetIfaceStats(const char* iface, Stats* stats);
int parseBpfNetworkStatsDetail(std::vector<stats_line>* lines,
const std::vector<std::string>& limitIfaces, int limitTag,
int limitUid);
int parseBpfNetworkStatsDetail(std::vector<stats_line>* lines);
int parseBpfNetworkStatsDev(std::vector<stats_line>* lines);
void groupNetworkStats(std::vector<stats_line>* lines);

View File

@@ -84,12 +84,9 @@ public class NetworkStatsFactory {
* are expected to monotonically increase since device boot.
*/
@NonNull
public NetworkStats getNetworkStatsDetail(int limitUid, @Nullable String[] limitIfaces,
int limitTag) throws IOException {
public NetworkStats getNetworkStatsDetail() throws IOException {
final NetworkStats stats = new NetworkStats(SystemClock.elapsedRealtime(), 0);
// TODO: remove both path and useBpfStats arguments.
// The path is never used if useBpfStats is true.
final int ret = nativeReadNetworkStatsDetail(stats, limitUid, limitIfaces, limitTag);
final int ret = nativeReadNetworkStatsDetail(stats);
if (ret != 0) {
throw new IOException("Failed to parse network stats");
}
@@ -213,8 +210,7 @@ public class NetworkStatsFactory {
requestSwapActiveStatsMapLocked();
// Stats are always read from the inactive map, so they must be read after the
// swap
final NetworkStats stats = mDeps.getNetworkStatsDetail(
UID_ALL, INTERFACES_ALL, TAG_ALL);
final NetworkStats stats = mDeps.getNetworkStatsDetail();
// BPF stats are incremental; fold into mPersistSnapshot.
mPersistSnapshot.setElapsedRealtime(stats.getElapsedRealtime());
mPersistSnapshot.combineAllValues(stats);
@@ -301,8 +297,7 @@ public class NetworkStatsFactory {
* are expected to monotonically increase since device boot.
*/
@VisibleForTesting
public static native int nativeReadNetworkStatsDetail(NetworkStats stats, int limitUid,
String[] limitIfaces, int limitTag);
public static native int nativeReadNetworkStatsDetail(NetworkStats stats);
@VisibleForTesting
public static native int nativeReadNetworkStatsDev(NetworkStats stats);

View File

@@ -472,8 +472,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest {
256L, 16L, 512L, 32L, 0L)
.insertEntry(TEST_IFACE, UID_GREEN, SET_DEFAULT, TAG_NONE, 64L, 3L, 1024L, 8L, 0L);
doReturn(stats).when(mDeps).getNetworkStatsDetail(anyInt(), any(),
anyInt());
doReturn(stats).when(mDeps).getNetworkStatsDetail();
final String[] ifaces = new String[]{TEST_IFACE};
final NetworkStats res = mFactory.readNetworkStatsDetail(UID_ALL, ifaces, TAG_ALL);
@@ -488,8 +487,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest {
mFactory.removeUidsLocked(removedUids);
// Return empty stats for reading the result of removing uids stats later.
doReturn(buildEmptyStats()).when(mDeps).getNetworkStatsDetail(anyInt(), any(),
anyInt());
doReturn(buildEmptyStats()).when(mDeps).getNetworkStatsDetail();
final NetworkStats removedUidsStats =
mFactory.readNetworkStatsDetail(UID_ALL, ifaces, TAG_ALL);
@@ -574,8 +572,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest {
final NetworkStats statsFromResource = parseNetworkStatsFromGoldenSample(resourceId,
24 /* initialSize */, true /* consumeHeader */, false /* checkActive */,
true /* isUidData */);
doReturn(statsFromResource).when(mDeps).getNetworkStatsDetail(anyInt(), any(),
anyInt());
doReturn(statsFromResource).when(mDeps).getNetworkStatsDetail();
return mFactory.readNetworkStatsDetail();
}