]> 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>
Fri, 17 May 2024 07:12:52 +0000 (09:12 +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 d965ed2ab0935d5ac69e1d49af5c6fd49fe80e8b..e55714f5c141e3d19ef638eb4d8dd18551ffe540 100644 (file)
@@ -556,6 +556,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 ccfa34f9c79121174c95a4a9beae22a2a7ea3f7b..84ceb16534eefe5aeddf3b93899f7e6e5a8a60c7 100644 (file)
@@ -1762,6 +1762,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 1915113507a72eed177b3d2c14e29722ab59f667..59c863f9f34d2df2f5081a1102964dfd774098b3 100644 (file)
@@ -1871,11 +1871,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);
@@ -1918,8 +1916,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);
@@ -1944,11 +1942,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 39b61f9d840a12b5d7248cf8388a70e225cbeb9b..913d8cfbf9ab11968aef0b5eecdf8c70b4fa49e7 100644 (file)
@@ -520,7 +520,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 e63d47b0ee369811af8330f51b11f37b45a5e63c..14d996970fb6cd90df091497ed8038fdef9f7f00 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,12 @@ 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;
+
+    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 +2815,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;
 }