]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
Filter: make bgpmask literals real constructors
authorJan Maria Matejka <mq@ucw.cz>
Wed, 28 Feb 2018 15:57:50 +0000 (16:57 +0100)
committerJan Maria Matejka <mq@ucw.cz>
Wed, 14 Mar 2018 10:34:29 +0000 (11:34 +0100)
The bgpmask literals can include expressions. This is OK but they have
to be interpreted as soon as the code is run, not in the time the code
is used as value.

This led to strange behavior like rewriting bgpmasks when they shan't
be rewritten:

function mask_generator(int as)
{
return [= * as * =];
}

function another()
bgpmask m1;
bgpmask m2;
{
m1 = mask_generator(10);
m2 = mask_generator(20);
if (m1 == m2) {
print("strange"); # this would happen
}
}

Moreover, sending this to CLI would cause stack overflow and knock down the
whole BIRD, as soon as there is at least one route to execute the given
filter on.

show route filter bgpmask mmm; bgppath ppp; { ppp = +empty+; mmm = [= (ppp ~ mmm) =]; print(mmm); accept; }

The magic match operator (~) inside the bgpmask literal would try to
resolve mmm, which points to the same bgpmask so it would resolve
itself, call the magic match operator and vice versa.

After this patch, the bgpmask literal will get resolved as soon as it's
assigned to mmm and it also will return a type error as bool is not
convertible to ASN in BIRD.

filter/config.Y
filter/filter.c
filter/filter.h
nest/a-path.c

index 1ef5a3a8b039ff2342716cb7b62307812ef5b54f..7c077b6570d7ea59a3de985d09aa39fc0e812978 100644 (file)
@@ -317,6 +317,26 @@ f_generate_lc(struct f_inst *t1, struct f_inst *t2, struct f_inst *t3)
   return rv;
 }
 
+static inline struct f_inst *
+f_generate_path_mask(struct f_path_mask *t)
+{
+  for (struct f_path_mask *tt = t; tt; tt = tt->next) {
+    if (tt->kind == PM_ASN_EXPR) {
+      struct f_inst *mrv = f_new_inst(FI_PATHMASK_CONSTRUCT);
+      mrv->a1.p = t;
+      return mrv;
+    }
+  }
+
+  NEW_F_VAL;
+  val->type = T_PATH_MASK;
+  val->val.path_mask = t;
+
+  struct f_inst *rv = f_new_inst(FI_CONSTANT_INDIRECT);
+  rv->a1.p = val;
+
+  return rv;
+}
 
 
 CF_DECLS
@@ -708,13 +728,13 @@ constant:
  | '[' set_items ']' { DBG( "We've got a set here..." ); $$ = f_new_inst(FI_CONSTANT); $$->aux = T_SET; $$->a2.p = build_tree($2); DBG( "ook\n" ); }
  | '[' fprefix_set ']' { $$ = f_new_inst(FI_CONSTANT); $$->aux = T_PREFIX_SET;  $$->a2.p = $2; }
  | ENUM          { $$ = f_new_inst(FI_CONSTANT); $$->aux = $1 >> 16; $$->a2.i = $1 & 0xffff; }
- | bgp_path { NEW_F_VAL; $$ = f_new_inst(FI_CONSTANT_INDIRECT); val->type = T_PATH_MASK; val->val.path_mask = $1; $$->a1.p = val; }
  ;
 
 constructor:
    '(' term ',' term ')' { $$ = f_generate_dpair($2, $4); }
  | '(' ec_kind ',' term ',' term ')' { $$ = f_generate_ec($2, $4, $6); }
  | '(' term ',' term ',' term ')' { $$ = f_generate_lc($2, $4, $6); }
+ | bgp_path { $$ = f_generate_path_mask($1); }
  ;
 
 
index 023f7e2f15efdd4627de3b6b4e72b45f90defd2f..8b66b57e23c42a2f929ed44cd16f5d580deca622 100644 (file)
@@ -82,8 +82,7 @@ pm_format(struct f_path_mask *p, buffer *buf)
       break;
 
     case PM_ASN_EXPR:
-      buffer_print(buf, "%u ", f_eval_asn((struct f_inst *) p->val));
-      break;
+      ASSERT(0);
     }
 
     p = p->next;
@@ -772,6 +771,32 @@ interpret(struct f_inst *what)
       break;
     }
 
+  case FI_PATHMASK_CONSTRUCT:
+    {
+      struct f_path_mask *tt = what->a1.p, *vbegin, **vv = &vbegin;
+
+      while (tt) {
+       *vv = lp_alloc(f_pool, sizeof(struct f_path_mask));
+       if (tt->kind == PM_ASN_EXPR) {
+         struct f_val res = interpret((struct f_inst *) tt->val);
+         (*vv)->kind = PM_ASN;
+         if (res.type != T_INT) {
+           runtime( "Error resolving path mask template: value not an integer" );
+           return (struct f_val) { .type = T_VOID };
+         }
+
+         (*vv)->val = res.val.i;
+       } else {
+         **vv = *tt;
+       }
+       tt = tt->next;
+       vv = &((*vv)->next);
+      }
+
+      res = (struct f_val) { .type = T_PATH_MASK, .val.path_mask = vbegin };
+      break;
+    }
+
 /* Relational operators */
 
 #define COMPARE(x) \
@@ -1550,6 +1575,8 @@ i_same(struct f_inst *f1, struct f_inst *f2)
   case FI_LT:
   case FI_LTE: TWOARGS; break;
 
+  case FI_PATHMASK_CONSTRUCT: if (!pm_same(f1->a1.p, f2->a1.p)) return 0; break;
+
   case FI_NOT: ONEARG; break;
   case FI_NOT_MATCH:
   case FI_MATCH: TWOARGS; break;
@@ -1771,14 +1798,6 @@ f_eval_int(struct f_inst *expr)
   return res.val.i;
 }
 
-u32
-f_eval_asn(struct f_inst *expr)
-{
-  /* Called as a part of another interpret call, therefore no log_reset() */
-  struct f_val res = interpret(expr);
-  return (res.type == T_INT) ? res.val.i : 0;
-}
-
 /**
  * filter_same - compare two filters
  * @new: first filter to be compared
index 1d0f389e09f18b8fb4a22c6ebcb456a8b19c436d..572459d86b73b8e1b3fa569c593d4fdf071be0b1 100644 (file)
@@ -27,6 +27,7 @@
   F(FI_PAIR_CONSTRUCT,         'm', 'p') \
   F(FI_EC_CONSTRUCT,           'm', 'c') \
   F(FI_LC_CONSTRUCT,           'm', 'l') \
+  F(FI_PATHMASK_CONSTRUCT,     'm', 'P') \
   F(FI_NEQ,                    '!', '=') \
   F(FI_EQ,                     '=', '=') \
   F(FI_LT,                       0, '<') \
@@ -196,7 +197,6 @@ int f_run(struct filter *filter, struct rte **rte, struct ea_list **tmp_attrs, s
 struct f_val f_eval_rte(struct f_inst *expr, struct rte **rte, struct linpool *tmp_pool);
 struct f_val f_eval(struct f_inst *expr, struct linpool *tmp_pool);
 uint f_eval_int(struct f_inst *expr);
-u32 f_eval_asn(struct f_inst *expr);
 
 char *filter_name(struct filter *filter);
 int filter_same(struct filter *new, struct filter *old);
index b453f702099e8dce770c6e1b890a74b077152d77..0272c6d718b32834f62501c018b37cd4b5f1f5e8 100644 (file)
@@ -535,8 +535,7 @@ as_path_match(struct adata *path, struct f_path_mask *mask)
          val2 = val = mask->val;
          goto step;
        case PM_ASN_EXPR:
-         val2 = val = f_eval_asn((struct f_inst *) mask->val);
-         goto step;
+         ASSERT(0);
        case PM_ASN_RANGE:
          val = mask->val;
          val2 = mask->val2;