Skip to content

Commit

Permalink
Fix bpf obj get() behavior. (#3238)
Browse files Browse the repository at this point in the history
* draft

* doc

* add test

* wip

* test

* feedback
  • Loading branch information
gtrevi authored Feb 7, 2024
1 parent ff35f7e commit b6bf2bb
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 14 deletions.
7 changes: 4 additions & 3 deletions libs/api/api_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -535,11 +535,12 @@ ebpf_object_pin(fd_t fd, _In_z_ const char* path) noexcept;
/**
* @brief Get fd for a pinned object by pin path.
* @param[in] path Pin path for the object.
* @param[out] fd File descriptor for the pinned object, -1 if not found.
*
* @return File descriptor for the pinned object, -1 if not found.
* @retval EBPF_SUCCESS on success, or an error code on failure.
*/
fd_t
ebpf_object_get(_In_z_ const char* path) noexcept;
ebpf_result_t
ebpf_object_get(_In_z_ const char* path, _Out_ fd_t* fd) noexcept;

/**
* @brief Open a file without loading the programs.
Expand Down
26 changes: 16 additions & 10 deletions libs/api/ebpf_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1409,10 +1409,12 @@ ebpf_map_unpin(_In_ struct bpf_map* map, _In_opt_z_ const char* path) NO_EXCEPT_
}
CATCH_NO_MEMORY_EBPF_RESULT

fd_t
ebpf_object_get(_In_z_ const char* path) NO_EXCEPT_TRY
ebpf_result_t
ebpf_object_get(_In_z_ const char* path, _Out_ fd_t* fd) NO_EXCEPT_TRY
{
EBPF_LOG_ENTRY();
ebpf_assert(fd);

size_t path_length = strlen(path);
ebpf_protocol_buffer_t request_buffer(offsetof(ebpf_operation_get_pinned_object_request_t, path) + path_length);
auto request = reinterpret_cast<ebpf_operation_get_pinned_object_request_t*>(request_buffer.data());
Expand All @@ -1424,19 +1426,22 @@ ebpf_object_get(_In_z_ const char* path) NO_EXCEPT_TRY
std::copy(path, path + path_length, request->path);
auto result = invoke_ioctl(request_buffer, reply);
if (result != ERROR_SUCCESS) {
EBPF_RETURN_FD(ebpf_fd_invalid);
*fd = (fd_t)ebpf_fd_invalid;
EBPF_RETURN_RESULT(win32_error_code_to_ebpf_result(result));
}

ebpf_assert(reply.header.id == ebpf_operation_id_t::EBPF_OPERATION_GET_PINNED_OBJECT);

ebpf_handle_t handle = reply.handle;
fd_t fd = _create_file_descriptor_for_handle(handle);
if (fd == ebpf_fd_invalid) {
*fd = _create_file_descriptor_for_handle(handle);
if (*fd == ebpf_fd_invalid) {
Platform::CloseHandle(handle);
result = EBPF_NO_MEMORY;
}
EBPF_RETURN_FD(fd);

EBPF_RETURN_RESULT(win32_error_code_to_ebpf_result(result));
}
CATCH_NO_MEMORY_FD
CATCH_NO_MEMORY_EBPF_RESULT

_Must_inspect_result_ ebpf_result_t
ebpf_program_query_info(
Expand Down Expand Up @@ -2958,9 +2963,10 @@ _ebpf_object_reuse_map(_Inout_ ebpf_map_t* map) NO_EXCEPT_TRY

ebpf_assert(map);

// Check if a map is already present with this pin path.
fd_t map_fd = ebpf_object_get(map->pin_path);
if (map_fd == ebpf_fd_invalid) {
// If there is no map at this pin path, then we can (re)use the map.
fd_t map_fd = ebpf_fd_invalid;
result = ebpf_object_get(map->pin_path, &map_fd);
if (result != EBPF_SUCCESS) {
EBPF_RETURN_RESULT(EBPF_SUCCESS);
}

Expand Down
4 changes: 3 additions & 1 deletion libs/api/libbpf_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ bpf_obj_pin(int fd, const char* pathname)
int
bpf_obj_get(const char* pathname)
{
return (int)ebpf_object_get(pathname);
fd_t fd = -1;
libbpf_result_err(ebpf_object_get(pathname, &fd)); // set the errno
return fd;
}

struct bpf_object*
Expand Down
9 changes: 9 additions & 0 deletions tests/unit/libbpf_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ TEST_CASE("libbpf program pinning", "[libbpf]")
_test_helper_libbpf test_helper;
test_helper.initialize();
const char* pin_path = "\\temp\\test";
const char* bad_pin_path = "\\bad\\path";

struct bpf_object* object = bpf_object__open("test_sample_ebpf.o");
REQUIRE(object != nullptr);
Expand All @@ -529,6 +530,14 @@ TEST_CASE("libbpf program pinning", "[libbpf]")
REQUIRE(result < 0);
REQUIRE(errno == EEXIST);

// Test bpf_obj_get() to return the fd and correctly set 'errno'.
fd_t obj_fd = bpf_obj_get(pin_path);
REQUIRE(obj_fd != ebpf_fd_invalid);
REQUIRE(errno == EEXIST);
obj_fd = bpf_obj_get(bad_pin_path);
REQUIRE(obj_fd == ebpf_fd_invalid);
REQUIRE(errno == ENOENT);

result = bpf_program__unpin(program, pin_path);
REQUIRE(result == 0);

Expand Down

0 comments on commit b6bf2bb

Please sign in to comment.