Merge "bpf network stats - move double accounting tag!=0 out of bpf"
diff --git a/bpf_progs/netd.c b/bpf_progs/netd.c
index e068d8a..d98fa5f 100644
--- a/bpf_progs/netd.c
+++ b/bpf_progs/netd.c
@@ -446,11 +446,6 @@
return match;
}
- if (key.tag) {
- update_stats_with_config(skb, egress, &key, *selectedMap);
- key.tag = 0;
- }
-
do_packet_tracing(skb, egress, uid, tag, enable_tracing, kver);
update_stats_with_config(skb, egress, &key, *selectedMap);
update_app_uid_stats_map(skb, egress, &uid);
diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp
index cdcb0f8..64a7a98 100644
--- a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp
+++ b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp
@@ -126,7 +126,13 @@
if (!statsEntry.ok()) {
return base::ResultError(statsEntry.error().message(), statsEntry.error().code());
}
- lines->push_back(populateStatsEntry(key, statsEntry.value(), ifname));
+ stats_line newLine = populateStatsEntry(key, statsEntry.value(), ifname);
+ lines->push_back(newLine);
+ if (newLine.tag) {
+ // account tagged traffic in the untagged stats (for historical reasons?)
+ newLine.tag = 0;
+ lines->push_back(newLine);
+ }
return Result<void>();
};
Result<void> res = statsMap.iterate(processDetailUidStats);
@@ -236,21 +242,20 @@
std::sort(lines->begin(), lines->end());
// Similar to std::unique(), but aggregates the duplicates rather than discarding them.
- size_t nextOutput = 0;
+ size_t currentOutput = 0;
for (size_t i = 1; i < lines->size(); i++) {
- if (lines->at(nextOutput) == lines->at(i)) {
- lines->at(nextOutput) += lines->at(i);
+ // note that == operator only compares the 'key' portion: iface/uid/tag/set
+ if (lines->at(currentOutput) == lines->at(i)) {
+ // while += operator only affects the 'data' portion: {rx,tx}{Bytes,Packets}
+ lines->at(currentOutput) += lines->at(i);
} else {
- nextOutput++;
- if (nextOutput != i) {
- lines->at(nextOutput) = lines->at(i);
- }
+ // okay, we're done aggregating the current line, move to the next one
+ lines->at(++currentOutput) = lines->at(i);
}
}
- if (lines->size() != nextOutput + 1) {
- lines->resize(nextOutput + 1);
- }
+ // possibly shrink the vector - currentOutput is the last line with valid data
+ lines->resize(currentOutput + 1);
}
// True if lhs equals to rhs, only compare iface, uid, tag and set.
diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp
index bf42b62..f8d9ec8 100644
--- a/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp
+++ b/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp
@@ -291,7 +291,7 @@
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
std::vector<stats_line> lines;
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
- ASSERT_EQ((unsigned long)4, lines.size());
+ ASSERT_EQ((unsigned long)7, lines.size());
}
TEST_F(BpfNetworkStatsHelperTest, TestGetStatsWithSkippedIface) {
@@ -409,12 +409,18 @@
.txPackets = TEST_PACKET0,
.txBytes = TEST_BYTES0,
};
- StatsValue value3 = {
+ StatsValue value3 = { // value1 *2
.rxPackets = TEST_PACKET0 * 2,
.rxBytes = TEST_BYTES0 * 2,
.txPackets = TEST_PACKET1 * 2,
.txBytes = TEST_BYTES1 * 2,
};
+ StatsValue value5 = { // value2 + value3
+ .rxPackets = TEST_PACKET1 + TEST_PACKET0 * 2,
+ .rxBytes = TEST_BYTES1 + TEST_BYTES0 * 2,
+ .txPackets = TEST_PACKET0 + TEST_PACKET1 * 2,
+ .txBytes = TEST_BYTES0 + TEST_BYTES1 * 2,
+ };
std::vector<stats_line> lines;
@@ -426,8 +432,9 @@
// Test 1 line stats.
populateFakeStats(TEST_UID1, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
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]);
+ ASSERT_EQ((size_t) 2, lines.size()); // TEST_TAG != 0 -> 1 entry becomes 2 lines
+ expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, 0, lines[0]);
+ expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[1]);
lines.clear();
// These items should not be grouped.
@@ -437,7 +444,7 @@
mFakeStatsMap);
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
- ASSERT_EQ((size_t) 5, lines.size());
+ ASSERT_EQ((size_t) 9, lines.size());
lines.clear();
// These items should be grouped.
@@ -445,14 +452,18 @@
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX3, TEST_COUNTERSET0, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
- ASSERT_EQ((size_t) 5, lines.size());
+ ASSERT_EQ((size_t) 9, lines.size());
// Verify Sorted & Grouped.
- expectStatsLineEqual(value3, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[0]);
- expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET1, TEST_TAG, lines[1]);
- expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG + 1, lines[2]);
- expectStatsLineEqual(value3, IFACE_NAME1, TEST_UID2, TEST_COUNTERSET0, TEST_TAG, lines[3]);
- expectStatsLineEqual(value2, IFACE_NAME2, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[4]);
+ expectStatsLineEqual(value5, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, 0, lines[0]);
+ expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET1, 0, lines[1]);
+ expectStatsLineEqual(value3, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[2]);
+ expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET1, TEST_TAG, lines[3]);
+ expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG + 1, lines[4]);
+ expectStatsLineEqual(value3, IFACE_NAME1, TEST_UID2, TEST_COUNTERSET0, 0, lines[5]);
+ expectStatsLineEqual(value3, IFACE_NAME1, TEST_UID2, TEST_COUNTERSET0, TEST_TAG, lines[6]);
+ expectStatsLineEqual(value2, IFACE_NAME2, TEST_UID1, TEST_COUNTERSET0, 0, lines[7]);
+ expectStatsLineEqual(value2, IFACE_NAME2, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[8]);
lines.clear();
// Perform test on IfaceStats.
@@ -485,39 +496,48 @@
.txPackets = TEST_PACKET1,
.txBytes = TEST_BYTES1,
};
+ StatsValue value4 = { // value1 * 4
+ .rxPackets = TEST_PACKET0 * 4,
+ .rxBytes = TEST_BYTES0 * 4,
+ .txPackets = TEST_PACKET1 * 4,
+ .txBytes = TEST_BYTES1 * 4,
+ };
// Mutate uid, 0 < TEST_UID1 < INT_MAX < INT_MIN < UINT_MAX.
- populateFakeStats(0, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
- populateFakeStats(UINT_MAX, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
- populateFakeStats(INT_MIN, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
- populateFakeStats(INT_MAX, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
+ populateFakeStats(0, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
+ populateFakeStats(UINT_MAX, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
+ populateFakeStats(INT_MIN, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
+ populateFakeStats(INT_MAX, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
// Mutate tag, 0 < TEST_TAG < INT_MAX < INT_MIN < UINT_MAX.
- populateFakeStats(TEST_UID1, INT_MAX, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
- populateFakeStats(TEST_UID1, INT_MIN, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
- populateFakeStats(TEST_UID1, 0, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
+ populateFakeStats(TEST_UID1, INT_MAX, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
+ populateFakeStats(TEST_UID1, INT_MIN, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
+ populateFakeStats(TEST_UID1, 0, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
populateFakeStats(TEST_UID1, UINT_MAX, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
// TODO: Mutate counterSet and enlarge TEST_MAP_SIZE if overflow on counterSet is possible.
std::vector<stats_line> lines;
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
- ASSERT_EQ((size_t) 8, lines.size());
+ ASSERT_EQ((size_t) 12, lines.size());
// Uid 0 first
- expectStatsLineEqual(value1, IFACE_NAME1, 0, TEST_COUNTERSET0, TEST_TAG, lines[0]);
+ expectStatsLineEqual(value1, IFACE_NAME1, 0, TEST_COUNTERSET0, 0, lines[0]);
+ expectStatsLineEqual(value1, IFACE_NAME1, 0, TEST_COUNTERSET0, TEST_TAG, lines[1]);
// Test uid, mutate tag.
- expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, 0, lines[1]);
- expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, INT_MAX, lines[2]);
- expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, INT_MIN, lines[3]);
- expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, UINT_MAX, lines[4]);
+ expectStatsLineEqual(value4, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, 0, lines[2]);
+ expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, INT_MAX, lines[3]);
+ expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, INT_MIN, lines[4]);
+ expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, UINT_MAX, lines[5]);
// Mutate uid.
- expectStatsLineEqual(value1, IFACE_NAME1, INT_MAX, TEST_COUNTERSET0, TEST_TAG, lines[5]);
- expectStatsLineEqual(value1, IFACE_NAME1, INT_MIN, TEST_COUNTERSET0, TEST_TAG, lines[6]);
- expectStatsLineEqual(value1, IFACE_NAME1, UINT_MAX, TEST_COUNTERSET0, TEST_TAG, lines[7]);
- lines.clear();
+ expectStatsLineEqual(value1, IFACE_NAME1, INT_MAX, TEST_COUNTERSET0, 0, lines[6]);
+ expectStatsLineEqual(value1, IFACE_NAME1, INT_MAX, TEST_COUNTERSET0, TEST_TAG, lines[7]);
+ expectStatsLineEqual(value1, IFACE_NAME1, INT_MIN, TEST_COUNTERSET0, 0, lines[8]);
+ expectStatsLineEqual(value1, IFACE_NAME1, INT_MIN, TEST_COUNTERSET0, TEST_TAG, lines[9]);
+ expectStatsLineEqual(value1, IFACE_NAME1, UINT_MAX, TEST_COUNTERSET0, 0, lines[10]);
+ expectStatsLineEqual(value1, IFACE_NAME1, UINT_MAX, TEST_COUNTERSET0, TEST_TAG, lines[11]);
}
} // namespace bpf
} // namespace android