From: Igor Putovny Date: Tue, 23 Sep 2025 14:58:14 +0000 (+0200) Subject: Filter: Allow merging of intervals in build_tree() X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=10e3a42641b2a469964b97f5ea1d28823cfbe47b;p=thirdparty%2Fbird.git Filter: Allow merging of intervals in build_tree() Fixes a bug that overlapping intervals in sets are not processed properly. --- diff --git a/filter/config.Y b/filter/config.Y index d4ffa2043..1fa17afdb 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -791,7 +791,7 @@ bgp_path_tail: | '[' ']' bgp_path_tail { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .set = NULL, .kind = PM_ASN_SET }, }); $$->next = $3; } | '[' set_items ']' bgp_path_tail { if ($2->from.type != T_INT) cf_error("Only integer sets allowed in path mask"); - $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .set = build_tree($2), .kind = PM_ASN_SET }, }); $$->next = $4; + $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .set = build_tree($2, true), .kind = PM_ASN_SET }, }); $$->next = $4; } | '*' bgp_path_tail { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .kind = PM_ASTERISK }, }); $$->next = $2; } | '?' bgp_path_tail { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .kind = PM_QUESTION }, }); $$->next = $2; } @@ -812,7 +812,7 @@ constant: | '[' ']' { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_SET, .val.t = NULL, }); } | '[' set_items ']' { DBG( "We've got a set here..." ); - $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_SET, .val.t = build_tree($2), }); + $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_SET, .val.t = build_tree($2, true), }); DBG( "ook\n" ); } | '[' fprefix_set ']' { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PREFIX_SET, .val.ti = $2, }); } diff --git a/filter/data.h b/filter/data.h index 82871daf2..a19864963 100644 --- a/filter/data.h +++ b/filter/data.h @@ -218,7 +218,7 @@ struct f_trie_walk_state }; struct f_tree *f_new_tree(void); -struct f_tree *build_tree(struct f_tree *); +struct f_tree *build_tree(struct f_tree *, bool merge); const struct f_tree *find_tree(const struct f_tree *t, const struct f_val *val); const struct f_tree *find_tree_linear(const struct f_tree *t, const struct f_val *val); int same_tree(const struct f_tree *t0, const struct f_tree *t2); diff --git a/filter/f-inst.c b/filter/f-inst.c index 234a36103..813604300 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -1258,7 +1258,10 @@ } /* Balance the tree */ - item->tree = build_tree(whati->tree); + item->tree = build_tree(whati->tree, false); + + if (whati->tree && !item->tree) + cf_error("Overlaping intervals in switch cases not allowed"); FID_ITERATE_BODY() tree_walk(whati->tree, f_add_tree_lines, fit); diff --git a/filter/test.conf b/filter/test.conf index 09d727c15..096798c51 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -215,7 +215,21 @@ function t_int_set() bt_assert([1,2] != [1,3]); bt_assert([1,4..10,20] = [1,4..10,20]); - bt_assert(format([ 1, 2, 1, 1, 1, 3, 4, 1, 1, 1, 5 ]) = "[1, 1, 1, 1, 1, 1, 1, 2, 3, 4, 5]"); + bt_assert(format([ 1, 2, 1, 1, 1, 3, 4, 1, 1, 1, 5 ]) = "[1, 2, 3, 4, 5]"); + bt_assert(format([1..3, 4..6, 7..9]) = "[1..3, 4..6, 7..9]"); + bt_assert(format([1..9, 2..4, 6..8]) = "[1..9]"); + bt_assert(format([1..9, 2..6, 5..8]) = "[1..9]"); + bt_assert(format([1..9, 2..8, 3..7]) = "[1..9]"); + bt_assert(format([1..4, 3..6, 5..9]) = "[1..9]"); + bt_assert(format([1..3, 3..6, 6..9]) = "[1..9]"); + bt_assert(format([1..6, 3..6, 6..9]) = "[1..9]"); + bt_assert(format([1..6, 4..7, 5..9]) = "[1..9]"); + bt_assert(format([1..9, 3..5, 5..7]) = "[1..9]"); + bt_assert(format([1..5, 6..7, 6..9]) = "[1..5, 6..9]"); + bt_assert(format([1..5, 6..8, 7..9]) = "[1..5, 6..9]"); + bt_assert(format([1..5, 6..9, 7..9]) = "[1..5, 6..9]"); + bt_assert(format([1..5, 6..9, 7..8]) = "[1..5, 6..9]"); + bt_assert(format([1..9, 1..9]) = "[1..9]"); bt_assert(format([]) = "[]"); case 3 { diff --git a/filter/tree.c b/filter/tree.c index 25cf93e49..008559111 100644 --- a/filter/tree.c +++ b/filter/tree.c @@ -87,9 +87,9 @@ tree_compare(const void *p1, const void *p2) * Transforms degenerated tree into balanced tree. */ struct f_tree * -build_tree(struct f_tree *from) +build_tree(struct f_tree *from, bool merge) { - struct f_tree *tmp, *root; + struct f_tree *tmp, *root = NULL; struct f_tree **buf; int len, i; @@ -112,8 +112,43 @@ build_tree(struct f_tree *from) qsort(buf, len, sizeof(struct f_tree *), tree_compare); - root = build_tree_rec(buf, 0, len); + int wpos = 0, rpos = 1; + + /* + * We maintain two indices: write position (wpos) for the current interval and + * read position (rpos) for the next interval. If the current interval overlaps + * with the next interval, we extend the current interval's end to encompass + * the next interval, and advance the read position. If not, we advance both + * positions and compact the output sequence, so the new current interval at + * wpos is the previous next interval. + */ + while (rpos < len) + { + if (val_compare(&buf[wpos]->to, &buf[rpos]->from) < 0) + { + wpos++; + + if (wpos != rpos) + buf[wpos] = buf[rpos]; + + rpos++; + } + else + { + /* When not merging intervals, overlaping intervals are not allowed */ + if (!merge) + goto done; + + if (val_compare(&buf[wpos]->to, &buf[rpos]->to) < 0) + buf[wpos]->to = buf[rpos]->to; + + rpos++; + } + } + + root = build_tree_rec(buf, 0, wpos + 1); +done: if (len > 1024) xfree(buf); diff --git a/filter/tree_test.c b/filter/tree_test.c index 05702f813..d1ae82c21 100644 --- a/filter/tree_test.c +++ b/filter/tree_test.c @@ -146,7 +146,7 @@ get_balanced_tree_with_ranged_values(uint nodes_count) n->to.val.i = idx++; } - return build_tree(tree); + return build_tree(tree, true); } @@ -166,7 +166,7 @@ t_balancing(void) struct f_tree *expected_balanced_tree = get_balanced_full_tree(height); show_tree(expected_balanced_tree); - struct f_tree *balanced_tree_from_simple = build_tree(simple_degenerated_tree); + struct f_tree *balanced_tree_from_simple = build_tree(simple_degenerated_tree, true); show_tree(balanced_tree_from_simple); bt_assert(same_tree(balanced_tree_from_simple, expected_balanced_tree)); @@ -194,7 +194,7 @@ t_balancing_random(void) struct f_tree *random_degenerated_tree = get_random_degenerated_left_tree(nodes_count); show_tree(random_degenerated_tree); - struct f_tree *balanced_tree_from_random = build_tree(random_degenerated_tree); + struct f_tree *balanced_tree_from_random = build_tree(random_degenerated_tree, true); show_tree(expected_balanced_tree); show_tree(balanced_tree_from_random); diff --git a/proto/l3vpn/config.Y b/proto/l3vpn/config.Y index 4048b9c90..c12a5bd60 100644 --- a/proto/l3vpn/config.Y +++ b/proto/l3vpn/config.Y @@ -96,7 +96,7 @@ l3vpn_proto: l3vpn_targets: ec_item { f_tree_only_rt($1); $$ = $1; } - | '[' ec_items ']' { f_tree_only_rt($2); $$ = build_tree($2); } + | '[' ec_items ']' { f_tree_only_rt($2); $$ = build_tree($2, true); } | '[' ']' { $$ = RT_NONE; } | NONE { $$ = RT_NONE; } ;