Please find inline. Thanks Govind
> -----Original Message----- > From: Dave Barach (dbarach) <dbar...@cisco.com> > Sent: Friday, February 28, 2020 4:17 PM > To: Govindarajan Mohandoss <govindarajan.mohand...@arm.com>; > Andrew 👽 Yourtchenko <ayour...@gmail.com>; Honnappa Nagarahalli > <honnappa.nagaraha...@arm.com> > Cc: Benoit Ganne (bganne) <bga...@cisco.com>; cho...@chopps.org; vpp- > d...@lists.fd.io; nd <n...@arm.com>; Lijian Zhang <lijian.zh...@arm.com> > Subject: RE: [vpp-dev] Q: how best to avoid locking for cleanup. > > On the data plane side, please use vm->main_loop_count. Mark the variable > volatile in src/vlib/main.h. > > Atomically update data structures, include a memory barrier. > Foreach_thread: snapshoot vm->main_loop_count. > Delay until all vm->main_loop_count values have changed. 2 consecutive snapshots can be obtained by introducing delay between the "vm->main_loop_count" polls. The delay timer value (??) will have an impact on the API configuration time. A delay timer can be replaced with a spinning loop till the "vm->main_loop_count" changes. This will be relatively better than hard coding a delay timer value. But, this will also have an impact on API config time. The API config time may not be crucial for IPSec SA deletion / IP route deletion, but it is sensitive to GTPu kind of protocols. As mentioned before, this problem can be solved by introducing a Pending list scheme in control plane. " Optimization Note: Instead of waiting for data plane sync, a pending list can be maintained to hold the memory objects that needs to be freed after all the inflight packets are flushed out. The memory objects in the Pending list can then be freed through next data plane table entry add/delete APIs." More details about API latency sensitive applications: ============================================ I noticed GTPu plugin in VPP. Not sure about the VPP use case of GTPu. https://github.com/FDio/vpp/tree/master/src/plugins/gtpu In general, GTPu is heavily used in Radio access Networks for LTE and 5G Transport. The GTPu Tunnel setup/tear down (Based on UE call setup/teardown) will happen at higher rate. Considering VPP to run on LTE/5G Transport nodes like eNodeB/gNB in the future (Not sure whether VPP is already used in eNodeB ?), we have to take API latency also as an important requirement and address it. > Clean up old data structures. > > FWIW... Dave > > -----Original Message----- > From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> On Behalf Of Govindarajan > Mohandoss > Sent: Friday, February 28, 2020 4:55 PM > To: Andrew 👽 Yourtchenko <ayour...@gmail.com>; Honnappa Nagarahalli > <honnappa.nagaraha...@arm.com> > Cc: Benoit Ganne (bganne) <bga...@cisco.com>; cho...@chopps.org; vpp- > d...@lists.fd.io; nd <n...@arm.com>; Lijian Zhang <lijian.zh...@arm.com> > Subject: Re: [vpp-dev] Q: how best to avoid locking for cleanup. > > Hi Chris, > <snip> > >>>>I do wonder how many other cases of "state associated with in-flight > >>>>packets" there might be, and if more sophisticated general solution > >>>>might be useful. > >>>>> RCU mechanism is the general solution to avoid Reader locks in data > plane. With this scheme, there is no lock needed in the data plane. The in- > flight packets can be tracked through the counters like the way you described > (or) through an alternate scheme explained below. The control plane can do > a delayed free of the shared memory objects (Shared with data plane) based > on these counters. > If the scope of the counters are widened across the entire packet processing > path in run to completion model, then all the data structures like Bihash, > IPSec SA and other unknown cases can be covered. > > Proposal: > ======= > Control Plane: > =========== > 1. Maintain a common running counter across all the control plane threads. > 2. Delete the shared memory objects from Bihash/IPSec SA table. > 3. Set the running counter in shared memory. > 4. Wait till all the data plane threads catch up with this counter value > (Maintain a catch up counter per data plane thread). > Optimization Note: Instead of waiting for data plane sync, a pending list > can be maintained to hold the memory objects that needs to be freed after > all the inflight packets are flushed out. > The memory objects in the Pending > list can then be > freed through next data plane table entry add/delete APIs. > 5. Free the shared memory objects once all the data plane threads catch up. > > In Fast path: > ========== > While (1) /* packet processing loop in run-to-completion model */ { > Ethernet processing, > IP Processing, > IPSec Processing, > Etc., > /* End of packet processing */ > Catch up counter = counter value set by control plane. <<<< Data > plane/Control plane sync } > > There is a solution to this problem already in DPDK. We can evaluate it and > decide whether it will fit VPP requirements (or) we can do a new design for > VPP. > https://doc.dpdk.org/guides/prog_guide/rcu_lib.html > > Thanks > Govind > > > -----Original Message----- > > From: Andrew 👽 Yourtchenko <ayour...@gmail.com> > > Sent: Friday, February 28, 2020 5:50 AM > > To: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> > > Cc: Benoit Ganne (bganne) <bga...@cisco.com>; cho...@chopps.org; > vpp- > > d...@lists.fd.io; nd <n...@arm.com>; Govindarajan Mohandoss > > <govindarajan.mohand...@arm.com>; Lijian Zhang > <lijian.zh...@arm.com> > > Subject: Re: [vpp-dev] Q: how best to avoid locking for cleanup. > > > > On 2/28/20, Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> > > wrote: > > > > >> On the other hand, if you do modify shared data structures in the > > >> datapath, you are on your own - you need to take care of the data > > >> consistency. > > >> Again, the way we usually deal with that is to do a "rpc" to the > > >> main thread - then the main thread can request the worker barrier, etc. > > >> > > >> Or do you refer to other situations? > > > I was looking at the bi-hash library on a standalone basis. The > > > entries are deleted and buckets are freed without any > > > synchronization between the writer > > > > FWIW, the above statement is incorrect if all we are talking is pure > > bihash operation with values that are not used as keys for subsequent > > memory accesses in other data structures. This code in > > include/vppinfra/bihash_template.c might be of interest: > > > > static inline int BV (clib_bihash_add_del_inline) > > (BVT (clib_bihash) * h, BVT (clib_bihash_kv) * add_v, int is_add, > > int (*is_stale_cb) (BVT (clib_bihash_kv) *, void *), void *arg) { > > u32 bucket_index; > > > > ... > > > > > > BV (clib_bihash_lock_bucket) (b); <------------- LOCK > > > > /* First elt in the bucket? */ > > if (BV (clib_bihash_bucket_is_empty) (b)) > > { > > if (is_add == 0) > > { > > BV (clib_bihash_unlock_bucket) (b); > > return (-1); > > } > > > > .... > > > > /* Move readers to a (locked) temp copy of the bucket */ > > BV (clib_bihash_alloc_lock) (h); <------------------------- LOCK > > BV (make_working_copy) (h, b); > > > > ----- > > > > and so on. > > > > when performing the actual bihash operations as a user of the bihash, > > you most definitely do *not* need any extra locking, the bihash is > > doing it for you behind the scenes. > > > > There is only one transient condition that I had seen - under intense > > add/delete workload, the readers in other threads may see the lookup > > successful but the value returned being ~0. > > That is fairly easy to deal with. > > > > But of course there is a race in case you are using bihash to store > > secondary indices into your own data structures - if you are deleting > > a bihash entry, another thread may have *just* made a lookup, obtained > > the index, and is using that index, so for that part you do need to > > take care of by yourself, indeed. > > > > --a > > > > > and the data plane threads. If the synchronization is handled > > > outside of this library using 'worker barrier' it should be alright. > > > > > >> > > >> Best > > >> Ben > > >> > > >> > -----Original Message----- > > >> > From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> On Behalf Of > > >> > Honnappa Nagarahalli > > >> > Sent: jeudi 27 février 2020 17:51 > > >> > To: cho...@chopps.org; vpp-dev@lists.fd.io; Honnappa Nagarahalli > > >> > <honnappa.nagaraha...@arm.com> > > >> > Cc: nd <n...@arm.com> > > >> > Subject: Re: [vpp-dev] Q: how best to avoid locking for cleanup. > > >> > > > >> > I think there are similar issues in bi-hash (i.e. the entry could > > >> > be deleted from control plane while the data plane threads are > > >> > doing the lookup). > > >> > > > >> > > > >> > > > >> > Thanks, > > >> > > > >> > Honnappa > > >> > > > >> > > > >> > > > >> > From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> On Behalf Of > > >> > Christian Hopps via Lists.Fd.Io > > >> > Sent: Thursday, February 27, 2020 5:09 AM > > >> > To: vpp-dev <vpp-dev@lists.fd.io> > > >> > Cc: vpp-dev@lists.fd.io > > >> > Subject: Re: [vpp-dev] Q: how best to avoid locking for cleanup. > > >> > > > >> > > > >> > > > >> > I received a private message indicating that one solution was to > > >> > just wait "long enough" for the packets to drain. This is the > > >> > method I'm going to go with as it's simple (albeit not as > > >> > deterministic as some marking/callback scheme :) > > >> > > > >> > For my case I think I can wait ridiculously long for "long enough" > > >> > and just have a process do garbage collection after a full second. > > >> > > > >> > I do wonder how many other cases of "state associated with > > >> > in-flight packets" there might be, and if more sophisticated > > >> > general solution might be useful. > > >> > > > >> > Thanks, > > >> > Chris. > > >> > > > >> > > On Feb 25, 2020, at 6:27 PM, Christian Hopps <cho...@chopps.org > > >> > <mailto:cho...@chopps.org> > wrote: > > >> > > > > >> > > I've got a (hopefully) interesting problem with locking in VPP. > > >> > > > > >> > > I need to add some cleanup code to my IPTFS additions to ipsec. > > >> > Basically I have some per-SA queues that I need to cleanup when > > >> > an SA is deleted. > > >> > > > > >> > > - ipsec only deletes it's SAs when its "fib_node" locks reach zero. > > >> > > - I hoping this means that ipsec will only be deleting the SA > > >> > > after the > > >> > FIB has stopped injecting packets "along" this SA path (i.e., > > >> > it's removed prior to the final unlock/deref). > > >> > > - I'm being called back by ipsec during the SA deletion. > > >> > > - I have queues (one RX for reordering, one TX for aggregation > > >> > > and > > >> > subsequent output) associated with the SA, both containing locks, > > >> > that need to be emptied and freed. > > >> > > - These queues are being used in multiple worker threads in > > >> > > various > > >> > graph nodes in parallel. > > >> > > > > >> > > What I think this means is that when my "SA deleted" callback > > >> > > is called, > > >> > no *new* packets will be delivered on the SA path. Good so far. > > >> > > > > >> > > What I'm concerned with is the packets that may currently be > > >> > > "in-flight" > > >> > in the graph, as these will have the SA associated with them, and > > >> > thus my code may try and use the per SA queues which I'm now > > >> > trying to delete. > > >> > > > > >> > > There's a somewhat clunky solution involving global locks prior > > >> > > to and > > >> > after using an SA in each node, tracking it's validity (which has > > >> > it's own issues), freeing when no longer in use etc.. but this > > >> > would introduce global locking in the packet path which I'm loathe to > do. > > >> > > > > >> > > What I'd really like is if there was something like this: > > >> > > > > >> > > - packet ingress to SA fib node, fib node lock count increment. > > >> > > - packet completes it's journey through the VPP graph (or at > > >> > > least my > > >> > part of it) and decrements that fib node lock count. > > >> > > - when the SA should be deleted it removes it's fib node from > > >> > > the fib, > > >> > thus preventing new packets entering the graph then unlocks. > > >> > > - the SA is either immediately deleted (no packets in flight), > > >> > > or > > >> > deleted when the last packet completes it's graph traversal. > > >> > > > > >> > > I could do something like this inside my own nodes (my first > > >> > > node is > > >> > point B), but then there's still the race between when the fib > > >> > node is used to inject the packet to the next node in the graph > > >> > (point > > >> > A) and that packet arriving at my first IPTFS node (point B), > > >> > when the SA deletion could occur. Maybe i could modify the fib > > >> > code to do this at point A. I haven't looked closely at the fib code > > >> > yet. > > >> > > > > >> > > Anyway I suspect this has been thought about before, and maybe > > >> > > there's > > >> > even a solution already present in VPP, so I wanted to ask. :) > > >> > > > > >> > > Thanks, > > >> > > Chris. > > >> > > > > >> > > > > >> > > > > > > > > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, > please notify the sender immediately and do not disclose the contents to any > other person, use it for any purpose, or store or copy the information in any > medium. Thank you. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#15636): https://lists.fd.io/g/vpp-dev/message/15636 Mute This Topic: https://lists.fd.io/mt/71544411/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
