]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
Using ea_lookup_tmp() for temporarily keeping attribute references
authorMaria Matejka <mq@ucw.cz>
Tue, 23 Apr 2024 16:28:34 +0000 (18:28 +0200)
committerMaria Matejka <mq@ucw.cz>
Sat, 25 May 2024 17:37:16 +0000 (19:37 +0200)
To avoid needs for keeping local temporary references for attributes,
now one can use ea_lookup_tmp() to ensure that the attributes are
valid and stored until the task ends. After that, the attributes are
automatically unref'd and also deallocated if needed.

lib/route.h
nest/rt-attr.c
nest/rt-table.c
proto/bgp/bgp.h
proto/bgp/packets.c

index 647219ce5a9d17fc89ec5aeb46ad9d174acfeda8..cc9f540bea79219d05cc52d8d98ac68c2a020bb2 100644 (file)
@@ -557,6 +557,29 @@ static inline ea_list *ea_lookup(ea_list *r, u32 squash_upto, enum ea_stored oid
     return ea_lookup_slow(r, squash_upto, oid);
 }
 
+struct ea_free_deferred {
+  struct deferred_call dc;
+  ea_list *attrs;
+};
+
+void ea_free_deferred(struct deferred_call *dc);
+
+static inline ea_list *ea_free_later(ea_list *r)
+{
+  struct ea_free_deferred efd = {
+    .dc.hook = ea_free_deferred,
+    .attrs = r,
+  };
+
+  defer_call(&efd.dc, sizeof efd);
+  return r;
+}
+
+static inline ea_list *ea_lookup_tmp(ea_list *r, u32 squash_upto, enum ea_stored oid)
+{
+  return ea_free_later(ea_lookup(r, squash_upto, oid));
+}
+
 static inline ea_list *ea_strip_to(ea_list *r, u32 strip_to)
 {
   ASSERT_DIE(strip_to);
index ff895bcaf8c5f7d38bef3dd2855df36905cf0e31..1ed849e42958100f6d27bbdb7af900dbd4228a5d 100644 (file)
@@ -1770,6 +1770,12 @@ ea__free(struct ea_storage *a)
   RTA_UNLOCK;
 }
 
+void
+ea_free_deferred(struct deferred_call *dc)
+{
+  ea_free(SKIP_BACK(struct ea_free_deferred, dc, dc)->attrs);
+}
+
 /**
  * rta_dump_all - dump attribute cache
  *
index bd93e71b92f7db4eaad92509e16045daab11d656..b40d2fd6a5bac7a92e1e10acd4b2bf675416a8c7 100644 (file)
@@ -1861,11 +1861,9 @@ rte_update(struct channel *c, const net_addr *n, rte *new, struct rte_src *src)
 
   ASSERT(c->channel_state == CS_UP);
 
-  ea_list *ea_prefilter = NULL, *ea_postfilter = NULL;
-
   /* Storing prefilter routes as an explicit layer */
   if (new && (c->in_keep & RIK_PREFILTER))
-    ea_prefilter = new->attrs = ea_lookup(new->attrs, 0, EALS_PREIMPORT);
+    new->attrs = ea_lookup_tmp(new->attrs, 0, EALS_PREIMPORT);
 
 #if 0
   debug("%s.%s -(prefilter)-> %s: %N ", c->proto->name, c->name, c->table->name, n);
@@ -1908,8 +1906,8 @@ rte_update(struct channel *c, const net_addr *n, rte *new, struct rte_src *src)
 
       if (new)
       {
-       ea_postfilter = new->attrs = ea_lookup(new->attrs,
-           ea_prefilter ? BIT32_ALL(EALS_PREIMPORT) : 0, EALS_FILTERED);
+       new->attrs = ea_lookup_tmp(new->attrs,
+           (c->in_keep & RIK_PREFILTER) ? BIT32_ALL(EALS_PREIMPORT) : 0, EALS_FILTERED);
 
        if (net_is_flow(n))
          rt_flowspec_resolve_rte(new, c);
@@ -1934,11 +1932,6 @@ rte_update(struct channel *c, const net_addr *n, rte *new, struct rte_src *src)
     mpls_unlock_fec(fec);
     DBGL( "Unlock FEC %p (rte_update %N)", fec, n);
   }
-
-  /* Now the route attributes are kept by the in-table cached version
-   * and we may drop the local handles */
-  ea_free(ea_prefilter);
-  ea_free(ea_postfilter);
 }
 
 void
index 29807f9b3af352f03ce61938180ad4fb50dd38b0..3d234fa276b8ed2d9804bd65b53cfd7c09c2ed88 100644 (file)
@@ -519,7 +519,6 @@ struct bgp_parse_state {
   /* Cached state for bgp_rte_update() */
   u32 last_id;
   struct rte_src *last_src;
-  ea_list *cached_ea;
 };
 
 #define BGP_PORT               179
index 7c16fd7b166dfbe27a659b0dd6b3f97df73f57d7..f57d7b53d5263abc182e48058b5b636995824c8e 100644 (file)
@@ -1533,10 +1533,6 @@ bgp_rte_update(struct bgp_parse_state *s, const net_addr *n, u32 path_id, ea_lis
     return;
   }
 
-  /* Prepare cached route attributes */
-  if (!s->mpls && (s->cached_ea == NULL))
-    a0 = s->cached_ea = ea_lookup(a0, 0, EALS_CUSTOM);
-
   rte e0 = {
     .attrs = a0,
     .src = s->last_src,
@@ -1593,10 +1589,6 @@ bgp_decode_mpls_labels(struct bgp_parse_state *s, byte **pos, uint *len, uint *p
   /* Update next hop entry in rta */
   bgp_apply_mpls_labels(s, to, lnum, labels);
 
-  /* Attributes were changed, invalidate cached entry */
-  ea_free(s->cached_ea);
-  s->cached_ea = NULL;
-
   return;
 }
 
@@ -1642,6 +1634,7 @@ bgp_decode_nlri_ip4(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 {
   while (len)
   {
+    ea_list *ea = a;
     net_addr_ip4 net;
     u32 path_id = 0;
 
@@ -1664,7 +1657,7 @@ bgp_decode_nlri_ip4(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 
     /* Decode MPLS labels */
     if (s->mpls)
-      bgp_decode_mpls_labels(s, &pos, &len, &l, &a);
+      bgp_decode_mpls_labels(s, &pos, &len, &l, &ea);
 
     if (l > IP4_MAX_PREFIX_LENGTH)
       bgp_parse_error(s, 10);
@@ -1680,7 +1673,7 @@ bgp_decode_nlri_ip4(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 
     // XXXX validate prefix
 
-    bgp_rte_update(s, (net_addr *) &net, path_id, a);
+    bgp_rte_update(s, (net_addr *) &net, path_id, ea);
   }
 }
 
@@ -1727,6 +1720,7 @@ bgp_decode_nlri_ip6(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 {
   while (len)
   {
+    ea_list *ea = a;
     net_addr_ip6 net;
     u32 path_id = 0;
 
@@ -1749,7 +1743,7 @@ bgp_decode_nlri_ip6(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 
     /* Decode MPLS labels */
     if (s->mpls)
-      bgp_decode_mpls_labels(s, &pos, &len, &l, &a);
+      bgp_decode_mpls_labels(s, &pos, &len, &l, &ea);
 
     if (l > IP6_MAX_PREFIX_LENGTH)
       bgp_parse_error(s, 10);
@@ -1765,7 +1759,7 @@ bgp_decode_nlri_ip6(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 
     // XXXX validate prefix
 
-    bgp_rte_update(s, (net_addr *) &net, path_id, a);
+    bgp_rte_update(s, (net_addr *) &net, path_id, ea);
   }
 }
 
@@ -1815,6 +1809,7 @@ bgp_decode_nlri_vpn4(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 {
   while (len)
   {
+    ea_list *ea = a;
     net_addr_vpn4 net;
     u32 path_id = 0;
 
@@ -1837,7 +1832,7 @@ bgp_decode_nlri_vpn4(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 
     /* Decode MPLS labels */
     if (s->mpls)
-      bgp_decode_mpls_labels(s, &pos, &len, &l, &a);
+      bgp_decode_mpls_labels(s, &pos, &len, &l, &ea);
 
     /* Decode route distinguisher */
     if (l < 64)
@@ -1861,7 +1856,7 @@ bgp_decode_nlri_vpn4(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 
     // XXXX validate prefix
 
-    bgp_rte_update(s, (net_addr *) &net, path_id, a);
+    bgp_rte_update(s, (net_addr *) &net, path_id, ea);
   }
 }
 
@@ -1912,6 +1907,7 @@ bgp_decode_nlri_vpn6(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 {
   while (len)
   {
+    ea_list *ea = a;
     net_addr_vpn6 net;
     u32 path_id = 0;
 
@@ -1934,7 +1930,7 @@ bgp_decode_nlri_vpn6(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 
     /* Decode MPLS labels */
     if (s->mpls)
-      bgp_decode_mpls_labels(s, &pos, &len, &l, &a);
+      bgp_decode_mpls_labels(s, &pos, &len, &l, &ea);
 
     /* Decode route distinguisher */
     if (l < 64)
@@ -1958,7 +1954,7 @@ bgp_decode_nlri_vpn6(struct bgp_parse_state *s, byte *pos, uint len, ea_list *a)
 
     // XXXX validate prefix
 
-    bgp_rte_update(s, (net_addr *) &net, path_id, a);
+    bgp_rte_update(s, (net_addr *) &net, path_id, ea);
   }
 }
 
@@ -2707,13 +2703,13 @@ bgp_decode_nlri(struct bgp_parse_state *s, u32 afi, byte *nlri, uint len, ea_lis
     /* Handle withdraw during next hop decoding */
     if (s->err_withdraw)
       ea = NULL;
+
+    if (ea)
+      ea = ea_lookup_tmp(ea, 0, EALS_CUSTOM);
   }
 
   c->desc->decode_nlri(s, nlri, len, ea);
 
-  ea_free(s->cached_ea);
-  s->cached_ea = NULL;
-
   rt_unlock_source(s->last_src);
 }
 
@@ -2820,7 +2816,6 @@ bgp_rx_update(struct bgp_conn *conn, byte *pkt, uint len)
                    ea, s.mp_next_hop_data, s.mp_next_hop_len);
 
 done:
-  ea_free(s.cached_ea);
   lp_restore(tmp_linpool, tmpp);
   return;
 }