joy/android_packages_modules_Connectivity
Watch
1
Fork
You've already forked android_packages_modules_Connectivity
 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge changes I40b29d98,I6b498579 into main

Browse source 
* changes:
  BpfNetworkStats: use bulk api on 5.10+
  BpfMap.h: on 5.10+ use bulk bpf map api
This commit is contained in:
Maciej Żenczykowski 2025-08-15 00:53:05 -07:00 • committed by Android (Google) Code Review
commit 80ee0dbbcf
2 changed files with 70 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

53
bpf/headers/include/bpf/BpfMap.h
View file

@ -147,6 +147,50 @@ class BpfMapRO {
return {};
}
// Observed 4 <= sizeof(Key) <= 16 (48 for Java), 1 <= sizeof(Value) <= 32 (64 for Java)
// You can uncomment the following to check:
// static_assert(sizeof(Key) >= 4);
// static_assert(sizeof(Key) <= 16); // 48 observed, but not in C++
// static_assert(sizeof(Value) >= 1);
// static_assert(sizeof(Value) <= 32); // 64 observed, but not in C++
// ~16KiB initial stack usage seems reasonable
static constexpr int BATCHSIZE = 16384 / (sizeof(Key) + sizeof(Value));
static_assert(BATCHSIZE >= 256, "consider Key/Value size, whether incr mem limit, decr batch req");
static_assert(BATCHSIZE * sizeof(Key) + BATCHSIZE * sizeof(Value) <= 16384);
Result<void> doBulkLookupAndMaybeDelete(bool del, const function<void(const Key &, const Value &)> &f) const {
union { Key k; uint32_t nr; } batch;
bool first = true;
// starting with N == 1 fails with -28/ENOSPC in:
// BpfNetworkStatsTest.cpp BpfNetworkStatsHelperTest#TestGetStatsSortedAndGrouped
// requiring us to loop back around, kernel code itself claims that in practice 5
// is almost always enough for a bucket (which is what you'd expect, it's not a good
// hashtable if there's lots of items in a single bucket)
//
// Since we start with 256+ we shouldn't ever actually need to increase N...
// Also note that the 'true' condition is not really an infinite loop,
// as we'll blow up the stack and crash instead of looping infinitely.
// But that also shouldn't happen cause it would imply/require a ridiculously
// large bpf map sitting entirely in one bucket...
for (int N = BATCHSIZE; true; N *= 2) {
// N is how many we have space for, can grow on demand as needed
Key keys[N];
Value values[N];
for (;;) {
uint32_t count = N; // how many to fetch (and possibly delete)
int rv = batchLookupAndMaybeDelete(mMapFd, first ? NULL : &batch, &batch, &keys, &values, &count, del);
if (rv && errno == ENOSPC) break; // not enough space for full HASH bucket, go around the *outer* loop
if (rv && errno != ENOENT) return ErrnoErrorf("BpfMap::doBulkLookupAndMaybeDelete() failed");
// count is now how many *were* fetched (and possibly delete)
for (unsigned i = 0; i < count; ++i) f(keys[i], values[i]);
if (rv) return {}; // ENOENT -> success
first = false;
}
}
}
public:
// Function that tries to get map from a pinned path.
[[clang::reinitializes]] Result<void> init(const char* path) {
@ -168,6 +212,12 @@ class BpfMapRO {
// Does not allow early termination (via f erroring out) - may be implemented with bulk api
Result<void> forAll(const function<void(const Key &)> &f) const {
// No kernel bpfmap bulk lookup api which doesn't return both keys & values.
if (isAtLeastKernelVersion(5, 10, 0)) return doBulkLookupAndMaybeDelete(/*delete*/ false,
[&f](const Key &key, const Value &) {
f(key);
}
);
return iterate(
[&f](const Key &key) -> Result<void> {
f(key);
@ -193,6 +243,7 @@ class BpfMapRO {
// Does not allow early termination (via f erroring out) - maybe implemented with bulk api
Result<void> forAll(const function<void(const Key &, const Value &)> &f) const {
if (isAtLeastKernelVersion(5, 10, 0)) return doBulkLookupAndMaybeDelete(/*delete*/ false, f);
return iterate(
[&f](const Key &key, const Value &value) -> Result<void> {
f(key, value);
@ -301,6 +352,7 @@ template <class Key, class Value>
class BpfMap : public BpfMapRW<Key, Value> {
protected:
using BpfMapRW<Key, Value>::mMapFd;
using BpfMapRW<Key, Value>::doBulkLookupAndMaybeDelete;
public:
using BpfMapRW<Key, Value>::BpfMapRW;
@ -356,6 +408,7 @@ class BpfMap : public BpfMapRW<Key, Value> {
// Does not allow early termination (via f erroring out) - maybe implemented with bulk api
Result<void> consume(const std::function<void(const Key&, const Value&)>& f) {
if (isAtLeastKernelVersion(5, 10, 0)) return doBulkLookupAndMaybeDelete(/*delete*/true, f);
Result<Key> curKey = getFirstKey();
while (curKey.ok()) {
const Result<Key> &nextKey = getNextKey(curKey.value());

18
service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp
View file

@ -112,6 +112,23 @@ int bpfGetIfaceStatsInternal(const char* iface, StatsValue* stats,
// specific iface
int64_t unknownIfaceBytesTotal = 0;
if (isAtLeastKernelVersion(5, 10, 0)) {
// On 5.10+ bulk lookup api returns values for free, so just use forAll(f(K,V))
auto res = ifaceStatsMap.forAll(
[iface, stats, ifindex2name, &unknownIfaceBytesTotal, &ifaceStatsMap](const uint32_t& key, const StatsValue& value) {
Result<IfaceValue> ifname = ifindex2name(key);
if (!ifname.ok()) {
maybeLogUnknownIface(key, ifaceStatsMap, key, &unknownIfaceBytesTotal);
return;
}
if (!strcmp(iface, ifname.value().name)) *stats += value;
}
);
return res.ok() ? 0 : -res.error().code();
}
// Before 5.10 we try to avoid doing readValue unless needed
auto res = ifaceStatsMap.iterate(
[iface, stats, ifindex2name, &unknownIfaceBytesTotal, &ifaceStatsMap](const uint32_t& key) -> Result<void> {
Result<IfaceValue> ifname = ifindex2name(key);
@ -120,7 +137,6 @@ int bpfGetIfaceStatsInternal(const char* iface, StatsValue* stats,
return {};
}
if (!strcmp(iface, ifname.value().name)) {
// TODO: bulk api makes this readValue() inefficient - use forAll(K, V)
Result<StatsValue> statsEntry = ifaceStatsMap.readValue(key);
if (!statsEntry.ok()) return statsEntry.error();
*stats += statsEntry.value();