lib: add some abstraction guards for table code

route_node->parent and route_node->link shouldn't be touched by user
code since that is a recipe for trouble once we have a hash table in
parallel.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/bgpd/rfapi/rfapi_import.c b/bgpd/rfapi/rfapi_import.c
index b0c6db2a1eee08e40a5f4b5b204eafc2f49f462b..b6c0e30295f56d86d764cfeac8cddc59a17f6d14 100644 (file)
--- a/bgpd/rfapi/rfapi_import.c
+++ b/bgpd/rfapi/rfapi_import.c
@@ -1783,6 +1783,8 @@ rfapiNhlAddSubtree (
   struct rfapi_ip_prefix rprefix;
   int                    rcount = 0;
 
+  /* FIXME: need to find a better way here to work without sticking our
+   * hands in node->link */
   if (rn->l_left && rn->l_left != omit_node)
     {
       if (rn->l_left->info)

diff --git a/bgpd/rfapi/rfapi_rib.c b/bgpd/rfapi/rfapi_rib.c
index 5c3976a0c125b6eba049c76ce5eb37b8839f0291..855901b5a73e73b9a67aec576eb491dd4e594e97 100644 (file)
--- a/bgpd/rfapi/rfapi_rib.c
+++ b/bgpd/rfapi/rfapi_rib.c
@@ -1804,6 +1804,8 @@ rfapiRibUpdatePendingNodeSubtree (
   struct route_node            *omit_subtree,    /* may be NULL */
   uint32_t                     lifetime)
 {
+  /* FIXME: need to find a better way here to work without sticking our
+   * hands in node->link */
   if (it_node->l_left && (it_node->l_left != omit_subtree))
     {
       if (it_node->l_left->info)

diff --git a/lib/table.c b/lib/table.c
index fdbd2913b20779632a44cdd8ac4a23e056dbc159..d4dbeb79cc4b235f45acef45b21398af954fa8e3 100644 (file)
--- a/lib/table.c
+++ b/lib/table.c
@@ -19,6 +19,8 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#define FRR_COMPILING_TABLE_C
+
 #include <zebra.h>
 
 #include "prefix.h"

diff --git a/lib/table.h b/lib/table.h
index 6a5291487bf61b544a5a3ad59f76e2816187aeeb..a906eb2434e11a8592e566a5e56340ba904596dd 100644 (file)
--- a/lib/table.h
+++ b/lib/table.h
@@ -71,6 +71,41 @@ struct route_table
   void *info;
 };
 
+/*
+ * node->link is really internal to the table code and should not be
+ * accessed by outside code.  We don't have any writers (yay), though some
+ * readers are left to be fixed.
+ *
+ * rationale: we need to add a hash table in parallel, to speed up
+ * exact-match lookups.
+ *
+ * same really applies for node->parent, though that's less of an issue.
+ * table->link should be - and is - NEVER written by outside code
+ */
+#ifdef FRR_COMPILING_TABLE_C
+#define table_rdonly(x)                x
+#define table_internal(x)      x
+#else
+#define table_rdonly(x)                const x
+#define table_internal(x)      const x \
+       __attribute__((deprecated("this should only be accessed by lib/table.c")))
+/* table_internal is for node->link and node->lock, once we have done
+ * something about remaining accesses */
+#endif
+
+/* so... the problem with this is that "const" doesn't mean "readonly".
+ * It in fact may allow the compiler to optimize based on the assumption
+ * that the value doesn't change.  Hence, since the only purpose of this
+ * is to aid in development, don't put the "const" in release builds.
+ *
+ * (I haven't seen this actually break, but GCC and LLVM are getting ever
+ * more aggressive in optimizing...)
+ */
+#ifndef DEV_BUILD
+#undef table_rdonly
+#define table_rdonly(x) x
+#endif
+
 /*
  * Macro that defines all fields in a route node.
  */
@@ -79,12 +114,12 @@ struct route_table
   struct prefix p;                             \
                                                \
   /* Tree link. */                             \
-  struct route_table *table;                   \
-  struct route_node *parent;                   \
-  struct route_node *link[2];                  \
+  struct route_table * table_rdonly(table);    \
+  struct route_node * table_rdonly(parent);    \
+  struct route_node * table_rdonly(link[2]);   \
                                                \
   /* Lock of this radix */                     \
-  unsigned int lock;                           \
+  unsigned int table_rdonly(lock);             \
                                                \
   /* Each node of route. */                    \
   void *info;                                  \

diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c
index cb522226ac220bb72a8ae8d044e49d6002340b4a..ce31e25a2f983b0c3da2a2c845e07d3c6ca147b4 100644 (file)
--- a/ospf6d/ospf6_lsdb.c
+++ b/ospf6d/ospf6_lsdb.c
@@ -242,6 +242,9 @@ ospf6_lsdb_lookup_next (u_int16_t type, u_int32_t id, u_int32_t adv_router,
     zlog_debug ("lsdb_lookup_next: key: %s", buf);
   }
 
+  /* FIXME: need to find a better way here to work without sticking our
+   * hands in node->link, e.g. route_node_match_maynull() */
+
   node = lsdb->table->top;
   /* walk down tree. */
   while (node && node->p.prefixlen <= p->prefixlen &&