Re: [dpdk-dev] rte_hash thread safe

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

A gentle reminder,

I am curious to know if/how rte_hash is thread safe for lookups.It is not obvious to me how following code is thread safe:

_rte_hash_lookup_with_hash(const struct rte_hash *h, const void *key,

                                        hash_sig_t sig, void **data)

{



…

                        if (rte_hash_cmp_eq(key, k->key, h) == 0) {

                                if (data != NULL)

                                        *data = k->pdata;

}

a key could be deleted and another key inserted in its slot while the lookup is happening. For example, in the following sequence of events:
The slot has Key1,V1
Lookup Thread T1 compares the input key to Key1 and it matches. The thread gets context switched out Thread T2 deletes Key1.
Thread T2 inserts Key2 with value V2.
T1 reads the data from the slot and returns V2. This is incorrect.

If T1 is a worker/data plane thread, which is supposed to be pinned to the CPU, is it required to consider the case of T1 getting pre-empted?

However, this case can exist with T1 not getting pre-empted. i.e. Key1 can get deleted by thread T2 while T1 is doing the comparison. 

In this case, 'k' is a memory pointer pointing to an array element. The memory itself is not freed. The code will still return the data associated with Key1 - assuming Key2 belongs to a different slot ID. But, it is possible that 'data' may not be valid anymore since it is owned by the application.


Regards,
Brijesh

On Wed, Apr 11, 2018 at 9:12 PM, Brijesh Singh <brijesh.s.singh@gmail.com> wrote:
> Hello,
>
> I  want to use DPDK's rte_hash library to keep track of tcp flows. The 
> lookups will be done by multiple threads but inserts will be done only 
> on one thread.
>
> As per the documentation rte_hash library has thread safe lookups. Key 
> /data inserts should be done on single thread, since those operations 
> are not thread safe. Is this documentation still correct?
>
> The lookup code compares the key and returns the data if the key 
> matches, this doesn't look like thread safe. Am I missing something?
>
> _rte_hash_lookup_with_hash(const struct rte_hash *h, const void *key,
>
>                                         hash_sig_t sig, void **data)
>
> {
>
>
>
> …
>
>                         if (rte_hash_cmp_eq(key, k->key, h) == 0) {
>
>                                 if (data != NULL)
>
>                                         *data = k->pdata;
>
> }
>
> Regards,
> Brijesh