hook: accept interface names in ifindex= option

doc/usage/bfcli.rst

@@ -384,10 +384,14 @@ With:
      - Required by
      - Supported by
      - Notes
+   * - ``iface=$INTERFACE``
+     - N/A
+     - ``BF_HOOK_XDP``, ``BF_HOOK_TC``
+     - Interface to attach the program to. ``$INTERFACE`` can be either an interface name (e.g. ``eth0``) or a numeric interface index. Interface names are resolved first; if resolution fails, the value is parsed as a numeric index.
    * - ``ifindex=$IFINDEX``
      - ``BF_HOOK_XDP``, ``BF_HOOK_TC``
      - N/A
-     - Interface index to attach the program to.
+     - Deprecated, use ``iface=`` instead. Only numeric interface indices are accepted. At most one of ``iface=`` or ``ifindex=`` may be set for a given chain.
    * - ``cgpath=$CGROUP_PATH``
      - ``BF_HOOK_CGROUP_SKB_*``, ``BF_HOOK_CGROUP_SOCK_ADDR_*``
      - N/A

src/bfcli/print.c

@@ -109,7 +109,11 @@ void bfc_chain_dump(struct bf_chain *chain, struct bf_hookopts *hookopts,
     if (hookopts) {
         (void)fprintf(stdout, "{");
 
-        if (bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFINDEX)) {
+        if (bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFACE)) {
+            (void)fprintf(stdout, "%siface=%d", need_comma ? "," : "",
+                          hookopts->ifindex);
+            need_comma = true;
+        } else if (bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFINDEX)) {
             (void)fprintf(stdout, "%sifindex=%d", need_comma ? "," : "",
                           hookopts->ifindex);
             need_comma = true;

src/libbpfilter/hook.c

@@ -21,6 +21,7 @@
 #include "bpfilter/dump.h"
 #include "bpfilter/flavor.h"
 #include "bpfilter/helper.h"
+#include "bpfilter/if.h"
 #include "bpfilter/logger.h"
 #include "bpfilter/pack.h"
 
@@ -203,9 +204,17 @@ static int _bf_hookopts_ifindex_parse(struct bf_hookopts *hookopts,
     assert(hookopts);
     assert(raw_opt);
 
+    if (hookopts->used_opts & BF_FLAG(BF_HOOKOPTS_IFACE)) {
+        return bf_err_r(-EEXIST,
+                        "ifindex= can't be set when iface= is already defined");
+    }
+
     if (hookopts->used_opts & BF_FLAG(BF_HOOKOPTS_IFINDEX))
         return bf_err_r(-EEXIST, "ifindex= is defined multiple times");
 
+    bf_warn(
+        "hook option 'ifindex=' is deprecated, use 'iface=' instead; 'iface=' accepts either an interface name or a numeric index");
+
     errno = 0;
     ifindex = strtoul(raw_opt, NULL, 0);
     if (errno != 0) {
@@ -222,12 +231,60 @@ static int _bf_hookopts_ifindex_parse(struct bf_hookopts *hookopts,
     return 0;
 }
 
+static int _bf_hookopts_iface_parse(struct bf_hookopts *hookopts,
+                                    const char *raw_opt)
+{
+    uint32_t ifindex;
+    int r;
+
+    assert(hookopts);
+    assert(raw_opt);
+
+    if (hookopts->used_opts & BF_FLAG(BF_HOOKOPTS_IFACE))
+        return bf_err_r(-EEXIST, "iface= is defined multiple times");
+
+    if (hookopts->used_opts & BF_FLAG(BF_HOOKOPTS_IFINDEX)) {
+        return bf_err_r(-EEXIST,
+                        "iface= can't be set when ifindex= is already defined");
+    }
+
+    r = bf_if_index_from_str(raw_opt, &ifindex);
+    if (r)
+        return bf_err_r(r, "failed to parse iface from '%s'", raw_opt);
+
+    if (ifindex > INT_MAX)
+        return bf_err_r(-E2BIG, "iface resolves to ifindex too big: %u",
+                        ifindex);
+
+    hookopts->ifindex = (int)ifindex;
+    hookopts->used_opts |= BF_FLAG(BF_HOOKOPTS_IFACE);
+    hookopts->used_opts |= BF_FLAG(BF_HOOKOPTS_IFINDEX);
+
+    return 0;
+}
+
+static void _bf_hookopts_iface_dump(const struct bf_hookopts *hookopts,
+                                    prefix_t *prefix)
+{
+    assert(hookopts);
+    assert(prefix);
+
+    DUMP(prefix, "iface: %d", hookopts->ifindex);
+}
+
 static void _bf_hookopts_ifindex_dump(const struct bf_hookopts *hookopts,
                                       prefix_t *prefix)
 {
     assert(hookopts);
     assert(prefix);
 
+    /* iface= sets both BF_HOOKOPTS_IFACE and BF_HOOKOPTS_IFINDEX so the
+     * XDP/TC ifindex requirement in bf_hookopts_validate is satisfied. In
+     * that case _bf_hookopts_iface_dump has already emitted the value;
+     * skip the ifindex line to avoid a duplicate dump entry. */
+    if (bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFACE))
+        return;
+
     DUMP(prefix, "ifindex: %d", hookopts->ifindex);
 }
 
@@ -366,6 +423,12 @@ static struct bf_hookopts_ops
     int (*parse)(struct bf_hookopts *, const char *);
     void (*dump)(const struct bf_hookopts *, prefix_t *);
 } _bf_hookopts_ops[] = {
+    [BF_HOOKOPTS_IFACE] = {.name = "iface",
+                           .type = BF_HOOKOPTS_IFACE,
+                           .required_by = 0,
+                           .supported_by = BF_FLAGS(BF_FLAVOR_XDP, BF_FLAVOR_TC),
+                           .parse = _bf_hookopts_iface_parse,
+                           .dump = _bf_hookopts_iface_dump},
     [BF_HOOKOPTS_IFINDEX] = {.name = "ifindex",
                              .type = BF_HOOKOPTS_IFINDEX,
                              .required_by =

src/libbpfilter/include/bpfilter/hook.h

@@ -157,6 +157,7 @@ enum bf_hookopts_type
     BF_HOOKOPTS_CGPATH,
     BF_HOOKOPTS_FAMILY,
     BF_HOOKOPTS_PRIORITIES,
+    BF_HOOKOPTS_IFACE,
     _BF_HOOKOPTS_MAX,
 };
 

tests/e2e/cli/hookopts.sh

@@ -4,6 +4,9 @@
 
 # Disallow duplicated hook options
 (! ${BFCLI} ruleset set --dry-run --from-str "chain ifindex BF_HOOK_XDP{ifindex=2,ifindex=3} ACCEPT")
+(! ${BFCLI} ruleset set --dry-run --from-str "chain iface BF_HOOK_XDP{iface=lo,iface=lo} ACCEPT")
+(! ${BFCLI} ruleset set --dry-run --from-str "chain iface_ifindex BF_HOOK_XDP{ifindex=2,iface=lo} ACCEPT")
+(! ${BFCLI} ruleset set --dry-run --from-str "chain ifindex_iface BF_HOOK_XDP{iface=lo,ifindex=2} ACCEPT")
 (! ${BFCLI} ruleset set --dry-run --from-str "chain cgpath BF_HOOK_CGROUP_SKB_INGRESS{cgpath=/sys/fs/cgroup,cgpath=/sys/fs/cgroup} ACCEPT")
 (! ${BFCLI} ruleset set --dry-run --from-str "chain family BF_HOOK_NF_LOCAL_IN{family=inet4,family=inet6} ACCEPT")
-(! ${BFCLI} ruleset set --dry-run --from-str "chain priorities BF_HOOK_NF_LOCAL_IN{priorities=1-2,priorities=3-4} ACCEPT")
+(! ${BFCLI} ruleset set --dry-run --from-str "chain priorities BF_HOOK_NF_LOCAL_IN{priorities=1-2,priorities=3-4} ACCEPT")

tests/unit/libbpfilter/hook.c

@@ -228,6 +228,86 @@ static void hookopts_parse_ifindex(void **state)
     assert_int_equal(hookopts->ifindex, 42);
 }
 
+static void hookopts_parse_iface(void **state)
+{
+    (void)state;
+
+    // iface= accepts a numeric interface index
+    {
+        _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL;
+        char opt[] = "iface=42";
+
+        assert_ok(bf_hookopts_new(&hookopts));
+        assert_ok(bf_hookopts_parse_opt(hookopts, opt));
+        assert_int_equal(hookopts->ifindex, 42);
+        assert_true(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFACE));
+        assert_true(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFINDEX));
+    }
+
+    // iface= accepts the loopback interface by name (always present)
+    {
+        _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL;
+        char opt[] = "iface=lo";
+
+        assert_ok(bf_hookopts_new(&hookopts));
+        assert_ok(bf_hookopts_parse_opt(hookopts, opt));
+        assert_true(hookopts->ifindex > 0);
+        assert_true(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFACE));
+        assert_true(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFINDEX));
+    }
+
+    // iface= rejects a non-existent interface name that isn't numeric
+    {
+        _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL;
+        char opt[] = "iface=this_iface_does_not_exist_xyz";
+
+        assert_ok(bf_hookopts_new(&hookopts));
+        assert_err(bf_hookopts_parse_opt(hookopts, opt));
+        assert_false(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFACE));
+        assert_false(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFINDEX));
+    }
+
+    // iface= cannot be specified twice
+    {
+        _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL;
+        char opt1[] = "iface=42";
+        char opt2[] = "iface=100";
+
+        assert_ok(bf_hookopts_new(&hookopts));
+        assert_ok(bf_hookopts_parse_opt(hookopts, opt1));
+        assert_err(bf_hookopts_parse_opt(hookopts, opt2));
+        assert_int_equal(hookopts->ifindex, 42);
+    }
+
+    // ifindex= followed by iface= is rejected
+    {
+        _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL;
+        char opt1[] = "ifindex=42";
+        char opt2[] = "iface=100";
+
+        assert_ok(bf_hookopts_new(&hookopts));
+        assert_ok(bf_hookopts_parse_opt(hookopts, opt1));
+        assert_err(bf_hookopts_parse_opt(hookopts, opt2));
+        assert_int_equal(hookopts->ifindex, 42);
+        assert_false(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFACE));
+        assert_true(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFINDEX));
+    }
+
+    // iface= followed by ifindex= is rejected
+    {
+        _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL;
+        char opt1[] = "iface=42";
+        char opt2[] = "ifindex=100";
+
+        assert_ok(bf_hookopts_new(&hookopts));
+        assert_ok(bf_hookopts_parse_opt(hookopts, opt1));
+        assert_err(bf_hookopts_parse_opt(hookopts, opt2));
+        assert_int_equal(hookopts->ifindex, 42);
+        assert_true(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFACE));
+        assert_true(bf_hookopts_is_used(hookopts, BF_HOOKOPTS_IFINDEX));
+    }
+}
+
 static void hookopts_parse_cgpath(void **state)
 {
     _free_bf_hookopts_ struct bf_hookopts *hookopts = NULL;
@@ -743,6 +823,7 @@ int main(void)
         cmocka_unit_test(nf_hook_to_str),
         cmocka_unit_test(hookopts_new_and_free),
         cmocka_unit_test(hookopts_parse_ifindex),
+        cmocka_unit_test(hookopts_parse_iface),
         cmocka_unit_test(hookopts_parse_cgpath),
         cmocka_unit_test(hookopts_parse_family),
         cmocka_unit_test(hookopts_parse_priorities),