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_buf issues #3354

Merged
merged 8 commits into from
Mar 16, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 1 addition & 1 deletion libs/api/ebpf_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4177,7 +4177,7 @@ _ebpf_ring_buffer_map_async_query_completion(_Inout_ void* completion_context) N
// Async IOCTL operation returned with success status. Read the ring buffer records and indicate it to the
// subscriber.

size_t ring_buffer_size;
size_t ring_buffer_size = 0;
uint32_t dummy;

result = _get_map_descriptor_properties(
Expand Down
29 changes: 24 additions & 5 deletions libs/runtime/ebpf_ring_buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ _ring_get_consumer_offset(_In_ const ebpf_ring_buffer_t* ring)
return ring->consumer_offset % ring->length;
}

inline static size_t
_ring_get_used_capacity(_In_ const ebpf_ring_buffer_t* ring)
{
ebpf_assert(ring->producer_offset >= ring->consumer_offset);
return ring->producer_offset - ring->consumer_offset;
}

inline static void
_ring_advance_producer_offset(_Inout_ ebpf_ring_buffer_t* ring, size_t length)
{
Expand Down Expand Up @@ -146,23 +153,30 @@ ebpf_ring_buffer_output(_Inout_ ebpf_ring_buffer_t* ring, _In_reads_bytes_(lengt
}

void
ebpf_ring_buffer_query(_In_ const ebpf_ring_buffer_t* ring, _Out_ size_t* consumer, _Out_ size_t* producer)
ebpf_ring_buffer_query(_In_ ebpf_ring_buffer_t* ring, _Out_ size_t* consumer, _Out_ size_t* producer)
saxena-anurag marked this conversation as resolved.
Show resolved Hide resolved
{
ebpf_lock_state_t state = ebpf_lock_lock(&ring->lock);
*consumer = ring->consumer_offset;
*producer = ring->producer_offset;
ebpf_lock_unlock(&ring->lock, state);
}

_Must_inspect_result_ ebpf_result_t
ebpf_ring_buffer_return(_Inout_ ebpf_ring_buffer_t* ring, size_t length)
{
EBPF_LOG_ENTRY();
ebpf_result_t result;
ebpf_lock_state_t state = ebpf_lock_lock(&ring->lock);
size_t local_length = length;
size_t offset = _ring_get_consumer_offset(ring);

// Check if length is valid.
if ((length > _ring_get_length(ring)) ||
(length + _ring_get_consumer_offset(ring) > _ring_get_producer_offset(ring))) {
if ((length > _ring_get_length(ring)) || length > _ring_get_used_capacity(ring)) {
EBPF_LOG_MESSAGE_UINT64_UINT64(
EBPF_TRACELOG_LEVEL_ERROR,
EBPF_TRACELOG_KEYWORD_MAP,
"ebpf_ring_buffer_return: Invalid length",
ring->producer_offset,
ring->consumer_offset);
result = EBPF_INVALID_ARGUMENT;
goto Done;
}
Expand All @@ -178,6 +192,11 @@ ebpf_ring_buffer_return(_Inout_ ebpf_ring_buffer_t* ring, size_t length)
}
// Did it end on a record boundary?
if (local_length != 0) {
EBPF_LOG_MESSAGE_UINT64(
EBPF_TRACELOG_LEVEL_ERROR,
EBPF_TRACELOG_KEYWORD_MAP,
"ebpf_ring_buffer_return: Invalid length",
saxena-anurag marked this conversation as resolved.
Show resolved Hide resolved
local_length);
result = EBPF_INVALID_ARGUMENT;
goto Done;
}
Expand All @@ -187,7 +206,7 @@ ebpf_ring_buffer_return(_Inout_ ebpf_ring_buffer_t* ring, size_t length)

Done:
ebpf_lock_unlock(&ring->lock, state);
return result;
EBPF_RETURN_RESULT(result);
saxena-anurag marked this conversation as resolved.
Show resolved Hide resolved
}

_Must_inspect_result_ ebpf_result_t
Expand Down
2 changes: 1 addition & 1 deletion libs/runtime/ebpf_ring_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ ebpf_ring_buffer_output(_Inout_ ebpf_ring_buffer_t* ring_buffer, _In_reads_bytes
* @param[out] producer Offset of the next buffer to be produced.
*/
void
ebpf_ring_buffer_query(_In_ const ebpf_ring_buffer_t* ring_buffer, _Out_ size_t* consumer, _Out_ size_t* producer);
ebpf_ring_buffer_query(_In_ ebpf_ring_buffer_t* ring_buffer, _Out_ size_t* consumer, _Out_ size_t* producer);

/**
* @brief Mark one or more records in the ring buffer as returned to the ring.
Expand Down
1 change: 1 addition & 0 deletions libs/shared/ebpf_ring_buffer_record.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ typedef struct _ebpf_ring_buffer_record
inline const ebpf_ring_buffer_record_t*
ebpf_ring_buffer_next_record(_In_ const uint8_t* buffer, size_t buffer_length, size_t consumer, size_t producer)
{
ebpf_assert(producer >= consumer);
if (producer == consumer) {
return NULL;
}
Expand Down
73 changes: 71 additions & 2 deletions tests/api_test/api_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,7 @@ TEST_CASE("load_native_program_invalid5", "[native][negative]")
{
_load_invalid_program("invalid_maps3.sys", EBPF_EXECUTION_NATIVE, -EINVAL);
}
#endif // _DEBUG

TEST_CASE("ioctl_stress", "[stress]")
{
Expand Down Expand Up @@ -1211,7 +1212,7 @@ TEST_CASE("ioctl_stress", "[stress]")
}
}

// Wait for 10 seconds
// Wait for 60 seconds
std::this_thread::sleep_for(std::chrono::seconds(60));

stop_requested = true;
Expand All @@ -1228,4 +1229,72 @@ TEST_CASE("ioctl_stress", "[stress]")
bpf_object__close(object);
}

#endif
TEST_CASE("test_ringbuffer_wraparound", "[stress]")
{
// Load bindmonitor_ringbuf.sys.
struct bpf_object* object = nullptr;
fd_t program_fd;
uint32_t event_count = 0;
std::string app_id = "api_test.exe";
uint32_t thread_count = 2;
native_module_helper_t native_helper;
native_helper.initialize("bindmonitor_ringbuf", EBPF_EXECUTION_NATIVE);

REQUIRE(
_program_load_helper(
native_helper.get_file_name().c_str(), BPF_PROG_TYPE_BIND, EBPF_EXECUTION_NATIVE, &object, &program_fd) ==
0);

// Get fd of process_map map.
fd_t process_map_fd = bpf_object__find_map_fd_by_name(object, "process_map");

uint32_t max_entries = bpf_map__max_entries(bpf_object__find_map_by_name(object, "process_map"));
uint32_t max_iterations = static_cast<uint32_t>(10 * (max_entries / app_id.size()));
printf("max_iterations: %d\n", max_iterations);
REQUIRE(max_iterations % thread_count == 0);
uint32_t iterations_per_thread = max_iterations / thread_count;

dthaler marked this conversation as resolved.
Show resolved Hide resolved
// Subscribe to the ring buffer.
auto ring = ring_buffer__new(
process_map_fd,
[](void* ctx, void*, size_t) {
(*((uint32_t*)ctx))++;
return 0;
},
&event_count,
nullptr);

// Create 2 threads that invoke the program to trigger ring buffer events.
std::vector<std::jthread> threads;
for (uint32_t i = 0; i < thread_count; i++) {
threads.emplace_back([&]() {
bind_md_t ctx = {};
bpf_test_run_opts opts = {};
opts.ctx_in = &ctx;
opts.ctx_size_in = sizeof(ctx);
opts.ctx_out = &ctx;
opts.ctx_size_out = sizeof(ctx);
opts.data_in = app_id.data();
opts.data_size_in = static_cast<uint32_t>(app_id.size());
opts.data_out = app_id.data();
opts.data_size_out = static_cast<uint32_t>(app_id.size());

for (uint32_t i = 0; i < iterations_per_thread; i++) {
int result = bpf_prog_test_run_opts(program_fd, &opts);
REQUIRE(result == 0);
}
});
}

// Wait for threads to complete.
for (auto& t : threads) {
t.join();
}
REQUIRE(event_count == max_iterations);

// Unsubscribe from the ring buffer.
ring_buffer__free(ring);

// Clean up.
bpf_object__close(object);
}
6 changes: 3 additions & 3 deletions tests/bpf2c_tests/expected/bindmonitor_ringbuf_dll.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ static map_entry_t _maps[] = {
BPF_MAP_TYPE_RINGBUF, // Type of map.
0, // Size in bytes of a map key.
0, // Size in bytes of a map value.
262144, // Maximum number of entries allowed in the map.
65536, // Maximum number of entries allowed in the map.
0, // Inner map index.
LIBBPF_PIN_NONE, // Pinning type for the map.
7, // Identifier for a map template.
Expand Down Expand Up @@ -111,7 +111,7 @@ bind_monitor(void* context)
if (r2 != IMMEDIATE(0))
#line 26 "sample/bindmonitor_ringbuf.c"
goto label_1;
// EBPF_OP_LDXDW pc=2 dst=r2 src=r1 offset=0 imm=0
// EBPF_OP_LDXDW pc=2 dst=r2 src=r1 offset=0 imm=0
#line 28 "sample/bindmonitor_ringbuf.c"
r2 = *(uint64_t*)(uintptr_t)(r1 + OFFSET(0));
// EBPF_OP_LDXDW pc=3 dst=r3 src=r1 offset=8 imm=0
Expand All @@ -122,7 +122,7 @@ bind_monitor(void* context)
if (r2 >= r3)
#line 28 "sample/bindmonitor_ringbuf.c"
goto label_1;
// EBPF_OP_SUB64_REG pc=5 dst=r3 src=r2 offset=0 imm=0
// EBPF_OP_SUB64_REG pc=5 dst=r3 src=r2 offset=0 imm=0
#line 29 "sample/bindmonitor_ringbuf.c"
r3 -= r2;
// EBPF_OP_LDDW pc=6 dst=r1 src=r0 offset=0 imm=0
Expand Down
6 changes: 3 additions & 3 deletions tests/bpf2c_tests/expected/bindmonitor_ringbuf_raw.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ static map_entry_t _maps[] = {
BPF_MAP_TYPE_RINGBUF, // Type of map.
0, // Size in bytes of a map key.
0, // Size in bytes of a map value.
262144, // Maximum number of entries allowed in the map.
65536, // Maximum number of entries allowed in the map.
0, // Inner map index.
LIBBPF_PIN_NONE, // Pinning type for the map.
7, // Identifier for a map template.
Expand Down Expand Up @@ -85,7 +85,7 @@ bind_monitor(void* context)
if (r2 != IMMEDIATE(0))
#line 26 "sample/bindmonitor_ringbuf.c"
goto label_1;
// EBPF_OP_LDXDW pc=2 dst=r2 src=r1 offset=0 imm=0
// EBPF_OP_LDXDW pc=2 dst=r2 src=r1 offset=0 imm=0
#line 28 "sample/bindmonitor_ringbuf.c"
r2 = *(uint64_t*)(uintptr_t)(r1 + OFFSET(0));
// EBPF_OP_LDXDW pc=3 dst=r3 src=r1 offset=8 imm=0
Expand All @@ -96,7 +96,7 @@ bind_monitor(void* context)
if (r2 >= r3)
#line 28 "sample/bindmonitor_ringbuf.c"
goto label_1;
// EBPF_OP_SUB64_REG pc=5 dst=r3 src=r2 offset=0 imm=0
// EBPF_OP_SUB64_REG pc=5 dst=r3 src=r2 offset=0 imm=0
#line 29 "sample/bindmonitor_ringbuf.c"
r3 -= r2;
// EBPF_OP_LDDW pc=6 dst=r1 src=r0 offset=0 imm=0
Expand Down
6 changes: 3 additions & 3 deletions tests/bpf2c_tests/expected/bindmonitor_ringbuf_sys.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ static map_entry_t _maps[] = {
BPF_MAP_TYPE_RINGBUF, // Type of map.
0, // Size in bytes of a map key.
0, // Size in bytes of a map value.
262144, // Maximum number of entries allowed in the map.
65536, // Maximum number of entries allowed in the map.
0, // Inner map index.
LIBBPF_PIN_NONE, // Pinning type for the map.
7, // Identifier for a map template.
Expand Down Expand Up @@ -246,7 +246,7 @@ bind_monitor(void* context)
if (r2 != IMMEDIATE(0))
#line 26 "sample/bindmonitor_ringbuf.c"
goto label_1;
// EBPF_OP_LDXDW pc=2 dst=r2 src=r1 offset=0 imm=0
// EBPF_OP_LDXDW pc=2 dst=r2 src=r1 offset=0 imm=0
#line 28 "sample/bindmonitor_ringbuf.c"
r2 = *(uint64_t*)(uintptr_t)(r1 + OFFSET(0));
// EBPF_OP_LDXDW pc=3 dst=r3 src=r1 offset=8 imm=0
Expand All @@ -257,7 +257,7 @@ bind_monitor(void* context)
if (r2 >= r3)
#line 28 "sample/bindmonitor_ringbuf.c"
goto label_1;
// EBPF_OP_SUB64_REG pc=5 dst=r3 src=r2 offset=0 imm=0
// EBPF_OP_SUB64_REG pc=5 dst=r3 src=r2 offset=0 imm=0
#line 29 "sample/bindmonitor_ringbuf.c"
r3 -= r2;
// EBPF_OP_LDDW pc=6 dst=r1 src=r0 offset=0 imm=0
Expand Down
2 changes: 1 addition & 1 deletion tests/sample/bindmonitor_ringbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
struct
{
__uint(type, BPF_MAP_TYPE_RINGBUF);
__uint(max_entries, 256 * 1024);
__uint(max_entries, 64 * 1024);
} process_map SEC(".maps");

SEC("bind")
Expand Down
Loading