Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for unlimited map size #3348

Merged
merged 25 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
57b5b28
backup
saxena-anurag Mar 7, 2024
ce0d6b1
fix tests
saxena-anurag Mar 11, 2024
cf0b425
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Mar 11, 2024
48447d9
Merge branch 'microsoft:main' into user/anusa/issue_3328
saxena-anurag Mar 15, 2024
16efb95
update tests
saxena-anurag Mar 15, 2024
42f07be
add tests
saxena-anurag Mar 16, 2024
520c3b6
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Mar 16, 2024
f90a19b
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Mar 26, 2024
1a75d31
fix tests
saxena-anurag Mar 26, 2024
5f68c88
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Mar 26, 2024
f594438
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Apr 13, 2024
286507e
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Apr 18, 2024
94c93ec
fix test
saxena-anurag Apr 18, 2024
81d25c0
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Apr 18, 2024
1870d7f
fix
saxena-anurag Apr 18, 2024
1ba29b2
Merge branch 'microsoft:main' into user/anusa/issue_3328
saxena-anurag Apr 22, 2024
389d5aa
fix tests
saxena-anurag Apr 22, 2024
16dfc9a
cr comments
saxena-anurag Apr 22, 2024
c0c4e06
Apply suggestions from code review
saxena-anurag Apr 22, 2024
2ea0d97
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Apr 22, 2024
a7fbc89
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Apr 22, 2024
6fa6a6a
cr comments
saxena-anurag Apr 22, 2024
9fae70b
Merge branch 'microsoft:main' into user/anusa/issue_3328
saxena-anurag Apr 24, 2024
18b6f0a
Merge branch 'main' into user/anusa/issue_3328
saxena-anurag Apr 25, 2024
c27ead4
Merge branch 'main' into user/anusa/issue_3328
Alan-Jowett Apr 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions libs/execution_context/ebpf_maps.c
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,7 @@ _create_hash_map_internal(
size_t map_struct_size,
_In_ const ebpf_map_definition_in_memory_t* map_definition,
size_t supplemental_value_size,
bool fixed_size_map,
_In_opt_ void (*extract_function)(
_In_ const uint8_t* value, _Outptr_ const uint8_t** data, _Out_ size_t* length_in_bits),
_In_opt_ ebpf_hash_table_notification_function notification_callback,
Expand All @@ -850,7 +851,7 @@ _create_hash_map_internal(
.key_size = local_map->ebpf_map_definition.key_size,
.value_size = local_map->ebpf_map_definition.value_size,
.minimum_bucket_count = local_map->ebpf_map_definition.max_entries,
.max_entries = local_map->ebpf_map_definition.max_entries,
.max_entries = fixed_size_map ? local_map->ebpf_map_definition.max_entries : EBPF_HASH_TABLE_NO_LIMIT,
.extract_function = extract_function,
.supplemental_value_size = supplemental_value_size,
.notification_context = local_map,
Expand Down Expand Up @@ -890,7 +891,7 @@ _create_hash_map(
if (inner_map_handle != ebpf_handle_invalid) {
return EBPF_INVALID_ARGUMENT;
}
return _create_hash_map_internal(sizeof(ebpf_core_map_t), map_definition, 0, NULL, NULL, map);
return _create_hash_map_internal(sizeof(ebpf_core_map_t), map_definition, 0, false, NULL, NULL, map);
}

static void
Expand Down Expand Up @@ -939,7 +940,8 @@ _create_object_hash_map(

*map = NULL;

result = _create_hash_map_internal(sizeof(ebpf_core_object_map_t), map_definition, 0, NULL, NULL, &local_map);
result =
_create_hash_map_internal(sizeof(ebpf_core_object_map_t), map_definition, 0, false, NULL, NULL, &local_map);
if (result != EBPF_SUCCESS) {
goto Exit;
}
Expand Down Expand Up @@ -1169,6 +1171,7 @@ _create_lru_hash_map(
sizeof(ebpf_core_lru_map_t),
map_definition,
supplemental_value_size,
true,
NULL,
_lru_hash_table_notification,
(ebpf_core_map_t**)&lru_map);
Expand Down Expand Up @@ -1393,11 +1396,10 @@ _update_hash_map_entry_with_handle(

uint8_t* old_value = NULL;
ebpf_result_t found_result = ebpf_hash_table_find((ebpf_hash_table_t*)map->data, key, &old_value);
if ((found_result != EBPF_SUCCESS) && (entry_count == map->ebpf_map_definition.max_entries)) {

if ((found_result != EBPF_SUCCESS) && (found_result != EBPF_KEY_NOT_FOUND)) {
// The hash table is already full.
EBPF_LOG_MESSAGE(EBPF_TRACELOG_LEVEL_ERROR, EBPF_TRACELOG_KEYWORD_MAP, "Hash table full");
result = EBPF_OUT_OF_SPACE;
EBPF_LOG_MESSAGE(EBPF_TRACELOG_LEVEL_ERROR, EBPF_TRACELOG_KEYWORD_MAP, "Failed to query existing entry");
result = found_result;
goto Done;
}

Expand Down Expand Up @@ -1578,6 +1580,7 @@ _create_lpm_map(
EBPF_OFFSET_OF(ebpf_core_lpm_map_t, data) + ebpf_bitmap_size(max_prefix_length),
map_definition,
0,
false,
_lpm_extract,
NULL,
(ebpf_core_map_t**)&lpm_map);
Expand Down
89 changes: 67 additions & 22 deletions libs/execution_context/unit/execution_context_unit_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,16 @@ _test_crud_operations(ebpf_map_type_t map_type)
bool is_array;
bool supports_find_and_delete;
bool replace_on_full;
bool insert_on_full = false;
dthaler marked this conversation as resolved.
Show resolved Hide resolved
bool run_at_dpc;
ebpf_result_t error_on_full;
ebpf_result_t expected_result;
switch (map_type) {
case BPF_MAP_TYPE_HASH:
is_array = false;
supports_find_and_delete = true;
replace_on_full = false;
insert_on_full = true;
run_at_dpc = false;
error_on_full = EBPF_OUT_OF_SPACE;
break;
Expand All @@ -145,6 +148,7 @@ _test_crud_operations(ebpf_map_type_t map_type)
is_array = false;
supports_find_and_delete = true;
replace_on_full = false;
insert_on_full = true;
run_at_dpc = true;
error_on_full = EBPF_OUT_OF_SPACE;
break;
Expand Down Expand Up @@ -178,6 +182,9 @@ _test_crud_operations(ebpf_map_type_t map_type)
dpc = {emulate_dpc_t(1)};
}

// For each map type, maximum only one of replace_on_full and insert_on_full should be true.
dthaler marked this conversation as resolved.
Show resolved Hide resolved
REQUIRE(((replace_on_full && insert_on_full) == false));

ebpf_map_definition_in_memory_t map_definition{map_type, sizeof(uint32_t), sizeof(uint64_t), _test_map_size};
map_ptr map;
{
Expand Down Expand Up @@ -212,19 +219,23 @@ _test_crud_operations(ebpf_map_type_t map_type)
value.size(),
value.data(),
EBPF_ANY,
0) == (replace_on_full ? EBPF_SUCCESS : error_on_full));
0) == ((replace_on_full || insert_on_full) ? EBPF_SUCCESS : error_on_full));

if (!replace_on_full) {
ebpf_result_t expected_result = is_array ? EBPF_INVALID_ARGUMENT : EBPF_KEY_NOT_FOUND;
expected_result = insert_on_full ? EBPF_SUCCESS : (is_array ? EBPF_INVALID_ARGUMENT : EBPF_KEY_NOT_FOUND);
REQUIRE(
ebpf_map_delete_entry(map.get(), sizeof(bad_key), reinterpret_cast<const uint8_t*>(&bad_key), 0) ==
expected_result);
}

// Now the map has `_test_map_size` entries.

for (uint32_t key = 0; key < _test_map_size; key++) {
ebpf_result_t expected_result;
if (replace_on_full) {
// If replace_on_full is true, then 0th entry would have been evicted.
expected_result = key == 0 ? EBPF_OBJECT_NOT_FOUND : EBPF_SUCCESS;
} else if (insert_on_full) {
expected_result = EBPF_SUCCESS;
} else {
expected_result = key == _test_map_size ? EBPF_OBJECT_NOT_FOUND : EBPF_SUCCESS;
}
Expand Down Expand Up @@ -252,6 +263,7 @@ _test_crud_operations(ebpf_map_type_t map_type)
keys.insert(previous_key);
}
REQUIRE(keys.size() == _test_map_size);

REQUIRE(
ebpf_map_next_key(
map.get(),
Expand Down Expand Up @@ -386,15 +398,6 @@ TEST_CASE("map_crud_operations_lpm_trie_32", "[execution_context]")
uint32_t prefix_length;
uint8_t value[4];
} lpm_trie_key_t;
ebpf_map_definition_in_memory_t map_definition{BPF_MAP_TYPE_LPM_TRIE, sizeof(lpm_trie_key_t), max_string, 10};
map_ptr map;
{
ebpf_map_t* local_map;
cxplat_utf8_string_t map_name = {0};
REQUIRE(
ebpf_map_create(&map_name, &map_definition, (uintptr_t)ebpf_handle_invalid, &local_map) == EBPF_SUCCESS);
map.reset(local_map);
}

std::vector<std::pair<lpm_trie_key_t, const char*>> keys{
{{24, 192, 168, 15, 0}, "192.168.15.0/24"},
Expand All @@ -420,6 +423,18 @@ TEST_CASE("map_crud_operations_lpm_trie_32", "[execution_context]")
{{32, 11, 0, 0, 0}, "0.0.0.0/0"},
};

uint32_t max_entries = static_cast<uint32_t>(keys.size());
ebpf_map_definition_in_memory_t map_definition{
BPF_MAP_TYPE_LPM_TRIE, sizeof(lpm_trie_key_t), max_string, max_entries};
map_ptr map;
{
ebpf_map_t* local_map;
cxplat_utf8_string_t map_name = {0};
REQUIRE(
ebpf_map_create(&map_name, &map_definition, (uintptr_t)ebpf_handle_invalid, &local_map) == EBPF_SUCCESS);
map.reset(local_map);
}

for (auto& [key, value] : keys) {
std::string local_value = value;
local_value.resize(max_string);
Expand All @@ -446,6 +461,20 @@ TEST_CASE("map_crud_operations_lpm_trie_32", "[execution_context]")
EBPF_MAP_FLAG_HELPER) == EBPF_SUCCESS);
REQUIRE(std::string(value) == result);
}

// Add a new entry to the map, it should succeed.
lpm_trie_key_t new_key = {32, 192, 168, 15, 1};
std::string new_value = "19.168.15.1/32";
new_value.resize(max_string);
REQUIRE(
ebpf_map_update_entry(
map.get(),
0,
reinterpret_cast<const uint8_t*>(&new_key),
0,
reinterpret_cast<const uint8_t*>(new_value.c_str()),
EBPF_ANY,
EBPF_MAP_FLAG_HELPER) == EBPF_SUCCESS);
}

void
Expand All @@ -471,16 +500,6 @@ TEST_CASE("map_crud_operations_lpm_trie_128", "[execution_context]")
uint8_t value[16];
} lpm_trie_key_t;

ebpf_map_definition_in_memory_t map_definition{BPF_MAP_TYPE_LPM_TRIE, sizeof(lpm_trie_key_t), max_string, 10};
map_ptr map;
{
ebpf_map_t* local_map;
cxplat_utf8_string_t map_name = {0};
REQUIRE(
ebpf_map_create(&map_name, &map_definition, (uintptr_t)ebpf_handle_invalid, &local_map) == EBPF_SUCCESS);
map.reset(local_map);
}

std::vector<std::pair<lpm_trie_key_t, const char*>> keys{
{{96}, "CC/96"},
{{96}, "CD/96"},
Expand All @@ -507,6 +526,19 @@ TEST_CASE("map_crud_operations_lpm_trie_128", "[execution_context]")
generate_prefix(keys[index].first.prefix_length, values[index], keys[index].first.value);
}
}

uint32_t max_entries = static_cast<uint32_t>(keys.size());
ebpf_map_definition_in_memory_t map_definition{
BPF_MAP_TYPE_LPM_TRIE, sizeof(lpm_trie_key_t), max_string, max_entries};
map_ptr map;
{
ebpf_map_t* local_map;
cxplat_utf8_string_t map_name = {0};
REQUIRE(
ebpf_map_create(&map_name, &map_definition, (uintptr_t)ebpf_handle_invalid, &local_map) == EBPF_SUCCESS);
map.reset(local_map);
}

std::vector<std::pair<lpm_trie_key_t, std::string>> tests{
{{96}, "CC/96"},
{{96}, "CD/96"},
Expand Down Expand Up @@ -561,6 +593,19 @@ TEST_CASE("map_crud_operations_lpm_trie_128", "[execution_context]")
EBPF_MAP_FLAG_HELPER) == EBPF_SUCCESS);
REQUIRE(std::string(value) == result);
}

// Add a new entry to the map, it should succeed.
lpm_trie_key_t new_key = {{32}, "BB/32"};
std::string new_value = "BB/32";
REQUIRE(
ebpf_map_update_entry(
map.get(),
0,
reinterpret_cast<const uint8_t*>(&new_key),
0,
reinterpret_cast<const uint8_t*>(new_value.c_str()),
EBPF_ANY,
EBPF_MAP_FLAG_HELPER) == EBPF_SUCCESS);
}

TEST_CASE("map_crud_operations_queue", "[execution_context]")
Expand Down
117 changes: 117 additions & 0 deletions tests/end_to_end/end_to_end.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3215,3 +3215,120 @@ TEST_CASE("multiple_map_insert", "[close_cleanup]")

bpf_object__close(unique_object.release());
}

void
test_no_limit_map_entries(ebpf_map_type_t type, bool max_entries_limited)
{
uint32_t max_entries = 2;
fd_t inner_map_fd = ebpf_fd_invalid;
fd_t map_fd = ebpf_fd_invalid;
bool nested_map = (type == BPF_MAP_TYPE_HASH_OF_MAPS);
saxena-anurag marked this conversation as resolved.
Show resolved Hide resolved
void* value = nullptr;
uint32_t key_size = 0;
uint32_t value_size = 0;
void* key = nullptr;

#define IS_LRU_MAP(type) ((type) == BPF_MAP_TYPE_LRU_HASH || (type) == BPF_MAP_TYPE_LRU_PERCPU_HASH)
#define IS_PERCPU_MAP(type) ((type) == BPF_MAP_TYPE_PERCPU_HASH || (type) == BPF_MAP_TYPE_LRU_PERCPU_HASH)
#define IS_LPM_MAP(type) ((type) == BPF_MAP_TYPE_LPM_TRIE)

typedef struct _lpm_trie_key
{
uint32_t prefix_length;
uint32_t value;
} lpm_trie_key_t;

lpm_trie_key_t trie_key = {0};

if (nested_map) {
// First create and pin the maps manually.
inner_map_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, nullptr, sizeof(int32_t), sizeof(int32_t), 1, nullptr);
REQUIRE(inner_map_fd > 0);

bpf_map_create_opts opts = {.inner_map_fd = (uint32_t)inner_map_fd};
key_size = sizeof(int32_t);
value_size = sizeof(fd_t);
map_fd = bpf_map_create(BPF_MAP_TYPE_HASH_OF_MAPS, nullptr, key_size, value_size, 1, &opts);
REQUIRE(map_fd > 0);
} else {
key_size = (type == BPF_MAP_TYPE_LPM_TRIE) ? sizeof(lpm_trie_key_t) : sizeof(int32_t);
saxena-anurag marked this conversation as resolved.
Show resolved Hide resolved
value_size = sizeof(int32_t);
map_fd = bpf_map_create(type, nullptr, key_size, value_size, max_entries, nullptr);
REQUIRE(map_fd > 0);
}

// Update value_size for percpu maps for read / update operations.
if (IS_PERCPU_MAP(type)) {
value_size = EBPF_PAD_8(value_size) * static_cast<size_t>(libbpf_num_possible_cpus());
}
std::vector<uint8_t> per_cpu_value(value_size);

auto compute_key = [&](uint32_t* i) -> void* {
if (IS_LPM_MAP(type)) {
trie_key.prefix_length = 32;
trie_key.value = *i;
return &trie_key;
} else {
return i;
}
};

// Add `max_entries` entries to the map.
for (uint32_t i = 0; i < max_entries; i++) {
key = compute_key(&i);
if (IS_PERCPU_MAP(type)) {
value = per_cpu_value.data();
} else {
value = nested_map ? &inner_map_fd : (int32_t*)&i;
}
REQUIRE(bpf_map_update_elem(map_fd, key, value, 0) == 0);
}

// Add one more entry to the map.
if (IS_PERCPU_MAP(type)) {
value = per_cpu_value.data();
} else {
value = nested_map ? &inner_map_fd : (int32_t*)&max_entries;
}
// In case of LRU_HASH, insert will succeed, but the oldest entry will be removed.
saxena-anurag marked this conversation as resolved.
Show resolved Hide resolved
int expected_error = (!max_entries_limited || IS_LRU_MAP(type)) ? 0 : -ENOSPC;
key = compute_key(&max_entries);
REQUIRE(bpf_map_update_elem(map_fd, key, value, 0) == (max_entries_limited ? expected_error : 0));

// In case of LRU_HASH, check that the number of entries is still `max_entries`.
if (IS_LRU_MAP(type) && max_entries_limited) {
uint32_t entries_count = 0;
lpm_trie_key_t local_key = {0};
void* old_key = nullptr;
void* next_key = &local_key;

while (bpf_map_get_next_key(map_fd, old_key, next_key) == 0) {
old_key = next_key;
entries_count++;
}
REQUIRE(entries_count == max_entries);
}
}

// This test case tests the map limits of various hash table based map types.
TEST_CASE("test_map_entries_limit", "[end_to_end]")
{
_test_helper_end_to_end test_helper;
test_helper.initialize();

// The below hash table based map types do not have a limit on the number of entries.
// 1. BPF_MAP_TYPE_HASH
// 2. BPF_MAP_TYPE_PERCPU_HASH
// 3. BPF_MAP_TYPE_HASH_OF_MAPS
// 4. BPF_MAP_TYPE_LPM_TRIE
test_no_limit_map_entries(BPF_MAP_TYPE_HASH, false);
test_no_limit_map_entries(BPF_MAP_TYPE_PERCPU_HASH, false);
test_no_limit_map_entries(BPF_MAP_TYPE_HASH_OF_MAPS, false);
test_no_limit_map_entries(BPF_MAP_TYPE_LPM_TRIE, false);

// The below hash table based map types have a limit on the number of entries.
// 1. BPF_MAP_TYPE_LRU_HASH
// 2. BPF_MAP_TYPE_LRU_PERCPU_HASH
test_no_limit_map_entries(BPF_MAP_TYPE_LRU_HASH, true);
test_no_limit_map_entries(BPF_MAP_TYPE_LRU_PERCPU_HASH, true);
}
Loading