Skip to content

fix: correct pinned maps handling and implement missing userspace setup #13

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.

Sign up for GitHub

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

Open
SiyuanSun0736 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: correct pinned maps handling and implement missing userspace setup #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 10 additions & 6 deletions src/ir_generator.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2825,12 +2825,15 @@ let lower_multi_program ast symbol_table source_name =
let ir_map = lower_map_declaration symbol_table map_decl in
((decl, `Map ir_map) :: decls, ir_map :: maps, vars)
| Ast.GlobalVarDecl var_decl when is_global_var_map var_decl ->
(* When a global variable has a map type, convert it to a map definition only.
Do NOT create a corresponding IR global variable, as that causes the map
to be incorrectly treated as a pinned global variable in userspace codegen. *)
let ir_map = match convert_global_var_to_map symbol_table var_decl with
| Some map -> map
| None -> failwith "Expected map conversion to succeed"
in
let ir_var = lower_global_variable_declaration symbol_table var_decl in
((decl, `MapFromGlobalVar (ir_map, ir_var)) :: decls, ir_map :: maps, ir_var :: vars)
(* Only track as map, not as global variable *)
((decl, `MapFromGlobalVar ir_map) :: decls, ir_map :: maps, vars)
| Ast.GlobalVarDecl var_decl ->
let ir_var = lower_global_variable_declaration symbol_table var_decl in
((decl, `GlobalVar ir_var) :: decls, maps, ir_var :: vars)
Expand Down Expand Up @@ -3136,10 +3139,11 @@ let lower_multi_program ast symbol_table source_name =
match processed with
| `Map ir_map ->
add_source_declaration (IRDeclMapDef ir_map) pos
| `MapFromGlobalVar (ir_map, ir_var) ->
(* For global variables with map types, add both the map and the variable *)
add_source_declaration (IRDeclMapDef ir_map) pos;
add_source_declaration (IRDeclGlobalVarDef ir_var) pos
| `MapFromGlobalVar ir_map ->
(* For global variables with map types, only add the map definition.
Do NOT add a global variable declaration, as it would incorrectly be treated
as a pinned global variable in userspace codegen. *)
add_source_declaration (IRDeclMapDef ir_map) pos
| `GlobalVar ir_var ->
add_source_declaration (IRDeclGlobalVarDef ir_var) pos
| `Other decl ->
Expand Down
89 changes: 73 additions & 16 deletions src/userspace_codegen.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2610,7 +2610,13 @@ let generate_c_function_from_ir ?(global_variables = []) ?(base_name = "") ?(con
let pinned_globals_vars = List.filter (fun gv -> gv.is_pinned) global_variables in
let has_pinned_globals = List.length pinned_globals_vars > 0 in

let setup_call = if needs_object_loading && (List.length config_declarations > 0 || func_usage.uses_map_operations || func_usage.uses_exec || has_pinned_globals) then
(* Check if there are any pinned maps that need setup *)
let has_pinned_maps = match ir_multi_prog with
| Some multi_prog -> List.exists (fun map -> map.pin_path <> None) (Ir.get_global_maps multi_prog)
| None -> false
in

let setup_call = if needs_object_loading && (List.length config_declarations > 0 || func_usage.uses_map_operations || func_usage.uses_exec || has_pinned_globals || has_pinned_maps) then
let all_setup_parts = List.filter (fun s -> s <> "") [
(if has_pinned_globals then
let project_name = base_name in
Expand All @@ -2637,8 +2643,8 @@ let generate_c_function_from_ir ?(global_variables = []) ?(base_name = "") ?(con
}
}|} pin_path pin_path
else "");
(* Include all_setup_code only once for maps, config, struct_ops, and ringbuf *)
(if func_usage.uses_map_operations || func_usage.uses_exec || List.length config_declarations > 0 then all_setup_code else "");
(* Include all_setup_code for maps (including pinned maps), config, struct_ops, and ringbuf *)
(if func_usage.uses_map_operations || func_usage.uses_exec || List.length config_declarations > 0 || has_pinned_maps then all_setup_code else "");
] in
if all_setup_parts <> [] then "\n" ^ String.concat "\n" all_setup_parts else ""
else "" in
Expand Down Expand Up @@ -2957,23 +2963,30 @@ let generate_unified_map_setup_code ?(obj_var="obj->obj") maps =
else map :: acc
) [] maps |> List.rev in

List.map (fun map ->
let map_setups = List.map (fun map ->
(* Always load from eBPF object first, then handle pinning if needed *)
let pin_logic = match map.pin_path with
| Some pin_path ->
(* Extract directory path from pin_path *)
let dir_path = Filename.dirname pin_path in
(* Generate unique variable name for each map's existing_fd *)
Printf.sprintf {|
// Check if map is already pinned
int existing_fd = bpf_obj_get("%s");
if (existing_fd >= 0) {
%s_fd = existing_fd;
int %s_existing_fd = bpf_obj_get("%s");
if (%s_existing_fd >= 0) {
%s_fd = %s_existing_fd;
} else {
// Map not pinned yet, pin it now
// Map not pinned yet, create directory and pin it
if (ensure_bpf_dir("%s") < 0) {
fprintf(stderr, "Failed to create directory %s: %%s\n", strerror(errno));
return 1;
}
if (bpf_map__pin(%s_map, "%s") < 0) {
fprintf(stderr, "Failed to pin %s map to %s\n");
return 1;
}
%s_fd = bpf_map__fd(%s_map);
}|} pin_path map.map_name map.map_name pin_path map.map_name pin_path map.map_name map.map_name
}|} map.map_name pin_path map.map_name map.map_name map.map_name dir_path dir_path map.map_name pin_path map.map_name pin_path map.map_name map.map_name
| None ->
Printf.sprintf {|
// Non-pinned map, just get file descriptor
Expand All @@ -2989,7 +3002,9 @@ let generate_unified_map_setup_code ?(obj_var="obj->obj") maps =
fprintf(stderr, "Failed to get fd for %s map\n");
return 1;
}|} map.map_name map.map_name obj_var map.map_name map.map_name map.map_name pin_logic map.map_name map.map_name
) deduplicated_maps |> String.concat "\n"
) deduplicated_maps in

String.concat "\n" map_setups

(** Generate config struct definition from config declaration - reusing eBPF logic *)
let generate_config_struct_from_decl (config_decl : Ast.config_declaration) =
Expand Down Expand Up @@ -3446,8 +3461,14 @@ let generate_complete_userspace_program_from_ir ?(config_declarations = []) ?(ta
maps_for_exec (* Use all global maps directly for exec *)
else used_global_maps in

(* Check if there are any pinned maps - this affects which headers we need *)
let has_any_pinned_maps = List.exists (fun map -> map.pin_path <> None) global_maps in

(* For header generation, use all global maps if there are pinned maps, otherwise use the filtered list *)
let maps_for_headers = if has_any_pinned_maps then global_maps else used_global_maps_with_exec in

let uses_bpf_functions = all_usage.uses_load || all_usage.uses_attach || all_usage.uses_detach in
let base_includes = generate_headers_for_maps ~uses_bpf_functions used_global_maps_with_exec in
let base_includes = generate_headers_for_maps ~uses_bpf_functions maps_for_headers in
let additional_includes = {|#include <stdbool.h>
#include <stdint.h>
#include <inttypes.h>
Expand Down Expand Up @@ -3520,8 +3541,12 @@ let generate_complete_userspace_program_from_ir ?(config_declarations = []) ?(ta
else "" in

(* Generate setup code first for use in main function *)
let map_setup_code = if all_usage.uses_map_operations || all_usage.uses_exec then
generate_unified_map_setup_code used_global_maps_with_exec
(* Check if there are any pinned maps that need setup *)
let has_pinned_maps = List.exists (fun map -> map.pin_path <> None) global_maps in
let map_setup_code = if all_usage.uses_map_operations || all_usage.uses_exec || has_pinned_maps then
(* For pinned maps, we need to include all of them in setup, not just used ones *)
let maps_for_setup = if has_pinned_maps then global_maps else used_global_maps_with_exec in
generate_unified_map_setup_code maps_for_setup
else "" in

(* Generate pinned globals support *)
Expand Down Expand Up @@ -3588,8 +3613,9 @@ let generate_complete_userspace_program_from_ir ?(config_declarations = []) ?(ta



let map_fd_declarations = if all_usage.uses_map_operations || all_usage.uses_exec then
generate_map_fd_declarations used_global_maps_with_exec
let map_fd_declarations = if all_usage.uses_map_operations || all_usage.uses_exec || has_pinned_maps then
let maps_for_fd = if has_pinned_maps then global_maps else used_global_maps_with_exec in
generate_map_fd_declarations maps_for_fd
else "" in

(* Generate config map file descriptors if there are config declarations *)
Expand Down Expand Up @@ -3666,6 +3692,9 @@ void cleanup_bpf_maps(void) {

(* Only generate BPF helper functions when they're actually used *)
let bpf_helper_functions =
(* Check if there are any pinned maps in the global maps *)
let has_pinned_maps = List.exists (fun map -> map.pin_path <> None) global_maps in

let load_function = generate_load_function_with_tail_calls base_name all_usage tail_call_analysis all_setup_code kfunc_dependencies (Ir.get_global_variables ir_multi_prog) in

(* Global attachment storage (generated only when attach/detach are used) *)
Expand Down Expand Up @@ -4140,7 +4169,35 @@ static int add_attachment(int prog_fd, const char *target, uint32_t flags,
) maps_for_exec |> String.concat "\n")
else "" in

let functions_list = List.filter (fun s -> s <> "") [attachment_storage; load_function; attach_function; detach_function; daemon_function; exec_function] in
(* Generate directory creation helper if there are pinned maps *)
let mkdir_helper_function = if has_pinned_maps then
{|// Helper function to create directory recursively
static int ensure_bpf_dir(const char *path) {
char tmp[256];
char *p = NULL;
size_t len;

snprintf(tmp, sizeof(tmp), "%s", path);
len = strlen(tmp);
if (tmp[len - 1] == '/') tmp[len - 1] = 0;

for (p = tmp + 1; *p; p++) {
if (*p == '/') {
*p = 0;
if (mkdir(tmp, 0755) != 0 && errno != EEXIST) {
return -1;
}
*p = '/';
}
}
if (mkdir(tmp, 0755) != 0 && errno != EEXIST) {
return -1;
}
return 0;
}|}
else "" in

let functions_list = List.filter (fun s -> s <> "") [mkdir_helper_function; attachment_storage; load_function; attach_function; detach_function; daemon_function; exec_function] in
if functions_list = [] && bpf_obj_decl = "" then ""
else
sprintf "\n/* BPF Helper Functions (generated only when used) */\n%s\n\n%s"
Expand Down