* [dpdk-dev] [PATCH v2] hash: added a new API to hash to query key id @ 2019-11-22 14:59 Kumar Amber 2019-11-22 18:21 ` [dpdk-dev] [PATCH v3] " Kumar Amber 0 siblings, 1 reply; 19+ messages in thread From: Kumar Amber @ 2019-11-22 14:59 UTC (permalink / raw) To: dev; +Cc: yipeng1.wang Adding new API function to query the maximum key ID that could possibly returned by add_key_functions. When multi_key_add flag set, the maximum key id is larger than the entry count specified by the user. Signed-off-by: Kumar Amber <kumar.amber@intel.com> --- lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- lib/librte_hash/rte_hash_version.map | 1 + 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 87a4c01f2..3a94f10b8 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) return h->hash_func(key, h->key_len, h->hash_func_init_val); } +uint32_t +rte_hash_max_key_id(const struct rte_hash *h) +{ + RETURN_IF_TRUE((h == NULL), -EINVAL); + if (h->use_local_cache) + /* + * Increase number of slots by total number of indices + * that can be stored in the lcore caches + */ + return (h->entries + ((RTE_MAX_LCORE - 1) * + (LCORE_CACHE_SIZE - 1))); + else + return h->entries; +} + int32_t rte_hash_count(const struct rte_hash *h) { diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 0d73370dc..c87861e72 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); int32_t rte_hash_count(const struct rte_hash *h); +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice + * + * Return the maximum key value ID that could possibly be returned by + * rte_hash_add_key function. + * + * @param h + * Hash table to query from + * @return + * - -EINVAL if parameters are invalid + * - A value indicating the max key Id key slots present in the table. + */ +__rte_experimental +uint32_t +rte_hash_max_key_id(const struct rte_hash *h); + /** * Add a key-value pair to an existing hash table. * This operation is not multi-thread safe @@ -234,7 +251,9 @@ rte_hash_add_key_with_hash_data(const struct rte_hash *h, const void *key, * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key id may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key(const struct rte_hash *h, const void *key); @@ -256,7 +275,9 @@ rte_hash_add_key(const struct rte_hash *h, const void *key); * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key ID may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map index 734ae28b0..562ceb8bc 100644 --- a/lib/librte_hash/rte_hash_version.map +++ b/lib/librte_hash/rte_hash_version.map @@ -58,5 +58,6 @@ EXPERIMENTAL { global: rte_hash_free_key_with_position; + rte_hash_max_key_id; }; -- 2.17.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-22 14:59 [dpdk-dev] [PATCH v2] hash: added a new API to hash to query key id Kumar Amber @ 2019-11-22 18:21 ` Kumar Amber 2019-11-25 11:08 ` Kumar Amber 2019-11-26 2:39 ` [dpdk-dev] [PATCH v4] " Kumar Amber 0 siblings, 2 replies; 19+ messages in thread From: Kumar Amber @ 2019-11-22 18:21 UTC (permalink / raw) To: dev; +Cc: yipeng1.wang Adding new API function to query the maximum key ID that could possibly returned by rte_hash_add_key and rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD is set, the maximum key id is larger than the entry count specified by the user. Signed-off-by: Kumar Amber <kumar.amber@intel.com> --- lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- lib/librte_hash/rte_hash_version.map | 1 + 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 87a4c01f2..3a94f10b8 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) return h->hash_func(key, h->key_len, h->hash_func_init_val); } +uint32_t +rte_hash_max_key_id(const struct rte_hash *h) +{ + RETURN_IF_TRUE((h == NULL), -EINVAL); + if (h->use_local_cache) + /* + * Increase number of slots by total number of indices + * that can be stored in the lcore caches + */ + return (h->entries + ((RTE_MAX_LCORE - 1) * + (LCORE_CACHE_SIZE - 1))); + else + return h->entries; +} + int32_t rte_hash_count(const struct rte_hash *h) { diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 0d73370dc..c87861e72 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); int32_t rte_hash_count(const struct rte_hash *h); +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice + * + * Return the maximum key value ID that could possibly be returned by + * rte_hash_add_key function. + * + * @param h + * Hash table to query from + * @return + * - -EINVAL if parameters are invalid + * - A value indicating the max key Id key slots present in the table. + */ +__rte_experimental +uint32_t +rte_hash_max_key_id(const struct rte_hash *h); + /** * Add a key-value pair to an existing hash table. * This operation is not multi-thread safe @@ -234,7 +251,9 @@ rte_hash_add_key_with_hash_data(const struct rte_hash *h, const void *key, * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key id may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key(const struct rte_hash *h, const void *key); @@ -256,7 +275,9 @@ rte_hash_add_key(const struct rte_hash *h, const void *key); * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key ID may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map index 734ae28b0..562ceb8bc 100644 --- a/lib/librte_hash/rte_hash_version.map +++ b/lib/librte_hash/rte_hash_version.map @@ -58,5 +58,6 @@ EXPERIMENTAL { global: rte_hash_free_key_with_position; + rte_hash_max_key_id; }; -- 2.17.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-22 18:21 ` [dpdk-dev] [PATCH v3] " Kumar Amber @ 2019-11-25 11:08 ` Kumar Amber 2019-11-25 15:50 ` Amber, Kumar 2019-11-25 19:41 ` Wang, Yipeng1 2019-11-26 2:39 ` [dpdk-dev] [PATCH v4] " Kumar Amber 1 sibling, 2 replies; 19+ messages in thread From: Kumar Amber @ 2019-11-25 11:08 UTC (permalink / raw) To: dev; +Cc: yipeng1.wang Adding new API function to query the maximum key ID that could possibly returned by rte_hash_add_key and rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD is set, the maximum key id is larger than the entry count specified by the user. Signed-off-by: Kumar Amber <kumar.amber@intel.com> --- lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- lib/librte_hash/rte_hash_version.map | 1 + 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 87a4c01f2..3a94f10b8 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) return h->hash_func(key, h->key_len, h->hash_func_init_val); } +uint32_t +rte_hash_max_key_id(const struct rte_hash *h) +{ + RETURN_IF_TRUE((h == NULL), -EINVAL); + if (h->use_local_cache) + /* + * Increase number of slots by total number of indices + * that can be stored in the lcore caches + */ + return (h->entries + ((RTE_MAX_LCORE - 1) * + (LCORE_CACHE_SIZE - 1))); + else + return h->entries; +} + int32_t rte_hash_count(const struct rte_hash *h) { diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 0d73370dc..c87861e72 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); int32_t rte_hash_count(const struct rte_hash *h); +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice + * + * Return the maximum key value ID that could possibly be returned by + * rte_hash_add_key function. + * + * @param h + * Hash table to query from + * @return + * - -EINVAL if parameters are invalid + * - A value indicating the max key Id key slots present in the table. + */ +__rte_experimental +uint32_t +rte_hash_max_key_id(const struct rte_hash *h); + /** * Add a key-value pair to an existing hash table. * This operation is not multi-thread safe @@ -234,7 +251,9 @@ rte_hash_add_key_with_hash_data(const struct rte_hash *h, const void *key, * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key id may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key(const struct rte_hash *h, const void *key); @@ -256,7 +275,9 @@ rte_hash_add_key(const struct rte_hash *h, const void *key); * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key ID may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map index 734ae28b0..562ceb8bc 100644 --- a/lib/librte_hash/rte_hash_version.map +++ b/lib/librte_hash/rte_hash_version.map @@ -58,5 +58,6 @@ EXPERIMENTAL { global: rte_hash_free_key_with_position; + rte_hash_max_key_id; }; -- 2.17.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 11:08 ` Kumar Amber @ 2019-11-25 15:50 ` Amber, Kumar 2019-11-25 16:01 ` Aaron Conole 2019-11-25 16:58 ` Aaron Conole 2019-11-25 19:41 ` Wang, Yipeng1 1 sibling, 2 replies; 19+ messages in thread From: Amber, Kumar @ 2019-11-25 15:50 UTC (permalink / raw) To: dev Cc: Wang, Yipeng1, Yigit, Ferruh, thomas, Van Haaren, Harry, aconole, Thakur, Sham Singh Hi all , I want to report random untouched unit test cases failed by Jenkins . pls can you guys check why it is unstable ? Every upload shows random test failed . http://mails.dpdk.org/archives/test-report/2019-November/109120.html Regards Amber -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of Kumar Amber Sent: Monday, November 25, 2019 4:38 PM To: dev@dpdk.org Cc: Wang, Yipeng1 <yipeng1.wang@intel.com> Subject: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id Adding new API function to query the maximum key ID that could possibly returned by rte_hash_add_key and rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD is set, the maximum key id is larger than the entry count specified by the user. Signed-off-by: Kumar Amber <kumar.amber@intel.com> --- lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- lib/librte_hash/rte_hash_version.map | 1 + 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 87a4c01f2..3a94f10b8 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) return h->hash_func(key, h->key_len, h->hash_func_init_val); } +uint32_t +rte_hash_max_key_id(const struct rte_hash *h) { + RETURN_IF_TRUE((h == NULL), -EINVAL); + if (h->use_local_cache) + /* + * Increase number of slots by total number of indices + * that can be stored in the lcore caches + */ + return (h->entries + ((RTE_MAX_LCORE - 1) * + (LCORE_CACHE_SIZE - 1))); + else + return h->entries; +} + int32_t rte_hash_count(const struct rte_hash *h) { diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 0d73370dc..c87861e72 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); int32_t rte_hash_count(const struct rte_hash *h); +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice + * + * Return the maximum key value ID that could possibly be returned by + * rte_hash_add_key function. + * + * @param h + * Hash table to query from + * @return + * - -EINVAL if parameters are invalid + * - A value indicating the max key Id key slots present in the table. + */ +__rte_experimental +uint32_t +rte_hash_max_key_id(const struct rte_hash *h); + /** * Add a key-value pair to an existing hash table. * This operation is not multi-thread safe @@ -234,7 +251,9 @@ rte_hash_add_key_with_hash_data(const struct rte_hash *h, const void *key, * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key id may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key(const struct rte_hash *h, const void *key); @@ -256,7 +275,9 @@ rte_hash_add_key(const struct rte_hash *h, const void *key); * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key ID may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map index 734ae28b0..562ceb8bc 100644 --- a/lib/librte_hash/rte_hash_version.map +++ b/lib/librte_hash/rte_hash_version.map @@ -58,5 +58,6 @@ EXPERIMENTAL { global: rte_hash_free_key_with_position; + rte_hash_max_key_id; }; -- 2.17.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 15:50 ` Amber, Kumar @ 2019-11-25 16:01 ` Aaron Conole 2019-11-25 16:58 ` Aaron Conole 1 sibling, 0 replies; 19+ messages in thread From: Aaron Conole @ 2019-11-25 16:01 UTC (permalink / raw) To: Amber, Kumar Cc: dev, Wang, Yipeng1, Yigit, Ferruh, thomas, Van Haaren, Harry, Thakur, Sham Singh "Amber, Kumar" <kumar.amber@intel.com> writes: > Hi all , > > I want to report random untouched unit test cases failed by Jenkins > . pls can you guys check why it is unstable ? > Every upload shows random test failed . I'll work on the service_autotest patch. > http://mails.dpdk.org/archives/test-report/2019-November/109120.html > > Regards > Amber > > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Kumar Amber > Sent: Monday, November 25, 2019 4:38 PM > To: dev@dpdk.org > Cc: Wang, Yipeng1 <yipeng1.wang@intel.com> > Subject: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id > > Adding new API function to query the maximum key ID that could possibly returned by rte_hash_add_key and rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD > is set, the maximum key id is larger than the entry count specified by the user. > > Signed-off-by: Kumar Amber <kumar.amber@intel.com> > --- > lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ > lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- > lib/librte_hash/rte_hash_version.map | 1 + > 3 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c > index 87a4c01f2..3a94f10b8 100644 > --- a/lib/librte_hash/rte_cuckoo_hash.c > +++ b/lib/librte_hash/rte_cuckoo_hash.c > @@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) > return h->hash_func(key, h->key_len, h->hash_func_init_val); } > > +uint32_t > +rte_hash_max_key_id(const struct rte_hash *h) { > + RETURN_IF_TRUE((h == NULL), -EINVAL); > + if (h->use_local_cache) > + /* > + * Increase number of slots by total number of indices > + * that can be stored in the lcore caches > + */ > + return (h->entries + ((RTE_MAX_LCORE - 1) * > + (LCORE_CACHE_SIZE - 1))); > + else > + return h->entries; > +} > + > int32_t > rte_hash_count(const struct rte_hash *h) { diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 0d73370dc..c87861e72 100644 > --- a/lib/librte_hash/rte_hash.h > +++ b/lib/librte_hash/rte_hash.h > @@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); int32_t rte_hash_count(const struct rte_hash *h); > > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice > + * > + * Return the maximum key value ID that could possibly be returned by > + * rte_hash_add_key function. > + * > + * @param h > + * Hash table to query from > + * @return > + * - -EINVAL if parameters are invalid > + * - A value indicating the max key Id key slots present in the table. > + */ > +__rte_experimental > +uint32_t > +rte_hash_max_key_id(const struct rte_hash *h); > + > /** > * Add a key-value pair to an existing hash table. > * This operation is not multi-thread safe @@ -234,7 +251,9 @@ rte_hash_add_key_with_hash_data(const struct rte_hash *h, const void *key, > * - -EINVAL if the parameters are invalid. > * - -ENOSPC if there is no space in the hash for this key. > * - A positive value that can be used by the caller as an offset into an > - * array of user data. This value is unique for this key. > + * array of user data. This value is unique for this key. This > + * unique key id may be larger than the user specified entry count > + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. > */ > int32_t > rte_hash_add_key(const struct rte_hash *h, const void *key); @@ -256,7 +275,9 @@ rte_hash_add_key(const struct rte_hash *h, const void *key); > * - -EINVAL if the parameters are invalid. > * - -ENOSPC if there is no space in the hash for this key. > * - A positive value that can be used by the caller as an offset into an > - * array of user data. This value is unique for this key. > + * array of user data. This value is unique for this key. This > + * unique key ID may be larger than the user specified entry count > + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. > */ > int32_t > rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map > index 734ae28b0..562ceb8bc 100644 > --- a/lib/librte_hash/rte_hash_version.map > +++ b/lib/librte_hash/rte_hash_version.map > @@ -58,5 +58,6 @@ EXPERIMENTAL { > global: > > rte_hash_free_key_with_position; > + rte_hash_max_key_id; > > }; > -- > 2.17.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 15:50 ` Amber, Kumar 2019-11-25 16:01 ` Aaron Conole @ 2019-11-25 16:58 ` Aaron Conole 2019-11-25 17:16 ` Van Haaren, Harry 1 sibling, 1 reply; 19+ messages in thread From: Aaron Conole @ 2019-11-25 16:58 UTC (permalink / raw) To: Amber, Kumar Cc: dev, Wang, Yipeng1, Yigit, Ferruh, thomas, Van Haaren, Harry, Thakur, Sham Singh, David Marchand "Amber, Kumar" <kumar.amber@intel.com> writes: > Hi all , > > I want to report random untouched unit test cases failed by Jenkins > . pls can you guys check why it is unstable ? > Every upload shows random test failed . > > http://mails.dpdk.org/archives/test-report/2019-November/109120.html I think the service core code needs the following patch: --- diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 79235c03f8..f2efb8c4a5 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -383,7 +383,7 @@ rte_service_may_be_active(uint32_t id) int32_t lcore_count = rte_service_lcore_list(ids, RTE_MAX_LCORE); int i; - if (!service_valid(id)) + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) return -EINVAL; for (i = 0; i < lcore_count; i++) { @@ -398,7 +398,7 @@ int32_t rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe) { /* run service on calling core, using all-ones as the service mask */ - if (!service_valid(id)) + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) return -EINVAL; struct core_state *cs = &lcore_states[rte_lcore_id()]; --- Harry? > Regards > Amber > > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Kumar Amber > Sent: Monday, November 25, 2019 4:38 PM > To: dev@dpdk.org > Cc: Wang, Yipeng1 <yipeng1.wang@intel.com> > Subject: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id > > Adding new API function to query the maximum key ID that could possibly returned by rte_hash_add_key and rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD > is set, the maximum key id is larger than the entry count specified by the user. > > Signed-off-by: Kumar Amber <kumar.amber@intel.com> > --- > lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ > lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- > lib/librte_hash/rte_hash_version.map | 1 + > 3 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c > index 87a4c01f2..3a94f10b8 100644 > --- a/lib/librte_hash/rte_cuckoo_hash.c > +++ b/lib/librte_hash/rte_cuckoo_hash.c > @@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) > return h->hash_func(key, h->key_len, h->hash_func_init_val); } > > +uint32_t > +rte_hash_max_key_id(const struct rte_hash *h) { > + RETURN_IF_TRUE((h == NULL), -EINVAL); > + if (h->use_local_cache) > + /* > + * Increase number of slots by total number of indices > + * that can be stored in the lcore caches > + */ > + return (h->entries + ((RTE_MAX_LCORE - 1) * > + (LCORE_CACHE_SIZE - 1))); > + else > + return h->entries; > +} > + > int32_t > rte_hash_count(const struct rte_hash *h) { diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 0d73370dc..c87861e72 100644 > --- a/lib/librte_hash/rte_hash.h > +++ b/lib/librte_hash/rte_hash.h > @@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); int32_t rte_hash_count(const struct rte_hash *h); > > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice > + * > + * Return the maximum key value ID that could possibly be returned by > + * rte_hash_add_key function. > + * > + * @param h > + * Hash table to query from > + * @return > + * - -EINVAL if parameters are invalid > + * - A value indicating the max key Id key slots present in the table. > + */ > +__rte_experimental > +uint32_t > +rte_hash_max_key_id(const struct rte_hash *h); > + > /** > * Add a key-value pair to an existing hash table. > * This operation is not multi-thread safe @@ -234,7 +251,9 @@ rte_hash_add_key_with_hash_data(const struct rte_hash *h, const void *key, > * - -EINVAL if the parameters are invalid. > * - -ENOSPC if there is no space in the hash for this key. > * - A positive value that can be used by the caller as an offset into an > - * array of user data. This value is unique for this key. > + * array of user data. This value is unique for this key. This > + * unique key id may be larger than the user specified entry count > + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. > */ > int32_t > rte_hash_add_key(const struct rte_hash *h, const void *key); @@ -256,7 +275,9 @@ rte_hash_add_key(const struct rte_hash *h, const void *key); > * - -EINVAL if the parameters are invalid. > * - -ENOSPC if there is no space in the hash for this key. > * - A positive value that can be used by the caller as an offset into an > - * array of user data. This value is unique for this key. > + * array of user data. This value is unique for this key. This > + * unique key ID may be larger than the user specified entry count > + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. > */ > int32_t > rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map > index 734ae28b0..562ceb8bc 100644 > --- a/lib/librte_hash/rte_hash_version.map > +++ b/lib/librte_hash/rte_hash_version.map > @@ -58,5 +58,6 @@ EXPERIMENTAL { > global: > > rte_hash_free_key_with_position; > + rte_hash_max_key_id; > > }; > -- > 2.17.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 16:58 ` Aaron Conole @ 2019-11-25 17:16 ` Van Haaren, Harry 2019-11-25 17:54 ` Thomas Monjalon 0 siblings, 1 reply; 19+ messages in thread From: Van Haaren, Harry @ 2019-11-25 17:16 UTC (permalink / raw) To: Aaron Conole, Amber, Kumar Cc: dev, Wang, Yipeng1, Yigit, Ferruh, thomas, Thakur, Sham Singh, David Marchand Hey Folks, > -----Original Message----- > From: Aaron Conole <aconole@redhat.com> > Sent: Monday, November 25, 2019 4:59 PM > To: Amber, Kumar <kumar.amber@intel.com> > Cc: dev@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>; Yigit, Ferruh > <ferruh.yigit@intel.com>; thomas@monjalon.net; Van Haaren, Harry > <harry.van.haaren@intel.com>; Thakur, Sham Singh > <sham.singh.thakur@intel.com>; David Marchand <dmarchan@redhat.com> > Subject: Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query > key id > > "Amber, Kumar" <kumar.amber@intel.com> writes: > > > Hi all , > > > > I want to report random untouched unit test cases failed by Jenkins > > . pls can you guys check why it is unstable ? > > Every upload shows random test failed . > > > > http://mails.dpdk.org/archives/test-report/2019-November/109120.html > > I think the service core code needs the following patch: > --- > diff --git a/lib/librte_eal/common/rte_service.c > b/lib/librte_eal/common/rte_service.c > index 79235c03f8..f2efb8c4a5 100644 > --- a/lib/librte_eal/common/rte_service.c > +++ b/lib/librte_eal/common/rte_service.c > @@ -383,7 +383,7 @@ rte_service_may_be_active(uint32_t id) > int32_t lcore_count = rte_service_lcore_list(ids, RTE_MAX_LCORE); > int i; > > - if (!service_valid(id)) > + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) > return -EINVAL; > > for (i = 0; i < lcore_count; i++) { > @@ -398,7 +398,7 @@ int32_t > rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t > serialize_mt_unsafe) > { > /* run service on calling core, using all-ones as the service mask */ > - if (!service_valid(id)) > + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) > return -EINVAL; > > struct core_state *cs = &lcore_states[rte_lcore_id()]; > --- > > Harry? Apologies - I've been AFK and hence this made no progress in the last weeks. Thanks Aaron for the suggested patch, I'll review/test now and report back tomorrow. <snip unrelated patch code> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 17:16 ` Van Haaren, Harry @ 2019-11-25 17:54 ` Thomas Monjalon 2019-11-25 18:10 ` Aaron Conole 0 siblings, 1 reply; 19+ messages in thread From: Thomas Monjalon @ 2019-11-25 17:54 UTC (permalink / raw) To: Van Haaren, Harry, Aaron Conole Cc: Amber, Kumar, dev, Wang, Yipeng1, Yigit, Ferruh, Thakur, Sham Singh, David Marchand > From: Aaron Conole <aconole@redhat.com> > > - if (!service_valid(id)) > > + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) Why not adding this check in service_valid()? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 17:54 ` Thomas Monjalon @ 2019-11-25 18:10 ` Aaron Conole 2019-11-25 22:53 ` Aaron Conole 0 siblings, 1 reply; 19+ messages in thread From: Aaron Conole @ 2019-11-25 18:10 UTC (permalink / raw) To: Thomas Monjalon Cc: Van Haaren, Harry, Amber, Kumar, dev, Wang, Yipeng1, Yigit, Ferruh, Thakur, Sham Singh, David Marchand Thomas Monjalon <thomas@monjalon.net> writes: >> From: Aaron Conole <aconole@redhat.com> >> > - if (!service_valid(id)) >> > + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) > > Why not adding this check in service_valid()? I think the best fix is to use SERVICE_VALID_GET_OR_ERR_RET() in these places. For this, I at least want to try and show that there aren't any further errors. And my test loop has been running for a while now without any more errors or segfaults, so I guess it's okay to build a proper patch. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 18:10 ` Aaron Conole @ 2019-11-25 22:53 ` Aaron Conole 2019-11-26 13:19 ` Van Haaren, Harry 0 siblings, 1 reply; 19+ messages in thread From: Aaron Conole @ 2019-11-25 22:53 UTC (permalink / raw) To: Thomas Monjalon Cc: Van Haaren, Harry, Amber, Kumar, dev, Wang, Yipeng1, Yigit, Ferruh, Thakur, Sham Singh, David Marchand Aaron Conole <aconole@redhat.com> writes: > Thomas Monjalon <thomas@monjalon.net> writes: > >>> From: Aaron Conole <aconole@redhat.com> >>> > - if (!service_valid(id)) >>> > + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) >> >> Why not adding this check in service_valid()? > > I think the best fix is to use SERVICE_VALID_GET_OR_ERR_RET() in these > places. For this, I at least want to try and show that there aren't any > further errors. And my test loop has been running for a while now > without any more errors or segfaults, so I guess it's okay to build a > proper patch. This popped up: EAL: Test assert service_lcore_en_dis_able line 487 failed: Ex-service core function call had no effect. So I'll spend some time in this area, it seems. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 22:53 ` Aaron Conole @ 2019-11-26 13:19 ` Van Haaren, Harry 2019-11-26 13:29 ` Van Haaren, Harry 2019-11-26 15:58 ` Aaron Conole 0 siblings, 2 replies; 19+ messages in thread From: Van Haaren, Harry @ 2019-11-26 13:19 UTC (permalink / raw) To: Aaron Conole, Thomas Monjalon Cc: Amber, Kumar, dev, Wang, Yipeng1, Yigit, Ferruh, Thakur, Sham Singh, David Marchand Hi Aaron, > -----Original Message----- > From: Aaron Conole <aconole@redhat.com> > Sent: Monday, November 25, 2019 10:54 PM > To: Thomas Monjalon <thomas@monjalon.net> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Amber, Kumar > <kumar.amber@intel.com>; dev@dpdk.org; Wang, Yipeng1 > <yipeng1.wang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Thakur, > Sham Singh <sham.singh.thakur@intel.com>; David Marchand > <dmarchan@redhat.com> > Subject: Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query > key id > > Aaron Conole <aconole@redhat.com> writes: > > > Thomas Monjalon <thomas@monjalon.net> writes: > > > >>> From: Aaron Conole <aconole@redhat.com> > >>> > - if (!service_valid(id)) > >>> > + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) > >> > >> Why not adding this check in service_valid()? > > > > I think the best fix is to use SERVICE_VALID_GET_OR_ERR_RET() in these > > places. For this, I at least want to try and show that there aren't any > > further errors. And my test loop has been running for a while now > > without any more errors or segfaults, so I guess it's okay to build a > > proper patch. > > This popped up: > > EAL: Test assert service_lcore_en_dis_able line 487 failed: Ex-service core > function call had no effect. > > So I'll spend some time in this area, it seems. The below diff makes it 100% reproducible here, failing every time. It seems like the main thread is returning, before the service thread has returned. The rte_eal_mp_wait_lcore() call seems to not wait on the service-core, which allows the main thread to read the "service_remote_launch_flag" value as 0 (before the service-thread writes it to 1). Adding the delay between the service launch and service write being performed makes this issue much much more likely to occur - so the above description I have confidence in. What I'm not clear on (yet) is why the eal_mp_wait_lcore() isn't waiting... -H diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c index 9fe38f5e0..846ad00d1 100644 --- a/app/test/test_service_cores.c +++ b/app/test/test_service_cores.c @@ -445,6 +445,7 @@ static int service_remote_launch_func(void *arg) { RTE_SET_USED(arg); + rte_delay_ms(100); service_remote_launch_flag = 1; return 0; } ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-26 13:19 ` Van Haaren, Harry @ 2019-11-26 13:29 ` Van Haaren, Harry 2019-11-26 13:57 ` Aaron Conole 2019-11-26 15:58 ` Aaron Conole 1 sibling, 1 reply; 19+ messages in thread From: Van Haaren, Harry @ 2019-11-26 13:29 UTC (permalink / raw) To: Aaron Conole, Thomas Monjalon Cc: Amber, Kumar, dev, Wang, Yipeng1, Yigit, Ferruh, Thakur, Sham Singh, David Marchand > -----Original Message----- > From: Van Haaren, Harry > Sent: Tuesday, November 26, 2019 1:19 PM > To: Aaron Conole <aconole@redhat.com>; Thomas Monjalon <thomas@monjalon.net> <snip> > > EAL: Test assert service_lcore_en_dis_able line 487 failed: Ex-service > core > > function call had no effect. > > > > So I'll spend some time in this area, it seems. > > > The below diff makes it 100% reproducible here, failing every time. > > It seems like the main thread is returning, before the service thread has > returned. > > The rte_eal_mp_wait_lcore() call seems to not wait on the service-core, > which allows > the main thread to read the "service_remote_launch_flag" value as 0 (before > the service-thread writes it to 1). > > Adding the delay between the service launch and service write being > performed makes this issue much much more likely to occur - so the above > description I have confidence in. > > What I'm not clear on (yet) is why the eal_mp_wait_lcore() isn't waiting... > > -H > > > diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c > index 9fe38f5e0..846ad00d1 100644 > --- a/app/test/test_service_cores.c > +++ b/app/test/test_service_cores.c > @@ -445,6 +445,7 @@ static int > service_remote_launch_func(void *arg) > { > RTE_SET_USED(arg); > + rte_delay_ms(100); > service_remote_launch_flag = 1; > return 0; > } Diff below seems to fix the problem here; Aaron would you test the below fix in your setup for a while too? I have a loop running here attempting to reproduce - but before 100% failures and so far 100% passes with the added wait_lcore() call. diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c index 9fe38f5e0..62ffedb19 100644 --- a/app/test/test_service_cores.c +++ b/app/test/test_service_cores.c @@ -445,6 +445,7 @@ static int service_remote_launch_func(void *arg) { RTE_SET_USED(arg); + rte_delay_ms(100); service_remote_launch_flag = 1; return 0; } @@ -483,6 +484,7 @@ service_lcore_en_dis_able(void) int ret = rte_eal_remote_launch(service_remote_launch_func, NULL, slcore_id); TEST_ASSERT_EQUAL(0, ret, "Ex-service core remote launch failed."); + rte_eal_wait_lcore(slcore_id); rte_eal_mp_wait_lcore(); TEST_ASSERT_EQUAL(1, service_remote_launch_flag, "Ex-service core function call had no effect."); ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-26 13:29 ` Van Haaren, Harry @ 2019-11-26 13:57 ` Aaron Conole 2019-11-27 11:37 ` Van Haaren, Harry 0 siblings, 1 reply; 19+ messages in thread From: Aaron Conole @ 2019-11-26 13:57 UTC (permalink / raw) To: Van Haaren, Harry Cc: Thomas Monjalon, Amber, Kumar, dev, Wang, Yipeng1, Yigit, Ferruh, Thakur, Sham Singh, David Marchand "Van Haaren, Harry" <harry.van.haaren@intel.com> writes: >> -----Original Message----- >> From: Van Haaren, Harry >> Sent: Tuesday, November 26, 2019 1:19 PM >> To: Aaron Conole <aconole@redhat.com>; Thomas Monjalon <thomas@monjalon.net> > > <snip> > >> > EAL: Test assert service_lcore_en_dis_able line 487 failed: Ex-service >> core >> > function call had no effect. >> > >> > So I'll spend some time in this area, it seems. >> >> >> The below diff makes it 100% reproducible here, failing every time. >> >> It seems like the main thread is returning, before the service thread has >> returned. >> >> The rte_eal_mp_wait_lcore() call seems to not wait on the service-core, >> which allows >> the main thread to read the "service_remote_launch_flag" value as 0 (before >> the service-thread writes it to 1). >> >> Adding the delay between the service launch and service write being >> performed makes this issue much much more likely to occur - so the above >> description I have confidence in. >> >> What I'm not clear on (yet) is why the eal_mp_wait_lcore() isn't waiting... >> >> -H >> >> >> diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c >> index 9fe38f5e0..846ad00d1 100644 >> --- a/app/test/test_service_cores.c >> +++ b/app/test/test_service_cores.c >> @@ -445,6 +445,7 @@ static int >> service_remote_launch_func(void *arg) >> { >> RTE_SET_USED(arg); >> + rte_delay_ms(100); >> service_remote_launch_flag = 1; >> return 0; >> } > > Diff below seems to fix the problem here; Aaron would you test the below fix in your setup for a while too? > I have a loop running here attempting to reproduce - but before 100% failures and so far 100% passes with the added wait_lcore() call. > > > diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c > index 9fe38f5e0..62ffedb19 100644 > --- a/app/test/test_service_cores.c > +++ b/app/test/test_service_cores.c > @@ -445,6 +445,7 @@ static int > service_remote_launch_func(void *arg) > { > RTE_SET_USED(arg); > + rte_delay_ms(100); > service_remote_launch_flag = 1; > return 0; > } > @@ -483,6 +484,7 @@ service_lcore_en_dis_able(void) > int ret = rte_eal_remote_launch(service_remote_launch_func, NULL, > slcore_id); > TEST_ASSERT_EQUAL(0, ret, "Ex-service core remote launch failed."); > + rte_eal_wait_lcore(slcore_id); > rte_eal_mp_wait_lcore(); Ahh, I see. Actually, this brings up a question - is the intent for mp_wait_lcore to cycle through the service cores as well? Because IIUC, the issue will be the lcore will be set to ROLE_RTE normally, but service cores will do: ROLE_SERVICE and then the wait cannot work. If the idea is that mp_wait_lcore should work (and looking at the test, it seems like it is the intent?) then it will need to cycle through service cores, too. If the intent is that it shouldn't, then we should remove those calls from the test application to prevent developer from misunderstanding. Either way, the documentation for `rte_service_lcore_start` is a bit too ambiguous and needs to reflect whether the mp_wait_lcore should work. I think either it should (which means updating rte_get_next_lcore to include ROLE_SERVICE), or none of the lcore functions should work, and we should have an rte_service...() equivalent that should be used. > TEST_ASSERT_EQUAL(1, service_remote_launch_flag, > "Ex-service core function call had no effect."); ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-26 13:57 ` Aaron Conole @ 2019-11-27 11:37 ` Van Haaren, Harry 0 siblings, 0 replies; 19+ messages in thread From: Van Haaren, Harry @ 2019-11-27 11:37 UTC (permalink / raw) To: Aaron Conole Cc: Thomas Monjalon, Amber, Kumar, dev, Wang, Yipeng1, Yigit, Ferruh, Thakur, Sham Singh, David Marchand > -----Original Message----- > From: Aaron Conole <aconole@redhat.com> > Sent: Tuesday, November 26, 2019 1:57 PM > To: Van Haaren, Harry <harry.van.haaren@intel.com> > Cc: Thomas Monjalon <thomas@monjalon.net>; Amber, Kumar > <kumar.amber@intel.com>; dev@dpdk.org; Wang, Yipeng1 > <yipeng1.wang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Thakur, > Sham Singh <sham.singh.thakur@intel.com>; David Marchand > <dmarchan@redhat.com> > Subject: Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query > key id > > "Van Haaren, Harry" <harry.van.haaren@intel.com> writes: > > >> -----Original Message----- > >> From: Van Haaren, Harry > >> Sent: Tuesday, November 26, 2019 1:19 PM > >> To: Aaron Conole <aconole@redhat.com>; Thomas Monjalon > <thomas@monjalon.net> > > > > <snip> > > > >> > EAL: Test assert service_lcore_en_dis_able line 487 failed: Ex-service > >> core > >> > function call had no effect. > >> > > >> > So I'll spend some time in this area, it seems. > >> > >> > >> The below diff makes it 100% reproducible here, failing every time. > >> > >> It seems like the main thread is returning, before the service thread has > >> returned. > >> > >> The rte_eal_mp_wait_lcore() call seems to not wait on the service-core, > >> which allows > >> the main thread to read the "service_remote_launch_flag" value as 0 > (before > >> the service-thread writes it to 1). > >> > >> Adding the delay between the service launch and service write being > >> performed makes this issue much much more likely to occur - so the above > >> description I have confidence in. > >> > >> What I'm not clear on (yet) is why the eal_mp_wait_lcore() isn't > waiting... > >> > >> -H > >> > >> > >> diff --git a/app/test/test_service_cores.c > b/app/test/test_service_cores.c > >> index 9fe38f5e0..846ad00d1 100644 > >> --- a/app/test/test_service_cores.c > >> +++ b/app/test/test_service_cores.c > >> @@ -445,6 +445,7 @@ static int > >> service_remote_launch_func(void *arg) > >> { > >> RTE_SET_USED(arg); > >> + rte_delay_ms(100); > >> service_remote_launch_flag = 1; > >> return 0; > >> } > > > > Diff below seems to fix the problem here; Aaron would you test the below > fix in your setup for a while too? > > I have a loop running here attempting to reproduce - but before 100% > failures and so far 100% passes with the added wait_lcore() call. > > > > > > diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c > > index 9fe38f5e0..62ffedb19 100644 > > --- a/app/test/test_service_cores.c > > +++ b/app/test/test_service_cores.c > > @@ -445,6 +445,7 @@ static int > > service_remote_launch_func(void *arg) > > { > > RTE_SET_USED(arg); > > + rte_delay_ms(100); > > service_remote_launch_flag = 1; > > return 0; > > } > > @@ -483,6 +484,7 @@ service_lcore_en_dis_able(void) > > int ret = rte_eal_remote_launch(service_remote_launch_func, NULL, > > slcore_id); > > TEST_ASSERT_EQUAL(0, ret, "Ex-service core remote launch > failed."); > > + rte_eal_wait_lcore(slcore_id); > > rte_eal_mp_wait_lcore(); > > Ahh, I see. Actually, this brings up a question - is the intent for > mp_wait_lcore to cycle through the service cores as well? Because IIUC, > the issue will be the lcore will be set to ROLE_RTE normally, but > service cores will do: ROLE_SERVICE and then the wait cannot work. > > If the idea is that mp_wait_lcore should work (and looking at the test, > it seems like it is the intent?) then it will need to cycle through > service cores, too. If the intent is that it shouldn't, then we should > remove those calls from the test application to prevent developer from > misunderstanding. > > Either way, the documentation for `rte_service_lcore_start` is a bit too > ambiguous and needs to reflect whether the mp_wait_lcore should work. I > think either it should (which means updating rte_get_next_lcore to > include ROLE_SERVICE), or none of the lcore functions should work, and > we should have an rte_service...() equivalent that should be used. Service cores are meant to be transparent to the application. The test application testing this particular usage is the corner-case. The rte_eal_mp_wait_lcore() is correct to NOT wait for service-cores, as they are not always under application control. The observed test failure is a bug in the test code, it should use the explicit rte_eal_wait_lcore() call. I'll send a patch later today. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-26 13:19 ` Van Haaren, Harry 2019-11-26 13:29 ` Van Haaren, Harry @ 2019-11-26 15:58 ` Aaron Conole 1 sibling, 0 replies; 19+ messages in thread From: Aaron Conole @ 2019-11-26 15:58 UTC (permalink / raw) To: Van Haaren, Harry Cc: Thomas Monjalon, Amber, Kumar, dev, Wang, Yipeng1, Yigit, Ferruh, Thakur, Sham Singh, David Marchand "Van Haaren, Harry" <harry.van.haaren@intel.com> writes: > Hi Aaron, > >> -----Original Message----- >> From: Aaron Conole <aconole@redhat.com> >> Sent: Monday, November 25, 2019 10:54 PM >> To: Thomas Monjalon <thomas@monjalon.net> >> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Amber, Kumar >> <kumar.amber@intel.com>; dev@dpdk.org; Wang, Yipeng1 >> <yipeng1.wang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Thakur, >> Sham Singh <sham.singh.thakur@intel.com>; David Marchand >> <dmarchan@redhat.com> >> Subject: Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query >> key id >> >> Aaron Conole <aconole@redhat.com> writes: >> >> > Thomas Monjalon <thomas@monjalon.net> writes: >> > >> >>> From: Aaron Conole <aconole@redhat.com> >> >>> > - if (!service_valid(id)) >> >>> > + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) >> >> >> >> Why not adding this check in service_valid()? >> > >> > I think the best fix is to use SERVICE_VALID_GET_OR_ERR_RET() in these >> > places. For this, I at least want to try and show that there aren't any >> > further errors. And my test loop has been running for a while now >> > without any more errors or segfaults, so I guess it's okay to build a >> > proper patch. >> >> This popped up: >> >> EAL: Test assert service_lcore_en_dis_able line 487 failed: Ex-service core >> function call had no effect. >> >> So I'll spend some time in this area, it seems. > > > The below diff makes it 100% reproducible here, failing every time. > > It seems like the main thread is returning, before the service thread has returned. > > The rte_eal_mp_wait_lcore() call seems to not wait on the service-core, which allows > the main thread to read the "service_remote_launch_flag" value as 0 (before the service-thread writes it to 1). > > Adding the delay between the service launch and service write being performed makes this issue much much more likely to occur - so the above description I have confidence in. > > What I'm not clear on (yet) is why the eal_mp_wait_lcore() isn't waiting... As I wrote in the other thread, it's because eal_mp_wait_lcore won't look at lcores with ROLE_SERVICE. > -H I've been running something similar to the suggested patch for 24 minutes now with no failure. I've also removed the eal_mp_wait_lcore() call in other areas throughout the test and switched to individual core waiting "just in case." I don't think it's the right fix, though. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] hash: added a new API to hash to query key id 2019-11-25 11:08 ` Kumar Amber 2019-11-25 15:50 ` Amber, Kumar @ 2019-11-25 19:41 ` Wang, Yipeng1 1 sibling, 0 replies; 19+ messages in thread From: Wang, Yipeng1 @ 2019-11-25 19:41 UTC (permalink / raw) To: Amber, Kumar, dev Hi, Amber, Thanks for the patch! A bit comment inlined: >-----Original Message----- >From: Amber, Kumar >Sent: Monday, November 25, 2019 3:08 AM >To: dev@dpdk.org >Cc: Wang, Yipeng1 <yipeng1.wang@intel.com> >Subject: [PATCH v3] hash: added a new API to hash to query key id > >Adding new API function to query the maximum key ID >that could possibly returned by rte_hash_add_key and [Wang, Yipeng] That could possibly "be" returned... >rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD >is set, the maximum key id is larger than the entry count specified >by the user. > >Signed-off-by: Kumar Amber <kumar.amber@intel.com> >--- > lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ > lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- > lib/librte_hash/rte_hash_version.map | 1 + > 3 files changed, 39 insertions(+), 2 deletions(-) > >diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c >index 87a4c01f2..3a94f10b8 100644 >--- a/lib/librte_hash/rte_cuckoo_hash.c >+++ b/lib/librte_hash/rte_cuckoo_hash.c >@@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) > return h->hash_func(key, h->key_len, h->hash_func_init_val); > } > >+uint32_t >+rte_hash_max_key_id(const struct rte_hash *h) >+{ >+ RETURN_IF_TRUE((h == NULL), -EINVAL); >+ if (h->use_local_cache) >+ /* >+ * Increase number of slots by total number of indices >+ * that can be stored in the lcore caches >+ */ >+ return (h->entries + ((RTE_MAX_LCORE - 1) * >+ (LCORE_CACHE_SIZE - 1))); >+ else >+ return h->entries; >+} >+ > int32_t > rte_hash_count(const struct rte_hash *h) > { >diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h >index 0d73370dc..c87861e72 100644 >--- a/lib/librte_hash/rte_hash.h >+++ b/lib/librte_hash/rte_hash.h >@@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); > int32_t > rte_hash_count(const struct rte_hash *h); > >+/** >+ * @warning >+ * @b EXPERIMENTAL: this API may change without prior notice >+ * >+ * Return the maximum key value ID that could possibly be returned by >+ * rte_hash_add_key function. [Wang, Yipeng] Seems you have two spaces between "Return" and "the", change to one. >+ * >+ * @param h >+ * Hash table to query from >+ * @return >+ * - -EINVAL if parameters are invalid >+ * - A value indicating the max key Id key slots present in the table. [Wang, Yipeng] Two spaces between "Id" and "key". Maybe you want to say key Id "of" key slots? >+ */ >+__rte_experimental >+uint32_t >+rte_hash_max_key_id(const struct rte_hash *h); >+ > /** > * Add a key-value pair to an existing hash table. > * This operation is not multi-thread safe >@@ -234,7 +251,9 @@ rte_hash_add_key_with_hash_data(const struct rte_hash *h, const void *key, > * - -EINVAL if the parameters are invalid. > * - -ENOSPC if there is no space in the hash for this key. > * - A positive value that can be used by the caller as an offset into an >- * array of user data. This value is unique for this key. >+ * array of user data. This value is unique for this key. This >+ * unique key id may be larger than the user specified entry count >+ * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. > */ > int32_t > rte_hash_add_key(const struct rte_hash *h, const void *key); >@@ -256,7 +275,9 @@ rte_hash_add_key(const struct rte_hash *h, const void *key); > * - -EINVAL if the parameters are invalid. > * - -ENOSPC if there is no space in the hash for this key. > * - A positive value that can be used by the caller as an offset into an >- * array of user data. This value is unique for this key. >+ * array of user data. This value is unique for this key. This >+ * unique key ID may be larger than the user specified entry count >+ * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. > */ > int32_t > rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); >diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map >index 734ae28b0..562ceb8bc 100644 >--- a/lib/librte_hash/rte_hash_version.map >+++ b/lib/librte_hash/rte_hash_version.map >@@ -58,5 +58,6 @@ EXPERIMENTAL { > global: > > rte_hash_free_key_with_position; >+ rte_hash_max_key_id; > [Wang, Yipeng] Fix the Indentation of this line. [Wang, Yipeng] I also realized that rte_add_key returns int32_t but the ID could potentially be uint32_t. But by default, the entry ID should never overflow the range. Let's just convert the new API's return to int32_t to be consistent for now. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [dpdk-dev] [PATCH v4] hash: added a new API to hash to query key id 2019-11-22 18:21 ` [dpdk-dev] [PATCH v3] " Kumar Amber 2019-11-25 11:08 ` Kumar Amber @ 2019-11-26 2:39 ` Kumar Amber 2019-11-27 1:59 ` Wang, Yipeng1 1 sibling, 1 reply; 19+ messages in thread From: Kumar Amber @ 2019-11-26 2:39 UTC (permalink / raw) To: dev; +Cc: yipeng1.wang Adding new API function to query the maximum key ID that could possibly be returned by rte_hash_add_key and rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD is set, the maximum key id is larger than the entry count specified by the user. Signed-off-by: Kumar Amber <kumar.amber@intel.com> --- lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- lib/librte_hash/rte_hash_version.map | 1 + 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 87a4c01f2..41a081f10 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) return h->hash_func(key, h->key_len, h->hash_func_init_val); } +int32_t +rte_hash_max_key_id(const struct rte_hash *h) +{ + RETURN_IF_TRUE((h == NULL), -EINVAL); + if (h->use_local_cache) + /* + * Increase number of slots by total number of indices + * that can be stored in the lcore caches + */ + return (h->entries + ((RTE_MAX_LCORE - 1) * + (LCORE_CACHE_SIZE - 1))); + else + return h->entries; +} + int32_t rte_hash_count(const struct rte_hash *h) { diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 0d73370dc..f93203122 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); int32_t rte_hash_count(const struct rte_hash *h); +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice + * + * Return the maximum key value ID that could possibly be returned by + * rte_hash_add_key function. + * + * @param h + * Hash table to query from + * @return + * - -EINVAL if parameters are invalid + * - A value indicating the max key of Id key slots present in the table. + */ +__rte_experimental +int32_t +rte_hash_max_key_id(const struct rte_hash *h); + /** * Add a key-value pair to an existing hash table. * This operation is not multi-thread safe @@ -234,7 +251,9 @@ rte_hash_add_key_with_hash_data(const struct rte_hash *h, const void *key, * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key id may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key(const struct rte_hash *h, const void *key); @@ -256,7 +275,9 @@ rte_hash_add_key(const struct rte_hash *h, const void *key); * - -EINVAL if the parameters are invalid. * - -ENOSPC if there is no space in the hash for this key. * - A positive value that can be used by the caller as an offset into an - * array of user data. This value is unique for this key. + * array of user data. This value is unique for this key. This + * unique key ID may be larger than the user specified entry count + * when RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD flag is set. */ int32_t rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map index 734ae28b0..f6de2d8a2 100644 --- a/lib/librte_hash/rte_hash_version.map +++ b/lib/librte_hash/rte_hash_version.map @@ -58,5 +58,6 @@ EXPERIMENTAL { global: rte_hash_free_key_with_position; + rte_hash_max_key_id; }; -- 2.17.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v4] hash: added a new API to hash to query key id 2019-11-26 2:39 ` [dpdk-dev] [PATCH v4] " Kumar Amber @ 2019-11-27 1:59 ` Wang, Yipeng1 2020-01-20 0:16 ` Thomas Monjalon 0 siblings, 1 reply; 19+ messages in thread From: Wang, Yipeng1 @ 2019-11-27 1:59 UTC (permalink / raw) To: Amber, Kumar, dev >-----Original Message----- >From: Amber, Kumar >Sent: Monday, November 25, 2019 6:39 PM >To: dev@dpdk.org >Cc: Wang, Yipeng1 <yipeng1.wang@intel.com> >Subject: [PATCH v4] hash: added a new API to hash to query key id > >Adding new API function to query the maximum key ID >that could possibly be returned by rte_hash_add_key and >rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD >is set, the maximum key id is larger than the entry count specified >by the user. > >Signed-off-by: Kumar Amber <kumar.amber@intel.com> >--- > lib/librte_hash/rte_cuckoo_hash.c | 15 +++++++++++++++ > lib/librte_hash/rte_hash.h | 25 +++++++++++++++++++++++-- > lib/librte_hash/rte_hash_version.map | 1 + > 3 files changed, 39 insertions(+), 2 deletions(-) > >diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c >index 87a4c01f2..41a081f10 100644 >--- a/lib/librte_hash/rte_cuckoo_hash.c >+++ b/lib/librte_hash/rte_cuckoo_hash.c >@@ -506,6 +506,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key) > return h->hash_func(key, h->key_len, h->hash_func_init_val); > } > >+int32_t >+rte_hash_max_key_id(const struct rte_hash *h) >+{ >+ RETURN_IF_TRUE((h == NULL), -EINVAL); >+ if (h->use_local_cache) >+ /* >+ * Increase number of slots by total number of indices >+ * that can be stored in the lcore caches >+ */ >+ return (h->entries + ((RTE_MAX_LCORE - 1) * >+ (LCORE_CACHE_SIZE - 1))); >+ else >+ return h->entries; >+} >+ > int32_t > rte_hash_count(const struct rte_hash *h) > { >diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h >index 0d73370dc..f93203122 100644 >--- a/lib/librte_hash/rte_hash.h >+++ b/lib/librte_hash/rte_hash.h >@@ -164,6 +164,23 @@ rte_hash_reset(struct rte_hash *h); > int32_t > rte_hash_count(const struct rte_hash *h); > >+/** >+ * @warning >+ * @b EXPERIMENTAL: this API may change without prior notice >+ * >+ * Return the maximum key value ID that could possibly be returned by >+ * rte_hash_add_key function. >+ * >+ * @param h >+ * Hash table to query from >+ * @return >+ * - -EINVAL if parameters are invalid >+ * - A value indicating the max key of Id key slots present in the table. >+ */ [Wang, Yipeng] typo: key of id -> key Id of Other than this: Acked-by: Yipeng Wang <yipeng1.wang@intel.com> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v4] hash: added a new API to hash to query key id 2019-11-27 1:59 ` Wang, Yipeng1 @ 2020-01-20 0:16 ` Thomas Monjalon 0 siblings, 0 replies; 19+ messages in thread From: Thomas Monjalon @ 2020-01-20 0:16 UTC (permalink / raw) To: Amber, Kumar; +Cc: dev, Wang, Yipeng1 27/11/2019 02:59, Wang, Yipeng1: > From: Amber, Kumar > >Adding new API function to query the maximum key ID > >that could possibly be returned by rte_hash_add_key and > >rte_hash_add_key_with_hash. When RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD > >is set, the maximum key id is larger than the entry count specified > >by the user. > > > >Signed-off-by: Kumar Amber <kumar.amber@intel.com> > >--- > >+ * - A value indicating the max key of Id key slots present in the table. > >+ */ > [Wang, Yipeng] typo: key of id -> key Id of > > Other than this: > Acked-by: Yipeng Wang <yipeng1.wang@intel.com> Applied with typo fixed, thanks ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-01-20 0:16 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-22 14:59 [dpdk-dev] [PATCH v2] hash: added a new API to hash to query key id Kumar Amber 2019-11-22 18:21 ` [dpdk-dev] [PATCH v3] " Kumar Amber 2019-11-25 11:08 ` Kumar Amber 2019-11-25 15:50 ` Amber, Kumar 2019-11-25 16:01 ` Aaron Conole 2019-11-25 16:58 ` Aaron Conole 2019-11-25 17:16 ` Van Haaren, Harry 2019-11-25 17:54 ` Thomas Monjalon 2019-11-25 18:10 ` Aaron Conole 2019-11-25 22:53 ` Aaron Conole 2019-11-26 13:19 ` Van Haaren, Harry 2019-11-26 13:29 ` Van Haaren, Harry 2019-11-26 13:57 ` Aaron Conole 2019-11-27 11:37 ` Van Haaren, Harry 2019-11-26 15:58 ` Aaron Conole 2019-11-25 19:41 ` Wang, Yipeng1 2019-11-26 2:39 ` [dpdk-dev] [PATCH v4] " Kumar Amber 2019-11-27 1:59 ` Wang, Yipeng1 2020-01-20 0:16 ` Thomas Monjalon
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).