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 d4ffa204346e8b892ec176997612e4818ffc130e..1fa17afdbff7ce5e90529f25e81b89ea3e1a9905 100644 (file)
--- 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 82871daf21d767780b2c4462d6d28665b5709125..a19864963829e4c3525fefc85b51b859cd7fd0d4 100644 (file)
--- 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 234a3610344add71ab8c6a3316dff01a02ccf941..813604300d2fb9f288402b160749ca4383a2a1e5 100644 (file)
--- 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 09d727c1541173c6bca366fb1bc4eda4d4128f50..096798c517700c03ec98a8cf39dfc3e13b3ee3bc 100644 (file)
--- 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 25cf93e49a66f5ba90a08be5e0f1af24a83fa06d..008559111fbff5f40ad1d3f673ae8947142185ee 100644 (file)
--- 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 05702f813a636b708b5efa8879b81fa03ccb9dd4..d1ae82c214042ac38c8bee0ec2595e0a0f507604 100644 (file)
--- 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 4048b9c9008172523ed71eff4e98211922c9cad5..c12a5bd60a50b0339e0f6ede21ff7972bc8c4a51 100644 (file)
--- 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; }
  ;