Skip to content

Conversation

@mikeagun
Copy link
Contributor

@mikeagun mikeagun commented Sep 16, 2025

Description

Implements libbpf synchronous API (poll/consume) for ring buffer map.

Closes #3633

Testing

Adds tests to api_test.cpp and libbpf_test.cpp covering new ring buffer map API and the libbpf ring buffer manager.

Documentation

Updates docs/RingBuffer.md to match the updated API.

Installation

N/A

@mikeagun mikeagun force-pushed the ringbuf-mmap-api-impl branch 6 times, most recently from c82fa10 to 4d31f1d Compare September 18, 2025 03:10
@mikeagun mikeagun force-pushed the ringbuf-mmap-api-impl branch 2 times, most recently from 37d5941 to f9398ab Compare November 19, 2025 17:25
@mikeagun mikeagun force-pushed the ringbuf-mmap-api-impl branch 2 times, most recently from 6fb3c18 to 74fe580 Compare January 6, 2026 20:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements the libbpf synchronous ring buffer API (poll/consume) for Windows, bringing it closer to Linux compatibility. The default behavior now uses synchronous callbacks (matching Linux), while asynchronous callbacks are available via a Windows-specific flag.

Key Changes:

  • Added ring_buffer__poll() and ring_buffer__consume() functions with Linux-compatible behavior
  • Implemented synchronous mode using memory-mapped ring buffer access with event-based notifications
  • Updated ring_buffer structures to support both synchronous and asynchronous modes
  • Fixed error code handling to return negative errno values consistently with libbpf conventions

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
libs/api/libbpf_map.cpp Core implementation of synchronous ring buffer APIs with memory mapping and event handling
include/ebpf_api.h Added public API definitions for ring buffer consumer/producer page structures and new functions
include/bpf/libbpf.h Added Linux-compatible ring buffer API declarations (poll, consume, add)
libs/api/api_internal.h Added forward declaration for ring_buffer_sample_fn callback type
libs/runtime/ebpf_ring_buffer.h Moved ring buffer page type definitions to public API header
tests/unit/libbpf_test.cpp Comprehensive tests for Linux-compatible and Windows-specific ring buffer APIs
tests/api_test/api_test.cpp End-to-end tests for synchronous ring buffer with real eBPF programs
tests/end_to_end/end_to_end.cpp Fixed error checking to expect negative errno values from libbpf_get_error
libs/runtime/unit/platform_unit_test.cpp Removed verbose console output from ring buffer stress tests
docs/RingBuffer.md Updated documentation to reflect synchronous API implementation and usage patterns
ebpfapi/Source.def Added exports for new ring buffer functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

result = EBPF_OPERATION_NOT_SUPPORTED;
goto Exit;
// Set up for synchronous mode - create shared wait handle for all maps
HANDLE wait_handle = CreateEvent(nullptr, FALSE, FALSE, nullptr);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CreateEvent call uses FALSE for the bManualReset parameter, creating an auto-reset event. However, this may cause a race condition in the ring buffer polling logic. When ring_buffer__poll is called, it first does a non-blocking wait with timeout 0 (line 668) to clear the event, then calls ring_buffer__consume (line 671), and only then waits on the handle if no data is available (line 682). If a producer writes data between the initial clear (line 668) and the wait (line 682), the event could be set and then immediately auto-reset by the OS when checked, causing the consumer to miss the notification and potentially wait unnecessarily. Consider using a manual-reset event (TRUE for bManualReset) and explicitly resetting it after consuming data to avoid this race condition.

Suggested change
HANDLE wait_handle = CreateEvent(nullptr, FALSE, FALSE, nullptr);
HANDLE wait_handle = CreateEvent(nullptr, TRUE, FALSE, nullptr);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support synchronous APIs for ring buffer map

1 participant