]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
Nest: Fix several issues with pflags
authorOndrej Zajicek <santiago@crfreenet.org>
Sun, 1 Jan 2023 19:10:23 +0000 (20:10 +0100)
committerOndrej Zajicek <santiago@crfreenet.org>
Sun, 1 Jan 2023 19:10:23 +0000 (20:10 +0100)
There were some confusion about validity and usage of pflags, which
caused incorrect usage after some flags from (now removed) protocol-
specific area were moved to pflags.

We state that pflags:

 - Are secondary data used by protocol-specific hooks
 - Can be changed on an existing route (in contrast to copy-on-write
   for primary data)
 - Are irrelevant for propagation (not propagated when changed)
 - Are specific to a routing table (not propagated by pipe)

The patch did these fixes:

 - Do not compare pflags in rte_same(), as they may keep cached values
   like BGP_REF_STALE, causing spurious propagation.

 - Initialize pflags to zero in rte_get_temp(), avoid initialization in
   protocol code, fixing at least two forgotten initializations (krt
   and one case in babel).

 - Improve documentation about pflags

nest/rt-dev.c
nest/rt-table.c
proto/babel/babel.c
proto/bgp/packets.c
proto/perf/perf.c
proto/pipe/pipe.c
proto/rpki/rpki.c
proto/static/static.c

index 05e64fc39bec82b16fa295dc98fbc549e3a9fe60..7932b8b7d039cc5ea3a8d7f9aaf834231b8bfdce 100644 (file)
@@ -92,7 +92,6 @@ dev_ifa_notify(struct proto *P, uint flags, struct ifa *ad)
 
       a = rta_lookup(&a0);
       e = rte_get_temp(a, src);
-      e->pflags = 0;
       rte_update2(c, net, e, src);
     }
 }
index 845b2483d351602e9cb7842c8d1cbb1bb3bfc825..e4b2781481cced2f4886e157662a323e99b32928 100644 (file)
  * on the list being the best one (i.e., the one we currently use
  * for routing), the order of the other ones is undetermined.
  *
- * The &rte contains information specific to the route (preference, protocol
- * metrics, time of last modification etc.) and a pointer to a &rta structure
- * (see the route attribute module for a precise explanation) holding the
- * remaining route attributes which are expected to be shared by multiple
- * routes in order to conserve memory.
+ * The &rte contains information about the route. There are net and src, which
+ * together forms a key identifying the route in a routing table. There is a
+ * pointer to a &rta structure (see the route attribute module for a precise
+ * explanation) holding the route attributes, which are primary data about the
+ * route. There are several technical fields used by routing table code (route
+ * id, REF_* flags), There is also the pflags field, holding protocol-specific
+ * flags. They are not used by routing table code, but by protocol-specific
+ * hooks. In contrast to route attributes, they are not primary data and their
+ * validity is also limited to the routing table.
  *
  * There are several mechanisms that allow automatic update of routes in one
  * routing table (dst) as a result of changes in another routing table (src).
@@ -558,10 +562,9 @@ rte_find(net *net, struct rte_src *src)
  * rte_get_temp - get a temporary &rte
  * @a: attributes to assign to the new route (a &rta; in case it's
  * un-cached, rte_update() will create a cached copy automatically)
+ * @src: route source
  *
  * Create a temporary &rte and bind it with the attributes @a.
- * Also set route preference to the default preference set for
- * the protocol.
  */
 rte *
 rte_get_temp(rta *a, struct rte_src *src)
@@ -571,6 +574,7 @@ rte_get_temp(rta *a, struct rte_src *src)
   e->attrs = a;
   e->id = 0;
   e->flags = 0;
+  e->pflags = 0;
   rt_lock_source(e->src = src);
   return e;
 }
@@ -1205,10 +1209,9 @@ rte_free_quick(rte *e)
 static int
 rte_same(rte *x, rte *y)
 {
-  /* rte.flags are not checked, as they are mostly internal to rtable */
+  /* rte.flags / rte.pflags are not checked, as they are internal to rtable */
   return
     x->attrs == y->attrs &&
-    x->pflags == y->pflags &&
     x->src == y->src &&
     rte_is_filtered(x) == rte_is_filtered(y);
 }
index 23cb67744c65fbf778076873c66cf0f148e1c6e5..86cec63b8a597b9f1d3dbe4e82d90e9b49243f6e 100644 (file)
@@ -715,7 +715,6 @@ babel_announce_rte(struct babel_proto *p, struct babel_entry *e)
 
     rta *a = rta_lookup(&a0);
     rte *rte = rte_get_temp(a, p->p.main_source);
-    rte->pflags = 0;
 
     e->unreachable = 1;
     rte_update2(c, e->n.addr, rte, p->p.main_source);
index 7ce2fd636db564f48fc869de905a4ed29aee9815..ddbc9226d9fecd495f26a51ddb03223bd63e09c9 100644 (file)
@@ -1464,7 +1464,6 @@ bgp_rte_update(struct bgp_parse_state *s, const net_addr *n, u32 path_id, rta *a
   rta *a = rta_clone(s->cached_rta);
   rte *e = rte_get_temp(a, s->last_src);
 
-  e->pflags = 0;
   rte_update3(&s->channel->c, n, e, s->last_src);
 }
 
index 5d228045e7275bf520883ea0751ce5c78456283a..75e405f0f88c2c0c16c26182491784f362835228 100644 (file)
@@ -162,7 +162,6 @@ perf_loop(void *data)
 
   for (uint i=0; i<N; i++) {
     rte *e = rte_get_temp(p->data[i].a, p->p.main_source);
-    e->pflags = 0;
 
     rte_update(P, &(p->data[i].net), e);
   }
index 1f1ad85744199fd4ca0fa535a7f239e20de85de6..a4c5b1d4ff996271da97e5bc6a497fd90da719ba 100644 (file)
@@ -77,13 +77,6 @@ pipe_rt_notify(struct proto *P, struct channel *src_ch, net *n, rte *new, rte *o
       a->cached = 0;
       a->hostentry = NULL;
       e = rte_get_temp(a, src);
-      e->pflags = new->pflags;
-
-#ifdef CONFIG_BGP
-      /* Hack to cleanup cached value */
-      if (e->src->proto->proto == &proto_bgp)
-       e->pflags &= ~(BGP_REF_STALE | BGP_REF_NOT_STALE);
-#endif
     }
   else
     {
index 3c27aa1a5fd1af7a48455c346d3cf4df40f83666..3e321627c23619c0de446e7e6eae11914053f7b3 100644 (file)
@@ -130,8 +130,6 @@ rpki_table_add_roa(struct rpki_cache *cache, struct channel *channel, const net_
   rta *a = rta_lookup(&a0);
   rte *e = rte_get_temp(a, p->p.main_source);
 
-  e->pflags = 0;
-
   rte_update2(channel, &pfxr->n, e, e->src);
 }
 
index cd31afd3aff9e68060c7ce4002b95e74e768031b..bb93305ed2fcc89d2d5ac8be316522be28b9d516 100644 (file)
@@ -104,7 +104,6 @@ static_announce_rte(struct static_proto *p, struct static_route *r)
 
   /* We skip rta_lookup() here */
   rte *e = rte_get_temp(a, src);
-  e->pflags = 0;
 
   if (r->cmds)
   {