Thanks for the patch Dave. With this I am not seeing the corruption issue of node hash key in node_by_name hash.
Regards, Sontu On Sat, 12 Jun, 2021, 11:09 AM sontu mazumdar, <[email protected]> wrote: > Thanks Dave. > I will try with your suggested code changes and will share the result. > > Regards, > Sontu > > On Fri, 11 Jun, 2021, 9:34 PM Dave Barach, <[email protected]> wrote: > >> Please try these diffs and report results. >> >> >> >> diff --git a/src/vlib/unix/cli.c b/src/vlib/unix/cli.c >> >> index 6c9886725..ce29e0723 100644 >> >> --- a/src/vlib/unix/cli.c >> >> +++ b/src/vlib/unix/cli.c >> >> @@ -2863,6 +2863,7 @@ unix_cli_file_add (unix_cli_main_t * cm, char >> *name, int fd) >> >> { >> >> unix_main_t *um = &unix_main; >> >> clib_file_main_t *fm = &file_main; >> >> + vlib_node_main_t *nm = &vlib_get_main()->node_main; >> >> unix_cli_file_t *cf; >> >> clib_file_t template = { 0 }; >> >> vlib_main_t *vm = um->vlib_main; >> >> @@ -2896,10 +2897,12 @@ unix_cli_file_add (unix_cli_main_t * cm, char >> *name, int fd) >> >> old_name = n->name; >> >> n->name = (u8 *) name; >> >> } >> >> + ASSERT(old_name); >> >> + hash_unset (nm->node_by_name, old_name); >> >> + hash_set (nm->node_by_name, name, n->index); >> >> vec_free (old_name); >> >> vlib_node_set_state (vm, n->index, VLIB_NODE_STATE_POLLING); >> >> - >> >> _vec_len (cm->unused_cli_process_node_indices) = l - 1; >> >> } >> >> else >> >> >> >> *From:* [email protected] <[email protected]> *On Behalf Of *sontu >> mazumdar >> *Sent:* Friday, June 11, 2021 11:34 AM >> *To:* [email protected] >> *Subject:* [vpp-dev] "unix-cli-local" node corruption in node_by_name >> hash #vpp #vpp-dev >> >> >> >> Hi, >> >> >> >> I observe that in node_by_name hash we store a node with name >> "unix-cli-local:0" and node index 720 (not sure the purpose of the node). >> >> The node name is stored as key in the node_by_name hash. >> >> But later at some time when I print the node_by_name hash's each entry I >> see the key of node i.e the node name is printing some junk value (I >> figured it out via checking it against the node index). >> >> >> >> When I looked at code in unix_cli_file_add(), below we are first time >> adding the node with name "unix-cli-local:0". >> >> >> >> static vlib_node_registration_t r = { >> >> .function = unix_cli_process, >> >> .type = VLIB_NODE_TYPE_PROCESS, >> >> .process_log2_n_stack_bytes = 18, >> >> }; >> >> >> >> r.name = name; >> >> >> >> vlib_worker_thread_barrier_sync (vm); >> >> >> >> vlib_register_node (vm, &r); <<<<<<<<<<<< >> >> vec_free (name); >> >> >> >> n = vlib_get_node (vm, r.index); >> >> vlib_worker_thread_node_runtime_update (); >> >> vlib_worker_thread_barrier_release (vm); >> >> >> >> >> >> Later it again calls unix_cli_file_add(), there we pass a different name >> "unix-cli-local:1". >> >> In this case we are overwriting the already existing node name from >> "unix-cli-local:0" to "unix-cli-local:1". >> >> >> >> for (i = 0; i < vec_len (vlib_mains); i++) >> >> { >> >> this_vlib_main = vlib_mains[i]; >> >> if (this_vlib_main == 0) >> >> continue; >> >> n = vlib_get_node (this_vlib_main, >> >> cm->unused_cli_process_node_indices[l - 1]); >> >> old_name = n->name; <<<<<<<<<<< >> >> n->name = (u8 *) name; <<<<<<<<<<< >> >> } >> >> vec_free (old_name); <<<<<<<<<< >> >> >> >> But the node name is already present in node_by_name hash as a key and >> there we haven't updated it instead we have deleted the old name. >> >> This is resulting in printing some corrupted node name for the above node >> in node_by_name hash, which I think can sometimes results in VPP crash also >> as the hash key points to some freed memory. >> >> >> >> Regards, >> >> Sontu >> >> >> >>
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#19563): https://lists.fd.io/g/vpp-dev/message/19563 Mute This Topic: https://lists.fd.io/mt/83471274/21656 Mute #vpp:https://lists.fd.io/g/vpp-dev/mutehashtag/vpp Mute #vpp-dev:https://lists.fd.io/g/vpp-dev/mutehashtag/vpp-dev Group Owner: [email protected] Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
