diff --git a/doc/usage/bfcli.rst b/doc/usage/bfcli.rst index 1d85e3f92..45ad813ef 100644 --- a/doc/usage/bfcli.rst +++ b/doc/usage/bfcli.rst @@ -340,7 +340,13 @@ With: - ``BF_HOOK_NF_LOCAL_OUT``: similar to ``nftables`` and ``iptables`` output hook. - ``BF_HOOK_NF_POST_ROUTING``: similar to ``nftables`` and ``iptables`` postrouting hook. - ``BF_HOOK_TC_EGRESS``: egress TC hook. - - ``$POLICY``: action taken if no rule matches the packet, either ``ACCEPT`` forward the packet to the kernel, or ``DROP`` to discard it. Note while ``CONTINUE`` is a valid verdict for rules, it is not supported for chain policy. + - ``$POLICY``: action taken if no rule matches the packet: + + - ``ACCEPT``: forward the packet to the kernel. + - ``DROP``: discard the packet. + - ``NEXT``: pass the packet to the next BPF program. For TC hooks, this maps to ``TCX_NEXT``, deferring the decision to the next program in the TCX link. For NF, XDP, and cgroup_skb hooks, ``NEXT`` behaves identically to ``ACCEPT`` since these hooks do not distinguish between "accept" and "pass to next program." + + Note: ``CONTINUE`` and ``REDIRECT`` are not valid chain policies. ``$OPTIONS`` are hook-specific comma separated key value pairs. A given hook option can only be specified once: @@ -391,13 +397,14 @@ With: - ``log``: optional. If set, log the requested protocol headers. ``link`` will log the link (layer 2) header, ``internet`` with log the internet (layer 3) header, and ``transport`` will log the transport (layer 4) header. At least one header type is required. - ``counter``: optional literal. If set, the filter will counter the number of packets and bytes matched by the rule. - ``mark``: optional, ``$MARK`` must be a valid decimal or hexadecimal 32-bits value. If set, write the packet's marker value. This marker can be used later on in a rule (see ``meta.mark``) or with a TC filter. - - ``$VERDICT``: action taken by the rule if the packet is matched against **all** the criteria: either ``ACCEPT``, ``DROP``, ``CONTINUE``, or ``REDIRECT``. + - ``$VERDICT``: action taken by the rule if the packet is matched against **all** the criteria: either ``ACCEPT``, ``DROP``, ``CONTINUE``, ``NEXT``, or ``REDIRECT``. - ``ACCEPT``: forward the packet to the kernel. - ``DROP``: discard the packet. - - ``CONTINUE``: continue processing subsequent rules. + - ``CONTINUE``: continue processing subsequent rules (non-terminal). + - ``NEXT``: stop rule processing and pass the packet to the next BPF program. For TC hooks, this returns ``TCX_NEXT``. For NF, XDP, and cgroup_skb hooks, this is equivalent to ``ACCEPT``. - ``REDIRECT $IFACE $DIR``: redirect the packet to interface ``$IFACE`` in direction ``$DIR``. ``$IFACE`` can be an interface name (e.g., "eth0") or an interface index (e.g., "2"). ``$DIR`` is either ``in`` for ingress or ``out`` for egress. -In a chain, as soon as a rule matches a packet, its verdict is applied. If the verdict is ``ACCEPT``, ``DROP``, or ``REDIRECT``, the subsequent rules are not processed. Hence, the rules' order matters. If no rule matches the packet, the chain's policy is applied. +In a chain, as soon as a rule matches a packet, its verdict is applied. If the verdict is ``ACCEPT``, ``DROP``, ``NEXT``, or ``REDIRECT``, the subsequent rules are not processed. Hence, the rules' order matters. If no rule matches the packet, the chain's policy is applied. Note ``CONTINUE`` means a packet can be counted more than once if multiple rules specify ``CONTINUE`` and ``counter``. @@ -409,6 +416,10 @@ Note ``CONTINUE`` means a packet can be counted more than once if multiple rules ``REDIRECT`` is **not** supported by Netfilter (``BF_HOOK_NF_*``) or cgroup_skb (``BF_HOOK_CGROUP_SKB_*``) hooks. +.. note:: + + ``NEXT`` has distinct behavior only for TC hooks (``BF_HOOK_TC_INGRESS``, ``BF_HOOK_TC_EGRESS``), where it maps to ``TCX_NEXT`` and defers to the next BPF program in the TCX link. For all other hooks (Netfilter, XDP, cgroup_skb), ``NEXT`` produces the same return code as ``ACCEPT``. + Sets ~~~~ diff --git a/src/bfcli/lexer.l b/src/bfcli/lexer.l index 8a04987a7..a019e71dc 100644 --- a/src/bfcli/lexer.l +++ b/src/bfcli/lexer.l @@ -68,7 +68,7 @@ BF_HOOK_[A-Z0-9_]+ { BEGIN(STATE_HOOK_OPTS); yylval.sval = strdup(yytext); retur } } /* Verdicts */ -(ACCEPT|DROP|CONTINUE) { yylval.sval = strdup(yytext); return VERDICT; } +(ACCEPT|DROP|CONTINUE|NEXT) { yylval.sval = strdup(yytext); return VERDICT; } REDIRECT { BEGIN(STATE_REDIRECT_IFACE); return REDIRECT_TOKEN; } { {int}|[0-9a-zA-Z_-]+ { diff --git a/src/bfcli/parser.y b/src/bfcli/parser.y index ffa177095..4323e074b 100644 --- a/src/bfcli/parser.y +++ b/src/bfcli/parser.y @@ -160,8 +160,8 @@ chain : CHAIN STRING hook hookopts verdict sets rules _free_bf_list_ bf_list *rules = $7; int r; - if ($5 >= _BF_TERMINAL_VERDICT_MAX) - bf_parse_err("'%s' is not supported for chains\n", bf_verdict_to_str($5)); + if (!bf_verdict_is_valid_policy($5)) + bf_parse_err("'%s' is not a valid chain policy\n", bf_verdict_to_str($5)); if (bf_chain_new(&chain, name, $3, $5, &ruleset->sets, rules) < 0) bf_parse_err("failed to create a new bf_chain\n"); diff --git a/src/bpfilter/cgen/cgroup_skb.c b/src/bpfilter/cgen/cgroup_skb.c index 459238e0e..428b1222a 100644 --- a/src/bpfilter/cgen/cgroup_skb.c +++ b/src/bpfilter/cgen/cgroup_skb.c @@ -18,7 +18,6 @@ #include #include -#include "cgen/cgen.h" #include "cgen/matcher/cmp.h" #include "cgen/matcher/packet.h" #include "cgen/program.h" @@ -141,14 +140,20 @@ static int _bf_cgroup_skb_gen_inline_matcher(struct bf_program *program, * Convert a standard verdict into a return value. * * @param verdict Verdict to convert. Must be valid. - * @return Cgroup return code corresponding to the verdict, as an integer. + * @param ret_code Cgroup return code. Can't be NULL. + * @return 0 on success, or a negative errno value on failure. */ -static int _bf_cgroup_skb_get_verdict(enum bf_verdict verdict) +static int _bf_cgroup_skb_get_verdict(enum bf_verdict verdict, int *ret_code) { + assert(ret_code); + switch (verdict) { case BF_VERDICT_ACCEPT: - return 1; + case BF_VERDICT_NEXT: + *ret_code = 1; + return 0; case BF_VERDICT_DROP: + *ret_code = 0; return 0; default: return -ENOTSUP; diff --git a/src/bpfilter/cgen/cgroup_sock_addr.c b/src/bpfilter/cgen/cgroup_sock_addr.c index 66b21fb73..06cd07b17 100644 --- a/src/bpfilter/cgen/cgroup_sock_addr.c +++ b/src/bpfilter/cgen/cgroup_sock_addr.c @@ -86,14 +86,21 @@ _bf_cgroup_sock_addr_gen_inline_matcher(struct bf_program *program, * @brief Convert a standard verdict into a return value. * * @param verdict Verdict to convert. Must be valid. - * @return Cgroup return code corresponding to the verdict, as an integer. + * @param ret_code Cgroup return code. Can't be NULL. + * @return 0 on success, or a negative errno value on failure. */ -static int _bf_cgroup_sock_addr_get_verdict(enum bf_verdict verdict) +static int _bf_cgroup_sock_addr_get_verdict(enum bf_verdict verdict, + int *ret_code) { + assert(ret_code); + switch (verdict) { case BF_VERDICT_ACCEPT: - return 1; + case BF_VERDICT_NEXT: + *ret_code = 1; + return 0; case BF_VERDICT_DROP: + *ret_code = 0; return 0; default: return -ENOTSUP; diff --git a/src/bpfilter/cgen/jmp.h b/src/bpfilter/cgen/jmp.h index 87298980c..3a634687e 100644 --- a/src/bpfilter/cgen/jmp.h +++ b/src/bpfilter/cgen/jmp.h @@ -20,10 +20,14 @@ * { * _clean_bf_jmpctx_ struct bf_jmpctx ctx = * bf_jmpctx_get(program, BPF_JMP_IMM(BPF_JEQ, BPF_REG_2, 0, 0)); + * int ret; + * int r; * - * EMIT(program, - * BPF_MOV64_IMM(BPF_REG_0, program->runtime.ops->get_verdict( - * BF_VERDICT_ACCEPT))); + * r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT, &ret); + * if (r) + * return r; + * + * EMIT(program, BPF_MOV64_IMM(BPF_REG_0, ret)); * EMIT(program, BPF_EXIT_INSN()); * } * @endcode diff --git a/src/bpfilter/cgen/nf.c b/src/bpfilter/cgen/nf.c index 08c596e81..671ed87f0 100644 --- a/src/bpfilter/cgen/nf.c +++ b/src/bpfilter/cgen/nf.c @@ -157,15 +157,21 @@ static int _bf_nf_gen_inline_matcher(struct bf_program *program, * Convert a standard verdict into a return value. * * @param verdict Verdict to convert. Must be valid. - * @return Netfilter return code corresponding to the verdict, as an integer. + * @param ret_code Netfilter return code. Can't be NULL. + * @return 0 on success, or a negative errno value on failure. */ -static int _bf_nf_get_verdict(enum bf_verdict verdict) +static int _bf_nf_get_verdict(enum bf_verdict verdict, int *ret_code) { + assert(ret_code); + switch (verdict) { case BF_VERDICT_ACCEPT: - return NF_ACCEPT; + case BF_VERDICT_NEXT: + *ret_code = NF_ACCEPT; + return 0; case BF_VERDICT_DROP: - return NF_DROP; + *ret_code = NF_DROP; + return 0; default: return -ENOTSUP; } diff --git a/src/bpfilter/cgen/program.c b/src/bpfilter/cgen/program.c index c89f13635..c9ab16e34 100644 --- a/src/bpfilter/cgen/program.c +++ b/src/bpfilter/cgen/program.c @@ -287,6 +287,7 @@ static int _bf_program_generate_rule(struct bf_program *program, struct bf_rule *rule) { uint32_t checked_layers = 0; + int ret_code; int r; assert(program); @@ -371,10 +372,11 @@ static int _bf_program_generate_rule(struct bf_program *program, switch (rule->verdict) { case BF_VERDICT_ACCEPT: case BF_VERDICT_DROP: - r = program->runtime.ops->get_verdict(rule->verdict); - if (r < 0) + case BF_VERDICT_NEXT: + r = program->runtime.ops->get_verdict(rule->verdict, &ret_code); + if (r) return r; - EMIT(program, BPF_MOV64_IMM(BPF_REG_0, r)); + EMIT(program, BPF_MOV64_IMM(BPF_REG_0, ret_code)); EMIT(program, BPF_EXIT_INSN()); break; case BF_VERDICT_REDIRECT: @@ -576,6 +578,7 @@ int bf_program_emit_fixup_elfstub(struct bf_program *program, int bf_program_generate(struct bf_program *program) { const struct bf_chain *chain = program->runtime.chain; + int ret_code; int r; // Save the program's argument into the context. @@ -623,10 +626,10 @@ int bf_program_generate(struct bf_program *program) BPF_MOV32_IMM(BPF_REG_3, bf_program_chain_counter_idx(program))); EMIT_FIXUP_ELFSTUB(program, BF_ELFSTUB_UPDATE_COUNTERS); - r = program->runtime.ops->get_verdict(chain->policy); - if (r < 0) + r = program->runtime.ops->get_verdict(chain->policy, &ret_code); + if (r) return r; - EMIT(program, BPF_MOV64_IMM(BPF_REG_0, r)); + EMIT(program, BPF_MOV64_IMM(BPF_REG_0, ret_code)); EMIT(program, BPF_EXIT_INSN()); r = _bf_program_generate_elfstubs(program); diff --git a/src/bpfilter/cgen/stub.c b/src/bpfilter/cgen/stub.c index f867d6c34..47e64771c 100644 --- a/src/bpfilter/cgen/stub.c +++ b/src/bpfilter/cgen/stub.c @@ -47,6 +47,7 @@ static int _bf_stub_make_ctx_dynptr(struct bf_program *program, int arg_reg, const char *kfunc) { + int ret_code; int r; assert(program); @@ -76,10 +77,11 @@ static int _bf_stub_make_ctx_dynptr(struct bf_program *program, int arg_reg, if (bf_ctx_is_verbose(BF_VERBOSE_BPF)) EMIT_PRINT(program, "failed to create a new dynamic pointer"); - r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT); - if (r < 0) + r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT, &ret_code); + if (r) return r; - EMIT(program, BPF_MOV64_IMM(BPF_REG_0, r)); + + EMIT(program, BPF_MOV64_IMM(BPF_REG_0, ret_code)); EMIT(program, BPF_EXIT_INSN()); } @@ -102,6 +104,7 @@ int bf_stub_make_ctx_skb_dynptr(struct bf_program *program, int skb_reg) int bf_stub_parse_l2_ethhdr(struct bf_program *program) { + int ret_code; int r; assert(program); @@ -135,10 +138,11 @@ int bf_stub_parse_l2_ethhdr(struct bf_program *program) if (bf_ctx_is_verbose(BF_VERBOSE_BPF)) EMIT_PRINT(program, "failed to create L2 dynamic pointer slice"); - r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT); - if (r < 0) + r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT, &ret_code); + if (r) return r; - EMIT(program, BPF_MOV64_IMM(BPF_REG_0, r)); + + EMIT(program, BPF_MOV64_IMM(BPF_REG_0, ret_code)); EMIT(program, BPF_EXIT_INSN()); } @@ -160,6 +164,7 @@ int bf_stub_parse_l2_ethhdr(struct bf_program *program) int bf_stub_parse_l3_hdr(struct bf_program *program) { _clean_bf_jmpctx_ struct bf_jmpctx _ = bf_jmpctx_default(); + int ret_code; int r; assert(program); @@ -210,10 +215,11 @@ int bf_stub_parse_l3_hdr(struct bf_program *program) if (bf_ctx_is_verbose(BF_VERBOSE_BPF)) EMIT_PRINT(program, "failed to create L3 dynamic pointer slice"); - r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT); - if (r < 0) + r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT, &ret_code); + if (r) return r; - EMIT(program, BPF_MOV64_IMM(BPF_REG_0, r)); + + EMIT(program, BPF_MOV64_IMM(BPF_REG_0, ret_code)); EMIT(program, BPF_EXIT_INSN()); } @@ -316,6 +322,7 @@ int bf_stub_parse_l3_hdr(struct bf_program *program) int bf_stub_parse_l4_hdr(struct bf_program *program) { _clean_bf_jmpctx_ struct bf_jmpctx _ = bf_jmpctx_default(); + int ret_code; int r; assert(program); @@ -369,10 +376,11 @@ int bf_stub_parse_l4_hdr(struct bf_program *program) if (bf_ctx_is_verbose(BF_VERBOSE_BPF)) EMIT_PRINT(program, "failed to create L4 dynamic pointer slice"); - r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT); - if (r < 0) + r = program->runtime.ops->get_verdict(BF_VERDICT_ACCEPT, &ret_code); + if (r) return r; - EMIT(program, BPF_MOV64_IMM(BPF_REG_0, r)); + + EMIT(program, BPF_MOV64_IMM(BPF_REG_0, ret_code)); EMIT(program, BPF_EXIT_INSN()); } diff --git a/src/bpfilter/cgen/tc.c b/src/bpfilter/cgen/tc.c index d83fcd411..cfcd3d84b 100644 --- a/src/bpfilter/cgen/tc.c +++ b/src/bpfilter/cgen/tc.c @@ -18,7 +18,6 @@ #include #include -#include "cgen/cgen.h" #include "cgen/matcher/cmp.h" #include "cgen/matcher/packet.h" #include "cgen/program.h" @@ -154,15 +153,23 @@ static int _bf_tc_gen_inline_redirect(struct bf_program *program, * Convert a standard verdict into a return value. * * @param verdict Verdict to convert. Must be valid. - * @return TC return code corresponding to the verdict, as an integer. + * @param ret_code TC return code. Can't be NULL. + * @return 0 on success, or a negative errno value on failure. */ -static int _bf_tc_get_verdict(enum bf_verdict verdict) +static int _bf_tc_get_verdict(enum bf_verdict verdict, int *ret_code) { + assert(ret_code); + switch (verdict) { case BF_VERDICT_ACCEPT: - return TCX_PASS; + *ret_code = TCX_PASS; + return 0; case BF_VERDICT_DROP: - return TCX_DROP; + *ret_code = TCX_DROP; + return 0; + case BF_VERDICT_NEXT: + *ret_code = TCX_NEXT; + return 0; default: return -ENOTSUP; } diff --git a/src/bpfilter/cgen/xdp.c b/src/bpfilter/cgen/xdp.c index a57ddeeca..53fd5a7c1 100644 --- a/src/bpfilter/cgen/xdp.c +++ b/src/bpfilter/cgen/xdp.c @@ -107,13 +107,25 @@ static int _bf_xdp_gen_inline_redirect(struct bf_program *program, return 0; } -static int _bf_xdp_get_verdict(enum bf_verdict verdict) +/** + * Convert a standard verdict into a return value. + * + * @param verdict Verdict to convert. Must be valid. + * @param ret_code XDP return code. Can't be NULL. + * @return 0 on success, or a negative errno value on failure. + */ +static int _bf_xdp_get_verdict(enum bf_verdict verdict, int *ret_code) { + assert(ret_code); + switch (verdict) { case BF_VERDICT_ACCEPT: - return XDP_PASS; + case BF_VERDICT_NEXT: + *ret_code = XDP_PASS; + return 0; case BF_VERDICT_DROP: - return XDP_DROP; + *ret_code = XDP_DROP; + return 0; default: return -ENOTSUP; } diff --git a/src/libbpfilter/chain.c b/src/libbpfilter/chain.c index 1fe5db666..ad9784813 100644 --- a/src/libbpfilter/chain.c +++ b/src/libbpfilter/chain.c @@ -185,8 +185,9 @@ int bf_chain_new(struct bf_chain **chain, const char *name, enum bf_hook hook, assert(chain && name); if (hook >= _BF_HOOK_MAX) return bf_err_r(-EINVAL, "unknown hook type"); - if (policy >= _BF_TERMINAL_VERDICT_MAX) - return bf_err_r(-EINVAL, "unknown policy type"); + if (!bf_verdict_is_valid_policy(policy)) + return bf_err_r(-EINVAL, "invalid policy '%s'", + bf_verdict_to_str(policy)); _chain = malloc(sizeof(*_chain)); if (!_chain) @@ -240,7 +241,7 @@ int bf_chain_new_from_pack(struct bf_chain **chain, bf_rpack_node_t node) if (r) return bf_rpack_key_err(r, "bf_chain.hook"); - r = bf_rpack_kv_enum(node, "policy", &policy, 0, _BF_TERMINAL_VERDICT_MAX); + r = bf_rpack_kv_enum(node, "policy", &policy, 0, _BF_VERDICT_MAX); if (r) return bf_rpack_key_err(r, "bf_chain.policy"); diff --git a/src/libbpfilter/include/bpfilter/flavor.h b/src/libbpfilter/include/bpfilter/flavor.h index 51c7440dd..7c70ecf15 100644 --- a/src/libbpfilter/include/bpfilter/flavor.h +++ b/src/libbpfilter/include/bpfilter/flavor.h @@ -121,10 +121,12 @@ struct bf_flavor_ops * stop further packet processing. Non-terminal verdicts do not need return * codes and therefore do not need to be handled by get_verdict(). * - * @return Flavor-specific verdict for `verdict`, or a negative errno - * value on failure. + * @param verdict Verdict to convert. Must be a terminal verdict. + * @param ret_code On success, set to the flavor-specific BPF return code. + * Can't be NULL. + * @return 0 on success, or negative errno value on failure. */ - int (*get_verdict)(enum bf_verdict verdict); + int (*get_verdict)(enum bf_verdict verdict, int *ret_code); /** * @brief Generate bytecode for a matcher. Required for all flavors. diff --git a/src/libbpfilter/include/bpfilter/verdict.h b/src/libbpfilter/include/bpfilter/verdict.h index c9d0abaea..0075ea068 100644 --- a/src/libbpfilter/include/bpfilter/verdict.h +++ b/src/libbpfilter/include/bpfilter/verdict.h @@ -5,6 +5,8 @@ #pragma once +#include + /** * Redirect direction for the REDIRECT verdict. */ @@ -36,22 +38,28 @@ int bf_redirect_dir_from_str(const char *str, enum bf_redirect_dir *dir); /** * Verdict to apply for a rule or chain. - * Chains can only use terminal verdicts, rules can use all verdicts. + * + * Only some verdicts are valid as chain policies (see + * `bf_verdict_is_valid_policy`). Rules can use all verdicts. */ enum bf_verdict { - /** Terminal verdicts that stop further packet processing. */ - /** Accept the packet. */ + /** Accept the packet. Terminal. */ BF_VERDICT_ACCEPT, - /** Drop the packet. */ + /** Drop the packet. Terminal. */ BF_VERDICT_DROP, - /** Non-terminal verdicts that allow further packet processing. */ - /** Continue processing the next rule. */ + /** Continue processing the next rule. Non-terminal. */ BF_VERDICT_CONTINUE, - /** Redirect the packet to another interface. */ + /** Redirect the packet to another interface. Terminal. */ BF_VERDICT_REDIRECT, + /** Pass the packet to the next BPF program. Terminal. + * + * For TC, this maps to TCX_NEXT which defers to the next program in + * the TCX link. For NF, XDP, and cgroup_skb, NEXT maps to the same + * return code as ACCEPT since these hooks do not distinguish between + * "accept" and "pass to next program." */ + BF_VERDICT_NEXT, _BF_VERDICT_MAX, - _BF_TERMINAL_VERDICT_MAX = BF_VERDICT_REDIRECT, }; /** @@ -70,3 +78,15 @@ const char *bf_verdict_to_str(enum bf_verdict verdict); * @return 0 on success, or negative errno value on error. */ int bf_verdict_from_str(const char *str, enum bf_verdict *verdict); + +/** + * Check if a verdict is valid as a chain policy. + * + * Only ACCEPT, DROP, and NEXT are valid chain policies. CONTINUE is + * non-terminal and cannot be a default action. REDIRECT requires per-rule + * parameters (interface, direction) so it cannot be a policy either. + * + * @param verdict Verdict to check. + * @return true if the verdict is valid as a chain policy, false otherwise. + */ +bool bf_verdict_is_valid_policy(enum bf_verdict verdict); diff --git a/src/libbpfilter/verdict.c b/src/libbpfilter/verdict.c index 6fe2a6e33..83ea5ce4a 100644 --- a/src/libbpfilter/verdict.c +++ b/src/libbpfilter/verdict.c @@ -40,10 +40,9 @@ int bf_redirect_dir_from_str(const char *str, enum bf_redirect_dir *dir) } static const char *_bf_verdict_strs[] = { - [BF_VERDICT_ACCEPT] = "ACCEPT", - [BF_VERDICT_DROP] = "DROP", - [BF_VERDICT_REDIRECT] = "REDIRECT", - [BF_VERDICT_CONTINUE] = "CONTINUE", + [BF_VERDICT_ACCEPT] = "ACCEPT", [BF_VERDICT_DROP] = "DROP", + [BF_VERDICT_REDIRECT] = "REDIRECT", [BF_VERDICT_CONTINUE] = "CONTINUE", + [BF_VERDICT_NEXT] = "NEXT", }; static_assert_enum_mapping(_bf_verdict_strs, _BF_VERDICT_MAX); @@ -68,3 +67,15 @@ int bf_verdict_from_str(const char *str, enum bf_verdict *verdict) return -EINVAL; } + +bool bf_verdict_is_valid_policy(enum bf_verdict verdict) +{ + switch (verdict) { + case BF_VERDICT_ACCEPT: + case BF_VERDICT_DROP: + case BF_VERDICT_NEXT: + return true; + default: + return false; + } +} diff --git a/tests/e2e/CMakeLists.txt b/tests/e2e/CMakeLists.txt index 12ac79093..13607104f 100644 --- a/tests/e2e/CMakeLists.txt +++ b/tests/e2e/CMakeLists.txt @@ -109,6 +109,7 @@ bf_add_e2e_test(e2e rules/icmp_tc.sh ROOT) bf_add_e2e_test(e2e rules/icmp_xdp.sh ROOT) bf_add_e2e_test(e2e rules/log.sh ROOT) bf_add_e2e_test(e2e rules/mark.sh ROOT) +bf_add_e2e_test(e2e rules/next_tc.sh ROOT) bf_add_e2e_test(e2e rules/redirect.sh ROOT) bf_add_e2e_test(e2e rulesets/rulesets.sh ROOT) diff --git a/tests/e2e/rules/next_tc.sh b/tests/e2e/rules/next_tc.sh new file mode 100755 index 000000000..9ff14fd57 --- /dev/null +++ b/tests/e2e/rules/next_tc.sh @@ -0,0 +1,92 @@ +#!/usr/bin/env bash + +. "$(dirname "$0")"/../e2e_test_util.sh + +get_counter() { + ${FROM_NS} bpftool map dump pinned ${WORKDIR}/bpf/bpfilter/$1/bf_cmap | jq ".[$2].value.count" +} + +make_sandbox +start_bpfilter + +# --- NEXT as chain policy (single chain) --- +# Without a next program in the TCX link, NEXT allows the packet through. +ping -c 1 -W 0.1 ${NS_IP_ADDR} +${FROM_NS} bfcli chain set --from-str \ + "chain pol_next BF_HOOK_TC_INGRESS{ifindex=${NS_IFINDEX}} NEXT" +ping -c 1 -W 0.1 ${NS_IP_ADDR} +${FROM_NS} bfcli ruleset flush + +# --- NEXT is terminal (stops rule evaluation) --- +# First rule matches ICMP and returns NEXT. +# Second rule also matches ICMP with a counter and returns DROP. +# NEXT is terminal: second rule is never evaluated, counter stays 0, ping +# succeeds (TCX_NEXT with no next program passes the packet). +${FROM_NS} bfcli chain set --from-str \ + "chain next_term BF_HOOK_TC_INGRESS{ifindex=${NS_IFINDEX}} ACCEPT \ + rule ip4.proto icmp counter NEXT \ + rule ip4.proto icmp counter DROP" +ping -c 1 -W 0.1 ${NS_IP_ADDR} +test "$(get_counter next_term 0)" = "1" +test "$(get_counter next_term 1)" = "0" +${FROM_NS} bfcli ruleset flush + +# --- TC ingress: NEXT defers to the next program --- +# Two chains on the same TC_INGRESS hook. Chain A returns NEXT for ICMP, +# deferring to chain B which drops the packet. +${FROM_NS} bfcli chain set --from-str \ + "chain ing_next_a BF_HOOK_TC_INGRESS{ifindex=${NS_IFINDEX}} ACCEPT \ + rule ip4.proto icmp counter NEXT" +${FROM_NS} bfcli chain set --from-str \ + "chain ing_next_b BF_HOOK_TC_INGRESS{ifindex=${NS_IFINDEX}} ACCEPT \ + rule ip4.proto icmp counter DROP" +test "$(get_counter ing_next_a 0)" = "0" +test "$(get_counter ing_next_b 0)" = "0" +(! ping -c 1 -W 0.1 ${NS_IP_ADDR}) +test "$(get_counter ing_next_a 0)" = "1" +test "$(get_counter ing_next_b 0)" = "1" +${FROM_NS} bfcli ruleset flush + +# --- TC ingress: ACCEPT does NOT defer to the next program --- +# Same setup but chain A uses ACCEPT. TCX_PASS bypasses chain B entirely. +${FROM_NS} bfcli chain set --from-str \ + "chain ing_accept_a BF_HOOK_TC_INGRESS{ifindex=${NS_IFINDEX}} ACCEPT \ + rule ip4.proto icmp counter ACCEPT" +${FROM_NS} bfcli chain set --from-str \ + "chain ing_accept_b BF_HOOK_TC_INGRESS{ifindex=${NS_IFINDEX}} ACCEPT \ + rule ip4.proto icmp counter DROP" +test "$(get_counter ing_accept_a 0)" = "0" +test "$(get_counter ing_accept_b 0)" = "0" +ping -c 1 -W 0.1 ${NS_IP_ADDR} +test "$(get_counter ing_accept_a 0)" = "1" +test "$(get_counter ing_accept_b 0)" = "0" +${FROM_NS} bfcli ruleset flush + +# --- TC ingress: NEXT policy defers to the next program --- +# Chain A has NEXT as default policy (no matching rules), deferring all +# packets to chain B. +${FROM_NS} bfcli chain set --from-str \ + "chain ing_pol_next BF_HOOK_TC_INGRESS{ifindex=${NS_IFINDEX}} NEXT" +${FROM_NS} bfcli chain set --from-str \ + "chain ing_pol_drop BF_HOOK_TC_INGRESS{ifindex=${NS_IFINDEX}} ACCEPT \ + rule ip4.proto icmp counter DROP" +test "$(get_counter ing_pol_drop 0)" = "0" +(! ping -c 1 -W 0.1 ${NS_IP_ADDR}) +test "$(get_counter ing_pol_drop 0)" = "1" +${FROM_NS} bfcli ruleset flush + +# --- TC egress: NEXT defers to the next program --- +# Same multi-chain test on the egress path. The ICMP echo reply is +# intercepted on its way out of the namespace. +${FROM_NS} bfcli chain set --from-str \ + "chain egr_next_a BF_HOOK_TC_EGRESS{ifindex=${NS_IFINDEX}} ACCEPT \ + rule ip4.proto icmp counter NEXT" +${FROM_NS} bfcli chain set --from-str \ + "chain egr_next_b BF_HOOK_TC_EGRESS{ifindex=${NS_IFINDEX}} ACCEPT \ + rule ip4.proto icmp counter DROP" +test "$(get_counter egr_next_a 0)" = "0" +test "$(get_counter egr_next_b 0)" = "0" +(! ping -c 1 -W 0.1 ${NS_IP_ADDR}) +test "$(get_counter egr_next_a 0)" = "1" +test "$(get_counter egr_next_b 0)" = "1" +${FROM_NS} bfcli ruleset flush diff --git a/tests/unit/libbpfilter/chain.c b/tests/unit/libbpfilter/chain.c index f46e1b91e..a75790878 100644 --- a/tests/unit/libbpfilter/chain.c +++ b/tests/unit/libbpfilter/chain.c @@ -228,6 +228,22 @@ static void incompatible_matchers_disable_rule(void **state) } } +static void policy_validation(void **state) +{ + _free_bf_chain_ struct bf_chain *chain = NULL; + + (void)state; + + assert_ok(bf_chain_new(&chain, "next", BF_HOOK_TC_EGRESS, BF_VERDICT_NEXT, + NULL, NULL)); + bf_chain_free(&chain); + + assert_err(bf_chain_new(&chain, "bad", BF_HOOK_TC_EGRESS, + BF_VERDICT_CONTINUE, NULL, NULL)); + assert_err(bf_chain_new(&chain, "bad", BF_HOOK_TC_EGRESS, + BF_VERDICT_REDIRECT, NULL, NULL)); +} + static void get_set_by_name(void **state) { _free_bf_chain_ struct bf_chain *chain = bft_chain_dummy(true); @@ -247,6 +263,7 @@ int main(void) cmocka_unit_test(get_set_from_matcher), cmocka_unit_test(mixed_enabled_disabled_log_flag), cmocka_unit_test(incompatible_matchers_disable_rule), + cmocka_unit_test(policy_validation), cmocka_unit_test(get_set_by_name), }; diff --git a/tests/unit/libbpfilter/verdict.c b/tests/unit/libbpfilter/verdict.c index bf5fb19cc..e07539aff 100644 --- a/tests/unit/libbpfilter/verdict.c +++ b/tests/unit/libbpfilter/verdict.c @@ -16,6 +16,19 @@ static void to_from_str(void **state) _BF_VERDICT_MAX); } +static void is_valid_policy(void **state) +{ + (void)state; + + assert_true(bf_verdict_is_valid_policy(BF_VERDICT_ACCEPT)); + assert_true(bf_verdict_is_valid_policy(BF_VERDICT_DROP)); + assert_true(bf_verdict_is_valid_policy(BF_VERDICT_NEXT)); + assert_false(bf_verdict_is_valid_policy(BF_VERDICT_CONTINUE)); + assert_false(bf_verdict_is_valid_policy(BF_VERDICT_REDIRECT)); + assert_false(bf_verdict_is_valid_policy(_BF_VERDICT_MAX)); + assert_false(bf_verdict_is_valid_policy(-1)); +} + static void redirect_dir_to_from_str(void **state) { (void)state; @@ -33,6 +46,7 @@ int main(void) { const struct CMUnitTest tests[] = { cmocka_unit_test(to_from_str), + cmocka_unit_test(is_valid_policy), cmocka_unit_test(redirect_dir_to_from_str), };