switch netd_configuration_map from hash map to array

This eliminates the need for netd_updatable BpfHandler.cpp
to initialize the hash map with a zero.

On startup the map will be freshly initialized and thus zero.

On restart it might not be empty, but it doesn't matter to netd.
Furthermore the mainline component of the system server will
re-initialize it again anyway:
see service/native/TrafficController.cpp initMaps()

This does remove the ability to call deleteValue on a key,
since that would always return -EINVAL, but since we don't
currently do that, that's really a feature.

(It does suggest though that we should have a BpfMapNonNullable
 class which is writeable, but without a deleteValue() function)

Additionally BpfMap arrays are more efficient for the kernel bpf jit
compiler, as - on newer kernels - it can optimize the read/write
into a simple memory access (as opposed to a bpf helper call).

Before:
  $ adb shell ls -l /sys/fs/bpf/netd_shared/map_netd_configuration_map
  -rw-rw---- 1 root net_bw_acct 0 2022-06-11 08:20 /sys/fs/bpf/netd_shared/ map_netd_configuration_map

After:
  $ adbz shell ls -l /sys/fs/bpf/netd_shared/map_netd_configuration_map
  -r--rw---- 1 root net_bw_acct 0 2022-06-16 15:03 /sys/fs/bpf/netd_shared/map_netd_configuration_map

Bug: 235590615
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I21730e4fa22fbf0c94ab0ca5c5db03aa000b7680
This commit is contained in:
Maciej Żenczykowski
2022-06-16 14:49:27 -07:00
parent 4cea149d66
commit b10e055f4b
6 changed files with 16 additions and 13 deletions

View File

@@ -190,9 +190,9 @@ typedef struct {
STRUCT_SIZE(UidOwnerValue, 2 * 4); // 8
// Entry in the configuration map that stores which UID rules are enabled.
#define UID_RULES_CONFIGURATION_KEY 1
#define UID_RULES_CONFIGURATION_KEY 0
// Entry in the configuration map that stores which stats map is currently in use.
#define CURRENT_STATS_MAP_CONFIGURATION_KEY 2
#define CURRENT_STATS_MAP_CONFIGURATION_KEY 1
typedef struct {
uint32_t iif; // The input interface index

View File

@@ -63,13 +63,18 @@
#define DEFINE_BPF_MAP_RW_NETD(the_map, TYPE, TypeOfKey, TypeOfValue, num_entries) \
DEFINE_BPF_MAP_UGM(the_map, TYPE, TypeOfKey, TypeOfValue, num_entries, AID_ROOT, AID_NET_BW_ACCT, 0660)
// Bpf map arrays on creation are preinitialized to 0 and do not support deletion of a key,
// see: kernel/bpf/arraymap.c array_map_delete_elem() returns -EINVAL (from both syscall and ebpf)
// Additionally on newer kernels the bpf jit can optimize out the lookups.
// only valid indexes are [0..CONFIGURATION_MAP_SIZE-1]
DEFINE_BPF_MAP_RO_NETD(configuration_map, ARRAY, uint32_t, uint32_t, CONFIGURATION_MAP_SIZE)
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_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_RW_NETD(configuration_map, HASH, uint32_t, uint32_t, CONFIGURATION_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)

View File

@@ -101,8 +101,6 @@ Status BpfHandler::initMaps() {
RETURN_IF_NOT_OK(mStatsMapA.init(STATS_MAP_A_PATH));
RETURN_IF_NOT_OK(mStatsMapB.init(STATS_MAP_B_PATH));
RETURN_IF_NOT_OK(mConfigurationMap.init(CONFIGURATION_MAP_PATH));
RETURN_IF_NOT_OK(mConfigurationMap.writeValue(CURRENT_STATS_MAP_CONFIGURATION_KEY, SELECT_MAP_A,
BPF_ANY));
RETURN_IF_NOT_OK(mUidPermissionMap.init(UID_PERMISSION_MAP_PATH));
ALOGI("%s successfully", __func__);

View File

@@ -63,7 +63,7 @@ class BpfHandler {
BpfMap<uint64_t, UidTagValue> mCookieTagMap;
BpfMap<StatsKey, StatsValue> mStatsMapA;
BpfMapRO<StatsKey, StatsValue> mStatsMapB;
BpfMap<uint32_t, uint32_t> mConfigurationMap;
BpfMapRO<uint32_t, uint32_t> mConfigurationMap;
BpfMap<uint32_t, uint8_t> mUidPermissionMap;
std::mutex mMutex;

View File

@@ -49,7 +49,7 @@ class BpfHandlerTest : public ::testing::Test {
BpfHandler mBh;
BpfMap<uint64_t, UidTagValue> mFakeCookieTagMap;
BpfMap<StatsKey, StatsValue> mFakeStatsMapA;
BpfMap<uint32_t, uint32_t> mFakeConfigurationMap;
BpfMapRO<uint32_t, uint32_t> mFakeConfigurationMap;
BpfMap<uint32_t, uint8_t> mFakeUidPermissionMap;
void SetUp() {
@@ -62,7 +62,7 @@ class BpfHandlerTest : public ::testing::Test {
mFakeStatsMapA.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
ASSERT_VALID(mFakeStatsMapA);
mFakeConfigurationMap.resetMap(BPF_MAP_TYPE_HASH, 1);
mFakeConfigurationMap.resetMap(BPF_MAP_TYPE_ARRAY, CONFIGURATION_MAP_SIZE);
ASSERT_VALID(mFakeConfigurationMap);
mFakeUidPermissionMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
@@ -75,8 +75,8 @@ class BpfHandlerTest : public ::testing::Test {
mBh.mConfigurationMap = mFakeConfigurationMap;
ASSERT_VALID(mBh.mConfigurationMap);
// Always write to stats map A by default.
ASSERT_RESULT_OK(mBh.mConfigurationMap.writeValue(CURRENT_STATS_MAP_CONFIGURATION_KEY,
SELECT_MAP_A, BPF_ANY));
static_assert(SELECT_MAP_A == 0, "bpf map arrays are zero-initialized");
mBh.mUidPermissionMap = mFakeUidPermissionMap;
ASSERT_VALID(mBh.mUidPermissionMap);
}

View File

@@ -98,7 +98,7 @@ class TrafficControllerTest : public ::testing::Test {
mFakeStatsMapA.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
ASSERT_VALID(mFakeStatsMapA);
mFakeConfigurationMap.resetMap(BPF_MAP_TYPE_HASH, 1);
mFakeConfigurationMap.resetMap(BPF_MAP_TYPE_ARRAY, CONFIGURATION_MAP_SIZE);
ASSERT_VALID(mFakeConfigurationMap);
mFakeUidOwnerMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);
@@ -122,8 +122,8 @@ class TrafficControllerTest : public ::testing::Test {
ASSERT_VALID(mTc.mConfigurationMap);
// Always write to stats map A by default.
ASSERT_RESULT_OK(mTc.mConfigurationMap.writeValue(CURRENT_STATS_MAP_CONFIGURATION_KEY,
SELECT_MAP_A, BPF_ANY));
static_assert(SELECT_MAP_A == 0);
mTc.mUidOwnerMap = mFakeUidOwnerMap;
ASSERT_VALID(mTc.mUidOwnerMap);
mTc.mUidPermissionMap = mFakeUidPermissionMap;