fib: IPv6 lookup data structure MP safe when prefixes change

Type: fix

adding routes should be MP safe. When new prefixes with differrent
prefix lengths are added, adjust the sorted list in an MP safe way.

Change-Id: Ib73a3c84d01eb86d17f8e79ea2bd2505dd9afb3d
Signed-off-by: Neale Ranns <nranns@cisco.com>

diff --git a/src/vlib/main.h b/src/vlib/main.h
index f7a4a1c..45a521a 100644 (file)
--- a/src/vlib/main.h
+++ b/src/vlib/main.h
@@ -133,7 +133,7 @@ typedef struct vlib_main_t
   u64 cpu_time_main_loop_start;
 
   /* Incremented once for each main loop. */
-  u32 main_loop_count;
+  volatile u32 main_loop_count;
 
   /* Count of vectors processed this main loop. */
   u32 main_loop_vectors_processed;

diff --git a/src/vlib/threads.c b/src/vlib/threads.c
index 4df550e..d5096c8 100644 (file)
--- a/src/vlib/threads.c
+++ b/src/vlib/threads.c
@@ -1444,6 +1444,18 @@ vlib_worker_thread_initial_barrier_sync_and_release (vlib_main_t * vm)
   *vlib_worker_threads->wait_at_barrier = 0;
 }
 
+/**
+ * Return true if the wroker thread barrier is held
+ */
+u8
+vlib_worker_thread_barrier_held (void)
+{
+  if (vec_len (vlib_mains) < 2)
+    return (1);
+
+  return (*vlib_worker_threads->wait_at_barrier == 1);
+}
+
 void
 vlib_worker_thread_barrier_sync_int (vlib_main_t * vm, const char *func_name)
 {
@@ -1647,6 +1659,41 @@ vlib_worker_thread_barrier_release (vlib_main_t * vm)
                         vm->clib_time.last_cpu_time, 1 /* leave */ );
 }
 
+/**
+ * Wait until each of the workers has been once around the track
+ */
+void
+vlib_worker_wait_one_loop (void)
+{
+  ASSERT (vlib_get_thread_index () == 0);
+
+  if (vec_len (vlib_mains) < 2)
+    return;
+
+  if (vlib_worker_thread_barrier_held ())
+    return;
+
+  u32 *counts = 0;
+  u32 ii;
+
+  vec_validate (counts, vec_len (vlib_mains) - 1);
+
+  /* record the current loop counts */
+  vec_foreach_index (ii, vlib_mains)
+    counts[ii] = vlib_mains[ii]->main_loop_count;
+
+  /* spin until each changes, apart from the main thread, or we'd be
+   * a while */
+  for (ii = 1; ii < vec_len (counts); ii++)
+    {
+      while (counts[ii] == vlib_mains[ii]->main_loop_count)
+       CLIB_PAUSE ();
+    }
+
+  vec_free (counts);
+  return;
+}
+
 /*
  * Check the frame queue to see if any frames are available.
  * If so, pull the packets off the frames and put them to

diff --git a/src/vlib/threads.h b/src/vlib/threads.h
index e8d4169..659f052 100644 (file)
--- a/src/vlib/threads.h
+++ b/src/vlib/threads.h
@@ -207,8 +207,13 @@ u32 vlib_frame_queue_main_init (u32 node_index, u32 frame_queue_nelts);
 void vlib_worker_thread_barrier_sync_int (vlib_main_t * vm,
                                          const char *func_name);
 void vlib_worker_thread_barrier_release (vlib_main_t * vm);
+u8 vlib_worker_thread_barrier_held (void);
 void vlib_worker_thread_initial_barrier_sync_and_release (vlib_main_t * vm);
 void vlib_worker_thread_node_refork (void);
+/**
+ * Wait until each of the workers has been once around the track
+ */
+void vlib_worker_wait_one_loop (void);
 
 static_always_inline uword
 vlib_get_thread_index (void)

diff --git a/src/vnet/fib/ip6_fib.c b/src/vnet/fib/ip6_fib.c
index 5664173..ddd7079 100644 (file)
--- a/src/vnet/fib/ip6_fib.c
+++ b/src/vnet/fib/ip6_fib.c
@@ -248,14 +248,29 @@ ip6_fib_table_lookup_exact_match (u32 fib_index,
 static void
 compute_prefix_lengths_in_search_order (ip6_fib_table_instance_t *table)
 {
+    u8 *old, *prefix_lengths_in_search_order = NULL;
     int i;
-    vec_reset_length (table->prefix_lengths_in_search_order);
+
+    /*
+     * build the list in a scratch space then cutover so the workers
+     * can continue uninterrupted.
+     */
+    old = table->prefix_lengths_in_search_order;
+
     /* Note: bitmap reversed so this is in fact a longest prefix match */
     clib_bitmap_foreach (i, table->non_empty_dst_address_length_bitmap,
     ({
        int dst_address_length = 128 - i;
-       vec_add1(table->prefix_lengths_in_search_order, dst_address_length);
+       vec_add1(prefix_lengths_in_search_order, dst_address_length);
     }));
+
+    table->prefix_lengths_in_search_order = prefix_lengths_in_search_order;
+
+    /*
+     * let the workers go once round the track before we free the old set
+     */
+    vlib_worker_wait_one_loop();
+    vec_free(old);
 }
 
 void
@@ -311,12 +326,13 @@ ip6_fib_table_entry_insert (u32 fib_index,
 
     clib_bihash_add_del_24_8(&table->ip6_hash, &kv, 1);
 
-    table->dst_address_length_refcounts[len]++;
-
-    table->non_empty_dst_address_length_bitmap =
-        clib_bitmap_set (table->non_empty_dst_address_length_bitmap, 
-                        128 - len, 1);
-    compute_prefix_lengths_in_search_order (table);
+    if (0 == table->dst_address_length_refcounts[len]++)
+    {
+        table->non_empty_dst_address_length_bitmap =
+            clib_bitmap_set (table->non_empty_dst_address_length_bitmap,
+                             128 - len, 1);
+        compute_prefix_lengths_in_search_order (table);
+    }
 }
 
 u32 ip6_fib_table_fwding_lookup_with_if_index (ip6_main_t * im,
@@ -363,12 +379,13 @@ ip6_fib_table_fwding_dpo_update (u32 fib_index,
 
     clib_bihash_add_del_24_8(&table->ip6_hash, &kv, 1);
 
-    table->dst_address_length_refcounts[len]++;
-
-    table->non_empty_dst_address_length_bitmap =
-        clib_bitmap_set (table->non_empty_dst_address_length_bitmap, 
-                        128 - len, 1);
-    compute_prefix_lengths_in_search_order (table);
+    if (0 == table->dst_address_length_refcounts[len]++)
+    {
+        table->non_empty_dst_address_length_bitmap =
+            clib_bitmap_set (table->non_empty_dst_address_length_bitmap,
+                             128 - len, 1);
+        compute_prefix_lengths_in_search_order (table);
+    }
 }
 
 void