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.
>> > >
>> > >
>> > >
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#15606): https://lists.fd.io/g/vpp-dev/message/15606
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]
-=-=-=-=-=-=-=-=-=-=-=-
