Add support for unlimited map size (#3348)

* backup

* fix tests

* update tests

* add tests

* fix tests

* fix test

* fix

* fix tests

* cr comments

* Apply suggestions from code review

Co-authored-by: Dave Thaler <dthaler1968@gmail.com>

* cr comments

---------

Co-authored-by: Dave Thaler <dthaler1968@gmail.com>
Co-authored-by: Alan Jowett <alanjo@microsoft.com>

libs/execution_context/ebpf_maps.c

@@ -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,
@@ -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,
@@ -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
@@ -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;
     }
@@ -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);
@@ -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");
+        EBPF_LOG_MESSAGE(EBPF_TRACELOG_LEVEL_ERROR, EBPF_TRACELOG_KEYWORD_MAP, "Failed to query existing entry");
-        result = EBPF_OUT_OF_SPACE;
+        result = found_result;
         goto Done;
     }
 
@@ -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);

libs/execution_context/unit/execution_context_unit_test.cpp

@@ -116,6 +116,13 @@ typedef std::unique_ptr<ebpf_link_t, ebpf_object_deleter<ebpf_link_t>> link_ptr;
 
 static const uint32_t _test_map_size = 512;
 
+typedef enum _map_behavior_on_max_entries
+{
+    MAP_BEHAVIOR_FAIL,
+    MAP_BEHAVIOR_REPLACE,
+    MAP_BEHAVIOR_INSERT,
+} map_behavior_on_max_entries_t;
+
 static void
 _test_crud_operations(ebpf_map_type_t map_type)
 {
@@ -123,49 +130,48 @@ _test_crud_operations(ebpf_map_type_t map_type)
     core.initialize();
     bool is_array;
     bool supports_find_and_delete;
-    bool replace_on_full;
+    map_behavior_on_max_entries_t behavior_on_max_entries = MAP_BEHAVIOR_FAIL;
     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;
+        behavior_on_max_entries = MAP_BEHAVIOR_INSERT;
         run_at_dpc = false;
         error_on_full = EBPF_OUT_OF_SPACE;
         break;
     case BPF_MAP_TYPE_ARRAY:
         is_array = true;
         supports_find_and_delete = false;
-        replace_on_full = false;
         run_at_dpc = false;
         error_on_full = EBPF_INVALID_ARGUMENT;
         break;
     case BPF_MAP_TYPE_PERCPU_HASH:
         is_array = false;
         supports_find_and_delete = true;
-        replace_on_full = false;
+        behavior_on_max_entries = MAP_BEHAVIOR_INSERT;
         run_at_dpc = true;
         error_on_full = EBPF_OUT_OF_SPACE;
         break;
     case BPF_MAP_TYPE_PERCPU_ARRAY:
         is_array = true;
         supports_find_and_delete = false;
-        replace_on_full = false;
         run_at_dpc = false;
         error_on_full = EBPF_INVALID_ARGUMENT;
         break;
     case BPF_MAP_TYPE_LRU_HASH:
         is_array = false;
         supports_find_and_delete = true;
-        replace_on_full = true;
+        behavior_on_max_entries = MAP_BEHAVIOR_REPLACE;
         run_at_dpc = false;
         error_on_full = EBPF_OUT_OF_SPACE;
         break;
     case BPF_MAP_TYPE_LRU_PERCPU_HASH:
         is_array = false;
         supports_find_and_delete = true;
-        replace_on_full = true;
+        behavior_on_max_entries = MAP_BEHAVIOR_REPLACE;
         run_at_dpc = true;
         error_on_full = EBPF_OUT_OF_SPACE;
         break;
@@ -212,19 +218,25 @@ _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) == ((behavior_on_max_entries != MAP_BEHAVIOR_FAIL) ? EBPF_SUCCESS : error_on_full));
 
-    if (!replace_on_full) {
+    if (behavior_on_max_entries != MAP_BEHAVIOR_REPLACE) {
-        ebpf_result_t expected_result = is_array ? EBPF_INVALID_ARGUMENT : EBPF_KEY_NOT_FOUND;
+        expected_result = (behavior_on_max_entries == MAP_BEHAVIOR_INSERT)
+                              ? 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 (behavior_on_max_entries == MAP_BEHAVIOR_REPLACE) {
-        if (replace_on_full) {
+            // If map behavior is MAP_BEHAVIOR_REPLACE, then 0th entry would have been evicted.
             expected_result = key == 0 ? EBPF_OBJECT_NOT_FOUND : EBPF_SUCCESS;
+        } else if (behavior_on_max_entries == MAP_BEHAVIOR_INSERT) {
+            expected_result = EBPF_SUCCESS;
         } else {
             expected_result = key == _test_map_size ? EBPF_OBJECT_NOT_FOUND : EBPF_SUCCESS;
         }
@@ -252,6 +264,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(),
@@ -386,15 +399,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"},
@@ -420,6 +424,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);
@@ -446,6 +462,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
@@ -471,16 +501,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"},
@@ -507,6 +527,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"},
@@ -561,6 +594,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]")

tests/end_to_end/end_to_end.cpp

@@ -3216,6 +3216,124 @@ 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;
+    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)
+#define IS_NESTED_MAP(type) ((type) == BPF_MAP_TYPE_HASH_OF_MAPS)
+
+    typedef struct _lpm_trie_key
+    {
+        uint32_t prefix_length;
+        uint32_t value;
+    } lpm_trie_key_t;
+
+    lpm_trie_key_t trie_key = {0};
+
+    if (IS_NESTED_MAP(type)) {
+        // 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 = IS_LPM_MAP(type) ? sizeof(lpm_trie_key_t) : sizeof(int32_t);
+        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 = IS_NESTED_MAP(type) ? &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 = IS_NESTED_MAP(type) ? &inner_map_fd : (int32_t*)&max_entries;
+    }
+
+    // In case of LRU_HASH, the insert will succeed, but the oldest entry will be removed.
+    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);
+}
+
 // This test validates that a different program type (XDP in this case) cannot call
 // a helper function that is not implemented for that program type. Program load should
 // fail for such a program.