Aggregator: polishing of filter API

conf/conf.h

@@ -238,6 +238,9 @@ struct symbol *cf_new_symbol(struct sym_scope *scope, pool *p, struct linpool *l
     sym_->var_ = def_; \
     sym_; })
 
+#define cf_create_symbol(conf_, name_, type_, var_, def_) \
+  cf_define_symbol(conf_, cf_get_symbol(conf_, name_), type_, var_, def_)
+
 void cf_push_scope(struct config *, struct symbol *);
 void cf_pop_scope(struct config *);
 void cf_push_soft_scope(struct config *);

filter/config.Y

@@ -339,7 +339,6 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN,
 	ACCEPT, REJECT, ERROR,
 	INT, BOOL, IP, PREFIX, RD, PAIR, QUAD, EC, LC,
 	SET, STRING, BYTESTRING, BGPMASK, BGPPATH, CLIST, ECLIST, LCLIST,
-    ROUTES,
 	IF, THEN, ELSE, CASE,
 	FOR, DO,
 	TRUE, FALSE, RT, RO, UNKNOWN, GENERIC,
@@ -917,7 +916,6 @@ term:
  | term_bs
  | function_call
 
- | ROUTES { $$ = f_new_inst(FI_ROUTES_GET); }
  | CF_SYM_KNOWN '.' static_attr {
      cf_assert_symbol($1, SYM_VARIABLE);
      $$ = f_new_inst(FI_RTA_GET, f_new_inst(FI_VAR_GET, $1), $3);

filter/f-inst.c

@@ -553,12 +553,6 @@
     RESULT_VAL(val);
   }
 
-  INST(FI_ROUTES_GET, 0, 1) {
-    NEVER_CONSTANT;
-    FID_INTERPRET_BODY()
-    RESULT(T_ROUTES_BLOCK, ad, fs->val->val.ad);
-  }
-
   METHOD_R(T_PATH, empty, T_PATH, ad, &null_adata);
   METHOD_R(T_CLIST, empty, T_CLIST, ad, &null_adata);
   METHOD_R(T_ECLIST, empty, T_ECLIST, ad, &null_adata);
@@ -625,7 +619,7 @@
 
   INST(FI_ROUTES_BLOCK_FOR_NEXT, 3, 0) {
     NEVER_CONSTANT;
-    ARG(1, T_ROUTE);
+    ARG(1, T_ROUTES_BLOCK);
     if (rte_set_walk(v1.val.ad, &v2.val.i, &v3.val.rte))
       LINE(2,0);
 

filter/filter.c

@@ -160,18 +160,20 @@ static struct tbf rl_runtime_err = TBF_DEFAULT_LOG_LIMITS;
  * TWOARGS macro to get both of them evaluated.
  */
 static enum filter_return
-interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val)
+interpret(struct filter_state *fs, const struct f_line *line, uint argc, const struct f_val *argv, struct f_val *val)
 {
   /* No arguments allowed */
-  ASSERT(line->args == 0);
+  ASSERT_DIE(line->args == argc);
 
   /* Initialize the filter stack */
   struct filter_stack *fstk = fs->stack;
 
-  fstk->vcnt = line->vars;
+  /* Set the arguments and top-level variables */
-  memset(fstk->vstk, 0, sizeof(struct f_val) * line->vars);
+  fstk->vcnt = line->vars + line->args;
+  memcpy(fstk->vstk, argv, sizeof(struct f_val) * line->args);
+  memset(fstk->vstk + line->args, 0, sizeof(struct f_val) * line->vars);
 
-  /* The same as with the value stack. Not resetting the stack for performance reasons. */
+  /* The same as with the value stack. Not resetting the stack completely for performance reasons. */
   fstk->ecnt = 1;
   fstk->estk[0].line = line;
   fstk->estk[0].pos = 0;
@@ -240,30 +242,6 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val)
   return F_ERROR;
 }
 
-/**
- * Evaluation for attributes comparison
- */
-enum filter_return
-f_aggr_eval_line(const struct f_line *line, struct rte **rte, struct linpool *pool, struct f_val *pres)
-{
-  /* Initialize the filter state */
-  filter_state = (struct filter_state) {
-    .stack = &filter_stack,
-    .rte = rte,
-    .pool = pool,
-  };
-
-  LOG_BUFFER_INIT(filter_state.buf);
-
-  /* Run the interpreter itself */
-  enum filter_return fret = interpret(&filter_state, line, pres);
-
-  if (filter_state.old_rta)
-    log("Warning: route changed during filter evaluation");
-
-  return fret;
-}
-
 /**
  * f_run - run a filter for a route
  * @filter: filter to run
@@ -297,11 +275,11 @@ f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, i
   if (filter == FILTER_REJECT)
     return F_REJECT;
 
-  return f_run_val(filter, rte, tmp_pool, NULL, flags);
+  return f_run_args(filter, rte, tmp_pool, 0, NULL, flags);
 }
 
 enum filter_return
-f_run_val(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, const struct f_val *val, int flags)
+f_run_args(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, uint argc, const struct f_val *argv, int flags)
 {
   int rte_cow = ((*rte)->flags & REF_COW);
   DBG( "Running filter `%s'...", filter->name );
@@ -311,14 +289,13 @@ f_run_val(const struct filter *filter, struct rte **rte, struct linpool *tmp_poo
     .stack = &filter_stack,
     .rte = rte,
     .pool = tmp_pool,
-    .val = val,
     .flags = flags,
   };
 
   LOG_BUFFER_INIT(filter_state.buf);
 
   /* Run the interpreter itself */
-  enum filter_return fret = interpret(&filter_state, filter->root, NULL);
+  enum filter_return fret = interpret(&filter_state, filter->root, argc, argv, NULL);
 
   if (filter_state.old_rta) {
     /*
@@ -370,8 +347,9 @@ f_run_val(const struct filter *filter, struct rte **rte, struct linpool *tmp_poo
  */
 
 enum filter_return
-f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool)
+f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool, uint argc, const struct f_val *argv, struct f_val *pres)
 {
+  struct rte *old_rte = *rte;
   filter_state = (struct filter_state) {
     .stack = &filter_stack,
     .rte = rte,
@@ -380,10 +358,17 @@ f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool
 
   LOG_BUFFER_INIT(filter_state.buf);
 
-  ASSERT(!((*rte)->flags & REF_COW));
+  enum filter_return fret = interpret(&filter_state, expr, argc, argv, pres);
-  ASSERT(!rta_is_cached((*rte)->attrs));
 
-  return interpret(&filter_state, expr, NULL);
+  if (filter_state.old_rta || (old_rte != *rte))
+  {
+    ASSERT_DIE(rta_is_cached(filter_state.old_rta) && !rta_is_cached((*rte)->attrs));
+    log(L_WARN "Attempted to change a read-only route, reverting");
+    (*rte)->attrs = filter_state.old_rta;
+    *rte = old_rte;
+  }
+
+  return fret;
 }
 
 /*
@@ -402,7 +387,7 @@ f_eval(const struct f_line *expr, struct linpool *tmp_pool, struct f_val *pres)
 
   LOG_BUFFER_INIT(filter_state.buf);
 
-  enum filter_return fret = interpret(&filter_state, expr, pres);
+  enum filter_return fret = interpret(&filter_state, expr, 0, NULL, pres);
   return fret;
 }
 

filter/filter.h

@@ -51,10 +51,9 @@ struct filter {
 
 struct rte;
 
-enum filter_return f_aggr_eval_line(const struct f_line *line, struct rte **rte, struct linpool *pool, struct f_val *pres);
 enum filter_return f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int flags);
-enum filter_return f_run_val(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, const struct f_val *val, int flags);
+enum filter_return f_run_args(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, uint argc, const struct f_val *argv, int flags);
-enum filter_return f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool);
+enum filter_return f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool, uint argc, const struct f_val *argv, struct f_val *pres);
 enum filter_return f_eval_buf(const struct f_line *expr, struct linpool *tmp_pool, buffer *buf);
 
 struct f_val cf_eval(const struct f_inst *inst, int type);

nest/aggregator.c

@@ -320,12 +320,13 @@ create_merged_rte(struct channel *c, struct network *net, const struct rte_val_l
   for (int i = 0; i < length; i++)
     rb->routes[i] = (struct rte *)rte_val[i]->rte;
 
+  /* merge filter needs one argument called "routes" */
   struct f_val val = {
     .type = T_ROUTES_BLOCK,
     .val.ad = &rb->ad,
   };
 
-  f_run_val(ail->merge_filter, &new, rte_update_pool, &val, 0);
+  f_eval_rte(ail->merge_filter, &new, rte_update_pool, 1, &val, 0);
 }
 
 /*
@@ -617,7 +618,7 @@ rt_notify_aggregated(struct channel *c, struct network *net, struct rte *new_cha
         case AGGR_ITEM_TERM: {
           const struct f_line *line = ail->items[val_idx].line;
           struct rte *rt1 = rt0;
-          enum filter_return fret = f_aggr_eval_line(line, &rt1, rte_update_pool, &rte_val->values[val_idx]);
+          enum filter_return fret = f_eval_rte(line, &rt1, rte_update_pool, 0, NULL, &rte_val->values[val_idx]);
 
           if (rt1 != rt0)
           {

nest/config.Y

@@ -141,6 +141,7 @@ CF_ENUM_PX(T_ENUM_AF, AF_, AFI_, IPV4, IPV6)
 
 %type <i32> idval
 %type <f> imexport
+%type <fl> merge_filter
 %type <r> rtable
 %type <s> optproto
 %type <ra> r_args
@@ -344,8 +345,18 @@ aggr_item:
     }
   ;
 
+merge_filter: MERGE BY {
+  cf_push_block_scope(new_config);
+  cf_create_symbol(new_config, "routes", SYM_VARIABLE | T_ROUTES_BLOCK, offset, f_new_var(sym_->scope));
+} function_body {
+  cf_pop_block_scope(new_config);
+  $4->args++;
+  $$ = $4;
+  }
+ ;
+
 aggr_definition:
-    AGGREGATE ON aggr_list MERGE BY filter {
+    AGGREGATE ON aggr_list merge_filter {
       _Bool net_present = 0;
       int count = 0;
 
@@ -375,7 +386,7 @@ aggr_definition:
      }
 
      linear->count = pos;
-     linear->merge_filter = $6;
+     linear->merge_filter = $4;
 
      int node = 1;
 

nest/proto.c

@@ -881,7 +881,7 @@ channel_reconfigure(struct channel *c, struct channel_config *cf)
   if (cf->ra_mode == RA_AGGREGATED)
   {
     export_changed |= !aggr_item_linearized_same(cf->ai_aggr, c->ai_aggr);
-    export_changed |= !filter_same(cf->ai_aggr->merge_filter, c->ai_aggr->merge_filter);
+    export_changed |= !f_same(cf->ai_aggr->merge_filter, c->ai_aggr->merge_filter);
     c->ai_aggr = cf->ai_aggr;
   }
 

nest/route.h

@@ -785,7 +785,7 @@ struct aggr_item {
 
 struct aggr_item_linearized {
   int count;
-  const struct filter *merge_filter;
+  const struct f_line *merge_filter;
   struct aggr_item_internal items[];
 };
 

proto/static/static.c

@@ -113,7 +113,7 @@ static_announce_rte(struct static_proto *p, struct static_route *r)
     net_copy(e->net->n.addr, r->net);
 
     /* Evaluate the filter */
-    f_eval_rte(r->cmds, &e, static_lp);
+    f_eval_rte(r->cmds, &e, static_lp, 0, NULL, NULL);
 
     /* Remove the temporary node */
     e->net = NULL;