From 51bdadb5d3a2136d96ff0e8c3f3314d919cb9982 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 08:48:23 +0100 Subject: [PATCH 01/12] daemon: cgen: create bf_link only when the program is attached bf_program creates the bf_link object in bf_program_new(), for every new program. If the program is not attached, then program->link->hookopts is NULL, and that's how we know. This approach is not ideal as we might create a bf_link object we don't need. Additionally, it leaves bf_link in a temporary state as long as we don't attach the program to the kernel: the bf_link is in the bf_program object and serialized, but not materialized. This commit only create bf_link when the BPF link object is needed. Calling bf_link_new() will create the BPF link, and fail on error. Doing so, we know that any bf_link object refers to a program attached to the kernel. --- src/bpfilter/cgen/cgen.c | 6 +- src/bpfilter/cgen/prog/link.c | 241 ++++++++++++++-------------------- src/bpfilter/cgen/prog/link.h | 39 ++---- src/bpfilter/cgen/program.c | 55 ++++---- src/bpfilter/xlate/cli.c | 10 +- 5 files changed, 143 insertions(+), 208 deletions(-) diff --git a/src/bpfilter/cgen/cgen.c b/src/bpfilter/cgen/cgen.c index 0087a1b0..139c40ab 100644 --- a/src/bpfilter/cgen/cgen.c +++ b/src/bpfilter/cgen/cgen.c @@ -378,10 +378,8 @@ int bf_cgen_update(struct bf_cgen *cgen, struct bf_chain **new_chain) if (bf_opts_persist()) bf_program_unpin(old_prog, pindir_fd); - if (old_prog->link->hookopts) { - // Chain is currently attached, update the link to use new program - r = bf_link_update(old_prog->link, cgen->chain->hook, - new_prog->runtime.prog_fd); + if (old_prog->link) { + r = bf_link_update(old_prog->link, new_prog->runtime.prog_fd); if (r) { bf_err_r(r, "failed to update bf_link object with new program"); if (bf_opts_persist() && bf_program_pin(old_prog, pindir_fd) < 0) diff --git a/src/bpfilter/cgen/prog/link.c b/src/bpfilter/cgen/prog/link.c index 7ab4b005..27bd33e3 100644 --- a/src/bpfilter/cgen/prog/link.c +++ b/src/bpfilter/cgen/prog/link.c @@ -24,23 +24,82 @@ #include #include -int bf_link_new(struct bf_link **link, const char *name) +static int _bf_link_try_attach_xdp(int prog_fd, int ifindex, enum bf_hook hook); + +int bf_link_new(struct bf_link **link, enum bf_hook hook, + struct bf_hookopts **hookopts, int prog_fd) { _free_bf_link_ struct bf_link *_link = NULL; + _cleanup_close_ int fd = -1; + _cleanup_close_ int fd_extra = -1; + _cleanup_close_ int cgroup_fd = -1; + struct bf_hookopts *_hookopts = *hookopts; + int r; assert(link); - assert(name); - assert(name[0] != '\0'); + assert(hookopts); - _link = malloc(sizeof(*_link)); + if (!*hookopts) { + return bf_err_r(-EINVAL, + "hookopts are required when created a new bf_link"); + } + + _link = calloc(1, sizeof(*_link)); if (!_link) return -ENOMEM; - bf_strncpy(_link->name, BPF_OBJ_NAME_LEN, name); + _link->hook = hook; + + switch (bf_hook_to_flavor(hook)) { + case BF_FLAVOR_XDP: + r = _bf_link_try_attach_xdp(prog_fd, _hookopts->ifindex, hook); + if (r < 0) + return bf_err_r(r, "failed to create XDP BPF link"); + + fd = r; + break; + case BF_FLAVOR_TC: + r = bf_bpf_link_create(prog_fd, _hookopts->ifindex, hook, 0, 0, 0); + if (r < 0) + return bf_err_r(r, "failed to create TC BPF link"); + + fd = r; + break; + case BF_FLAVOR_CGROUP: + cgroup_fd = open(_hookopts->cgpath, O_DIRECTORY | O_RDONLY); + if (cgroup_fd < 0) { + return bf_err_r(errno, "failed to open cgroup '%s'", + _hookopts->cgpath); + } + + r = bf_bpf_link_create(prog_fd, cgroup_fd, hook, 0, 0, 0); + if (r < 0) + return bf_err_r(r, "failed to create cgroup BPF link"); + + fd = r; + break; + case BF_FLAVOR_NF: + r = bf_bpf_link_create(prog_fd, 0, hook, 0, PF_INET, + _hookopts->priorities[0]); + if (r < 0) + return bf_err_r(r, "failed to create nf_inet BPF link"); + + fd = r; + + r = bf_bpf_link_create(prog_fd, 0, hook, 0, PF_INET6, + _hookopts->priorities[0]); + if (r < 0) + return bf_err_r(r, "failed to create nf_inet6 BPF link"); + + fd_extra = r; + break; + default: + return -ENOTSUP; + } - _link->hookopts = NULL; - _link->fd = -1; - _link->fd_extra = -1; + _link->fd = TAKE_FD(fd); + _link->fd_extra = TAKE_FD(fd_extra); + _link->hookopts = TAKE_PTR(*hookopts); *link = TAKE_PTR(_link); @@ -51,39 +110,34 @@ int bf_link_new_from_pack(struct bf_link **link, int dir_fd, bf_rpack_node_t node) { _free_bf_link_ struct bf_link *_link = NULL; - _cleanup_free_ char *name = NULL; bf_rpack_node_t child; int r; assert(link); - r = bf_rpack_kv_str(node, "name", &name); - if (r) - return bf_rpack_key_err(r, "bf_link.name"); + _link = malloc(sizeof(*_link)); + if (!_link) + return -ENOMEM; - r = bf_link_new(&_link, name); + r = bf_rpack_kv_enum(node, "hook", &_link->hook, 0, _BF_HOOK_MAX); if (r) - return bf_err_r(r, "failed to create bf_link from pack"); + return bf_rpack_key_err(r, "bf_link.hook"); r = bf_rpack_kv_node(node, "hookopts", &child); if (r) return bf_rpack_key_err(r, "bf_link.hookopts"); - if (!bf_rpack_is_nil(child)) { - r = bf_hookopts_new_from_pack(&_link->hookopts, child); - if (r) - return r; - } + r = bf_hookopts_new_from_pack(&_link->hookopts, child); + if (r) + return r; - if (dir_fd != -1) { - r = bf_bpf_obj_get("bf_link", dir_fd, &_link->fd); - if (r) - return bf_err_r(r, "failed to open pinned BPF link 'bf_link'"); + r = bf_bpf_obj_get("bf_link", dir_fd, &_link->fd); + if (r) + return bf_err_r(r, "failed to open pinned BPF link 'bf_link'"); - r = bf_bpf_obj_get("bf_link_extra", dir_fd, &_link->fd_extra); - if (r && r != -ENOENT) { - return bf_err_r( - r, "failed to open pinned extra BPF link 'bf_link_extra'"); - } + r = bf_bpf_obj_get("bf_link_extra", dir_fd, &_link->fd_extra); + if (r && r != -ENOENT) { + return bf_err_r(r, + "failed to open pinned extra BPF link 'bf_link_extra'"); } *link = TAKE_PTR(_link); @@ -109,15 +163,10 @@ int bf_link_pack(const struct bf_link *link, bf_wpack_t *pack) assert(link); assert(pack); - bf_wpack_kv_str(pack, "name", link->name); - - if (link->hookopts) { - bf_wpack_open_object(pack, "hookopts"); - bf_hookopts_pack(link->hookopts, pack); - bf_wpack_close_object(pack); - } else { - bf_wpack_kv_nil(pack, "hookopts"); - } + bf_wpack_kv_enum(pack, "hook", link->hook); + bf_wpack_open_object(pack, "hookopts"); + bf_hookopts_pack(link->hookopts, pack); + bf_wpack_close_object(pack); return bf_wpack_is_valid(pack) ? 0 : -EINVAL; } @@ -130,18 +179,15 @@ void bf_link_dump(const struct bf_link *link, prefix_t *prefix) DUMP(prefix, "struct bf_link at %p", link); bf_dump_prefix_push(prefix); - DUMP(prefix, "name: %s", link->name); - - if (link->hookopts) { - DUMP(prefix, "hookopts: struct bf_hookopts *"); - bf_dump_prefix_push(prefix); - bf_hookopts_dump(link->hookopts, prefix); - bf_dump_prefix_pop(prefix); - } else { - DUMP(prefix, "hookopts: struct bf_hookopts * (NULL)"); - } - DUMP(bf_dump_prefix_last(prefix), "fd: %d", link->fd); + DUMP(prefix, "hook: %s", bf_hook_to_str(link->hook)); + + DUMP(prefix, "hookopts: struct bf_hookopts *"); + bf_dump_prefix_push(prefix); + bf_hookopts_dump(link->hookopts, prefix); + bf_dump_prefix_pop(prefix); + + DUMP(prefix, "fd: %d", link->fd); DUMP(bf_dump_prefix_last(prefix), "fd_extra: %d", link->fd_extra); bf_dump_prefix_pop(prefix); } @@ -178,72 +224,6 @@ static int _bf_link_try_attach_xdp(int prog_fd, int ifindex, enum bf_hook hook) return r; } -int bf_link_attach(struct bf_link *link, enum bf_hook hook, - struct bf_hookopts **hookopts, int prog_fd) -{ - _cleanup_close_ int fd = -1; - _cleanup_close_ int fd_extra = -1; - _cleanup_close_ int cgroup_fd = -1; - struct bf_hookopts *_hookopts = *hookopts; - int r; - - assert(link); - assert(hookopts); - - switch (bf_hook_to_flavor(hook)) { - case BF_FLAVOR_XDP: - r = _bf_link_try_attach_xdp(prog_fd, _hookopts->ifindex, hook); - if (r < 0) - return bf_err_r(r, "failed to create XDP BPF link"); - - fd = r; - break; - case BF_FLAVOR_TC: - r = bf_bpf_link_create(prog_fd, _hookopts->ifindex, hook, 0, 0, 0); - if (r < 0) - return bf_err_r(r, "failed to create TC BPF link"); - - fd = r; - break; - case BF_FLAVOR_CGROUP: - cgroup_fd = open(_hookopts->cgpath, O_DIRECTORY | O_RDONLY); - if (cgroup_fd < 0) { - return bf_err_r(errno, "failed to open cgroup '%s'", - _hookopts->cgpath); - } - - r = bf_bpf_link_create(prog_fd, cgroup_fd, hook, 0, 0, 0); - if (r < 0) - return bf_err_r(r, "failed to create cgroup BPF link"); - - fd = r; - break; - case BF_FLAVOR_NF: - r = bf_bpf_link_create(prog_fd, 0, hook, 0, PF_INET, - _hookopts->priorities[0]); - if (r < 0) - return bf_err_r(r, "failed to create nf_inet BPF link"); - - fd = r; - - r = bf_bpf_link_create(prog_fd, 0, hook, 0, PF_INET6, - _hookopts->priorities[0]); - if (r < 0) - return bf_err_r(r, "failed to create nf_inet6 BPF link"); - - fd_extra = r; - break; - default: - return -ENOTSUP; - } - - link->fd = TAKE_FD(fd); - link->fd_extra = TAKE_FD(fd_extra); - link->hookopts = TAKE_PTR(*hookopts); - - return 0; -} - static int _bf_link_update_nf(struct bf_link *link, enum bf_hook hook, int prog_fd) { @@ -281,20 +261,20 @@ static int _bf_link_update_nf(struct bf_link *link, enum bf_hook hook, return 0; } -int bf_link_update(struct bf_link *link, enum bf_hook hook, int prog_fd) +int bf_link_update(struct bf_link *link, int prog_fd) { assert(link); int r; - switch (bf_hook_to_flavor(hook)) { + switch (bf_hook_to_flavor(link->hook)) { case BF_FLAVOR_XDP: case BF_FLAVOR_TC: case BF_FLAVOR_CGROUP: r = bf_bpf_link_update(link->fd, prog_fd); break; case BF_FLAVOR_NF: - r = _bf_link_update_nf(link, hook, prog_fd); + r = _bf_link_update_nf(link, link->hook, prog_fd); break; default: return -ENOTSUP; @@ -303,33 +283,6 @@ int bf_link_update(struct bf_link *link, enum bf_hook hook, int prog_fd) return r; } -void bf_link_detach(struct bf_link *link) -{ - assert(link); - - int r; - - r = bf_bpf_link_detach(link->fd); - if (r) { - bf_warn_r( - r, - "call to BPF_LINK_DETACH failed, closing the file descriptor and assuming the link is destroyed"); - } - - if (link->fd_extra > 0) { - r = bf_bpf_link_detach(link->fd_extra); - if (r) { - bf_warn_r( - r, - "call to BPF_LINK_DETACH for extra link failed, closing the file descriptor and assuming the link is destroyed"); - } - } - - bf_hookopts_free(&link->hookopts); - closep(&link->fd); - closep(&link->fd_extra); -} - int bf_link_pin(struct bf_link *link, int dir_fd) { int r; diff --git a/src/bpfilter/cgen/prog/link.h b/src/bpfilter/cgen/prog/link.h index 71ef814b..804ee7ad 100644 --- a/src/bpfilter/cgen/prog/link.h +++ b/src/bpfilter/cgen/prog/link.h @@ -17,10 +17,8 @@ */ struct bf_link { - /** Name of the link. From the kernel perspective, BPF link objects don't - * have a name like the programs, but this name will be used to pin the - * link. */ - char name[BPF_OBJ_NAME_LEN]; + /** Hook to which the link is attached. */ + enum bf_hook hook; /** Hook options used for the link. Only valid if the link is materialized */ struct bf_hookopts *hookopts; @@ -44,10 +42,14 @@ struct bf_link * * @param link `bf_link` object to allocate and initialize. On failure, * this parameter is unchanged. Can't be NULL. - * @param name Name of the link. Can't be empty or NULL. + * @param hook Hook to attach the link to. + * @param hookopts Hook options used to create the link. Each hook will require + * specific hook options. On success, `link` will own it. Can't be NULL. + * @param prog_fd File descriptor of the BPF program to attach. * @return 0 on success, or a negative errno value on failure. */ -int bf_link_new(struct bf_link **link, const char *name); +int bf_link_new(struct bf_link **link, enum bf_hook hook, + struct bf_hookopts **hookopts, int prog_fd); /** * @brief Allocate and initialize a new link from serialized data. @@ -91,21 +93,6 @@ int bf_link_pack(const struct bf_link *link, bf_wpack_t *pack); */ void bf_link_dump(const struct bf_link *link, prefix_t *prefix); -/** - * Attach a BPF program to a hook using a the link. - * - * @todo Validate `hookopts` before attaching the link. - * - * @param link `bf_link` object to use to attach the program. Can't be NULL. - * @param hook Hook to attach the program to. - * @param hookopts Hook-specific options to use to attach the program to the - * hook. Can't be NULL. - * @param prog_fd BPF program to attach. - * @return 0 on success, or a negative errno value on failure. - */ -int bf_link_attach(struct bf_link *link, enum bf_hook hook, - struct bf_hookopts **hookopts, int prog_fd); - /** * Replace the program attached to the link. * @@ -113,18 +100,10 @@ int bf_link_attach(struct bf_link *link, enum bf_hook hook, * with another program. * * @param link Link to update. Can't be NULL. - * @param hook Hook the link is already attached to. * @param prog_fd File descriptor of the new program to attach to the link. * @return 0 on success, or a negative errno value on failure. */ -int bf_link_update(struct bf_link *link, enum bf_hook hook, int prog_fd); - -/** - * Detach the BPF link from the hook. - * - * @param link Link to detach. Can't be NULL. - */ -void bf_link_detach(struct bf_link *link); +int bf_link_update(struct bf_link *link, int prog_fd); /** * Pin the link to the system. diff --git a/src/bpfilter/cgen/program.c b/src/bpfilter/cgen/program.c index 88b7345c..d5b6b084 100644 --- a/src/bpfilter/cgen/program.c +++ b/src/bpfilter/cgen/program.c @@ -148,10 +148,6 @@ int bf_program_new(struct bf_program **program, const struct bf_chain *chain) set_idx++; }; - r = bf_link_new(&_program->link, "bf_link"); - if (r) - return r; - r = bf_printer_new(&_program->printer); if (r) return r; @@ -225,17 +221,15 @@ int bf_program_new_from_pack(struct bf_program **program, return bf_err_r(r, "failed to unpack bf_map into bf_program.sets"); } - /* Try to restore the link: on success, replace the program's link with the - * restored on. If -ENOENT is returned, the link doesn't exist, meaning the - * program is not attached. Otherwise, return an error. */ - r = bf_rpack_kv_obj(node, "link", &child); + // Restore the link if it exists + r = bf_rpack_kv_node(node, "link", &child); if (r) return bf_rpack_key_err(r, "bf_program.link"); - r = bf_link_new_from_pack(&link, dir_fd, child); - if (!r) - bf_swap(_program->link, link); - else if (r != -ENOENT) - return bf_err_r(r, "failed to restore bf_program.link"); + if (!bf_rpack_is_nil(child)) { + r = bf_link_new_from_pack(&_program->link, dir_fd, child); + if (r) + return bf_rpack_key_err(r, "bf_program.link"); + } bf_printer_free(&_program->printer); r = bf_rpack_kv_obj(node, "printer", &child); @@ -308,9 +302,13 @@ int bf_program_pack(const struct bf_program *program, bf_wpack_t *pack) bf_wpack_kv_list(pack, "sets", &program->sets); - bf_wpack_open_object(pack, "link"); - bf_link_pack(program->link, pack); - bf_wpack_close_object(pack); + if (program->link) { + bf_wpack_open_object(pack, "link"); + bf_link_pack(program->link, pack); + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_nil(pack, "link"); + } bf_wpack_open_object(pack, "printer"); bf_printer_pack(program->printer, pack); @@ -363,10 +361,14 @@ void bf_program_dump(const struct bf_program *program, prefix_t *prefix) } bf_dump_prefix_pop(prefix); - DUMP(prefix, "link: struct bf_link *"); - bf_dump_prefix_push(prefix); - bf_link_dump(program->link, prefix); - bf_dump_prefix_pop(prefix); + if (program->link) { + DUMP(prefix, "link: struct bf_link *"); + bf_dump_prefix_push(prefix); + bf_link_dump(program->link, prefix); + bf_dump_prefix_pop(prefix); + } else { + DUMP(prefix, "link: struct bf_link * (NULL)"); + } DUMP(prefix, "printer: struct bf_printer *"); bf_dump_prefix_push(prefix); @@ -1106,8 +1108,8 @@ int bf_program_attach(struct bf_program *prog, struct bf_hookopts **hookopts) assert(prog); assert(hookopts); - r = bf_link_attach(prog->link, prog->runtime.chain->hook, hookopts, - prog->runtime.prog_fd); + r = bf_link_new(&prog->link, prog->runtime.chain->hook, hookopts, + prog->runtime.prog_fd); if (r) { return bf_err_r(r, "failed to attach bf_link for %s program", bf_flavor_to_str(prog->flavor)); @@ -1120,7 +1122,7 @@ void bf_program_detach(struct bf_program *prog) { assert(prog); - bf_link_detach(prog->link); + bf_link_free(&prog->link); } void bf_program_unload(struct bf_program *prog) @@ -1128,7 +1130,7 @@ void bf_program_unload(struct bf_program *prog) assert(prog); closep(&prog->runtime.prog_fd); - bf_link_detach(prog->link); + bf_link_free(&prog->link); bf_map_destroy(prog->cmap); bf_map_destroy(prog->pmap); bf_map_destroy(prog->lmap); @@ -1209,7 +1211,7 @@ int bf_program_pin(struct bf_program *prog, int dir_fd) } // If a link exists, pin it too. - if (prog->link->hookopts) { + if (prog->link) { r = bf_link_pin(prog->link, dir_fd); if (r) { bf_err_r(r, "failed to pin BPF link for '%s'", name); @@ -1238,7 +1240,8 @@ void bf_program_unpin(struct bf_program *prog, int dir_fd) bf_map_unpin(map, dir_fd); } - bf_link_unpin(prog->link, dir_fd); + if (prog->link) + bf_link_unpin(prog->link, dir_fd); unlinkat(dir_fd, prog->prog_name, 0); } diff --git a/src/bpfilter/xlate/cli.c b/src/bpfilter/xlate/cli.c index 8519da15..7a99b819 100644 --- a/src/bpfilter/xlate/cli.c +++ b/src/bpfilter/xlate/cli.c @@ -90,7 +90,9 @@ static int _bf_cli_ruleset_get(const struct bf_request *request, if (r) return bf_err_r(r, "failed to add chain to list"); - r = bf_list_add_tail(&hookopts, cgen->program->link->hookopts); + r = bf_list_add_tail(&hookopts, cgen->program->link ? + cgen->program->link->hookopts : + NULL); if (r) return bf_err_r(r, "failed to add hookopts to list"); @@ -297,7 +299,7 @@ static int _bf_cli_chain_get(const struct bf_request *request, return r; bf_wpack_close_object(wpack); - if (cgen->program->link->hookopts) { + if (cgen->program->link) { bf_wpack_open_object(wpack, "hookopts"); r = bf_hookopts_pack(cgen->program->link->hookopts, wpack); if (r) @@ -463,7 +465,7 @@ int _bf_cli_chain_attach(const struct bf_request *request, cgen = bf_ctx_get_cgen(name); if (!cgen) return bf_err_r(-ENOENT, "chain '%s' does not exist", name); - if (cgen->program->link->hookopts) + if (cgen->program->link) return bf_err_r(-EBUSY, "chain '%s' is already linked to a hook", name); r = bf_hookopts_validate(hookopts, cgen->chain->hook); @@ -542,7 +544,7 @@ int _bf_cli_chain_flush(const struct bf_request *request, } int _bf_cli_chain_update_set(const struct bf_request *request, - struct bf_response **response) + struct bf_response **response) { _free_bf_set_ struct bf_set *to_add = NULL; _free_bf_set_ struct bf_set *to_remove = NULL; From fc16f9330bbd6e37ff4d89e41ad8875c4a5d069b Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 08:49:29 +0100 Subject: [PATCH 02/12] cli,lib: fix style --- src/bfcli/chain.c | 6 ++++-- src/bfcli/opts.c | 12 ++++++------ src/bfcli/opts.h | 3 ++- src/libbpfilter/chain.c | 3 ++- src/libbpfilter/cli.c | 7 ++++--- src/libbpfilter/include/bpfilter/chain.h | 3 ++- src/libbpfilter/matcher.c | 6 ++++-- src/libbpfilter/set.c | 7 +++---- 8 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/bfcli/chain.c b/src/bfcli/chain.c index aed59d02..a9492c48 100644 --- a/src/bfcli/chain.c +++ b/src/bfcli/chain.c @@ -302,7 +302,8 @@ int bfc_chain_update_set(const struct bfc_opts *opts) return bf_err_r(r, "failed to parse set element '%s'", raw_elem); } - r = bf_set_new(&to_remove, opts->set_name, dest_set->key, dest_set->n_comps); + r = bf_set_new(&to_remove, opts->set_name, dest_set->key, + dest_set->n_comps); if (r) return bf_err_r(r, "failed to create set"); @@ -316,7 +317,8 @@ int bfc_chain_update_set(const struct bfc_opts *opts) r = bf_chain_update_set(opts->name, to_add, to_remove); if (r) - return bf_err_r(r, "failed to update set '%s' in chain '%s'", opts->set_name, opts->name); + return bf_err_r(r, "failed to update set '%s' in chain '%s'", + opts->set_name, opts->name); bf_info("updated set '%s' in chain '%s'", opts->set_name, opts->name); diff --git a/src/bfcli/opts.c b/src/bfcli/opts.c index 3b5f5e68..fd2cdd80 100644 --- a/src/bfcli/opts.c +++ b/src/bfcli/opts.c @@ -381,8 +381,8 @@ static void _bfc_opts_set_name_cb(struct argp_state *state, const char *arg, opts->set_name = arg; }; -static void _bfc_opts_set_add_cb(struct argp_state *state, - const char *arg, struct bfc_opts *opts) +static void _bfc_opts_set_add_cb(struct argp_state *state, const char *arg, + struct bfc_opts *opts) { int r; @@ -394,9 +394,8 @@ static void _bfc_opts_set_add_cb(struct argp_state *state, argp_error(state, "failed to add element to list"); }; -static void _bfc_opts_set_remove_cb(struct argp_state *state, - const char *arg, - struct bfc_opts *opts) +static void _bfc_opts_set_remove_cb(struct argp_state *state, const char *arg, + struct bfc_opts *opts) { int r; @@ -522,7 +521,8 @@ struct bfc_opts_opt .key = 'R', .name = "remove", .arg = "ELEMENT", - .doc = "Element to remove from the set. Can be specified multiple times.", + .doc = + "Element to remove from the set. Can be specified multiple times.", .parser = _bfc_opts_set_remove_cb, }, { diff --git a/src/bfcli/opts.h b/src/bfcli/opts.h index 766cf70f..0ccfa4b8 100644 --- a/src/bfcli/opts.h +++ b/src/bfcli/opts.h @@ -96,7 +96,8 @@ struct bfc_opts_cmd * @brief Initialize a `bfc_opts` object to default values. */ #define bfc_opts_default() \ - {.object = _BFC_OBJECT_MAX, .action = _BFC_ACTION_MAX, \ + {.object = _BFC_OBJECT_MAX, \ + .action = _BFC_ACTION_MAX, \ .set_add = bf_list_default(NULL, NULL), \ .set_remove = bf_list_default(NULL, NULL)}; diff --git a/src/libbpfilter/chain.c b/src/libbpfilter/chain.c index 5e59f548..9213febf 100644 --- a/src/libbpfilter/chain.c +++ b/src/libbpfilter/chain.c @@ -319,7 +319,8 @@ struct bf_set *bf_chain_get_set_for_matcher(const struct bf_chain *chain, return bf_list_get_at(&chain->sets, set_id); } -struct bf_set *bf_chain_get_set_by_name(struct bf_chain *chain, const char *set_name) +struct bf_set *bf_chain_get_set_by_name(struct bf_chain *chain, + const char *set_name) { assert(chain); assert(set_name); diff --git a/src/libbpfilter/cli.c b/src/libbpfilter/cli.c index b25c1623..912e3070 100644 --- a/src/libbpfilter/cli.c +++ b/src/libbpfilter/cli.c @@ -535,10 +535,11 @@ int bf_chain_update_set(const char *name, const struct bf_set *to_add, return r; bf_wpack_close_object(wpack); - r = bf_request_new_from_pack(&request, BF_FRONT_CLI, BF_REQ_CHAIN_UPDATE_SET, - wpack); + r = bf_request_new_from_pack(&request, BF_FRONT_CLI, + BF_REQ_CHAIN_UPDATE_SET, wpack); if (r) - return bf_err_r(r, "bf_chain_update_set: failed to create a new request"); + return bf_err_r(r, + "bf_chain_update_set: failed to create a new request"); fd = bf_connect_to_daemon(); if (fd < 0) diff --git a/src/libbpfilter/include/bpfilter/chain.h b/src/libbpfilter/include/bpfilter/chain.h index 872bab6a..82add0af 100644 --- a/src/libbpfilter/include/bpfilter/chain.h +++ b/src/libbpfilter/include/bpfilter/chain.h @@ -156,7 +156,8 @@ struct bf_set *bf_chain_get_set_for_matcher(const struct bf_chain *chain, * @param set_name Name of the set to retrieve. Can't be NULL. * @return Pointer to the set, or NULL if not found. */ -struct bf_set *bf_chain_get_set_by_name(struct bf_chain *chain, const char *set_name); +struct bf_set *bf_chain_get_set_by_name(struct bf_chain *chain, + const char *set_name); /** Allocate and initialize a chain as a copy of another chain. * diff --git a/src/libbpfilter/matcher.c b/src/libbpfilter/matcher.c index 605595ce..354d6f13 100644 --- a/src/libbpfilter/matcher.c +++ b/src/libbpfilter/matcher.c @@ -173,9 +173,11 @@ int _bf_parse_iface(enum bf_matcher_type type, enum bf_matcher_op op, r = bf_if_index_from_str(raw_payload, (uint32_t *)payload); if (r) { - return bf_err_r(r, + return bf_err_r( + r, "\"%s %s\" expects an interface name (e.g., \"eth0\", \"wlan0\") or a decimal interface index (e.g., \"1\", \"2\"), not '%s'", - bf_matcher_type_to_str(type), bf_matcher_op_to_str(op), raw_payload); + bf_matcher_type_to_str(type), bf_matcher_op_to_str(op), + raw_payload); } return 0; diff --git a/src/libbpfilter/set.c b/src/libbpfilter/set.c index 57c50e64..d7e685ad 100644 --- a/src/libbpfilter/set.c +++ b/src/libbpfilter/set.c @@ -445,9 +445,7 @@ static int _bf_set_cmp_key_format(const struct bf_set *first, if (memcmp(first->key, second->key, first->n_comps * sizeof(enum bf_matcher_type)) != 0) - return bf_err_r( - -EINVAL, - "set key component type mismatch"); + return bf_err_r(-EINVAL, "set key component type mismatch"); return 0; } @@ -481,7 +479,8 @@ int bf_set_add_many(struct bf_set *dest, struct bf_set **to_add) } if (!found) { - r = bf_list_add_tail(&dest->elems, bf_list_node_get_data(elem_node)); + r = bf_list_add_tail(&dest->elems, + bf_list_node_get_data(elem_node)); if (r) return bf_err_r(r, "failed to add element to set"); // Take ownership of data to stop to_add cleanup from freeing it. From 0671059475fed68396c2b73d0f984749d09fa1bc Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 08:50:16 +0100 Subject: [PATCH 03/12] tests: e2e: add test to validate sets are restored on daemon restart --- tests/e2e/CMakeLists.txt | 1 + tests/e2e/daemon/restore_with_sets.sh | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100755 tests/e2e/daemon/restore_with_sets.sh diff --git a/tests/e2e/CMakeLists.txt b/tests/e2e/CMakeLists.txt index dbfe8d1d..77f6f63a 100644 --- a/tests/e2e/CMakeLists.txt +++ b/tests/e2e/CMakeLists.txt @@ -68,6 +68,7 @@ bf_add_e2e_test(e2e daemon/netns_to_host.sh ROOT) bf_add_e2e_test(e2e daemon/pin_updated_chain.sh ROOT) bf_add_e2e_test(e2e daemon/restore_attached.sh ROOT) bf_add_e2e_test(e2e daemon/restore_non_attached.sh ROOT) +bf_add_e2e_test(e2e daemon/restore_with_sets.sh ROOT) bf_add_e2e_test(e2e daemon/sock_exists.sh ROOT) bf_add_e2e_test(e2e matchers/icmp_code.sh) diff --git a/tests/e2e/daemon/restore_with_sets.sh b/tests/e2e/daemon/restore_with_sets.sh new file mode 100755 index 00000000..3ef5fe8c --- /dev/null +++ b/tests/e2e/daemon/restore_with_sets.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash + +set -eux +set -o pipefail + +. "$(dirname "$0")"/../e2e_test_util.sh + +make_sandbox + +start_bpfilter + ${FROM_NS} bfcli chain set --from-str "chain test BF_HOOK_XDP{ifindex=${NS_IFINDEX}} ACCEPT + set myset (ip4.saddr) in { 192.168.1.1; 192.168.1.2 } + rule (ip4.saddr) in myset counter DROP" +stop_bpfilter --skip-cleanup + +start_bpfilter + # Verify chain with sets is restored properly + ${FROM_NS} bfcli chain get --name test +stop_bpfilter From 6b9fd027d4fe86c7bb6960e631ae0f49f4e467a8 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 09:20:59 +0100 Subject: [PATCH 04/12] daemon: cgen: create program's maps on demand bf_program creates the necessary maps objects by default (in bf_program_new()), then update them with the proper size and number of elements. This commit improves this behaviour by following bf_link's example: bf_map are not created until they are required by the program. Calling bf_map_new() will create the BPF map object, which makes the behaviour consistent with bf_link. --- src/bpfilter/cgen/prog/map.c | 367 ++++++++++++++--------------------- src/bpfilter/cgen/prog/map.h | 72 +------ src/bpfilter/cgen/program.c | 326 ++++++++++++++----------------- 3 files changed, 293 insertions(+), 472 deletions(-) diff --git a/src/bpfilter/cgen/prog/map.c b/src/bpfilter/cgen/prog/map.c index ac4bc23b..0a6dc216 100644 --- a/src/bpfilter/cgen/prog/map.c +++ b/src/bpfilter/cgen/prog/map.c @@ -24,20 +24,141 @@ #include "ctx.h" +#define _free_bf_btf_ __attribute__((__cleanup__(_bf_btf_free))) + +static void _bf_btf_free(struct bf_btf **btf); + +static int _bf_btf_new(struct bf_btf **btf) +{ + _free_bf_btf_ struct bf_btf *_btf = NULL; + + assert(btf); + + _btf = malloc(sizeof(struct bf_btf)); + if (!_btf) + return -ENOMEM; + + _btf->fd = -1; + + _btf->btf = btf__new_empty(); + if (!_btf->btf) + return -errno; + + *btf = TAKE_PTR(_btf); + + return 0; +} + +static void _bf_btf_free(struct bf_btf **btf) +{ + assert(btf); + + if (!*btf) + return; + + btf__free((*btf)->btf); + closep(&(*btf)->fd); + freep((void *)btf); +} + +static int _bf_btf_load(struct bf_btf *btf) +{ + union bpf_attr attr = {}; + const void *raw; + int r; + + assert(btf); + + raw = btf__raw_data(btf->btf, &attr.btf_size); + if (!raw) + return bf_err_r(errno, "failed to request BTF raw data"); + + r = bf_bpf_btf_load(raw, bf_ctx_token()); + if (r < 0) + return r; + + btf->fd = r; + + return 0; +} + +/** + * @brief Create the BTF data for the map. + * + * @param map Map to create the BTF data for. @c map.type will define the + * exact content of the BTF object. Can't be NULL. + * @return A @ref bf_btf structure on success, or NULL on error. The + * @ref bf_btf structure is owned by the caller. + */ +static struct bf_btf *_bf_map_make_btf(const struct bf_map *map) +{ + _free_bf_btf_ struct bf_btf *btf = NULL; + struct btf *kbtf; + int r; + + assert(map); + + r = _bf_btf_new(&btf); + if (r < 0) + return NULL; + + kbtf = btf->btf; + + switch (map->type) { + case BF_MAP_TYPE_COUNTERS: + btf__add_int(kbtf, "u64", 8, 0); + btf->key_type_id = btf__add_int(kbtf, "u32", 4, 0); + btf->value_type_id = btf__add_struct(kbtf, "bf_counters", 16); + btf__add_field(kbtf, "packets", 1, 0, 0); + btf__add_field(kbtf, "bytes", 1, 64, 0); + break; + case BF_MAP_TYPE_PRINTER: + case BF_MAP_TYPE_SET: + case BF_MAP_TYPE_LOG: + // No BTF data available for this map types + return NULL; + default: + bf_err_r(-ENOTSUP, "bf_map type %d is not supported", map->type); + return NULL; + } + + r = _bf_btf_load(btf); + if (r) { + bf_warn_r(r, "failed to load BTF data for %s, ignoring", map->name); + return NULL; + } + + return TAKE_PTR(btf); +} + static int _bf_map_new(struct bf_map **map, const char *name, enum bf_map_type type, enum bf_bpf_map_type bpf_type, size_t key_size, size_t value_size, size_t n_elems) { + _free_bf_map_ struct bf_map *_map = NULL; + _free_bf_btf_ struct bf_btf *btf = NULL; + _cleanup_close_ int fd = -1; + assert(map); assert(name); - _free_bf_map_ struct bf_map *_map = NULL; - if (name[0] == '\0') - return bf_err_r(-EINVAL, "map name can't be empty"); + return bf_err_r(-EINVAL, "bf_map %s: name can't be empty", name); + + if (type != BF_MAP_TYPE_LOG && key_size == 0) + return bf_err_r(-EINVAL, "bf_map %s: key size can't be 0", name); + if (type == BF_MAP_TYPE_LOG && key_size != 0) + return bf_err_r(-EINVAL, "bf_map %s: key size must be", name); - if (n_elems == 0) - return bf_err_r(-EINVAL, "map can't be created without elements"); + if (type != BF_MAP_TYPE_LOG && value_size == 0) + return bf_err_r(-EINVAL, "bf_map %s: value size can't be 0", name); + if (type == BF_MAP_TYPE_LOG && value_size != 0) + return bf_err_r(-EINVAL, "bf_map %s: value size must be 0", name); + + if (n_elems == 0) { + return bf_err_r(-EINVAL, "bf_map %s: number of elements can't be 0", + name); + } _map = malloc(sizeof(*_map)); if (!_map) @@ -52,6 +173,15 @@ static int _bf_map_new(struct bf_map **map, const char *name, bf_strncpy(_map->name, BPF_OBJ_NAME_LEN, name); + btf = _bf_map_make_btf(_map); + + fd = + bf_bpf_map_create(_map->name, _map->bpf_type, _map->key_size, + _map->value_size, _map->n_elems, btf, bf_ctx_token()); + if (fd < 0) + return bf_err_r(fd, "bf_map %s: failed to create map", name); + + _map->fd = TAKE_FD(fd); *map = TAKE_PTR(_map); return 0; @@ -66,6 +196,9 @@ int bf_map_new(struct bf_map **map, const char *name, enum bf_map_type type, [BF_MAP_TYPE_LOG] = BF_BPF_MAP_TYPE_RINGBUF, }; + assert(map); + assert(name); + if (type == BF_MAP_TYPE_SET) { return bf_err_r( -EINVAL, @@ -79,6 +212,10 @@ int bf_map_new(struct bf_map **map, const char *name, enum bf_map_type type, int bf_map_new_from_set(struct bf_map **map, const char *name, const struct bf_set *set) { + assert(map); + assert(name); + assert(set); + return _bf_map_new(map, name, BF_MAP_TYPE_SET, set->use_trie ? BF_BPF_MAP_TYPE_LPM_TRIE : BF_BPF_MAP_TYPE_HASH, @@ -127,14 +264,9 @@ int bf_map_new_from_pack(struct bf_map **map, int dir_fd, bf_rpack_node_t node) if (_map->n_elems == 0) return bf_err_r(-EINVAL, "bf_map should not have 0 elements"); - if (dir_fd != -1) { - r = bf_bpf_obj_get(_map->name, dir_fd, &_map->fd); - if (r < 0) { - return bf_err_r(r, "failed to open pinned BPF map '%s'", - _map->name); - } - } else { - _map->fd = -1; + r = bf_bpf_obj_get(_map->name, dir_fd, &_map->fd); + if (r < 0) { + return bf_err_r(r, "failed to open pinned BPF map '%s'", _map->name); } *map = TAKE_PTR(_map); @@ -210,176 +342,12 @@ void bf_map_dump(const struct bf_map *map, prefix_t *prefix) DUMP(prefix, "key_size: %lu", map->key_size); DUMP(prefix, "value_size: %lu", map->value_size); - if (map->n_elems == BF_MAP_N_ELEMS_UNKNOWN) { - DUMP(prefix, "n_elems: unknown"); - } else { - DUMP(prefix, "n_elems: %lu", map->n_elems); - } - bf_dump_prefix_last(prefix); DUMP(prefix, "fd: %d", map->fd); bf_dump_prefix_pop(prefix); } -#define _free_bf_btf_ __attribute__((__cleanup__(_bf_btf_free))) - -static void _bf_btf_free(struct bf_btf **btf); - -static int _bf_btf_new(struct bf_btf **btf) -{ - _free_bf_btf_ struct bf_btf *_btf = NULL; - - assert(btf); - - _btf = malloc(sizeof(struct bf_btf)); - if (!_btf) - return -ENOMEM; - - _btf->fd = -1; - - _btf->btf = btf__new_empty(); - if (!_btf->btf) - return -errno; - - *btf = TAKE_PTR(_btf); - - return 0; -} - -static void _bf_btf_free(struct bf_btf **btf) -{ - assert(btf); - - if (!*btf) - return; - - btf__free((*btf)->btf); - closep(&(*btf)->fd); - freep((void *)btf); -} - -static int _bf_btf_load(struct bf_btf *btf) -{ - union bpf_attr attr = {}; - const void *raw; - int r; - - assert(btf); - - raw = btf__raw_data(btf->btf, &attr.btf_size); - if (!raw) - return bf_err_r(errno, "failed to request BTF raw data"); - - r = bf_bpf_btf_load(raw, bf_ctx_token()); - if (r < 0) - return r; - - btf->fd = r; - - return 0; -} - -/** - * Create the BTF data for the map. - * - * @param map Map to create the BTF data for. @c map.type will define the - * exact content of the BTF object. Can't be NULL. - * @return A @ref bf_btf structure on success, or NULL on error. The - * @ref bf_btf structure is owned by the caller. - */ -static struct bf_btf *_bf_map_make_btf(const struct bf_map *map) -{ - _free_bf_btf_ struct bf_btf *btf = NULL; - struct btf *kbtf; - int r; - - assert(map); - - r = _bf_btf_new(&btf); - if (r < 0) - return NULL; - - kbtf = btf->btf; - - switch (map->type) { - case BF_MAP_TYPE_COUNTERS: - btf__add_int(kbtf, "u64", 8, 0); - btf->key_type_id = btf__add_int(kbtf, "u32", 4, 0); - btf->value_type_id = btf__add_struct(kbtf, "bf_counters", 16); - btf__add_field(kbtf, "packets", 1, 0, 0); - btf__add_field(kbtf, "bytes", 1, 64, 0); - break; - case BF_MAP_TYPE_PRINTER: - case BF_MAP_TYPE_SET: - case BF_MAP_TYPE_LOG: - // No BTF data available for this map types - return NULL; - default: - bf_err_r(-ENOTSUP, "bf_map type %d is not supported", map->type); - return NULL; - } - - r = _bf_btf_load(btf); - if (r) { - bf_warn_r(r, "failed to load BTF data for %s, ignoring", map->name); - return NULL; - } - - return TAKE_PTR(btf); -} - -int bf_map_create(struct bf_map *map) -{ - _free_bf_btf_ struct bf_btf *btf = NULL; - int r; - - assert(map); - - if (map->key_size == BF_MAP_KEY_SIZE_UNKNOWN) { - return bf_err_r( - -EINVAL, - "can't create a map with BF_MAP_KEY_SIZE_UNKNOWN key size"); - } - - if (map->value_size == BF_MAP_VALUE_SIZE_UNKNOWN) { - return bf_err_r( - -EINVAL, - "can't create a map with BF_MAP_VALUE_SIZE_UNKNOWN value size"); - } - - if (map->n_elems == BF_MAP_N_ELEMS_UNKNOWN) { - return bf_err_r( - -EINVAL, - "can't create a map with BF_MAP_N_ELEMS_UNKNOWN number of elements"); - } - - /** The BTF data is not mandatory to use the map, but a good addition. - * Hence, bpfilter will try to make the BTF data available, but will - * ignore if that fails. @ref _bf_map_make_btf is used to isolate the - * BTF data generation: if it fails we ignore the issue, but if it - * succeeds we update the @c bpf_attr structure with the BTF details. - * There is some boilerplate for @ref bf_btf structure, it could be - * simpler, but the current implementation ensure the BTF data is properly - * freed on error, without preventing the BPF map to be created. */ - r = bf_bpf_map_create(map->name, map->bpf_type, map->key_size, - map->value_size, map->n_elems, _bf_map_make_btf(map), - bf_ctx_token()); - if (r < 0) - return bf_err_r(r, "failed to create BPF map '%s'", map->name); - - map->fd = r; - - return 0; -} - -void bf_map_destroy(struct bf_map *map) -{ - assert(map); - - closep(&map->fd); -} - int bf_map_pin(const struct bf_map *map, int dir_fd) { int r; @@ -409,51 +377,6 @@ void bf_map_unpin(const struct bf_map *map, int dir_fd) } } -int bf_map_set_key_size(struct bf_map *map, size_t key_size) -{ - assert(key_size != 0); - - if (map->fd != -1) { - return bf_err_r( - -EPERM, - "can't change the size of the map key once it has been created"); - } - - map->key_size = key_size; - - return 0; -} - -int bf_map_set_value_size(struct bf_map *map, size_t value_size) -{ - assert(value_size != 0); - - if (map->fd != -1) { - return bf_err_r( - -EPERM, - "can't change the size of the map value once it has been created"); - } - - map->value_size = value_size; - - return 0; -} - -int bf_map_set_n_elems(struct bf_map *map, size_t n_elems) -{ - assert(n_elems != 0); - - if (map->fd != -1) { - return bf_err_r( - -EPERM, - "can't change the number of elements in a map once the map has been created"); - } - - map->n_elems = n_elems; - - return 0; -} - int bf_map_set_elem(const struct bf_map *map, void *key, void *value) { assert(map); diff --git a/src/bpfilter/cgen/prog/map.h b/src/bpfilter/cgen/prog/map.h index 6e911780..0b33f939 100644 --- a/src/bpfilter/cgen/prog/map.h +++ b/src/bpfilter/cgen/prog/map.h @@ -26,10 +26,6 @@ enum bf_map_type #define BF_PIN_PATH_LEN 64 -#define BF_MAP_KEY_SIZE_UNKNOWN SIZE_MAX -#define BF_MAP_VALUE_SIZE_UNKNOWN SIZE_MAX -#define BF_MAP_N_ELEMS_UNKNOWN SIZE_MAX - struct bf_map { enum bf_map_type type; @@ -44,10 +40,7 @@ struct bf_map #define _free_bf_map_ __attribute__((__cleanup__(bf_map_free))) /** - * Allocates and initializes a new BPF map object. - * - * @note This function won't create a new BPF map, but a bpfilter-specific - * object used to keep track of a BPF map on the system. + * @brief Allocates and initializes a new BPF map object. * * @param map BPF map object to allocate and initialize. Can't be NULL. On * success, @c *map points to a valid @ref bf_map . On failure, @@ -58,10 +51,7 @@ struct bf_map * @param type Map type, defines the set of available operations. * @param key_size Size (in bytes) of a key in the map. * @param value_size Size (in bytes) of an element of the map. - * @param n_elems Number of elements to reserve room for in the map. Can't be 0. - * If you don't yet know the number of elements in the map, use - * @ref BF_MAP_N_ELEMS_UNKNOWN , but @ref bf_map_create can't be called - * until you set an actual size with @ref bf_map_set_n_elems . + * @param n_elems Number of elements to reserve room for in the map. * @return 0 on success, or a negative errno value on error. */ int bf_map_new(struct bf_map **map, const char *name, enum bf_map_type type, @@ -127,25 +117,6 @@ int bf_map_pack(const struct bf_map *map, bf_wpack_t *pack); */ void bf_map_dump(const struct bf_map *map, prefix_t *prefix); -/** - * Create the BPF map. - * - * @param map BPF map to create. Can't be NULL. - * @return 0 on success, or a negative errno value on failure. - */ -int bf_map_create(struct bf_map *map); - -/** - * Destroy the BPF map. - * - * While this function will effectively close the file descriptor used to - * reference the BPF map, it might survive if a BPF program uses it, or if - * it is pinned to the filesystem. - * - * @param map BPF map to destroy. Can't be NULL. - */ -void bf_map_destroy(struct bf_map *map); - /** * Pin the map to the system. * @@ -165,45 +136,6 @@ int bf_map_pin(const struct bf_map *map, int dir_fd); */ void bf_map_unpin(const struct bf_map *map, int dir_fd); -/** - * Set the size of the map's keys. - * - * This function can be used to change the size of the map's keys, up - * until @ref bf_map_create is called. Once the map has been created, the size - * of the keys can't be changed. - * - * @param map The map to modify Can't be NULL. - * @param key_size Size of the keys. Can't be 0. - * @return 0 on success, or a negative errno value on error. - */ -int bf_map_set_key_size(struct bf_map *map, size_t key_size); - -/** - * Set the size of the map's values. - * - * This function can be used to change the size of the map's values, up - * until @ref bf_map_create is called. Once the map has been created, the size - * of the values can't be changed. - * - * @param map The map to modify Can't be NULL. - * @param value_size Size of the values. Can't be 0. - * @return 0 on success, or a negative errno value on error. - */ -int bf_map_set_value_size(struct bf_map *map, size_t value_size); - -/** - * Set the number of elements in the map. - * - * This function can be used to change the number of element of a map, up - * until @ref bf_map_create is called. Once the map has been created, the - * number of elements can't be changed. - * - * @param map The map to set the number of elements for. Can't be NULL. - * @param n_elems Number of elements in the map. Can't be 0. - * @return 0 on success, or a negative errno value on error. - */ -int bf_map_set_n_elems(struct bf_map *map, size_t n_elems); - /** * Insert or update an element to the map. * diff --git a/src/bpfilter/cgen/program.c b/src/bpfilter/cgen/program.c index d5b6b084..07346b22 100644 --- a/src/bpfilter/cgen/program.c +++ b/src/bpfilter/cgen/program.c @@ -64,6 +64,7 @@ #define _BF_LOG_MAP_N_ENTRIES 1000 #define _BF_LOG_MAP_SIZE \ _bf_round_next_power_of_2(sizeof(struct bf_log) * _BF_LOG_MAP_N_ENTRIES) +#define _BF_PROG_NAME "bf_prog" static inline size_t _bf_round_next_power_of_2(size_t value) { @@ -95,8 +96,6 @@ static const struct bf_flavor_ops *bf_flavor_ops_get(enum bf_flavor flavor) int bf_program_new(struct bf_program **program, const struct bf_chain *chain) { _free_bf_program_ struct bf_program *_program = NULL; - char name[BPF_OBJ_NAME_LEN]; - uint32_t set_idx = 0; int r; assert(program); @@ -106,55 +105,18 @@ int bf_program_new(struct bf_program **program, const struct bf_chain *chain) if (!_program) return -ENOMEM; + (void)snprintf(_program->prog_name, BPF_OBJ_NAME_LEN, "%s", _BF_PROG_NAME); _program->flavor = bf_hook_to_flavor(chain->hook); _program->runtime.prog_fd = -1; _program->runtime.ops = bf_flavor_ops_get(_program->flavor); _program->runtime.chain = chain; - - (void)snprintf(_program->prog_name, BPF_OBJ_NAME_LEN, "%s", "bf_prog"); - - r = bf_map_new(&_program->cmap, "counters_map", BF_MAP_TYPE_COUNTERS, - sizeof(uint32_t), sizeof(struct bf_counter), 1); - if (r < 0) - return bf_err_r(r, "failed to create the counters bf_map object"); - - r = bf_map_new(&_program->pmap, "printer_map", BF_MAP_TYPE_PRINTER, - sizeof(uint32_t), BF_MAP_VALUE_SIZE_UNKNOWN, 1); - if (r < 0) - return bf_err_r(r, "failed to create the printer bf_map object"); - - r = bf_map_new(&_program->lmap, "log_map", BF_MAP_TYPE_LOG, 0, 0, - _BF_LOG_MAP_SIZE); - if (r < 0) - return bf_err_r(r, "failed to create the log bf_map object"); - _program->sets = bf_list_default(bf_map_free, bf_map_pack); - bf_list_foreach (&chain->sets, set_node) { - struct bf_set *set = bf_list_node_get_data(set_node); - _free_bf_map_ struct bf_map *map = NULL; - - if (!bf_set_is_empty(set)) { - (void)snprintf(name, BPF_OBJ_NAME_LEN, "set_%04x", - (uint8_t)set_idx); - r = bf_map_new_from_set(&map, name, set); - if (r < 0) - return r; - } - - r = bf_list_push(&_program->sets, (void **)&map); - if (r < 0) - return r; - - set_idx++; - }; + _program->fixups = bf_list_default(bf_fixup_free, NULL); r = bf_printer_new(&_program->printer); if (r) return r; - bf_list_init(&_program->fixups, - (bf_list_ops[]) {{.free = (bf_list_ops_free)bf_fixup_free}}); - *program = TAKE_PTR(_program); return 0; @@ -175,32 +137,35 @@ int bf_program_new_from_pack(struct bf_program **program, assert(chain); r = bf_program_new(&_program, chain); - if (r < 0) + if (r) return r; - bf_map_free(&_program->cmap); - r = bf_rpack_kv_obj(node, "cmap", &child); + r = bf_rpack_kv_node(node, "cmap", &child); if (r) return bf_rpack_key_err(r, "bf_program.cmap"); - r = bf_map_new_from_pack(&_program->cmap, dir_fd, child); - if (r) - return r; + if (!bf_rpack_is_nil(child)) { + r = bf_map_new_from_pack(&_program->cmap, dir_fd, child); + if (r) + return bf_rpack_key_err(r, "bf_program.cmap"); + } - bf_map_free(&_program->pmap); - r = bf_rpack_kv_obj(node, "pmap", &child); + r = bf_rpack_kv_node(node, "pmap", &child); if (r) return bf_rpack_key_err(r, "bf_program.pmap"); - r = bf_map_new_from_pack(&_program->pmap, dir_fd, child); - if (r) - return r; + if (!bf_rpack_is_nil(child)) { + r = bf_map_new_from_pack(&_program->pmap, dir_fd, child); + if (r) + return bf_rpack_key_err(r, "bf_program.pmap"); + } - bf_map_free(&_program->lmap); - r = bf_rpack_kv_obj(node, "lmap", &child); + r = bf_rpack_kv_node(node, "lmap", &child); if (r) return bf_rpack_key_err(r, "bf_program.lmap"); - r = bf_map_new_from_pack(&_program->lmap, dir_fd, child); - if (r) - return r; + if (!bf_rpack_is_nil(child)) { + r = bf_map_new_from_pack(&_program->lmap, dir_fd, child); + if (r) + return bf_rpack_key_err(r, "bf_program.lmap"); + } bf_list_clean(&_program->sets); _program->sets = bf_list_default(bf_map_free, bf_map_pack); @@ -288,17 +253,29 @@ int bf_program_pack(const struct bf_program *program, bf_wpack_t *pack) assert(program); assert(pack); - bf_wpack_open_object(pack, "cmap"); - bf_map_pack(program->cmap, pack); - bf_wpack_close_object(pack); + if (program->cmap) { + bf_wpack_open_object(pack, "cmap"); + bf_map_pack(program->cmap, pack); + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_nil(pack, "cmap"); + } - bf_wpack_open_object(pack, "pmap"); - bf_map_pack(program->pmap, pack); - bf_wpack_close_object(pack); + if (program->pmap) { + bf_wpack_open_object(pack, "pmap"); + bf_map_pack(program->pmap, pack); + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_nil(pack, "pmap"); + } - bf_wpack_open_object(pack, "lmap"); - bf_map_pack(program->lmap, pack); - bf_wpack_close_object(pack); + if (program->lmap) { + bf_wpack_open_object(pack, "lmap"); + bf_map_pack(program->lmap, pack); + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_nil(pack, "lmap"); + } bf_wpack_kv_list(pack, "sets", &program->sets); @@ -331,20 +308,32 @@ void bf_program_dump(const struct bf_program *program, prefix_t *prefix) DUMP(prefix, "prog_name: %s", program->prog_name); - DUMP(prefix, "cmap: struct bf_map *"); - bf_dump_prefix_push(prefix); - bf_map_dump(program->cmap, bf_dump_prefix_last(prefix)); - bf_dump_prefix_pop(prefix); + if (program->cmap) { + DUMP(prefix, "cmap: struct bf_map *"); + bf_dump_prefix_push(prefix); + bf_map_dump(program->cmap, bf_dump_prefix_last(prefix)); + bf_dump_prefix_pop(prefix); + } else { + DUMP(prefix, "cmap: struct bf_map * (NULL)"); + } - DUMP(prefix, "pmap: struct bf_map *"); - bf_dump_prefix_push(prefix); - bf_map_dump(program->pmap, bf_dump_prefix_last(prefix)); - bf_dump_prefix_pop(prefix); + if (program->pmap) { + DUMP(prefix, "pmap: struct bf_map *"); + bf_dump_prefix_push(prefix); + bf_map_dump(program->pmap, bf_dump_prefix_last(prefix)); + bf_dump_prefix_pop(prefix); + } else { + DUMP(prefix, "pmap: struct bf_map * (NULL)"); + } - DUMP(prefix, "lmap: struct bf_map *"); - bf_dump_prefix_push(prefix); - bf_map_dump(program->lmap, bf_dump_prefix_last(prefix)); - bf_dump_prefix_pop(prefix); + if (program->lmap) { + DUMP(prefix, "lmap: struct bf_map *"); + bf_dump_prefix_push(prefix); + bf_map_dump(program->lmap, bf_dump_prefix_last(prefix)); + bf_dump_prefix_pop(prefix); + } else { + DUMP(prefix, "lmap: struct bf_map * (NULL)"); + } DUMP(prefix, "sets: bf_list[%lu]", bf_list_size(&program->sets)); bf_dump_prefix_push(prefix); @@ -909,23 +898,18 @@ static int _bf_program_load_printer_map(struct bf_program *program) if (r) return bf_err_r(r, "failed to assemble printer map string"); - r = bf_map_set_value_size(program->pmap, pstr_len); - if (r < 0) - return r; - - r = bf_map_create(program->pmap); - if (r < 0) - return r; + r = bf_map_new(&program->pmap, "printer_map", BF_MAP_TYPE_PRINTER, + sizeof(uint32_t), pstr_len, 1); + if (r) + return bf_err_r(r, "failed to create the printer bf_map object"); r = bf_map_set_elem(program->pmap, &key, pstr); if (r) - return r; + return bf_err_r(r, "failed to set print map elem"); r = _bf_program_fixup(program, BF_FIXUP_TYPE_PRINTER_MAP_FD); - if (r) { - bf_map_destroy(program->pmap); + if (r) return bf_err_r(r, "failed to fixup printer map FD"); - } return 0; } @@ -937,20 +921,15 @@ static int _bf_program_load_counters_map(struct bf_program *program) assert(program); - r = bf_map_set_n_elems(program->cmap, - bf_list_size(&program->runtime.chain->rules) + 2); - if (r < 0) - return r; - - r = bf_map_create(program->cmap); - if (r < 0) - return r; + r = bf_map_new(&program->cmap, "counters_map", BF_MAP_TYPE_COUNTERS, + sizeof(uint32_t), sizeof(struct bf_counter), + bf_list_size(&program->runtime.chain->rules) + 2); + if (r) + return bf_err_r(r, "failed to create the counters bf_map object"); r = _bf_program_fixup(program, BF_FIXUP_TYPE_COUNTERS_MAP_FD); - if (r < 0) { - bf_map_destroy(program->cmap); + if (r) return bf_err_r(r, "failed to fixup counters map FD"); - } return 0; } @@ -962,64 +941,53 @@ static int _bf_program_load_log_map(struct bf_program *program) assert(program); - r = bf_map_create(program->lmap); - if (r < 0) - return r; + // Do not create a log map if it's unused in the chain + if (!(program->runtime.chain->flags & BF_FLAG(BF_CHAIN_LOG))) + return 0; + + r = bf_map_new(&program->lmap, "log_map", BF_MAP_TYPE_LOG, 0, 0, + _BF_LOG_MAP_SIZE); + if (r) + return bf_err_r(r, "failed to create the log bf_map object"); r = _bf_program_fixup(program, BF_FIXUP_TYPE_LOG_MAP_FD); - if (r < 0) { - bf_map_destroy(program->lmap); + if (r) return bf_err_r(r, "failed to fixup log map FD"); - } return 0; } static int _bf_program_load_sets_maps(struct bf_program *new_prog) { - const bf_list_node *set_node; - const bf_list_node *map_node; + char name[BPF_OBJ_NAME_LEN]; + size_t set_idx = 0; int r; assert(new_prog); - set_node = bf_list_get_head(&new_prog->runtime.chain->sets); - map_node = bf_list_get_head(&new_prog->sets); - - // Fill the bf_map with the sets content - while (set_node && map_node) { + bf_list_foreach (&new_prog->runtime.chain->sets, set_node) { + struct bf_set *set = bf_list_node_get_data(set_node); + _free_bf_map_ struct bf_map *map = NULL; _cleanup_free_ uint8_t *values = NULL; _cleanup_free_ uint8_t *keys = NULL; - struct bf_set *set = bf_list_node_get_data(set_node); - struct bf_map *map = bf_list_node_get_data(map_node); - - // Skip null maps (empty sets) - if (!map) { - set_node = bf_list_node_next(set_node); - map_node = bf_list_node_next(map_node); - continue; - } - size_t nelems = bf_list_size(&set->elems); size_t idx = 0; - r = bf_map_create(map); - if (r < 0) { - r = bf_err_r(r, "failed to create BPF map for set"); - goto err_destroy_maps; - } + if (bf_set_is_empty(set)) + continue; + + (void)snprintf(name, BPF_OBJ_NAME_LEN, "set_%04x", (uint8_t)set_idx++); + r = bf_map_new_from_set(&map, name, set); + if (r) + return r; values = malloc(nelems); - if (!values) { - r = bf_err_r(errno, "failed to allocate map values"); - goto err_destroy_maps; - } + if (!values) + return bf_err_r(errno, "failed to allocate map values"); keys = malloc(set->elem_size * nelems); - if (!keys) { - r = bf_err_r(errno, "failed to allocate map keys"); - goto err_destroy_maps; - } + if (!keys) + return bf_err_r(errno, "failed to allocate map keys"); bf_list_foreach (&set->elems, elem_node) { void *elem = bf_list_node_get_data(elem_node); @@ -1030,28 +998,19 @@ static int _bf_program_load_sets_maps(struct bf_program *new_prog) } r = bf_bpf_map_update_batch(map->fd, keys, values, nelems, BPF_ANY); - if (r < 0) { - bf_err_r(r, "failed to add set elements to the map"); - goto err_destroy_maps; - } + if (r) + return bf_err_r(r, "failed to add set elements to the map"); - set_node = bf_list_node_next(set_node); - map_node = bf_list_node_next(map_node); - } + r = bf_list_push(&new_prog->sets, (void **)&map); + if (r) + return r; + }; r = _bf_program_fixup(new_prog, BF_FIXUP_TYPE_SET_MAP_FD); - if (r < 0) - goto err_destroy_maps; + if (r) + return r; return 0; - -err_destroy_maps: - bf_list_foreach (&new_prog->sets, map_node) { - struct bf_map *map = bf_list_node_get_data(map_node); - if (map) - bf_map_destroy(map); - } - return r; } int bf_program_load(struct bf_program *prog) @@ -1063,19 +1022,19 @@ int bf_program_load(struct bf_program *prog) r = _bf_program_load_sets_maps(prog); if (r) - return r; + return bf_err_r(r, "failed to load the sets map"); r = _bf_program_load_counters_map(prog); if (r) - return r; + return bf_err_r(r, "failed to load the counter map"); r = _bf_program_load_printer_map(prog); if (r) - return r; + return bf_err_r(r, "failed to load the printer map"); r = _bf_program_load_log_map(prog); if (r) - return r; + return bf_err_r(r, "failed to load the log map"); if (bf_opts_is_verbose(BF_VERBOSE_DEBUG)) { log_buf = malloc(_BF_LOG_BUF_SIZE); @@ -1127,18 +1086,16 @@ void bf_program_detach(struct bf_program *prog) void bf_program_unload(struct bf_program *prog) { + _clean_bf_list_ bf_list list = bf_list_default_from(prog->sets); + assert(prog); closep(&prog->runtime.prog_fd); bf_link_free(&prog->link); - bf_map_destroy(prog->cmap); - bf_map_destroy(prog->pmap); - bf_map_destroy(prog->lmap); - bf_list_foreach (&prog->sets, map_node) { - struct bf_map *map = bf_list_node_get_data(map_node); - if (map) - bf_map_destroy(map); - } + bf_map_free(&prog->cmap); + bf_map_free(&prog->pmap); + bf_map_free(&prog->lmap); + bf_swap(list, prog->sets); } int bf_program_get_counter(const struct bf_program *program, @@ -1180,22 +1137,28 @@ int bf_program_pin(struct bf_program *prog, int dir_fd) goto err_unpin_all; } - r = bf_map_pin(prog->cmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF counters map for '%s'", name); - goto err_unpin_all; + if (prog->cmap) { + r = bf_map_pin(prog->cmap, dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF counters map for '%s'", name); + goto err_unpin_all; + } } - r = bf_map_pin(prog->pmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF printer map for '%s'", name); - goto err_unpin_all; + if (prog->pmap) { + r = bf_map_pin(prog->pmap, dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF printer map for '%s'", name); + goto err_unpin_all; + } } - r = bf_map_pin(prog->lmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF log map for '%s'", name); - goto err_unpin_all; + if (prog->lmap) { + r = bf_map_pin(prog->lmap, dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF log map for '%s'", name); + goto err_unpin_all; + } } bf_list_foreach (&prog->sets, set_node) { @@ -1230,9 +1193,12 @@ void bf_program_unpin(struct bf_program *prog, int dir_fd) { assert(prog); - bf_map_unpin(prog->cmap, dir_fd); - bf_map_unpin(prog->pmap, dir_fd); - bf_map_unpin(prog->lmap, dir_fd); + if (prog->cmap) + bf_map_unpin(prog->cmap, dir_fd); + if (prog->pmap) + bf_map_unpin(prog->pmap, dir_fd); + if (prog->lmap) + bf_map_unpin(prog->lmap, dir_fd); bf_list_foreach (&prog->sets, set_node) { struct bf_map *map = bf_list_node_get_data(set_node); From fbd63739d34a85a1346483f02f1a7c85ecc83419 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 10:06:46 +0100 Subject: [PATCH 05/12] daemon: cgen: introduce bf_handle to store references to BPF objects Create bf_handle to store references to all the BPF objects defined for a chain, instead of centralizing them in bf_program. The benefit is that bf_cgen can take the ownership of bf_handle and discard the bf_program object that becomes irrelevant once the bytecode is loaded. --- src/bpfilter/CMakeLists.txt | 1 + src/bpfilter/cgen/cgen.c | 9 +- src/bpfilter/cgen/handle.c | 384 ++++++++++++++++++++++++++++++++++++ src/bpfilter/cgen/handle.h | 176 +++++++++++++++++ src/bpfilter/cgen/program.c | 303 ++++------------------------ src/bpfilter/cgen/program.h | 39 ++-- src/bpfilter/xlate/cli.c | 22 ++- 7 files changed, 635 insertions(+), 299 deletions(-) create mode 100644 src/bpfilter/cgen/handle.c create mode 100644 src/bpfilter/cgen/handle.h diff --git a/src/bpfilter/CMakeLists.txt b/src/bpfilter/CMakeLists.txt index 610605f8..042c07ce 100644 --- a/src/bpfilter/CMakeLists.txt +++ b/src/bpfilter/CMakeLists.txt @@ -20,6 +20,7 @@ add_executable(bpfilter ${CMAKE_CURRENT_SOURCE_DIR}/cgen/dump.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/dump.c ${CMAKE_CURRENT_SOURCE_DIR}/cgen/elfstub.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/elfstub.c ${CMAKE_CURRENT_SOURCE_DIR}/cgen/fixup.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/fixup.c + ${CMAKE_CURRENT_SOURCE_DIR}/cgen/handle.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/handle.c ${CMAKE_CURRENT_SOURCE_DIR}/cgen/jmp.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/jmp.c ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/ip4.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/ip4.c ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/ip6.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/ip6.c diff --git a/src/bpfilter/cgen/cgen.c b/src/bpfilter/cgen/cgen.c index 139c40ab..a935b2f9 100644 --- a/src/bpfilter/cgen/cgen.c +++ b/src/bpfilter/cgen/cgen.c @@ -25,6 +25,7 @@ #include #include "cgen/dump.h" +#include "cgen/handle.h" #include "cgen/prog/link.h" #include "cgen/program.h" #include "ctx.h" @@ -333,7 +334,7 @@ int bf_cgen_attach(struct bf_cgen *cgen, const struct bf_ns *ns, bf_abort("failed to restore previous namespaces, aborting"); if (bf_opts_persist()) { - r = bf_link_pin(cgen->program->link, pindir_fd); + r = bf_link_pin(cgen->program->handle->link, pindir_fd); if (r) { bf_program_detach(cgen->program); return r; @@ -378,8 +379,8 @@ int bf_cgen_update(struct bf_cgen *cgen, struct bf_chain **new_chain) if (bf_opts_persist()) bf_program_unpin(old_prog, pindir_fd); - if (old_prog->link) { - r = bf_link_update(old_prog->link, new_prog->runtime.prog_fd); + if (old_prog->handle->link) { + r = bf_link_update(old_prog->handle->link, new_prog->handle->prog_fd); if (r) { bf_err_r(r, "failed to update bf_link object with new program"); if (bf_opts_persist() && bf_program_pin(old_prog, pindir_fd) < 0) @@ -388,7 +389,7 @@ int bf_cgen_update(struct bf_cgen *cgen, struct bf_chain **new_chain) } // We updated the old link, we need to store it in the new program - bf_swap(new_prog->link, old_prog->link); + bf_swap(new_prog->handle->link, old_prog->handle->link); } if (bf_opts_persist()) { diff --git a/src/bpfilter/cgen/handle.c b/src/bpfilter/cgen/handle.c new file mode 100644 index 00000000..39170826 --- /dev/null +++ b/src/bpfilter/cgen/handle.c @@ -0,0 +1,384 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2023 Meta Platforms, Inc. and affiliates. + */ + +#include "cgen/handle.h" + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "cgen/prog/link.h" +#include "cgen/prog/map.h" + +int bf_handle_new(struct bf_handle **handle, const char *prog_name) +{ + _free_bf_handle_ struct bf_handle *_handle = NULL; + + assert(handle); + assert(prog_name); + + _handle = calloc(1, sizeof(*_handle)); + if (!_handle) + return -ENOMEM; + + (void)snprintf(_handle->prog_name, BPF_OBJ_NAME_LEN, "%s", prog_name); + _handle->prog_fd = -1; + _handle->sets = bf_list_default(bf_map_free, bf_map_pack); + + *handle = TAKE_PTR(_handle); + + return 0; +} + +int bf_handle_new_from_pack(struct bf_handle **handle, int dir_fd, + bf_rpack_node_t node) +{ + _free_bf_handle_ struct bf_handle *_handle = NULL; + _cleanup_free_ char *name = NULL; + bf_rpack_node_t child, array_node; + int r; + + assert(handle); + + r = bf_rpack_kv_str(node, "prog_name", &name); + if (r) + return bf_rpack_key_err(r, "bf_handle.name"); + + r = bf_handle_new(&_handle, name); + if (r) + return r; + + r = bf_bpf_obj_get(_handle->prog_name, dir_fd, &_handle->prog_fd); + if (r < 0) + return bf_err_r(r, "failed to restore bf_handle.prog_fd from pin"); + + r = bf_rpack_kv_node(node, "link", &child); + if (r) + return bf_rpack_key_err(r, "bf_handle.link"); + if (!bf_rpack_is_nil(child)) { + r = bf_link_new_from_pack(&_handle->link, dir_fd, child); + if (r) + return bf_rpack_key_err(r, "bf_handle.link"); + } + + r = bf_rpack_kv_node(node, "cmap", &child); + if (r) + return bf_rpack_key_err(r, "bf_handle.cmap"); + if (!bf_rpack_is_nil(child)) { + r = bf_map_new_from_pack(&_handle->cmap, dir_fd, child); + if (r) + return bf_rpack_key_err(r, "bf_handle.cmap"); + } + + r = bf_rpack_kv_node(node, "pmap", &child); + if (r) + return bf_rpack_key_err(r, "bf_handle.pmap"); + if (!bf_rpack_is_nil(child)) { + r = bf_map_new_from_pack(&_handle->pmap, dir_fd, child); + if (r) + return bf_rpack_key_err(r, "bf_handle.pmap"); + } + + r = bf_rpack_kv_node(node, "lmap", &child); + if (r) + return bf_rpack_key_err(r, "bf_handle.lmap"); + if (!bf_rpack_is_nil(child)) { + r = bf_map_new_from_pack(&_handle->lmap, dir_fd, child); + if (r) + return bf_rpack_key_err(r, "bf_handle.lmap"); + } + + r = bf_rpack_kv_array(node, "sets", &child); + if (r) + return bf_rpack_key_err(r, "bf_handle.sets"); + bf_rpack_array_foreach (child, array_node) { + _free_bf_map_ struct bf_map *map = NULL; + + r = bf_list_emplace(&_handle->sets, bf_map_new_from_pack, map, dir_fd, + array_node); + if (r) + return bf_err_r(r, "failed to unpack bf_map into bf_handle.sets"); + } + + *handle = TAKE_PTR(_handle); + + return 0; +} + +void bf_handle_free(struct bf_handle **handle) +{ + if (!*handle) + return; + + closep(&(*handle)->prog_fd); + + bf_link_free(&(*handle)->link); + bf_map_free(&(*handle)->cmap); + bf_map_free(&(*handle)->pmap); + bf_map_free(&(*handle)->lmap); + bf_list_clean(&(*handle)->sets); + + free(*handle); + *handle = NULL; +} + +int bf_handle_pack(const struct bf_handle *handle, bf_wpack_t *pack) +{ + assert(handle); + assert(pack); + + bf_wpack_kv_str(pack, "prog_name", handle->prog_name); + + if (handle->link) { + bf_wpack_open_object(pack, "link"); + bf_link_pack(handle->link, pack); + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_nil(pack, "link"); + } + + if (handle->cmap) { + bf_wpack_open_object(pack, "cmap"); + bf_map_pack(handle->cmap, pack); + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_nil(pack, "cmap"); + } + + if (handle->pmap) { + bf_wpack_open_object(pack, "pmap"); + bf_map_pack(handle->pmap, pack); + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_nil(pack, "pmap"); + } + + if (handle->lmap) { + bf_wpack_open_object(pack, "lmap"); + bf_map_pack(handle->lmap, pack); + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_nil(pack, "lmap"); + } + + bf_wpack_kv_list(pack, "sets", &handle->sets); + + return bf_wpack_is_valid(pack) ? 0 : -EINVAL; +} + +void bf_handle_dump(const struct bf_handle *handle, prefix_t *prefix) +{ + assert(handle); + assert(prefix); + + DUMP(prefix, "struct bf_handle at %p", handle); + + bf_dump_prefix_push(prefix); + + DUMP(prefix, "prog_name: %s", handle->prog_name); + DUMP(prefix, "prog_fd: %d", handle->prog_fd); + + if (handle->link) { + DUMP(bf_dump_prefix_last(prefix), "link: struct bf_link *"); + bf_dump_prefix_push(prefix); + bf_link_dump(handle->link, bf_dump_prefix_last(prefix)); + bf_dump_prefix_pop(prefix); + } else { + DUMP(bf_dump_prefix_last(prefix), "link: struct bf_link * (NULL)"); + } + + if (handle->cmap) { + DUMP(prefix, "cmap: struct bf_map *"); + bf_dump_prefix_push(prefix); + bf_map_dump(handle->cmap, bf_dump_prefix_last(prefix)); + bf_dump_prefix_pop(prefix); + } else { + DUMP(prefix, "cmap: struct bf_map * (NULL)"); + } + + if (handle->pmap) { + DUMP(prefix, "pmap: struct bf_map *"); + bf_dump_prefix_push(prefix); + bf_map_dump(handle->pmap, bf_dump_prefix_last(prefix)); + bf_dump_prefix_pop(prefix); + } else { + DUMP(prefix, "pmap: struct bf_map * (NULL)"); + } + + if (handle->lmap) { + DUMP(prefix, "lmap: struct bf_map *"); + bf_dump_prefix_push(prefix); + bf_map_dump(handle->lmap, bf_dump_prefix_last(prefix)); + bf_dump_prefix_pop(prefix); + } else { + DUMP(prefix, "lmap: struct bf_map * (NULL)"); + } + + if (handle->lmap) { + DUMP(prefix, "lmap: struct bf_map *"); + bf_dump_prefix_push(prefix); + bf_map_dump(handle->lmap, bf_dump_prefix_last(prefix)); + bf_dump_prefix_pop(prefix); + } else { + DUMP(prefix, "lmap: struct bf_map * (NULL)"); + } + + DUMP(prefix, "sets: bf_list[%lu]", bf_list_size(&handle->sets)); + bf_dump_prefix_push(prefix); + bf_list_foreach (&handle->sets, map_node) { + struct bf_map *map = bf_list_node_get_data(map_node); + + if (bf_list_is_tail(&handle->sets, map_node)) + bf_dump_prefix_last(prefix); + + bf_map_dump(map, prefix); + } + bf_dump_prefix_pop(prefix); + + bf_dump_prefix_pop(prefix); +} + +int bf_handle_pin(struct bf_handle *handle, int dir_fd) +{ + int r; + + assert(handle); + + r = bf_bpf_obj_pin(handle->prog_name, handle->prog_fd, dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF program"); + goto err_unpin_all; + } + + if (handle->cmap) { + r = bf_map_pin(handle->cmap, dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF counters map"); + goto err_unpin_all; + } + } + + if (handle->pmap) { + r = bf_map_pin(handle->pmap, dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF printer map"); + goto err_unpin_all; + } + } + + if (handle->lmap) { + r = bf_map_pin(handle->lmap, dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF log map"); + goto err_unpin_all; + } + } + + bf_list_foreach (&handle->sets, set_node) { + r = bf_map_pin(bf_list_node_get_data(set_node), dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF set map"); + goto err_unpin_all; + } + } + + if (handle->link) { + r = bf_link_pin(handle->link, dir_fd); + if (r) { + bf_err_r(r, "failed to pin BPF link"); + goto err_unpin_all; + } + } + + return 0; + +err_unpin_all: + bf_handle_unpin(handle, dir_fd); + return r; +} + +void bf_handle_unpin(struct bf_handle *handle, int dir_fd) +{ + assert(handle); + + if (handle->cmap) + bf_map_unpin(handle->cmap, dir_fd); + if (handle->pmap) + bf_map_unpin(handle->pmap, dir_fd); + if (handle->lmap) + bf_map_unpin(handle->lmap, dir_fd); + + bf_list_foreach (&handle->sets, set_node) + bf_map_unpin(bf_list_node_get_data(set_node), dir_fd); + + if (handle->link) + bf_link_unpin(handle->link, dir_fd); + + unlinkat(dir_fd, handle->prog_name, 0); +} + +int bf_handle_get_counter(const struct bf_handle *handle, uint32_t counter_idx, + struct bf_counter *counter) +{ + int r; + + assert(handle); + assert(counter); + + if (!handle->cmap) + return bf_err_r(-ENOENT, "handle has no counters map"); + + r = bf_bpf_map_lookup_elem(handle->cmap->fd, &counter_idx, counter); + if (r < 0) + return bf_err_r(errno, "failed to lookup counters map"); + + return 0; +} + +int bf_handle_attach(struct bf_handle *handle, enum bf_hook hook, + struct bf_hookopts **hookopts) +{ + int r; + + assert(handle); + assert(hookopts); + + r = bf_link_new(&handle->link, hook, hookopts, handle->prog_fd); + if (r) + return bf_err_r(r, "failed to attach bf_link"); + + return 0; +} + +void bf_handle_detach(struct bf_handle *handle) +{ + assert(handle); + + bf_link_free(&handle->link); +} + +void bf_handle_unload(struct bf_handle *handle) +{ + _clean_bf_list_ bf_list list = bf_list_default_from(handle->sets); + + assert(handle); + + closep(&handle->prog_fd); + bf_link_free(&handle->link); + bf_map_free(&handle->cmap); + bf_map_free(&handle->pmap); + bf_map_free(&handle->lmap); + bf_swap(list, handle->sets); +} diff --git a/src/bpfilter/cgen/handle.h b/src/bpfilter/cgen/handle.h new file mode 100644 index 00000000..ea84f263 --- /dev/null +++ b/src/bpfilter/cgen/handle.h @@ -0,0 +1,176 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2023 Meta Platforms, Inc. and affiliates. + */ + +#pragma once + +#include + +#include +#include +#include +#include + +struct bf_link; +struct bf_map; +struct bf_counter; + +/** + * @file handle.h + * + * @ref bf_handle is used to manage BPF object references (program fd, link, + * maps) separately from the bytecode generation context. This allows the + * bytecode generator (bf_program) to be discarded after generation while + * keeping the BPF objects alive. + */ + +/** + * BPF object handle. + * + * Holds references to BPF objects (program fd, link, maps) that need to + * persist after bytecode generation is complete. + */ +struct bf_handle +{ + /** BPF program name, used as filename when pinning. */ + char prog_name[BPF_OBJ_NAME_LEN]; + + /** File descriptor of the loaded BPF program. -1 if not loaded. */ + int prog_fd; + + /** Link attaching the program to a hook. NULL if not attached. */ + struct bf_link *link; + + /** Counters map. NULL if not created. */ + struct bf_map *cmap; + + /** Printer map. NULL if not created. */ + struct bf_map *pmap; + + /** Log map. NULL if not created. */ + struct bf_map *lmap; + + /** List of set maps. */ + bf_list sets; +}; + +#define _free_bf_handle_ __attribute__((__cleanup__(bf_handle_free))) + +/** + * @brief Allocate and initialize a new bf_handle object. + * + * @param handle `bf_handle` object to allocate and initialize. Can't be NULL. + * @param prog_name Name of the BPF program. Used as filename when pinning. + * Can't be NULL or empty. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_handle_new(struct bf_handle **handle, const char *prog_name); + +/** + * @brief Allocate and initialize a bf_handle from serialized data. + * + * @param handle `bf_handle` object to allocate and initialize. Can't be NULL. + * @param dir_fd File descriptor of the directory containing the pinned objects. + * Must be a valid file descriptor. + * @param node Node containing the serialized handle data. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_handle_new_from_pack(struct bf_handle **handle, int dir_fd, + bf_rpack_node_t node); + +/** + * @brief Free a `bf_handle` object. + * + * Closes all file descriptors and frees all owned objects. If the BPF objects + * are pinned, they will survive the close. + * + * @param handle `bf_handle` object to free. If `*handle` is NULL, this function + * has no effect. Can't be NULL. + */ +void bf_handle_free(struct bf_handle **handle); + +/** + * Serialize a bf_handle. + * + * Only serializes the prog_name (for pin path restoration). The actual BPF + * objects are restored from pins, not from serialized data. + * + * @param handle Handle to serialize. Can't be NULL. + * @param pack bf_wpack_t object to serialize the handle into. Can't be NULL. + * @return 0 on success, or a negative error value on failure. + */ +int bf_handle_pack(const struct bf_handle *handle, bf_wpack_t *pack); + +/** + * @brief Dump the content of a `bf_handle` object. + * + * @param handle `bf_handle` object to dump. Can't be NULL. + * @param prefix Prefix to use for the dump. Can't be NULL. + */ +void bf_handle_dump(const struct bf_handle *handle, prefix_t *prefix); + +/** + * @brief Pin the BPF objects to the filesystem. + * + * Pins the program and all maps/link to the directory specified by `dir_fd`. + * The link is only pinned if the program is attached. + * + * @param handle Handle containing the BPF objects to pin. Can't be NULL. + * @param dir_fd File descriptor of the directory to pin into. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_handle_pin(struct bf_handle *handle, int dir_fd); + +/** + * @brief Unpin the BPF objects from the filesystem. + * + * Unpins all BPF objects from the directory. This function never fails. + * + * @param handle Handle containing the BPF objects to unpin. Can't be NULL. + * @param dir_fd File descriptor of the directory containing the pins. + */ +void bf_handle_unpin(struct bf_handle *handle, int dir_fd); + +/** + * @brief Get a counter value from the handle's counters map. + * + * @param handle Handle to get the counter from. Can't be NULL. + * @param counter_idx Index of the counter to get. + * @param counter Counter structure to fill with the values. Can't be NULL. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_handle_get_counter(const struct bf_handle *handle, uint32_t counter_idx, + struct bf_counter *counter); + +/** + * @brief Attach the BPF program to a hook. + * + * Creates a `bf_link` to attach the program to the specified hook. + * + * @param handle Handle containing the loaded BPF program. Can't be NULL. + * @param hook Hook to attach the program to. + * @param hookopts Hook-specific options. Can't be NULL. Ownership is taken. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_handle_attach(struct bf_handle *handle, enum bf_hook hook, + struct bf_hookopts **hookopts); + +/** + * @brief Detach the program from its hook. + * + * Frees the link object, which detaches the program from the hook. + * + * @param handle Handle to detach. Can't be NULL. + */ +void bf_handle_detach(struct bf_handle *handle); + +/** + * @brief Unload the BPF program and destroy all BPF objects. + * + * Closes all file descriptors and frees all BPF objects. After this call, + * the handle is empty but still valid. + * + * @param handle Handle to unload. Can't be NULL. + */ +void bf_handle_unload(struct bf_handle *handle); diff --git a/src/bpfilter/cgen/program.c b/src/bpfilter/cgen/program.c index 07346b22..f2d32880 100644 --- a/src/bpfilter/cgen/program.c +++ b/src/bpfilter/cgen/program.c @@ -39,6 +39,7 @@ #include "cgen/cgroup.h" #include "cgen/dump.h" #include "cgen/fixup.h" +#include "cgen/handle.h" #include "cgen/jmp.h" #include "cgen/matcher/icmp.h" #include "cgen/matcher/ip4.h" @@ -105,14 +106,15 @@ int bf_program_new(struct bf_program **program, const struct bf_chain *chain) if (!_program) return -ENOMEM; - (void)snprintf(_program->prog_name, BPF_OBJ_NAME_LEN, "%s", _BF_PROG_NAME); _program->flavor = bf_hook_to_flavor(chain->hook); - _program->runtime.prog_fd = -1; _program->runtime.ops = bf_flavor_ops_get(_program->flavor); _program->runtime.chain = chain; - _program->sets = bf_list_default(bf_map_free, bf_map_pack); _program->fixups = bf_list_default(bf_fixup_free, NULL); + r = bf_handle_new(&_program->handle, _BF_PROG_NAME); + if (r) + return r; + r = bf_printer_new(&_program->printer); if (r) return r; @@ -127,10 +129,9 @@ int bf_program_new_from_pack(struct bf_program **program, bf_rpack_node_t node) { _free_bf_program_ struct bf_program *_program = NULL; - _free_bf_link_ struct bf_link *link = NULL; const void *img; size_t img_len; - bf_rpack_node_t child, array_node; + bf_rpack_node_t child; int r; assert(program); @@ -140,61 +141,13 @@ int bf_program_new_from_pack(struct bf_program **program, if (r) return r; - r = bf_rpack_kv_node(node, "cmap", &child); - if (r) - return bf_rpack_key_err(r, "bf_program.cmap"); - if (!bf_rpack_is_nil(child)) { - r = bf_map_new_from_pack(&_program->cmap, dir_fd, child); - if (r) - return bf_rpack_key_err(r, "bf_program.cmap"); - } - - r = bf_rpack_kv_node(node, "pmap", &child); + bf_handle_free(&_program->handle); + r = bf_rpack_kv_obj(node, "handle", &child); if (r) - return bf_rpack_key_err(r, "bf_program.pmap"); - if (!bf_rpack_is_nil(child)) { - r = bf_map_new_from_pack(&_program->pmap, dir_fd, child); - if (r) - return bf_rpack_key_err(r, "bf_program.pmap"); - } - - r = bf_rpack_kv_node(node, "lmap", &child); + return bf_rpack_key_err(r, "bf_program.handle"); + r = bf_handle_new_from_pack(&_program->handle, dir_fd, child); if (r) - return bf_rpack_key_err(r, "bf_program.lmap"); - if (!bf_rpack_is_nil(child)) { - r = bf_map_new_from_pack(&_program->lmap, dir_fd, child); - if (r) - return bf_rpack_key_err(r, "bf_program.lmap"); - } - - bf_list_clean(&_program->sets); - _program->sets = bf_list_default(bf_map_free, bf_map_pack); - r = bf_rpack_kv_array(node, "sets", &child); - if (r) - return bf_rpack_key_err(r, "bf_program.sets"); - bf_rpack_array_foreach (child, array_node) { - _free_bf_map_ struct bf_map *map = NULL; - - if (!bf_rpack_is_nil(array_node)) { - r = bf_list_emplace(&_program->sets, bf_map_new_from_pack, map, - dir_fd, array_node); - } else { - r = bf_list_add_tail(&_program->sets, NULL); - } - - if (r) - return bf_err_r(r, "failed to unpack bf_map into bf_program.sets"); - } - - // Restore the link if it exists - r = bf_rpack_kv_node(node, "link", &child); - if (r) - return bf_rpack_key_err(r, "bf_program.link"); - if (!bf_rpack_is_nil(child)) { - r = bf_link_new_from_pack(&_program->link, dir_fd, child); - if (r) - return bf_rpack_key_err(r, "bf_program.link"); - } + return r; bf_printer_free(&_program->printer); r = bf_rpack_kv_obj(node, "printer", &child); @@ -213,10 +166,6 @@ int bf_program_new_from_pack(struct bf_program **program, _program->img_size = img_len / sizeof(struct bpf_insn); _program->img_cap = _program->img_size; - r = bf_bpf_obj_get(_program->prog_name, dir_fd, &_program->runtime.prog_fd); - if (r < 0) - return bf_err_r(r, "failed to restore bf_program.fd"); - *program = TAKE_PTR(_program); return 0; @@ -230,18 +179,7 @@ void bf_program_free(struct bf_program **program) bf_list_clean(&(*program)->fixups); free((*program)->img); - /* Close the file descriptors if they are still open. If --transient is - * used, then the file descriptors are already closed (as - * bf_program_unload() has been called). Otherwise, bf_program_unload() - * won't be called, but the programs are pinned, so they can be closed - * safely. */ - closep(&(*program)->runtime.prog_fd); - - bf_map_free(&(*program)->cmap); - bf_map_free(&(*program)->pmap); - bf_map_free(&(*program)->lmap); - bf_list_clean(&(*program)->sets); - bf_link_free(&(*program)->link); + bf_handle_free(&(*program)->handle); bf_printer_free(&(*program)->printer); free(*program); @@ -253,39 +191,9 @@ int bf_program_pack(const struct bf_program *program, bf_wpack_t *pack) assert(program); assert(pack); - if (program->cmap) { - bf_wpack_open_object(pack, "cmap"); - bf_map_pack(program->cmap, pack); - bf_wpack_close_object(pack); - } else { - bf_wpack_kv_nil(pack, "cmap"); - } - - if (program->pmap) { - bf_wpack_open_object(pack, "pmap"); - bf_map_pack(program->pmap, pack); - bf_wpack_close_object(pack); - } else { - bf_wpack_kv_nil(pack, "pmap"); - } - - if (program->lmap) { - bf_wpack_open_object(pack, "lmap"); - bf_map_pack(program->lmap, pack); - bf_wpack_close_object(pack); - } else { - bf_wpack_kv_nil(pack, "lmap"); - } - - bf_wpack_kv_list(pack, "sets", &program->sets); - - if (program->link) { - bf_wpack_open_object(pack, "link"); - bf_link_pack(program->link, pack); - bf_wpack_close_object(pack); - } else { - bf_wpack_kv_nil(pack, "link"); - } + bf_wpack_open_object(pack, "handle"); + bf_handle_pack(program->handle, pack); + bf_wpack_close_object(pack); bf_wpack_open_object(pack, "printer"); bf_printer_pack(program->printer, pack); @@ -308,57 +216,11 @@ void bf_program_dump(const struct bf_program *program, prefix_t *prefix) DUMP(prefix, "prog_name: %s", program->prog_name); - if (program->cmap) { - DUMP(prefix, "cmap: struct bf_map *"); - bf_dump_prefix_push(prefix); - bf_map_dump(program->cmap, bf_dump_prefix_last(prefix)); - bf_dump_prefix_pop(prefix); - } else { - DUMP(prefix, "cmap: struct bf_map * (NULL)"); - } - - if (program->pmap) { - DUMP(prefix, "pmap: struct bf_map *"); - bf_dump_prefix_push(prefix); - bf_map_dump(program->pmap, bf_dump_prefix_last(prefix)); - bf_dump_prefix_pop(prefix); - } else { - DUMP(prefix, "pmap: struct bf_map * (NULL)"); - } - - if (program->lmap) { - DUMP(prefix, "lmap: struct bf_map *"); - bf_dump_prefix_push(prefix); - bf_map_dump(program->lmap, bf_dump_prefix_last(prefix)); - bf_dump_prefix_pop(prefix); - } else { - DUMP(prefix, "lmap: struct bf_map * (NULL)"); - } - - DUMP(prefix, "sets: bf_list[%lu]", bf_list_size(&program->sets)); + DUMP(prefix, "handle: struct bf_handle *"); bf_dump_prefix_push(prefix); - bf_list_foreach (&program->sets, map_node) { - struct bf_map *map = bf_list_node_get_data(map_node); - - if (bf_list_is_tail(&program->sets, map_node)) - bf_dump_prefix_last(prefix); - - if (map) - bf_map_dump(map, prefix); - else - DUMP(prefix, ""); - } + bf_handle_dump(program->handle, bf_dump_prefix_last(prefix)); bf_dump_prefix_pop(prefix); - if (program->link) { - DUMP(prefix, "link: struct bf_link *"); - bf_dump_prefix_push(prefix); - bf_link_dump(program->link, prefix); - bf_dump_prefix_pop(prefix); - } else { - DUMP(prefix, "link: struct bf_link * (NULL)"); - } - DUMP(prefix, "printer: struct bf_printer *"); bf_dump_prefix_push(prefix); bf_printer_dump(program->printer, prefix); @@ -383,7 +245,6 @@ void bf_program_dump(const struct bf_program *program, prefix_t *prefix) DUMP(bf_dump_prefix_last(prefix), "runtime: "); bf_dump_prefix_push(prefix); - DUMP(prefix, "prog_fd: %d", program->runtime.prog_fd); DUMP(bf_dump_prefix_last(prefix), "ops: %p", program->runtime.ops); bf_dump_prefix_pop(prefix); @@ -456,18 +317,18 @@ static int _bf_program_fixup(struct bf_program *program, break; case BF_FIXUP_TYPE_COUNTERS_MAP_FD: insn_type = BF_FIXUP_INSN_IMM; - value = program->cmap->fd; + value = program->handle->cmap->fd; break; case BF_FIXUP_TYPE_PRINTER_MAP_FD: insn_type = BF_FIXUP_INSN_IMM; - value = program->pmap->fd; + value = program->handle->pmap->fd; break; case BF_FIXUP_TYPE_LOG_MAP_FD: insn_type = BF_FIXUP_INSN_IMM; - value = program->lmap->fd; + value = program->handle->lmap->fd; break; case BF_FIXUP_TYPE_SET_MAP_FD: - map = bf_list_get_at(&program->sets, fixup->attr.set_index); + map = bf_list_get_at(&program->handle->sets, fixup->attr.set_index); if (!map) { return bf_err_r(-ENOENT, "can't find set map at index %lu", fixup->attr.set_index); @@ -898,12 +759,12 @@ static int _bf_program_load_printer_map(struct bf_program *program) if (r) return bf_err_r(r, "failed to assemble printer map string"); - r = bf_map_new(&program->pmap, "printer_map", BF_MAP_TYPE_PRINTER, + r = bf_map_new(&program->handle->pmap, "printer_map", BF_MAP_TYPE_PRINTER, sizeof(uint32_t), pstr_len, 1); if (r) return bf_err_r(r, "failed to create the printer bf_map object"); - r = bf_map_set_elem(program->pmap, &key, pstr); + r = bf_map_set_elem(program->handle->pmap, &key, pstr); if (r) return bf_err_r(r, "failed to set print map elem"); @@ -916,12 +777,11 @@ static int _bf_program_load_printer_map(struct bf_program *program) static int _bf_program_load_counters_map(struct bf_program *program) { - _cleanup_close_ int _fd = -1; int r; assert(program); - r = bf_map_new(&program->cmap, "counters_map", BF_MAP_TYPE_COUNTERS, + r = bf_map_new(&program->handle->cmap, "counters_map", BF_MAP_TYPE_COUNTERS, sizeof(uint32_t), sizeof(struct bf_counter), bf_list_size(&program->runtime.chain->rules) + 2); if (r) @@ -936,7 +796,6 @@ static int _bf_program_load_counters_map(struct bf_program *program) static int _bf_program_load_log_map(struct bf_program *program) { - _cleanup_close_ int _fd = -1; int r; assert(program); @@ -945,7 +804,7 @@ static int _bf_program_load_log_map(struct bf_program *program) if (!(program->runtime.chain->flags & BF_FLAG(BF_CHAIN_LOG))) return 0; - r = bf_map_new(&program->lmap, "log_map", BF_MAP_TYPE_LOG, 0, 0, + r = bf_map_new(&program->handle->lmap, "log_map", BF_MAP_TYPE_LOG, 0, 0, _BF_LOG_MAP_SIZE); if (r) return bf_err_r(r, "failed to create the log bf_map object"); @@ -1001,7 +860,7 @@ static int _bf_program_load_sets_maps(struct bf_program *new_prog) if (r) return bf_err_r(r, "failed to add set elements to the map"); - r = bf_list_push(&new_prog->sets, (void **)&map); + r = bf_list_push(&new_prog->handle->sets, (void **)&map); if (r) return r; }; @@ -1051,7 +910,7 @@ int bf_program_load(struct bf_program *prog) prog->prog_name, bf_hook_to_bpf_prog_type(prog->runtime.chain->hook), prog->img, prog->img_size, bf_hook_to_bpf_attach_type(prog->runtime.chain->hook), log_buf, - log_buf ? _BF_LOG_BUF_SIZE : 0, bf_ctx_token(), &prog->runtime.prog_fd); + log_buf ? _BF_LOG_BUF_SIZE : 0, bf_ctx_token(), &prog->handle->prog_fd); if (r) { return bf_err_r(r, "failed to load bf_program (%lu bytes):\n%s\nerrno:", prog->img_size, log_buf ? log_buf : ""); @@ -1067,8 +926,8 @@ int bf_program_attach(struct bf_program *prog, struct bf_hookopts **hookopts) assert(prog); assert(hookopts); - r = bf_link_new(&prog->link, prog->runtime.chain->hook, hookopts, - prog->runtime.prog_fd); + r = bf_link_new(&prog->handle->link, prog->runtime.chain->hook, hookopts, + prog->handle->prog_fd); if (r) { return bf_err_r(r, "failed to attach bf_link for %s program", bf_flavor_to_str(prog->flavor)); @@ -1081,21 +940,21 @@ void bf_program_detach(struct bf_program *prog) { assert(prog); - bf_link_free(&prog->link); + bf_link_free(&prog->handle->link); } void bf_program_unload(struct bf_program *prog) { - _clean_bf_list_ bf_list list = bf_list_default_from(prog->sets); + assert(prog); + bf_handle_unload(prog->handle); +} + +struct bf_handle *bf_program_take_handle(struct bf_program *prog) +{ assert(prog); - closep(&prog->runtime.prog_fd); - bf_link_free(&prog->link); - bf_map_free(&prog->cmap); - bf_map_free(&prog->pmap); - bf_map_free(&prog->lmap); - bf_swap(list, prog->sets); + return TAKE_PTR(prog->handle); } int bf_program_get_counter(const struct bf_program *program, @@ -1104,13 +963,7 @@ int bf_program_get_counter(const struct bf_program *program, assert(program); assert(counter); - int r; - - r = bf_bpf_map_lookup_elem(program->cmap->fd, &counter_idx, counter); - if (r < 0) - return bf_err_r(errno, "failed to lookup counters map"); - - return 0; + return bf_handle_get_counter(program->handle, counter_idx, counter); } int bf_cgen_set_counters(struct bf_program *program, @@ -1124,92 +977,16 @@ int bf_cgen_set_counters(struct bf_program *program, int bf_program_pin(struct bf_program *prog, int dir_fd) { - const char *name; - int r; - assert(prog); - name = prog->runtime.chain->name; - - r = bf_bpf_obj_pin(prog->prog_name, prog->runtime.prog_fd, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF program for '%s'", name); - goto err_unpin_all; - } - - if (prog->cmap) { - r = bf_map_pin(prog->cmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF counters map for '%s'", name); - goto err_unpin_all; - } - } - - if (prog->pmap) { - r = bf_map_pin(prog->pmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF printer map for '%s'", name); - goto err_unpin_all; - } - } - - if (prog->lmap) { - r = bf_map_pin(prog->lmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF log map for '%s'", name); - goto err_unpin_all; - } - } - - bf_list_foreach (&prog->sets, set_node) { - struct bf_map *map = bf_list_node_get_data(set_node); - if (!map) - continue; - - r = bf_map_pin(map, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF set map for '%s'", name); - goto err_unpin_all; - } - } - - // If a link exists, pin it too. - if (prog->link) { - r = bf_link_pin(prog->link, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF link for '%s'", name); - goto err_unpin_all; - } - } - - return 0; - -err_unpin_all: - bf_program_unpin(prog, dir_fd); - return r; + return bf_handle_pin(prog->handle, dir_fd); } void bf_program_unpin(struct bf_program *prog, int dir_fd) { assert(prog); - if (prog->cmap) - bf_map_unpin(prog->cmap, dir_fd); - if (prog->pmap) - bf_map_unpin(prog->pmap, dir_fd); - if (prog->lmap) - bf_map_unpin(prog->lmap, dir_fd); - - bf_list_foreach (&prog->sets, set_node) { - struct bf_map *map = bf_list_node_get_data(set_node); - if (map) - bf_map_unpin(map, dir_fd); - } - - if (prog->link) - bf_link_unpin(prog->link, dir_fd); - - unlinkat(dir_fd, prog->prog_name, 0); + bf_handle_unpin(prog->handle, dir_fd); } size_t bf_program_chain_counter_idx(const struct bf_program *program) diff --git a/src/bpfilter/cgen/program.h b/src/bpfilter/cgen/program.h index c18e9ef0..03447779 100644 --- a/src/bpfilter/cgen/program.h +++ b/src/bpfilter/cgen/program.h @@ -185,10 +185,9 @@ }) struct bf_chain; -struct bf_map; struct bf_counter; -struct bf_link; struct bf_hookopts; +struct bf_handle; struct bf_program { @@ -198,19 +197,10 @@ struct bf_program /// Log messages printer struct bf_printer *printer; - /// Counters map - struct bf_map *cmap; - /// Printer map - struct bf_map *pmap; - /// Log map - struct bf_map *lmap; - /// List of set maps. Entries can be NULL for empty sets. - bf_list sets; - - /** Link objects attaching the program to a hook. - * @todo A ``bf_program`` should not have any link until the program is - * attached. */ - struct bf_link *link; + /** Handle containing BPF object references (prog_fd, maps, link). + * Created in bf_program_new(), populated during load/attach. + * Can be transferred to bf_cgen via bf_program_take_handle(). */ + struct bf_handle *handle; /* Bytecode */ uint32_t elfstubs_location[_BF_ELFSTUB_MAX]; @@ -223,13 +213,6 @@ struct bf_program * This data is not serialized. */ struct { - /** File descriptor of the program. */ - int prog_fd; - - /** File descriptor of the directory to pin the program into. Unused - * in transient mode. */ - int pindir_fd; - /** Hook-specific ops to use to generate the program. */ const struct bf_flavor_ops *ops; @@ -369,6 +352,18 @@ void bf_program_detach(struct bf_program *prog); */ void bf_program_unload(struct bf_program *prog); +/** + * @brief Transfer ownership of the handle from the program. + * + * After this call, the program no longer owns the handle and the caller + * becomes responsible for freeing it. The program's handle pointer is set + * to NULL. + * + * @param prog Program to take the handle from. Can't be NULL. + * @return The handle, or NULL if the program has no handle. + */ +struct bf_handle *bf_program_take_handle(struct bf_program *prog); + int bf_program_get_counter(const struct bf_program *program, uint32_t counter_idx, struct bf_counter *counter); int bf_program_set_counters(struct bf_program *program, diff --git a/src/bpfilter/xlate/cli.c b/src/bpfilter/xlate/cli.c index 7a99b819..f45111a8 100644 --- a/src/bpfilter/xlate/cli.c +++ b/src/bpfilter/xlate/cli.c @@ -19,6 +19,7 @@ #include "bpfilter/set.h" #include "cgen/cgen.h" +#include "cgen/handle.h" #include "cgen/prog/link.h" #include "cgen/prog/map.h" #include "cgen/program.h" @@ -90,9 +91,10 @@ static int _bf_cli_ruleset_get(const struct bf_request *request, if (r) return bf_err_r(r, "failed to add chain to list"); - r = bf_list_add_tail(&hookopts, cgen->program->link ? - cgen->program->link->hookopts : - NULL); + r = bf_list_add_tail(&hookopts, + cgen->program->handle->link ? + cgen->program->handle->link->hookopts : + NULL); if (r) return bf_err_r(r, "failed to add hookopts to list"); @@ -299,9 +301,9 @@ static int _bf_cli_chain_get(const struct bf_request *request, return r; bf_wpack_close_object(wpack); - if (cgen->program->link) { + if (cgen->program->handle->link) { bf_wpack_open_object(wpack, "hookopts"); - r = bf_hookopts_pack(cgen->program->link->hookopts, wpack); + r = bf_hookopts_pack(cgen->program->handle->link->hookopts, wpack); if (r) return r; bf_wpack_close_object(wpack); @@ -337,10 +339,10 @@ int _bf_cli_chain_prog_fd(const struct bf_request *request, if (!cgen) return bf_err_r(-ENOENT, "failed to find chain '%s'", name); - if (!cgen->program || cgen->program->runtime.prog_fd == -1) + if (!cgen->program || cgen->program->handle->prog_fd == -1) return bf_err_r(-ENODEV, "chain '%s' has no loaded program", name); - r = bf_send_fd(bf_request_fd(request), cgen->program->runtime.prog_fd); + r = bf_send_fd(bf_request_fd(request), cgen->program->handle->prog_fd); if (r < 0) return bf_err_r(errno, "failed to send prog FD for '%s'", name); @@ -370,10 +372,10 @@ int _bf_cli_chain_logs_fd(const struct bf_request *request, if (!cgen) return bf_err_r(-ENOENT, "failed to find chain '%s'", name); - if (!cgen->program || !cgen->program->lmap->fd) + if (!cgen->program || !cgen->program->handle->lmap) return bf_err_r(-ENOENT, "chain '%s' has no logs buffer", name); - r = bf_send_fd(bf_request_fd(request), cgen->program->lmap->fd); + r = bf_send_fd(bf_request_fd(request), cgen->program->handle->lmap->fd); if (r < 0) return bf_err_r(errno, "failed to send logs FD for '%s'", name); @@ -465,7 +467,7 @@ int _bf_cli_chain_attach(const struct bf_request *request, cgen = bf_ctx_get_cgen(name); if (!cgen) return bf_err_r(-ENOENT, "chain '%s' does not exist", name); - if (cgen->program->link) + if (cgen->program->handle->link) return bf_err_r(-EBUSY, "chain '%s' is already linked to a hook", name); r = bf_hookopts_validate(hookopts, cgen->chain->hook); From 0a6b76878aab19373d6f503fdcf38eaf2f3e8be4 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 10:17:52 +0100 Subject: [PATCH 06/12] daemon: cgen: remove bf_program in favor of bf_handle Replace bf_program in bf_cgen with bf_handle. The bytecode generation context (bf_program) don't need to be stored anymore, as we can keep a reference to the BPF objects. --- src/bpfilter/cgen/cgen.c | 87 ++++++++++++++++++++----------------- src/bpfilter/cgen/cgen.h | 16 +++---- src/bpfilter/cgen/program.c | 45 ------------------- src/bpfilter/cgen/program.h | 57 ------------------------ src/bpfilter/xlate/cli.c | 21 +++++---- 5 files changed, 62 insertions(+), 164 deletions(-) diff --git a/src/bpfilter/cgen/cgen.c b/src/bpfilter/cgen/cgen.c index a935b2f9..da760891 100644 --- a/src/bpfilter/cgen/cgen.c +++ b/src/bpfilter/cgen/cgen.c @@ -60,7 +60,7 @@ int bf_cgen_new(struct bf_cgen **cgen, enum bf_front front, return -ENOMEM; (*cgen)->front = front; - (*cgen)->program = NULL; + (*cgen)->handle = NULL; (*cgen)->chain = NULL; (*cgen)->chain = TAKE_PTR(*chain); @@ -79,7 +79,7 @@ int bf_cgen_new_from_pack(struct bf_cgen **cgen, bf_rpack_node_t node) if (!_cgen) return -ENOMEM; - _cgen->program = NULL; + _cgen->handle = NULL; r = bf_rpack_kv_enum(node, "front", &_cgen->front, 0, _BF_FRONT_MAX); if (r) @@ -93,9 +93,9 @@ int bf_cgen_new_from_pack(struct bf_cgen **cgen, bf_rpack_node_t node) if (r) return bf_rpack_key_err(r, "bf_cgen.chain"); - r = bf_rpack_kv_node(node, "program", &child); + r = bf_rpack_kv_node(node, "handle", &child); if (r) - return bf_rpack_key_err(r, "bf_cgen.program"); + return bf_rpack_key_err(r, "bf_cgen.handle"); if (!bf_rpack_is_nil(child)) { _cleanup_close_ int dir_fd = -1; @@ -105,8 +105,7 @@ int bf_cgen_new_from_pack(struct bf_cgen **cgen, bf_rpack_node_t node) _cgen->chain->name); } - r = bf_program_new_from_pack(&_cgen->program, _cgen->chain, dir_fd, - child); + r = bf_handle_new_from_pack(&_cgen->handle, dir_fd, child); if (r) return r; } @@ -132,7 +131,7 @@ void bf_cgen_free(struct bf_cgen **cgen) if (bf_opts_persist() && (pin_fd = bf_ctx_get_pindir_fd()) >= 0) bf_rmdir_at(pin_fd, (*cgen)->chain->name, false); - bf_program_free(&(*cgen)->program); + bf_handle_free(&(*cgen)->handle); bf_chain_free(&(*cgen)->chain); free(*cgen); @@ -150,12 +149,12 @@ int bf_cgen_pack(const struct bf_cgen *cgen, bf_wpack_t *pack) bf_chain_pack(cgen->chain, pack); bf_wpack_close_object(pack); - if (cgen->program) { - bf_wpack_open_object(pack, "program"); - bf_program_pack(cgen->program, pack); + if (cgen->handle) { + bf_wpack_open_object(pack, "handle"); + bf_handle_pack(cgen->handle, pack); bf_wpack_close_object(pack); } else { - bf_wpack_kv_nil(pack, "program"); + bf_wpack_kv_nil(pack, "handle"); } return bf_wpack_is_valid(pack) ? 0 : -EINVAL; @@ -177,14 +176,13 @@ void bf_cgen_dump(const struct bf_cgen *cgen, prefix_t *prefix) bf_chain_dump(cgen->chain, bf_dump_prefix_last(prefix)); bf_dump_prefix_pop(prefix); - // Programs - if (cgen->program) { - DUMP(bf_dump_prefix_last(prefix), "program: struct bf_program *"); + if (cgen->handle) { + DUMP(bf_dump_prefix_last(prefix), "handle: struct bf_handle *"); bf_dump_prefix_push(prefix); - bf_program_dump(cgen->program, bf_dump_prefix_last(prefix)); + bf_handle_dump(cgen->handle, bf_dump_prefix_last(prefix)); bf_dump_prefix_pop(prefix); } else { - DUMP(bf_dump_prefix_last(prefix), "program: (struct bf_program *)NULL"); + DUMP(bf_dump_prefix_last(prefix), "handle: (struct bf_handle *)NULL"); } bf_dump_prefix_pop(prefix); @@ -208,13 +206,14 @@ int bf_cgen_get_counter(const struct bf_cgen *cgen, return -EINVAL; } - return bf_program_get_counter(cgen->program, counter_idx, counter); + return bf_handle_get_counter(cgen->handle, counter_idx, counter); } int bf_cgen_set(struct bf_cgen *cgen, const struct bf_ns *ns, struct bf_hookopts **hookopts) { _free_bf_program_ struct bf_program *prog = NULL; + _free_bf_handle_ struct bf_handle *handle = NULL; _cleanup_close_ int pindir_fd = -1; int r; @@ -238,12 +237,14 @@ int bf_cgen_set(struct bf_cgen *cgen, const struct bf_ns *ns, if (r < 0) return bf_err_r(r, "failed to load the chain"); + handle = bf_program_take_handle(prog); + if (hookopts) { r = bf_ns_set(ns, bf_ctx_get_ns()); if (r) return bf_err_r(r, "failed to switch to the client's namespaces"); - r = bf_program_attach(prog, hookopts); + r = bf_handle_attach(handle, cgen->chain->hook, hookopts); if (r < 0) return bf_err_r(r, "failed to load and attach the chain"); @@ -252,12 +253,12 @@ int bf_cgen_set(struct bf_cgen *cgen, const struct bf_ns *ns, } if (bf_opts_persist()) { - r = bf_program_pin(prog, pindir_fd); + r = bf_handle_pin(handle, pindir_fd); if (r) return r; } - cgen->program = TAKE_PTR(prog); + cgen->handle = TAKE_PTR(handle); return r; } @@ -265,6 +266,7 @@ int bf_cgen_set(struct bf_cgen *cgen, const struct bf_ns *ns, int bf_cgen_load(struct bf_cgen *cgen) { _free_bf_program_ struct bf_program *prog = NULL; + _free_bf_handle_ struct bf_handle *handle = NULL; _cleanup_close_ int pindir_fd = -1; int r; @@ -288,8 +290,10 @@ int bf_cgen_load(struct bf_cgen *cgen) if (r < 0) return bf_err_r(r, "failed to load the chain"); + handle = bf_program_take_handle(prog); + if (bf_opts_persist()) { - r = bf_program_pin(prog, pindir_fd); + r = bf_handle_pin(handle, pindir_fd); if (r) return r; } @@ -297,7 +301,7 @@ int bf_cgen_load(struct bf_cgen *cgen) bf_info("load %s", cgen->chain->name); bf_cgen_dump(cgen, EMPTY_PREFIX); - cgen->program = TAKE_PTR(prog); + cgen->handle = TAKE_PTR(handle); return r; } @@ -326,7 +330,7 @@ int bf_cgen_attach(struct bf_cgen *cgen, const struct bf_ns *ns, if (r) return bf_err_r(r, "failed to switch to the client's namespaces"); - r = bf_program_attach(cgen->program, hookopts); + r = bf_handle_attach(cgen->handle, cgen->chain->hook, hookopts); if (r < 0) return bf_err_r(r, "failed to attach chain '%s'", cgen->chain->name); @@ -334,9 +338,9 @@ int bf_cgen_attach(struct bf_cgen *cgen, const struct bf_ns *ns, bf_abort("failed to restore previous namespaces, aborting"); if (bf_opts_persist()) { - r = bf_link_pin(cgen->program->handle->link, pindir_fd); + r = bf_link_pin(cgen->handle->link, pindir_fd); if (r) { - bf_program_detach(cgen->program); + bf_handle_detach(cgen->handle); return r; } } @@ -347,14 +351,15 @@ int bf_cgen_attach(struct bf_cgen *cgen, const struct bf_ns *ns, int bf_cgen_update(struct bf_cgen *cgen, struct bf_chain **new_chain) { _free_bf_program_ struct bf_program *new_prog = NULL; + _free_bf_handle_ struct bf_handle *new_handle = NULL; _cleanup_close_ int pindir_fd = -1; - struct bf_program *old_prog; + struct bf_handle *old_handle; int r; assert(cgen); assert(new_chain); - old_prog = cgen->program; + old_handle = cgen->handle; if (bf_opts_persist()) { pindir_fd = _bf_cgen_get_chain_pindir_fd((*new_chain)->name); @@ -376,29 +381,31 @@ int bf_cgen_update(struct bf_cgen *cgen, struct bf_chain **new_chain) if (r) return bf_err_r(r, "failed to load new program"); + new_handle = bf_program_take_handle(new_prog); + if (bf_opts_persist()) - bf_program_unpin(old_prog, pindir_fd); + bf_handle_unpin(old_handle, pindir_fd); - if (old_prog->handle->link) { - r = bf_link_update(old_prog->handle->link, new_prog->handle->prog_fd); + if (old_handle->link) { + r = bf_link_update(old_handle->link, new_handle->prog_fd); if (r) { bf_err_r(r, "failed to update bf_link object with new program"); - if (bf_opts_persist() && bf_program_pin(old_prog, pindir_fd) < 0) - bf_err("failed to repin old program, ignoring"); + if (bf_opts_persist() && bf_handle_pin(old_handle, pindir_fd) < 0) + bf_err("failed to repin old handle, ignoring"); return r; } - // We updated the old link, we need to store it in the new program - bf_swap(new_prog->handle->link, old_prog->handle->link); + // We updated the old link, we need to store it in the new handle + bf_swap(new_handle->link, old_handle->link); } if (bf_opts_persist()) { - r = bf_program_pin(new_prog, pindir_fd); + r = bf_handle_pin(new_handle, pindir_fd); if (r) - bf_warn_r(r, "failed to pin new prog, ignoring"); + bf_warn_r(r, "failed to pin new handle, ignoring"); } - bf_swap(cgen->program, new_prog); + bf_swap(cgen->handle, new_handle); bf_chain_free(&cgen->chain); cgen->chain = TAKE_PTR(*new_chain); @@ -410,7 +417,7 @@ void bf_cgen_detach(struct bf_cgen *cgen) { assert(cgen); - bf_program_detach(cgen->program); + bf_handle_detach(cgen->handle); } void bf_cgen_unload(struct bf_cgen *cgen) @@ -427,8 +434,8 @@ void bf_cgen_unload(struct bf_cgen *cgen) } // The chain's pin directory will be removed in bf_cgen_free() - bf_program_unpin(cgen->program, chain_fd); - bf_program_unload(cgen->program); + bf_handle_unpin(cgen->handle, chain_fd); + bf_handle_unload(cgen->handle); } int bf_cgen_get_counters(const struct bf_cgen *cgen, bf_list *counters) diff --git a/src/bpfilter/cgen/cgen.h b/src/bpfilter/cgen/cgen.h index c4467860..90886808 100644 --- a/src/bpfilter/cgen/cgen.h +++ b/src/bpfilter/cgen/cgen.h @@ -14,7 +14,7 @@ #include struct bf_chain; -struct bf_program; +struct bf_handle; struct bf_ns; struct bf_hookopts; @@ -34,12 +34,10 @@ struct bf_cgen /// Chain containing the rules, sets, and policy. struct bf_chain *chain; - /** Program generated by the codegen. - * @todo The codegen should not store the program: it creates a - * ``bf_program`` to generate the bytecode, attach it, and only keep the - * program's FD (and maps). No need to keep the bytecode, nor - * (de)serializing it. */ - struct bf_program *program; + /** Handle containing BPF object references (prog_fd, maps, link). + * Created during bf_cgen_set/load when the program is generated. The + * program is discarded after generation, only the handle is kept. */ + struct bf_handle *handle; }; /** @@ -94,10 +92,6 @@ int bf_cgen_pack(const struct bf_cgen *cgen, bf_wpack_t *pack); * Generate and load a new chain. Attach the chain if `hookopts` is not NULL. It * is assumed that no chain with the same name exist. * - * @todo `bf_cgen` should not store a `bf_program` object pointer, it should - * create a new program during generation, use it, then discard it. The program - * is not needed once loaded, it should be considered as a temporary artifact. - * * @param cgen Codegen to attach to the kernel. Can't be NULL. * @param ns Namespaces to switch to before attaching the programs. Can't be NULL. * @param hookopts Hook options. diff --git a/src/bpfilter/cgen/program.c b/src/bpfilter/cgen/program.c index f2d32880..4203aed5 100644 --- a/src/bpfilter/cgen/program.c +++ b/src/bpfilter/cgen/program.c @@ -919,37 +919,6 @@ int bf_program_load(struct bf_program *prog) return r; } -int bf_program_attach(struct bf_program *prog, struct bf_hookopts **hookopts) -{ - int r; - - assert(prog); - assert(hookopts); - - r = bf_link_new(&prog->handle->link, prog->runtime.chain->hook, hookopts, - prog->handle->prog_fd); - if (r) { - return bf_err_r(r, "failed to attach bf_link for %s program", - bf_flavor_to_str(prog->flavor)); - } - - return r; -} - -void bf_program_detach(struct bf_program *prog) -{ - assert(prog); - - bf_link_free(&prog->handle->link); -} - -void bf_program_unload(struct bf_program *prog) -{ - assert(prog); - - bf_handle_unload(prog->handle); -} - struct bf_handle *bf_program_take_handle(struct bf_program *prog) { assert(prog); @@ -975,20 +944,6 @@ int bf_cgen_set_counters(struct bf_program *program, return -ENOTSUP; } -int bf_program_pin(struct bf_program *prog, int dir_fd) -{ - assert(prog); - - return bf_handle_pin(prog->handle, dir_fd); -} - -void bf_program_unpin(struct bf_program *prog, int dir_fd) -{ - assert(prog); - - bf_handle_unpin(prog->handle, dir_fd); -} - size_t bf_program_chain_counter_idx(const struct bf_program *program) { return bf_list_size(&program->runtime.chain->rules); diff --git a/src/bpfilter/cgen/program.h b/src/bpfilter/cgen/program.h index 03447779..fe1954fb 100644 --- a/src/bpfilter/cgen/program.h +++ b/src/bpfilter/cgen/program.h @@ -295,63 +295,6 @@ int bf_program_generate(struct bf_program *program); */ int bf_program_load(struct bf_program *prog); -/** - * Attach a loaded program to a hook. - * - * @warning If the program hasn't been loaded (using `bf_program_load`), - * `bf_program_attach` will fail. - * - * The program is attached to a hook using a `bf_link` object. In persistent - * mode, the link will be pinned to the filesystem. If the link can't be pinned, - * the program will be detached from the hook. - * - * @param prog Program to attach. Can't be NULL. - * @param hookopts Hook-specific options to attach the program to the hook. - * Can't be NULL. - * @return 0 on success, or negative errno value on failure. - */ -int bf_program_attach(struct bf_program *prog, struct bf_hookopts **hookopts); - -/** - * @brief Pin the BPF program. - * - * The program and all the BPF objects it uses will be pinned into `dir_fd`. - * The BPF link is only pinned if the program is attached to a hook. - * - * @param prog Program to pin. Can't be NULL. - * @param dir_fd File descriptor of the directory to pin the program and its - * BPF objects into. - * @return 0 on success, or a negative errno value on error. - */ -int bf_program_pin(struct bf_program *prog, int dir_fd); - -/** - * @brief Unpin the BPF program. - * - * This function never fails. If the program is not pinned, no file will be - * removed. - * - * @param prog Program to unpin. Can't be NULL. - * @param dir_fd File descriptor of the directory containing the pinned objects. - */ -void bf_program_unpin(struct bf_program *prog, int dir_fd); - -/** - * Detach the program from the kernel. - * - * The program is detached but not unloaded. - * - * @param prog Program to detach. Can't be NULL. - */ -void bf_program_detach(struct bf_program *prog); - -/** - * Unload the program. - * - * @param prog Program to unload. Must not be attached. Can't be NULL. - */ -void bf_program_unload(struct bf_program *prog); - /** * @brief Transfer ownership of the handle from the program. * diff --git a/src/bpfilter/xlate/cli.c b/src/bpfilter/xlate/cli.c index f45111a8..67997e5f 100644 --- a/src/bpfilter/xlate/cli.c +++ b/src/bpfilter/xlate/cli.c @@ -91,10 +91,9 @@ static int _bf_cli_ruleset_get(const struct bf_request *request, if (r) return bf_err_r(r, "failed to add chain to list"); - r = bf_list_add_tail(&hookopts, - cgen->program->handle->link ? - cgen->program->handle->link->hookopts : - NULL); + r = bf_list_add_tail(&hookopts, cgen->handle->link ? + cgen->handle->link->hookopts : + NULL); if (r) return bf_err_r(r, "failed to add hookopts to list"); @@ -301,9 +300,9 @@ static int _bf_cli_chain_get(const struct bf_request *request, return r; bf_wpack_close_object(wpack); - if (cgen->program->handle->link) { + if (cgen->handle->link) { bf_wpack_open_object(wpack, "hookopts"); - r = bf_hookopts_pack(cgen->program->handle->link->hookopts, wpack); + r = bf_hookopts_pack(cgen->handle->link->hookopts, wpack); if (r) return r; bf_wpack_close_object(wpack); @@ -339,10 +338,10 @@ int _bf_cli_chain_prog_fd(const struct bf_request *request, if (!cgen) return bf_err_r(-ENOENT, "failed to find chain '%s'", name); - if (!cgen->program || cgen->program->handle->prog_fd == -1) + if (cgen->handle->prog_fd == -1) return bf_err_r(-ENODEV, "chain '%s' has no loaded program", name); - r = bf_send_fd(bf_request_fd(request), cgen->program->handle->prog_fd); + r = bf_send_fd(bf_request_fd(request), cgen->handle->prog_fd); if (r < 0) return bf_err_r(errno, "failed to send prog FD for '%s'", name); @@ -372,10 +371,10 @@ int _bf_cli_chain_logs_fd(const struct bf_request *request, if (!cgen) return bf_err_r(-ENOENT, "failed to find chain '%s'", name); - if (!cgen->program || !cgen->program->handle->lmap) + if (!cgen->handle->lmap) return bf_err_r(-ENOENT, "chain '%s' has no logs buffer", name); - r = bf_send_fd(bf_request_fd(request), cgen->program->handle->lmap->fd); + r = bf_send_fd(bf_request_fd(request), cgen->handle->lmap->fd); if (r < 0) return bf_err_r(errno, "failed to send logs FD for '%s'", name); @@ -467,7 +466,7 @@ int _bf_cli_chain_attach(const struct bf_request *request, cgen = bf_ctx_get_cgen(name); if (!cgen) return bf_err_r(-ENOENT, "chain '%s' does not exist", name); - if (cgen->program->handle->link) + if (cgen->handle->link) return bf_err_r(-EBUSY, "chain '%s' is already linked to a hook", name); r = bf_hookopts_validate(hookopts, cgen->chain->hook); From 86884e50547ff6464d1f5e75193235d399d35287 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Wed, 11 Feb 2026 14:49:34 +0100 Subject: [PATCH 07/12] lib: bpf: require BTF data len on bf_bpf_btf_load() --- src/bpfilter/cgen/prog/map.c | 2 +- src/libbpfilter/bpf.c | 3 ++- src/libbpfilter/include/bpfilter/bpf.h | 3 ++- tests/unit/libbpfilter/bpf.c | 4 ++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/bpfilter/cgen/prog/map.c b/src/bpfilter/cgen/prog/map.c index 0a6dc216..bde5695e 100644 --- a/src/bpfilter/cgen/prog/map.c +++ b/src/bpfilter/cgen/prog/map.c @@ -73,7 +73,7 @@ static int _bf_btf_load(struct bf_btf *btf) if (!raw) return bf_err_r(errno, "failed to request BTF raw data"); - r = bf_bpf_btf_load(raw, bf_ctx_token()); + r = bf_bpf_btf_load(raw, attr.btf_size, bf_ctx_token()); if (r < 0) return r; diff --git a/src/libbpfilter/bpf.c b/src/libbpfilter/bpf.c index a0489b67..c14c039b 100644 --- a/src/libbpfilter/bpf.c +++ b/src/libbpfilter/bpf.c @@ -176,7 +176,7 @@ int bf_bpf_token_create(int bpffs_fd) return bf_bpf(BF_BPF_TOKEN_CREATE, &attr); } -int bf_bpf_btf_load(const void *btf_data, int token_fd) +int bf_bpf_btf_load(const void *btf_data, size_t btf_data_len, int token_fd) { assert(btf_data); @@ -185,6 +185,7 @@ int bf_bpf_btf_load(const void *btf_data, int token_fd) memset(&attr, 0, sizeof(attr)); attr.btf = bf_ptr_to_u64(btf_data); + attr.btf_size = btf_data_len; if (token_fd != -1) { attr.btf_token_fd = token_fd; diff --git a/src/libbpfilter/include/bpfilter/bpf.h b/src/libbpfilter/include/bpfilter/bpf.h index 7f7761ea..7f9ff8fd 100644 --- a/src/libbpfilter/include/bpfilter/bpf.h +++ b/src/libbpfilter/include/bpfilter/bpf.h @@ -111,12 +111,13 @@ int bf_bpf_token_create(int bpffs_fd); * @brief Load BTF data into the kernel. * * @param btf_data Raw BTF data to send to the kernel. Can't be NULL. + * @param btf_data_len Length of `btf_data`. * @param token_fd File descriptor of the BPF token to use, or -1 if no token * should be used. * @return A valid BTF file descriptor on success (owned by the caller), * or a negative error value on failure. */ -int bf_bpf_btf_load(const void *btf_data, int token_fd); +int bf_bpf_btf_load(const void *btf_data, size_t btf_data_len, int token_fd); /** * @brief Create a new BPF map. diff --git a/tests/unit/libbpfilter/bpf.c b/tests/unit/libbpfilter/bpf.c index ddb15be7..8a8bf5b4 100644 --- a/tests/unit/libbpfilter/bpf.c +++ b/tests/unit/libbpfilter/bpf.c @@ -289,7 +289,7 @@ static void bpf_btf_load_success(void **state) (void)mock; bft_mock_syscall_set_retval(25); - r = bf_bpf_btf_load(btf_data, -1); + r = bf_bpf_btf_load(btf_data, 1, -1); assert_int_equal(r, 25); } @@ -304,7 +304,7 @@ static void bpf_btf_load_with_token(void **state) (void)mock; bft_mock_syscall_set_retval(26); - r = bf_bpf_btf_load(btf_data, 5); + r = bf_bpf_btf_load(btf_data, 1, 5); assert_int_equal(r, 26); } From bb2bf932d99fecd91b584d8e2dae51b212a75348 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 10:26:18 +0100 Subject: [PATCH 08/12] daemon: cgen: tag maps with BTF decl tags for identification Refactor BTF data handling to always create BTF data for BPF maps. The main purpose is to define a decl tag defining the map's bpfilter type. --- src/bpfilter/cgen/prog/map.c | 208 ++++++++++++++----------- src/libbpfilter/include/bpfilter/btf.h | 4 +- 2 files changed, 115 insertions(+), 97 deletions(-) diff --git a/src/bpfilter/cgen/prog/map.c b/src/bpfilter/cgen/prog/map.c index bde5695e..9ed512c4 100644 --- a/src/bpfilter/cgen/prog/map.c +++ b/src/bpfilter/cgen/prog/map.c @@ -24,17 +24,70 @@ #include "ctx.h" +static const char *_bf_map_type_strs[] = { + [BF_MAP_TYPE_COUNTERS] = "BF_MAP_TYPE_COUNTERS", + [BF_MAP_TYPE_PRINTER] = "BF_MAP_TYPE_PRINTER", + [BF_MAP_TYPE_LOG] = "BF_MAP_TYPE_LOG", + [BF_MAP_TYPE_SET] = "BF_MAP_TYPE_SET", +}; +static_assert(ARRAY_SIZE(_bf_map_type_strs) == _BF_MAP_TYPE_MAX, + "missing entries in _bf_map_type_strs array"); + +static const char *_bf_map_type_to_str(enum bf_map_type type) +{ + if (type < 0 || _BF_MAP_TYPE_MAX <= type) + return NULL; + + return _bf_map_type_strs[type]; +} + +static int _bf_map_type_from_str(const char *str, enum bf_map_type *type) +{ + assert(type); + + for (enum bf_map_type i = 0; i < _BF_MAP_TYPE_MAX; ++i) { + if (bf_streq(_bf_map_type_strs[i], str)) { + *type = i; + return 0; + } + } + + return -EINVAL; +} + #define _free_bf_btf_ __attribute__((__cleanup__(_bf_btf_free))) static void _bf_btf_free(struct bf_btf **btf); -static int _bf_btf_new(struct bf_btf **btf) +/** + * @brief Create BTF data for a map. + * + * The BTF data is necessary to identify maps when creating a `bf_map` from + * its file descriptor. While most types defined will help `bpftool` dumping + * the map content and pretty-print it (e.g. counters map), the most important + * part of the BTF data is the decl tag (`btf__add_decl_tag`), as it tags the + * map's value for bpfilter to recognize it. + * + * BPF ring buffer maps do not support BTF data, so we will only rely on the + * map type for now. + * + * @param btf BTF data to create. On success, `*btf` points to valid BTF data + * loaded into the kernel. Can't be NULL. + * @param map Map associated to the BTF data. Can't be NULL. + * @return 0 on success, or a negative errno value on failure. + */ +static int _bf_btf_new(struct bf_btf **btf, const struct bf_map *map) { _free_bf_btf_ struct bf_btf *_btf = NULL; + struct btf *raw; + const void *data; + uint32_t data_len; + int r; assert(btf); + assert(map); - _btf = malloc(sizeof(struct bf_btf)); + _btf = calloc(1, sizeof(struct bf_btf)); if (!_btf) return -ENOMEM; @@ -42,8 +95,55 @@ static int _bf_btf_new(struct bf_btf **btf) _btf->btf = btf__new_empty(); if (!_btf->btf) - return -errno; + return bf_err_r(-errno, "failed to create BTF structure"); + + raw = _btf->btf; + + /* It's not necessary to check the return value of each btf__xxx() call, + * on error libbpf/kernel will refuse the BTF data. */ + switch (map->type) { + case BF_MAP_TYPE_COUNTERS: + _btf->key_type_id = btf__add_int(raw, "u32", 4, 0); + + _btf->value_type_id = btf__add_struct(raw, "bf_counters", 16); + int counter_type_id = btf__add_int(raw, "u64", 8, 0); + btf__add_field(raw, "packets", counter_type_id, 0, 0); + btf__add_field(raw, "bytes", counter_type_id, 64, 0); + break; + case BF_MAP_TYPE_PRINTER: + /* Printer maps are array maps: keys are an integer type, and values are + * a placeholder struct. */ + _btf->key_type_id = btf__add_int(raw, "placeholder_key", 4, 0); + _btf->value_type_id = + btf__add_struct(raw, "placeholder_value", map->value_size); + break; + case BF_MAP_TYPE_SET: + /* Set maps are hash maps: keys are a structure of fixed size, so are + * values. */ + _btf->key_type_id = + btf__add_struct(raw, "placeholder_key", map->key_size); + _btf->value_type_id = + btf__add_struct(raw, "placeholder_value", map->value_size); + break; + case BF_MAP_TYPE_LOG: /* BTF data on ring buffer maps is not supported */ + default: + return bf_err_r(-ENOTSUP, "bf_map type %d is not supported", map->type); + } + + r = btf__add_decl_tag(raw, _bf_map_type_to_str(map->type), + _btf->value_type_id, -1); + if (r < 0) + return bf_err_r(r, "failed to add decl tag to bf_map BTF data"); + + data = btf__raw_data(raw, &data_len); + if (!data) + return bf_err_r(errno, "failed to request BTF raw data from libbpf"); + + r = bf_bpf_btf_load(data, data_len, bf_ctx_token()); + if (r < 0) + return bf_err_r(r, "failed to load BTF data for bf_map"); + _btf->fd = r; *btf = TAKE_PTR(_btf); return 0; @@ -61,76 +161,6 @@ static void _bf_btf_free(struct bf_btf **btf) freep((void *)btf); } -static int _bf_btf_load(struct bf_btf *btf) -{ - union bpf_attr attr = {}; - const void *raw; - int r; - - assert(btf); - - raw = btf__raw_data(btf->btf, &attr.btf_size); - if (!raw) - return bf_err_r(errno, "failed to request BTF raw data"); - - r = bf_bpf_btf_load(raw, attr.btf_size, bf_ctx_token()); - if (r < 0) - return r; - - btf->fd = r; - - return 0; -} - -/** - * @brief Create the BTF data for the map. - * - * @param map Map to create the BTF data for. @c map.type will define the - * exact content of the BTF object. Can't be NULL. - * @return A @ref bf_btf structure on success, or NULL on error. The - * @ref bf_btf structure is owned by the caller. - */ -static struct bf_btf *_bf_map_make_btf(const struct bf_map *map) -{ - _free_bf_btf_ struct bf_btf *btf = NULL; - struct btf *kbtf; - int r; - - assert(map); - - r = _bf_btf_new(&btf); - if (r < 0) - return NULL; - - kbtf = btf->btf; - - switch (map->type) { - case BF_MAP_TYPE_COUNTERS: - btf__add_int(kbtf, "u64", 8, 0); - btf->key_type_id = btf__add_int(kbtf, "u32", 4, 0); - btf->value_type_id = btf__add_struct(kbtf, "bf_counters", 16); - btf__add_field(kbtf, "packets", 1, 0, 0); - btf__add_field(kbtf, "bytes", 1, 64, 0); - break; - case BF_MAP_TYPE_PRINTER: - case BF_MAP_TYPE_SET: - case BF_MAP_TYPE_LOG: - // No BTF data available for this map types - return NULL; - default: - bf_err_r(-ENOTSUP, "bf_map type %d is not supported", map->type); - return NULL; - } - - r = _bf_btf_load(btf); - if (r) { - bf_warn_r(r, "failed to load BTF data for %s, ignoring", map->name); - return NULL; - } - - return TAKE_PTR(btf); -} - static int _bf_map_new(struct bf_map **map, const char *name, enum bf_map_type type, enum bf_bpf_map_type bpf_type, size_t key_size, size_t value_size, size_t n_elems) @@ -138,6 +168,7 @@ static int _bf_map_new(struct bf_map **map, const char *name, _free_bf_map_ struct bf_map *_map = NULL; _free_bf_btf_ struct bf_btf *btf = NULL; _cleanup_close_ int fd = -1; + int r; assert(map); assert(name); @@ -173,15 +204,18 @@ static int _bf_map_new(struct bf_map **map, const char *name, bf_strncpy(_map->name, BPF_OBJ_NAME_LEN, name); - btf = _bf_map_make_btf(_map); + if (type != BF_MAP_TYPE_LOG) { + r = _bf_btf_new(&btf, _map); + if (r) + return r; + } - fd = - bf_bpf_map_create(_map->name, _map->bpf_type, _map->key_size, + r = bf_bpf_map_create(name, _map->bpf_type, _map->key_size, _map->value_size, _map->n_elems, btf, bf_ctx_token()); - if (fd < 0) - return bf_err_r(fd, "bf_map %s: failed to create map", name); + if (r < 0) + return bf_err_r(r, "failed to create BPF map '%s'", name); - _map->fd = TAKE_FD(fd); + _map->fd = r; *map = TAKE_PTR(_map); return 0; @@ -300,22 +334,6 @@ int bf_map_pack(const struct bf_map *map, bf_wpack_t *pack) return bf_wpack_is_valid(pack) ? 0 : -EINVAL; } -static const char *_bf_map_type_to_str(enum bf_map_type type) -{ - static const char *type_strs[] = { - [BF_MAP_TYPE_COUNTERS] = "BF_MAP_TYPE_COUNTERS", - [BF_MAP_TYPE_PRINTER] = "BF_MAP_TYPE_PRINTER", - [BF_MAP_TYPE_LOG] = "BF_MAP_TYPE_LOG", - [BF_MAP_TYPE_SET] = "BF_MAP_TYPE_SET", - }; - - static_assert(ARRAY_SIZE(type_strs) == _BF_MAP_TYPE_MAX, - "missing entries in _bf_map_type_strs array"); - assert(0 <= type && type < _BF_MAP_TYPE_MAX); - - return type_strs[type]; -} - static const char *_bf_bpf_type_to_str(enum bf_bpf_map_type type) { static const char *type_strs[] = { diff --git a/src/libbpfilter/include/bpfilter/btf.h b/src/libbpfilter/include/bpfilter/btf.h index 5d7335b4..bd115426 100644 --- a/src/libbpfilter/include/bpfilter/btf.h +++ b/src/libbpfilter/include/bpfilter/btf.h @@ -10,8 +10,8 @@ struct bf_btf { struct btf *btf; - uint32_t key_type_id; - uint32_t value_type_id; + int key_type_id; + int value_type_id; int fd; }; From 4f0a6c0d21c213455c03f0ec04acb163449cbcd2 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 14:33:56 +0100 Subject: [PATCH 09/12] lib: bpf: add bpf syscall for get info, link by id, and map by id --- src/libbpfilter/bpf.c | 37 ++++++++++++++++++++ src/libbpfilter/include/bpfilter/bpf.h | 33 +++++++++++++++++ src/libbpfilter/include/bpfilter/bpf_types.h | 3 ++ 3 files changed, 73 insertions(+) diff --git a/src/libbpfilter/bpf.c b/src/libbpfilter/bpf.c index c14c039b..649edb22 100644 --- a/src/libbpfilter/bpf.c +++ b/src/libbpfilter/bpf.c @@ -267,6 +267,43 @@ int bf_bpf_map_update_batch(int map_fd, const void *keys, const void *values, return bf_bpf(BF_BPF_MAP_UPDATE_BATCH, &attr); } +int bf_bpf_obj_get_info(int fd, void *info, uint32_t info_len) +{ + union bpf_attr attr; + + assert(info); + + memset(&attr, 0, sizeof(attr)); + + attr.info.bpf_fd = fd; + attr.info.info_len = info_len; + attr.info.info = bf_ptr_to_u64(info); + + return bf_bpf(BF_BPF_OBJ_GET_INFO_BY_FD, &attr); +} + +int bf_bpf_map_get_fd_by_id(uint32_t id) +{ + union bpf_attr attr; + + memset(&attr, 0, sizeof(attr)); + + attr.map_id = id; + + return bf_bpf(BF_BPF_MAP_GET_FD_BY_ID, &attr); +} + +int bf_bpf_btf_get_fd_by_id(uint32_t id) +{ + union bpf_attr attr; + + memset(&attr, 0, sizeof(attr)); + + attr.btf_id = id; + + return bf_bpf(BF_BPF_BTF_GET_FD_BY_ID, &attr); +} + int bf_bpf_link_create(int prog_fd, int target_fd, enum bf_hook hook, int flags, uint32_t family, int32_t priority) { diff --git a/src/libbpfilter/include/bpfilter/bpf.h b/src/libbpfilter/include/bpfilter/bpf.h index 7f9ff8fd..dc7cc873 100644 --- a/src/libbpfilter/include/bpfilter/bpf.h +++ b/src/libbpfilter/include/bpfilter/bpf.h @@ -161,6 +161,39 @@ int bf_bpf_map_update_elem(int map_fd, const void *key, const void *value, int bf_bpf_map_update_batch(int map_fd, const void *keys, const void *values, size_t count, int flags); +/** + * @brief Get information about a BPF object from its file descriptor. + * + * Uses `BPF_OBJ_GET_INFO_BY_FD` to query the kernel for information about + * a BPF object (map, program, BTF, or link). The caller must provide the + * appropriate info structure for the object type (e.g. `struct bpf_map_info` + * for maps, `struct bpf_btf_info` for BTF). + * + * @param fd File descriptor of the BPF object to query. Must be valid. + * @param info Pointer to the info structure to fill. Can't be NULL. + * @param info_len Size of the info structure. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_bpf_obj_get_info(int fd, void *info, uint32_t info_len); + +/** + * @brief Get a file descriptor for a BPF map from its ID. + * + * @param id Map ID to look up. + * @return A valid map file descriptor on success (owned by the caller), + * or a negative errno value on failure. + */ +int bf_bpf_map_get_fd_by_id(uint32_t id); + +/** + * @brief Get a file descriptor for a BTF object from its ID. + * + * @param id BTF ID to look up. + * @return A valid BTF file descriptor on success (owned by the caller), + * or a negative errno value on failure. + */ +int bf_bpf_btf_get_fd_by_id(uint32_t id); + /** * @brief Create a new BPF link. * diff --git a/src/libbpfilter/include/bpfilter/bpf_types.h b/src/libbpfilter/include/bpfilter/bpf_types.h index aa8b2457..764474fe 100644 --- a/src/libbpfilter/include/bpfilter/bpf_types.h +++ b/src/libbpfilter/include/bpfilter/bpf_types.h @@ -17,6 +17,9 @@ enum bf_bpf_cmd BF_BPF_MAP_CREATE = 0, BF_BPF_MAP_UPDATE_ELEM = 2, BF_BPF_MAP_UPDATE_BATCH = 26, + BF_BPF_MAP_GET_FD_BY_ID = 14, + BF_BPF_OBJ_GET_INFO_BY_FD = 15, + BF_BPF_BTF_GET_FD_BY_ID = 19, BF_BPF_LINK_CREATE = 28, BF_BPF_LINK_UPDATE = 29, BF_BPF_LINK_DETACH = 34, From 95104ee23eda7c7eebb83d8cf372a21086f328be Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 16:40:27 +0100 Subject: [PATCH 10/12] daemon: cgen: restore bf_map from FD instead of serialized context Use the BPF map FD to restore the handle's maps instead of the serialized context. Most of the information from bf_map can be collected using BPF_OBJ_GET_INFO_BY_FD, the internal map type is stored as a decl tag in the associated BTF data. --- src/bpfilter/cgen/handle.c | 148 ++++++++++++----------------------- src/bpfilter/cgen/prog/map.c | 113 ++++++++++++++++++++++++++ src/bpfilter/cgen/prog/map.h | 20 +++++ tests/e2e/matchers/set.sh | 10 ++- 4 files changed, 192 insertions(+), 99 deletions(-) diff --git a/src/bpfilter/cgen/handle.c b/src/bpfilter/cgen/handle.c index 39170826..219b171b 100644 --- a/src/bpfilter/cgen/handle.c +++ b/src/bpfilter/cgen/handle.c @@ -5,7 +5,10 @@ #include "cgen/handle.h" +#include + #include +#include #include #include #include @@ -21,6 +24,7 @@ #include "cgen/prog/link.h" #include "cgen/prog/map.h" +#include "ctx.h" int bf_handle_new(struct bf_handle **handle, const char *prog_name) { @@ -46,8 +50,10 @@ int bf_handle_new_from_pack(struct bf_handle **handle, int dir_fd, bf_rpack_node_t node) { _free_bf_handle_ struct bf_handle *_handle = NULL; + _cleanup_free_ uint32_t *map_ids = NULL; + struct bpf_prog_info prog_info = {}; _cleanup_free_ char *name = NULL; - bf_rpack_node_t child, array_node; + bf_rpack_node_t child; int r; assert(handle); @@ -73,43 +79,59 @@ int bf_handle_new_from_pack(struct bf_handle **handle, int dir_fd, return bf_rpack_key_err(r, "bf_handle.link"); } - r = bf_rpack_kv_node(node, "cmap", &child); + r = bf_bpf_obj_get_info(_handle->prog_fd, &prog_info, sizeof(prog_info)); if (r) - return bf_rpack_key_err(r, "bf_handle.cmap"); - if (!bf_rpack_is_nil(child)) { - r = bf_map_new_from_pack(&_handle->cmap, dir_fd, child); - if (r) - return bf_rpack_key_err(r, "bf_handle.cmap"); - } + return bf_err_r(r, "failed to get program info for '%s'", name); - r = bf_rpack_kv_node(node, "pmap", &child); - if (r) - return bf_rpack_key_err(r, "bf_handle.pmap"); - if (!bf_rpack_is_nil(child)) { - r = bf_map_new_from_pack(&_handle->pmap, dir_fd, child); - if (r) - return bf_rpack_key_err(r, "bf_handle.pmap"); - } + if (prog_info.nr_map_ids > 0) { + uint32_t nr_map_ids = prog_info.nr_map_ids; - r = bf_rpack_kv_node(node, "lmap", &child); - if (r) - return bf_rpack_key_err(r, "bf_handle.lmap"); - if (!bf_rpack_is_nil(child)) { - r = bf_map_new_from_pack(&_handle->lmap, dir_fd, child); + map_ids = calloc(nr_map_ids, sizeof(*map_ids)); + if (!map_ids) + return -ENOMEM; + + memset(&prog_info, 0, sizeof(prog_info)); + prog_info.nr_map_ids = nr_map_ids; + prog_info.map_ids = bf_ptr_to_u64(map_ids); + + r = bf_bpf_obj_get_info(_handle->prog_fd, &prog_info, + sizeof(prog_info)); if (r) - return bf_rpack_key_err(r, "bf_handle.lmap"); + return bf_err_r(r, "failed to get map IDs for '%s'", name); } - r = bf_rpack_kv_array(node, "sets", &child); - if (r) - return bf_rpack_key_err(r, "bf_handle.sets"); - bf_rpack_array_foreach (child, array_node) { + for (uint32_t i = 0; i < prog_info.nr_map_ids; ++i) { + _cleanup_close_ int map_fd = -1; _free_bf_map_ struct bf_map *map = NULL; - r = bf_list_emplace(&_handle->sets, bf_map_new_from_pack, map, dir_fd, - array_node); + map_fd = bf_bpf_map_get_fd_by_id(map_ids[i]); + if (map_fd < 0) { + return bf_err_r(map_fd, "failed to get fd for map ID %u", + map_ids[i]); + } + + r = bf_map_new_from_fd(&map, map_fd); if (r) - return bf_err_r(r, "failed to unpack bf_map into bf_handle.sets"); + return bf_err_r(r, "failed to restore map from ID %u", map_ids[i]); + + switch (map->type) { + case BF_MAP_TYPE_COUNTERS: + _handle->cmap = TAKE_PTR(map); + break; + case BF_MAP_TYPE_PRINTER: + _handle->pmap = TAKE_PTR(map); + break; + case BF_MAP_TYPE_LOG: + _handle->lmap = TAKE_PTR(map); + break; + case BF_MAP_TYPE_SET: + r = bf_list_push(&_handle->sets, (void **)&map); + if (r) + return r; + break; + default: + return bf_err_r(-EINVAL, "unknown bf_map type %d", map->type); + } } *handle = TAKE_PTR(_handle); @@ -149,32 +171,6 @@ int bf_handle_pack(const struct bf_handle *handle, bf_wpack_t *pack) bf_wpack_kv_nil(pack, "link"); } - if (handle->cmap) { - bf_wpack_open_object(pack, "cmap"); - bf_map_pack(handle->cmap, pack); - bf_wpack_close_object(pack); - } else { - bf_wpack_kv_nil(pack, "cmap"); - } - - if (handle->pmap) { - bf_wpack_open_object(pack, "pmap"); - bf_map_pack(handle->pmap, pack); - bf_wpack_close_object(pack); - } else { - bf_wpack_kv_nil(pack, "pmap"); - } - - if (handle->lmap) { - bf_wpack_open_object(pack, "lmap"); - bf_map_pack(handle->lmap, pack); - bf_wpack_close_object(pack); - } else { - bf_wpack_kv_nil(pack, "lmap"); - } - - bf_wpack_kv_list(pack, "sets", &handle->sets); - return bf_wpack_is_valid(pack) ? 0 : -EINVAL; } @@ -262,38 +258,6 @@ int bf_handle_pin(struct bf_handle *handle, int dir_fd) goto err_unpin_all; } - if (handle->cmap) { - r = bf_map_pin(handle->cmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF counters map"); - goto err_unpin_all; - } - } - - if (handle->pmap) { - r = bf_map_pin(handle->pmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF printer map"); - goto err_unpin_all; - } - } - - if (handle->lmap) { - r = bf_map_pin(handle->lmap, dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF log map"); - goto err_unpin_all; - } - } - - bf_list_foreach (&handle->sets, set_node) { - r = bf_map_pin(bf_list_node_get_data(set_node), dir_fd); - if (r) { - bf_err_r(r, "failed to pin BPF set map"); - goto err_unpin_all; - } - } - if (handle->link) { r = bf_link_pin(handle->link, dir_fd); if (r) { @@ -313,16 +277,6 @@ void bf_handle_unpin(struct bf_handle *handle, int dir_fd) { assert(handle); - if (handle->cmap) - bf_map_unpin(handle->cmap, dir_fd); - if (handle->pmap) - bf_map_unpin(handle->pmap, dir_fd); - if (handle->lmap) - bf_map_unpin(handle->lmap, dir_fd); - - bf_list_foreach (&handle->sets, set_node) - bf_map_unpin(bf_list_node_get_data(set_node), dir_fd); - if (handle->link) bf_link_unpin(handle->link, dir_fd); diff --git a/src/bpfilter/cgen/prog/map.c b/src/bpfilter/cgen/prog/map.c index 9ed512c4..f7a34e8a 100644 --- a/src/bpfilter/cgen/prog/map.c +++ b/src/bpfilter/cgen/prog/map.c @@ -6,6 +6,7 @@ #include "cgen/prog/map.h" #include +#include #include #include @@ -308,6 +309,118 @@ int bf_map_new_from_pack(struct bf_map **map, int dir_fd, bf_rpack_node_t node) return 0; } +/** + * @brief Get the `bf_map` type from a map's BTF data. + * + * Retrieve the BTF data associated with the given BTF ID, parse it, + * and look for a `BTF_KIND_DECL_TAG` whose value matches a known + * `bf_map_type` string. + * + * @param btf_id BTF ID associated with the map. Must be non-zero. + * @param type On success, set to the corresponding `bf_map_type`. Can't + * be NULL. + * @return 0 on success, or a negative errno value on failure. + */ +static int _bf_map_type_from_btf(uint32_t btf_id, enum bf_map_type *type) +{ + _cleanup_close_ int btf_fd = -1; + _cleanup_free_ void *data = NULL; + struct bpf_btf_info btf_info = {}; + struct btf *btf; + int r; + + assert(btf_id); + assert(type); + + btf_fd = bf_bpf_btf_get_fd_by_id(btf_id); + if (btf_fd < 0) + return bf_err_r(btf_fd, "failed to get BTF fd for ID %u", btf_id); + + r = bf_bpf_obj_get_info(btf_fd, &btf_info, sizeof(btf_info)); + if (r) + return bf_err_r(r, "failed to query BTF info size for ID %u", btf_id); + + if (btf_info.btf_size == 0) + return bf_err_r(-ENODATA, "BTF data is empty for ID %u", btf_id); + + data = malloc(btf_info.btf_size); + if (!data) + return -ENOMEM; + + btf_info.btf = bf_ptr_to_u64(data); + + r = bf_bpf_obj_get_info(btf_fd, &btf_info, sizeof(btf_info)); + if (r) + return bf_err_r(r, "failed to retrieve BTF data for ID %u", btf_id); + + btf = btf__new(data, btf_info.btf_size); + if (!btf) + return bf_err_r(-errno, "failed to parse BTF data for ID %u", btf_id); + + for (uint32_t i = 1; i < btf__type_cnt(btf); ++i) { + const struct btf_type *btf_type = btf__type_by_id(btf, i); + + if (BTF_INFO_KIND(btf_type->info) != BTF_KIND_DECL_TAG) + continue; + + r = _bf_map_type_from_str(btf__name_by_offset(btf, btf_type->name_off), + type); + if (r == 0) { + btf__free(btf); + return 0; + } + } + + btf__free(btf); + + return bf_err_r( + -ENOENT, "no bf_map type decl tag found in BTF data for ID %u", btf_id); +} + +int bf_map_new_from_fd(struct bf_map **map, int fd) +{ + _free_bf_map_ struct bf_map *_map = NULL; + struct bpf_map_info info = {}; + int r; + + assert(map); + + r = bf_bpf_obj_get_info(fd, &info, sizeof(info)); + if (r) + return bf_err_r(r, "failed to get BPF map info from fd %d", fd); + + _map = malloc(sizeof(*_map)); + if (!_map) + return -ENOMEM; + + bf_strncpy(_map->name, BPF_OBJ_NAME_LEN, info.name); + _map->bpf_type = info.type; + _map->key_size = info.key_size; + _map->value_size = info.value_size; + _map->n_elems = info.max_entries; + + _map->fd = dup(fd); + if (_map->fd < 0) + return bf_err_r(-errno, "failed to duplicate map fd %d", fd); + + if (info.btf_id) { + r = _bf_map_type_from_btf(info.btf_id, &_map->type); + if (r) + return r; + } else if (info.type == BF_BPF_MAP_TYPE_RINGBUF) { + _map->type = BF_MAP_TYPE_LOG; + } else { + return bf_err_r( + -ENOTSUP, + "BPF map '%s' has no BTF data, can't determine bf_map type", + info.name); + } + + *map = TAKE_PTR(_map); + + return 0; +} + void bf_map_free(struct bf_map **map) { assert(map); diff --git a/src/bpfilter/cgen/prog/map.h b/src/bpfilter/cgen/prog/map.h index 0b33f939..a8cfc378 100644 --- a/src/bpfilter/cgen/prog/map.h +++ b/src/bpfilter/cgen/prog/map.h @@ -72,6 +72,26 @@ int bf_map_new(struct bf_map **map, const char *name, enum bf_map_type type, int bf_map_new_from_set(struct bf_map **map, const char *name, const struct bf_set *set); +/** + * @brief Allocate and initialize a new map from an existing BPF map file + * descriptor. + * + * Uses `BPF_OBJ_GET_INFO_BY_FD` to query the map's properties and BTF data. + * The internal `bf_map_type` is determined from the BTF decl tag attached to + * the map's value type. Ring buffer maps do not allow for BTF data, so any + * ring buffer map is assumed to be a log map. + * + * The file descriptor is duplicated internally; the caller retains ownership + * of `fd`. + * + * @param map Map object to allocate and initialize. On success, `*map` points + * to a valid @ref bf_map . On failure, `*map` is unchanged. Can't be + * NULL. + * @param fd File descriptor of an existing BPF map. Must be valid. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_map_new_from_fd(struct bf_map **map, int fd); + /** * @brief Allocate and initialize a new map from serialized data. * diff --git a/tests/e2e/matchers/set.sh b/tests/e2e/matchers/set.sh index 6abb4016..c922008b 100755 --- a/tests/e2e/matchers/set.sh +++ b/tests/e2e/matchers/set.sh @@ -75,7 +75,13 @@ start_bpfilter ${FROM_NS} bfcli chain set --from-str "chain test BF_HOOK_XDP{ifindex=${NS_IFINDEX}} ACCEPT rule (ip4.saddr) in { 192.168.1.1 } DROP rule (ip4.saddr) in {} ACCEPT" - # Verify only 1 set map was pinned (empty set should not create a map) - MAP_COUNT=$(${FROM_NS} find ${WORKDIR}/bpf/bpfilter/test/ -name 'set_*' | wc -l) + + # Verify only 1 set map is associated to the program (empty set should not create a map) + MAP_IDS=$(${FROM_NS} bpftool -j prog show pinned ${WORKDIR}/bpf/bpfilter/test/bf_prog | jq -r '.map_ids[]') + MAP_COUNT=0 + for map_id in ${MAP_IDS}; do + name=$(${FROM_NS} bpftool -j map show id ${map_id} | jq -r '.name') + [[ "${name}" == set_* ]] && MAP_COUNT=$((MAP_COUNT + 1)) + done [ "${MAP_COUNT}" -eq 1 ] || { echo "ERROR: Expected 1 set map, found ${MAP_COUNT}"; exit 1; } stop_bpfilter From 2678cfd85bc49e94b11696e53926d36367e0e1f7 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 17:09:50 +0100 Subject: [PATCH 11/12] daemon: cgen: restore bf_link from FD instead of serialized data Similarly to bf_map, restore bf_handle.link from the link's FD (up to two links). The hook options can't be restored fully from the link's data for now, so we serialize handle->link->hookopts directly from bf_handle for now. --- src/bpfilter/cgen/handle.c | 32 ++++++-- src/bpfilter/cgen/prog/link.c | 115 +++++++++++++++++++++++++++ src/bpfilter/cgen/prog/link.h | 19 +++++ tests/e2e/daemon/restore_attached.sh | 2 + 4 files changed, 160 insertions(+), 8 deletions(-) diff --git a/src/bpfilter/cgen/handle.c b/src/bpfilter/cgen/handle.c index 219b171b..79fb2997 100644 --- a/src/bpfilter/cgen/handle.c +++ b/src/bpfilter/cgen/handle.c @@ -49,10 +49,13 @@ int bf_handle_new(struct bf_handle **handle, const char *prog_name) int bf_handle_new_from_pack(struct bf_handle **handle, int dir_fd, bf_rpack_node_t node) { + _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL; _free_bf_handle_ struct bf_handle *_handle = NULL; _cleanup_free_ uint32_t *map_ids = NULL; - struct bpf_prog_info prog_info = {}; + _cleanup_close_ int link_fd = -1; + _cleanup_close_ int link_extra_fd = -1; _cleanup_free_ char *name = NULL; + struct bpf_prog_info prog_info = {}; bf_rpack_node_t child; int r; @@ -70,13 +73,26 @@ int bf_handle_new_from_pack(struct bf_handle **handle, int dir_fd, if (r < 0) return bf_err_r(r, "failed to restore bf_handle.prog_fd from pin"); - r = bf_rpack_kv_node(node, "link", &child); + r = bf_rpack_kv_node(node, "hookopts", &child); if (r) - return bf_rpack_key_err(r, "bf_handle.link"); + return bf_rpack_key_err(r, "bf_handle.hookopts"); if (!bf_rpack_is_nil(child)) { - r = bf_link_new_from_pack(&_handle->link, dir_fd, child); + r = bf_hookopts_new_from_pack(&hookopts, child); + if (r) + return bf_rpack_key_err(r, "bf_handle.hookopts"); + + r = bf_bpf_obj_get("bf_link", dir_fd, &link_fd); + if (r) + return bf_err_r(r, "failed to open chain's link"); + + r = bf_bpf_obj_get("bf_link_extra", dir_fd, &link_extra_fd); + if (r != 0 && r != -ENOENT) + return bf_err_r(r, "failed to open chain's link extra"); + + r = bf_link_new_from_fd(&_handle->link, &hookopts, link_fd, + link_extra_fd); if (r) - return bf_rpack_key_err(r, "bf_handle.link"); + return bf_err_r(r, "failed to restore bf_link"); } r = bf_bpf_obj_get_info(_handle->prog_fd, &prog_info, sizeof(prog_info)); @@ -164,11 +180,11 @@ int bf_handle_pack(const struct bf_handle *handle, bf_wpack_t *pack) bf_wpack_kv_str(pack, "prog_name", handle->prog_name); if (handle->link) { - bf_wpack_open_object(pack, "link"); - bf_link_pack(handle->link, pack); + bf_wpack_open_object(pack, "hookopts"); + bf_hookopts_pack(handle->link->hookopts, pack); bf_wpack_close_object(pack); } else { - bf_wpack_kv_nil(pack, "link"); + bf_wpack_kv_nil(pack, "hookopts"); } return bf_wpack_is_valid(pack) ? 0 : -EINVAL; diff --git a/src/bpfilter/cgen/prog/link.c b/src/bpfilter/cgen/prog/link.c index 27bd33e3..366ce202 100644 --- a/src/bpfilter/cgen/prog/link.c +++ b/src/bpfilter/cgen/prog/link.c @@ -3,6 +3,8 @@ * Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */ +#define _GNU_SOURCE + #include "cgen/prog/link.h" #include @@ -17,6 +19,7 @@ #include #include +#include #include #include #include @@ -49,6 +52,8 @@ int bf_link_new(struct bf_link **link, enum bf_hook hook, return -ENOMEM; _link->hook = hook; + _link->fd = -1; + _link->fd_extra = -1; switch (bf_hook_to_flavor(hook)) { case BF_FLAVOR_XDP: @@ -145,6 +150,116 @@ int bf_link_new_from_pack(struct bf_link **link, int dir_fd, return 0; } +int bf_link_new_from_fd(struct bf_link **link, struct bf_hookopts **hookopts, + int fd, int fd_extra) +{ + _free_bf_link_ struct bf_link *_link = NULL; + struct bpf_link_info info = {}; + int r; + + assert(link); + + r = bf_bpf_obj_get_info(fd, &info, sizeof(info)); + if (r) + return bf_err_r(r, "failed to get BPF link info from fd %d", fd); + + _link = calloc(1, sizeof(*_link)); + if (!_link) + return -ENOMEM; + + _link->fd = -1; + _link->fd_extra = -1; + + switch (info.type) { + case BPF_LINK_TYPE_XDP: + _link->hook = BF_HOOK_XDP; + if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_IFINDEX)) || + (int)info.xdp.ifindex != (*hookopts)->ifindex) { + return bf_err_r(-EINVAL, + "hookopts doesn't define same ifindex as the link"); + } + break; + case BPF_LINK_TYPE_TCX: + if (info.tcx.attach_type == BF_BPF_TCX_INGRESS) + _link->hook = BF_HOOK_TC_INGRESS; + else if (info.tcx.attach_type == BF_BPF_TCX_ENGRESS) + _link->hook = BF_HOOK_TC_EGRESS; + else + return bf_err_r(-EINVAL, "unknown TCX attach type %u", + info.tcx.attach_type); + if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_IFINDEX)) || + (int)info.tcx.ifindex != (*hookopts)->ifindex) { + return bf_err_r(-EINVAL, + "hookopts doesn't define same ifindex as the link"); + } + break; + case BPF_LINK_TYPE_NETFILTER: + _link->hook = bf_hook_from_nf_hook(info.netfilter.hooknum); + if ((int)_link->hook < 0) + return bf_err_r(-EINVAL, "unknown NF hooknum %u", + info.netfilter.hooknum); + if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_PRIORITIES)) || + info.netfilter.priority != (*hookopts)->priorities[0]) { + return bf_err_r( + -EINVAL, "hookopts doesn't define same priority as the link"); + } + break; + case BPF_LINK_TYPE_CGROUP: + if (info.cgroup.attach_type == BF_BPF_CGROUP_INET_INGRESS) + _link->hook = BF_HOOK_CGROUP_INGRESS; + else if (info.cgroup.attach_type == BF_BPF_CGROUP_INET_EGRESS) + _link->hook = BF_HOOK_CGROUP_EGRESS; + else + return bf_err_r(-EINVAL, "unknown cgroup attach type %u", + info.cgroup.attach_type); + + { + /* MAX_HANDLE_SZ is 128 */ + struct cgid_file_handle + { + unsigned int handle_bytes; + int handle_type; + unsigned char f_handle[128]; + }; + + struct cgid_file_handle handle = {.handle_bytes = 128}; + int mount_id; + + if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_PRIORITIES))) + return bf_err_r(-EINVAL, "missing cgpath in hookopts"); + + if (name_to_handle_at(AT_FDCWD, (*hookopts)->cgpath, + (struct file_handle *)&handle, &mount_id, + 0) < 0) + return bf_err_r(errno, "failed to validate cgid"); + + if (info.cgroup.cgroup_id != *(uint64_t *)handle.f_handle) { + return bf_err_r( + -EINVAL, "hookopts doesn't define same cgroup as the link"); + } + } + break; + default: + return bf_err_r(-ENOTSUP, "unsupported BPF link type %u", info.type); + } + + _link->fd = dup(fd); + if (_link->fd < 0) + return bf_err_r(-errno, "failed to duplicate link fd %d", fd); + + if (fd_extra >= 0) { + _link->fd_extra = dup(fd_extra); + if (_link->fd_extra < 0) + return bf_err_r(-errno, "failed to duplicate extra link fd %d", + fd_extra); + } + + _link->hookopts = TAKE_PTR(*hookopts); + *link = TAKE_PTR(_link); + + return 0; +} + void bf_link_free(struct bf_link **link) { assert(link); diff --git a/src/bpfilter/cgen/prog/link.h b/src/bpfilter/cgen/prog/link.h index 804ee7ad..2066d9f9 100644 --- a/src/bpfilter/cgen/prog/link.h +++ b/src/bpfilter/cgen/prog/link.h @@ -64,6 +64,25 @@ int bf_link_new(struct bf_link **link, enum bf_hook hook, int bf_link_new_from_pack(struct bf_link **link, int dir_fd, bf_rpack_node_t node); +/** + * @brief Create a new link from its file descriptor. + * + * Reconstruct a `bf_link` from kernel state by querying `bpf_link_info`. + * The hook type and hook options are derived from the link's type-specific + * fields. Both file descriptors are duplicated internally. + * + * @param link Link object to allocate and initialize. On failure, @c *link is + * unchanged. Can't be NULL. + * @param hookopts Hook options used when creating the link. On success, the + * new link takes ownership of it. Can't be NULL. + * @param fd File descriptor of the BPF link. Must be valid. + * @param fd_extra File descriptor of the extra BPF link (e.g. inet6 for + * Netfilter). Pass -1 if not applicable. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_link_new_from_fd(struct bf_link **link, struct bf_hookopts **hookopts, + int fd, int fd_extra); + /** * Deallocate a `bf_link` object. * diff --git a/tests/e2e/daemon/restore_attached.sh b/tests/e2e/daemon/restore_attached.sh index 1cb9fc7f..9e649089 100755 --- a/tests/e2e/daemon/restore_attached.sh +++ b/tests/e2e/daemon/restore_attached.sh @@ -10,5 +10,7 @@ stop_bpfilter --skip-cleanup start_bpfilter # Ensure it's restored properly + LINE_COUNT=$(${FROM_NS} bfcli ruleset get | wc -l) + test "$LINE_COUNT" -ne 0 ${FROM_NS} bfcli chain set --from-str "chain test_chain BF_HOOK_XDP{ifindex=${NS_IFINDEX}} ACCEPT" stop_bpfilter \ No newline at end of file From a9498db8319da6dca9c30851115c6711875440bf Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 15:47:06 +0100 Subject: [PATCH 12/12] tests: e2e: restore --skip-cleanup for stop_bpfilter() --- tests/e2e/e2e_test_util.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/e2e/e2e_test_util.sh b/tests/e2e/e2e_test_util.sh index 335ae9b7..94bf2218 100755 --- a/tests/e2e/e2e_test_util.sh +++ b/tests/e2e/e2e_test_util.sh @@ -154,7 +154,10 @@ stop_bpfilter() { echo "Stop bpfilter" - bfcli ruleset flush || true + if [ "$skip_cleanup" -eq 0 ]; then + bfcli ruleset flush || true + fi + kill $BPFILTER_PID 2>/dev/null || true wait $BPFILTER_PID || true