Prevent serialization of bf_program object

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);
 

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,
     },
     {

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)};
 

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

src/bpfilter/cgen/cgen.c

@@ -25,6 +25,7 @@
 #include <bpfilter/pack.h>
 
 #include "cgen/dump.h"
+#include "cgen/handle.h"
 #include "cgen/prog/link.h"
 #include "cgen/program.h"
 #include "ctx.h"
@@ -59,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);
 
@@ -78,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)
@@ -92,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;
 
@@ -104,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;
     }
@@ -131,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);
@@ -149,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;
@@ -176,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);
@@ -207,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;
 
@@ -237,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");
 
@@ -251,19 +253,20 @@ 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;
 }
 
 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;
 
@@ -287,16 +290,18 @@ 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;
     }
 
     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;
 }
@@ -325,17 +330,17 @@ 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);
 
     if (bf_ns_set(bf_ctx_get_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->handle->link, pindir_fd);
         if (r) {
-            bf_program_detach(cgen->program);
+            bf_handle_detach(cgen->handle);
             return r;
         }
     }
@@ -346,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);
@@ -375,31 +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->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_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->link, old_prog->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);
@@ -411,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)
@@ -428,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)

src/bpfilter/cgen/cgen.h

@@ -14,7 +14,7 @@
 #include <bpfilter/pack.h>
 
 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.