Skip QD1A.190821.011 in stage-aosp-master
Bug: 141248619
Change-Id: I37c725007bee3b7210d62b69bc0833abf4524f80
diff --git a/bpfloader/BpfLoader.cpp b/bpfloader/BpfLoader.cpp
index 01c02cc..c9bed5e 100644
--- a/bpfloader/BpfLoader.cpp
+++ b/bpfloader/BpfLoader.cpp
@@ -86,12 +86,6 @@
}
int main() {
- std::string value = android::base::GetProperty("bpf.progs_loaded", "");
- if (value == "1") {
- ALOGI("Property bpf.progs_loaded is set, progs already loaded.\n");
- return 0;
- }
-
if (android::bpf::getBpfSupportLevel() != android::bpf::BpfLevel::NONE) {
// Load all ELF objects, create programs and maps, and pin them
loadAllElfObjects();
diff --git a/libbpf_android/Android.bp b/libbpf_android/Android.bp
index da5c6a6..c369c8c 100644
--- a/libbpf_android/Android.bp
+++ b/libbpf_android/Android.bp
@@ -83,6 +83,7 @@
"libnetdutils",
"libutils",
],
+ require_root: true,
}
cc_test {
diff --git a/libbpf_android/BpfMapTest.cpp b/libbpf_android/BpfMapTest.cpp
index 895dda6..94d273e 100644
--- a/libbpf_android/BpfMapTest.cpp
+++ b/libbpf_android/BpfMapTest.cpp
@@ -52,7 +52,13 @@
class BpfMapTest : public testing::Test {
protected:
BpfMapTest() {}
- int mMapFd;
+
+ // SetUp() will always populate this with a map, but only some tests will use it.
+ // They may use it once via 'mMapFd.release()', or multiple times via 'dup(mMapFd)'
+ // to initialize a BpfMap object.
+ // If it's not used or only dup'ed then TearDown() will close() it, otherwise
+ // whoever got ownership via mMapFd.release() will close() it - possibly much earlier.
+ unique_fd mMapFd;
void SetUp() {
SKIP_IF_BPF_NOT_SUPPORTED;
@@ -61,9 +67,9 @@
if (!access(PINNED_MAP_PATH, R_OK)) {
EXPECT_EQ(0, remove(PINNED_MAP_PATH));
}
- mMapFd = createMap(BPF_MAP_TYPE_HASH, sizeof(uint32_t), sizeof(uint32_t), TEST_MAP_SIZE,
- BPF_F_NO_PREALLOC);
- EXPECT_LE(0, mMapFd);
+ mMapFd.reset(createMap(BPF_MAP_TYPE_HASH, sizeof(uint32_t), sizeof(uint32_t), TEST_MAP_SIZE,
+ BPF_F_NO_PREALLOC));
+ ASSERT_LE(0, mMapFd);
}
void TearDown() {
@@ -72,7 +78,8 @@
if (!access(PINNED_MAP_PATH, R_OK)) {
EXPECT_EQ(0, remove(PINNED_MAP_PATH));
}
- close(mMapFd);
+
+ mMapFd.reset(-1); // close(mMapFd) if still open
}
void checkMapInvalid(BpfMap<uint32_t, uint32_t>& map) {
@@ -117,7 +124,7 @@
BpfMap<uint32_t, uint32_t> testMap1;
checkMapInvalid(testMap1);
- BpfMap<uint32_t, uint32_t> testMap2(mMapFd);
+ BpfMap<uint32_t, uint32_t> testMap2(mMapFd.release());
checkMapValid(testMap2);
BpfMap<uint32_t, uint32_t> testMap3(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC);
@@ -127,7 +134,7 @@
TEST_F(BpfMapTest, basicHelpers) {
SKIP_IF_BPF_NOT_SUPPORTED;
- BpfMap<uint32_t, uint32_t> testMap(mMapFd);
+ BpfMap<uint32_t, uint32_t> testMap(mMapFd.release());
uint32_t key = TEST_KEY1;
uint32_t value_write = TEST_VALUE1;
writeToMapAndCheck(testMap, key, value_write);
@@ -144,21 +151,21 @@
SKIP_IF_BPF_NOT_SUPPORTED;
BpfMap<uint32_t, uint32_t> testMap;
- testMap.reset(mMapFd);
+ testMap.reset(mMapFd.release());
uint32_t key = TEST_KEY1;
uint32_t value_write = TEST_VALUE1;
writeToMapAndCheck(testMap, key, value_write);
+
testMap.reset();
checkMapInvalid(testMap);
- unique_fd invalidFd(mMapFd);
- ASSERT_GT(0, findMapEntry(invalidFd, &key, &value_write));
+ ASSERT_GT(0, findMapEntry(testMap.getMap(), &key, &value_write));
ASSERT_EQ(EBADF, errno);
}
TEST_F(BpfMapTest, moveConstructor) {
SKIP_IF_BPF_NOT_SUPPORTED;
- BpfMap<uint32_t, uint32_t> testMap1(mMapFd);
+ BpfMap<uint32_t, uint32_t> testMap1(mMapFd.release());
BpfMap<uint32_t, uint32_t> testMap2;
testMap2 = std::move(testMap1);
uint32_t key = TEST_KEY1;
@@ -188,7 +195,7 @@
TEST_F(BpfMapTest, iterate) {
SKIP_IF_BPF_NOT_SUPPORTED;
- BpfMap<uint32_t, uint32_t> testMap(mMapFd);
+ BpfMap<uint32_t, uint32_t> testMap(mMapFd.release());
populateMap(TEST_MAP_SIZE, testMap);
int totalCount = 0;
int totalSum = 0;
@@ -208,7 +215,7 @@
TEST_F(BpfMapTest, iterateWithValue) {
SKIP_IF_BPF_NOT_SUPPORTED;
- BpfMap<uint32_t, uint32_t> testMap(mMapFd);
+ BpfMap<uint32_t, uint32_t> testMap(mMapFd.release());
populateMap(TEST_MAP_SIZE, testMap);
int totalCount = 0;
int totalSum = 0;
@@ -230,7 +237,7 @@
TEST_F(BpfMapTest, mapIsEmpty) {
SKIP_IF_BPF_NOT_SUPPORTED;
- BpfMap<uint32_t, uint32_t> testMap(mMapFd);
+ BpfMap<uint32_t, uint32_t> testMap(mMapFd.release());
expectMapEmpty(testMap);
uint32_t key = TEST_KEY1;
uint32_t value_write = TEST_VALUE1;
@@ -262,7 +269,7 @@
TEST_F(BpfMapTest, mapClear) {
SKIP_IF_BPF_NOT_SUPPORTED;
- BpfMap<uint32_t, uint32_t> testMap(mMapFd);
+ BpfMap<uint32_t, uint32_t> testMap(mMapFd.release());
populateMap(TEST_MAP_SIZE, testMap);
auto isEmpty = testMap.isEmpty();
ASSERT_TRUE(isOk(isEmpty));
diff --git a/libbpf_android/include/bpf/BpfMap.h b/libbpf_android/include/bpf/BpfMap.h
index d47698e..a894b57 100644
--- a/libbpf_android/include/bpf/BpfMap.h
+++ b/libbpf_android/include/bpf/BpfMap.h
@@ -46,8 +46,19 @@
template <class Key, class Value>
class BpfMap {
public:
- BpfMap<Key, Value>() : mMapFd(-1){};
+ 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;
+
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) {
diff --git a/progs/include/bpf_helpers.h b/progs/include/bpf_helpers.h
index 408a981..3ab68af 100644
--- a/progs/include/bpf_helpers.h
+++ b/progs/include/bpf_helpers.h
@@ -18,7 +18,7 @@
* Type-unsafe bpf map functions - avoid if possible.
*
* Using these it is possible to pass in keys/values of the wrong type/size,
- * or, for 'unsafe_bpf_map_lookup_elem' receive into a pointer to the wrong type.
+ * or, for 'bpf_map_lookup_elem_unsafe' receive into a pointer to the wrong type.
* You will not get a compile time failure, and for certain types of errors you
* might not even get a failure from the kernel's ebpf verifier during program load,
* instead stuff might just not work right at runtime.
@@ -36,11 +36,11 @@
* This will make sure that if you change the type of a map you'll get compile
* errors at any spots you forget to update with the new type.
*/
-static void* (*unsafe_bpf_map_lookup_elem)(void* map, void* key) = (void*)BPF_FUNC_map_lookup_elem;
-static int (*unsafe_bpf_map_update_elem)(void* map, void* key, void* value,
+static void* (*bpf_map_lookup_elem_unsafe)(void* map, void* key) = (void*)BPF_FUNC_map_lookup_elem;
+static int (*bpf_map_update_elem_unsafe)(void* map, void* key, void* value,
unsigned long long flags) = (void*)
BPF_FUNC_map_update_elem;
-static int (*unsafe_bpf_map_delete_elem)(void* map, void* key) = (void*)BPF_FUNC_map_delete_elem;
+static int (*bpf_map_delete_elem_unsafe)(void* map, void* key) = (void*)BPF_FUNC_map_delete_elem;
/* type safe macro to declare a map and related accessor functions */
#define DEFINE_BPF_MAP_NO_ACCESSORS(the_map, TYPE, TypeOfKey, TypeOfValue, num_entries) \
@@ -56,19 +56,20 @@
\
static inline __always_inline __unused TypeOfValue* bpf_##the_map##_lookup_elem( \
TypeOfKey* k) { \
- return unsafe_bpf_map_lookup_elem(&the_map, k); \
+ return bpf_map_lookup_elem_unsafe(&the_map, k); \
}; \
\
static inline __always_inline __unused int bpf_##the_map##_update_elem( \
TypeOfKey* k, TypeOfValue* v, unsigned long long flags) { \
- return unsafe_bpf_map_update_elem(&the_map, k, v, flags); \
+ return bpf_map_update_elem_unsafe(&the_map, k, v, flags); \
}; \
\
static inline __always_inline __unused int bpf_##the_map##_delete_elem(TypeOfKey* k) { \
- return unsafe_bpf_map_delete_elem(&the_map, k); \
+ return bpf_map_delete_elem_unsafe(&the_map, k); \
};
static int (*bpf_probe_read)(void* dst, int size, void* unsafe_ptr) = (void*) BPF_FUNC_probe_read;
+static int (*bpf_probe_read_str)(void* dst, int size, void* unsafe_ptr) = (void*) BPF_FUNC_probe_read_str;
static unsigned long long (*bpf_ktime_get_ns)(void) = (void*) BPF_FUNC_ktime_get_ns;
static int (*bpf_trace_printk)(const char* fmt, int fmt_size, ...) = (void*) BPF_FUNC_trace_printk;
static unsigned long long (*bpf_get_current_pid_tgid)(void) = (void*) BPF_FUNC_get_current_pid_tgid;