From: Steven Luong <sluong@cisco.com>
Date: Thu, 11 Jun 2020 06:38:41 +0000 (-0700)
Subject: vlib: node recyling and node deletion missing triggering graph node sync
X-Git-Tag: v21.01-rc0~289
X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=1eae8ecb7acc7d80d5c08e300295bec94bf78f0b;p=vpp.git

vlib: node recyling and node deletion missing triggering graph node sync

When recycling a graph node vnet_register_interface, it is missing an
explicit call to vlib_worker_thread_node_runtime_update(). However,
there is an implicit call to vlib_worker_thread_node_runtime_update()
via vnet_sw_interface_set_flags_helper() if it enables a new feature on
the interface for the first time. But that implicit call is not
guaranteed. For example, if an interface is created, deleted, and
created, then it may skip the implicit call to
vlib_worker_thread_node_runtime_update(). When that happens, the graph
nodes on thread 0 are not sync'ed to the worker threads. So the worker
thread's graph nodes are out of sync momentarily with the main thread's
graph nodes until some other event happens which calls for a sync is
needed. During this window, the worker thread's graph node is
vulnerable and may experience a crash.

When deleting a graph node, we never trigger a sync to the worker
thread. A patch was committed 3 years ago via
https://gerrit.fd.io/r/c/vpp/+/7523 to fix a show run crash. In
hindsight, the approach taken by 7523 is not orthogonal. While at it,
let's fix it right for both issues with a call to
vlib_worker_thread_node_runtime_update() in the appropriate place and
remove 7523.

Type: fix
Ticket: VPPSUPP-86
Fixes: gerrit 7523 / 19e9d954bd9eb4f04d48640d6540198e84ef65d7

Signed-off-by: Steven Luong <sluong@cisco.com>
Change-Id: Ic9472bd2d3a212dbfeceb526506ed0400983a142
---

diff --git a/src/vlib/node.c b/src/vlib/node.c
index 3838d950040..ab3574bd9a3 100644
--- a/src/vlib/node.c
+++ b/src/vlib/node.c
@@ -72,32 +72,6 @@ node_set_elog_name (vlib_main_t * vm, uword node_index)
   n->name_elog_string = elog_string (&vm->elog_main, "%v%c", n->name, 0);
 }
 
-static void
-vlib_worker_thread_node_rename (u32 node_index)
-{
-  int i;
-  vlib_main_t *vm;
-  vlib_node_t *n;
-
-  if (vec_len (vlib_mains) == 1)
-    return;
-
-  vm = vlib_mains[0];
-  n = vlib_get_node (vm, node_index);
-
-  ASSERT (vlib_get_thread_index () == 0);
-  ASSERT (*vlib_worker_threads->wait_at_barrier == 1);
-
-  for (i = 1; i < vec_len (vlib_mains); i++)
-    {
-      vlib_main_t *vm_worker = vlib_mains[i];
-      vlib_node_t *n_worker = vlib_get_node (vm_worker, node_index);
-
-      n_worker->name = n->name;
-      n_worker->name_elog_string = n->name_elog_string;
-    }
-}
-
 void
 vlib_node_rename (vlib_main_t * vm, u32 node_index, char *fmt, ...)
 {
@@ -115,7 +89,7 @@ vlib_node_rename (vlib_main_t * vm, u32 node_index, char *fmt, ...)
   node_set_elog_name (vm, node_index);
 
   /* Propagate the change to all worker threads */
-  vlib_worker_thread_node_rename (node_index);
+  vlib_worker_thread_node_runtime_update ();
 }
 
 static void