From ca2ee91a80aa87b7f18898c28e93ff22cebf73d7 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Thu, 20 Dec 2018 15:25:04 +0100 Subject: [PATCH] Filter refactoring: The constant f_val is simply included inside the instruction With 32 bits, size of the args is 12 bytes, the f_val is 20 bytes. With 64 bits, size of the args is 24 bytes, the f_val the same. This is not so nice on 32 bits, anyway the f_inst itself is 24 vs. 32 bytes and the overall size of filters must be 32k of instructions to get to one megabyte of RAM eaten by f_inst. Therefore it seems to be improbable for common user to get into problems with this change. --- filter/config.Y | 91 ++++++++++++++++++++++--------------------------- filter/f-inst.c | 11 +----- filter/filter.h | 76 +++++++++++++++++++++-------------------- 3 files changed, 80 insertions(+), 98 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index 20a19075..e1f3fec8 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -189,15 +189,17 @@ f_generate_dpair(struct f_inst *t1, struct f_inst *t2) struct f_inst *rv; if ((t1->fi_code == FI_CONSTANT) && (t2->fi_code == FI_CONSTANT)) { - if ((t1->aux != T_INT) || (t2->aux != T_INT)) + if ((t1->val.type != T_INT) || (t2->val.type != T_INT)) cf_error( "Can't operate with value of non-integer type in pair constructor"); check_u16(t1->a[1].i); check_u16(t2->a[1].i); rv = f_new_inst(FI_CONSTANT); - rv->aux = T_PAIR; - rv->a[1].i = pair(t1->a[1].i, t2->a[1].i); + rv->val = (struct f_val) { + .type = T_PAIR, + .val.i = pair(t1->a[1].i, t2->a[1].i), + }; } else { rv = f_new_inst(FI_PAIR_CONSTRUCT); @@ -217,26 +219,12 @@ f_generate_ec(u16 kind, struct f_inst *tk, struct f_inst *tv) if (tk->fi_code == FI_CONSTANT) { c1 = 1; - - if (tk->aux == T_INT) { - ipv4_used = 0; key = tk->a[1].i; - } - else if (tk->aux == T_QUAD) { - ipv4_used = 1; key = tk->a[1].i; - } - else - cf_error("Can't operate with key of non-integer/IPv4 type in EC constructor"); - } - - /* IP->Quad implicit conversion */ - else if (tk->fi_code == FI_CONSTANT_INDIRECT) { - c1 = 1; - struct f_val *val = tk->a[0].p; + struct f_val *val = &(tk->val); if (val->type == T_INT) { ipv4_used = 0; key = val->val.i; } - else if (val->type == T_QUAD) { + else if (tk->val.type == T_QUAD) { ipv4_used = 1; key = val->val.i; } else if ((val->type == T_IP) && ipa_is_ip4(val->val.ip)) { @@ -247,10 +235,10 @@ f_generate_ec(u16 kind, struct f_inst *tk, struct f_inst *tv) } if (tv->fi_code == FI_CONSTANT) { - if (tv->aux != T_INT) + if (tv->val.type != T_INT) cf_error("Can't operate with value of non-integer type in EC constructor"); c2 = 1; - val2 = tv->a[1].i; + val2 = tv->val.val.i; } if (c1 && c2) { @@ -271,11 +259,11 @@ f_generate_ec(u16 kind, struct f_inst *tk, struct f_inst *tv) ec = ec_as4(kind, key, val2); } - NEW_F_VAL; - rv = f_new_inst(FI_CONSTANT_INDIRECT); - rv->a[0].p = val; - val->type = T_EC; - val->val.ec = ec; + rv = f_new_inst(FI_CONSTANT); + rv->val = (struct f_val) { + .type = T_EC, + .val.ec = ec, + }; } else { rv = f_new_inst(FI_EC_CONSTRUCT); @@ -293,15 +281,14 @@ f_generate_lc(struct f_inst *t1, struct f_inst *t2, struct f_inst *t3) struct f_inst *rv; if ((t1->fi_code == FI_CONSTANT) && (t2->fi_code == FI_CONSTANT) && (t3->fi_code == FI_CONSTANT)) { - if ((t1->aux != T_INT) || (t2->aux != T_INT) || (t3->aux != T_INT)) + if ((t1->val.type != T_INT) || (t2->val.type != T_INT) || (t3->val.type != T_INT)) cf_error( "LC - Can't operate with value of non-integer type in tuple constructor"); - rv = f_new_inst(FI_CONSTANT_INDIRECT); - - NEW_F_VAL; - rv->a[0].p = val; - val->type = T_LC; - val->val.lc = (lcomm) { t1->a[1].i, t2->a[1].i, t3->a[1].i }; + rv = f_new_inst(FI_CONSTANT); + rv->val = (struct f_val) { + .type = T_LC, + .val.lc = (lcomm) { t1->a[1].i, t2->a[1].i, t3->a[1].i }, + }; } else { @@ -325,12 +312,11 @@ f_generate_path_mask(struct f_path_mask *t) } } - NEW_F_VAL; - val->type = T_PATH_MASK; - val->val.path_mask = t; - - struct f_inst *rv = f_new_inst(FI_CONSTANT_INDIRECT); - rv->a[0].p = val; + struct f_inst *rv = f_new_inst(FI_CONSTANT); + rv->val = (struct f_val) { + .type = T_PATH_MASK, + .val.path_mask = t, + }; return rv; } @@ -771,8 +757,6 @@ switch_body: /* EMPTY */ { $$ = NULL; } } ; -/* CONST '(' expr ')' { $$ = f_new_inst(FI_CONSTANT); $$->aux = T_INT; $$->a[1].i = $3; } */ - bgp_path_expr: symbol { $$ = $1; } | '(' term ')' { $$ = $2; } @@ -792,16 +776,21 @@ bgp_path_tail: ; constant: - NUM { $$ = f_new_inst(FI_CONSTANT); $$->aux = T_INT; $$->a[1].i = $1; } - | TRUE { $$ = f_new_inst(FI_CONSTANT); $$->aux = T_BOOL; $$->a[1].i = 1; } - | FALSE { $$ = f_new_inst(FI_CONSTANT); $$->aux = T_BOOL; $$->a[1].i = 0; } - | TEXT { $$ = f_new_inst(FI_CONSTANT); $$->aux = T_STRING; $$->a[1].p = $1; } - | fipa { NEW_F_VAL; $$ = f_new_inst(FI_CONSTANT_INDIRECT); $$->a[0].p = val; *val = $1; } - | VPN_RD { NEW_F_VAL; $$ = f_new_inst(FI_CONSTANT_INDIRECT); val->type = T_RD; val->val.ec = $1; $$->a[0].p = val; } - | net_ { NEW_F_VAL; $$ = f_new_inst(FI_CONSTANT_INDIRECT); val->type = T_NET; val->val.net = $1; $$->a[0].p = val; } - | '[' set_items ']' { DBG( "We've got a set here..." ); $$ = f_new_inst(FI_CONSTANT); $$->aux = T_SET; $$->a[1].p = build_tree($2); DBG( "ook\n" ); } - | '[' fprefix_set ']' { $$ = f_new_inst(FI_CONSTANT); $$->aux = T_PREFIX_SET; $$->a[1].p = $2; } - | ENUM { $$ = f_new_inst(FI_CONSTANT); $$->aux = $1 >> 16; $$->a[1].i = $1 & 0xffff; } + NUM { $$ = f_new_inst(FI_CONSTANT); $$->val = (struct f_val) { .type = T_INT, .val.i = $1, }; } + | TRUE { $$ = f_new_inst(FI_CONSTANT); $$->val = (struct f_val) { .type = T_BOOL, .val.i = 1, }; } + | FALSE { $$ = f_new_inst(FI_CONSTANT); $$->val = (struct f_val) { .type = T_BOOL, .val.i = 0, }; } + | TEXT { $$ = f_new_inst(FI_CONSTANT); $$->val = (struct f_val) { .type = T_STRING, .val.s = $1, }; } + | fipa { $$ = f_new_inst(FI_CONSTANT); $$->val = $1; } + | VPN_RD { $$ = f_new_inst(FI_CONSTANT); $$->val = (struct f_val) { .type = T_RD, .val.ec = $1, }; } + | net_ { $$ = f_new_inst(FI_CONSTANT); $$->val = (struct f_val) { .type = T_NET, .val.net = $1, }; } + | '[' set_items ']' { + DBG( "We've got a set here..." ); + $$ = f_new_inst(FI_CONSTANT); + $$->val = (struct f_val) { .type = T_SET, .val.t = build_tree($2), }; + DBG( "ook\n" ); + } + | '[' fprefix_set ']' { $$ = f_new_inst(FI_CONSTANT); $$->val = (struct f_val) { .type = T_PREFIX_SET, .val.ti = $2, }; } + | ENUM { $$ = f_new_inst(FI_CONSTANT); $$->val = (struct f_val) { .type = $1 >> 16, .val.i = $1 & 0xffff, }; } ; constructor: diff --git a/filter/f-inst.c b/filter/f-inst.c index a7d381c2..fb2f043c 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -240,16 +240,7 @@ /* some constants have value in a[1], some in *a[0].p, strange. */ case FI_CONSTANT: /* integer (or simple type) constant, string, set, or prefix_set */ - res.type = what->aux; - - if (res.type == T_PREFIX_SET) - res.val.ti = what->a[1].p; - else if (res.type == T_SET) - res.val.t = what->a[1].p; - else if (res.type == T_STRING) - res.val.s = what->a[1].p; - else - res.val.i = what->a[1].i; + res = what->val; break; case FI_VARIABLE: case FI_CONSTANT_INDIRECT: diff --git a/filter/filter.h b/filter/filter.h index 8dd531b1..e483d223 100644 --- a/filter/filter.h +++ b/filter/filter.h @@ -14,6 +14,39 @@ #include "nest/route.h" #include "nest/attrs.h" +struct f_prefix { + net_addr net; + u8 lo, hi; +}; + +struct f_val { + int type; /* T_* */ + union { + uint i; + u64 ec; + lcomm lc; + ip_addr ip; + const net_addr *net; + char *s; + struct f_tree *t; + struct f_trie *ti; + struct adata *ad; + struct f_path_mask *path_mask; + } val; +}; + +struct f_dynamic_attr { + int type; + int f_type; + int ea_code; +}; + +struct f_static_attr { + int f_type; + int sa_code; + int readonly; +}; + /* Filter instruction types */ #define FI__TWOCHAR(a,b) ((a<<8) | b) @@ -88,9 +121,12 @@ struct f_inst { /* Instruction */ enum f_instruction_code fi_code; u16 aux; /* Extension to instruction code, T_*, EA_*, EAF_* */ union { - uint i; - void *p; - } a[3]; /* The three arguments */ + union { + uint i; + void *p; + } a[3]; /* The three arguments */ + struct f_val val; /* The value if FI_CONSTANT */ + }; int lineno; }; @@ -103,40 +139,6 @@ struct f_inst_roa_check { struct f_inst i; struct rtable_config *rtc; }; - -struct f_prefix { - net_addr net; - u8 lo, hi; -}; - -struct f_val { - int type; /* T_* */ - union { - uint i; - u64 ec; - lcomm lc; - ip_addr ip; - const net_addr *net; - char *s; - struct f_tree *t; - struct f_trie *ti; - struct adata *ad; - struct f_path_mask *path_mask; - } val; -}; - -struct f_dynamic_attr { - int type; - int f_type; - int ea_code; -}; - -struct f_static_attr { - int f_type; - int sa_code; - int readonly; -}; - struct filter { char *name; struct f_inst *root;