On 08/17/2018 03:41 PM, Honnappa Nagarahalli wrote:
Do we also need to have 'rte_hash_iterate_conflict_entries_with_hash' API?I may have not understood the question. We are already working with the hash (i.e. sig). Did you mean something else? Let me elaborate. For the API 'rte_hash_lookup', there are multiple variations such as 'rte_hash_lookup_with_hash', 'rte_hash_lookup_data', 'rte_hash_lookup_with_hash_data' etc. We do not need to create similar variations for 'rte_hash_iterate_conflict_entries' API right now. But the naming of the API should be such that these variations can be created in the future.
So you mean that we should actually name rte_hash_iterator_conflict_entries_init() as rte_hash_iterator_conflict_entries_init_with_hash()? I'd be fine with this.
diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index f71ca9fbf..7ecb6a7eb 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -61,6 +61,11 @@ struct rte_hash_parameters { /** @internal A hash table structure. */ struct rte_hash;+/** @internal A hash table conflict iterator state structure. */+struct rte_conflict_iterator_state { + uint8_t space[64]; +}; +Needs aligning to cache line.
Ok.
The size depends on the current size of the state, which is subject to change with the algorithm used.We chose a size that should be robust for any future underlying algorithm. Do you have a suggestion on how to go about it? We chose to have a simple struct to enable applications to allocate a state as a local variable and avoid a memory allocation. This looks fine after your explanation. The structure name can be changed to 'rte_iterator_state' so that it can be used in other iterator APIs too.
I like this suggestion. What about the name "rte_hash_iterator_state" to make it specific to the hash table?
[ ]'s Michel Machado
