From 51bdadb5d3a2136d96ff0e8c3f3314d919cb9982 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Sat, 14 Feb 2026 08:48:23 +0100 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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); }