BGP: Maintain valid route attribute flags even in local tables

BGP route attributes have flags (Optional, Transitive) that are validated
on decode and set to valid value on export. But if such attribute is
modified by filter or set internally by BGP during import, then its flags
would be zero in local tables. That usually does not matter, as they are
not used locally and they were fixed on export, but invalid flags leaked
in BMP and MRT dumps.

Keep route attribute flags set to valid values even when set by filters
or modified by BGP.

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 4346cd5d484ca7574afb873792db28e8e297c06e..85646647f4b98a183e798dec548b75d8f790a4be 100644 (file)
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -89,7 +89,7 @@ bgp_set_attr(ea_list **attrs, struct linpool *pool, uint code, uint flags, uintp
       attrs,
       pool,
       EA_CODE(PROTOCOL_BGP, code),
-      flags & ~BAF_EXT_LEN,
+      bgp_attr_table[code].flags | (flags & BAF_PARTIAL),
       bgp_attr_table[code].type,
       val
   );
@@ -1158,13 +1158,6 @@ bgp_attr_name(uint code)
   return (code < ARRAY_SIZE(bgp_attr_table)) ? bgp_attr_table[code].name : NULL;
 }
 
-void bgp_fix_attr_flags(ea_list *attrs)
-{
-  for (u8 i = 0; i < attrs->count; i++)
-  {
-    attrs->attrs[i].flags = bgp_attr_table[EA_ID(attrs->attrs[i].id)].flags;
-  }
-}
 
 /*
  *     Attribute export
@@ -1182,7 +1175,8 @@ bgp_export_attr(struct bgp_export_state *s, eattr *a, ea_list *to)
   {
     const struct bgp_attr_desc *desc = &bgp_attr_table[code];
 
-    /* The flags might have been zero if the attr was added by filters */
+    /* The flags should be correct, we reset them just to be sure */
+    ASSERT(!((a->flags ^ desc->flags) & (BAF_OPTIONAL | BAF_TRANSITIVE)));
     a->flags = (a->flags & BAF_PARTIAL) | desc->flags;
 
     /* Set partial bit if new opt-trans attribute is attached to non-local route */

diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y
index 1173ff060079c2c2ebd338d11634c5a36b5b4719..4ce06488615a12a1d310d4736c701ebe495df939 100644 (file)
--- a/proto/bgp/config.Y
+++ b/proto/bgp/config.Y
@@ -349,33 +349,33 @@ bgp_proto_channel: bgp_channel_start bgp_channel_opt_list bgp_channel_end;
 
 
 dynamic_attr: BGP_ORIGIN
-       { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_ENUM_BGP_ORIGIN, EA_CODE(PROTOCOL_BGP, BA_ORIGIN)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_ENUM_BGP_ORIGIN, EA_CODE(PROTOCOL_BGP, BA_ORIGIN));   $$.flags = BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_PATH
-       { $$ = f_new_dynamic_attr(EAF_TYPE_AS_PATH, T_PATH, EA_CODE(PROTOCOL_BGP, BA_AS_PATH)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_AS_PATH, T_PATH, EA_CODE(PROTOCOL_BGP, BA_AS_PATH));         $$.flags = BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_NEXT_HOP
-       { $$ = f_new_dynamic_attr(EAF_TYPE_IP_ADDRESS, T_IP, EA_CODE(PROTOCOL_BGP, BA_NEXT_HOP)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_IP_ADDRESS, T_IP, EA_CODE(PROTOCOL_BGP, BA_NEXT_HOP));       $$.flags = BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_MED
-       { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_MULTI_EXIT_DISC)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_MULTI_EXIT_DISC));      $$.flags = BAF_OPTIONAL; } ;
 dynamic_attr: BGP_LOCAL_PREF
-       { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_LOCAL_PREF)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_LOCAL_PREF));           $$.flags = BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_ATOMIC_AGGR
-       { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_ATOMIC_AGGR)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_ATOMIC_AGGR)); $$.flags = BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_AGGREGATOR
-       { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_AGGREGATOR)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_AGGREGATOR)); $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_COMMUNITY
-       { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(PROTOCOL_BGP, BA_COMMUNITY)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(PROTOCOL_BGP, BA_COMMUNITY));      $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_ORIGINATOR_ID
-       { $$ = f_new_dynamic_attr(EAF_TYPE_ROUTER_ID, T_QUAD, EA_CODE(PROTOCOL_BGP, BA_ORIGINATOR_ID)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_ROUTER_ID, T_QUAD, EA_CODE(PROTOCOL_BGP, BA_ORIGINATOR_ID)); $$.flags = BAF_OPTIONAL; } ;
 dynamic_attr: BGP_CLUSTER_LIST
-       { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(PROTOCOL_BGP, BA_CLUSTER_LIST)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(PROTOCOL_BGP, BA_CLUSTER_LIST));   $$.flags = BAF_OPTIONAL; } ;
 dynamic_attr: BGP_EXT_COMMUNITY
-       { $$ = f_new_dynamic_attr(EAF_TYPE_EC_SET, T_ECLIST, EA_CODE(PROTOCOL_BGP, BA_EXT_COMMUNITY)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_EC_SET, T_ECLIST, EA_CODE(PROTOCOL_BGP, BA_EXT_COMMUNITY));  $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_AIGP
-       { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_AIGP)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_AIGP));       $$.flags = BAF_OPTIONAL; } ;
 dynamic_attr: BGP_LARGE_COMMUNITY
-       { $$ = f_new_dynamic_attr(EAF_TYPE_LC_SET, T_LCLIST, EA_CODE(PROTOCOL_BGP, BA_LARGE_COMMUNITY)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_LC_SET, T_LCLIST, EA_CODE(PROTOCOL_BGP, BA_LARGE_COMMUNITY)); $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ;
 dynamic_attr: BGP_OTC
-       { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_ONLY_TO_CUSTOMER)); } ;
+       { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_ONLY_TO_CUSTOMER));     $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ;
 
 custom_attr: ATTRIBUTE BGP expr type symbol ';' {
   if ($3 > 255 || $3 < 1)