From 8b9628620097fef80b1ea61805e7022bbea5ef17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Fri, 8 Aug 2025 14:34:32 -0700 Subject: [PATCH] BpfMap.h: introduce BpfMapRW MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit which allows writes, but not deletes. This is technically what bpf map arrays are capable of. Test: TreeHugger Flag: EXEMPT mainline Signed-off-by: Maciej Żenczykowski Change-Id: I1f73bea5428c190af17442539c296282ce9e0786 --- bpf/headers/BpfMapTest.cpp | 46 +++++++++++------- bpf/headers/include/bpf/BpfMap.h | 48 +++++++++++-------- bpf/netd/BpfBaseTest.cpp | 4 +- bpf/netd/BpfHandler.h | 1 + .../libs/libnetworkstats/BpfNetworkStats.cpp | 4 +- .../libnetworkstats/BpfNetworkStatsTest.cpp | 10 ++-- .../include/netdbpf/BpfNetworkStats.h | 2 +- .../include/netdbpf/NetworkTracePoller.h | 2 +- tests/native/utilities/firewall.h | 5 +- 9 files changed, 71 insertions(+), 51 deletions(-) diff --git a/bpf/headers/BpfMapTest.cpp b/bpf/headers/BpfMapTest.cpp index 21b1dc19c5..39c2811ec1 100644 --- a/bpf/headers/BpfMapTest.cpp +++ b/bpf/headers/BpfMapTest.cpp @@ -67,17 +67,17 @@ class BpfMapTest : public testing::Test { } } - void checkMapInvalid(BpfMap& map) { + void checkMapInvalid(BpfMapRO& map) { EXPECT_FALSE(map.isValid()); EXPECT_EQ(-1, map.getMap().get()); } - void checkMapValid(BpfMap& map) { + void checkMapValid(BpfMapRO& map) { EXPECT_LE(0, map.getMap().get()); EXPECT_TRUE(map.isValid()); } - void writeToMapAndCheck(BpfMap& map, uint32_t key, uint32_t value) { + void writeToMapAndCheck(BpfMapRW& map, uint32_t key, uint32_t value) { ASSERT_RESULT_OK(map.writeValue(key, value, BPF_ANY)); uint32_t value_read; ASSERT_EQ(0, findMapEntry(map.getMap(), &key, &value_read)); @@ -89,27 +89,39 @@ class BpfMapTest : public testing::Test { ASSERT_EQ(refValue, value.value()); } - void populateMap(uint32_t total, BpfMap& map) { + void populateMap(uint32_t total, BpfMapRW& map) { for (uint32_t key = 0; key < total; key++) { uint32_t value = key * 10; EXPECT_RESULT_OK(map.writeValue(key, value, BPF_ANY)); } } - void expectMapEmpty(BpfMap& map) { + void expectMapEmpty(BpfMapRO& map) { Result isEmpty = map.isEmpty(); ASSERT_RESULT_OK(isEmpty); ASSERT_TRUE(isEmpty.value()); } }; -TEST_F(BpfMapTest, constructor) { - BpfMap testMap1; - checkMapInvalid(testMap1); +TEST_F(BpfMapTest, constructorRO) { + BpfMapRO testMap; + checkMapInvalid(testMap); +} - BpfMap testMap2; - ASSERT_RESULT_OK(testMap2.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); - checkMapValid(testMap2); +TEST_F(BpfMapTest, constructorRW) { + BpfMapRW testMap; + checkMapInvalid(testMap); + + ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); + checkMapValid(testMap); +} + +TEST_F(BpfMapTest, constructor) { + BpfMap testMap; + checkMapInvalid(testMap); + + ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); + checkMapValid(testMap); } TEST_F(BpfMapTest, basicHelpers) { @@ -128,7 +140,7 @@ TEST_F(BpfMapTest, basicHelpers) { } TEST_F(BpfMapTest, reset) { - BpfMap testMap; + BpfMapRW testMap; ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); uint32_t key = TEST_KEY1; uint32_t value_write = TEST_VALUE1; @@ -141,9 +153,9 @@ TEST_F(BpfMapTest, reset) { } TEST_F(BpfMapTest, moveConstructor) { - BpfMap testMap1; + BpfMapRW testMap1; ASSERT_RESULT_OK(testMap1.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); - BpfMap testMap2; + BpfMapRW testMap2; testMap2 = std::move(testMap1); uint32_t key = TEST_KEY1; checkMapInvalid(testMap1); @@ -153,12 +165,12 @@ TEST_F(BpfMapTest, moveConstructor) { TEST_F(BpfMapTest, SetUpMap) { EXPECT_NE(0, access(PINNED_MAP_PATH, R_OK)); - BpfMap testMap1; + BpfMapRW testMap1; ASSERT_RESULT_OK(testMap1.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); ASSERT_EQ(0, bpfFdPin(testMap1.getMap(), PINNED_MAP_PATH)); EXPECT_EQ(0, access(PINNED_MAP_PATH, R_OK)); checkMapValid(testMap1); - BpfMap testMap2; + BpfMapRW testMap2; EXPECT_RESULT_OK(testMap2.init(PINNED_MAP_PATH)); checkMapValid(testMap2); uint32_t key = TEST_KEY1; @@ -250,7 +262,7 @@ TEST_F(BpfMapTest, mapClear) { } TEST_F(BpfMapTest, testGTSbitmapTestOpen) { - BpfMap bitmap; + BpfMapRW bitmap; ASSERT_RESULT_OK(bitmap.init("/sys/fs/bpf/tethering/map_test_bitmap")); } diff --git a/bpf/headers/include/bpf/BpfMap.h b/bpf/headers/include/bpf/BpfMap.h index 984e7b926a..e53a267e7d 100644 --- a/bpf/headers/include/bpf/BpfMap.h +++ b/bpf/headers/include/bpf/BpfMap.h @@ -257,19 +257,15 @@ class BpfMapRO { }; template -class BpfMap : public BpfMapRO { +class BpfMapRW : public BpfMapRO { protected: using BpfMapRO::mMapFd; using BpfMapRO::abortOnMismatch; public: - using BpfMapRO::getFirstKey; - using BpfMapRO::getNextKey; - using BpfMapRO::readValue; + using BpfMapRO::BpfMapRO; - BpfMap() {}; - - explicit BpfMap(const char* pathname) { + explicit BpfMapRW(const char* pathname) { mMapFd.reset(mapRetrieveRW(pathname)); abortOnMismatch(/* writable */ true); } @@ -286,6 +282,30 @@ class BpfMap : public BpfMapRO { return {}; } +#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING + [[clang::reinitializes]] Result resetMap(bpf_map_type map_type, + uint32_t max_entries, + uint32_t map_flags = 0) { + if (map_flags & BPF_F_WRONLY) Abort(0, "map_flags is write-only"); + if (map_flags & BPF_F_RDONLY) Abort(0, "map_flags is read-only"); + mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries, + map_flags)); + if (!mMapFd.ok()) return ErrnoErrorf("BpfMap::resetMap() failed"); + abortOnMismatch(/* writable */ true); + return {}; + } +#endif +}; + +template +class BpfMap : public BpfMapRW { + protected: + using BpfMapRW::mMapFd; + + public: + using BpfMapRW::BpfMapRW; + using BpfMapRW::getFirstKey; + Result deleteValue(const Key& key) { if (deleteMapEntry(mMapFd, &key)) { return ErrnoErrorf("BpfMap::deleteValue() failed"); @@ -309,20 +329,6 @@ class BpfMap : public BpfMapRO { } } } - -#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING - [[clang::reinitializes]] Result resetMap(bpf_map_type map_type, - uint32_t max_entries, - uint32_t map_flags = 0) { - if (map_flags & BPF_F_WRONLY) Abort(0, "map_flags is write-only"); - if (map_flags & BPF_F_RDONLY) Abort(0, "map_flags is read-only"); - mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries, - map_flags)); - if (!mMapFd.ok()) return ErrnoErrorf("BpfMap::resetMap() failed"); - abortOnMismatch(/* writable */ true); - return {}; - } -#endif }; } // namespace bpf diff --git a/bpf/netd/BpfBaseTest.cpp b/bpf/netd/BpfBaseTest.cpp index 4b8a04e944..0cb8acd705 100644 --- a/bpf/netd/BpfBaseTest.cpp +++ b/bpf/netd/BpfBaseTest.cpp @@ -54,7 +54,7 @@ class BpfBasicTest : public NetNativeTestBase { }; TEST_F(BpfBasicTest, TestTagSocket) { - BpfMap cookieTagMap(COOKIE_TAG_MAP_PATH); + BpfMapRO cookieTagMap(COOKIE_TAG_MAP_PATH); ASSERT_TRUE(cookieTagMap.isValid()); int sock = socket(AF_INET6, SOCK_STREAM | SOCK_CLOEXEC, 0); ASSERT_LE(0, sock); @@ -72,7 +72,7 @@ TEST_F(BpfBasicTest, TestTagSocket) { } TEST_F(BpfBasicTest, TestCloseSocketWithoutUntag) { - BpfMap cookieTagMap(COOKIE_TAG_MAP_PATH); + BpfMapRO cookieTagMap(COOKIE_TAG_MAP_PATH); ASSERT_TRUE(cookieTagMap.isValid()); int sock = socket(AF_INET6, SOCK_STREAM | SOCK_CLOEXEC, 0); ASSERT_LE(0, sock); diff --git a/bpf/netd/BpfHandler.h b/bpf/netd/BpfHandler.h index 9e69efc42f..b601b53f8a 100644 --- a/bpf/netd/BpfHandler.h +++ b/bpf/netd/BpfHandler.h @@ -22,6 +22,7 @@ using android::bpf::BpfMap; using android::bpf::BpfMapRO; +using android::bpf::BpfMapRW; namespace android { namespace net { diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp index 2dccf3dd3e..1cbc7b715c 100644 --- a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp +++ b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp @@ -40,8 +40,8 @@ namespace bpf { using base::Result; -BpfMap& getIfaceIndexNameMap() { - static BpfMap ifaceIndexNameMap(IFACE_INDEX_NAME_MAP_PATH); +BpfMapRW& getIfaceIndexNameMap() { + static BpfMapRW ifaceIndexNameMap(IFACE_INDEX_NAME_MAP_PATH); return ifaceIndexNameMap; } diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp index f283543061..bc573c97d2 100644 --- a/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp +++ b/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp @@ -72,10 +72,10 @@ class BpfNetworkStatsHelperTest : public testing::Test { protected: BpfNetworkStatsHelperTest() {} BpfMap mFakeCookieTagMap; - BpfMap mFakeAppUidStatsMap; - BpfMap mFakeStatsMap; - BpfMap mFakeIfaceIndexNameMap; - BpfMap mFakeIfaceStatsMap; + BpfMapRW mFakeAppUidStatsMap; + BpfMapRW mFakeStatsMap; + BpfMapRW mFakeIfaceIndexNameMap; + BpfMapRW mFakeIfaceStatsMap; IfIndexToNameFunc mIfIndex2Name = [this](const uint32_t ifindex){ return mFakeIfaceIndexNameMap.readValue(ifindex); @@ -108,7 +108,7 @@ class BpfNetworkStatsHelperTest : public testing::Test { } void populateFakeStats(uid_t uid, uint32_t tag, uint32_t ifaceIndex, uint32_t counterSet, - StatsValue value, BpfMap& map) { + StatsValue value, BpfMapRW& map) { StatsKey key = { .uid = (uint32_t)uid, .tag = tag, .counterSet = counterSet, .ifaceIndex = ifaceIndex}; EXPECT_RESULT_OK(map.writeValue(key, value, BPF_ANY)); diff --git a/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h b/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h index 59eb195440..68fa344d90 100644 --- a/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h +++ b/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h @@ -57,7 +57,7 @@ bool operator<(const stats_line& lhs, const stats_line& rhs); // This mirrors BpfMap.h's: // Result readValue(const Key key) const -// for a BpfMap +// for a BpfMapRW using IfIndexToNameFunc = std::function(const uint32_t)>; // For test only diff --git a/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTracePoller.h b/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTracePoller.h index 72fa66e56a..f90af9fab0 100644 --- a/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTracePoller.h +++ b/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTracePoller.h @@ -88,7 +88,7 @@ class NetworkTracePoller { std::unique_ptr> mRingBuffer GUARDED_BY(mBufferMutex); // The packet tracing config map (really a 1-element array). - BpfMap mConfigurationMap GUARDED_BY(mMutex); + BpfMapRW mConfigurationMap GUARDED_BY(mMutex); // This must be the last member, causing it to be the first deleted. If it is // not, members required for callbacks can be deleted before it's stopped. diff --git a/tests/native/utilities/firewall.h b/tests/native/utilities/firewall.h index a5cb0b9df1..00609237d5 100644 --- a/tests/native/utilities/firewall.h +++ b/tests/native/utilities/firewall.h @@ -24,6 +24,7 @@ using android::base::Result; using android::bpf::BpfMap; +using android::bpf::BpfMapRW; class Firewall { public: @@ -37,8 +38,8 @@ class Firewall { Result getDataSaverSetting(); Result setDataSaver(bool enabled); private: - BpfMap mConfigurationMap GUARDED_BY(mMutex); + BpfMapRW mConfigurationMap GUARDED_BY(mMutex); BpfMap mUidOwnerMap GUARDED_BY(mMutex); - BpfMap mDataSaverEnabledMap GUARDED_BY(mMutex); + BpfMapRW mDataSaverEnabledMap GUARDED_BY(mMutex); std::mutex mMutex; };