microsoft / ebpf-for-windows

eBPF implementation that runs on top of Windows
MIT License
2.82k stars 216 forks source link

[bpf_map_lookup_and_delete_batch api]: Failed to delete and retrieve data for per-cpu and non-per-cpu map types. #3687

Open shpalani opened 2 months ago

shpalani commented 2 months ago

Describe the bug

Problem:

  1. bpf_map_lookup_and_delete_batch() fails to delete and retrieve data for per-cpu map types.
  2. Missing test case to validate for non-percpu and percpu map types.

OS information

Windows 10 and above.

Steps taken to reproduce bug

Add the bpf_map_lookup_and_delete_batch() in _test_maps_batch() in libbpf_test.cpp. Then run the test case. Please see "Additional details"

Expected behavior

bpf_map_lookup_and_delete_batch() should retrieve the keys and values and delete those entries in the hash map.

Actual outcome

Returns 22 (EINVAL)

Additional details

.\unit_tests.exe "libbpf hash map batch" -d yes
Filters: "libbpf hash map batch"
Randomness seeded to: 3861804828

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
unit_tests.exe is a Catch2 v3.6.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
libbpf hash map batch
-------------------------------------------------------------------------------
ebpf-for-windows-2024\ebpf-for-windows\tests\unit\libbpf_test.cpp(3592)
...............................................................................

ebpf-for-windows-2024\ebpf-for-windows\tests\unit\libbpf_test.cpp(3515): FAILED:
  REQUIRE( bpf_map_lookup_and_delete_batch( map_fd, nullptr, &next_key, fetched_keys.data(), fetched_values.data(), &fetched_batch_size, &opts) == 0 )
with expansion:
  -22 == 0

0.000 s: libbpf hash map batch
===============================================================================
test cases:     1 |     0 passed | 1 failed
assertions: 80088 | 80087 passed | 1 failed
shpalani commented 2 months ago

RCA: previous_key is null.

In ebpf_map_get_next_key_and_value_batch(...)
{
   ...
   if (flags & EBPF_MAP_FIND_FLAG_DELETE) {
            // If the caller requested deletion, delete the entry.
            result = ebpf_map_metadata_tables[map->ebpf_map_definition.type].delete_entry(map, previous_key);
            if (result != EBPF_SUCCESS) {
                break;
            }
        }
   ...
}

static ebpf_result_t
_delete_hash_map_entry(_Inout_ ebpf_core_map_t* map, _In_ const uint8_t* key)
{
    if (!map || !key) {
        return **EBPF_INVALID_ARGUMENT;**
    }

    return ebpf_hash_table_delete((ebpf_hash_table_t*)map->data, key);
}
shpalani commented 2 months ago

I'm working on the fix.