[PATCH 11/12] Use filter types with extended attributes.

Sergey Popovich popovich_sergei at mail.ru
Mon Sep 30 20:34:51 CEST 2013


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



More information about the Bird-users mailing list