From 732b3981b5fd097afe8a9681fb7b3b471af6b441 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Fri, 23 Jun 2023 11:28:19 +0200 Subject: [PATCH] Aggregator: polishing of filter API --- conf/conf.h | 3 +++ filter/config.Y | 2 -- filter/f-inst.c | 8 +----- filter/filter.c | 63 +++++++++++++++++-------------------------- filter/filter.h | 5 ++-- nest/aggregator.c | 5 ++-- nest/config.Y | 15 +++++++++-- nest/proto.c | 2 +- nest/route.h | 2 +- proto/static/static.c | 2 +- 10 files changed, 49 insertions(+), 58 deletions(-) diff --git a/conf/conf.h b/conf/conf.h index 1413cb58..56bde039 100644 --- a/conf/conf.h +++ b/conf/conf.h @@ -237,6 +237,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 *); diff --git a/filter/config.Y b/filter/config.Y index 4229f937..fbefb82d 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -356,7 +356,6 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, ACCEPT, REJECT, ERROR, INT, BOOL, IP, PREFIX, RD, PAIR, QUAD, EC, LC, SET, STRING, BGPMASK, BGPPATH, CLIST, ECLIST, LCLIST, - ROUTES, IF, THEN, ELSE, CASE, FOR, IN, DO, TRUE, FALSE, RT, RO, UNKNOWN, GENERIC, @@ -950,7 +949,6 @@ term: | 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); diff --git a/filter/f-inst.c b/filter/f-inst.c index 12ec60f5..b400172f 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -541,12 +541,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, 0, T_PATH, ad, &null_adata); METHOD_R(T_CLIST, empty, 0, T_CLIST, ad, &null_adata); METHOD_R(T_ECLIST, empty, 0, T_ECLIST, ad, &null_adata); @@ -613,7 +607,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); diff --git a/filter/filter.c b/filter/filter.c index f98eec97..b91f821a 100644 --- a/filter/filter.c +++ b/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; - memset(fstk->vstk, 0, sizeof(struct f_val) * line->vars); + /* Set the arguments and top-level variables */ + 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)); - ASSERT(!rta_is_cached((*rte)->attrs)); + enum filter_return fret = interpret(&filter_state, expr, argc, argv, pres); - 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; } @@ -424,7 +409,7 @@ f_eval_int(const struct f_line *expr) LOG_BUFFER_INIT(filter_state.buf); - if (interpret(&filter_state, expr, &val) > F_RETURN) + if (interpret(&filter_state, expr, 0, NULL, &val) > F_RETURN) cf_error("Runtime error while evaluating expression; see log for details"); if (val.type != T_INT) diff --git a/filter/filter.h b/filter/filter.h index f5035805..46d976a4 100644 --- a/filter/filter.h +++ b/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_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool); +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, uint argc, const struct f_val *argv, struct f_val *pres); uint f_eval_int(const struct f_line *expr); enum filter_return f_eval_buf(const struct f_line *expr, struct linpool *tmp_pool, buffer *buf); diff --git a/nest/aggregator.c b/nest/aggregator.c index dfaae1dd..4e374cc0 100644 --- a/nest/aggregator.c +++ b/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) { diff --git a/nest/config.Y b/nest/config.Y index bcd238d6..024f4dc0 100644 --- a/nest/config.Y +++ b/nest/config.Y @@ -141,6 +141,7 @@ CF_ENUM_PX(T_ENUM_AF, AF_, AFI_, IPV4, IPV6) %type idval %type imexport +%type merge_filter %type rtable %type optproto %type r_args @@ -346,8 +347,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; @@ -377,7 +388,7 @@ aggr_definition: } linear->count = pos; - linear->merge_filter = $6; + linear->merge_filter = $4; int node = 1; diff --git a/nest/proto.c b/nest/proto.c index 5b435f29..dcad77d7 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -879,7 +879,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; } diff --git a/nest/route.h b/nest/route.h index 9169a64c..e13de8c7 100644 --- a/nest/route.h +++ b/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[]; }; diff --git a/proto/static/static.c b/proto/static/static.c index bb93305e..cf7a4768 100644 --- a/proto/static/static.c +++ b/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;