From aa62162cbc44160b871092529824c87b60fb64df Mon Sep 17 00:00:00 2001 From: Steven Luong Date: Wed, 10 Jun 2020 23:38:41 -0700 Subject: 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 Change-Id: Ic9472bd2d3a212dbfeceb526506ed0400983a142 (cherry picked from commit 1eae8ecb7acc7d80d5c08e300295bec94bf78f0b) --- src/vlib/node.c | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) (limited to 'src') diff --git a/src/vlib/node.c b/src/vlib/node.c index 2bb5bceadbc..fb250e2ce61 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 -- cgit 1.2.3-korg