reverse BpfMap & BpfMapRO inheritence to make sense

Bug: 235590615
Bug: 235907076
Bug: 286003437
Test: TreeHugger, m droid gpuservice_unittest libtimeinstate_test bpf_benchmark bpf_module_test libbpf_load_test && mma
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I020a898fe8b257040a100f146654d4a04b19f843

bpf_progs/netd.c

@@ -89,11 +89,11 @@ DEFINE_BPF_MAP_RO_NETD(configuration_map, ARRAY, uint32_t, uint32_t, CONFIGURATI
 DEFINE_BPF_MAP_RW_NETD(cookie_tag_map, HASH, uint64_t, UidTagValue, COOKIE_UID_MAP_SIZE)
 DEFINE_BPF_MAP_NO_NETD(uid_counterset_map, HASH, uint32_t, uint8_t, UID_COUNTERSET_MAP_SIZE)
 DEFINE_BPF_MAP_NO_NETD(app_uid_stats_map, HASH, uint32_t, StatsValue, APP_STATS_MAP_SIZE)
-DEFINE_BPF_MAP_RW_NETD(stats_map_A, HASH, StatsKey, StatsValue, STATS_MAP_SIZE)
+DEFINE_BPF_MAP_RO_NETD(stats_map_A, HASH, StatsKey, StatsValue, STATS_MAP_SIZE)
 DEFINE_BPF_MAP_RO_NETD(stats_map_B, HASH, StatsKey, StatsValue, STATS_MAP_SIZE)
 DEFINE_BPF_MAP_NO_NETD(iface_stats_map, HASH, uint32_t, StatsValue, IFACE_STATS_MAP_SIZE)
 DEFINE_BPF_MAP_NO_NETD(uid_owner_map, HASH, uint32_t, UidOwnerValue, UID_OWNER_MAP_SIZE)
-DEFINE_BPF_MAP_RW_NETD(uid_permission_map, HASH, uint32_t, uint8_t, UID_OWNER_MAP_SIZE)
+DEFINE_BPF_MAP_RO_NETD(uid_permission_map, HASH, uint32_t, uint8_t, UID_OWNER_MAP_SIZE)
 DEFINE_BPF_MAP_NO_NETD(ingress_discard_map, HASH, IngressDiscardKey, IngressDiscardValue,
                        INGRESS_DISCARD_MAP_SIZE)
 

netd/BpfHandler.cpp

@@ -214,7 +214,7 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU
     // which might toggle the live stats map and clean it.
     const auto countUidStatsEntries = [chargeUid, &totalEntryCount, &perUidEntryCount](
                                               const StatsKey& key,
-                                              const BpfMap<StatsKey, StatsValue>&) {
+                                              const BpfMapRO<StatsKey, StatsValue>&) {
         if (key.uid == chargeUid) {
             perUidEntryCount++;
         }
@@ -232,9 +232,8 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU
         return -EINVAL;
     }
 
-    BpfMap<StatsKey, StatsValue>& currentMap =
+    BpfMapRO<StatsKey, StatsValue>& currentMap =
             (configuration.value() == SELECT_MAP_A) ? mStatsMapA : mStatsMapB;
-    // HACK: mStatsMapB becomes RW BpfMap here, but countUidStatsEntries doesn't modify so it works
     base::Result<void> res = currentMap.iterate(countUidStatsEntries);
     if (!res.ok()) {
         ALOGE("Failed to count the stats entry in map: %s",

netd/BpfHandler.h

@@ -59,10 +59,10 @@ class BpfHandler {
     bool hasUpdateDeviceStatsPermission(uid_t uid);
 
     BpfMap<uint64_t, UidTagValue> mCookieTagMap;
-    BpfMap<StatsKey, StatsValue> mStatsMapA;
+    BpfMapRO<StatsKey, StatsValue> mStatsMapA;
     BpfMapRO<StatsKey, StatsValue> mStatsMapB;
     BpfMapRO<uint32_t, uint32_t> mConfigurationMap;
-    BpfMap<uint32_t, uint8_t> mUidPermissionMap;
+    BpfMapRO<uint32_t, uint8_t> mUidPermissionMap;
 
     // The limit on the number of stats entries a uid can have in the per uid stats map. BpfHandler
     // will block that specific uid from tagging new sockets after the limit is reached.

netd/BpfHandlerTest.cpp

@@ -49,7 +49,7 @@ class BpfHandlerTest : public ::testing::Test {
     BpfHandler mBh;
     BpfMap<uint64_t, UidTagValue> mFakeCookieTagMap;
     BpfMap<StatsKey, StatsValue> mFakeStatsMapA;
-    BpfMapRO<uint32_t, uint32_t> mFakeConfigurationMap;
+    BpfMap<uint32_t, uint32_t> mFakeConfigurationMap;
     BpfMap<uint32_t, uint8_t> mFakeUidPermissionMap;
 
     void SetUp() {

service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp

@@ -41,7 +41,7 @@ namespace bpf {
 using base::Result;
 
 int bpfGetUidStatsInternal(uid_t uid, StatsValue* stats,
-                           const BpfMap<uint32_t, StatsValue>& appUidStatsMap) {
+                           const BpfMapRO<uint32_t, StatsValue>& appUidStatsMap) {
     auto statsEntry = appUidStatsMap.readValue(uid);
     if (!statsEntry.ok()) {
         *stats = {};
@@ -57,14 +57,14 @@ int bpfGetUidStats(uid_t uid, StatsValue* stats) {
 }
 
 int bpfGetIfaceStatsInternal(const char* iface, StatsValue* stats,
-                             const BpfMap<uint32_t, StatsValue>& ifaceStatsMap,
-                             const BpfMap<uint32_t, IfaceValue>& ifaceNameMap) {
+                             const BpfMapRO<uint32_t, StatsValue>& ifaceStatsMap,
+                             const BpfMapRO<uint32_t, IfaceValue>& ifaceNameMap) {
     *stats = {};
     int64_t unknownIfaceBytesTotal = 0;
     const auto processIfaceStats =
             [iface, stats, &ifaceNameMap, &unknownIfaceBytesTotal](
                     const uint32_t& key,
-                    const BpfMap<uint32_t, StatsValue>& ifaceStatsMap) -> Result<void> {
+                    const BpfMapRO<uint32_t, StatsValue>& ifaceStatsMap) -> Result<void> {
         char ifname[IFNAMSIZ];
         if (getIfaceNameFromMap(ifaceNameMap, ifaceStatsMap, key, ifname, key,
                                 &unknownIfaceBytesTotal)) {
@@ -90,7 +90,7 @@ int bpfGetIfaceStats(const char* iface, StatsValue* stats) {
 }
 
 int bpfGetIfIndexStatsInternal(uint32_t ifindex, StatsValue* stats,
-                               const BpfMap<uint32_t, StatsValue>& ifaceStatsMap) {
+                               const BpfMapRO<uint32_t, StatsValue>& ifaceStatsMap) {
     auto statsEntry = ifaceStatsMap.readValue(ifindex);
     if (!statsEntry.ok()) {
         *stats = {};
@@ -120,13 +120,13 @@ stats_line populateStatsEntry(const StatsKey& statsKey, const StatsValue& statsE
 }
 
 int parseBpfNetworkStatsDetailInternal(std::vector<stats_line>& lines,
-                                       const BpfMap<StatsKey, StatsValue>& statsMap,
-                                       const BpfMap<uint32_t, IfaceValue>& ifaceMap) {
+                                       const BpfMapRO<StatsKey, StatsValue>& statsMap,
+                                       const BpfMapRO<uint32_t, IfaceValue>& ifaceMap) {
     int64_t unknownIfaceBytesTotal = 0;
     const auto processDetailUidStats =
             [&lines, &unknownIfaceBytesTotal, &ifaceMap](
                     const StatsKey& key,
-                    const BpfMap<StatsKey, StatsValue>& statsMap) -> Result<void> {
+                    const BpfMapRO<StatsKey, StatsValue>& statsMap) -> Result<void> {
         char ifname[IFNAMSIZ];
         if (getIfaceNameFromMap(ifaceMap, statsMap, key.ifaceIndex, ifname, key,
                                 &unknownIfaceBytesTotal)) {
@@ -212,12 +212,12 @@ int parseBpfNetworkStatsDetail(std::vector<stats_line>* lines) {
 }
 
 int parseBpfNetworkStatsDevInternal(std::vector<stats_line>& lines,
-                                    const BpfMap<uint32_t, StatsValue>& statsMap,
-                                    const BpfMap<uint32_t, IfaceValue>& ifaceMap) {
+                                    const BpfMapRO<uint32_t, StatsValue>& statsMap,
+                                    const BpfMapRO<uint32_t, IfaceValue>& ifaceMap) {
     int64_t unknownIfaceBytesTotal = 0;
     const auto processDetailIfaceStats = [&lines, &unknownIfaceBytesTotal, &ifaceMap, &statsMap](
                                              const uint32_t& key, const StatsValue& value,
-                                             const BpfMap<uint32_t, StatsValue>&) {
+                                             const BpfMapRO<uint32_t, StatsValue>&) {
         char ifname[IFNAMSIZ];
         if (getIfaceNameFromMap(ifaceMap, statsMap, key, ifname, key, &unknownIfaceBytesTotal)) {
             return Result<void>();

service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp

@@ -80,19 +80,19 @@ class BpfNetworkStatsHelperTest : public testing::Test {
     void SetUp() {
         ASSERT_EQ(0, setrlimitForTest());
 
-        mFakeCookieTagMap = BpfMap<uint64_t, UidTagValue>(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0);
+        mFakeCookieTagMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
         ASSERT_TRUE(mFakeCookieTagMap.isValid());
 
-        mFakeAppUidStatsMap = BpfMap<uint32_t, StatsValue>(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0);
+        mFakeAppUidStatsMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
         ASSERT_TRUE(mFakeAppUidStatsMap.isValid());
 
-        mFakeStatsMap = BpfMap<StatsKey, StatsValue>(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0);
+        mFakeStatsMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
         ASSERT_TRUE(mFakeStatsMap.isValid());
 
-        mFakeIfaceIndexNameMap = BpfMap<uint32_t, IfaceValue>(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0);
+        mFakeIfaceIndexNameMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
         ASSERT_TRUE(mFakeIfaceIndexNameMap.isValid());
 
-        mFakeIfaceStatsMap = BpfMap<uint32_t, StatsValue>(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0);
+        mFakeIfaceStatsMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
         ASSERT_TRUE(mFakeIfaceStatsMap.isValid());
     }
 

service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h

@@ -57,24 +57,25 @@ bool operator<(const stats_line& lhs, const stats_line& rhs);
 
 // For test only
 int bpfGetUidStatsInternal(uid_t uid, StatsValue* stats,
-                           const BpfMap<uint32_t, StatsValue>& appUidStatsMap);
+                           const BpfMapRO<uint32_t, StatsValue>& appUidStatsMap);
 // For test only
 int bpfGetIfaceStatsInternal(const char* iface, StatsValue* stats,
-                             const BpfMap<uint32_t, StatsValue>& ifaceStatsMap,
-                             const BpfMap<uint32_t, IfaceValue>& ifaceNameMap);
+                             const BpfMapRO<uint32_t, StatsValue>& ifaceStatsMap,
+                             const BpfMapRO<uint32_t, IfaceValue>& ifaceNameMap);
 // For test only
 int bpfGetIfIndexStatsInternal(uint32_t ifindex, StatsValue* stats,
-                               const BpfMap<uint32_t, StatsValue>& ifaceStatsMap);
+                               const BpfMapRO<uint32_t, StatsValue>& ifaceStatsMap);
 // For test only
 int parseBpfNetworkStatsDetailInternal(std::vector<stats_line>& lines,
-                                       const BpfMap<StatsKey, StatsValue>& statsMap,
-                                       const BpfMap<uint32_t, IfaceValue>& ifaceMap);
+                                       const BpfMapRO<StatsKey, StatsValue>& statsMap,
+                                       const BpfMapRO<uint32_t, IfaceValue>& ifaceMap);
 // For test only
 int cleanStatsMapInternal(const base::unique_fd& cookieTagMap, const base::unique_fd& tagStatsMap);
 // For test only
 template <class Key>
-int getIfaceNameFromMap(const BpfMap<uint32_t, IfaceValue>& ifaceMap,
-                        const BpfMap<Key, StatsValue>& statsMap, uint32_t ifaceIndex, char* ifname,
+int getIfaceNameFromMap(const BpfMapRO<uint32_t, IfaceValue>& ifaceMap,
+                        const BpfMapRO<Key, StatsValue>& statsMap,
+                        uint32_t ifaceIndex, char* ifname,
                         const Key& curKey, int64_t* unknownIfaceBytesTotal) {
     auto iface = ifaceMap.readValue(ifaceIndex);
     if (!iface.ok()) {
@@ -86,7 +87,7 @@ int getIfaceNameFromMap(const BpfMap<uint32_t, IfaceValue>& ifaceMap,
 }
 
 template <class Key>
-void maybeLogUnknownIface(int ifaceIndex, const BpfMap<Key, StatsValue>& statsMap,
+void maybeLogUnknownIface(int ifaceIndex, const BpfMapRO<Key, StatsValue>& statsMap,
                           const Key& curKey, int64_t* unknownIfaceBytesTotal) {
     // Have we already logged an error?
     if (*unknownIfaceBytesTotal == -1) {
@@ -110,8 +111,8 @@ void maybeLogUnknownIface(int ifaceIndex, const BpfMap<Key, StatsValue>& statsMa
 
 // For test only
 int parseBpfNetworkStatsDevInternal(std::vector<stats_line>& lines,
-                                    const BpfMap<uint32_t, StatsValue>& statsMap,
-                                    const BpfMap<uint32_t, IfaceValue>& ifaceMap);
+                                    const BpfMapRO<uint32_t, StatsValue>& statsMap,
+                                    const BpfMapRO<uint32_t, IfaceValue>& ifaceMap);
 
 int bpfGetUidStats(uid_t uid, StatsValue* stats);
 int bpfGetIfaceStats(const char* iface, StatsValue* stats);

staticlibs/native/bpf_headers/BpfMapTest.cpp

@@ -107,12 +107,14 @@ TEST_F(BpfMapTest, constructor) {
     BpfMap<uint32_t, uint32_t> testMap1;
     checkMapInvalid(testMap1);
 
-    BpfMap<uint32_t, uint32_t> testMap2(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> testMap2;
+    ASSERT_RESULT_OK(testMap2.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC));
     checkMapValid(testMap2);
 }
 
 TEST_F(BpfMapTest, basicHelpers) {
-    BpfMap<uint32_t, uint32_t> testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> 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;
     writeToMapAndCheck(testMap, key, value_write);
@@ -126,7 +128,8 @@ TEST_F(BpfMapTest, basicHelpers) {
 }
 
 TEST_F(BpfMapTest, reset) {
-    BpfMap<uint32_t, uint32_t> testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> 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;
     writeToMapAndCheck(testMap, key, value_write);
@@ -138,7 +141,8 @@ TEST_F(BpfMapTest, reset) {
 }
 
 TEST_F(BpfMapTest, moveConstructor) {
-    BpfMap<uint32_t, uint32_t> testMap1(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> testMap1;
+    ASSERT_RESULT_OK(testMap1.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC));
     BpfMap<uint32_t, uint32_t> testMap2;
     testMap2 = std::move(testMap1);
     uint32_t key = TEST_KEY1;
@@ -149,7 +153,8 @@ TEST_F(BpfMapTest, moveConstructor) {
 
 TEST_F(BpfMapTest, SetUpMap) {
     EXPECT_NE(0, access(PINNED_MAP_PATH, R_OK));
-    BpfMap<uint32_t, uint32_t> testMap1(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> 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);
@@ -164,7 +169,8 @@ TEST_F(BpfMapTest, SetUpMap) {
 }
 
 TEST_F(BpfMapTest, iterate) {
-    BpfMap<uint32_t, uint32_t> testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> testMap;
+    ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC));
     populateMap(TEST_MAP_SIZE, testMap);
     int totalCount = 0;
     int totalSum = 0;
@@ -182,7 +188,8 @@ TEST_F(BpfMapTest, iterate) {
 }
 
 TEST_F(BpfMapTest, iterateWithValue) {
-    BpfMap<uint32_t, uint32_t> testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> testMap;
+    ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC));
     populateMap(TEST_MAP_SIZE, testMap);
     int totalCount = 0;
     int totalSum = 0;
@@ -202,7 +209,8 @@ TEST_F(BpfMapTest, iterateWithValue) {
 }
 
 TEST_F(BpfMapTest, mapIsEmpty) {
-    BpfMap<uint32_t, uint32_t> testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> testMap;
+    ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC));
     expectMapEmpty(testMap);
     uint32_t key = TEST_KEY1;
     uint32_t value_write = TEST_VALUE1;
@@ -232,7 +240,8 @@ TEST_F(BpfMapTest, mapIsEmpty) {
 }
 
 TEST_F(BpfMapTest, mapClear) {
-    BpfMap<uint32_t, uint32_t> testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
+    BpfMap<uint32_t, uint32_t> testMap;
+    ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE));
     populateMap(TEST_MAP_SIZE, testMap);
     Result<bool> isEmpty = testMap.isEmpty();
     ASSERT_RESULT_OK(isEmpty);

staticlibs/native/bpf_headers/include/bpf/BpfMap.h

@@ -48,41 +48,32 @@ using std::function;
 // is not safe to iterate over a map while another thread or process is deleting
 // from it. In this case the iteration can return duplicate entries.
 template <class Key, class Value>
-class BpfMap {
+class BpfMapRO {
   public:
-    BpfMap<Key, Value>() {};
+    BpfMapRO<Key, Value>() {};
 
     // explicitly force no copy constructor, since it would need to dup the fd
     // (later on, for testing, we still make available a copy assignment operator)
-    BpfMap<Key, Value>(const BpfMap<Key, Value>&) = delete;
+    BpfMapRO<Key, Value>(const BpfMapRO<Key, Value>&) = delete;
 
-  private:
-    void abortOnKeyOrValueSizeMismatch() {
+  protected:
+    void abortOnMismatch(bool writable) const {
         if (!mMapFd.ok()) abort();
         if (isAtLeastKernelVersion(4, 14, 0)) {
+            int flags = bpfGetFdMapFlags(mMapFd);
+            if (flags < 0) abort();
+            if (flags & BPF_F_WRONLY) abort();
+            if (writable && (flags & BPF_F_RDONLY)) abort();
             if (bpfGetFdKeySize(mMapFd) != sizeof(Key)) abort();
             if (bpfGetFdValueSize(mMapFd) != sizeof(Value)) abort();
         }
     }
 
-  protected:
-    // flag must be within BPF_OBJ_FLAG_MASK, ie. 0, BPF_F_RDONLY, BPF_F_WRONLY
-    BpfMap<Key, Value>(const char* pathname, uint32_t flags) {
-        mMapFd.reset(mapRetrieve(pathname, flags));
-        abortOnKeyOrValueSizeMismatch();
-    }
-
   public:
-    explicit BpfMap<Key, Value>(const char* pathname) : BpfMap<Key, Value>(pathname, 0) {}
-
-#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING
-    // All bpf maps should be created by the bpfloader, so this constructor
-    // is reserved for tests
-    BpfMap<Key, Value>(bpf_map_type map_type, uint32_t max_entries, uint32_t map_flags = 0) {
-        mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries, map_flags));
-        if (!mMapFd.ok()) abort();
+    explicit BpfMapRO<Key, Value>(const char* pathname) {
+        mMapFd.reset(mapRetrieveRO(pathname));
+        abortOnMismatch(/* writable */ false);
     }
-#endif
 
     Result<Key> getFirstKey() const {
         Key firstKey;
@@ -100,13 +91,6 @@ class BpfMap {
         return nextKey;
     }
 
-    Result<void> writeValue(const Key& key, const Value& value, uint64_t flags) {
-        if (writeToMapEntry(mMapFd, &key, &value, flags)) {
-            return ErrnoErrorf("Write to map {} failed", mMapFd.get());
-        }
-        return {};
-    }
-
     Result<Value> readValue(const Key key) const {
         Value value;
         if (findMapEntry(mMapFd, &key, &value)) {
@@ -115,6 +99,155 @@ class BpfMap {
         return value;
     }
 
+  protected:
+    [[clang::reinitializes]] Result<void> init(const char* path, int fd, bool writable) {
+        mMapFd.reset(fd);
+        if (!mMapFd.ok()) {
+            return ErrnoErrorf("Pinned map not accessible or does not exist: ({})", path);
+        }
+        // Normally we should return an error here instead of calling abort,
+        // but this cannot happen at runtime without a massive code bug (K/V type mismatch)
+        // and as such it's better to just blow the system up and let the developer fix it.
+        // Crashes are much more likely to be noticed than logs and missing functionality.
+        abortOnMismatch(writable);
+        return {};
+    }
+
+  public:
+    // Function that tries to get map from a pinned path.
+    [[clang::reinitializes]] Result<void> init(const char* path) {
+        return init(path, mapRetrieveRO(path), /* writable */ false);
+    }
+
+    // Iterate through the map and handle each key retrieved based on the filter
+    // without modification of map content.
+    Result<void> iterate(
+            const function<Result<void>(const Key& key,
+                                        const BpfMapRO<Key, Value>& map)>& filter) const;
+
+    // Iterate through the map and get each <key, value> pair, handle each <key,
+    // value> pair based on the filter without modification of map content.
+    Result<void> iterateWithValue(
+            const function<Result<void>(const Key& key, const Value& value,
+                                        const BpfMapRO<Key, Value>& map)>& filter) const;
+
+#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING
+    const unique_fd& getMap() const { return mMapFd; };
+
+    // Copy assignment operator - due to need for fd duping, should not be used in non-test code.
+    BpfMapRO<Key, Value>& operator=(const BpfMapRO<Key, Value>& other) {
+        if (this != &other) mMapFd.reset(fcntl(other.mMapFd.get(), F_DUPFD_CLOEXEC, 0));
+        return *this;
+    }
+#else
+    BpfMapRO<Key, Value>& operator=(const BpfMapRO<Key, Value>&) = delete;
+#endif
+
+    // Move assignment operator
+    BpfMapRO<Key, Value>& operator=(BpfMapRO<Key, Value>&& other) noexcept {
+        if (this != &other) {
+            mMapFd = std::move(other.mMapFd);
+            other.reset();
+        }
+        return *this;
+    }
+
+#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING
+    // Note that unique_fd.reset() carefully saves and restores the errno,
+    // and BpfMap.reset() won't touch the errno if passed in fd is negative either,
+    // hence you can do something like BpfMap.reset(systemcall()) and then
+    // check BpfMap.isValid() and look at errno and see why systemcall() failed.
+    [[clang::reinitializes]] void reset(int fd) {
+        mMapFd.reset(fd);
+        if (mMapFd.ok()) abortOnMismatch(/* writable */ false);  // false isn't ideal
+    }
+
+    // unique_fd has an implicit int conversion defined, which combined with the above
+    // reset(int) would result in double ownership of the fd, hence we either need a custom
+    // implementation of reset(unique_fd), or to delete it and thus cause compile failures
+    // to catch this and prevent it.
+    void reset(unique_fd fd) = delete;
+#endif
+
+    [[clang::reinitializes]] void reset() {
+        mMapFd.reset();
+    }
+
+    bool isValid() const { return mMapFd.ok(); }
+
+    Result<bool> isEmpty() const {
+        auto key = getFirstKey();
+        if (key.ok()) return false;
+        if (key.error().code() == ENOENT) return true;
+        return key.error();
+    }
+
+  protected:
+    unique_fd mMapFd;
+};
+
+template <class Key, class Value>
+Result<void> BpfMapRO<Key, Value>::iterate(
+        const function<Result<void>(const Key& key,
+                                    const BpfMapRO<Key, Value>& map)>& filter) const {
+    Result<Key> curKey = getFirstKey();
+    while (curKey.ok()) {
+        const Result<Key>& nextKey = getNextKey(curKey.value());
+        Result<void> status = filter(curKey.value(), *this);
+        if (!status.ok()) return status;
+        curKey = nextKey;
+    }
+    if (curKey.error().code() == ENOENT) return {};
+    return curKey.error();
+}
+
+template <class Key, class Value>
+Result<void> BpfMapRO<Key, Value>::iterateWithValue(
+        const function<Result<void>(const Key& key, const Value& value,
+                                    const BpfMapRO<Key, Value>& map)>& filter) const {
+    Result<Key> curKey = getFirstKey();
+    while (curKey.ok()) {
+        const Result<Key>& nextKey = getNextKey(curKey.value());
+        Result<Value> curValue = readValue(curKey.value());
+        if (!curValue.ok()) return curValue.error();
+        Result<void> status = filter(curKey.value(), curValue.value(), *this);
+        if (!status.ok()) return status;
+        curKey = nextKey;
+    }
+    if (curKey.error().code() == ENOENT) return {};
+    return curKey.error();
+}
+
+template <class Key, class Value>
+class BpfMap : public BpfMapRO<Key, Value> {
+  protected:
+    using BpfMapRO<Key, Value>::mMapFd;
+    using BpfMapRO<Key, Value>::abortOnMismatch;
+
+  public:
+    using BpfMapRO<Key, Value>::getFirstKey;
+    using BpfMapRO<Key, Value>::getNextKey;
+    using BpfMapRO<Key, Value>::readValue;
+
+    BpfMap<Key, Value>() {};
+
+    explicit BpfMap<Key, Value>(const char* pathname) {
+        mMapFd.reset(mapRetrieveRW(pathname));
+        abortOnMismatch(/* writable */ true);
+    }
+
+    // Function that tries to get map from a pinned path.
+    [[clang::reinitializes]] Result<void> init(const char* path) {
+        return BpfMapRO<Key,Value>::init(path, mapRetrieveRW(path), /* writable */ true);
+    }
+
+    Result<void> writeValue(const Key& key, const Value& value, uint64_t flags) {
+        if (writeToMapEntry(mMapFd, &key, &value, flags)) {
+            return ErrnoErrorf("Write to map {} failed", mMapFd.get());
+        }
+        return {};
+    }
+
     Result<void> deleteValue(const Key& key) {
         if (deleteMapEntry(mMapFd, &key)) {
             return ErrnoErrorf("Delete entry from map {} failed", mMapFd.get());
@@ -122,37 +255,33 @@ class BpfMap {
         return {};
     }
 
-  protected:
-    [[clang::reinitializes]] Result<void> init(const char* path, int fd) {
-        mMapFd.reset(fd);
-        if (!mMapFd.ok()) {
-            return ErrnoErrorf("Pinned map not accessible or does not exist: ({})", path);
+    Result<void> clear() {
+        while (true) {
+            auto key = getFirstKey();
+            if (!key.ok()) {
+                if (key.error().code() == ENOENT) return {};  // empty: success
+                return key.error();                           // Anything else is an error
+            }
+            auto res = deleteValue(key.value());
+            if (!res.ok()) {
+                // Someone else could have deleted the key, so ignore ENOENT
+                if (res.error().code() == ENOENT) continue;
+                ALOGE("Failed to delete data %s", strerror(res.error().code()));
+                return res.error();
+            }
         }
-        // Normally we should return an error here instead of calling abort,
-        // but this cannot happen at runtime without a massive code bug (K/V type mismatch)
-        // and as such it's better to just blow the system up and let the developer fix it.
-        // Crashes are much more likely to be noticed than logs and missing functionality.
-        abortOnKeyOrValueSizeMismatch();
-        return {};
     }
 
-  public:
-    // Function that tries to get map from a pinned path.
-    [[clang::reinitializes]] Result<void> init(const char* path) {
-        return init(path, mapRetrieveRW(path));
-    }
-
-
 #ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING
-    // due to Android SELinux limitations which prevent map creation by anyone besides the bpfloader
-    // this should only ever be used by test code, it is equivalent to:
-    //   .reset(createMap(type, keysize, valuesize, max_entries, map_flags)
-    // TODO: derive map_flags from BpfMap vs BpfMapRO
     [[clang::reinitializes]] Result<void> resetMap(bpf_map_type map_type,
-                                                         uint32_t max_entries,
-                                                         uint32_t map_flags = 0) {
-        mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries, map_flags));
+                                                   uint32_t max_entries,
+                                                   uint32_t map_flags = 0) {
+        if (map_flags & BPF_F_WRONLY) abort();
+        if (map_flags & BPF_F_RDONLY) abort();
+        mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries,
+                               map_flags));
         if (!mMapFd.ok()) return ErrnoErrorf("Unable to create map.");
+        abortOnMismatch(/* writable */ true);
         return {};
     }
 #endif
@@ -180,72 +309,6 @@ class BpfMap {
             const function<Result<void>(const Key& key, const Value& value,
                                         BpfMap<Key, Value>& map)>& filter);
 
-#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING
-    const unique_fd& getMap() const { return mMapFd; };
-
-    // Copy assignment operator - due to need for fd duping, should not be used in non-test code.
-    BpfMap<Key, Value>& operator=(const BpfMap<Key, Value>& other) {
-        if (this != &other) mMapFd.reset(fcntl(other.mMapFd.get(), F_DUPFD_CLOEXEC, 0));
-        return *this;
-    }
-#else
-    BpfMap<Key, Value>& operator=(const BpfMap<Key, Value>&) = delete;
-#endif
-
-    // Move assignment operator
-    BpfMap<Key, Value>& operator=(BpfMap<Key, Value>&& other) noexcept {
-        if (this != &other) {
-            mMapFd = std::move(other.mMapFd);
-            other.reset();
-        }
-        return *this;
-    }
-
-    void reset(unique_fd fd) = delete;
-
-#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING
-    // Note that unique_fd.reset() carefully saves and restores the errno,
-    // and BpfMap.reset() won't touch the errno if passed in fd is negative either,
-    // hence you can do something like BpfMap.reset(systemcall()) and then
-    // check BpfMap.isValid() and look at errno and see why systemcall() failed.
-    [[clang::reinitializes]] void reset(int fd) {
-        mMapFd.reset(fd);
-        if (mMapFd.ok()) abortOnKeyOrValueSizeMismatch();
-    }
-#endif
-
-    [[clang::reinitializes]] void reset() {
-        mMapFd.reset();
-    }
-
-    bool isValid() const { return mMapFd.ok(); }
-
-    Result<void> clear() {
-        while (true) {
-            auto key = getFirstKey();
-            if (!key.ok()) {
-                if (key.error().code() == ENOENT) return {};  // empty: success
-                return key.error();                           // Anything else is an error
-            }
-            auto res = deleteValue(key.value());
-            if (!res.ok()) {
-                // Someone else could have deleted the key, so ignore ENOENT
-                if (res.error().code() == ENOENT) continue;
-                ALOGE("Failed to delete data %s", strerror(res.error().code()));
-                return res.error();
-            }
-        }
-    }
-
-    Result<bool> isEmpty() const {
-        auto key = getFirstKey();
-        if (key.ok()) return false;
-        if (key.error().code() == ENOENT) return true;
-        return key.error();
-    }
-
-  private:
-    unique_fd mMapFd;
 };
 
 template <class Key, class Value>
@@ -312,19 +375,5 @@ Result<void> BpfMap<Key, Value>::iterateWithValue(
     return curKey.error();
 }
 
-template <class Key, class Value>
-class BpfMapRO : public BpfMap<Key, Value> {
-  public:
-    BpfMapRO<Key, Value>() {};
-
-    explicit BpfMapRO<Key, Value>(const char* pathname)
-        : BpfMap<Key, Value>(pathname, BPF_F_RDONLY) {}
-
-    // Function that tries to get map from a pinned path.
-    [[clang::reinitializes]] Result<void> init(const char* path) {
-        return BpfMap<Key,Value>::init(path, mapRetrieveRO(path));
-    }
-};
-
 }  // namespace bpf
 }  // namespace android