Merge changes from topic "b129773125"
* changes:
make BpfMap.reset() harder to use
BpfLoadTest.cpp - construct BpfMap from path not fd
BpfMap - add pinned path based constructors
diff --git a/libbpf_android/BpfLoadTest.cpp b/libbpf_android/BpfLoadTest.cpp
index d8d6da6..d27c70b 100644
--- a/libbpf_android/BpfLoadTest.cpp
+++ b/libbpf_android/BpfLoadTest.cpp
@@ -35,7 +35,7 @@
class BpfLoadTest : public testing::Test {
protected:
BpfLoadTest() {}
- int mProgFd, mMapFd;
+ int mProgFd;
void SetUp() {
SKIP_IF_BPF_NOT_SUPPORTED;
@@ -48,9 +48,6 @@
mProgFd = bpf_obj_get(tp_prog_path);
EXPECT_GT(mProgFd, 0);
- mMapFd = bpf_obj_get(tp_map_path);
- EXPECT_GT(mMapFd, 0);
-
int ret = bpf_attach_tracepoint(mProgFd, "sched", "sched_switch");
EXPECT_NE(ret, 0);
}
@@ -59,7 +56,6 @@
SKIP_IF_BPF_NOT_SUPPORTED;
close(mProgFd);
- close(mMapFd);
unlink(tp_prog_path);
unlink(tp_map_path);
}
@@ -67,7 +63,7 @@
void checkMapNonZero() {
// The test program installs a tracepoint on sched:sched_switch
// and expects the kernel to populate a PID corresponding to CPU
- android::bpf::BpfMap<uint32_t, uint32_t> m(mMapFd);
+ android::bpf::BpfMap<uint32_t, uint32_t> m(tp_map_path);
// Wait for program to run a little
sleep(1);
diff --git a/libbpf_android/BpfMapTest.cpp b/libbpf_android/BpfMapTest.cpp
index 6c28c05..a3c9004 100644
--- a/libbpf_android/BpfMapTest.cpp
+++ b/libbpf_android/BpfMapTest.cpp
@@ -140,7 +140,7 @@
uint32_t value_write = TEST_VALUE1;
writeToMapAndCheck(testMap, key, value_write);
- testMap.reset();
+ testMap.reset(-1);
checkMapInvalid(testMap);
ASSERT_GT(0, findMapEntry(testMap.getMap(), &key, &value_write));
ASSERT_EQ(EBADF, errno);
diff --git a/libbpf_android/include/bpf/BpfMap.h b/libbpf_android/include/bpf/BpfMap.h
index 31efd58..243d9dc 100644
--- a/libbpf_android/include/bpf/BpfMap.h
+++ b/libbpf_android/include/bpf/BpfMap.h
@@ -46,25 +46,20 @@
class BpfMap {
public:
BpfMap<Key, Value>() {};
- explicit BpfMap<Key, Value>(int fd) : mMapFd(fd){};
- // We could technically implement this constructor either with
- // : mMapFd(dup(fd)) {} // fd valid in caller, we have our own local copy
- // or
- // : mMapFd(fd.release()) {} // fd no longer valid in caller, we 'stole' it
- //
- // However, I think we're much better off with a compile time failure, since
- // it's better for whoever passes in a unique_fd to think twice about whether
- // they're trying to pass in ownership or not.
- explicit BpfMap<Key, Value>(base::unique_fd fd) = delete;
+ 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) {
+ int map_fd = mapRetrieve(pathname, flags);
+ if (map_fd >= 0) mMapFd.reset(map_fd);
+ }
+
+ public:
+ explicit BpfMap<Key, Value>(const char* pathname) : BpfMap<Key, Value>(pathname, 0) {}
BpfMap<Key, Value>(bpf_map_type map_type, uint32_t max_entries, uint32_t map_flags) {
int map_fd = createMap(map_type, sizeof(Key), sizeof(Value), max_entries, map_flags);
- if (map_fd < 0) {
- mMapFd.reset(-1);
- } else {
- mMapFd.reset(map_fd);
- }
+ if (map_fd >= 0) mMapFd.reset(map_fd);
}
base::Result<Key> getFirstKey() const {
@@ -136,12 +131,12 @@
// Move constructor
void operator=(BpfMap<Key, Value>&& other) noexcept {
mMapFd = std::move(other.mMapFd);
- other.reset();
+ other.reset(-1);
}
- void reset(int fd = -1) {
- mMapFd.reset(fd);
- }
+ void reset(base::unique_fd fd) = delete;
+
+ void reset(int fd) { mMapFd.reset(fd); }
bool isValid() const { return mMapFd != -1; }
@@ -177,7 +172,6 @@
base::Result<void> BpfMap<Key, Value>::init(const char* path) {
mMapFd = base::unique_fd(mapRetrieve(path, 0));
if (mMapFd == -1) {
- reset();
return base::ErrnoErrorf("Pinned map not accessible or does not exist: ({})", path);
}
return {};
@@ -246,6 +240,13 @@
return curKey.error();
}
+template <class Key, class Value>
+class BpfMapRO : public BpfMap<Key, Value> {
+ public:
+ explicit BpfMapRO<Key, Value>(const char* pathname)
+ : BpfMap<Key, Value>(pathname, BPF_F_RDONLY) {}
+};
+
} // namespace bpf
} // namespace android