diff --git a/doc/usage/bfcli.rst b/doc/usage/bfcli.rst index 8594b9be4..8d702e467 100644 --- a/doc/usage/bfcli.rst +++ b/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 diff --git a/src/bfcli/print.c b/src/bfcli/print.c index a15106e3f..b4fbb1cf2 100644 --- a/src/bfcli/print.c +++ b/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; diff --git a/src/libbpfilter/hook.c b/src/libbpfilter/hook.c index d5c467a07..829c9d4a7 100644 --- a/src/libbpfilter/hook.c +++ b/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 = diff --git a/src/libbpfilter/include/bpfilter/hook.h b/src/libbpfilter/include/bpfilter/hook.h index 630953c96..467ccb92c 100644 --- a/src/libbpfilter/include/bpfilter/hook.h +++ b/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, }; diff --git a/tests/e2e/cli/hookopts.sh b/tests/e2e/cli/hookopts.sh index 721a0abb0..3c9b16fae 100755 --- a/tests/e2e/cli/hookopts.sh +++ b/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") \ No newline at end of file +(! ${BFCLI} ruleset set --dry-run --from-str "chain priorities BF_HOOK_NF_LOCAL_IN{priorities=1-2,priorities=3-4} ACCEPT") diff --git a/tests/unit/libbpfilter/hook.c b/tests/unit/libbpfilter/hook.c index 59f6a3a56..c626643fc 100644 --- a/tests/unit/libbpfilter/hook.c +++ b/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),