On 08/21/2018 01:10 AM, Honnappa Nagarahalli wrote:
On 08/17/2018 03:41 PM, Honnappa Nagarahalli wrote:
Can you elaborate more on using ' struct rte_conflict_iterator_state' as the
argument for the API?
If the API signature is changed to: rte_hash_iterate_conflict_entries (const
struct rte_hash *h, void **key, void **data, const hash_sig_t sig, struct
rte_conflict_iterator_state *state) - it will be inline with the existing APIs.
Contents of 'state' must be initialized to 0 for the first call. This will also
avoid creating 'rte_hash_iterator_conflict_entries_init' API.
Testing `state' every time rte_hash_iterate_conflict_entries() is called
to find out if it's the first call of the iterator will possibly add some
small, but unnecessary, overhead on
rte_hash_iterate_conflict_entries() and constraints on struct
rte_conflict_iterator_state. Moreover,
rte_hash_iterator_conflict_entries_init() enables one to easily add variations
of the init function to initialize the state (e.g. using a key instead of a
sig) and still use the exactly same iterator.
IMO, I think, this over-head will be trivial. Looking at the function
'rte_hash_iterate_conflict_entries' the check for '(__state->vnext <
RTE_HASH_BUCKET_ENTRIES * 2)' already exists. If the primary/secondary bucket indices
are calculated as well in 'rte_hash_iterate_conflict_entries' API ('rte_hash_iterate'
API does such calculations), storing them in the state can be avoided. I am wondering
if it makes sense to benchmark with these changes and then take a decision?
We have come up with the init function and struct
rte_conflict_iterator_state in v2 to make the new iterator as future
proof to a change of the underlying algorithm as possible. But going
through your feedback, it seems to me that your top concern is to not
deviate much of the current interface of rte_hash_iterate(). We are fine
with pushing v3 using the interface you've suggested to avoid the init
function and struct rte_conflict_iterator_state:
int32_t
rte_hash_iterate_conflict_entries__with_hash(const struct rte_hash *h,
const void **key, void **data, hash_sig_t sig, uint32_t *next);
[ ]'s
Michel Machado
