[PATCH 0/3] filter: improve runtime error diagnostics
Hi, Filter runtime errors (`F_ERROR`) currently share filtered counters and the `filtered out` route trace with intentional policy rejects (`F_REJECT`). The detailed interpreter error is rate-limited, which can make route-dependent filter failures difficult to classify. One example is constructing a standard-community pair from a 32-bit Large Community field. Configuration parsing succeeds because the concrete value is route-dependent, but a value above 65535 correctly fails the pair constructor at runtime. This series: 1. reports both evaluated values in pair range errors; 2. keeps the existing filtered aggregate, adds a runtime-error subset for import and export, and uses a distinct route trace; 3. documents runtime pair checks and a safe LC-field guard. The series does not change log rate limits or add another error-log source. Silent export evaluations do not alter the new counters, and import keep-filtered behavior is unchanged. Base: thread-next 06ca9bf5c588df6c0587eb64bd375eb0ade526ab Validation: - autoreconf, configure, full build, and make check; - BIRD user and programmer HTML documentation; - daemon-level import/export errors and explicit-reject controls; - keep-filtered and silent export-query behavior. An equivalent series for current master is available if this should also land in BIRD 2. Alice39s (3): filter: report values for out-of-range pair components nest: track filter runtime errors in channel statistics doc: explain pair construction range failures Thanks, Alice39s alice39s@xmsl.dev
Filter runtime errors and intentional rejects currently share filtered counters and trace text. Keep errors as a subset of filtered updates for compatibility, but count them separately and identify them in route traces and protocol statistics. The dedicated counters remain useful when the underlying error log is rate-limited. This avoids classifying filter failures as invalid protocol updates or adding another log source. --- nest/proto.c | 4 ++++ nest/protocol.h | 2 ++ nest/rt-table.c | 21 ++++++++++++++------- proto/pipe/pipe.c | 4 ++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/nest/proto.c b/nest/proto.c index 341fe9bc..b4a068e0 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -2999,6 +2999,10 @@ channel_show_stats(struct channel *c) cli_msg(-1006, " Export withdraws: %10u --- --- %10u --- --- %10u", SRE(withdraws_received), SCE(withdraws_ignored), SCE(withdraws_accepted)); + if (SCI(updates_filter_errors) || SCE(updates_filter_errors)) + cli_msg(-1006, " Filter runtime errors: %u import, %u export", + SCI(updates_filter_errors), SCE(updates_filter_errors)); + #undef SRI #undef SRE #undef SCI diff --git a/nest/protocol.h b/nest/protocol.h index 0d5b14fb..32d73bdd 100644 --- a/nest/protocol.h +++ b/nest/protocol.h @@ -649,6 +649,7 @@ struct channel { u32 updates_received; /* Number of route updates received */ u32 updates_invalid; /* Number of route updates rejected as invalid */ u32 updates_filtered; /* Number of route updates rejected by filters */ + u32 updates_filter_errors; /* Filter runtime errors (subset of filtered) */ u32 updates_limited_rx; /* Number of route updates exceeding the rx_limit */ u32 updates_limited_in; /* Number of route updates exceeding the in_limit */ u32 withdraws_received; /* Number of route withdraws received */ @@ -660,6 +661,7 @@ struct channel { u32 updates_ignored; /* Number of route updates ignored (squashed) by channel */ u32 updates_rejected; /* Number of route updates rejected by protocol */ u32 updates_filtered; /* Number of route updates rejected by filters */ + u32 updates_filter_errors; /* Filter runtime errors (subset of filtered) */ u32 updates_accepted; /* Number of route updates accepted and exported */ u32 updates_limited; /* Number of route updates exceeding the out_limit */ u32 withdraws_ignored; /* Number of route withdraws ignored (squashed) by channel */ diff --git a/nest/rt-table.c b/nest/rt-table.c index dee61676..d6cedc09 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -1300,16 +1300,19 @@ export_filter(struct channel *c, rte *rt, int silent) } /* Evaluate actual filters */ - v = filter && ((filter == FILTER_REJECT) || - (f_run(filter, rt, - (silent ? FF_SILENT : 0)) > F_ACCEPT)); - if (v) + v = filter ? ((filter == FILTER_REJECT) ? F_REJECT : + f_run(filter, rt, (silent ? FF_SILENT : 0))) : F_ACCEPT; + if (v > F_ACCEPT) { if (silent) return false; stats->updates_filtered++; - channel_rte_trace_out(D_FILTERS, c, rt, "filtered out"); + if (v == F_ERROR) + stats->updates_filter_errors++; + + channel_rte_trace_out(D_FILTERS, c, rt, + (v == F_ERROR) ? "filter runtime error" : "filtered out"); return false; } @@ -2706,14 +2709,18 @@ rte_update(struct channel *c, const net_addr *n, rte *new, struct rte_src *src) new->net = n; new->sender = c->in_req.hook; - int fr; + int fr = F_REJECT; stats->updates_received++; if ((filter == FILTER_REJECT) || ((fr = f_run(filter, new, 0)) > F_ACCEPT)) { stats->updates_filtered++; - channel_rte_trace_in(D_FILTERS, c, new, "filtered out"); + if (fr == F_ERROR) + stats->updates_filter_errors++; + + channel_rte_trace_in(D_FILTERS, c, new, + (fr == F_ERROR) ? "filter runtime error" : "filtered out"); if (c->in_keep & RIK_REJECTED) new->flags |= REF_FILTERED; diff --git a/proto/pipe/pipe.c b/proto/pipe/pipe.c index 6cbb5799..3a008cb3 100644 --- a/proto/pipe/pipe.c +++ b/proto/pipe/pipe.c @@ -261,6 +261,10 @@ pipe_show_stats(struct pipe_proto *p) cli_msg(-1006, " Export withdraws: %10u %10u --- %10u %10u", rs1e->withdraws_received, s2i->withdraws_invalid, rs2i->withdraws_ignored, rs2i->withdraws_accepted); + + if (s2e->updates_filter_errors || s1e->updates_filter_errors) + cli_msg(-1006, " Filter runtime errors: %u import, %u export", + s2e->updates_filter_errors, s1e->updates_filter_errors); } static void -- 2.47.3
Howdy! The filter error stats make some sense but it's kinda considered a configuration error which a user should generally avoid, so the number is expected to stay at zero in regular deployment. What is worse, it's gonna change the CLI output format expected by users' tooling. That problem will be fixed by implementing the API, we'll revisit this topic then. The error message update is a reasonable thing. Please check other community flavors so that everything gets fixed at once. It would be even better if you could update the filter/test.conf and maybe filter/filter_test.c so that there is some basic unit test for these filter runtime failure messages. Note: it would be helpful if you prepared your commit messages in the same style and format as the other recent commits use. Thanks! Maria On August 11, 2026 8:55:29 PM GMT+02:00, Alice39s <alice39s@xmsl.dev> wrote:
Hi,
Filter runtime errors (`F_ERROR`) currently share filtered counters and the `filtered out` route trace with intentional policy rejects (`F_REJECT`). The detailed interpreter error is rate-limited, which can make route-dependent filter failures difficult to classify.
One example is constructing a standard-community pair from a 32-bit Large Community field. Configuration parsing succeeds because the concrete value is route-dependent, but a value above 65535 correctly fails the pair constructor at runtime.
This series:
1. reports both evaluated values in pair range errors; 2. keeps the existing filtered aggregate, adds a runtime-error subset for import and export, and uses a distinct route trace; 3. documents runtime pair checks and a safe LC-field guard.
The series does not change log rate limits or add another error-log source. Silent export evaluations do not alter the new counters, and import keep-filtered behavior is unchanged.
Base: thread-next 06ca9bf5c588df6c0587eb64bd375eb0ade526ab
Validation:
- autoreconf, configure, full build, and make check; - BIRD user and programmer HTML documentation; - daemon-level import/export errors and explicit-reject controls; - keep-filtered and silent export-query behavior.
An equivalent series for current master is available if this should also land in BIRD 2.
Alice39s (3): filter: report values for out-of-range pair components nest: track filter runtime errors in channel statistics doc: explain pair construction range failures
Thanks, Alice39s alice39s@xmsl.dev
-- Maria Matejka (she/her) | BIRD Team Leader | CZ.NIC, z.s.p.o.
Hi Maria, Thanks for the review. This version keeps the diagnostic change, covers the other community constructors, and adds focused unit tests. Community constructors can reject values that are only known during filter evaluation. Include the values that determine these range failures so the failing route-dependent input can be identified from the existing runtime log. Changes since v1: - drop the channel statistics, CLI output, and route-trace changes; - report both pair components and the relevant extended-community key and value; - audit Large Community construction and test its full 32-bit field range; - add exact-message tests for the pair and extended-community failures, including both IP and quad inputs to the IPv4-key form; - align the commit subjects and bodies with recent upstream style. The series does not change log rate limits or add another error-log source. Base: thread-next 06ca9bf5c588df6c0587eb64bd375eb0ade526ab Validation: - autoreconf, configure, full build, and make check; - BIRD user and programmer HTML documentation; - clean-base application of the packaged v2 series. A BIRD 2 follow-up can be prepared if this should also land on master. Alice39s (3): Filter: Include values in community constructor errors Filter: Test community constructor runtime errors Doc: Explain pair constructor range checks Thanks, Alice39s alice39s@xmsl.dev
Extend configured filter test suites with optional expected-error matching. Require both F_ERROR and the expected runtime log for pair and extended community constructor failures. Reset the runtime-error limiter for each expected failure so earlier tests cannot hide the message. Also cover the full 32-bit range of large community fields. Target: patch --- filter/config.Y | 24 ++++++++++---- filter/f-inst.h | 5 +-- filter/filter.c | 6 ++++ filter/filter_test.c | 42 ++++++++++++++++++++++- filter/test.conf | 79 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 10 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index c0a341c2..df894a2c 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -359,6 +359,19 @@ assert_assign(struct f_lval *lval, struct f_inst *expr, const char *start, const return assert_done(setter, start, end); } +static void +add_bt_test_suite(struct symbol *sym, const char *dsc, const char *expected_error) +{ + cf_assert_symbol(sym, SYM_FUNCTION); + struct f_bt_test_suite *t = cfg_allocz(sizeof(struct f_bt_test_suite)); + t->fn = sym->function; + t->fn_name = sym->name; + t->dsc = dsc; + t->expected_error = expected_error; + + add_tail(&new_config->tests, &t->n); +} + CF_DECLS CF_KEYWORDS_EXCLUSIVE(IN, FROM) @@ -453,13 +466,10 @@ custom_attr: ATTRIBUTE type symbol ';' { conf: bt_test_suite ; bt_test_suite: BT_TEST_SUITE '(' CF_SYM_KNOWN ',' text ')' { - cf_assert_symbol($3, SYM_FUNCTION); - struct f_bt_test_suite *t = cfg_allocz(sizeof(struct f_bt_test_suite)); - t->fn = $3->function; - t->fn_name = $3->name; - t->dsc = $5; - - add_tail(&new_config->tests, &t->n); + add_bt_test_suite($3, $5, NULL); + } + | BT_TEST_SUITE '(' CF_SYM_KNOWN ',' text ',' text ')' { + add_bt_test_suite($3, $5, $7); } ; diff --git a/filter/f-inst.h b/filter/f-inst.h index 49c4eb14..5c4bd3ef 100644 --- a/filter/f-inst.h +++ b/filter/f-inst.h @@ -117,16 +117,17 @@ static inline struct f_static_attr f_new_static_attr(btype type, int code, int r struct f_inst *f_generate_roa_check(struct rtable_config *table, struct f_inst *prefix, struct f_inst *asn); -/* Hook for call bt_assert() function in configuration */ +/* Bird Tests */ extern void (*bt_assert_hook)(int result, const struct f_line_item *assert); +void f_bt_reset_runtime_error_limiter(void); -/* Bird Tests */ struct f_bt_test_suite { node n; /* Node in config->tests */ const struct f_line *fn; /* Root of function */ const struct f_line *cmp; /* Compare to this function */ const char *fn_name; /* Name of test */ const char *dsc; /* Description */ + const char *expected_error; /* Expected runtime error message */ int result; /* Desired result */ }; diff --git a/filter/filter.c b/filter/filter.c index ed056c4e..fab077fe 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -105,6 +105,12 @@ void (*bt_assert_hook)(int result, const struct f_line_item *assert); static struct tbf rl_runtime_err = TBF_DEFAULT_LOG_LIMITS; +void +f_bt_reset_runtime_error_limiter(void) +{ + rl_runtime_err = (struct tbf) TBF_DEFAULT_LOG_LIMITS; +} + /** * interpret * @fs: filter state diff --git a/filter/filter_test.c b/filter/filter_test.c index 83501ba3..ce75088e 100644 --- a/filter/filter_test.c +++ b/filter/filter_test.c @@ -10,8 +10,10 @@ #define _GNU_SOURCE #endif -#include <string.h> +#include <stdio.h> #include <stdlib.h> +#include <string.h> +#include <unistd.h> #include "test/birdtest.h" #include "test/bt-utils.h" @@ -38,6 +40,41 @@ t_reconfig(const void *arg) return 1; } +static int +run_function_error(const struct f_bt_test_suite *t) +{ + FILE *capture = tmpfile(); + bt_syscall(!capture, "tmpfile"); + + int capture_fd = fileno(capture); + bt_syscall(capture_fd < 0, "fileno"); + int stderr_fd = dup(STDERR_FILENO); + bt_syscall(stderr_fd < 0, "dup"); + + bt_syscall(fflush(stderr) != 0, "fflush"); + bt_syscall(dup2(capture_fd, STDERR_FILENO) < 0, "dup2"); + f_bt_reset_runtime_error_limiter(); + enum filter_return fret = f_eval(t->fn, NULL); + bt_syscall(fflush(stderr) != 0, "fflush"); + bt_syscall(dup2(stderr_fd, STDERR_FILENO) < 0, "dup2"); + bt_syscall(close(stderr_fd) < 0, "close"); + + bt_syscall(fseek(capture, 0, SEEK_SET) != 0, "fseek"); + + char buf[1024]; + size_t len = fread(buf, 1, sizeof(buf) - 1, capture); + bt_syscall(ferror(capture), "fread"); + buf[len] = 0; + bt_syscall(fclose(capture) != 0, "fclose"); + + int result = (fret == F_ERROR) && strstr(buf, t->expected_error); + if (!result) + bt_log("Expected F_ERROR containing '%s', got %s and '%s'", + t->expected_error, filter_return_str(fret), buf); + + return result; +} + static int run_function(const void *arg) { @@ -46,6 +83,9 @@ run_function(const void *arg) if (t->cmp) return t->result == f_same(t->fn, t->cmp); + if (t->expected_error) + return run_function_error(t); + enum filter_return fret = f_eval(t->fn, NULL); return (fret < F_REJECT); diff --git a/filter/test.conf b/filter/test.conf index 3171b9e9..0b51c093 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -471,6 +471,36 @@ function t_pair() bt_test_suite(t_pair, "Testing pairs"); +function t_pair_runtime_error() +{ + int asn = 64512; + int data = 65536; + pair pp = (asn, data); +} + +bt_test_suite(t_pair_runtime_error, "Testing pair constructor runtime error", + "Pair component out of range 0..65535 (got 64512, 65536)"); + +function t_pair_boundary() +{ + int asn = 64512; + int data = 65535; + pair pp = (asn, data); + bt_assert(pp = (64512, 65535)); +} + +bt_test_suite(t_pair_boundary, "Testing pair constructor upper boundary"); + +function t_pair_runtime_error_first() +{ + int asn = 65536; + int data = 1; + pair pp = (asn, data); +} + +bt_test_suite(t_pair_runtime_error_first, "Testing pair constructor first-component runtime error", + "Pair component out of range 0..65535 (got 65536, 1)"); + @@ -1578,6 +1608,49 @@ function t_ec() bt_test_suite(t_ec, "Testing extended communities"); +function t_ec_ip_runtime_error() +{ + ip test_key = 192.0.2.1; + int test_value = 65536; + ec cc = (rt, test_key, test_value); +} + +bt_test_suite(t_ec_ip_runtime_error, + "Testing extended community IPv4-key runtime error", + "Extended community value out of range 0..65535 (IPv4 key 192.0.2.1, value 65536)"); + +function t_ec_quad_runtime_error() +{ + quad test_key = 192.0.2.1; + int test_value = 65536; + ec cc = (rt, test_key, test_value); +} + +bt_test_suite(t_ec_quad_runtime_error, + "Testing extended community quad-key runtime error", + "Extended community value out of range 0..65535 (IPv4 key 192.0.2.1, value 65536)"); + +function t_ec_asn_runtime_error() +{ + int test_key = 65536; + int test_value = 65536; + ec cc = (rt, test_key, test_value); +} + +bt_test_suite(t_ec_asn_runtime_error, + "Testing extended community 4-byte-ASN runtime error", + "Extended community value out of range 0..65535 (4-byte ASN 65536, value 65536)"); + +function t_ec_asn_boundary() +{ + int test_key = 65536; + int test_value = 65535; + ec cc = (rt, test_key, test_value); + bt_assert(cc = (rt, 65536, 65535)); +} + +bt_test_suite(t_ec_asn_boundary, "Testing extended community 4-byte-ASN upper boundary"); + @@ -1836,12 +1909,18 @@ lclist ll; lclist ll2; lclist r; { + int max_lc_value = 4294967295; + lc max_lc = (max_lc_value, max_lc_value, max_lc_value); + bt_assert(---empty--- = ---empty---); bt_assert((10, 20, 30) !~ ---empty---); bt_assert((10, 20, 30).asn = 10); bt_assert((10, 20, 30).data1 = 20); bt_assert((10, 20, 30).data2 = 30); + bt_assert(max_lc.asn = max_lc_value); + bt_assert(max_lc.data1 = max_lc_value); + bt_assert(max_lc.data2 = max_lc_value); ll = --- empty ---; ll = add(ll, (ten, 20, 30)); -- 2.47.3
Pair and extended community constructors may reject values only known during filter evaluation. Include the evaluated components that determine these errors so operators can identify the failing data without reproducing a route. Large community fields use the full 32-bit integer range and need no additional range check. Target: patch --- filter/f-inst.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/filter/f-inst.c b/filter/f-inst.c index 967887f6..3a339f47 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -338,7 +338,8 @@ uint u1 = v1.val.i; uint u2 = v2.val.i; if ((u1 > 0xFFFF) || (u2 > 0xFFFF)) - runtime( "Can't operate with value out of bounds in pair constructor" ); + runtime("Pair component out of range 0..65535 (got %u, %u)", + u1, u2); RESULT(T_PAIR, i, (u1 << 16) | u2); } @@ -372,14 +373,16 @@ if (val <= 0xFFFF) RESULT(T_EC, ec, ec_ip4(ecs, key, val)); else - runtime("4-byte value %u can't be used with IP-address key in extended community", val); + runtime("Extended community value out of range 0..65535 " + "(IPv4 key %I, value %u)", ipa_from_u32(key), val); else if (key < 0x10000) RESULT(T_EC, ec, ec_as2(ecs, key, val)); else if (val <= 0xFFFF) RESULT(T_EC, ec, ec_as4(ecs, key, val)); else - runtime("4-byte value %u can't be used with 4-byte ASN in extended community", val); + runtime("Extended community value out of range 0..65535 " + "(4-byte ASN %u, value %u)", key, val); } INST(FI_LC_CONSTRUCT, 3, 1) { base-commit: 06ca9bf5c588df6c0587eb64bd375eb0ade526ab -- 2.47.3
Pair components built from expressions are checked only when the filter runs. Document this failure mode and show how to guard a 32-bit large community field before using it as a 16-bit pair component. Target: patch --- doc/bird.sgml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/doc/bird.sgml b/doc/bird.sgml index b97793f4..335a2d76 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -1848,6 +1848,10 @@ in the foot). Operators <cf/.asn/ and <cf/.data/ can be used to extract corresponding components of a pair: <cf>(<m/asn/, <m/data/)</cf>. + Pair construction from expressions is range-checked at filter runtime. + If either expression evaluates to a value greater than 65535, filter + evaluation fails with a runtime error. + <tag><label id="type-quad">quad</tag> This is a dotted quad of numbers used to represent router IDs (and others). Each component can have a value from 0 to 255. Literals of @@ -1990,6 +1994,18 @@ in the foot). to extract corresponding components of LCs: <cf>(<m/asn/, <m/data1/, <m/data2/)</cf>. + Large community fields are 32-bit, while pair components are 16-bit. + Therefore, check the range before using an LC field to construct a pair. + For example, given an LC value in <cf/op/: + <code> + if op.data2 <= 65535 then + bgp_community.add((64512, op.data2)); + else + reject; + </code> + The explicit <cf/reject/ is optional policy; a filter may instead ignore + an LC that cannot be represented as a pair. + <tag><label id="type-set">int|pair|quad|ip|prefix|ec|lc|rd|enum set</tag> Filters recognize several types of sets. Sets are similar to strings: you can pass them around but you cannot modify them. Literals of type <cf>int -- 2.47.3
participants (2)
-
Alice39s -
Maria Matejka