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

Fix ring-buffer map consumer pointer on multiple subscribe/unsubscribe. #3002

Merged
merged 15 commits into from
Nov 8, 2023
3 changes: 2 additions & 1 deletion libs/api/ebpf_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3882,7 +3882,8 @@ ebpf_ring_buffer_map_subscribe(
ebpf_operation_ring_buffer_map_async_query_request_t async_query_request{
sizeof(async_query_request),
ebpf_operation_id_t::EBPF_OPERATION_RING_BUFFER_MAP_ASYNC_QUERY,
local_subscription->ring_buffer_map_handle};
local_subscription->ring_buffer_map_handle,
query_buffer_reply.consumer_offset};
result = win32_error_code_to_ebpf_result(invoke_ioctl(
async_query_request,
local_subscription->reply,
Expand Down
3 changes: 2 additions & 1 deletion libs/execution_context/ebpf_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1831,7 +1831,8 @@ _ebpf_core_protocol_ring_buffer_map_query_buffer(
goto Exit;
}

result = ebpf_ring_buffer_map_query_buffer(map, (uint8_t**)(uintptr_t*)&reply->buffer_address);
result =
ebpf_ring_buffer_map_query_buffer(map, (uint8_t**)(uintptr_t*)&reply->buffer_address, &reply->consumer_offset);
gtrevi marked this conversation as resolved.
Show resolved Hide resolved

Exit:
EBPF_OBJECT_RELEASE_REFERENCE((ebpf_core_object_t*)map);
Expand Down
4 changes: 3 additions & 1 deletion libs/execution_context/ebpf_maps.c
Original file line number Diff line number Diff line change
Expand Up @@ -1867,8 +1867,10 @@ _ebpf_ring_buffer_map_cancel_async_query(_In_ _Frees_ptr_ void* cancel_context)
}

_Must_inspect_result_ ebpf_result_t
ebpf_ring_buffer_map_query_buffer(_In_ const ebpf_map_t* map, _Outptr_ uint8_t** buffer)
ebpf_ring_buffer_map_query_buffer(_In_ const ebpf_map_t* map, _Outptr_ uint8_t** buffer, _Out_ size_t* consumer_offset)
gtrevi marked this conversation as resolved.
Show resolved Hide resolved
{
size_t producer_offset;
ebpf_ring_buffer_query((ebpf_ring_buffer_t*)map->data, consumer_offset, &producer_offset);
return ebpf_ring_buffer_map_buffer((ebpf_ring_buffer_t*)map->data, buffer);
}

Expand Down
4 changes: 3 additions & 1 deletion libs/execution_context/ebpf_maps.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,13 @@ extern "C"
*
* @param[in] map Ring buffer map to query.
* @param[out] buffer Pointer to ring buffer data.
* @param[out] consumer_offset Offset of consumer in ring buffer data.
* @retval EPBF_SUCCESS Successfully mapped the ring buffer.
* @retval EBPF_INVALID_ARGUMENT Unable to map the ring buffer.
*/
_Must_inspect_result_ ebpf_result_t
ebpf_ring_buffer_map_query_buffer(_In_ const ebpf_map_t* map, _Outptr_ uint8_t** buffer);
ebpf_ring_buffer_map_query_buffer(
_In_ const ebpf_map_t* map, _Outptr_ uint8_t** buffer, _Out_ size_t* consumer_offset);

/**
* @brief Return consumed buffer back to the ring buffer map.
Expand Down
2 changes: 2 additions & 0 deletions libs/execution_context/ebpf_protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,8 @@ typedef struct _ebpf_operation_ring_buffer_map_query_buffer_reply
struct _ebpf_operation_header header;
// Address to user-space read-only buffer for the ring-buffer records.
uint64_t buffer_address;
// The current consumer offset, so that subsequent reads can start from here.
size_t consumer_offset;
} ebpf_operation_ring_buffer_map_query_buffer_reply_t;

typedef struct _ebpf_operation_ring_buffer_map_async_query_request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -896,11 +896,13 @@ TEST_CASE("ring_buffer_async_query", "[execution_context]")
struct _completion
{
uint8_t* buffer = nullptr;
size_t consumer_offset = 0;
ebpf_ring_buffer_map_async_query_result_t async_query_result = {};
uint64_t value{};
} completion;

REQUIRE(ebpf_ring_buffer_map_query_buffer(map.get(), &completion.buffer) == EBPF_SUCCESS);
REQUIRE(
ebpf_ring_buffer_map_query_buffer(map.get(), &completion.buffer, &completion.consumer_offset) == EBPF_SUCCESS);

REQUIRE(
ebpf_async_set_completion_callback(
Expand Down
17 changes: 11 additions & 6 deletions tests/end_to_end/end_to_end.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -924,12 +924,17 @@ bindmonitor_ring_buffer_test(ebpf_execution_type_t execution_type)
std::function<ebpf_result_t(void*, uint32_t*)> invoke =
[&hook](_Inout_ void* context, _Out_ uint32_t* result) -> ebpf_result_t { return hook.fire(context, result); };

ring_buffer_api_test_helper(process_map_fd, fake_app_ids, [&](int i) {
// Emulate bind operation.
std::vector<char> fake_app_id = fake_app_ids[i];
fake_app_id.push_back('\0');
REQUIRE(emulate_bind(invoke, fake_pid + i, fake_app_id.data()) == BIND_PERMIT);
});
// Test multiple subscriptions to the same ring buffer map, to ensure that the ring buffer map will continue
// to provide notifications to the subscriber.
for (int i = 0; i < 3; i++) {

ring_buffer_api_test_helper(process_map_fd, fake_app_ids, [&](int i) {
// Emulate bind operation.
std::vector<char> fake_app_id = fake_app_ids[i];
fake_app_id.push_back('\0');
REQUIRE(emulate_bind(invoke, fake_pid + i, fake_app_id.data()) == BIND_PERMIT);
});
}

hook.detach_and_close_link(&link);

Expand Down