* [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
@ 2017-08-18 20:09 mstolarchuk
2017-10-17 13:59 ` Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: mstolarchuk @ 2017-08-18 20:09 UTC (permalink / raw)
To: bruce.richardson, pablo.de.lara.guarch; +Cc: dev
Improves consistency, allows identifcation of use-sites
Signed-off-by: mstolarchuk <mike.stolarchuk@bigswitch.com>
---
lib/librte_hash/rte_cuckoo_hash.c | 64 +++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 33 deletions(-)
diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 87b25c0..d1fbb0b 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -500,6 +500,20 @@ struct rte_hash *
rte_ring_sp_enqueue(h->free_slots, slot_id);
}
+/*
+ * Function to compute the location of rte_hash_key from an index
+ */
+static inline struct rte_hash_key *
+__rte_hash_key_from_idx(const struct rte_hash *h, const uint32_t idx)
+{
+ /* private structure only to compute rte_hash_key offset */
+ struct key_entry {
+ char element_size[h->key_entry_size];
+ } __attribute__((packed)) *key_store = h->key_store;
+
+ return (struct rte_hash_key *)(key_store + idx);
+}
+
static inline int32_t
__rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key,
hash_sig_t sig, void *data)
@@ -508,7 +522,7 @@ struct rte_hash *
uint32_t prim_bucket_idx, sec_bucket_idx;
unsigned i;
struct rte_hash_bucket *prim_bkt, *sec_bkt;
- struct rte_hash_key *new_k, *k, *keys = h->key_store;
+ struct rte_hash_key *new_k, *k;
void *slot_id = NULL;
uint32_t new_idx;
int ret;
@@ -556,7 +570,7 @@ struct rte_hash *
}
}
- new_k = RTE_PTR_ADD(keys, (uintptr_t)slot_id * h->key_entry_size);
+ new_k = __rte_hash_key_from_idx(h, (uintptr_t)slot_id);
rte_prefetch0(new_k);
new_idx = (uint32_t)((uintptr_t) slot_id);
@@ -564,8 +578,7 @@ struct rte_hash *
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
if (prim_bkt->sig_current[i] == sig &&
prim_bkt->sig_alt[i] == alt_hash) {
- k = (struct rte_hash_key *) ((char *)keys +
- prim_bkt->key_idx[i] * h->key_entry_size);
+ k = __rte_hash_key_from_idx(h, prim_bkt->key_idx[i]);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
/* Enqueue index of free slot back in the ring. */
enqueue_slot_back(h, cached_free_slots, slot_id);
@@ -584,8 +597,7 @@ struct rte_hash *
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
if (sec_bkt->sig_alt[i] == sig &&
sec_bkt->sig_current[i] == alt_hash) {
- k = (struct rte_hash_key *) ((char *)keys +
- sec_bkt->key_idx[i] * h->key_entry_size);
+ k = __rte_hash_key_from_idx(h, sec_bkt->key_idx[i]);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
/* Enqueue index of free slot back in the ring. */
enqueue_slot_back(h, cached_free_slots, slot_id);
@@ -711,6 +723,7 @@ struct rte_hash *
else
return ret;
}
+
static inline int32_t
__rte_hash_lookup_with_hash(const struct rte_hash *h, const void *key,
hash_sig_t sig, void **data)
@@ -719,7 +732,7 @@ struct rte_hash *
hash_sig_t alt_hash;
unsigned i;
struct rte_hash_bucket *bkt;
- struct rte_hash_key *k, *keys = h->key_store;
+ struct rte_hash_key *k;
bucket_idx = sig & h->bucket_bitmask;
bkt = &h->buckets[bucket_idx];
@@ -728,8 +741,7 @@ struct rte_hash *
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
if (bkt->sig_current[i] == sig &&
bkt->key_idx[i] != EMPTY_SLOT) {
- k = (struct rte_hash_key *) ((char *)keys +
- bkt->key_idx[i] * h->key_entry_size);
+ k = __rte_hash_key_from_idx(h, bkt->key_idx[i]);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
if (data != NULL)
*data = k->pdata;
@@ -751,8 +763,7 @@ struct rte_hash *
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
if (bkt->sig_current[i] == alt_hash &&
bkt->sig_alt[i] == sig) {
- k = (struct rte_hash_key *) ((char *)keys +
- bkt->key_idx[i] * h->key_entry_size);
+ k = __rte_hash_key_from_idx(h, bkt->key_idx[i]);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
if (data != NULL)
*data = k->pdata;
@@ -835,7 +846,7 @@ struct rte_hash *
hash_sig_t alt_hash;
unsigned i;
struct rte_hash_bucket *bkt;
- struct rte_hash_key *k, *keys = h->key_store;
+ struct rte_hash_key *k;
int32_t ret;
bucket_idx = sig & h->bucket_bitmask;
@@ -845,8 +856,7 @@ struct rte_hash *
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
if (bkt->sig_current[i] == sig &&
bkt->key_idx[i] != EMPTY_SLOT) {
- k = (struct rte_hash_key *) ((char *)keys +
- bkt->key_idx[i] * h->key_entry_size);
+ k = __rte_hash_key_from_idx(h, bkt->key_idx[i]);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
remove_entry(h, bkt, i);
@@ -870,8 +880,7 @@ struct rte_hash *
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
if (bkt->sig_current[i] == alt_hash &&
bkt->key_idx[i] != EMPTY_SLOT) {
- k = (struct rte_hash_key *) ((char *)keys +
- bkt->key_idx[i] * h->key_entry_size);
+ k = __rte_hash_key_from_idx(h, bkt->key_idx[i]);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
remove_entry(h, bkt, i);
@@ -910,9 +919,7 @@ struct rte_hash *
{
RETURN_IF_TRUE(((h == NULL) || (key == NULL)), -EINVAL);
- struct rte_hash_key *k, *keys = h->key_store;
- k = (struct rte_hash_key *) ((char *) keys + (position + 1) *
- h->key_entry_size);
+ struct rte_hash_key *k = __rte_hash_key_from_idx(h, position + 1);
*key = k->key;
if (position !=
@@ -1037,9 +1044,7 @@ struct rte_hash *
uint32_t first_hit = __builtin_ctzl(prim_hitmask[i]);
uint32_t key_idx = primary_bkt[i]->key_idx[first_hit];
const struct rte_hash_key *key_slot =
- (const struct rte_hash_key *)(
- (const char *)h->key_store +
- key_idx * h->key_entry_size);
+ __rte_hash_key_from_idx(h, key_idx);
rte_prefetch0(key_slot);
continue;
}
@@ -1048,9 +1053,7 @@ struct rte_hash *
uint32_t first_hit = __builtin_ctzl(sec_hitmask[i]);
uint32_t key_idx = secondary_bkt[i]->key_idx[first_hit];
const struct rte_hash_key *key_slot =
- (const struct rte_hash_key *)(
- (const char *)h->key_store +
- key_idx * h->key_entry_size);
+ __rte_hash_key_from_idx(h, key_idx);
rte_prefetch0(key_slot);
}
}
@@ -1063,9 +1066,7 @@ struct rte_hash *
uint32_t key_idx = primary_bkt[i]->key_idx[hit_index];
const struct rte_hash_key *key_slot =
- (const struct rte_hash_key *)(
- (const char *)h->key_store +
- key_idx * h->key_entry_size);
+ __rte_hash_key_from_idx(h, key_idx);
/*
* If key index is 0, do not compare key,
* as it is checking the dummy slot
@@ -1086,9 +1087,7 @@ struct rte_hash *
uint32_t key_idx = secondary_bkt[i]->key_idx[hit_index];
const struct rte_hash_key *key_slot =
- (const struct rte_hash_key *)(
- (const char *)h->key_store +
- key_idx * h->key_entry_size);
+ __rte_hash_key_from_idx(h, key_idx);
/*
* If key index is 0, do not compare key,
* as it is checking the dummy slot
@@ -1170,8 +1169,7 @@ struct rte_hash *
/* Get position of entry in key table */
position = h->buckets[bucket_idx].key_idx[idx];
- next_key = (struct rte_hash_key *) ((char *)h->key_store +
- position * h->key_entry_size);
+ next_key = __rte_hash_key_from_idx(h, position);
/* Return key and data */
*key = next_key->key;
*data = next_key->pdata;
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
2017-08-18 20:09 [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key mstolarchuk
@ 2017-10-17 13:59 ` Thomas Monjalon
2017-10-19 8:10 ` De Lara Guarch, Pablo
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2017-10-17 13:59 UTC (permalink / raw)
To: bruce.richardson, pablo.de.lara.guarch; +Cc: dev, mstolarchuk
18/08/2017 22:09, mstolarchuk:
> Improves consistency, allows identifcation of use-sites
>
> Signed-off-by: mstolarchuk <mike.stolarchuk@bigswitch.com>
Any comment on this patch and others from the same author?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
2017-10-17 13:59 ` Thomas Monjalon
@ 2017-10-19 8:10 ` De Lara Guarch, Pablo
2017-10-27 15:00 ` Mike Stolarchuk
0 siblings, 1 reply; 6+ messages in thread
From: De Lara Guarch, Pablo @ 2017-10-19 8:10 UTC (permalink / raw)
To: Thomas Monjalon, Richardson, Bruce; +Cc: dev, mstolarchuk
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, October 17, 2017 2:59 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch,
> Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; mstolarchuk <mike.stolarchuk@bigswitch.com>
> Subject: Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
>
> 18/08/2017 22:09, mstolarchuk:
> > Improves consistency, allows identifcation of use-sites
> >
> > Signed-off-by: mstolarchuk <mike.stolarchuk@bigswitch.com>
>
> Any comment on this patch and others from the same author?
Hi,
Two of the patches submitted are actually the same, although they have a different enumeration.
The patches look like they were part of two different patchsets, so it doesn't look right.
Also, patch 6/6 is not applicable anymore, as a similar fix was sent previously.
Mike, could you send another patchset, that is rebased on top of the latest code?
Thanks,
Pablo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
2017-10-19 8:10 ` De Lara Guarch, Pablo
@ 2017-10-27 15:00 ` Mike Stolarchuk
2017-10-27 15:31 ` De Lara Guarch, Pablo
0 siblings, 1 reply; 6+ messages in thread
From: Mike Stolarchuk @ 2017-10-27 15:00 UTC (permalink / raw)
To: De Lara Guarch, Pablo; +Cc: Thomas Monjalon, Richardson, Bruce, dev
Pablo,
Also, what about the other patch?
the use of a static variable in a recursive call?
obviously incorrect for a threaded environment ... has that been accepted?
regards,
mts.
On Thu, Oct 19, 2017 at 4:10 AM, De Lara Guarch, Pablo <
pablo.de.lara.guarch@intel.com> wrote:
>
>
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Tuesday, October 17, 2017 2:59 PM
> > To: Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch,
> > Pablo <pablo.de.lara.guarch@intel.com>
> > Cc: dev@dpdk.org; mstolarchuk <mike.stolarchuk@bigswitch.com>
> > Subject: Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
> >
> > 18/08/2017 22:09, mstolarchuk:
> > > Improves consistency, allows identifcation of use-sites
> > >
> > > Signed-off-by: mstolarchuk <mike.stolarchuk@bigswitch.com>
> >
> > Any comment on this patch and others from the same author?
>
> Hi,
>
> Two of the patches submitted are actually the same, although they have a
> different enumeration.
> The patches look like they were part of two different patchsets, so it
> doesn't look right.
> Also, patch 6/6 is not applicable anymore, as a similar fix was sent
> previously.
>
> Mike, could you send another patchset, that is rebased on top of the
> latest code?
>
> Thanks,
> Pablo
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
2017-10-27 15:00 ` Mike Stolarchuk
@ 2017-10-27 15:31 ` De Lara Guarch, Pablo
2019-03-01 17:15 ` Ferruh Yigit
0 siblings, 1 reply; 6+ messages in thread
From: De Lara Guarch, Pablo @ 2017-10-27 15:31 UTC (permalink / raw)
To: Mike Stolarchuk; +Cc: Thomas Monjalon, Richardson, Bruce, dev
> From: Mike Stolarchuk [mailto:mike.stolarchuk@bigswitch.com]
> Sent: Friday, October 27, 2017 4:00 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Richardson, Bruce
> <bruce.richardson@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
>
> Pablo,
>
> Also, what about the other patch?
> the use of a static variable in a recursive call? obviously incorrect for a
> threaded environment ... has that been accepted?
Hi Mike,
Yes, that patch was accepted.
Thanks,
Pablo
>
> regards,
> mts.
>
>
>
> On Thu, Oct 19, 2017 at 4:10 AM, De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com> wrote:
>
>
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Tuesday, October 17, 2017 2:59 PM
> > To: Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch,
> > Pablo <pablo.de.lara.guarch@intel.com>
> > Cc: dev@dpdk.org; mstolarchuk <mike.stolarchuk@bigswitch.com>
> > Subject: Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
> >
> > 18/08/2017 22:09, mstolarchuk:
> > > Improves consistency, allows identifcation of use-sites
> > >
> > > Signed-off-by: mstolarchuk <mike.stolarchuk@bigswitch.com>
> >
> > Any comment on this patch and others from the same author?
> Hi,
>
> Two of the patches submitted are actually the same, although they have a
> different enumeration.
> The patches look like they were part of two different patchsets, so it doesn't
> look right.
> Also, patch 6/6 is not applicable anymore, as a similar fix was sent
> previously.
>
> Mike, could you send another patchset, that is rebased on top of the latest
> code?
>
> Thanks,
> Pablo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
2017-10-27 15:31 ` De Lara Guarch, Pablo
@ 2019-03-01 17:15 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2019-03-01 17:15 UTC (permalink / raw)
To: Pablo de Lara, Mike Stolarchuk
Cc: Thomas Monjalon, Bruce Richardson, dpdk-dev
On 10/27/2017 4:31 PM, pablo.de.lara.guarch at intel.com (De Lara Guarch, Pablo)
wrote:
>> From: Mike Stolarchuk [mailto:mike.stolarchuk at bigswitch.com]
>> Sent: Friday, October 27, 2017 4:00 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>
>> Cc: Thomas Monjalon <thomas at monjalon.net>; Richardson, Bruce
>> <bruce.richardson at intel.com>; dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
>>
>> Pablo,
>>
>> Also, what about the other patch?
>> the use of a static variable in a recursive call? obviously incorrect for a
>> threaded environment ... has that been accepted?
>
> Hi Mike,
>
> Yes, that patch was accepted.
Hi Pablo, Mike,
The patch is remaining from 2017 and sitting on the patchwork without any comment.
I am marking the patchset as rejected, if they are still relevant please send a
new version on top of latest repo.
Sorry for any inconvenience caused.
For reference patches:
https://patches.dpdk.org/patch/27668/
https://patches.dpdk.org/patch/27669/
https://patches.dpdk.org/patch/27670/
https://patches.dpdk.org/patch/27671/
>
> Thanks,
> Pablo
>
>>
>> regards,
>> mts.
>>
>>
>>
>> On Thu, Oct 19, 2017 at 4:10 AM, De Lara Guarch, Pablo
>> <pablo.de.lara.guarch at intel.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: Thomas Monjalon [mailto:thomas at monjalon.net]
>>> Sent: Tuesday, October 17, 2017 2:59 PM
>>> To: Richardson, Bruce <bruce.richardson at intel.com>; De Lara Guarch,
>>> Pablo <pablo.de.lara.guarch at intel.com>
>>> Cc: dev at dpdk.org; mstolarchuk <mike.stolarchuk at bigswitch.com>
>>> Subject: Re: [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key
>>>
>>> 18/08/2017 22:09, mstolarchuk:
>>>> Improves consistency, allows identifcation of use-sites
>>>>
>>>> Signed-off-by: mstolarchuk <mike.stolarchuk at bigswitch.com>
>>>
>>> Any comment on this patch and others from the same author?
>> Hi,
>>
>> Two of the patches submitted are actually the same, although they have a
>> different enumeration.
>> The patches look like they were part of two different patchsets, so it doesn't
>> look right.
>> Also, patch 6/6 is not applicable anymore, as a similar fix was sent
>> previously.
>>
>> Mike, could you send another patchset, that is rebased on top of the latest
>> code?
>>
>> Thanks,
>> Pablo
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-01 17:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-18 20:09 [dpdk-dev] [PATCH 1/7] Use an accessor for rte_hash_key mstolarchuk
2017-10-17 13:59 ` Thomas Monjalon
2017-10-19 8:10 ` De Lara Guarch, Pablo
2017-10-27 15:00 ` Mike Stolarchuk
2017-10-27 15:31 ` De Lara Guarch, Pablo
2019-03-01 17:15 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).