Merge "bpf network stats - move double accounting tag!=0 out of bpf"

This commit is contained in:
Maciej Żenczykowski
2023-04-17 19:25:50 +00:00
committed by Gerrit Code Review
3 changed files with 64 additions and 44 deletions

View File

@@ -446,11 +446,6 @@ static __always_inline inline int bpf_traffic_account(struct __sk_buff* skb, boo
return match; 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); do_packet_tracing(skb, egress, uid, tag, enable_tracing, kver);
update_stats_with_config(skb, egress, &key, *selectedMap); update_stats_with_config(skb, egress, &key, *selectedMap);
update_app_uid_stats_map(skb, egress, &uid); update_app_uid_stats_map(skb, egress, &uid);

View File

@@ -126,7 +126,13 @@ int parseBpfNetworkStatsDetailInternal(std::vector<stats_line>* lines,
if (!statsEntry.ok()) { if (!statsEntry.ok()) {
return base::ResultError(statsEntry.error().message(), statsEntry.error().code()); 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>(); return Result<void>();
}; };
Result<void> res = statsMap.iterate(processDetailUidStats); Result<void> res = statsMap.iterate(processDetailUidStats);
@@ -236,21 +242,20 @@ void groupNetworkStats(std::vector<stats_line>* lines) {
std::sort(lines->begin(), lines->end()); std::sort(lines->begin(), lines->end());
// Similar to std::unique(), but aggregates the duplicates rather than discarding them. // 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++) { for (size_t i = 1; i < lines->size(); i++) {
if (lines->at(nextOutput) == lines->at(i)) { // note that == operator only compares the 'key' portion: iface/uid/tag/set
lines->at(nextOutput) += lines->at(i); 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 { } else {
nextOutput++; // okay, we're done aggregating the current line, move to the next one
if (nextOutput != i) { lines->at(++currentOutput) = lines->at(i);
lines->at(nextOutput) = lines->at(i);
}
} }
} }
if (lines->size() != nextOutput + 1) { // possibly shrink the vector - currentOutput is the last line with valid data
lines->resize(nextOutput + 1); lines->resize(currentOutput + 1);
}
} }
// True if lhs equals to rhs, only compare iface, uid, tag and set. // True if lhs equals to rhs, only compare iface, uid, tag and set.

View File

@@ -291,7 +291,7 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsDetail) {
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap); populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
std::vector<stats_line> lines; std::vector<stats_line> lines;
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap)); 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) { TEST_F(BpfNetworkStatsHelperTest, TestGetStatsWithSkippedIface) {
@@ -409,12 +409,18 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortedAndGrouped) {
.txPackets = TEST_PACKET0, .txPackets = TEST_PACKET0,
.txBytes = TEST_BYTES0, .txBytes = TEST_BYTES0,
}; };
StatsValue value3 = { StatsValue value3 = { // value1 *2
.rxPackets = TEST_PACKET0 * 2, .rxPackets = TEST_PACKET0 * 2,
.rxBytes = TEST_BYTES0 * 2, .rxBytes = TEST_BYTES0 * 2,
.txPackets = TEST_PACKET1 * 2, .txPackets = TEST_PACKET1 * 2,
.txBytes = TEST_BYTES1 * 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; std::vector<stats_line> lines;
@@ -426,8 +432,9 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortedAndGrouped) {
// Test 1 line stats. // Test 1 line stats.
populateFakeStats(TEST_UID1, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap); populateFakeStats(TEST_UID1, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap)); ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 1, lines.size()); ASSERT_EQ((size_t) 2, lines.size()); // TEST_TAG != 0 -> 1 entry becomes 2 lines
expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[0]); 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(); lines.clear();
// These items should not be grouped. // These items should not be grouped.
@@ -437,7 +444,7 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortedAndGrouped) {
mFakeStatsMap); mFakeStatsMap);
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap); populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap)); ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 5, lines.size()); ASSERT_EQ((size_t) 9, lines.size());
lines.clear(); lines.clear();
// These items should be grouped. // These items should be grouped.
@@ -445,14 +452,18 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortedAndGrouped) {
populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX3, TEST_COUNTERSET0, value1, mFakeStatsMap); populateFakeStats(TEST_UID2, TEST_TAG, IFACE_INDEX3, TEST_COUNTERSET0, value1, mFakeStatsMap);
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap)); ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 5, lines.size()); ASSERT_EQ((size_t) 9, lines.size());
// Verify Sorted & Grouped. // Verify Sorted & Grouped.
expectStatsLineEqual(value3, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[0]); expectStatsLineEqual(value5, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, 0, lines[0]);
expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET1, TEST_TAG, lines[1]); expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET1, 0, lines[1]);
expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG + 1, lines[2]); expectStatsLineEqual(value3, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[2]);
expectStatsLineEqual(value3, IFACE_NAME1, TEST_UID2, TEST_COUNTERSET0, TEST_TAG, lines[3]); expectStatsLineEqual(value2, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET1, TEST_TAG, lines[3]);
expectStatsLineEqual(value2, IFACE_NAME2, TEST_UID1, TEST_COUNTERSET0, TEST_TAG, lines[4]); 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(); lines.clear();
// Perform test on IfaceStats. // Perform test on IfaceStats.
@@ -485,6 +496,12 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortAndOverflow) {
.txPackets = TEST_PACKET1, .txPackets = TEST_PACKET1,
.txBytes = TEST_BYTES1, .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. // Mutate uid, 0 < TEST_UID1 < INT_MAX < INT_MIN < UINT_MAX.
populateFakeStats(0, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap); populateFakeStats(0, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
@@ -502,22 +519,25 @@ TEST_F(BpfNetworkStatsHelperTest, TestGetStatsSortAndOverflow) {
std::vector<stats_line> lines; std::vector<stats_line> lines;
ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap)); ASSERT_EQ(0, parseBpfNetworkStatsDetailInternal(&lines, mFakeStatsMap, mFakeIfaceIndexNameMap));
ASSERT_EQ((size_t) 8, lines.size()); ASSERT_EQ((size_t) 12, lines.size());
// Uid 0 first // 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. // Test uid, mutate tag.
expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, 0, lines[1]); expectStatsLineEqual(value4, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, 0, lines[2]);
expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, INT_MAX, 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[3]); expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, INT_MIN, lines[4]);
expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, UINT_MAX, lines[4]); expectStatsLineEqual(value1, IFACE_NAME1, TEST_UID1, TEST_COUNTERSET0, UINT_MAX, lines[5]);
// Mutate uid. // Mutate uid.
expectStatsLineEqual(value1, IFACE_NAME1, INT_MAX, TEST_COUNTERSET0, TEST_TAG, lines[5]); expectStatsLineEqual(value1, IFACE_NAME1, INT_MAX, TEST_COUNTERSET0, 0, lines[6]);
expectStatsLineEqual(value1, IFACE_NAME1, INT_MIN, TEST_COUNTERSET0, TEST_TAG, lines[6]); expectStatsLineEqual(value1, IFACE_NAME1, INT_MAX, TEST_COUNTERSET0, TEST_TAG, lines[7]);
expectStatsLineEqual(value1, IFACE_NAME1, UINT_MAX, TEST_COUNTERSET0, TEST_TAG, lines[7]); expectStatsLineEqual(value1, IFACE_NAME1, INT_MIN, TEST_COUNTERSET0, 0, lines[8]);
lines.clear(); 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 bpf
} // namespace android } // namespace android