Remove bpf map iterator class
The bpf map iterator class implementation is not very helpful since the
map iterate process may fail because of the bpf syscall. So using a
iterator to go through the map is not very robust if it cannot through
exceptions when it failed. Instead, the map class will have some iterate
function that go through the map and apply operations based on the
lambda function passed in. And it can return failure when the
actual bpf syscall failed.
Bug: 78250686
Test: ./libbpf_test
Change-Id: Ie431ebd6e46c228347c3852fb5b3556534088439
diff --git a/libbpf/BpfMapTest.cpp b/libbpf/BpfMapTest.cpp
index 925117b..3822c10 100644
--- a/libbpf/BpfMapTest.cpp
+++ b/libbpf/BpfMapTest.cpp
@@ -105,6 +105,12 @@
EXPECT_TRUE(isOk(map.writeValue(key, value, BPF_ANY)));
}
}
+
+ void expectMapEmpty(BpfMap<uint32_t, uint32_t>& map) {
+ auto isEmpty = map.isEmpty();
+ ASSERT_TRUE(isOk(isEmpty));
+ ASSERT_TRUE(isEmpty.value());
+ }
};
TEST_F(BpfMapTest, constructor) {
@@ -157,66 +163,6 @@
writeToMapAndCheck(testMap2, key, value);
}
-TEST_F(BpfMapTest, iterateEmptyMap) {
- BpfMap<uint32_t, uint32_t> testMap(mMapFd);
- auto itr = testMap.begin();
- ASSERT_NE(testMap.end(), itr);
- itr.start();
- ASSERT_EQ(testMap.end(), itr);
- ASSERT_FALSE(isOk(itr.next()));
- ASSERT_EQ(testMap.end(), itr);
-}
-
-TEST_F(BpfMapTest, iterator) {
- BpfMap<uint32_t, uint32_t> testMap(mMapFd);
- for (uint32_t key = 0; key < TEST_MAP_SIZE; key++) {
- uint32_t value = key * 10;
- ASSERT_TRUE(isOk(testMap.writeValue(key, value, BPF_ANY)));
- }
- std::vector<uint32_t> valueList;
- auto itr = testMap.begin();
- for (itr.start(); itr != testMap.end(); itr.next()) {
- uint32_t readKey = *itr;
- StatusOr<uint32_t> readValue = testMap.readValue(readKey);
- ASSERT_TRUE(isOk(readValue.status()));
- valueList.push_back(readValue.value());
- }
- ASSERT_EQ((size_t)TEST_MAP_SIZE, valueList.size());
- std::sort(valueList.begin(), valueList.end());
- for (uint32_t key = 0; key < TEST_MAP_SIZE; key++) {
- EXPECT_EQ(key * 10, valueList[key]);
- }
-}
-
-TEST_F(BpfMapTest, twoIterator) {
- BpfMap<uint32_t, uint32_t> testMap(mMapFd);
- for (uint32_t key = 0; key < TEST_MAP_SIZE; key++) {
- uint32_t value = key * 10;
- ASSERT_TRUE(isOk(testMap.writeValue(key, value, BPF_ANY)));
- }
- auto itr1 = testMap.begin();
- auto itr2 = testMap.begin();
- ASSERT_EQ(itr1, itr2);
- ASSERT_TRUE(isOk(itr1.start()));
- ASSERT_NE(itr1, itr2);
- ASSERT_TRUE(isOk(itr2.start()));
- ASSERT_EQ(itr1, itr2);
- uint32_t count = 0;
- while (itr1 != testMap.end()) {
- ASSERT_TRUE(isOk(itr1.next()));
- count++;
- }
- ASSERT_EQ(testMap.end(), itr1);
- ASSERT_EQ(TEST_MAP_SIZE, count);
- while (count != 0) {
- ASSERT_NE(testMap.end(), itr2);
- count--;
- ASSERT_TRUE(isOk(itr2.next()));
- }
- ASSERT_EQ(itr1, itr2);
- ASSERT_EQ(testMap.end(), itr2);
-}
-
TEST_F(BpfMapTest, pinnedToPath) {
BpfMap<uint32_t, uint32_t> testMap1(mMapFd);
EXPECT_TRUE(isOk(testMap1.pinToPath(PINNED_MAP_PATH)));
@@ -263,7 +209,7 @@
EXPECT_TRUE(isOk(testMap.iterate(iterateWithDeletion)));
EXPECT_EQ((int)TEST_MAP_SIZE, totalCount);
EXPECT_EQ(((1 + TEST_MAP_SIZE - 1) * (TEST_MAP_SIZE - 1)) / 2, (uint32_t)totalSum);
- EXPECT_FALSE(isOk(testMap.getFirstKey()));
+ expectMapEmpty(testMap);
}
TEST_F(BpfMapTest, iterateWithValue) {
@@ -283,7 +229,46 @@
EXPECT_TRUE(isOk(testMap.iterateWithValue(iterateWithDeletion)));
EXPECT_EQ((int)TEST_MAP_SIZE, totalCount);
EXPECT_EQ(((1 + TEST_MAP_SIZE - 1) * (TEST_MAP_SIZE - 1)) * 5, (uint32_t)totalSum);
- EXPECT_FALSE(isOk(testMap.getFirstKey()));
+ expectMapEmpty(testMap);
+}
+
+TEST_F(BpfMapTest, mapIsEmpty) {
+ BpfMap<uint32_t, uint32_t> testMap(mMapFd);
+ expectMapEmpty(testMap);
+ uint32_t key = TEST_KEY1;
+ uint32_t value_write = TEST_VALUE1;
+ writeToMapAndCheck(testMap, key, value_write);
+ auto isEmpty = testMap.isEmpty();
+ ASSERT_TRUE(isOk(isEmpty));
+ ASSERT_FALSE(isEmpty.value());
+ ASSERT_TRUE(isOk(testMap.deleteValue(key)));
+ ASSERT_GT(0, findMapEntry(testMap.getMap(), &key, &value_write));
+ ASSERT_EQ(ENOENT, errno);
+ expectMapEmpty(testMap);
+ int entriesSeen = 0;
+ testMap.iterate([&entriesSeen](const unsigned int&,
+ const BpfMap<unsigned int, unsigned int>&) -> netdutils::Status {
+ entriesSeen++;
+ return netdutils::status::ok;
+ });
+ EXPECT_EQ(0, entriesSeen);
+ testMap.iterateWithValue(
+ [&entriesSeen](const unsigned int&, const unsigned int&,
+ const BpfMap<unsigned int, unsigned int>&) -> netdutils::Status {
+ entriesSeen++;
+ return netdutils::status::ok;
+ });
+ EXPECT_EQ(0, entriesSeen);
+}
+
+TEST_F(BpfMapTest, mapClear) {
+ BpfMap<uint32_t, uint32_t> testMap(mMapFd);
+ populateMap(TEST_MAP_SIZE, testMap);
+ auto isEmpty = testMap.isEmpty();
+ ASSERT_TRUE(isOk(isEmpty));
+ ASSERT_FALSE(isEmpty.value());
+ ASSERT_TRUE(isOk(testMap.clear()));
+ expectMapEmpty(testMap);
}
} // namespace bpf
diff --git a/libbpf/include/bpf/BpfMap.h b/libbpf/include/bpf/BpfMap.h
index 10c6a3b..51459f1 100644
--- a/libbpf/include/bpf/BpfMap.h
+++ b/libbpf/include/bpf/BpfMap.h
@@ -46,59 +46,6 @@
template <class Key, class Value>
class BpfMap {
public:
- class const_iterator {
- public:
- netdutils::Status start() {
- if (mMap == nullptr) {
- return netdutils::statusFromErrno(EINVAL, "Invalid map iterator");
- }
- auto firstKey = mMap->getFirstKey();
- if (isOk(firstKey)) {
- mCurKey = firstKey.value();
- } else if (firstKey.status().code() == ENOENT) {
- // The map is empty.
- mMap = nullptr;
- memset(&mCurKey, 0, sizeof(Key));
- } else {
- return firstKey.status();
- }
- return netdutils::status::ok;
- }
-
- netdutils::StatusOr<Key> next() {
- if (mMap == nullptr) {
- return netdutils::statusFromErrno(ENOENT, "Iterating past end of map");
- }
- auto nextKey = mMap->getNextKey(mCurKey);
- if (isOk(nextKey)) {
- mCurKey = nextKey.value();
- } else if (nextKey.status().code() == ENOENT) {
- // iterator reached the end of map
- mMap = nullptr;
- memset(&mCurKey, 0, sizeof(Key));
- } else {
- return nextKey.status();
- }
- return mCurKey;
- }
-
- const Key operator*() { return mCurKey; }
-
- bool operator==(const const_iterator& other) const {
- return (mMap == other.mMap) && (mCurKey == other.mCurKey);
- }
-
- bool operator!=(const const_iterator& other) const { return !(*this == other); }
-
- const_iterator(const BpfMap<Key, Value>* map) : mMap(map) {
- memset(&mCurKey, 0, sizeof(Key));
- }
-
- private:
- const BpfMap<Key, Value> * mMap;
- Key mCurKey;
- };
-
BpfMap<Key, Value>() : mMapFd(-1){};
BpfMap<Key, Value>(int fd) : mMapFd(fd){};
BpfMap<Key, Value>(bpf_map_type map_type, uint32_t max_entries, uint32_t map_flags) {
@@ -226,9 +173,13 @@
return netdutils::status::ok;
}
- const_iterator begin() const { return const_iterator(this); }
-
- const_iterator end() const { return const_iterator(nullptr); }
+ netdutils::StatusOr<bool> isEmpty() const {
+ auto key = this->getFirstKey();
+ // Return error code ENOENT means the map is empty
+ if (!isOk(key) && key.status().code() == ENOENT) return true;
+ RETURN_IF_NOT_OK(key);
+ return false;
+ }
private:
base::unique_fd mMapFd;
@@ -276,63 +227,55 @@
netdutils::Status BpfMap<Key, Value>::iterate(
const std::function<netdutils::Status(const Key& key, const BpfMap<Key, Value>& map)>& filter)
const {
- const_iterator itr = this->begin();
- RETURN_IF_NOT_OK(itr.start());
- while (itr != this->end()) {
- Key prevKey = *itr;
- netdutils::Status advanceStatus = itr.next();
- RETURN_IF_NOT_OK(filter(prevKey, *this));
- RETURN_IF_NOT_OK(advanceStatus);
+ netdutils::StatusOr<Key> curKey = getFirstKey();
+ while (isOk(curKey)) {
+ const netdutils::StatusOr<Key>& nextKey = getNextKey(curKey.value());
+ RETURN_IF_NOT_OK(filter(curKey.value(), *this));
+ curKey = nextKey;
}
- return netdutils::status::ok;
+ return curKey.status().code() == ENOENT ? netdutils::status::ok : curKey.status();
}
template <class Key, class Value>
netdutils::Status BpfMap<Key, Value>::iterateWithValue(
const std::function<netdutils::Status(const Key& key, const Value& value,
const BpfMap<Key, Value>& map)>& filter) const {
- const_iterator itr = this->begin();
- RETURN_IF_NOT_OK(itr.start());
- while (itr != this->end()) {
- Key prevKey = *itr;
- Value prevValue;
- ASSIGN_OR_RETURN(prevValue, this->readValue(prevKey));
- netdutils::Status advanceStatus = itr.next();
- RETURN_IF_NOT_OK(filter(prevKey, prevValue, *this));
- RETURN_IF_NOT_OK(advanceStatus);
+ netdutils::StatusOr<Key> curKey = getFirstKey();
+ while (isOk(curKey)) {
+ const netdutils::StatusOr<Key>& nextKey = getNextKey(curKey.value());
+ Value curValue;
+ ASSIGN_OR_RETURN(curValue, this->readValue(curKey.value()));
+ RETURN_IF_NOT_OK(filter(curKey.value(), curValue, *this));
+ curKey = nextKey;
}
- return netdutils::status::ok;
+ return curKey.status().code() == ENOENT ? netdutils::status::ok : curKey.status();
}
template <class Key, class Value>
netdutils::Status BpfMap<Key, Value>::iterate(
const std::function<netdutils::Status(const Key& key, BpfMap<Key, Value>& map)>& filter) {
- const_iterator itr = this->begin();
- RETURN_IF_NOT_OK(itr.start());
- while (itr != this->end()) {
- Key prevKey = *itr;
- netdutils::Status advanceStatus = itr.next();
- RETURN_IF_NOT_OK(filter(prevKey, *this));
- RETURN_IF_NOT_OK(advanceStatus);
+ netdutils::StatusOr<Key> curKey = getFirstKey();
+ while (isOk(curKey)) {
+ const netdutils::StatusOr<Key>& nextKey = getNextKey(curKey.value());
+ RETURN_IF_NOT_OK(filter(curKey.value(), *this));
+ curKey = nextKey;
}
- return netdutils::status::ok;
+ return curKey.status().code() == ENOENT ? netdutils::status::ok : curKey.status();
}
template <class Key, class Value>
netdutils::Status BpfMap<Key, Value>::iterateWithValue(
const std::function<netdutils::Status(const Key& key, const Value& value,
BpfMap<Key, Value>& map)>& filter) {
- const_iterator itr = this->begin();
- RETURN_IF_NOT_OK(itr.start());
- while (itr != this->end()) {
- Key prevKey = *itr;
- Value prevValue;
- ASSIGN_OR_RETURN(prevValue, this->readValue(prevKey));
- netdutils::Status advanceStatus = itr.next();
- RETURN_IF_NOT_OK(filter(prevKey, prevValue, *this));
- RETURN_IF_NOT_OK(advanceStatus);
+ netdutils::StatusOr<Key> curKey = getFirstKey();
+ while (isOk(curKey)) {
+ const netdutils::StatusOr<Key>& nextKey = getNextKey(curKey.value());
+ Value curValue;
+ ASSIGN_OR_RETURN(curValue, this->readValue(curKey.value()));
+ RETURN_IF_NOT_OK(filter(curKey.value(), curValue, *this));
+ curKey = nextKey;
}
- return netdutils::status::ok;
+ return curKey.status().code() == ENOENT ? netdutils::status::ok : curKey.status();
}
} // namespace bpf
diff --git a/server/TrafficControllerTest.cpp b/server/TrafficControllerTest.cpp
index c7b6f48..eb9ad43 100644
--- a/server/TrafficControllerTest.cpp
+++ b/server/TrafficControllerTest.cpp
@@ -151,8 +151,6 @@
void expectNoTag(uint64_t cookie) { EXPECT_FALSE(isOk(mFakeCookieTagMap.readValue(cookie))); }
- void expectTagMapEmpty() { EXPECT_FALSE(isOk(mFakeCookieTagMap.getFirstKey())); }
-
void populateFakeStats(uint64_t cookie, uint32_t uid, uint32_t tag, StatsKey* key) {
UidTag cookieMapkey = {.uid = (uint32_t)uid, .tag = tag};
EXPECT_TRUE(isOk(mFakeCookieTagMap.writeValue(cookie, cookieMapkey, BPF_ANY)));
@@ -238,6 +236,18 @@
}
}
+ void expectMapEmpty(BpfMap<uint64_t, UidTag>& map) {
+ auto isEmpty = map.isEmpty();
+ EXPECT_TRUE(isOk(isEmpty));
+ EXPECT_TRUE(isEmpty.value());
+ }
+
+ void expectMapEmpty(BpfMap<uint32_t, uint8_t>& map) {
+ auto isEmpty = map.isEmpty();
+ ASSERT_TRUE(isOk(isEmpty));
+ ASSERT_TRUE(isEmpty.value());
+ }
+
void TearDown() {
std::lock_guard<std::mutex> ownerGuard(mTc.mOwnerMatchMutex);
mFakeCookieTagMap.reset();
@@ -259,7 +269,7 @@
expectUidTag(sockCookie, TEST_UID, TEST_TAG);
ASSERT_EQ(0, mTc.untagSocket(v4socket));
expectNoTag(sockCookie);
- expectTagMapEmpty();
+ expectMapEmpty(mFakeCookieTagMap);
}
TEST_F(TrafficControllerTest, TestReTagSocket) {
@@ -295,7 +305,7 @@
expectUidTag(sockCookie, TEST_UID, TEST_TAG);
ASSERT_EQ(0, mTc.untagSocket(v6socket));
expectNoTag(sockCookie);
- expectTagMapEmpty();
+ expectMapEmpty(mFakeCookieTagMap);
}
TEST_F(TrafficControllerTest, TestTagInvalidSocket) {
@@ -303,7 +313,7 @@
int invalidSocket = -1;
ASSERT_GT(0, mTc.tagSocket(invalidSocket, TEST_TAG, TEST_UID));
- expectTagMapEmpty();
+ expectMapEmpty(mFakeCookieTagMap);
}
TEST_F(TrafficControllerTest, TestUntagInvalidSocket) {
@@ -313,7 +323,7 @@
ASSERT_GT(0, mTc.untagSocket(invalidSocket));
int v4socket = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
ASSERT_GT(0, mTc.untagSocket(v4socket));
- expectTagMapEmpty();
+ expectMapEmpty(mFakeCookieTagMap);
}
TEST_F(TrafficControllerTest, TestSetCounterSet) {
@@ -326,7 +336,7 @@
ASSERT_EQ(TEST_COUNTERSET, counterSetResult.value());
ASSERT_EQ(0, mTc.setCounterSet(DEFAULT_COUNTERSET, TEST_UID));
ASSERT_FALSE(isOk(mFakeUidCounterSetMap.readValue(uid)));
- ASSERT_FALSE(isOk(mFakeUidCounterSetMap.getFirstKey()));
+ expectMapEmpty(mFakeUidCounterSetMap);
}
TEST_F(TrafficControllerTest, TestSetInvalidCounterSet) {
@@ -335,7 +345,7 @@
ASSERT_GT(0, mTc.setCounterSet(OVERFLOW_COUNTERSET, TEST_UID));
uid_t uid = TEST_UID;
ASSERT_FALSE(isOk(mFakeUidCounterSetMap.readValue(uid)));
- ASSERT_FALSE(isOk(mFakeUidCounterSetMap.getFirstKey()));
+ expectMapEmpty(mFakeUidCounterSetMap);
}
TEST_F(TrafficControllerTest, TestDeleteTagData) {
@@ -505,7 +515,7 @@
expectBandwidthMapValues(appStrUids, BLACKLISTMATCH);
ASSERT_TRUE(isOk(mTc.updateBandwidthUidMap(appStrUids, BandwidthController::IptJumpReject,
BandwidthController::IptOpDelete)));
- ASSERT_FALSE(isOk(mFakeBandwidthUidMap.getFirstKey()));
+ expectMapEmpty(mFakeBandwidthUidMap);
}
TEST_F(TrafficControllerTest, TestWhitelistUidMatch) {
@@ -517,7 +527,7 @@
expectBandwidthMapValues(appStrUids, WHITELISTMATCH);
ASSERT_TRUE(isOk(mTc.updateBandwidthUidMap(appStrUids, BandwidthController::IptJumpReturn,
BandwidthController::IptOpDelete)));
- ASSERT_FALSE(isOk(mFakeBandwidthUidMap.getFirstKey()));
+ expectMapEmpty(mFakeBandwidthUidMap);
}
TEST_F(TrafficControllerTest, TestReplaceMatchUid) {
@@ -548,7 +558,7 @@
// If the uid does not exist in the map, trying to delete a rule about it will fail.
ASSERT_FALSE(isOk(mTc.updateBandwidthUidMap(appStrUids, BandwidthController::IptJumpReject,
BandwidthController::IptOpDelete)));
- ASSERT_FALSE(isOk(mFakeBandwidthUidMap.getFirstKey()));
+ expectMapEmpty(mFakeBandwidthUidMap);
// Add blacklist rules for appStrUids.
ASSERT_TRUE(isOk(mTc.updateBandwidthUidMap(appStrUids, BandwidthController::IptJumpReturn,