> On Nov 1, 2019, at 4:20 PM, Dave Barach (dbarach) <dbar...@cisco.com> wrote: > > Shows you how often people have actually hacked graph arcs. > > One way to handle it would be to add an explicit vlib_node_delete_next (vm, > node_index, next_node_index) to clean out the hash table entry / set the arc > next node index to ~0 or some such. > > If that sounds OK, I can build it or you can build it. > > Thoughts?
So I pushed my local change which is a patch that just fixes the function itself: https://gerrit.fd.io/r/c/vpp/+/23178 I wanted to keep the change as small as possible to shrink the test surface, and make cherry-picking to stable/1908 as simple as possible. I think a delete function would also be useful too, providing for nice API symmetry, but perhaps that could be done in a different "feature" (vs fix) commit that wouldn't necessarily have the same urgency. And, yes I realize this bug's probably been there a while, so neither change may seem very urgent to others. :) Thanks, Chris. > > Thanks... Dave > > From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> On Behalf Of Christian Hopps > Sent: Friday, November 1, 2019 3:53 PM > To: vpp-dev <vpp-dev@lists.fd.io> > Cc: Christian Hopps <cho...@chopps.org> > Subject: [vpp-dev] vlib_node_add_next_with_slot bug? > > In my code based on the user changing the ipsec backend I need to update the > node in my "encrypt" slots (for v4 and v6). I am calling > vlib_node_add_next_with_slot to do this; however, the function isn't doing > what I want. > > Basically vlib_node_add_next_with_slot does the following: > > if ((p = hash_get (node->next_slot_by_node, next_node_index))) > { > /* Next already exists: slot must match. */ > if (slot != ~0) > ASSERT (slot == p[0]); > > vlib_worker_thread_barrier_release (vm); > return p[0]; > } > > Prior to making the change. It does not remove any old node from > next_slot_by_node after it makes a change though. The result is that if you > try and add back a node that was previously added (and removed) to that slot > the function does nothing b/c it has this dangling reference in the hash > table. > > Now this could have been by design, but I suspect not as it really doesn't > serve any purpose, that I can see, to leave an old node "back" reference > there when it's no longer true. > > If people agree I can submit a patch to remove the old hash table entry when > the slot is changed. > > Thanks, > Chris. > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > > View/Reply Online (#14466): https://lists.fd.io/g/vpp-dev/message/14466 > Mute This Topic: https://lists.fd.io/mt/40479963/675269 > Group Owner: vpp-dev+ow...@lists.fd.io > Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [dbar...@cisco.com] > -=-=-=-=-=-=-=-=-=-=-=-
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#14475): https://lists.fd.io/g/vpp-dev/message/14475 Mute This Topic: https://lists.fd.io/mt/40479963/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
