Add update-set operation for delta updates to sets

doc/usage/bfcli.rst

@@ -239,6 +239,37 @@ If you want to modify the hook options, use ``bfcli chain set`` instead.
               counters 0 packets 0 bytes
               DROP
 
+``chain update-set``
+~~~~~~~~~~~~~~~~~~~~
+
+Atomically update the content of a named set in a chain using delta operations. This is more efficient than replacing the entire chain when you only need to modify set membership.
+
+**Options**
+  - ``--name NAME``: name of the chain containing the set.
+  - ``--set-name NAME``: name of the set to update.
+  - ``--add ELEMENT``: element to add to the set. Can be specified multiple times.
+  - ``--remove ELEMENT``: element to remove from the set. Can be specified multiple times.
+
+At least one of ``--add`` or ``--remove`` must be specified.
+
+**Examples**
+
+.. code:: shell
+
+    $ # Create a chain with a named set
+    $ sudo bfcli chain set --from-str "
+          chain my_filter BF_HOOK_XDP{ifindex=2} ACCEPT
+              set blocklist (ip4.saddr) in { 192.168.1.100 }
+              rule
+                  (ip4.saddr) in blocklist
+                  DROP"
+
+    $ # Add addresses to the blocklist
+    $ sudo bfcli chain update-set --name my_filter --set-name blocklist --add 192.168.1.101 --add 192.168.1.102
+
+    $ # Remove an address from the blocklist
+    $ sudo bfcli chain update-set --name my_filter --set-name blocklist --remove 192.168.1.100
+
 ``chain flush``
 ~~~~~~~~~~~~~~~
 

src/bfcli/chain.c

@@ -24,6 +24,7 @@
 #include <bpfilter/hook.h>
 #include <bpfilter/list.h>
 #include <bpfilter/logger.h>
+#include <bpfilter/set.h>
 
 #include "helper.h"
 #include "opts.h"
@@ -264,3 +265,60 @@ int bfc_chain_flush(const struct bfc_opts *opts)
 
     return r;
 }
+
+int bfc_chain_update_set(const struct bfc_opts *opts)
+{
+    _free_bf_set_ struct bf_set *to_add = NULL;
+    _free_bf_set_ struct bf_set *to_remove = NULL;
+    _free_bf_chain_ struct bf_chain *chain = NULL;
+    _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL;
+    _clean_bf_list_ bf_list counters = bf_list_default(bf_counter_free, NULL);
+    struct bf_set *dest_set = NULL;
+    int r;
+
+    if (bf_list_is_empty(&opts->set_add) && bf_list_is_empty(&opts->set_remove))
+        return bf_err_r(-EINVAL, "no elements to add or remove");
+
+    // Fetch dest_set to get key format
+    r = bf_chain_get(opts->name, &chain, &hookopts, &counters);
+    if (r == -ENOENT)
+        return bf_err_r(r, "chain '%s' not found", opts->name);
+    if (r)
+        return bf_err_r(r, "unknown error");
+
+    dest_set = bf_chain_get_set_by_name(chain, opts->set_name);
+    if (!dest_set)
+        return bf_err_r(-ENOENT, "set '%s' does not exist", opts->set_name);
+
+    r = bf_set_new(&to_add, opts->set_name, dest_set->key, dest_set->n_comps);
+    if (r)
+        return bf_err_r(r, "failed to create set");
+
+    bf_list_foreach (&opts->set_add, node) {
+        const char *raw_elem = bf_list_node_get_data(node);
+
+        r = bf_set_add_elem_raw(to_add, raw_elem);
+        if (r)
+            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);
+    if (r)
+        return bf_err_r(r, "failed to create set");
+
+    bf_list_foreach (&opts->set_remove, node) {
+        const char *raw_elem = bf_list_node_get_data(node);
+
+        r = bf_set_add_elem_raw(to_remove, raw_elem);
+        if (r)
+            return bf_err_r(r, "failed to parse set element '%s'", raw_elem);
+    }
+
+    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);
+
+    bf_info("updated set '%s' in chain '%s'", opts->set_name, opts->name);
+
+    return 0;
+}

src/bfcli/chain.h

@@ -14,4 +14,5 @@ int bfc_chain_logs(const struct bfc_opts *opts);
 int bfc_chain_load(const struct bfc_opts *opts);
 int bfc_chain_attach(const struct bfc_opts *opts);
 int bfc_chain_update(const struct bfc_opts *opts);
+int bfc_chain_update_set(const struct bfc_opts *opts);
 int bfc_chain_flush(const struct bfc_opts *opts);

src/bfcli/opts.c

@@ -9,6 +9,7 @@
 
 #include <bpfilter/helper.h>
 
+#include "bpfilter/list.h"
 #include "chain.h"
 #include "ruleset.h"
 
@@ -73,6 +74,7 @@ static const char * const _bfc_action_strs[] = {
     "load", // BFC_ACTION_LOAD
     "attach", // BFC_ACTION_ATTACH
     "update", // BFC_ACTION_UPDATE
+    "update-set", // BFC_ACTION_UPDATE_SET
     "flush", // BFC_ACTION_FLUSH
 };
 static_assert(ARRAY_SIZE(_bfc_action_strs) == _BFC_ACTION_MAX,
@@ -106,6 +108,9 @@ enum bfc_opts_option_id
     BFC_OPT_CHAIN_FROM_FILE,
     BFC_OPT_CHAIN_NAME,
     BFC_OPT_CHAIN_HOOK_OPTS,
+    BFC_OPT_SET_NAME,
+    BFC_OPT_SET_ADD,
+    BFC_OPT_SET_REMOVE,
     BFC_OPT_DRY_RUN,
     _BFC_OPT_MAX,
 };
@@ -224,6 +229,19 @@ static const struct bfc_opts_cmd _bfc_opts_cmds[] = {
                "definition provided by --from-str or --from-file.",
         .cb = bfc_chain_update,
     },
+    {
+        .name = "bfcli chain update-set",
+        .object = BFC_OBJECT_CHAIN,
+        .action = BFC_ACTION_UPDATE_SET,
+        .valid_opts = BF_FLAGS(BFC_OPT_CHAIN_NAME, BFC_OPT_SET_NAME,
+                               BFC_OPT_SET_ADD, BFC_OPT_SET_REMOVE),
+        .required_opts = BF_FLAGS(BFC_OPT_CHAIN_NAME, BFC_OPT_SET_NAME),
+        .doc = "Update a set in a chain\vAtomically update the content of a "
+               "named set in a chain using delta operation. Use --add to "
+               "add elements and --remove to remove elements. At least one "
+               "of --add or --remove must be specified.",
+        .cb = bfc_chain_update_set,
+    },
     {
         .name = "bfcli chain flush",
         .object = BFC_OBJECT_CHAIN,
@@ -354,6 +372,42 @@ static void _bfc_opts_chain_hook_opts_cb(struct argp_state *state,
         argp_error(state, "failed to parse hook option '%s'", arg);
 };
 
+static void _bfc_opts_set_name_cb(struct argp_state *state, const char *arg,
+                                  struct bfc_opts *opts)
+{
+    if (strlen(arg) == 0)
+        argp_error(state, "--set-name can't be empty");
+
+    opts->set_name = arg;
+};
+
+static void _bfc_opts_set_add_cb(struct argp_state *state,
+                                         const char *arg, struct bfc_opts *opts)
+{
+    int r;
+
+    if (strlen(arg) == 0)
+        argp_error(state, "--add requires an element");
+
+    r = bf_list_add_tail(&opts->set_add, (void *)arg);
+    if (r)
+        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)
+{
+    int r;
+
+    if (strlen(arg) == 0)
+        argp_error(state, "--remove requires an element");
+
+    r = bf_list_add_tail(&opts->set_remove, (void *)arg);
+    if (r)
+        argp_error(state, "failed to add element to list");
+};
+
 static void _bfc_opts_dry_run(struct argp_state *state, const char *arg,
                               struct bfc_opts *opts)
 {
@@ -447,6 +501,30 @@ struct bfc_opts_opt
         .doc = "Hook option to attach the chain",
         .parser = _bfc_opts_chain_hook_opts_cb,
     },
+    {
+        .id = BFC_OPT_SET_NAME,
+        .key = 'S',
+        .name = "set-name",
+        .arg = "NAME",
+        .doc = "Name of the set to update",
+        .parser = _bfc_opts_set_name_cb,
+    },
+    {
+        .id = BFC_OPT_SET_ADD,
+        .key = 'A',
+        .name = "add",
+        .arg = "ELEMENT",
+        .doc = "Element to add to the set. Can be specified multiple times.",
+        .parser = _bfc_opts_set_add_cb,
+    },
+    {
+        .id = BFC_OPT_SET_REMOVE,
+        .key = 'R',
+        .name = "remove",
+        .arg = "ELEMENT",
+        .doc = "Element to remove from the set. Can be specified multiple times.",
+        .parser = _bfc_opts_set_remove_cb,
+    },
     {
         .id = BFC_OPT_DRY_RUN,
         .key = 'd',
@@ -550,6 +628,8 @@ void bfc_opts_clean(struct bfc_opts *opts)
     assert(opts);
 
     bf_hookopts_clean(&opts->hookopts);
+    bf_list_clean(&opts->set_add);
+    bf_list_clean(&opts->set_remove);
 }
 
 #define _BFC_NAME_LEN (PATH_MAX + 32)
@@ -571,7 +651,8 @@ int bfc_opts_parse(struct bfc_opts *opts, int argc, char **argv)
         BFC_HELP_ENTRY(BFC_ACTION_LOGS, "Print the logged packets"),
         BFC_HELP_ENTRY(BFC_ACTION_LOAD, "Load a new chain, do not attach it"),
         BFC_HELP_ENTRY(BFC_ACTION_ATTACH, "Attach a loaded chain"),
-        BFC_HELP_ENTRY(BFC_ACTION_UPDATE, "Update an attached chain"),
+        BFC_HELP_ENTRY(BFC_ACTION_UPDATE, "Update an existing chain"),
+        BFC_HELP_ENTRY(BFC_ACTION_UPDATE_SET, "Update a set in a chain"),
         BFC_HELP_ENTRY(BFC_ACTION_FLUSH, "Remove a chain"),
         {.name = "help", .key = 'h', .group = -1, .doc = "Print help"},
         {.name = "usage",

src/bfcli/opts.h

@@ -48,6 +48,7 @@ enum bfc_action
     BFC_ACTION_LOAD,
     BFC_ACTION_ATTACH,
     BFC_ACTION_UPDATE,
+    BFC_ACTION_UPDATE_SET,
     BFC_ACTION_FLUSH,
     _BFC_ACTION_MAX,
 };
@@ -71,6 +72,12 @@ struct bfc_opts
     const char *name;
     struct bf_hookopts hookopts;
 
+    const char *set_name;
+    // set_add and set_remove don't own their data.
+    // They store strings from argv.
+    bf_list set_add;
+    bf_list set_remove;
+
     bool dry_run;
 };
 
@@ -89,7 +96,9 @@ 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)};
 
 void bfc_opts_clean(struct bfc_opts *opts);
 int bfc_opts_parse(struct bfc_opts *opts, int argc, char **argv);

src/bpfilter/cgen/cgen.c

@@ -378,17 +378,20 @@ int bf_cgen_update(struct bf_cgen *cgen, struct bf_chain **new_chain)
     if (bf_opts_persist())
         bf_program_unpin(old_prog, pindir_fd);
 
-    r = bf_link_update(old_prog->link, cgen->chain->hook,
-                       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)
-            bf_err("failed to repin old program, ignoring");
-        return r;
-    }
+    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 (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");
+            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 program
+        bf_swap(new_prog->link, old_prog->link);
+    }
 
     if (bf_opts_persist()) {
         r = bf_program_pin(new_prog, pindir_fd);

src/bpfilter/cgen/cgen.h

@@ -144,8 +144,7 @@ int bf_cgen_attach(struct bf_cgen *cgen, const struct bf_ns *ns,
  * On success, the new program is stored in the codegen, and the previous
  * program is unloaded and freed.
  *
- * @param cgen Codegen to update. It should already contain a program attached
- *        to a hook. Can't be NULL.
+ * @param cgen Codegen to update. Can't be NULL.
  * @param new_chain Chain containing the new rules, sets, and policy.
  *        Can't be NULL.
  * @return 0 on success, or negative errno value on failure.

src/bpfilter/main.c

@@ -360,6 +360,7 @@ static int _bf_process_request(struct bf_request *request,
          bf_request_cmd(request) == BF_REQ_CHAIN_LOAD ||
          bf_request_cmd(request) == BF_REQ_CHAIN_ATTACH ||
          bf_request_cmd(request) == BF_REQ_CHAIN_UPDATE ||
+         bf_request_cmd(request) == BF_REQ_CHAIN_UPDATE_SET ||
          bf_request_cmd(request) == BF_REQ_CHAIN_FLUSH))
         r = _bf_save(ctx_path);