* Add minor optimisations on speed when comparing with int_cmp(), uint_cmp(), u64_cmp(), by removing branching. * Reorder type checks in val_compare() and val_in_range(). --- filter/filter.c | 64 ++++++++++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/filter/filter.c b/filter/filter.c index 0fc10f1..f92ee9a 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -112,25 +112,22 @@ pm_format(struct f_path_mask *p, byte *buf, unsigned int size) *buf = 0; } -static inline int int_cmp(int i1, int i2) +static inline int +int_cmp(int i1, int i2) { - if (i1 == i2) return 0; - if (i1 < i2) return -1; - else return 1; + return (i1 > i2) - (i1 < i2); } -static inline int uint_cmp(unsigned int i1, unsigned int i2) +static inline int +uint_cmp(unsigned int i1, unsigned int i2) { - if (i1 == i2) return 0; - if (i1 < i2) return -1; - else return 1; + return (i1 > i2) - (i1 < i2); } -static inline int u64_cmp(u64 i1, u64 i2) +static inline int +u64_cmp(u64 i1, u64 i2) { - if (i1 == i2) return 0; - if (i1 < i2) return -1; - else return 1; + return (i1 > i2) - (i1 < i2); } /** @@ -147,14 +144,11 @@ val_compare(struct f_val v1, struct f_val v2) { int rc; - if ((v1.type == T_VOID) && (v2.type == T_VOID)) - return 0; - if (v1.type == T_VOID) /* Hack for else */ - return -1; - if (v2.type == T_VOID) - return 1; - if (v1.type != v2.type) { + if (v1.type == T_VOID) /* Hack for else */ + return -1; + if (v2.type == T_VOID) + return 1; #ifndef IPV6 /* IP->Quad implicit conversion */ if ((v1.type == T_QUAD) && (v2.type == T_IP)) @@ -181,15 +175,13 @@ val_compare(struct f_val v1, struct f_val v2) case T_PREFIX: if (rc = ipa_compare(v1.val.px.ip, v2.val.px.ip)) return rc; - if (v1.val.px.len < v2.val.px.len) - return -1; - if (v1.val.px.len > v2.val.px.len) - return 1; - return 0; + return int_cmp(v1.val.px.len, v2.val.px.len); case T_PATH_MASK: return pm_path_compare(v1.val.path_mask, v2.val.path_mask); case T_STRING: return strcmp(v1.val.s, v2.val.s); + case T_VOID: + return 0; default: debug( "Compare of unknown entities: %x\n", v1.type ); return CMP_ERROR; @@ -408,18 +400,6 @@ val_in_range(struct f_val v1, struct f_val v2) if (res != CMP_ERROR) return res; - - if ((v1.type == T_PREFIX) && (v2.type == T_PREFIX_SET)) - return trie_match_fprefix(v2.val.ti, &v1.val.px); - - if ((v1.type == T_CLIST) && (v2.type == T_SET)) - return clist_match_set(v1.val.ad, v2.val.t); - - if ((v1.type == T_ECLIST) && (v2.type == T_SET)) - return eclist_match_set(v1.val.ad, v2.val.t); - - if ((v1.type == T_PATH) && (v2.type == T_SET)) - return as_path_match_set(v1.val.ad, v2.val.t); if (v2.type == T_SET) switch (v1.type) { @@ -436,7 +416,19 @@ val_in_range(struct f_val v1, struct f_val v2) return 0; return !! (val_simple_in_range(v1, n->from)); /* We turn CMP_ERROR into compared ok, and that's fine */ } + case T_PATH: + return as_path_match_set(v1.val.ad, v2.val.t); + case T_CLIST: + return clist_match_set(v1.val.ad, v2.val.t); + case T_ECLIST: + return eclist_match_set(v1.val.ad, v2.val.t); + } + else if (v2.type == T_PREFIX_SET) + switch (v1.type) { + case T_PREFIX: + return trie_match_fprefix(v2.val.ti, &v1.val.px); } + return CMP_ERROR; } -- 1.7.10.4

* Intdodoce list_compare() to compare clist and eclist as arrays of u32. eclist stored as u64 with MSB first, so we could compare it also as u32. * Introduce path_compare() to compare bgppath type. Compares path length, and element by element, considering AS_PATH_SET more longer. --- filter/filter.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ nest/a-path.c | 7 ----- nest/attrs.h | 4 +++ 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/filter/filter.c b/filter/filter.c index f92ee9a..6f59193 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -130,6 +130,77 @@ u64_cmp(u64 i1, u64 i2) return (i1 > i2) - (i1 < i2); } +static int +list_compare(struct adata *a1, struct adata *a2) +{ + u32 *l1, *l2; + int l, rc; + + rc = (a1 > a2) - (a1 < a2); + if (!rc) + return rc; + if ((!!a1) != (!!a2)) + return rc; + + rc = uint_cmp(a1->length, a2->length); + if (rc) + return rc; + + l1 = int_set_get_data(a1); + l2 = int_set_get_data(a2); + l = int_set_get_size(a1); + + while (l--) { + rc = uint_cmp(*l1++, *l2++); + if (rc) + return rc; + } + + return rc; +} + +static int +path_compare(struct adata *a1, struct adata *a2) +{ + u8 *p1, *q1; + u8 *p2, *q2; + int l, rc; + + rc = (a1 > a2) - (a1 < a2); + if (!rc) + return rc; + if ((!!a1) != (!!a2)) + return rc; + + rc = int_cmp(as_path_getlen(a1), as_path_getlen(a2)); + if (rc) + return rc; + + p1 = a1->data; + q1 = p1 + a1->length; + p2 = a2->data; + q2 = p2 + a2->length; + + while ((p1 < q1) && (p2 < q2)) { + rc = uint_cmp(*p1++, *p2++); + if (rc) + return -rc; /* AS_PATH_SET longer */ + + l = *p1++; + rc = int_cmp(l, *p2++); + if (rc) + return rc; + + while (l--) { + rc = uint_cmp(get_as(p1++), get_as(p2++)); + if (rc) + return rc; + } + } + + return int_cmp((p1 < q1), (p2 < q2)); +} + /** * val_compare - compare two values * @v1: first value @@ -180,6 +251,11 @@ val_compare(struct f_val v1, struct f_val v2) return pm_path_compare(v1.val.path_mask, v2.val.path_mask); case T_STRING: return strcmp(v1.val.s, v2.val.s); + case T_CLIST: + case T_ECLIST: + return list_compare(v1.val.ad, v2.val.ad); + case T_PATH: + return path_compare(v1.val.ad, v2.val.ad); case T_VOID: return 0; default: diff --git a/nest/a-path.c b/nest/a-path.c index b181298..1679886 100644 --- a/nest/a-path.c +++ b/nest/a-path.c @@ -15,13 +15,6 @@ #include "lib/string.h" #include "filter/filter.h" -// static inline void put_as(byte *data, u32 as) { put_u32(data, as); } -// static inline u32 get_as(byte *data) { return get_u32(data); } - -#define put_as put_u32 -#define get_as get_u32 -#define BS 4 - struct adata * as_path_prepend(struct linpool *pool, struct adata *olda, u32 as) { diff --git a/nest/attrs.h b/nest/attrs.h index 44a23e1..38639ae 100644 --- a/nest/attrs.h +++ b/nest/attrs.h @@ -25,6 +25,10 @@ * to 16bit slot (like in 16bit AS_PATH). See RFC 4893 for details */ +#define put_as put_u32 +#define get_as get_u32 +#define BS 4 + struct f_tree; struct adata *as_path_prepend(struct linpool *pool, struct adata *olda, u32 as); -- 1.7.10.4

val_same() compares passed @v1 and @v2 and returns 1 if they are same, 0 if not and 999 in case their types not comparable. Change pm_path_compare() to return 1 - if @m1 and @m2 are the same and 0 overwise. Rename pm_path_compare() to pm_path_same() according to its return values. Use it in val_same() to compare path masks. --- filter/filter.c | 102 +++++++++++++++++++++++++++++-------------------------- filter/filter.h | 1 + 2 files changed, 54 insertions(+), 49 deletions(-) diff --git a/filter/filter.c b/filter/filter.c index 6f59193..c089d94 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -58,20 +58,6 @@ adata_empty(struct linpool *pool, int l) return res; } -static int -pm_path_compare(struct f_path_mask *m1, struct f_path_mask *m2) -{ - while (1) { - if ((!m1) || (!m2)) - return !((!m1) && (!m2)); - - /* FIXME: buggy, should return -1, 0, 1; but it doesn't matter */ - if ((m1->kind != m2->kind) || (m1->val != m2->val)) return 1; - m1 = m1->next; - m2 = m2->next; - } -} - u32 f_eval_asn(struct f_inst *expr); static void @@ -247,8 +233,6 @@ val_compare(struct f_val v1, struct f_val v2) if (rc = ipa_compare(v1.val.px.ip, v2.val.px.ip)) return rc; return int_cmp(v1.val.px.len, v2.val.px.len); - case T_PATH_MASK: - return pm_path_compare(v1.val.path_mask, v2.val.path_mask); case T_STRING: return strcmp(v1.val.s, v2.val.s); case T_CLIST: @@ -259,7 +243,50 @@ val_compare(struct f_val v1, struct f_val v2) case T_VOID: return 0; default: - debug( "Compare of unknown entities: %x\n", v1.type ); + return CMP_ERROR; + } +} + +static int +pm_path_same(struct f_path_mask *m1, struct f_path_mask *m2) +{ + while (1) { + if ((!m1) || (!m2)) + return (!m1) && (!m2); + + if ((m1->kind != m2->kind) || (m1->val != m2->val)) + return 0; + + m1 = m1->next; + m2 = m2->next; + } +} + +/** + * val_same - compare two values + * @v1: first value + * @v2: second value + * + * Compares two values and returns 1 if they are same, 0 if not + * and 999 on error. + */ +int +val_same(struct f_val v1, struct f_val v2) +{ + int rc; + + rc = val_compare(v1, v2); + if (rc != CMP_ERROR) + return !rc; + + switch (v1.type) { + case T_PATH_MASK: + return pm_path_same(v1.val.path_mask, v2.val.path_mask); + case T_SET: + return same_tree(v1.val.t, v2.val.t); + case T_PREFIX_SET: + return trie_same(v1.val.ti, v2.val.ti); + default: return CMP_ERROR; } } @@ -776,19 +803,19 @@ interpret(struct f_inst *what) /* Relational operators */ -#define COMPARE(x) \ +#define COMPARE(f,x) \ TWOARGS; \ - i = val_compare(v1, v2); \ - if (i==CMP_ERROR) \ + i = f(v1, v2); \ + if (i == CMP_ERROR) \ runtime( "Can't compare values of incompatible types" ); \ res.type = T_BOOL; \ res.val.i = (x); \ break; - case P('!','='): COMPARE(i!=0); - case P('=','='): COMPARE(i==0); - case '<': COMPARE(i==-1); - case P('<','='): COMPARE(i!=1); + case P('!','='): COMPARE(val_same, i == 0); + case P('=','='): COMPARE(val_same, i != 0); + case '<': COMPARE(val_compare, i < 0); + case P('<','='): COMPARE(val_compare, i <= 0); case '!': ONEARG; @@ -1478,31 +1505,8 @@ i_same(struct f_inst *f1, struct f_inst *f2) } break; case 'C': - { - struct f_val *v1 = (struct f_val *) f1->a1.p; - struct f_val *v2 = (struct f_val *) f2->a1.p; - - /* Handle some cases that are not handled by val_compare() - also T_PATH, T_CLIST and T_ECLIST does not work, - but you cannot easily create such constants */ - - if ((v1->type == T_SET) && (v2->type == T_SET)) - { - if (!same_tree(v1->val.t, v2->val.t)) - return 0; - break; - } - - if ((v1->type == T_PREFIX_SET) && (v2->type == T_PREFIX_SET)) - { - if (!trie_same(v1->val.ti, v2->val.ti)) - return 0; - break; - } - - if (val_compare(*v1 , *v2)) - return 0; - } + if (!val_same(* (struct f_val *) f1->a1.p, * (struct f_val *) f2->a1.p)) + return 0; break; case 'V': if (strcmp((char *) f1->a2.p, (char *) f2->a2.p)) diff --git a/filter/filter.h b/filter/filter.h index dcac825..a3de530 100644 --- a/filter/filter.h +++ b/filter/filter.h @@ -116,6 +116,7 @@ int filter_same(struct filter *new, struct filter *old); int i_same(struct f_inst *f1, struct f_inst *f2); int val_compare(struct f_val v1, struct f_val v2); +int val_same(struct f_val v1, struct f_val v2); int tree_compare(const void *p1, const void *p2); void val_print(struct f_val v); -- 1.7.10.4

* Remove duplicate f_eval_asn() declaration: already in filter/filter.h * Move pm_format() from filter/filter.c to nest/a-path.c where other pm_* functions reside and also similar *_set_format() family already resides in nest/a-set.c. * Move tree_compare() from filter/filter.c to filter/tree.c and make it static as it only used in qsort(). Also remove definition from filter/filter.h. * Inline fprefix_get_bounds() from filter/filter.c in trie_add_prefix() at filter/filter.h as there is only one place where this code used. * Move tree_node_print() and tree_print() from filter/filter.c to filter/tree.c Make tree_node_print() static. Add global comment to tree_print() and declare it in filter.h. This is similar to trie_print() from filter/trie.c * Make printing of zero prefix in trie_print() at filter/trie.c to print AFI specific value. --- filter/filter.c | 93 ------------------------------------------------------- filter/filter.h | 19 +++++++++--- filter/tree.c | 41 ++++++++++++++++++++++++ filter/trie.c | 2 +- nest/a-path.c | 38 +++++++++++++++++++++++ nest/attrs.h | 1 + 6 files changed, 95 insertions(+), 99 deletions(-) diff --git a/filter/filter.c b/filter/filter.c index c089d94..7e91c71 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -58,46 +58,6 @@ adata_empty(struct linpool *pool, int l) return res; } -u32 f_eval_asn(struct f_inst *expr); - -static void -pm_format(struct f_path_mask *p, byte *buf, unsigned int size) -{ - byte *end = buf + size - 16; - - while (p) - { - if (buf > end) - { - strcpy(buf, " ..."); - return; - } - - switch(p->kind) - { - case PM_ASN: - buf += bsprintf(buf, " %u", p->val); - break; - - case PM_QUESTION: - buf += bsprintf(buf, " ?"); - break; - - case PM_ASTERISK: - buf += bsprintf(buf, " *"); - break; - - case PM_ASN_EXPR: - buf += bsprintf(buf, " %u", f_eval_asn((struct f_inst *) p->val)); - break; - } - - p = p->next; - } - - *buf = 0; -} - static inline int int_cmp(int i1, int i2) { @@ -291,30 +251,6 @@ val_same(struct f_val v1, struct f_val v2) } } -int -tree_compare(const void *p1, const void *p2) -{ - return val_compare((* (struct f_tree **) p1)->from, (* (struct f_tree **) p2)->from); -} - -void -fprefix_get_bounds(struct f_prefix *px, int *l, int *h) -{ - *l = *h = px->len & LEN_MASK; - - if (px->len & LEN_MINUS) - *l = 0; - - else if (px->len & LEN_PLUS) - *h = MAX_PREFIX_LENGTH; - - else if (px->len & LEN_RANGE) - { - *l = 0xff & (px->len >> 16); - *h = 0xff & (px->len >> 8); - } -} - /* * val_simple_in_range - check if @v1 ~ @v2 for everything except sets */ @@ -535,35 +471,6 @@ val_in_range(struct f_val v1, struct f_val v2) return CMP_ERROR; } -static void -tree_node_print(struct f_tree *t, char **sep) -{ - if (t == NULL) - return; - - tree_node_print(t->left, sep); - - logn(*sep); - val_print(t->from); - if (val_compare(t->from, t->to) != 0) - { - logn( ".." ); - val_print(t->to); - } - *sep = ", "; - - tree_node_print(t->right, sep); -} - -static void -tree_print(struct f_tree *t) -{ - char *sep = ""; - logn( "[" ); - tree_node_print(t, &sep); - logn( "] " ); -} - /* * val_print - format filter value */ diff --git a/filter/filter.h b/filter/filter.h index a3de530..9e60ef8 100644 --- a/filter/filter.h +++ b/filter/filter.h @@ -78,6 +78,7 @@ struct f_inst *f_generate_roa_check(struct symbol *sym, struct f_inst *prefix, s struct f_tree *build_tree(struct f_tree *); struct f_tree *find_tree(struct f_tree *t, struct f_val val); int same_tree(struct f_tree *t1, struct f_tree *t2); +void tree_print(struct f_tree *t); struct f_trie *f_new_trie(linpool *lp); void trie_add_prefix(struct f_trie *t, ip_addr px, int plen, int l, int h); @@ -85,13 +86,23 @@ int trie_match_prefix(struct f_trie *t, ip_addr px, int plen); int trie_same(struct f_trie *t1, struct f_trie *t2); void trie_print(struct f_trie *t); -void fprefix_get_bounds(struct f_prefix *px, int *l, int *h); - static inline void trie_add_fprefix(struct f_trie *t, struct f_prefix *px) { int l, h; - fprefix_get_bounds(px, &l, &h); + + l = h = px->len & LEN_MASK; + + if (px->len & LEN_MINUS) + l = 0; + else if (px->len & LEN_PLUS) + h = MAX_PREFIX_LENGTH; + else if (px->len & LEN_RANGE) + { + l = 0xff & (px->len >> 16); + h = 0xff & (px->len >> 8); + } + trie_add_prefix(t, px->ip, px->len & LEN_MASK, l, h); } @@ -117,8 +128,6 @@ int i_same(struct f_inst *f1, struct f_inst *f2); int val_compare(struct f_val v1, struct f_val v2); int val_same(struct f_val v1, struct f_val v2); -int tree_compare(const void *p1, const void *p2); - void val_print(struct f_val v); #define F_NOP 0 diff --git a/filter/tree.c b/filter/tree.c index f6ab75b..4690f85 100644 --- a/filter/tree.c +++ b/filter/tree.c @@ -53,6 +53,12 @@ build_tree_rec(struct f_tree **buf, int l, int h) return n; } +static int +tree_compare(const void *p1, const void *p2) +{ + return val_compare((* (struct f_tree **) p1)->from, + (* (struct f_tree **) p2)->from); +} /** * build_tree @@ -132,3 +138,38 @@ same_tree(struct f_tree *t1, struct f_tree *t2) return 0; return 1; } + +static void +tree_node_print(struct f_tree *t, char **sep) +{ + if (t == NULL) + return; + + tree_node_print(t->left, sep); + + logn(*sep); + val_print(t->from); + if (val_compare(t->from, t->to) != 0) + { + logn( ".." ); + val_print(t->to); + } + *sep = ", "; + + tree_node_print(t->right, sep); +} + +/** + * tree_print + * @t: tree to be printed + * + * Prints the tree to the log buffer. + */ +void +tree_print(struct f_tree *t) +{ + char *sep = ""; + logn( "[" ); + tree_node_print(t, &sep); + logn( "] " ); +} diff --git a/filter/trie.c b/filter/trie.c index 581332c..f42afb8 100644 --- a/filter/trie.c +++ b/filter/trie.c @@ -293,7 +293,7 @@ trie_print(struct f_trie *t) logn("["); if (t->zero) { - logn("0.0.0.0/0"); + logn("%I/%d", IPA_NONE, 0); sep = ", "; } trie_node_print(&t->root, &sep); diff --git a/nest/a-path.c b/nest/a-path.c index 1679886..cf129c1 100644 --- a/nest/a-path.c +++ b/nest/a-path.c @@ -434,6 +434,44 @@ pm_mark(struct pm_pos *pos, int i, int plen, int *nl, int *nh) *nh = j; } +void +pm_format(struct f_path_mask *p, byte *buf, unsigned int size) +{ + byte *end = buf + size - 16; + + while (p) + { + if (buf > end) + { + strcpy(buf, " ..."); + return; + } + + switch(p->kind) + { + case PM_ASN: + buf += bsprintf(buf, " %u", p->val); + break; + + case PM_QUESTION: + buf += bsprintf(buf, " ?"); + break; + + case PM_ASTERISK: + buf += bsprintf(buf, " *"); + break; + + case PM_ASN_EXPR: + buf += bsprintf(buf, " %u", f_eval_asn((struct f_inst *) p->val)); + break; + } + + p = p->next; + } + + *buf = 0; +} + /* AS path matching is nontrivial. Because AS path can * contain sets, it is not a plain wildcard matching. A set * in an AS path is interpreted as it might represent any diff --git a/nest/attrs.h b/nest/attrs.h index 38639ae..9468355 100644 --- a/nest/attrs.h +++ b/nest/attrs.h @@ -56,6 +56,7 @@ struct f_path_mask { }; int as_path_match(struct adata *path, struct f_path_mask *mask); +void pm_format(struct f_path_mask *p, byte *buf, unsigned int size); /* a-set.c */ -- 1.7.10.4

Currently isn't possible to determine if clist or eclist defined (using defined() always return true for extended attributes of clist and eclist type, as they are special cased in P('e','s') instruction in interpret() function) or even known number of entries in these list. One of the way to determine if extended attribute of such type is empty is compare it to appropriate predefined constant: "- EMPTY -" for clist type, and "-- EMPTY --" for eclist type. However these constants, among with "+ EMPTY +" for bgppath type are intended for internal use and testing purposes and never documented. Add C.len operator just like for bgppath type to get number of elements in lists. Thats also enables to check if list is empty by checking if number of elements is zero. --- doc/bird.sgml | 2 ++ filter/filter.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/bird.sgml b/doc/bird.sgml index f43eb4b..050acf3 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -1077,6 +1077,8 @@ incompatible with each other (that is to prevent you from shooting in the foot). no literals of this type. There are three special operators on clists: + <cf><m/C/.len</cf> returns the length of clist <m/C/. + <cf>add(<m/C/,<m/P/)</cf> adds pair (or quad) <m/P/ to clist <m/C/ and returns the result. If item <m/P/ is already in clist <m/C/, it does nothing. <m/P/ may also be a clist, diff --git a/filter/filter.c b/filter/filter.c index 7e91c71..8164ead 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -1065,7 +1065,9 @@ interpret(struct f_inst *what) switch(v1.type) { case T_PREFIX: res.val.i = v1.val.px.len; break; case T_PATH: res.val.i = as_path_getlen(v1.val.ad); break; - default: runtime( "Prefix or path expected" ); + case T_CLIST: res.val.i = v1.val.ad->length / 4; break; + case T_ECLIST: res.val.i = v1.val.ad->length / 8; break; + default: runtime( "Prefix, path, clist or eclist expected" ); } break; case P('c','p'): /* Convert prefix to ... */ -- 1.7.10.4

Use RESET as predicate to set to empty value of dynamic attribute value in filters. Leave EMPTY for backwad compatibility, altought it not documented enywhere. Uninline f_generate_empty() and refactor code in error path. Remove MATCH and CONTAINS from keyword list. --- doc/bird.sgml | 4 ++++ filter/config.Y | 35 ++++++++++++++++------------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/doc/bird.sgml b/doc/bird.sgml index 050acf3..d1d7860 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -1053,6 +1053,8 @@ incompatible with each other (that is to prevent you from shooting in the foot). I.e., <cf/filter/ do the same as <cf/delete/ with inverted set <m/A/. + <cf><m/P/.reset</cf> deletes all ASNs from path <m/P/. + Statement <cf><m/P/ = prepend(<m/P/, <m/A/);</cf> can be shortened to <cf><m/P/.prepend(<m/A/);</cf> if <m/P/ is appropriate route attribute (for example <cf/bgp_path/). Similarly for <cf/delete/ and <cf/filter/. @@ -1098,6 +1100,8 @@ incompatible with each other (that is to prevent you from shooting in the foot). set <m/P/. <m/P/ may also be a clist, which works analogously; i.e., it works as clist intersection. + <cf><m/C/.reset</cf> deletes all items from clist <m/C/. + Statement <cf><m/C/ = add(<m/C/, <m/P/);</cf> can be shortened to <cf><m/C/.add(<m/P/);</cf> if <m/C/ is appropriate route attribute (for example <cf/bgp_community/). Similarly for diff --git a/filter/config.Y b/filter/config.Y index 04acfba..faa02f5 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -115,32 +115,36 @@ f_new_ec_item(u32 kind, u32 ipv4_used, u32 key, u32 vf, u32 vt) return t; } -static inline struct f_inst * +static struct f_inst * f_generate_empty(struct f_inst *dyn) -{ - struct f_inst *e = f_new_inst(); - e->code = 'E'; +{ + struct f_inst *e; + u16 aux; switch (dyn->aux & EAF_TYPE_MASK) { case EAF_TYPE_AS_PATH: - e->aux = T_PATH; + aux = T_PATH; break; case EAF_TYPE_INT_SET: - e->aux = T_CLIST; + aux = T_CLIST; break; case EAF_TYPE_EC_SET: - e->aux = T_ECLIST; + aux = T_ECLIST; break; default: cf_error("Can't empty that attribute"); } + e = f_new_inst(); + e->code = 'E'; + e->aux = aux; + dyn->code = P('e','S'); dyn->a1.p = e; + return dyn; } - static inline struct f_inst * f_generate_dpair(struct f_inst *t1, struct f_inst *t2) { @@ -265,10 +269,9 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, PREFERENCE, LEN, DEFINED, - ADD, DELETE, CONTAINS, RESET, - PREPEND, FIRST, LAST, MATCH, + ADD, DELETE, RESET, EMPTY, + PREPEND, FIRST, LAST, ROA_CHECK, - EMPTY, FILTER, WHERE, EVAL) %nonassoc THEN @@ -727,14 +730,6 @@ term: | term '.' FIRST { $$ = f_new_inst(); $$->code = P('a','f'); $$->a1.p = $1; } | term '.' LAST { $$ = f_new_inst(); $$->code = P('a','l'); $$->a1.p = $1; } -/* Communities */ -/* This causes one shift/reduce conflict - | rtadot dynamic_attr '.' ADD '(' term ')' { } - | rtadot dynamic_attr '.' DELETE '(' term ')' { } - | rtadot dynamic_attr '.' CONTAINS '(' term ')' { } - | rtadot dynamic_attr '.' RESET{ } -*/ - | '+' EMPTY '+' { $$ = f_new_inst(); $$->code = 'E'; $$->aux = T_PATH; } | '-' EMPTY '-' { $$ = f_new_inst(); $$->code = 'E'; $$->aux = T_CLIST; } | '-' '-' EMPTY '-' '-' { $$ = f_new_inst(); $$->code = 'E'; $$->aux = T_ECLIST; } @@ -880,6 +875,8 @@ cmd: | rtadot dynamic_attr '.' EMPTY ';' { $$ = f_generate_empty($2); } + | rtadot dynamic_attr '.' RESET ';' { $$ = f_generate_empty($2); } + | rtadot dynamic_attr '.' PREPEND '(' term ')' ';' { $$ = f_generate_complex( P('A','p'), 'x', $2, $6 ); } | rtadot dynamic_attr '.' ADD '(' term ')' ';' { $$ = f_generate_complex( P('C','a'), 'a', $2, $6 ); } | rtadot dynamic_attr '.' DELETE '(' term ')' ';' { $$ = f_generate_complex( P('C','a'), 'd', $2, $6 ); } -- 1.7.10.4

There are two types of filter errors: which could be checked and reported on configuration file parsing and runtime, which could not be checked at compile time but could be only triggered at runtime filter statement evaluation. Currently there no mechanism in BIRD to branch to alternative action (or gust ignore) in case of filter runtime error. This patch introduces try ... catch statement to catch an runtime errors and execute alternative action. --- doc/bird.sgml | 23 ++++++++++++++++++++++- filter/config.Y | 8 +++++++- filter/filter.c | 7 +++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/doc/bird.sgml b/doc/bird.sgml index d1d7860..65fc207 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -1139,7 +1139,7 @@ specify a prefix and an ASN as arguments. <sect>Control structures -<p>Filters support two control structures: conditions and case switches. +<p>Filters support tree control structures: conditions, case switches and filter runtime error handing. <p>Syntax of a condition is: <cf>if <M>boolean expression</M> then <M>command1</M>; else <M>command2</M>;</cf> and you can use <cf>{ @@ -1167,6 +1167,27 @@ if 1234 = i then printn "."; else { } </code> +<p>Syntax of filter runtime error handing statement is: +<cf>try <m/command1/ catch <m/command2/</cf>, where <cf/try/ statement +executes <cf><m/command1/</cf> and if it throws an filter runtime error +(for example dynamic attribute not present), <cf/catch/ statement +executes <cf><m/command2/</cf>. +An log entry is still created for statement in <cf><m/command1/</cf> which +throws an error. + +<p>Here is example on how to use <cf>try ... catch</cf> statement: + +<code> +try { + # If neighbor is not on directly connected subnet, or it denotes + # host itself: generate runtime error + gw = 192.0.2.2; +} catch { + # Setting gw unsuccessfully; mark route as blackhole + dest = RTD_BLACKHOLE; +} +</code> + <sect>Route attributes <p>A filter is implicitly passed a route, and it can access its diff --git a/filter/config.Y b/filter/config.Y index faa02f5..42b9e19 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -263,7 +263,7 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, ACCEPT, REJECT, ERROR, QUITBIRD, INT, BOOL, IP, PREFIX, PAIR, QUAD, EC, SET, STRING, BGPMASK, BGPPATH, CLIST, ECLIST, - IF, THEN, ELSE, CASE, + IF, THEN, ELSE, CASE, TRY, CATCH, TRUE, FALSE, RT, RO, UNKNOWN, GENERIC, FROM, GW, NET, MASK, PROTO, SOURCE, SCOPE, CAST, DEST, IFNAME, IFINDEX, PREFERENCE, @@ -826,6 +826,12 @@ cmd: $$->a1.p = i; $$->a2.p = $6; } + | TRY block CATCH block { + $$ = f_new_inst(); + $$->code = P('T','C'); + $$->a1.p = $2; + $$->a2.p = $4; + } | SYM '=' term ';' { $$ = f_new_inst(); DBG( "Ook, we'll set value\n" ); diff --git a/filter/filter.c b/filter/filter.c index 8164ead..5610f3f 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -1323,6 +1323,12 @@ interpret(struct f_inst *what) res.val.i = roa_check(rtc->table, v1.val.px.ip, v1.val.px.len, as); break; + case P('T','C'): + res = interpret(what->a1.p); + if ((res.type == T_RETURN) && (res.val.i == F_ERROR)) + ARG(res, a2.p); + break; + default: bug( "Unknown instruction %d (%c)", what->code, what->code & 0xff); } @@ -1453,6 +1459,7 @@ i_same(struct f_inst *f1, struct f_inst *f2) if (strcmp(((struct f_inst_roa_check *) f1)->rtc->name, ((struct f_inst_roa_check *) f2)->rtc->name)) return 0; + case P('T','C'): TWOARGS; break; break; default: bug( "Unknown instruction %d in same (%c)", f1->code, f1->code & 0xff); -- 1.7.10.4

There are no separate operator in BIRD filter syntax which executes NO OPeration. However interpret() at filter/filter.c contains an instruction '0' to handle such operator. Use ';' as NOOP statement. --- filter/config.Y | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/filter/config.Y b/filter/config.Y index 42b9e19..fa87c76 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -810,7 +810,11 @@ var_list: /* EMPTY */ { $$ = NULL; } ; cmd: - IF term THEN block { + ';' { + $$ = f_new_inst(); + $$->code = '0'; + } + | IF term THEN block { $$ = f_new_inst(); $$->code = '?'; $$->a1.p = $2; -- 1.7.10.4

Replace debug() calls with DBG() macro to call debug() only when building BIRD with GLOBAL_DEBUG or LOCAL_DEBUG. Additionally fix build with GLOBAL_DEBUG enabled and --with-protocols=all. --- TODO | 1 - filter/filter.c | 6 +++--- lib/event.c | 2 +- lib/mempool.c | 2 +- lib/resource.c | 14 +++++++------- lib/slab.c | 4 ++-- nest/iface.c | 30 +++++++++++++++--------------- nest/locks.c | 4 ++-- nest/neighbor.c | 16 ++++++++-------- nest/proto.c | 18 +++++++++--------- nest/rt-attr.c | 40 ++++++++++++++++++++-------------------- nest/rt-fib.c | 12 ++++++------ nest/rt-table.c | 12 ++++++------ proto/ospf/neighbor.c | 2 +- proto/ospf/packet.c | 2 +- proto/rip/rip.c | 10 +++++----- proto/static/static.c | 12 ++++++------ sysdep/unix/io.c | 26 +++++++++++++------------- sysdep/unix/krt.c | 4 ++-- sysdep/unix/main.c | 4 ++-- 20 files changed, 110 insertions(+), 111 deletions(-) diff --git a/TODO b/TODO index 23cd187..d9b6a20 100644 --- a/TODO +++ b/TODO @@ -6,7 +6,6 @@ Core Globals ~~~~~~~ -- right usage of DBG vs. debug - logging and tracing; use appropriate log levels - check incoming packets and log errors!! - check log calls for trailing newlines and log levels followed by comma diff --git a/filter/filter.c b/filter/filter.c index 3e8af1e..69cf579 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -174,7 +174,7 @@ val_compare(struct f_val v1, struct f_val v2) return uint_cmp(ipa_to_u32(v1.val.px.ip), v2.val.i); #endif - debug( "Types do not match in val_compare\n" ); + DBG( "Types do not match in val_compare\n" ); return CMP_ERROR; } switch (v1.type) { @@ -797,7 +797,7 @@ interpret(struct f_inst *what) res.type = T_BOOL; break; case '0': - debug( "No operation\n" ); + DBG( "No operation\n" ); break; case P('p',','): ONEARG; @@ -1126,7 +1126,7 @@ interpret(struct f_inst *what) v1.type = T_VOID; t = find_tree(what->a2.p, v1); if (!t) { - debug( "No else statement?\n"); + DBG( "No else statement?\n"); break; } } diff --git a/lib/event.c b/lib/event.c index 916cf55..f1703cb 100644 --- a/lib/event.c +++ b/lib/event.c @@ -39,7 +39,7 @@ ev_dump(resource *r) { event *e = (event *) r; - debug("(code %p, data %p, %s)\n", + DBG("(code %p, data %p, %s)\n", e->hook, e->data, e->n.next ? "scheduled" : "inactive"); diff --git a/lib/mempool.c b/lib/mempool.c index 65072f9..42418e9 100644 --- a/lib/mempool.c +++ b/lib/mempool.c @@ -228,7 +228,7 @@ lp_dump(resource *r) ; for(cntl=0, c=m->first_large; c; c=c->next, cntl++) ; - debug("(chunk=%d threshold=%d count=%d+%d total=%d+%d)\n", + DBG("(chunk=%d threshold=%d count=%d+%d total=%d+%d)\n", m->chunk_size, m->threshold, cnt, diff --git a/lib/resource.c b/lib/resource.c index 42243aa..ae955a4 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -90,7 +90,7 @@ pool_dump(resource *P) pool *p = (pool *) P; resource *r; - debug("%s\n", p->name); + DBG("%s\n", p->name); indent += 3; WALK_LIST(r, p->inside) rdump(r); @@ -182,14 +182,14 @@ rdump(void *res) resource *r = res; bsprintf(x, "%%%ds%%p ", indent); - debug(x, "", r); + DBG(x, "", r); if (r) { - debug("%s ", r->class->name); + DBG("%s ", r->class->name); r->class->dump(r); } else - debug("NULL\n"); + DBG("NULL\n"); } size_t @@ -240,11 +240,11 @@ rlookup(unsigned long a) { resource *r; - debug("Looking up %08lx\n", a); + DBG("Looking up %08lx\n", a); if (r = pool_lookup(&root_pool.r, a)) rdump(r); else - debug("Not found.\n"); + DBG("Not found.\n"); } /** @@ -290,7 +290,7 @@ static void mbl_debug(resource *r) { struct mblock *m = (struct mblock *) r; - debug("(size=%d)\n", m->size); + DBG("(size=%d)\n", m->size); } static resource * diff --git a/lib/slab.c b/lib/slab.c index e236e26..78d535f 100644 --- a/lib/slab.c +++ b/lib/slab.c @@ -116,7 +116,7 @@ slab_dump(resource *r) WALK_LIST(o, s->objs) cnt++; - debug("(%d objects per %d bytes)\n", cnt, s->size); + DBG("(%d objects per %d bytes)\n", cnt, s->size); } static size_t @@ -339,7 +339,7 @@ slab_dump(resource *r) pc++; WALK_LIST(h, s->full_heads) fc++; - debug("(%de+%dp+%df blocks per %d objs per %d bytes)\n", ec, pc, fc, s->objs_per_slab, s->obj_size); + DBG("(%de+%dp+%df blocks per %d objs per %d bytes)\n", ec, pc, fc, s->objs_per_slab, s->obj_size); } static size_t diff --git a/nest/iface.c b/nest/iface.c index b4ab70c..c7050be 100644 --- a/nest/iface.c +++ b/nest/iface.c @@ -46,7 +46,7 @@ list iface_list; void ifa_dump(struct ifa *a) { - debug("\t%I, net %I/%-2d bc %I -> %I%s%s%s\n", a->ip, a->prefix, a->pxlen, a->brd, a->opposite, + DBG("\t%I, net %I/%-2d bc %I -> %I%s%s%s\n", a->ip, a->prefix, a->pxlen, a->brd, a->opposite, (a->flags & IF_UP) ? "" : " DOWN", (a->flags & IA_PRIMARY) ? "" : " SEC", (a->flags & IA_PEER) ? "PEER" : ""); @@ -64,28 +64,28 @@ if_dump(struct iface *i) { struct ifa *a; - debug("IF%d: %s", i->index, i->name); + DBG("IF%d: %s", i->index, i->name); if (i->flags & IF_SHUTDOWN) - debug(" SHUTDOWN"); + DBG(" SHUTDOWN"); if (i->flags & IF_UP) - debug(" UP"); + DBG(" UP"); else - debug(" DOWN"); + DBG(" DOWN"); if (i->flags & IF_ADMIN_UP) - debug(" LINK-UP"); + DBG(" LINK-UP"); if (i->flags & IF_MULTIACCESS) - debug(" MA"); + DBG(" MA"); if (i->flags & IF_BROADCAST) - debug(" BC"); + DBG(" BC"); if (i->flags & IF_MULTICAST) - debug(" MC"); + DBG(" MC"); if (i->flags & IF_LOOPBACK) - debug(" LOOP"); + DBG(" LOOP"); if (i->flags & IF_IGNORE) - debug(" IGN"); + DBG(" IGN"); if (i->flags & IF_TMP_DOWN) - debug(" TDOWN"); - debug(" MTU=%d\n", i->mtu); + DBG(" TDOWN"); + DBG(" MTU=%d\n", i->mtu); WALK_LIST(a, i->addrs) { ifa_dump(a); @@ -104,10 +104,10 @@ if_dump_all(void) { struct iface *i; - debug("Known network interfaces:\n"); + DBG("Known network interfaces:\n"); WALK_LIST(i, iface_list) if_dump(i); - debug("Router ID: %08x\n", config->router_id); + DBG("Router ID: %08x\n", config->router_id); } static inline unsigned diff --git a/nest/locks.c b/nest/locks.c index 7044d6a..ec7a3d6 100644 --- a/nest/locks.c +++ b/nest/locks.c @@ -88,9 +88,9 @@ olock_dump(resource *r) struct object_lock *l = (struct object_lock *) r; static char *olock_states[] = { "free", "locked", "waiting", "event" }; - debug("(%d:%s:%I:%d) [%s]\n", l->type, (l->iface ? l->iface->name : "?"), l->addr, l->port, olock_states[l->state]); + DBG("(%d:%s:%I:%d) [%s]\n", l->type, (l->iface ? l->iface->name : "?"), l->addr, l->port, olock_states[l->state]); if (!EMPTY_LIST(l->waiters)) - debug(" [wanted]\n"); + DBG(" [wanted]\n"); } static struct resclass olock_class = { diff --git a/nest/neighbor.c b/nest/neighbor.c index 11a980b..feaee5d 100644 --- a/nest/neighbor.c +++ b/nest/neighbor.c @@ -183,15 +183,15 @@ neigh_find2(struct proto *p, ip_addr *a, struct iface *ifa, unsigned flags) void neigh_dump(neighbor *n) { - debug("%p %I ", n, n->addr); + DBG("%p %I ", n, n->addr); if (n->iface) - debug("%s ", n->iface->name); + DBG("%s ", n->iface->name); else - debug("[] "); - debug("%s %p %08x scope %s", n->proto->name, n->data, n->aux, ip_scope_text(n->scope)); + DBG("[] "); + DBG("%s %p %08x scope %s", n->proto->name, n->data, n->aux, ip_scope_text(n->scope)); if (n->flags & NEF_STICKY) - debug(" STICKY"); - debug("\n"); + DBG(" STICKY"); + DBG("\n"); } /** @@ -206,13 +206,13 @@ neigh_dump_all(void) neighbor *n; int i; - debug("Known neighbors:\n"); + DBG("Known neighbors:\n"); WALK_LIST(n, sticky_neigh_list) neigh_dump(n); for(i=0; i<NEIGH_HASH_SIZE; i++) WALK_LIST(n, neigh_hash_table[i]) neigh_dump(n); - debug("\n"); + DBG("\n"); } static void diff --git a/nest/proto.c b/nest/proto.c index 60495aa..f66449e 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -635,31 +635,31 @@ protos_dump_all(void) struct proto *p; struct announce_hook *a; - debug("Protocols:\n"); + DBG("Protocols:\n"); WALK_LIST(p, active_proto_list) { - debug(" protocol %s state %s/%s\n", p->name, + DBG(" protocol %s state %s/%s\n", p->name, p_states[p->proto_state], c_states[p->core_state]); for (a = p->ahooks; a; a = a->next) { - debug("\tTABLE %s\n", a->table->name); + DBG("\tTABLE %s\n", a->table->name); if (a->in_filter) - debug("\tInput filter: %s\n", filter_name(a->in_filter)); + DBG("\tInput filter: %s\n", filter_name(a->in_filter)); if (a->out_filter != FILTER_REJECT) - debug("\tOutput filter: %s\n", filter_name(a->out_filter)); + DBG("\tOutput filter: %s\n", filter_name(a->out_filter)); } if (p->disabled) - debug("\tDISABLED\n"); + DBG("\tDISABLED\n"); else if (p->proto->dump) p->proto->dump(p); } WALK_LIST(p, inactive_proto_list) - debug(" inactive %s: state %s/%s\n", p->name, p_states[p->proto_state], c_states[p->core_state]); + DBG(" inactive %s: state %s/%s\n", p->name, p_states[p->proto_state], c_states[p->core_state]); WALK_LIST(p, initial_proto_list) - debug(" initial %s\n", p->name); + DBG(" initial %s\n", p->name); WALK_LIST(p, flush_proto_list) - debug(" flushing %s\n", p->name); + DBG(" flushing %s\n", p->name); } /** diff --git a/nest/rt-attr.c b/nest/rt-attr.c index 6aed318..64a6cc4 100644 --- a/nest/rt-attr.c +++ b/nest/rt-attr.c @@ -560,36 +560,36 @@ ea_dump(ea_list *e) if (!e) { - debug("NONE"); + DBG("NONE"); return; } while (e) { - debug("[%c%c%c]", + DBG("[%c%c%c]", (e->flags & EALF_SORTED) ? 'S' : 's', (e->flags & EALF_BISECT) ? 'B' : 'b', (e->flags & EALF_CACHED) ? 'C' : 'c'); for(i=0; i<e->count; i++) { eattr *a = &e->attrs[i]; - debug(" %02x:%02x.%02x", EA_PROTO(a->id), EA_ID(a->id), a->flags); + DBG(" %02x:%02x.%02x", EA_PROTO(a->id), EA_ID(a->id), a->flags); if (a->type & EAF_TEMP) - debug("T"); - debug("=%c", "?iO?I?P???S?????" [a->type & EAF_TYPE_MASK]); + DBG("T"); + DBG("=%c", "?iO?I?P???S?????" [a->type & EAF_TYPE_MASK]); if (a->type & EAF_ORIGINATED) - debug("o"); + DBG("o"); if (a->type & EAF_EMBEDDED) - debug(":%08x", a->u.data); + DBG(":%08x", a->u.data); else { int j, len = a->u.ptr->length; - debug("[%d]:", len); + DBG("[%d]:", len); for(j=0; j<len; j++) - debug("%02x", a->u.ptr->data[j]); + DBG("%02x", a->u.ptr->data[j]); } } if (e = e->next) - debug(" | "); + DBG(" | "); } } @@ -826,19 +826,19 @@ rta_dump(rta *a) static char *rtc[] = { "", " BC", " MC", " AC" }; static char *rtd[] = { "", " DEV", " HOLE", " UNREACH", " PROHIBIT" }; - debug("p=%s uc=%d %s %s%s%s h=%04x", + DBG("p=%s uc=%d %s %s%s%s h=%04x", a->proto->name, a->uc, rts[a->source], ip_scope_text(a->scope), rtc[a->cast], rtd[a->dest], a->hash_key); if (!(a->aflags & RTAF_CACHED)) - debug(" !CACHED"); - debug(" <-%I", a->from); + DBG(" !CACHED"); + DBG(" <-%I", a->from); if (a->dest == RTD_ROUTER) - debug(" ->%I", a->gw); + DBG(" ->%I", a->gw); if (a->dest == RTD_DEVICE || a->dest == RTD_ROUTER) - debug(" [%s]", a->iface ? a->iface->name : "???" ); + DBG(" [%s]", a->iface ? a->iface->name : "???" ); if (a->eattrs) { - debug(" EA: "); + DBG(" EA: "); ea_dump(a->eattrs); } } @@ -855,15 +855,15 @@ rta_dump_all(void) rta *a; unsigned int h; - debug("Route attribute cache (%d entries, rehash at %d):\n", rta_cache_count, rta_cache_limit); + DBG("Route attribute cache (%d entries, rehash at %d):\n", rta_cache_count, rta_cache_limit); for(h=0; h<rta_cache_size; h++) for(a=rta_hash_table[h]; a; a=a->next) { - debug("%p ", a); + DBG("%p ", a); rta_dump(a); - debug("\n"); + DBG("\n"); } - debug("\n"); + DBG("\n"); } void diff --git a/nest/rt-fib.c b/nest/rt-fib.c index 510aa76..b6d5d06 100644 --- a/nest/rt-fib.c +++ b/nest/rt-fib.c @@ -491,21 +491,21 @@ void dump(char *m) { unsigned int i; - debug("%s ... order=%d, size=%d, entries=%d\n", m, f.hash_order, f.hash_size, f.hash_size); + DBG("%s ... order=%d, size=%d, entries=%d\n", m, f.hash_order, f.hash_size, f.hash_size); for(i=0; i<f.hash_size; i++) { struct fib_node *n; struct fib_iterator *j; for(n=f.hash_table[i]; n; n=n->next) { - debug("%04x %04x %p %I/%2d", i, ipa_hash(n->prefix), n, n->prefix, n->pxlen); + DBG("%04x %04x %p %I/%2d", i, ipa_hash(n->prefix), n, n->prefix, n->pxlen); for(j=n->readers; j; j=j->next) - debug(" %p[%p]", j, j->node); - debug("\n"); + DBG(" %p[%p]", j, j->node); + DBG("\n"); } } fib_check(&f); - debug("-----\n"); + DBG("-----\n"); } void init(struct fib_node *n) @@ -552,7 +552,7 @@ next: goto next; } c = 1; - debug("got %p\n", z); + DBG("got %p\n", z); } FIB_ITERATE_END; dump("iter end"); diff --git a/nest/rt-table.c b/nest/rt-table.c index 16dd9bc..883cfb8 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -1106,12 +1106,12 @@ void rte_dump(rte *e) { net *n = e->net; - debug("%-1I/%2d ", n->n.prefix, n->n.pxlen); - debug("KF=%02x PF=%02x pref=%d lm=%d ", n->n.flags, e->pflags, e->pref, now-e->lastmod); + DBG("%-1I/%2d ", n->n.prefix, n->n.pxlen); + DBG("KF=%02x PF=%02x pref=%d lm=%d ", n->n.flags, e->pflags, e->pref, now-e->lastmod); rta_dump(e->attrs); if (e->attrs->proto->proto->dump_attrs) e->attrs->proto->proto->dump_attrs(e); - debug("\n"); + DBG("\n"); } /** @@ -1127,7 +1127,7 @@ rt_dump(rtable *t) net *n; struct announce_hook *a; - debug("Dump of routing table <%s>\n", t->name); + DBG("Dump of routing table <%s>\n", t->name); #ifdef DEBUGGING fib_check(&t->fib); #endif @@ -1139,8 +1139,8 @@ rt_dump(rtable *t) } FIB_WALK_END; WALK_LIST(a, t->hooks) - debug("\tAnnounces routes to protocol %s\n", a->proto->name); - debug("\n"); + DBG("\tAnnounces routes to protocol %s\n", a->proto->name); + DBG("\n"); } /** diff --git a/proto/ospf/neighbor.c b/proto/ospf/neighbor.c index 26d81dc..c1f2a0b 100644 --- a/proto/ospf/neighbor.c +++ b/proto/ospf/neighbor.c @@ -619,7 +619,7 @@ static void rxmt_timer_hook(timer * timer) { struct ospf_neighbor *n = (struct ospf_neighbor *) timer->data; - // struct proto *p = &n->ifa->oa->po->proto; + struct proto *p = &n->ifa->oa->po->proto; struct top_hash_entry *en; DBG("%s: RXMT timer fired on interface %s for neigh: %I.\n", diff --git a/proto/ospf/packet.c b/proto/ospf/packet.c index 4338bc1..fb02f68 100644 --- a/proto/ospf/packet.c +++ b/proto/ospf/packet.c @@ -268,7 +268,7 @@ ospf_rx_hook(sock *sk, int size) /* Initially, the packet is associated with the 'master' iface */ struct ospf_iface *ifa = sk->data; struct proto_ospf *po = ifa->oa->po; - // struct proto *p = &po->proto; + struct proto *p = &po->proto; int src_local, dst_local UNUSED, dst_mcast; src_local = ipa_in_net(sk->faddr, ifa->addr->prefix, ifa->addr->pxlen); diff --git a/proto/rip/rip.c b/proto/rip/rip.c index 3ec070b..9f49c6b 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -512,9 +512,9 @@ rip_rx(sock *s, int size) static void rip_dump_entry( struct rip_entry *e ) { - debug( "%I told me %d/%d ago: to %I/%d go via %I, metric %d ", + DBG( "%I told me %d/%d ago: to %I/%d go via %I, metric %d ", e->whotoldme, e->updated-now, e->changed-now, e->n.prefix, e->n.pxlen, e->nexthop, e->metric ); - debug( "\n" ); + DBG( "\n" ); } /** @@ -637,16 +637,16 @@ rip_dump(struct proto *p) CHK_MAGIC; WALK_LIST( w, P->connections ) { struct rip_connection *n = (void *) w; - debug( "RIP: connection #%d: %I\n", n->num, n->addr ); + DBG( "RIP: connection #%d: %I\n", n->num, n->addr ); } i = 0; FIB_WALK( &P->rtable, e ) { - debug( "RIP: entry #%d: ", i++ ); + DBG( "RIP: entry #%d: ", i++ ); rip_dump_entry( (struct rip_entry *)e ); } FIB_WALK_END; i = 0; WALK_LIST( rif, P->interfaces ) { - debug( "RIP: interface #%d: %s, %I, busy = %x\n", i++, rif->iface?rif->iface->name:"(dummy)", rif->sock->daddr, rif->busy ); + DBG( "RIP: interface #%d: %s, %I, busy = %x\n", i++, rif->iface?rif->iface->name:"(dummy)", rif->sock->daddr, rif->busy ); } } diff --git a/proto/static/static.c b/proto/static/static.c index 9eee820..041c6a0 100644 --- a/proto/static/static.c +++ b/proto/static/static.c @@ -304,17 +304,17 @@ static_neigh_notify(struct neighbor *n) static void static_dump_rt(struct static_route *r) { - debug("%-1I/%2d: ", r->net, r->masklen); + DBG("%-1I/%2d: ", r->net, r->masklen); switch (r->dest) { case RTD_ROUTER: - debug("via %I\n", r->via); + DBG("via %I\n", r->via); break; case RTD_DEVICE: - debug("dev %s\n", r->if_name); + DBG("dev %s\n", r->if_name); break; default: - debug("rtd %d\n", r->dest); + DBG("rtd %d\n", r->dest); break; } } @@ -325,10 +325,10 @@ static_dump(struct proto *p) struct static_config *c = (void *) p->cf; struct static_route *r; - debug("Independent static routes:\n"); + DBG("Independent static routes:\n"); WALK_LIST(r, c->other_routes) static_dump_rt(r); - debug("Device static routes:\n"); + DBG("Device static routes:\n"); WALK_LIST(r, c->iface_routes) static_dump_rt(r); } diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c index fcf5dd1..6fa1cc8 100644 --- a/sysdep/unix/io.c +++ b/sysdep/unix/io.c @@ -70,7 +70,7 @@ rf_dump(resource *r) { struct rfile *a = (struct rfile *) r; - debug("(FILE *%p)\n", a->f); + DBG("(FILE *%p)\n", a->f); } static struct resclass rf_class = { @@ -190,15 +190,15 @@ tm_dump(resource *r) { timer *t = (timer *) r; - debug("(code %p, data %p, ", t->hook, t->data); + DBG("(code %p, data %p, ", t->hook, t->data); if (t->randomize) - debug("rand %d, ", t->randomize); + DBG("rand %d, ", t->randomize); if (t->recurrent) - debug("recur %d, ", t->recurrent); + DBG("recur %d, ", t->recurrent); if (t->expires) - debug("expires in %d sec)\n", t->expires - now); + DBG("expires in %d sec)\n", t->expires - now); else - debug("inactive)\n"); + DBG("inactive)\n"); } static struct resclass tm_class = { @@ -298,14 +298,14 @@ tm_dump_them(char *name, list *l) node *n; timer *t; - debug("%s timers:\n", name); + DBG("%s timers:\n", name); WALK_LIST(n, *l) { t = SKIP_BACK(timer, n, n); - debug("%p ", t); + DBG("%p ", t); tm_dump(&t->r); } - debug("\n"); + DBG("\n"); } void @@ -560,7 +560,7 @@ sk_dump(resource *r) sock *s = (sock *) r; static char *sk_type_names[] = { "TCP<", "TCP>", "TCP", "UDP", "UDP/MC", "IP", "IP/MC", "MAGIC", "UNIX<", "UNIX", "DEL!" }; - debug("(%s, ud=%p, sa=%08x, sp=%d, da=%08x, dp=%d, tos=%d, ttl=%d, if=%s)\n", + DBG("(%s, ud=%p, sa=%08x, sp=%d, da=%08x, dp=%d, tos=%d, ttl=%d, if=%s)\n", sk_type_names[s->type], s->data, s->saddr, @@ -1527,14 +1527,14 @@ sk_dump_all(void) node *n; sock *s; - debug("Open sockets:\n"); + DBG("Open sockets:\n"); WALK_LIST(n, sock_list) { s = SKIP_BACK(sock, n, n); - debug("%p ", s); + DBG("%p ", s); sk_dump(&s->r); } - debug("\n"); + DBG("\n"); } #undef ERR diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c index 6de6077..f3df5ba 100644 --- a/sysdep/unix/krt.c +++ b/sysdep/unix/krt.c @@ -551,14 +551,14 @@ krt_dump(struct proto *P) if (!KRT_CF->learn) return; - debug("KRT: Table of inheritable routes\n"); + DBG("KRT: Table of inheritable routes\n"); rt_dump(&p->krt_table); } static void krt_dump_attrs(rte *e) { - debug(" [m=%d,p=%d,t=%d]", e->u.krt.metric, e->u.krt.proto, e->u.krt.type); + DBG(" [m=%d,p=%d,t=%d]", e->u.krt.metric, e->u.krt.proto, e->u.krt.type); } #endif diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c index bd80ba2..740c3ac 100644 --- a/sysdep/unix/main.c +++ b/sysdep/unix/main.c @@ -50,7 +50,7 @@ static int debug_flag = 0; void async_dump(void) { - debug("INTERNAL STATE DUMP\n\n"); + DBG("INTERNAL STATE DUMP\n\n"); rdump(&root_pool); sk_dump_all(); @@ -61,7 +61,7 @@ async_dump(void) rt_dump_all(); protos_dump_all(); - debug("\n"); + DBG("\n"); } /* -- 1.7.10.4

Currently filter code uses extended attribyte flags (EAF_*) in P('e','s') and P('e','S'). Filter types (T_*) in extended attributes are completely ignored in filter/f-util.c f_new_dynamic_attr(). It is difficult to map transparently in interpret() EAF_* to T_* and this could cause some errors in filter expressions: currently one known issue is handing "bgp_origin" attribute, that is defined with EAF_TYPE_INT at proto/bgp/config.Y, but really its value represented with T_ENUM_BGP_ORIGIN. Thus operating on "bgp_origin" requires type int rather than enum. Introduce empty type which implements * real * void type for bgp_atomic_aggr and could be used for not implemented types as stub, rather than wasting entry in T_ENUM_range. Make hidden function unset(), which unsets extended attributes from rta to work again (remove EAF_TEMP). --- filter/config.Y | 19 +++---- filter/f-util.c | 5 +- filter/filter.c | 158 ++++++++++++++++++++++------------------------------ filter/filter.h | 28 +++++++++- proto/bgp/config.Y | 4 +- 5 files changed, 103 insertions(+), 111 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index fa87c76..a7a69a4 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -119,17 +119,12 @@ static struct f_inst * f_generate_empty(struct f_inst *dyn) { struct f_inst *e; - u16 aux; + u8 f_type = f_inst_aux_f_type(dyn->aux); - switch (dyn->aux & EAF_TYPE_MASK) { - case EAF_TYPE_AS_PATH: - aux = T_PATH; - break; - case EAF_TYPE_INT_SET: - aux = T_CLIST; - break; - case EAF_TYPE_EC_SET: - aux = T_ECLIST; + switch (f_type) { + case T_PATH: + case T_CLIST: + case T_ECLIST: break; default: cf_error("Can't empty that attribute"); @@ -137,7 +132,7 @@ f_generate_empty(struct f_inst *dyn) e = f_new_inst(); e->code = 'E'; - e->aux = aux; + e->aux = f_type; dyn->code = P('e','S'); dyn->a1.p = e; @@ -870,7 +865,7 @@ cmd: } | UNSET '(' rtadot dynamic_attr ')' ';' { $$ = $4; - $$->aux = EAF_TYPE_UNDEF | EAF_TEMP; + $$->aux = f_inst_aux(EAF_TYPE_UNDEF, T_VOID); $$->code = P('e','S'); $$->a1.p = NULL; } diff --git a/filter/f-util.c b/filter/f-util.c index def2b24..ad91002 100644 --- a/filter/f-util.c +++ b/filter/f-util.c @@ -24,11 +24,10 @@ f_new_inst(void) } struct f_inst * -f_new_dynamic_attr(int type, int f_type UNUSED, int code) +f_new_dynamic_attr(int type, int f_type, int code) { - /* FIXME: Remove the f_type parameter? */ struct f_inst *f = f_new_inst(); - f->aux = type; + f->aux = f_inst_aux(type, f_type); f->a2.i = code; return f; } diff --git a/filter/filter.c b/filter/filter.c index 69cf579..9b99f28 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -178,6 +178,7 @@ val_compare(struct f_val v1, struct f_val v2) return CMP_ERROR; } switch (v1.type) { + case T_EMPTY: case T_ENUM: case T_INT: case T_BOOL: @@ -480,6 +481,7 @@ val_print(struct f_val v) char buf2[1024]; switch (v.type) { case T_VOID: logn("(void)"); return; + case T_EMPTY: logn("(empty)"); return; case T_BOOL: logn(v.val.i ? "TRUE" : "FALSE"); return; case T_INT: logn("%d", v.val.i); return; case T_STRING: logn("%s", v.val.s); return; @@ -902,6 +904,9 @@ interpret(struct f_inst *what) ACCESS_RTE; { eattr *e = NULL; + + res.type = f_inst_aux_f_type(what->aux); + if (!(f_flags & FF_FORCE_TMPATTR)) e = ea_find( (*f_rte)->attrs->eattrs, what->a2.i ); if (!e) @@ -910,59 +915,38 @@ interpret(struct f_inst *what) e = ea_find( (*f_rte)->attrs->eattrs, what->a2.i ); if (!e) { - /* A special case: undefined int_set looks like empty int_set */ - if ((what->aux & EAF_TYPE_MASK) == EAF_TYPE_INT_SET) { - res.type = T_CLIST; + /* A special case: undefined int_set looks like empty int_set. + * The same special case applied also to ec_set. + */ + if ((res.type == T_CLIST) || (res.type == T_ECLIST)) res.val.ad = adata_empty(f_pool, 0); - break; - } - /* The same special case for ec_set */ - else if ((what->aux & EAF_TYPE_MASK) == EAF_TYPE_EC_SET) { - res.type = T_ECLIST; - res.val.ad = adata_empty(f_pool, 0); - break; - } - - /* Undefined value */ - res.type = T_VOID; + else + res.type = T_VOID; /* Undefined value */ break; } - switch (what->aux & EAF_TYPE_MASK) { - case EAF_TYPE_INT: - res.type = T_INT; - res.val.i = e->u.data; - break; - case EAF_TYPE_ROUTER_ID: - res.type = T_QUAD; + switch (res.type) { + case T_INT: + case T_QUAD: + case T_ENUM_BGP_ORIGIN: res.val.i = e->u.data; break; - case EAF_TYPE_OPAQUE: - res.type = T_ENUM_EMPTY; - res.val.i = 0; - break; - case EAF_TYPE_IP_ADDRESS: - res.type = T_IP; - struct adata * ad = e->u.ptr; - res.val.px.ip = * (ip_addr *) ad->data; - break; - case EAF_TYPE_AS_PATH: - res.type = T_PATH; - res.val.ad = e->u.ptr; - break; - case EAF_TYPE_INT_SET: - res.type = T_CLIST; - res.val.ad = e->u.ptr; + case T_IP: + { + struct adata * ad = e->u.ptr; + res.val.px.ip = * (ip_addr *) ad->data; + } break; - case EAF_TYPE_EC_SET: - res.type = T_ECLIST; + case T_PATH: + case T_CLIST: + case T_ECLIST: res.val.ad = e->u.ptr; break; - case EAF_TYPE_UNDEF: - res.type = T_VOID; + case T_EMPTY: + res.val.i = 0; break; default: - bug("Unknown type in e,a"); + bug( "Unknown type in 'e,a': %x", res.type ); } } break; @@ -970,71 +954,63 @@ interpret(struct f_inst *what) ACCESS_RTE; ONEARG; { - struct ea_list *l = lp_alloc(f_pool, sizeof(struct ea_list) + sizeof(eattr)); + struct ea_list *l; + u8 eaf = f_inst_aux_eaf(what->aux); + u8 f_type = f_inst_aux_f_type(what->aux); + + if (f_type != v1.type) { + if (f_type != T_QUAD) + runtime( "Attempt to set attribute to incompatible type" ); +#ifndef IPV6 + /* IP->Quad implicit conversion */ + if (v1.type == T_IP) + v1.val.i = ipa_to_u32(v1.val.px.ip); + else +#endif + /* T_INT for backward compatibility */ + if (v1.type != T_INT) + runtime( "Setting quad attribute to non-quad value" ); + } else { + if (f_type == T_EMPTY) + runtime( "Attempt to set empty attribute" ); + } + + l = lp_alloc(f_pool, sizeof(struct ea_list) + sizeof(eattr)); l->next = NULL; l->flags = EALF_SORTED; l->count = 1; l->attrs[0].id = what->a2.i; l->attrs[0].flags = 0; - l->attrs[0].type = what->aux | EAF_ORIGINATED; - switch (what->aux & EAF_TYPE_MASK) { - case EAF_TYPE_INT: - if (v1.type != T_INT) - runtime( "Setting int attribute to non-int value" ); - l->attrs[0].u.data = v1.val.i; - break; + l->attrs[0].type = (eaf | EAF_ORIGINATED); - case EAF_TYPE_ROUTER_ID: -#ifndef IPV6 - /* IP->Quad implicit conversion */ - if (v1.type == T_IP) { - l->attrs[0].u.data = ipa_to_u32(v1.val.px.ip); - break; - } -#endif - /* T_INT for backward compatibility */ - if ((v1.type != T_QUAD) && (v1.type != T_INT)) - runtime( "Setting quad attribute to non-quad value" ); + switch (f_type) { + case T_INT: + case T_QUAD: + case T_ENUM_BGP_ORIGIN: l->attrs[0].u.data = v1.val.i; break; - - case EAF_TYPE_OPAQUE: - runtime( "Setting opaque attribute is not allowed" ); - break; - case EAF_TYPE_IP_ADDRESS: - if (v1.type != T_IP) - runtime( "Setting ip attribute to non-ip value" ); - int len = sizeof(ip_addr); - struct adata *ad = lp_alloc(f_pool, sizeof(struct adata) + len); - ad->length = len; - (* (ip_addr *) ad->data) = v1.val.px.ip; - l->attrs[0].u.ptr = ad; - break; - case EAF_TYPE_AS_PATH: - if (v1.type != T_PATH) - runtime( "Setting path attribute to non-path value" ); - l->attrs[0].u.ptr = v1.val.ad; - break; - case EAF_TYPE_INT_SET: - if (v1.type != T_CLIST) - runtime( "Setting clist attribute to non-clist value" ); - l->attrs[0].u.ptr = v1.val.ad; + case T_IP: + { + struct adata *ad = lp_alloc(f_pool, sizeof(struct adata) + sizeof(ip_addr)); + ad->length = sizeof(ip_addr); + (* (ip_addr *) ad->data) = v1.val.px.ip; + l->attrs[0].u.ptr = ad; + } break; - case EAF_TYPE_EC_SET: - if (v1.type != T_ECLIST) - runtime( "Setting eclist attribute to non-eclist value" ); + case T_PATH: + case T_CLIST: + case T_ECLIST: l->attrs[0].u.ptr = v1.val.ad; break; - case EAF_TYPE_UNDEF: - if (v1.type != T_VOID) - runtime( "Setting void attribute to non-void value" ); + case T_VOID: l->attrs[0].u.data = 0; break; - default: bug("Unknown type in e,S"); + default: + bug( "Unknown type in 'e,S': %x", f_type ); } - if (!(what->aux & EAF_TEMP) && (!(f_flags & FF_FORCE_TMPATTR))) { + if (!(eaf & EAF_TEMP) && !(f_flags & FF_FORCE_TMPATTR)) { f_rta_cow(); l->next = (*f_rte)->attrs->eattrs; (*f_rte)->attrs->eattrs = l; diff --git a/filter/filter.h b/filter/filter.h index 9e60ef8..aed4418 100644 --- a/filter/filter.h +++ b/filter/filter.h @@ -144,8 +144,8 @@ void val_print(struct f_val v); #define T_MASK 0xff /* Internal types */ -/* Do not use type of zero, that way we'll see errors easier. */ -#define T_VOID 1 +#define T_VOID 1 /* Do not use type of zero, that way we'll see errors easier. */ +#define T_EMPTY 2 /* Special hack for atomic_aggr */ /* User visible types, which fit in int */ #define T_INT 0x10 @@ -164,7 +164,6 @@ void val_print(struct f_val v); #define T_ENUM_RTD 0x34 #define T_ENUM_ROA 0x35 /* new enums go here */ -#define T_ENUM_EMPTY 0x3f /* Special hack for atomic_aggr */ #define T_ENUM T_ENUM_LO ... T_ENUM_HI @@ -195,6 +194,29 @@ void val_print(struct f_val v); #define SA_IFINDEX 10 +/* Encode EAF_* and T_* in 'struct f_inst' aux field, + * as filters operate mostly on its native internal types T_*, + * but sometimes need access to EAF_* (for example EAF_TEMP). + */ + +static inline u16 +f_inst_aux(u8 eaf, u8 f_type) +{ + return (((u16) eaf) << 8) | (f_type & T_MASK); +} + +static inline int +f_inst_aux_eaf(u16 aux) +{ + return (aux >> 8); +} + +static inline int +f_inst_aux_f_type(u16 aux) +{ + return ((aux) & T_MASK); +} + struct f_tree { struct f_tree *left, *right; struct f_val from, to; diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y index d5e5aac..19f7a21 100644 --- a/proto/bgp/config.Y +++ b/proto/bgp/config.Y @@ -123,9 +123,9 @@ CF_ADDTO(dynamic_attr, BGP_MED CF_ADDTO(dynamic_attr, BGP_LOCAL_PREF { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(EAP_BGP, BA_LOCAL_PREF)); }) CF_ADDTO(dynamic_attr, BGP_ATOMIC_AGGR - { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(EAP_BGP, BA_ATOMIC_AGGR)); }) + { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_EMPTY, EA_CODE(EAP_BGP, BA_ATOMIC_AGGR)); }) CF_ADDTO(dynamic_attr, BGP_AGGREGATOR - { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(EAP_BGP, BA_AGGREGATOR)); }) + { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_EMPTY, EA_CODE(EAP_BGP, BA_AGGREGATOR)); }) CF_ADDTO(dynamic_attr, BGP_COMMUNITY { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(EAP_BGP, BA_COMMUNITY)); }) CF_ADDTO(dynamic_attr, BGP_ORIGINATOR_ID -- 1.7.10.4