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