From 17c21dce45b04f140f6cb944903ea220da554a11 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Fri, 13 Mar 2026 15:18:11 +0100 Subject: [PATCH 1/6] daemon: cgen: use output parameter for get_verdict The get_verdict flavor callback returns the BPF return code directly, using negative values for errors. This will conflict with TCX_NEXT (-1) when adding the NEXT verdict. Switch to an output parameter so that the return value is reserved for error reporting. No functional change. --- src/bpfilter/cgen/cgroup_skb.c | 8 ++++-- src/bpfilter/cgen/nf.c | 10 ++++--- src/bpfilter/cgen/program.c | 14 +++++----- src/bpfilter/cgen/stub.c | 32 ++++++++++++++--------- src/bpfilter/cgen/tc.c | 10 ++++--- src/bpfilter/cgen/xdp.c | 10 ++++--- src/libbpfilter/include/bpfilter/flavor.h | 8 +++--- 7 files changed, 60 insertions(+), 32 deletions(-) diff --git a/src/bpfilter/cgen/cgroup_skb.c b/src/bpfilter/cgen/cgroup_skb.c index 459238e0e..d857cadb6 100644 --- a/src/bpfilter/cgen/cgroup_skb.c +++ b/src/bpfilter/cgen/cgroup_skb.c @@ -143,12 +143,16 @@ static int _bf_cgroup_skb_gen_inline_matcher(struct bf_program *program, * @param verdict Verdict to convert. Must be valid. * @return Cgroup return code corresponding to the verdict, as an integer. */ -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; + *ret_code = 1; + return 0; case BF_VERDICT_DROP: + *ret_code = 0; return 0; default: return -ENOTSUP; diff --git a/src/bpfilter/cgen/nf.c b/src/bpfilter/cgen/nf.c index 08c596e81..f11042616 100644 --- a/src/bpfilter/cgen/nf.c +++ b/src/bpfilter/cgen/nf.c @@ -159,13 +159,17 @@ static int _bf_nf_gen_inline_matcher(struct bf_program *program, * @param verdict Verdict to convert. Must be valid. * @return Netfilter return code corresponding to the verdict, as an integer. */ -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; + *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..64ada8b58 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,10 @@ 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) + 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 +577,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 +625,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..4ce24e377 100644 --- a/src/bpfilter/cgen/tc.c +++ b/src/bpfilter/cgen/tc.c @@ -156,13 +156,17 @@ static int _bf_tc_gen_inline_redirect(struct bf_program *program, * @param verdict Verdict to convert. Must be valid. * @return TC return code corresponding to the verdict, as an integer. */ -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; default: return -ENOTSUP; } diff --git a/src/bpfilter/cgen/xdp.c b/src/bpfilter/cgen/xdp.c index a57ddeeca..d948c23ee 100644 --- a/src/bpfilter/cgen/xdp.c +++ b/src/bpfilter/cgen/xdp.c @@ -107,13 +107,17 @@ static int _bf_xdp_gen_inline_redirect(struct bf_program *program, return 0; } -static int _bf_xdp_get_verdict(enum bf_verdict verdict) +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; + *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/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. From e1207d1c17f0c3aef4a4908e77b5ed2c25a8e7a0 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Fri, 13 Mar 2026 15:19:57 +0100 Subject: [PATCH 2/6] lib: verdict: add BF_VERDICT_NEXT Add a new terminal verdict BF_VERDICT_NEXT that means "pass to next BPF program." For TC this will map to TCX_NEXT; for other flavors it maps to the same return code as ACCEPT. Replace the _BF_TERMINAL_VERDICT_MAX sentinel with an explicit bf_verdict_is_valid_policy() function, which is easier to reason about and extend. Update chain creation and the CLI parser to use it. Add NEXT to the lexer so it is recognised as a verdict token. --- src/bfcli/lexer.l | 2 +- src/bfcli/parser.y | 4 ++-- src/libbpfilter/chain.c | 6 ++--- src/libbpfilter/include/bpfilter/verdict.h | 26 ++++++++++++++++++++-- src/libbpfilter/verdict.c | 13 +++++++++++ 5 files changed, 43 insertions(+), 8 deletions(-) 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/libbpfilter/chain.c b/src/libbpfilter/chain.c index 1fe5db666..05143490c 100644 --- a/src/libbpfilter/chain.c +++ b/src/libbpfilter/chain.c @@ -185,8 +185,8 @@ 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 +240,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/verdict.h b/src/libbpfilter/include/bpfilter/verdict.h index c9d0abaea..b650d651d 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,7 +38,9 @@ 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 { @@ -50,8 +54,14 @@ enum bf_verdict BF_VERDICT_CONTINUE, /** Redirect the packet to another interface. */ BF_VERDICT_REDIRECT, + /** Pass the packet to the next BPF program. + * + * 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 +80,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..c55234c4e 100644 --- a/src/libbpfilter/verdict.c +++ b/src/libbpfilter/verdict.c @@ -44,6 +44,7 @@ static const char *_bf_verdict_strs[] = { [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 +69,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; + } +} From 841473020d4edd86e64ed95e97674c74f2975b46 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Fri, 13 Mar 2026 15:21:51 +0100 Subject: [PATCH 3/6] daemon: cgen: add NEXT verdict support to all flavors Map BF_VERDICT_NEXT to flavor-specific return codes: - TC: TCX_NEXT (-1), distinct from TCX_PASS - Netfilter: NF_ACCEPT (same as ACCEPT) - XDP: XDP_PASS (same as ACCEPT) - cgroup_skb: 1 (same as ACCEPT) Handle NEXT as a terminal verdict in rule codegen, alongside ACCEPT and DROP. --- src/bpfilter/cgen/cgroup_skb.c | 5 +++-- src/bpfilter/cgen/cgroup_sock_addr.c | 13 ++++++++++--- src/bpfilter/cgen/nf.c | 4 +++- src/bpfilter/cgen/program.c | 1 + src/bpfilter/cgen/tc.c | 7 +++++-- src/bpfilter/cgen/xdp.c | 8 ++++++++ src/libbpfilter/chain.c | 3 ++- src/libbpfilter/include/bpfilter/verdict.h | 12 +++++------- src/libbpfilter/verdict.c | 6 ++---- 9 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/bpfilter/cgen/cgroup_skb.c b/src/bpfilter/cgen/cgroup_skb.c index d857cadb6..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,7 +140,8 @@ 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, int *ret_code) { @@ -149,6 +149,7 @@ static int _bf_cgroup_skb_get_verdict(enum bf_verdict verdict, int *ret_code) switch (verdict) { case BF_VERDICT_ACCEPT: + case BF_VERDICT_NEXT: *ret_code = 1; return 0; case BF_VERDICT_DROP: 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/nf.c b/src/bpfilter/cgen/nf.c index f11042616..671ed87f0 100644 --- a/src/bpfilter/cgen/nf.c +++ b/src/bpfilter/cgen/nf.c @@ -157,7 +157,8 @@ 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, int *ret_code) { @@ -165,6 +166,7 @@ static int _bf_nf_get_verdict(enum bf_verdict verdict, int *ret_code) switch (verdict) { case BF_VERDICT_ACCEPT: + case BF_VERDICT_NEXT: *ret_code = NF_ACCEPT; return 0; case BF_VERDICT_DROP: diff --git a/src/bpfilter/cgen/program.c b/src/bpfilter/cgen/program.c index 64ada8b58..c9ab16e34 100644 --- a/src/bpfilter/cgen/program.c +++ b/src/bpfilter/cgen/program.c @@ -372,6 +372,7 @@ static int _bf_program_generate_rule(struct bf_program *program, switch (rule->verdict) { case BF_VERDICT_ACCEPT: case BF_VERDICT_DROP: + case BF_VERDICT_NEXT: r = program->runtime.ops->get_verdict(rule->verdict, &ret_code); if (r) return r; diff --git a/src/bpfilter/cgen/tc.c b/src/bpfilter/cgen/tc.c index 4ce24e377..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,7 +153,8 @@ 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, int *ret_code) { @@ -167,6 +167,9 @@ static int _bf_tc_get_verdict(enum bf_verdict verdict, int *ret_code) case BF_VERDICT_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 d948c23ee..53fd5a7c1 100644 --- a/src/bpfilter/cgen/xdp.c +++ b/src/bpfilter/cgen/xdp.c @@ -107,12 +107,20 @@ static int _bf_xdp_gen_inline_redirect(struct bf_program *program, return 0; } +/** + * 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: + case BF_VERDICT_NEXT: *ret_code = XDP_PASS; return 0; case BF_VERDICT_DROP: diff --git a/src/libbpfilter/chain.c b/src/libbpfilter/chain.c index 05143490c..ad9784813 100644 --- a/src/libbpfilter/chain.c +++ b/src/libbpfilter/chain.c @@ -186,7 +186,8 @@ int bf_chain_new(struct bf_chain **chain, const char *name, enum bf_hook hook, if (hook >= _BF_HOOK_MAX) return bf_err_r(-EINVAL, "unknown hook type"); if (!bf_verdict_is_valid_policy(policy)) - return bf_err_r(-EINVAL, "invalid policy '%s'", bf_verdict_to_str(policy)); + return bf_err_r(-EINVAL, "invalid policy '%s'", + bf_verdict_to_str(policy)); _chain = malloc(sizeof(*_chain)); if (!_chain) diff --git a/src/libbpfilter/include/bpfilter/verdict.h b/src/libbpfilter/include/bpfilter/verdict.h index b650d651d..0075ea068 100644 --- a/src/libbpfilter/include/bpfilter/verdict.h +++ b/src/libbpfilter/include/bpfilter/verdict.h @@ -44,17 +44,15 @@ int bf_redirect_dir_from_str(const char *str, enum bf_redirect_dir *dir); */ 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. + /** 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 diff --git a/src/libbpfilter/verdict.c b/src/libbpfilter/verdict.c index c55234c4e..83ea5ce4a 100644 --- a/src/libbpfilter/verdict.c +++ b/src/libbpfilter/verdict.c @@ -40,10 +40,8 @@ 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); From 4ce74d0508f2e797700bd5e7e5192aed1e65cc47 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Fri, 13 Mar 2026 15:23:25 +0100 Subject: [PATCH 4/6] tests: unit: add tests for NEXT verdict and policy validation --- tests/e2e/CMakeLists.txt | 1 + tests/e2e/rules/next_tc.sh | 92 ++++++++++++++++++++++++++++++++ tests/unit/libbpfilter/chain.c | 17 ++++++ tests/unit/libbpfilter/verdict.c | 14 +++++ 4 files changed, 124 insertions(+) create mode 100755 tests/e2e/rules/next_tc.sh 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), }; From 0ba579ebde9734c4a64f8900dc27aa30a35837a2 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Fri, 13 Mar 2026 15:31:47 +0100 Subject: [PATCH 5/6] doc: document NEXT verdict behavior per flavor Update bfcli usage documentation to describe the NEXT verdict for both chain policies and rule verdicts. Note that NEXT has distinct behavior only for TC hooks (TCX_NEXT); for NF, XDP, and cgroup_skb it maps to the same return code as ACCEPT. --- doc/usage/bfcli.rst | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) 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 ~~~~ From 982fc4ef8b45761a8c30e9642fea363f7d783622 Mon Sep 17 00:00:00 2001 From: Quentin Deslandes Date: Tue, 17 Mar 2026 08:31:57 +0100 Subject: [PATCH 6/6] daemon: cgen: fix jmp.h documentation --- src/bpfilter/cgen/jmp.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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