* [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
@ 2024-02-07 15:33 Abdullah Ömer Yamaç
2024-02-16 12:41 ` Thomas Monjalon
2024-02-19 17:36 ` Medvedkin, Vladimir
0 siblings, 2 replies; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-02-07 15:33 UTC (permalink / raw)
To: dev
Cc: Abdullah Ömer Yamaç,
Honnappa Nagarahalli, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, Vladimir Medvedkin
This patch introduce a new API to get the hidden key count in the hash
table if the rcu qsbr is enabled. When using rte_hash_count with rcu
qsbr enabled, it will return the number of elements that are not in the
free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
elements in the defer queue will not be counted and freed. Therefore I
added a new API to get the number of hidden (defer queue) elements
in the hash table. Then the user can calculate the total number of
elements that are available in the hash table.
Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
---
Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Cc: Yipeng Wang <yipeng1.wang@intel.com>
Cc: Sameh Gobriel <sameh.gobriel@intel.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>
Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
lib/hash/rte_cuckoo_hash.c | 9 +++++++++
lib/hash/rte_hash.h | 13 +++++++++++++
lib/hash/version.map | 1 +
lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
lib/rcu/version.map | 1 +
6 files changed, 43 insertions(+)
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 70456754c4..3553f3efc7 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
return h->entries;
}
+int32_t
+rte_hash_dq_count(const struct rte_hash *h)
+{
+ if (h->dq == NULL)
+ return -EINVAL;
+
+ return rte_rcu_qsbr_dq_count(h->dq);
+}
+
int32_t
rte_hash_count(const struct rte_hash *h)
{
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..8ea97e297d 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
void
rte_hash_reset(struct rte_hash *h);
+
+/**
+ * Return the number of records in the defer queue of the hash table
+ * if RCU is enabled.
+ * @param h
+ * Hash table to query from
+ * @return
+ * - -EINVAL if parameters are invalid
+ * - A value indicating how many records were inserted in the table.
+ */
+int32_t
+rte_hash_dq_count(const struct rte_hash *h);
+
/**
* Return the number of keys in the hash table
* @param h
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6b2afebf6b..7f7b158cf1 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -9,6 +9,7 @@ DPDK_24 {
rte_hash_add_key_with_hash;
rte_hash_add_key_with_hash_data;
rte_hash_count;
+ rte_hash_dq_count;
rte_hash_crc32_alg;
rte_hash_crc_set_alg;
rte_hash_create;
diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
index bd0b83be0c..89f8da4c4c 100644
--- a/lib/rcu/rte_rcu_qsbr.c
+++ b/lib/rcu/rte_rcu_qsbr.c
@@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
return 0;
}
+/**
+ * Return the number of entries in a defer queue.
+ */
+unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
+{
+ return rte_ring_count(dq->r);
+}
+
/* Delete a defer queue. */
int
rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
index 23c9f89805..ed5a590edd 100644
--- a/lib/rcu/rte_rcu_qsbr.h
+++ b/lib/rcu/rte_rcu_qsbr.h
@@ -794,6 +794,17 @@ int
rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
unsigned int *freed, unsigned int *pending, unsigned int *available);
+/**
+ * Return the number of entries in a defer queue.
+ *
+ * @param dq
+ * Defer queue.
+ * @return
+ * The number of entries in the defer queue.
+ */
+unsigned int
+rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
+
/**
* Delete a defer queue.
*
diff --git a/lib/rcu/version.map b/lib/rcu/version.map
index 982ffd59d9..f410ab41e7 100644
--- a/lib/rcu/version.map
+++ b/lib/rcu/version.map
@@ -5,6 +5,7 @@ DPDK_24 {
rte_rcu_qsbr_dq_create;
rte_rcu_qsbr_dq_delete;
rte_rcu_qsbr_dq_enqueue;
+ rte_rcu_qsbr_dq_count;
rte_rcu_qsbr_dq_reclaim;
rte_rcu_qsbr_dump;
rte_rcu_qsbr_get_memsize;
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-07 15:33 [PATCH] lib/hash,lib/rcu: feature hidden key count in hash Abdullah Ömer Yamaç
@ 2024-02-16 12:41 ` Thomas Monjalon
2024-02-19 17:36 ` Medvedkin, Vladimir
1 sibling, 0 replies; 30+ messages in thread
From: Thomas Monjalon @ 2024-02-16 12:41 UTC (permalink / raw)
To: dev
Cc: Abdullah Ömer Yamaç,
Honnappa Nagarahalli, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, Vladimir Medvedkin,
Abdullah Ömer Yamaç
Any review please?
07/02/2024 16:33, Abdullah Ömer Yamaç:
> This patch introduce a new API to get the hidden key count in the hash
> table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> qsbr enabled, it will return the number of elements that are not in the
> free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> elements in the defer queue will not be counted and freed. Therefore I
> added a new API to get the number of hidden (defer queue) elements
> in the hash table. Then the user can calculate the total number of
> elements that are available in the hash table.
>
> Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
>
> ---
> Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Cc: Yipeng Wang <yipeng1.wang@intel.com>
> Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>
> Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---
> lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> lib/hash/rte_hash.h | 13 +++++++++++++
> lib/hash/version.map | 1 +
> lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> lib/rcu/version.map | 1 +
> 6 files changed, 43 insertions(+)
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 70456754c4..3553f3efc7 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> return h->entries;
> }
>
> +int32_t
> +rte_hash_dq_count(const struct rte_hash *h)
> +{
> + if (h->dq == NULL)
> + return -EINVAL;
> +
> + return rte_rcu_qsbr_dq_count(h->dq);
> +}
> +
> int32_t
> rte_hash_count(const struct rte_hash *h)
> {
> diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> index 7ecc021111..8ea97e297d 100644
> --- a/lib/hash/rte_hash.h
> +++ b/lib/hash/rte_hash.h
> @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> void
> rte_hash_reset(struct rte_hash *h);
>
> +
> +/**
> + * Return the number of records in the defer queue of the hash table
> + * if RCU is enabled.
> + * @param h
> + * Hash table to query from
> + * @return
> + * - -EINVAL if parameters are invalid
> + * - A value indicating how many records were inserted in the table.
> + */
> +int32_t
> +rte_hash_dq_count(const struct rte_hash *h);
> +
> /**
> * Return the number of keys in the hash table
> * @param h
> diff --git a/lib/hash/version.map b/lib/hash/version.map
> index 6b2afebf6b..7f7b158cf1 100644
> --- a/lib/hash/version.map
> +++ b/lib/hash/version.map
> @@ -9,6 +9,7 @@ DPDK_24 {
> rte_hash_add_key_with_hash;
> rte_hash_add_key_with_hash_data;
> rte_hash_count;
> + rte_hash_dq_count;
> rte_hash_crc32_alg;
> rte_hash_crc_set_alg;
> rte_hash_create;
> diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> index bd0b83be0c..89f8da4c4c 100644
> --- a/lib/rcu/rte_rcu_qsbr.c
> +++ b/lib/rcu/rte_rcu_qsbr.c
> @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> return 0;
> }
>
> +/**
> + * Return the number of entries in a defer queue.
> + */
> +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> +{
> + return rte_ring_count(dq->r);
> +}
> +
> /* Delete a defer queue. */
> int
> rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> index 23c9f89805..ed5a590edd 100644
> --- a/lib/rcu/rte_rcu_qsbr.h
> +++ b/lib/rcu/rte_rcu_qsbr.h
> @@ -794,6 +794,17 @@ int
> rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> unsigned int *freed, unsigned int *pending, unsigned int *available);
>
> +/**
> + * Return the number of entries in a defer queue.
> + *
> + * @param dq
> + * Defer queue.
> + * @return
> + * The number of entries in the defer queue.
> + */
> +unsigned int
> +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> +
> /**
> * Delete a defer queue.
> *
> diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> index 982ffd59d9..f410ab41e7 100644
> --- a/lib/rcu/version.map
> +++ b/lib/rcu/version.map
> @@ -5,6 +5,7 @@ DPDK_24 {
> rte_rcu_qsbr_dq_create;
> rte_rcu_qsbr_dq_delete;
> rte_rcu_qsbr_dq_enqueue;
> + rte_rcu_qsbr_dq_count;
> rte_rcu_qsbr_dq_reclaim;
> rte_rcu_qsbr_dump;
> rte_rcu_qsbr_get_memsize;
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-07 15:33 [PATCH] lib/hash,lib/rcu: feature hidden key count in hash Abdullah Ömer Yamaç
2024-02-16 12:41 ` Thomas Monjalon
@ 2024-02-19 17:36 ` Medvedkin, Vladimir
2024-02-19 21:28 ` Abdullah Ömer Yamaç
1 sibling, 1 reply; 30+ messages in thread
From: Medvedkin, Vladimir @ 2024-02-19 17:36 UTC (permalink / raw)
To: Abdullah Ömer Yamaç, dev
Cc: Honnappa Nagarahalli, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, Thomas Monjalon
Hi Abdullah,
Could you please tell more about use cases where this API may be useful?
>a new API to get the hidden key count in the hash table if the rcu qsbr is enabled
Here in commit message and down below in doxygen comments, I think this
statement should be more specific because rcu can be created with
RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
Also, new API must be reflected in release notes
On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> This patch introduce a new API to get the hidden key count in the hash
> table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> qsbr enabled, it will return the number of elements that are not in the
> free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> elements in the defer queue will not be counted and freed. Therefore I
> added a new API to get the number of hidden (defer queue) elements
> in the hash table. Then the user can calculate the total number of
> elements that are available in the hash table.
>
> Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
>
> ---
> Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Cc: Yipeng Wang <yipeng1.wang@intel.com>
> Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>
> Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---
> lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> lib/hash/rte_hash.h | 13 +++++++++++++
> lib/hash/version.map | 1 +
> lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> lib/rcu/version.map | 1 +
> 6 files changed, 43 insertions(+)
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 70456754c4..3553f3efc7 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> return h->entries;
> }
>
> +int32_t
> +rte_hash_dq_count(const struct rte_hash *h)
> +{
> + if (h->dq == NULL)
input arguments must be checked since this is a public API, the same is
true for rte_rcu_qsbr_dq_count()
> + return -EINVAL;
why not just return 0?
> +
> + return rte_rcu_qsbr_dq_count(h->dq);
> +}
> +
> int32_t
> rte_hash_count(const struct rte_hash *h)
> {
> diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> index 7ecc021111..8ea97e297d 100644
> --- a/lib/hash/rte_hash.h
> +++ b/lib/hash/rte_hash.h
> @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> void
> rte_hash_reset(struct rte_hash *h);
>
> +
> +/**
> + * Return the number of records in the defer queue of the hash table
> + * if RCU is enabled.
> + * @param h
> + * Hash table to query from
> + * @return
> + * - -EINVAL if parameters are invalid
> + * - A value indicating how many records were inserted in the table.
did you mean how many records are kept in defer queue?
> + */
> +int32_t
> +rte_hash_dq_count(const struct rte_hash *h);
> +
> /**
> * Return the number of keys in the hash table
> * @param h
> diff --git a/lib/hash/version.map b/lib/hash/version.map
> index 6b2afebf6b..7f7b158cf1 100644
> --- a/lib/hash/version.map
> +++ b/lib/hash/version.map
> @@ -9,6 +9,7 @@ DPDK_24 {
> rte_hash_add_key_with_hash;
> rte_hash_add_key_with_hash_data;
> rte_hash_count;
> + rte_hash_dq_count;
new API must introduced as an experimental API. The same is true for
rte_rcu_qsbr_dq_count()
> rte_hash_crc32_alg;
> rte_hash_crc_set_alg;
> rte_hash_create;
> diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> index bd0b83be0c..89f8da4c4c 100644
> --- a/lib/rcu/rte_rcu_qsbr.c
> +++ b/lib/rcu/rte_rcu_qsbr.c
> @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> return 0;
> }
>
> +/**
> + * Return the number of entries in a defer queue.
> + */
> +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> +{
> + return rte_ring_count(dq->r);
> +}
> +
> /* Delete a defer queue. */
> int
> rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> index 23c9f89805..ed5a590edd 100644
> --- a/lib/rcu/rte_rcu_qsbr.h
> +++ b/lib/rcu/rte_rcu_qsbr.h
> @@ -794,6 +794,17 @@ int
> rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> unsigned int *freed, unsigned int *pending, unsigned int *available);
>
> +/**
> + * Return the number of entries in a defer queue.
> + *
> + * @param dq
> + * Defer queue.
> + * @return
> + * The number of entries in the defer queue.
> + */
> +unsigned int
> +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> +
> /**
> * Delete a defer queue.
> *
> diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> index 982ffd59d9..f410ab41e7 100644
> --- a/lib/rcu/version.map
> +++ b/lib/rcu/version.map
> @@ -5,6 +5,7 @@ DPDK_24 {
> rte_rcu_qsbr_dq_create;
> rte_rcu_qsbr_dq_delete;
> rte_rcu_qsbr_dq_enqueue;
> + rte_rcu_qsbr_dq_count;
> rte_rcu_qsbr_dq_reclaim;
> rte_rcu_qsbr_dump;
> rte_rcu_qsbr_get_memsize;
--
Regards,
Vladimir
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-19 17:36 ` Medvedkin, Vladimir
@ 2024-02-19 21:28 ` Abdullah Ömer Yamaç
2024-02-19 23:35 ` Honnappa Nagarahalli
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-02-19 21:28 UTC (permalink / raw)
To: Medvedkin, Vladimir
Cc: dev, Honnappa Nagarahalli, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, Thomas Monjalon
[-- Attachment #1: Type: text/plain, Size: 6700 bytes --]
Hello,
Let me explain a use case;
I have a hash table whose key value is IP addresses, and data (let's say
the username of the IP) is related to the IP address. The key point is
matching these data with flows. Flows are dynamic, and this hash table is
dynamic, as well; both can change anytime. For example, when a flow starts,
we look up the hash table with the corresponding IP and retrieve the
username. We need to hold this username until the flow terminates, although
we removed this IP key from the hash table (multithread). That's why we
have RCU and defer queue is necessary for high performance. In my
application, I need to know the number of IP-username entries. These
numbers can be calculated by rte_hash_count - defer queue size.
I think if you need a non-blocking and multithreaded hash table, an
RCU-enabled hash table is necessary. Also, this API is necessary if you
need to get the actual matchable size.
On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <
vladimir.medvedkin@intel.com> wrote:
> Hi Abdullah,
>
> Could you please tell more about use cases where this API may be useful?
>
> >a new API to get the hidden key count in the hash table if the rcu qsbr
> is enabled
>
> Here in commit message and down below in doxygen comments, I think this
> statement should be more specific because rcu can be created with
> RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
>
> Also, new API must be reflected in release notes
>
> On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > This patch introduce a new API to get the hidden key count in the hash
> > table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> > qsbr enabled, it will return the number of elements that are not in the
> > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > elements in the defer queue will not be counted and freed. Therefore I
> > added a new API to get the number of hidden (defer queue) elements
> > in the hash table. Then the user can calculate the total number of
> > elements that are available in the hash table.
> >
> > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> >
> > ---
> > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > ---
> > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > lib/hash/rte_hash.h | 13 +++++++++++++
> > lib/hash/version.map | 1 +
> > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > lib/rcu/version.map | 1 +
> > 6 files changed, 43 insertions(+)
> >
> > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > index 70456754c4..3553f3efc7 100644
> > --- a/lib/hash/rte_cuckoo_hash.c
> > +++ b/lib/hash/rte_cuckoo_hash.c
> > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > return h->entries;
> > }
> >
> > +int32_t
> > +rte_hash_dq_count(const struct rte_hash *h)
> > +{
> > + if (h->dq == NULL)
> input arguments must be checked since this is a public API, the same is
> true for rte_rcu_qsbr_dq_count()
> > + return -EINVAL;
> why not just return 0?
> > +
> > + return rte_rcu_qsbr_dq_count(h->dq);
> > +}
> > +
> > int32_t
> > rte_hash_count(const struct rte_hash *h)
> > {
> > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > index 7ecc021111..8ea97e297d 100644
> > --- a/lib/hash/rte_hash.h
> > +++ b/lib/hash/rte_hash.h
> > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > void
> > rte_hash_reset(struct rte_hash *h);
> >
> > +
> > +/**
> > + * Return the number of records in the defer queue of the hash table
> > + * if RCU is enabled.
> > + * @param h
> > + * Hash table to query from
> > + * @return
> > + * - -EINVAL if parameters are invalid
> > + * - A value indicating how many records were inserted in the table.
> did you mean how many records are kept in defer queue?
> > + */
> > +int32_t
> > +rte_hash_dq_count(const struct rte_hash *h);
> > +
> > /**
> > * Return the number of keys in the hash table
> > * @param h
> > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > index 6b2afebf6b..7f7b158cf1 100644
> > --- a/lib/hash/version.map
> > +++ b/lib/hash/version.map
> > @@ -9,6 +9,7 @@ DPDK_24 {
> > rte_hash_add_key_with_hash;
> > rte_hash_add_key_with_hash_data;
> > rte_hash_count;
> > + rte_hash_dq_count;
> new API must introduced as an experimental API. The same is true for
> rte_rcu_qsbr_dq_count()
> > rte_hash_crc32_alg;
> > rte_hash_crc_set_alg;
> > rte_hash_create;
> > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > index bd0b83be0c..89f8da4c4c 100644
> > --- a/lib/rcu/rte_rcu_qsbr.c
> > +++ b/lib/rcu/rte_rcu_qsbr.c
> > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq,
> unsigned int n,
> > return 0;
> > }
> >
> > +/**
> > + * Return the number of entries in a defer queue.
> > + */
> > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > +{
> > + return rte_ring_count(dq->r);
> > +}
> > +
> > /* Delete a defer queue. */
> > int
> > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > index 23c9f89805..ed5a590edd 100644
> > --- a/lib/rcu/rte_rcu_qsbr.h
> > +++ b/lib/rcu/rte_rcu_qsbr.h
> > @@ -794,6 +794,17 @@ int
> > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > unsigned int *freed, unsigned int *pending, unsigned int
> *available);
> >
> > +/**
> > + * Return the number of entries in a defer queue.
> > + *
> > + * @param dq
> > + * Defer queue.
> > + * @return
> > + * The number of entries in the defer queue.
> > + */
> > +unsigned int
> > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> > +
> > /**
> > * Delete a defer queue.
> > *
> > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > index 982ffd59d9..f410ab41e7 100644
> > --- a/lib/rcu/version.map
> > +++ b/lib/rcu/version.map
> > @@ -5,6 +5,7 @@ DPDK_24 {
> > rte_rcu_qsbr_dq_create;
> > rte_rcu_qsbr_dq_delete;
> > rte_rcu_qsbr_dq_enqueue;
> > + rte_rcu_qsbr_dq_count;
> > rte_rcu_qsbr_dq_reclaim;
> > rte_rcu_qsbr_dump;
> > rte_rcu_qsbr_get_memsize;
>
> --
> Regards,
> Vladimir
>
>
[-- Attachment #2: Type: text/html, Size: 8366 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-19 21:28 ` Abdullah Ömer Yamaç
@ 2024-02-19 23:35 ` Honnappa Nagarahalli
2024-02-20 18:58 ` Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Honnappa Nagarahalli @ 2024-02-19 23:35 UTC (permalink / raw)
To: Abdullah Ömer Yamaç
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
> On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> Hello,
>
> Let me explain a use case;
>
> I have a hash table whose key value is IP addresses, and data (let's say the username of the IP) is related to the IP address. The key point is matching these data with flows. Flows are dynamic, and this hash table is dynamic, as well; both can change anytime. For example, when a flow starts, we look up the hash table with the corresponding IP and retrieve the username. We need to hold this username until the flow terminates, although we removed this IP key from the hash table (multithread). That's why we have RCU and defer queue is necessary for high performance. In my application, I need to know the number of IP-username entries. These numbers can be calculated by rte_hash_count - defer queue size.
The entries in the defer queue are not reclaimed (there is a probability that all of them can be reclaimed) and hence they are not available for allocation. So, rte_hash_count - defer queue size might not give you the correct number you are expecting.
Currently, there is no API in hash library that forces a reclaim. Does it makes sense to have an API that just does the reclaim (and returns the number of entries pending in the defer queue)? A call to rte_hash_count should provide the exact count you are looking for.
>
> I think if you need a non-blocking and multithreaded hash table, an RCU-enabled hash table is necessary. Also, this API is necessary if you need to get the actual matchable size.
>
>
>
>
>
> On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <vladimir.medvedkin@intel.com> wrote:
> Hi Abdullah,
>
> Could you please tell more about use cases where this API may be useful?
>
> >a new API to get the hidden key count in the hash table if the rcu qsbr is enabled
>
> Here in commit message and down below in doxygen comments, I think this
> statement should be more specific because rcu can be created with
> RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
>
> Also, new API must be reflected in release notes
>
> On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > This patch introduce a new API to get the hidden key count in the hash
> > table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> > qsbr enabled, it will return the number of elements that are not in the
> > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > elements in the defer queue will not be counted and freed. Therefore I
> > added a new API to get the number of hidden (defer queue) elements
> > in the hash table. Then the user can calculate the total number of
> > elements that are available in the hash table.
> >
> > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> >
> > ---
> > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > ---
> > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > lib/hash/rte_hash.h | 13 +++++++++++++
> > lib/hash/version.map | 1 +
> > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > lib/rcu/version.map | 1 +
> > 6 files changed, 43 insertions(+)
> >
> > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > index 70456754c4..3553f3efc7 100644
> > --- a/lib/hash/rte_cuckoo_hash.c
> > +++ b/lib/hash/rte_cuckoo_hash.c
> > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > return h->entries;
> > }
> >
> > +int32_t
> > +rte_hash_dq_count(const struct rte_hash *h)
> > +{
> > + if (h->dq == NULL)
> input arguments must be checked since this is a public API, the same is
> true for rte_rcu_qsbr_dq_count()
> > + return -EINVAL;
> why not just return 0?
> > +
> > + return rte_rcu_qsbr_dq_count(h->dq);
> > +}
> > +
> > int32_t
> > rte_hash_count(const struct rte_hash *h)
> > {
> > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > index 7ecc021111..8ea97e297d 100644
> > --- a/lib/hash/rte_hash.h
> > +++ b/lib/hash/rte_hash.h
> > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > void
> > rte_hash_reset(struct rte_hash *h);
> >
> > +
> > +/**
> > + * Return the number of records in the defer queue of the hash table
> > + * if RCU is enabled.
> > + * @param h
> > + * Hash table to query from
> > + * @return
> > + * - -EINVAL if parameters are invalid
> > + * - A value indicating how many records were inserted in the table.
> did you mean how many records are kept in defer queue?
> > + */
> > +int32_t
> > +rte_hash_dq_count(const struct rte_hash *h);
> > +
> > /**
> > * Return the number of keys in the hash table
> > * @param h
> > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > index 6b2afebf6b..7f7b158cf1 100644
> > --- a/lib/hash/version.map
> > +++ b/lib/hash/version.map
> > @@ -9,6 +9,7 @@ DPDK_24 {
> > rte_hash_add_key_with_hash;
> > rte_hash_add_key_with_hash_data;
> > rte_hash_count;
> > + rte_hash_dq_count;
> new API must introduced as an experimental API. The same is true for
> rte_rcu_qsbr_dq_count()
> > rte_hash_crc32_alg;
> > rte_hash_crc_set_alg;
> > rte_hash_create;
> > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > index bd0b83be0c..89f8da4c4c 100644
> > --- a/lib/rcu/rte_rcu_qsbr.c
> > +++ b/lib/rcu/rte_rcu_qsbr.c
> > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > return 0;
> > }
> >
> > +/**
> > + * Return the number of entries in a defer queue.
> > + */
> > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > +{
Please validate dq here.
> > + return rte_ring_count(dq->r);
> > +}
> > +
> > /* Delete a defer queue. */
> > int
> > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > index 23c9f89805..ed5a590edd 100644
> > --- a/lib/rcu/rte_rcu_qsbr.h
> > +++ b/lib/rcu/rte_rcu_qsbr.h
> > @@ -794,6 +794,17 @@ int
> > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > unsigned int *freed, unsigned int *pending, unsigned int *available);
> >
> > +/**
> > + * Return the number of entries in a defer queue.
> > + *
> > + * @param dq
> > + * Defer queue.
> > + * @return
> > + * The number of entries in the defer queue.
> > + */
> > +unsigned int
> > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
Agree on the need for this API in RCU
> > +
> > /**
> > * Delete a defer queue.
> > *
> > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > index 982ffd59d9..f410ab41e7 100644
> > --- a/lib/rcu/version.map
> > +++ b/lib/rcu/version.map
> > @@ -5,6 +5,7 @@ DPDK_24 {
> > rte_rcu_qsbr_dq_create;
> > rte_rcu_qsbr_dq_delete;
> > rte_rcu_qsbr_dq_enqueue;
> > + rte_rcu_qsbr_dq_count;
> > rte_rcu_qsbr_dq_reclaim;
> > rte_rcu_qsbr_dump;
> > rte_rcu_qsbr_get_memsize;
>
> --
> Regards,
> Vladimir
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-19 23:35 ` Honnappa Nagarahalli
@ 2024-02-20 18:58 ` Abdullah Ömer Yamaç
2024-02-21 3:23 ` Honnappa Nagarahalli
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-02-20 18:58 UTC (permalink / raw)
To: Honnappa Nagarahalli
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
[-- Attachment #1: Type: text/plain, Size: 8462 bytes --]
I appreciate that you gave me suggestions and comments. I will make changes
according to all your recommendations, but before that, I want to make
everyone's minds clear. Then, I will apply modifications.
On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> >
> > Hello,
> >
> > Let me explain a use case;
> >
> > I have a hash table whose key value is IP addresses, and data (let's say
> the username of the IP) is related to the IP address. The key point is
> matching these data with flows. Flows are dynamic, and this hash table is
> dynamic, as well; both can change anytime. For example, when a flow starts,
> we look up the hash table with the corresponding IP and retrieve the
> username. We need to hold this username until the flow terminates, although
> we removed this IP key from the hash table (multithread). That's why we
> have RCU and defer queue is necessary for high performance. In my
> application, I need to know the number of IP-username entries. These
> numbers can be calculated by rte_hash_count - defer queue size.
> The entries in the defer queue are not reclaimed (there is a probability
> that all of them can be reclaimed) and hence they are not available for
> allocation. So, rte_hash_count - defer queue size might not give you the
> correct number you are expecting.
>
> Currently, there is no API in hash library that forces a reclaim. Does it
> makes sense to have an API that just does the reclaim (and returns the
> number of entries pending in the defer queue)? A call to rte_hash_count
> should provide the exact count you are looking for.
You are right; no API in the hash library forces a reclaim. In my
application, I periodically call rte_count to retrieve hash size, and this
data is shown in my GUI. So that means I need to call regularly reclaim. I
am trying to figure out which is better, calling reclaim or retrieving the
defer queue size. Any comment about this?
> >
> > I think if you need a non-blocking and multithreaded hash table, an
> RCU-enabled hash table is necessary. Also, this API is necessary if you
> need to get the actual matchable size.
> >
> >
> >
> >
> >
> > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <
> vladimir.medvedkin@intel.com> wrote:
> > Hi Abdullah,
> >
> > Could you please tell more about use cases where this API may be useful?
> >
> > >a new API to get the hidden key count in the hash table if the rcu qsbr
> is enabled
> >
> > Here in commit message and down below in doxygen comments, I think this
> > statement should be more specific because rcu can be created with
> > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> >
> > Also, new API must be reflected in release notes
> >
> > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > This patch introduce a new API to get the hidden key count in the hash
> > > table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> > > qsbr enabled, it will return the number of elements that are not in the
> > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > > elements in the defer queue will not be counted and freed. Therefore I
> > > added a new API to get the number of hidden (defer queue) elements
> > > in the hash table. Then the user can calculate the total number of
> > > elements that are available in the hash table.
> > >
> > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > >
> > > ---
> > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > ---
> > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > lib/hash/version.map | 1 +
> > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > lib/rcu/version.map | 1 +
> > > 6 files changed, 43 insertions(+)
> > >
> > > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > > index 70456754c4..3553f3efc7 100644
> > > --- a/lib/hash/rte_cuckoo_hash.c
> > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > > return h->entries;
> > > }
> > >
> > > +int32_t
> > > +rte_hash_dq_count(const struct rte_hash *h)
> > > +{
> > > + if (h->dq == NULL)
> > input arguments must be checked since this is a public API, the same is
> > true for rte_rcu_qsbr_dq_count()
> > > + return -EINVAL;
> > why not just return 0?
> > > +
> > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > +}
> > > +
> > > int32_t
> > > rte_hash_count(const struct rte_hash *h)
> > > {
> > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > index 7ecc021111..8ea97e297d 100644
> > > --- a/lib/hash/rte_hash.h
> > > +++ b/lib/hash/rte_hash.h
> > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > void
> > > rte_hash_reset(struct rte_hash *h);
> > >
> > > +
> > > +/**
> > > + * Return the number of records in the defer queue of the hash table
> > > + * if RCU is enabled.
> > > + * @param h
> > > + * Hash table to query from
> > > + * @return
> > > + * - -EINVAL if parameters are invalid
> > > + * - A value indicating how many records were inserted in the table.
> > did you mean how many records are kept in defer queue?
> > > + */
> > > +int32_t
> > > +rte_hash_dq_count(const struct rte_hash *h);
> > > +
> > > /**
> > > * Return the number of keys in the hash table
> > > * @param h
> > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > index 6b2afebf6b..7f7b158cf1 100644
> > > --- a/lib/hash/version.map
> > > +++ b/lib/hash/version.map
> > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > rte_hash_add_key_with_hash;
> > > rte_hash_add_key_with_hash_data;
> > > rte_hash_count;
> > > + rte_hash_dq_count;
> > new API must introduced as an experimental API. The same is true for
> > rte_rcu_qsbr_dq_count()
> > > rte_hash_crc32_alg;
> > > rte_hash_crc_set_alg;
> > > rte_hash_create;
> > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > index bd0b83be0c..89f8da4c4c 100644
> > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq
> *dq, unsigned int n,
> > > return 0;
> > > }
> > >
> > > +/**
> > > + * Return the number of entries in a defer queue.
> > > + */
> > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > +{
> Please validate dq here.
>
> > > + return rte_ring_count(dq->r);
> > > +}
> > > +
> > > /* Delete a defer queue. */
> > > int
> > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > index 23c9f89805..ed5a590edd 100644
> > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > @@ -794,6 +794,17 @@ int
> > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > unsigned int *freed, unsigned int *pending, unsigned int
> *available);
> > >
> > > +/**
> > > + * Return the number of entries in a defer queue.
> > > + *
> > > + * @param dq
> > > + * Defer queue.
> > > + * @return
> > > + * The number of entries in the defer queue.
> > > + */
> > > +unsigned int
> > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> Agree on the need for this API in RCU
>
> > > +
> > > /**
> > > * Delete a defer queue.
> > > *
> > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > index 982ffd59d9..f410ab41e7 100644
> > > --- a/lib/rcu/version.map
> > > +++ b/lib/rcu/version.map
> > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > rte_rcu_qsbr_dq_create;
> > > rte_rcu_qsbr_dq_delete;
> > > rte_rcu_qsbr_dq_enqueue;
> > > + rte_rcu_qsbr_dq_count;
> > > rte_rcu_qsbr_dq_reclaim;
> > > rte_rcu_qsbr_dump;
> > > rte_rcu_qsbr_get_memsize;
> >
> > --
> > Regards,
> > Vladimir
> >
>
>
[-- Attachment #2: Type: text/html, Size: 10903 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-20 18:58 ` Abdullah Ömer Yamaç
@ 2024-02-21 3:23 ` Honnappa Nagarahalli
2024-02-21 21:51 ` Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Honnappa Nagarahalli @ 2024-02-21 3:23 UTC (permalink / raw)
To: Abdullah Ömer Yamaç
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
> On Feb 20, 2024, at 12:58 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> I appreciate that you gave me suggestions and comments. I will make changes according to all your recommendations, but before that, I want to make everyone's minds clear. Then, I will apply modifications.
>
> On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> >
> > Hello,
> >
> > Let me explain a use case;
> >
> > I have a hash table whose key value is IP addresses, and data (let's say the username of the IP) is related to the IP address. The key point is matching these data with flows. Flows are dynamic, and this hash table is dynamic, as well; both can change anytime. For example, when a flow starts, we look up the hash table with the corresponding IP and retrieve the username. We need to hold this username until the flow terminates, although we removed this IP key from the hash table (multithread). That's why we have RCU and defer queue is necessary for high performance. In my application, I need to know the number of IP-username entries. These numbers can be calculated by rte_hash_count - defer queue size.
> The entries in the defer queue are not reclaimed (there is a probability that all of them can be reclaimed) and hence they are not available for allocation. So, rte_hash_count - defer queue size might not give you the correct number you are expecting.
>
> Currently, there is no API in hash library that forces a reclaim. Does it makes sense to have an API that just does the reclaim (and returns the number of entries pending in the defer queue)? A call to rte_hash_count should provide the exact count you are looking for.
> You are right; no API in the hash library forces a reclaim. In my application, I periodically call rte_count to retrieve hash size, and this data is shown in my GUI. So that means I need to call regularly reclaim. I am trying to figure out which is better, calling reclaim or retrieving the defer queue size. Any comment about this?
Retrieving the defer queue size will be cheaper. However, calling the reclaim API will ensure the entries are freed hence providing an accurate number. Calling the reclaim API on an empty defer queue does not consume many cycles. If needed we could add a check for empty defer queue in the reclaim API and return early.
I am also wondering if a reclaim API in hash library is needed. Why not call rte_rcu_qsbr_dq_reclaim API from the application?
> >
> > I think if you need a non-blocking and multithreaded hash table, an RCU-enabled hash table is necessary. Also, this API is necessary if you need to get the actual matchable size.
> >
> >
> >
> >
> >
> > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <vladimir.medvedkin@intel.com> wrote:
> > Hi Abdullah,
> >
> > Could you please tell more about use cases where this API may be useful?
> >
> > >a new API to get the hidden key count in the hash table if the rcu qsbr is enabled
> >
> > Here in commit message and down below in doxygen comments, I think this
> > statement should be more specific because rcu can be created with
> > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> >
> > Also, new API must be reflected in release notes
> >
> > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > This patch introduce a new API to get the hidden key count in the hash
> > > table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> > > qsbr enabled, it will return the number of elements that are not in the
> > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > > elements in the defer queue will not be counted and freed. Therefore I
> > > added a new API to get the number of hidden (defer queue) elements
> > > in the hash table. Then the user can calculate the total number of
> > > elements that are available in the hash table.
> > >
> > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > >
> > > ---
> > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > ---
> > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > lib/hash/version.map | 1 +
> > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > lib/rcu/version.map | 1 +
> > > 6 files changed, 43 insertions(+)
> > >
> > > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > > index 70456754c4..3553f3efc7 100644
> > > --- a/lib/hash/rte_cuckoo_hash.c
> > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > > return h->entries;
> > > }
> > >
> > > +int32_t
> > > +rte_hash_dq_count(const struct rte_hash *h)
> > > +{
> > > + if (h->dq == NULL)
> > input arguments must be checked since this is a public API, the same is
> > true for rte_rcu_qsbr_dq_count()
> > > + return -EINVAL;
> > why not just return 0?
> > > +
> > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > +}
> > > +
> > > int32_t
> > > rte_hash_count(const struct rte_hash *h)
> > > {
> > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > index 7ecc021111..8ea97e297d 100644
> > > --- a/lib/hash/rte_hash.h
> > > +++ b/lib/hash/rte_hash.h
> > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > void
> > > rte_hash_reset(struct rte_hash *h);
> > >
> > > +
> > > +/**
> > > + * Return the number of records in the defer queue of the hash table
> > > + * if RCU is enabled.
> > > + * @param h
> > > + * Hash table to query from
> > > + * @return
> > > + * - -EINVAL if parameters are invalid
> > > + * - A value indicating how many records were inserted in the table.
> > did you mean how many records are kept in defer queue?
> > > + */
> > > +int32_t
> > > +rte_hash_dq_count(const struct rte_hash *h);
> > > +
> > > /**
> > > * Return the number of keys in the hash table
> > > * @param h
> > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > index 6b2afebf6b..7f7b158cf1 100644
> > > --- a/lib/hash/version.map
> > > +++ b/lib/hash/version.map
> > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > rte_hash_add_key_with_hash;
> > > rte_hash_add_key_with_hash_data;
> > > rte_hash_count;
> > > + rte_hash_dq_count;
> > new API must introduced as an experimental API. The same is true for
> > rte_rcu_qsbr_dq_count()
> > > rte_hash_crc32_alg;
> > > rte_hash_crc_set_alg;
> > > rte_hash_create;
> > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > index bd0b83be0c..89f8da4c4c 100644
> > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > return 0;
> > > }
> > >
> > > +/**
> > > + * Return the number of entries in a defer queue.
> > > + */
> > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > +{
> Please validate dq here.
>
> > > + return rte_ring_count(dq->r);
> > > +}
> > > +
> > > /* Delete a defer queue. */
> > > int
> > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > index 23c9f89805..ed5a590edd 100644
> > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > @@ -794,6 +794,17 @@ int
> > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > unsigned int *freed, unsigned int *pending, unsigned int *available);
> > >
> > > +/**
> > > + * Return the number of entries in a defer queue.
> > > + *
> > > + * @param dq
> > > + * Defer queue.
> > > + * @return
> > > + * The number of entries in the defer queue.
> > > + */
> > > +unsigned int
> > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> Agree on the need for this API in RCU
>
> > > +
> > > /**
> > > * Delete a defer queue.
> > > *
> > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > index 982ffd59d9..f410ab41e7 100644
> > > --- a/lib/rcu/version.map
> > > +++ b/lib/rcu/version.map
> > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > rte_rcu_qsbr_dq_create;
> > > rte_rcu_qsbr_dq_delete;
> > > rte_rcu_qsbr_dq_enqueue;
> > > + rte_rcu_qsbr_dq_count;
> > > rte_rcu_qsbr_dq_reclaim;
> > > rte_rcu_qsbr_dump;
> > > rte_rcu_qsbr_get_memsize;
> >
> > --
> > Regards,
> > Vladimir
> >
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-21 3:23 ` Honnappa Nagarahalli
@ 2024-02-21 21:51 ` Abdullah Ömer Yamaç
2024-02-22 2:37 ` Honnappa Nagarahalli
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-02-21 21:51 UTC (permalink / raw)
To: Honnappa Nagarahalli
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
[-- Attachment #1: Type: text/plain, Size: 9699 bytes --]
On Wed, Feb 21, 2024 at 6:24 AM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 20, 2024, at 12:58 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> >
> > I appreciate that you gave me suggestions and comments. I will make
> changes according to all your recommendations, but before that, I want to
> make everyone's minds clear. Then, I will apply modifications.
> >
> > On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> >
> >
> > > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> > >
> > > Hello,
> > >
> > > Let me explain a use case;
> > >
> > > I have a hash table whose key value is IP addresses, and data (let's
> say the username of the IP) is related to the IP address. The key point is
> matching these data with flows. Flows are dynamic, and this hash table is
> dynamic, as well; both can change anytime. For example, when a flow starts,
> we look up the hash table with the corresponding IP and retrieve the
> username. We need to hold this username until the flow terminates, although
> we removed this IP key from the hash table (multithread). That's why we
> have RCU and defer queue is necessary for high performance. In my
> application, I need to know the number of IP-username entries. These
> numbers can be calculated by rte_hash_count - defer queue size.
> > The entries in the defer queue are not reclaimed (there is a probability
> that all of them can be reclaimed) and hence they are not available for
> allocation. So, rte_hash_count - defer queue size might not give you the
> correct number you are expecting.
> >
> > Currently, there is no API in hash library that forces a reclaim. Does
> it makes sense to have an API that just does the reclaim (and returns the
> number of entries pending in the defer queue)? A call to rte_hash_count
> should provide the exact count you are looking for.
> > You are right; no API in the hash library forces a reclaim. In my
> application, I periodically call rte_count to retrieve hash size, and this
> data is shown in my GUI. So that means I need to call regularly reclaim. I
> am trying to figure out which is better, calling reclaim or retrieving the
> defer queue size. Any comment about this?
> Retrieving the defer queue size will be cheaper. However, calling the
> reclaim API will ensure the entries are freed hence providing an accurate
> number. Calling the reclaim API on an empty defer queue does not consume
> many cycles. If needed we could add a check for empty defer queue in the
> reclaim API and return early.
>
> I am also wondering if a reclaim API in hash library is needed. Why not
> call rte_rcu_qsbr_dq_reclaim API from the application?
>
The reason is simple. struct rte_hash *h is an internal structure and we
cannot access the h->dq. So it is not possible to call reclaim.
>
>
> > >
> > > I think if you need a non-blocking and multithreaded hash table, an
> RCU-enabled hash table is necessary. Also, this API is necessary if you
> need to get the actual matchable size.
> > >
> > >
> > >
> > >
> > >
> > > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <
> vladimir.medvedkin@intel.com> wrote:
> > > Hi Abdullah,
> > >
> > > Could you please tell more about use cases where this API may be
> useful?
> > >
> > > >a new API to get the hidden key count in the hash table if the rcu
> qsbr is enabled
> > >
> > > Here in commit message and down below in doxygen comments, I think
> this
> > > statement should be more specific because rcu can be created with
> > > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> > >
> > > Also, new API must be reflected in release notes
> > >
> > > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > > This patch introduce a new API to get the hidden key count in the
> hash
> > > > table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> > > > qsbr enabled, it will return the number of elements that are not in
> the
> > > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > > > elements in the defer queue will not be counted and freed. Therefore
> I
> > > > added a new API to get the number of hidden (defer queue) elements
> > > > in the hash table. Then the user can calculate the total number of
> > > > elements that are available in the hash table.
> > > >
> > > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > > >
> > > > ---
> > > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > > ---
> > > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > > lib/hash/version.map | 1 +
> > > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > > lib/rcu/version.map | 1 +
> > > > 6 files changed, 43 insertions(+)
> > > >
> > > > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > > > index 70456754c4..3553f3efc7 100644
> > > > --- a/lib/hash/rte_cuckoo_hash.c
> > > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > > > return h->entries;
> > > > }
> > > >
> > > > +int32_t
> > > > +rte_hash_dq_count(const struct rte_hash *h)
> > > > +{
> > > > + if (h->dq == NULL)
> > > input arguments must be checked since this is a public API, the same
> is
> > > true for rte_rcu_qsbr_dq_count()
> > > > + return -EINVAL;
> > > why not just return 0?
> > > > +
> > > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > > +}
> > > > +
> > > > int32_t
> > > > rte_hash_count(const struct rte_hash *h)
> > > > {
> > > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > > index 7ecc021111..8ea97e297d 100644
> > > > --- a/lib/hash/rte_hash.h
> > > > +++ b/lib/hash/rte_hash.h
> > > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > > void
> > > > rte_hash_reset(struct rte_hash *h);
> > > >
> > > > +
> > > > +/**
> > > > + * Return the number of records in the defer queue of the hash table
> > > > + * if RCU is enabled.
> > > > + * @param h
> > > > + * Hash table to query from
> > > > + * @return
> > > > + * - -EINVAL if parameters are invalid
> > > > + * - A value indicating how many records were inserted in the
> table.
> > > did you mean how many records are kept in defer queue?
> > > > + */
> > > > +int32_t
> > > > +rte_hash_dq_count(const struct rte_hash *h);
> > > > +
> > > > /**
> > > > * Return the number of keys in the hash table
> > > > * @param h
> > > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > > index 6b2afebf6b..7f7b158cf1 100644
> > > > --- a/lib/hash/version.map
> > > > +++ b/lib/hash/version.map
> > > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > > rte_hash_add_key_with_hash;
> > > > rte_hash_add_key_with_hash_data;
> > > > rte_hash_count;
> > > > + rte_hash_dq_count;
> > > new API must introduced as an experimental API. The same is true for
> > > rte_rcu_qsbr_dq_count()
> > > > rte_hash_crc32_alg;
> > > > rte_hash_crc_set_alg;
> > > > rte_hash_create;
> > > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > > index bd0b83be0c..89f8da4c4c 100644
> > > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq
> *dq, unsigned int n,
> > > > return 0;
> > > > }
> > > >
> > > > +/**
> > > > + * Return the number of entries in a defer queue.
> > > > + */
> > > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > > +{
> > Please validate dq here.
> >
> > > > + return rte_ring_count(dq->r);
> > > > +}
> > > > +
> > > > /* Delete a defer queue. */
> > > > int
> > > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > > index 23c9f89805..ed5a590edd 100644
> > > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > > @@ -794,6 +794,17 @@ int
> > > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > unsigned int *freed, unsigned int *pending, unsigned int
> *available);
> > > >
> > > > +/**
> > > > + * Return the number of entries in a defer queue.
> > > > + *
> > > > + * @param dq
> > > > + * Defer queue.
> > > > + * @return
> > > > + * The number of entries in the defer queue.
> > > > + */
> > > > +unsigned int
> > > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> > Agree on the need for this API in RCU
> >
> > > > +
> > > > /**
> > > > * Delete a defer queue.
> > > > *
> > > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > > index 982ffd59d9..f410ab41e7 100644
> > > > --- a/lib/rcu/version.map
> > > > +++ b/lib/rcu/version.map
> > > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > > rte_rcu_qsbr_dq_create;
> > > > rte_rcu_qsbr_dq_delete;
> > > > rte_rcu_qsbr_dq_enqueue;
> > > > + rte_rcu_qsbr_dq_count;
> > > > rte_rcu_qsbr_dq_reclaim;
> > > > rte_rcu_qsbr_dump;
> > > > rte_rcu_qsbr_get_memsize;
> > >
> > > --
> > > Regards,
> > > Vladimir
> > >
> >
>
>
[-- Attachment #2: Type: text/html, Size: 12843 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-21 21:51 ` Abdullah Ömer Yamaç
@ 2024-02-22 2:37 ` Honnappa Nagarahalli
2024-02-22 12:39 ` Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Honnappa Nagarahalli @ 2024-02-22 2:37 UTC (permalink / raw)
To: Abdullah Ömer Yamaç
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
> On Feb 21, 2024, at 3:51 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
>
>
> On Wed, Feb 21, 2024 at 6:24 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 20, 2024, at 12:58 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> >
> > I appreciate that you gave me suggestions and comments. I will make changes according to all your recommendations, but before that, I want to make everyone's minds clear. Then, I will apply modifications.
> >
> > On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> >
> >
> > > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> > >
> > > Hello,
> > >
> > > Let me explain a use case;
> > >
> > > I have a hash table whose key value is IP addresses, and data (let's say the username of the IP) is related to the IP address. The key point is matching these data with flows. Flows are dynamic, and this hash table is dynamic, as well; both can change anytime. For example, when a flow starts, we look up the hash table with the corresponding IP and retrieve the username. We need to hold this username until the flow terminates, although we removed this IP key from the hash table (multithread). That's why we have RCU and defer queue is necessary for high performance. In my application, I need to know the number of IP-username entries. These numbers can be calculated by rte_hash_count - defer queue size.
> > The entries in the defer queue are not reclaimed (there is a probability that all of them can be reclaimed) and hence they are not available for allocation. So, rte_hash_count - defer queue size might not give you the correct number you are expecting.
> >
> > Currently, there is no API in hash library that forces a reclaim. Does it makes sense to have an API that just does the reclaim (and returns the number of entries pending in the defer queue)? A call to rte_hash_count should provide the exact count you are looking for.
> > You are right; no API in the hash library forces a reclaim. In my application, I periodically call rte_count to retrieve hash size, and this data is shown in my GUI. So that means I need to call regularly reclaim. I am trying to figure out which is better, calling reclaim or retrieving the defer queue size. Any comment about this?
> Retrieving the defer queue size will be cheaper. However, calling the reclaim API will ensure the entries are freed hence providing an accurate number. Calling the reclaim API on an empty defer queue does not consume many cycles. If needed we could add a check for empty defer queue in the reclaim API and return early.
>
> I am also wondering if a reclaim API in hash library is needed. Why not call rte_rcu_qsbr_dq_reclaim API from the application?
> The reason is simple. struct rte_hash *h is an internal structure and we cannot access the h->dq. So it is not possible to call reclaim.
Ack. This will be just a wrapper around the rte_rcu_qsbr_dq_reclaim.
>
>
> > >
> > > I think if you need a non-blocking and multithreaded hash table, an RCU-enabled hash table is necessary. Also, this API is necessary if you need to get the actual matchable size.
> > >
> > >
> > >
> > >
> > >
> > > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <vladimir.medvedkin@intel.com> wrote:
> > > Hi Abdullah,
> > >
> > > Could you please tell more about use cases where this API may be useful?
> > >
> > > >a new API to get the hidden key count in the hash table if the rcu qsbr is enabled
> > >
> > > Here in commit message and down below in doxygen comments, I think this
> > > statement should be more specific because rcu can be created with
> > > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> > >
> > > Also, new API must be reflected in release notes
> > >
> > > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > > This patch introduce a new API to get the hidden key count in the hash
> > > > table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> > > > qsbr enabled, it will return the number of elements that are not in the
> > > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > > > elements in the defer queue will not be counted and freed. Therefore I
> > > > added a new API to get the number of hidden (defer queue) elements
> > > > in the hash table. Then the user can calculate the total number of
> > > > elements that are available in the hash table.
> > > >
> > > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > > >
> > > > ---
> > > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > > ---
> > > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > > lib/hash/version.map | 1 +
> > > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > > lib/rcu/version.map | 1 +
> > > > 6 files changed, 43 insertions(+)
> > > >
> > > > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > > > index 70456754c4..3553f3efc7 100644
> > > > --- a/lib/hash/rte_cuckoo_hash.c
> > > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > > > return h->entries;
> > > > }
> > > >
> > > > +int32_t
> > > > +rte_hash_dq_count(const struct rte_hash *h)
> > > > +{
> > > > + if (h->dq == NULL)
> > > input arguments must be checked since this is a public API, the same is
> > > true for rte_rcu_qsbr_dq_count()
> > > > + return -EINVAL;
> > > why not just return 0?
> > > > +
> > > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > > +}
> > > > +
> > > > int32_t
> > > > rte_hash_count(const struct rte_hash *h)
> > > > {
> > > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > > index 7ecc021111..8ea97e297d 100644
> > > > --- a/lib/hash/rte_hash.h
> > > > +++ b/lib/hash/rte_hash.h
> > > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > > void
> > > > rte_hash_reset(struct rte_hash *h);
> > > >
> > > > +
> > > > +/**
> > > > + * Return the number of records in the defer queue of the hash table
> > > > + * if RCU is enabled.
> > > > + * @param h
> > > > + * Hash table to query from
> > > > + * @return
> > > > + * - -EINVAL if parameters are invalid
> > > > + * - A value indicating how many records were inserted in the table.
> > > did you mean how many records are kept in defer queue?
> > > > + */
> > > > +int32_t
> > > > +rte_hash_dq_count(const struct rte_hash *h);
> > > > +
> > > > /**
> > > > * Return the number of keys in the hash table
> > > > * @param h
> > > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > > index 6b2afebf6b..7f7b158cf1 100644
> > > > --- a/lib/hash/version.map
> > > > +++ b/lib/hash/version.map
> > > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > > rte_hash_add_key_with_hash;
> > > > rte_hash_add_key_with_hash_data;
> > > > rte_hash_count;
> > > > + rte_hash_dq_count;
> > > new API must introduced as an experimental API. The same is true for
> > > rte_rcu_qsbr_dq_count()
> > > > rte_hash_crc32_alg;
> > > > rte_hash_crc_set_alg;
> > > > rte_hash_create;
> > > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > > index bd0b83be0c..89f8da4c4c 100644
> > > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > return 0;
> > > > }
> > > >
> > > > +/**
> > > > + * Return the number of entries in a defer queue.
> > > > + */
> > > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > > +{
> > Please validate dq here.
> >
> > > > + return rte_ring_count(dq->r);
> > > > +}
> > > > +
> > > > /* Delete a defer queue. */
> > > > int
> > > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > > index 23c9f89805..ed5a590edd 100644
> > > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > > @@ -794,6 +794,17 @@ int
> > > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > unsigned int *freed, unsigned int *pending, unsigned int *available);
> > > >
> > > > +/**
> > > > + * Return the number of entries in a defer queue.
> > > > + *
> > > > + * @param dq
> > > > + * Defer queue.
> > > > + * @return
> > > > + * The number of entries in the defer queue.
> > > > + */
> > > > +unsigned int
> > > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> > Agree on the need for this API in RCU
> >
> > > > +
> > > > /**
> > > > * Delete a defer queue.
> > > > *
> > > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > > index 982ffd59d9..f410ab41e7 100644
> > > > --- a/lib/rcu/version.map
> > > > +++ b/lib/rcu/version.map
> > > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > > rte_rcu_qsbr_dq_create;
> > > > rte_rcu_qsbr_dq_delete;
> > > > rte_rcu_qsbr_dq_enqueue;
> > > > + rte_rcu_qsbr_dq_count;
> > > > rte_rcu_qsbr_dq_reclaim;
> > > > rte_rcu_qsbr_dump;
> > > > rte_rcu_qsbr_get_memsize;
> > >
> > > --
> > > Regards,
> > > Vladimir
> > >
> >
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-22 2:37 ` Honnappa Nagarahalli
@ 2024-02-22 12:39 ` Abdullah Ömer Yamaç
2024-02-22 16:44 ` Honnappa Nagarahalli
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-02-22 12:39 UTC (permalink / raw)
To: Honnappa Nagarahalli
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
[-- Attachment #1: Type: text/plain, Size: 10501 bytes --]
As a final decision, I will add a new hash API that forces the reclaim. Is
it ok for everyone?
On Thu, Feb 22, 2024 at 5:37 AM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 21, 2024, at 3:51 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> >
> >
> >
> > On Wed, Feb 21, 2024 at 6:24 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> >
> >
> > > On Feb 20, 2024, at 12:58 PM, Abdullah Ömer Yamaç <
> aomeryamac@gmail.com> wrote:
> > >
> > > I appreciate that you gave me suggestions and comments. I will make
> changes according to all your recommendations, but before that, I want to
> make everyone's minds clear. Then, I will apply modifications.
> > >
> > > On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> > >
> > >
> > > > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <
> aomeryamac@gmail.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > Let me explain a use case;
> > > >
> > > > I have a hash table whose key value is IP addresses, and data (let's
> say the username of the IP) is related to the IP address. The key point is
> matching these data with flows. Flows are dynamic, and this hash table is
> dynamic, as well; both can change anytime. For example, when a flow starts,
> we look up the hash table with the corresponding IP and retrieve the
> username. We need to hold this username until the flow terminates, although
> we removed this IP key from the hash table (multithread). That's why we
> have RCU and defer queue is necessary for high performance. In my
> application, I need to know the number of IP-username entries. These
> numbers can be calculated by rte_hash_count - defer queue size.
> > > The entries in the defer queue are not reclaimed (there is a
> probability that all of them can be reclaimed) and hence they are not
> available for allocation. So, rte_hash_count - defer queue size might not
> give you the correct number you are expecting.
> > >
> > > Currently, there is no API in hash library that forces a reclaim. Does
> it makes sense to have an API that just does the reclaim (and returns the
> number of entries pending in the defer queue)? A call to rte_hash_count
> should provide the exact count you are looking for.
> > > You are right; no API in the hash library forces a reclaim. In my
> application, I periodically call rte_count to retrieve hash size, and this
> data is shown in my GUI. So that means I need to call regularly reclaim. I
> am trying to figure out which is better, calling reclaim or retrieving the
> defer queue size. Any comment about this?
> > Retrieving the defer queue size will be cheaper. However, calling the
> reclaim API will ensure the entries are freed hence providing an accurate
> number. Calling the reclaim API on an empty defer queue does not consume
> many cycles. If needed we could add a check for empty defer queue in the
> reclaim API and return early.
> >
> > I am also wondering if a reclaim API in hash library is needed. Why not
> call rte_rcu_qsbr_dq_reclaim API from the application?
> > The reason is simple. struct rte_hash *h is an internal structure and we
> cannot access the h->dq. So it is not possible to call reclaim.
> Ack. This will be just a wrapper around the rte_rcu_qsbr_dq_reclaim.
>
> >
> >
> > > >
> > > > I think if you need a non-blocking and multithreaded hash table, an
> RCU-enabled hash table is necessary. Also, this API is necessary if you
> need to get the actual matchable size.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <
> vladimir.medvedkin@intel.com> wrote:
> > > > Hi Abdullah,
> > > >
> > > > Could you please tell more about use cases where this API may be
> useful?
> > > >
> > > > >a new API to get the hidden key count in the hash table if the rcu
> qsbr is enabled
> > > >
> > > > Here in commit message and down below in doxygen comments, I think
> this
> > > > statement should be more specific because rcu can be created with
> > > > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> > > >
> > > > Also, new API must be reflected in release notes
> > > >
> > > > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > > > This patch introduce a new API to get the hidden key count in the
> hash
> > > > > table if the rcu qsbr is enabled. When using rte_hash_count with
> rcu
> > > > > qsbr enabled, it will return the number of elements that are not
> in the
> > > > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > > > > elements in the defer queue will not be counted and freed.
> Therefore I
> > > > > added a new API to get the number of hidden (defer queue) elements
> > > > > in the hash table. Then the user can calculate the total number of
> > > > > elements that are available in the hash table.
> > > > >
> > > > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > > > >
> > > > > ---
> > > > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > > > ---
> > > > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > > > lib/hash/version.map | 1 +
> > > > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > > > lib/rcu/version.map | 1 +
> > > > > 6 files changed, 43 insertions(+)
> > > > >
> > > > > diff --git a/lib/hash/rte_cuckoo_hash.c
> b/lib/hash/rte_cuckoo_hash.c
> > > > > index 70456754c4..3553f3efc7 100644
> > > > > --- a/lib/hash/rte_cuckoo_hash.c
> > > > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > > > > return h->entries;
> > > > > }
> > > > >
> > > > > +int32_t
> > > > > +rte_hash_dq_count(const struct rte_hash *h)
> > > > > +{
> > > > > + if (h->dq == NULL)
> > > > input arguments must be checked since this is a public API, the same
> is
> > > > true for rte_rcu_qsbr_dq_count()
> > > > > + return -EINVAL;
> > > > why not just return 0?
> > > > > +
> > > > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > > > +}
> > > > > +
> > > > > int32_t
> > > > > rte_hash_count(const struct rte_hash *h)
> > > > > {
> > > > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > > > index 7ecc021111..8ea97e297d 100644
> > > > > --- a/lib/hash/rte_hash.h
> > > > > +++ b/lib/hash/rte_hash.h
> > > > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > > > void
> > > > > rte_hash_reset(struct rte_hash *h);
> > > > >
> > > > > +
> > > > > +/**
> > > > > + * Return the number of records in the defer queue of the hash
> table
> > > > > + * if RCU is enabled.
> > > > > + * @param h
> > > > > + * Hash table to query from
> > > > > + * @return
> > > > > + * - -EINVAL if parameters are invalid
> > > > > + * - A value indicating how many records were inserted in the
> table.
> > > > did you mean how many records are kept in defer queue?
> > > > > + */
> > > > > +int32_t
> > > > > +rte_hash_dq_count(const struct rte_hash *h);
> > > > > +
> > > > > /**
> > > > > * Return the number of keys in the hash table
> > > > > * @param h
> > > > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > > > index 6b2afebf6b..7f7b158cf1 100644
> > > > > --- a/lib/hash/version.map
> > > > > +++ b/lib/hash/version.map
> > > > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > > > rte_hash_add_key_with_hash;
> > > > > rte_hash_add_key_with_hash_data;
> > > > > rte_hash_count;
> > > > > + rte_hash_dq_count;
> > > > new API must introduced as an experimental API. The same is true for
> > > > rte_rcu_qsbr_dq_count()
> > > > > rte_hash_crc32_alg;
> > > > > rte_hash_crc_set_alg;
> > > > > rte_hash_create;
> > > > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > > > index bd0b83be0c..89f8da4c4c 100644
> > > > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct
> rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +/**
> > > > > + * Return the number of entries in a defer queue.
> > > > > + */
> > > > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > > > +{
> > > Please validate dq here.
> > >
> > > > > + return rte_ring_count(dq->r);
> > > > > +}
> > > > > +
> > > > > /* Delete a defer queue. */
> > > > > int
> > > > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > > > index 23c9f89805..ed5a590edd 100644
> > > > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > > > @@ -794,6 +794,17 @@ int
> > > > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int
> n,
> > > > > unsigned int *freed, unsigned int *pending, unsigned int
> *available);
> > > > >
> > > > > +/**
> > > > > + * Return the number of entries in a defer queue.
> > > > > + *
> > > > > + * @param dq
> > > > > + * Defer queue.
> > > > > + * @return
> > > > > + * The number of entries in the defer queue.
> > > > > + */
> > > > > +unsigned int
> > > > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> > > Agree on the need for this API in RCU
> > >
> > > > > +
> > > > > /**
> > > > > * Delete a defer queue.
> > > > > *
> > > > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > > > index 982ffd59d9..f410ab41e7 100644
> > > > > --- a/lib/rcu/version.map
> > > > > +++ b/lib/rcu/version.map
> > > > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > > > rte_rcu_qsbr_dq_create;
> > > > > rte_rcu_qsbr_dq_delete;
> > > > > rte_rcu_qsbr_dq_enqueue;
> > > > > + rte_rcu_qsbr_dq_count;
> > > > > rte_rcu_qsbr_dq_reclaim;
> > > > > rte_rcu_qsbr_dump;
> > > > > rte_rcu_qsbr_get_memsize;
> > > >
> > > > --
> > > > Regards,
> > > > Vladimir
> > > >
> > >
> >
>
>
[-- Attachment #2: Type: text/html, Size: 14231 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-22 12:39 ` Abdullah Ömer Yamaç
@ 2024-02-22 16:44 ` Honnappa Nagarahalli
2024-02-28 11:44 ` Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Honnappa Nagarahalli @ 2024-02-22 16:44 UTC (permalink / raw)
To: Abdullah Ömer Yamaç
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
> On Feb 22, 2024, at 6:39 AM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> As a final decision, I will add a new hash API that forces the reclaim. Is it ok for everyone?
Ack from my side
>
> On Thu, Feb 22, 2024 at 5:37 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 21, 2024, at 3:51 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> >
> >
> >
> > On Wed, Feb 21, 2024 at 6:24 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> >
> >
> > > On Feb 20, 2024, at 12:58 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> > >
> > > I appreciate that you gave me suggestions and comments. I will make changes according to all your recommendations, but before that, I want to make everyone's minds clear. Then, I will apply modifications.
> > >
> > > On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> > >
> > >
> > > > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > Let me explain a use case;
> > > >
> > > > I have a hash table whose key value is IP addresses, and data (let's say the username of the IP) is related to the IP address. The key point is matching these data with flows. Flows are dynamic, and this hash table is dynamic, as well; both can change anytime. For example, when a flow starts, we look up the hash table with the corresponding IP and retrieve the username. We need to hold this username until the flow terminates, although we removed this IP key from the hash table (multithread). That's why we have RCU and defer queue is necessary for high performance. In my application, I need to know the number of IP-username entries. These numbers can be calculated by rte_hash_count - defer queue size.
> > > The entries in the defer queue are not reclaimed (there is a probability that all of them can be reclaimed) and hence they are not available for allocation. So, rte_hash_count - defer queue size might not give you the correct number you are expecting.
> > >
> > > Currently, there is no API in hash library that forces a reclaim. Does it makes sense to have an API that just does the reclaim (and returns the number of entries pending in the defer queue)? A call to rte_hash_count should provide the exact count you are looking for.
> > > You are right; no API in the hash library forces a reclaim. In my application, I periodically call rte_count to retrieve hash size, and this data is shown in my GUI. So that means I need to call regularly reclaim. I am trying to figure out which is better, calling reclaim or retrieving the defer queue size. Any comment about this?
> > Retrieving the defer queue size will be cheaper. However, calling the reclaim API will ensure the entries are freed hence providing an accurate number. Calling the reclaim API on an empty defer queue does not consume many cycles. If needed we could add a check for empty defer queue in the reclaim API and return early.
> >
> > I am also wondering if a reclaim API in hash library is needed. Why not call rte_rcu_qsbr_dq_reclaim API from the application?
> > The reason is simple. struct rte_hash *h is an internal structure and we cannot access the h->dq. So it is not possible to call reclaim.
> Ack. This will be just a wrapper around the rte_rcu_qsbr_dq_reclaim.
>
> >
> >
> > > >
> > > > I think if you need a non-blocking and multithreaded hash table, an RCU-enabled hash table is necessary. Also, this API is necessary if you need to get the actual matchable size.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <vladimir.medvedkin@intel.com> wrote:
> > > > Hi Abdullah,
> > > >
> > > > Could you please tell more about use cases where this API may be useful?
> > > >
> > > > >a new API to get the hidden key count in the hash table if the rcu qsbr is enabled
> > > >
> > > > Here in commit message and down below in doxygen comments, I think this
> > > > statement should be more specific because rcu can be created with
> > > > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> > > >
> > > > Also, new API must be reflected in release notes
> > > >
> > > > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > > > This patch introduce a new API to get the hidden key count in the hash
> > > > > table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> > > > > qsbr enabled, it will return the number of elements that are not in the
> > > > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > > > > elements in the defer queue will not be counted and freed. Therefore I
> > > > > added a new API to get the number of hidden (defer queue) elements
> > > > > in the hash table. Then the user can calculate the total number of
> > > > > elements that are available in the hash table.
> > > > >
> > > > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > > > >
> > > > > ---
> > > > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > > > ---
> > > > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > > > lib/hash/version.map | 1 +
> > > > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > > > lib/rcu/version.map | 1 +
> > > > > 6 files changed, 43 insertions(+)
> > > > >
> > > > > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > > > > index 70456754c4..3553f3efc7 100644
> > > > > --- a/lib/hash/rte_cuckoo_hash.c
> > > > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > > > > return h->entries;
> > > > > }
> > > > >
> > > > > +int32_t
> > > > > +rte_hash_dq_count(const struct rte_hash *h)
> > > > > +{
> > > > > + if (h->dq == NULL)
> > > > input arguments must be checked since this is a public API, the same is
> > > > true for rte_rcu_qsbr_dq_count()
> > > > > + return -EINVAL;
> > > > why not just return 0?
> > > > > +
> > > > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > > > +}
> > > > > +
> > > > > int32_t
> > > > > rte_hash_count(const struct rte_hash *h)
> > > > > {
> > > > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > > > index 7ecc021111..8ea97e297d 100644
> > > > > --- a/lib/hash/rte_hash.h
> > > > > +++ b/lib/hash/rte_hash.h
> > > > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > > > void
> > > > > rte_hash_reset(struct rte_hash *h);
> > > > >
> > > > > +
> > > > > +/**
> > > > > + * Return the number of records in the defer queue of the hash table
> > > > > + * if RCU is enabled.
> > > > > + * @param h
> > > > > + * Hash table to query from
> > > > > + * @return
> > > > > + * - -EINVAL if parameters are invalid
> > > > > + * - A value indicating how many records were inserted in the table.
> > > > did you mean how many records are kept in defer queue?
> > > > > + */
> > > > > +int32_t
> > > > > +rte_hash_dq_count(const struct rte_hash *h);
> > > > > +
> > > > > /**
> > > > > * Return the number of keys in the hash table
> > > > > * @param h
> > > > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > > > index 6b2afebf6b..7f7b158cf1 100644
> > > > > --- a/lib/hash/version.map
> > > > > +++ b/lib/hash/version.map
> > > > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > > > rte_hash_add_key_with_hash;
> > > > > rte_hash_add_key_with_hash_data;
> > > > > rte_hash_count;
> > > > > + rte_hash_dq_count;
> > > > new API must introduced as an experimental API. The same is true for
> > > > rte_rcu_qsbr_dq_count()
> > > > > rte_hash_crc32_alg;
> > > > > rte_hash_crc_set_alg;
> > > > > rte_hash_create;
> > > > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > > > index bd0b83be0c..89f8da4c4c 100644
> > > > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +/**
> > > > > + * Return the number of entries in a defer queue.
> > > > > + */
> > > > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > > > +{
> > > Please validate dq here.
> > >
> > > > > + return rte_ring_count(dq->r);
> > > > > +}
> > > > > +
> > > > > /* Delete a defer queue. */
> > > > > int
> > > > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > > > index 23c9f89805..ed5a590edd 100644
> > > > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > > > @@ -794,6 +794,17 @@ int
> > > > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > > unsigned int *freed, unsigned int *pending, unsigned int *available);
> > > > >
> > > > > +/**
> > > > > + * Return the number of entries in a defer queue.
> > > > > + *
> > > > > + * @param dq
> > > > > + * Defer queue.
> > > > > + * @return
> > > > > + * The number of entries in the defer queue.
> > > > > + */
> > > > > +unsigned int
> > > > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> > > Agree on the need for this API in RCU
> > >
> > > > > +
> > > > > /**
> > > > > * Delete a defer queue.
> > > > > *
> > > > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > > > index 982ffd59d9..f410ab41e7 100644
> > > > > --- a/lib/rcu/version.map
> > > > > +++ b/lib/rcu/version.map
> > > > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > > > rte_rcu_qsbr_dq_create;
> > > > > rte_rcu_qsbr_dq_delete;
> > > > > rte_rcu_qsbr_dq_enqueue;
> > > > > + rte_rcu_qsbr_dq_count;
> > > > > rte_rcu_qsbr_dq_reclaim;
> > > > > rte_rcu_qsbr_dump;
> > > > > rte_rcu_qsbr_get_memsize;
> > > >
> > > > --
> > > > Regards,
> > > > Vladimir
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-22 16:44 ` Honnappa Nagarahalli
@ 2024-02-28 11:44 ` Abdullah Ömer Yamaç
2024-02-28 14:51 ` Honnappa Nagarahalli
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-02-28 11:44 UTC (permalink / raw)
To: Honnappa Nagarahalli
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
[-- Attachment #1: Type: text/plain, Size: 12265 bytes --]
While I was implementing the new API, I realized one issue, and it would be
good to discuss it here. First of all rte_rcu_qsbr_dq_reclaim function
checks the state of the qsbr values. It means that all threads should
report the quiescent states. It conflicts with my aim.
Let's think about below scenario:
Eight threads use a hash table and periodically report their quiescent
states. One additional thread (main thread) periodically reports the hash
size. I implemented the reclaim function in that thread. I mean, the main
thread calls reclaim before the rte_hash_count.
Here is the exceptional case that I couldn't retrieve the correct hash size:
Assume that 6 of 8 threads reported quiescent states and 2 of them are
still working on some process and haven't reported quiescent states yet.
The main thread calls reclaim functions every time, but elements in dq will
not be freed because 2 of the worker threads haven't reported their states
(especially if they are waiting for some packets). So, my first proposed
method is more suitable for this case. Any idea?
On Thu, Feb 22, 2024 at 7:44 PM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 22, 2024, at 6:39 AM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> >
> > As a final decision, I will add a new hash API that forces the reclaim.
> Is it ok for everyone?
> Ack from my side
>
> >
> > On Thu, Feb 22, 2024 at 5:37 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> >
> >
> > > On Feb 21, 2024, at 3:51 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> > >
> > >
> > >
> > > On Wed, Feb 21, 2024 at 6:24 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> > >
> > >
> > > > On Feb 20, 2024, at 12:58 PM, Abdullah Ömer Yamaç <
> aomeryamac@gmail.com> wrote:
> > > >
> > > > I appreciate that you gave me suggestions and comments. I will make
> changes according to all your recommendations, but before that, I want to
> make everyone's minds clear. Then, I will apply modifications.
> > > >
> > > > On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> > > >
> > > >
> > > > > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <
> aomeryamac@gmail.com> wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > Let me explain a use case;
> > > > >
> > > > > I have a hash table whose key value is IP addresses, and data
> (let's say the username of the IP) is related to the IP address. The key
> point is matching these data with flows. Flows are dynamic, and this hash
> table is dynamic, as well; both can change anytime. For example, when a
> flow starts, we look up the hash table with the corresponding IP and
> retrieve the username. We need to hold this username until the flow
> terminates, although we removed this IP key from the hash table
> (multithread). That's why we have RCU and defer queue is necessary for high
> performance. In my application, I need to know the number of IP-username
> entries. These numbers can be calculated by rte_hash_count - defer queue
> size.
> > > > The entries in the defer queue are not reclaimed (there is a
> probability that all of them can be reclaimed) and hence they are not
> available for allocation. So, rte_hash_count - defer queue size might not
> give you the correct number you are expecting.
> > > >
> > > > Currently, there is no API in hash library that forces a reclaim.
> Does it makes sense to have an API that just does the reclaim (and returns
> the number of entries pending in the defer queue)? A call to rte_hash_count
> should provide the exact count you are looking for.
> > > > You are right; no API in the hash library forces a reclaim. In my
> application, I periodically call rte_count to retrieve hash size, and this
> data is shown in my GUI. So that means I need to call regularly reclaim. I
> am trying to figure out which is better, calling reclaim or retrieving the
> defer queue size. Any comment about this?
> > > Retrieving the defer queue size will be cheaper. However, calling the
> reclaim API will ensure the entries are freed hence providing an accurate
> number. Calling the reclaim API on an empty defer queue does not consume
> many cycles. If needed we could add a check for empty defer queue in the
> reclaim API and return early.
> > >
> > > I am also wondering if a reclaim API in hash library is needed. Why
> not call rte_rcu_qsbr_dq_reclaim API from the application?
> > > The reason is simple. struct rte_hash *h is an internal structure and
> we cannot access the h->dq. So it is not possible to call reclaim.
> > Ack. This will be just a wrapper around the rte_rcu_qsbr_dq_reclaim.
> >
> > >
> > >
> > > > >
> > > > > I think if you need a non-blocking and multithreaded hash table,
> an RCU-enabled hash table is necessary. Also, this API is necessary if you
> need to get the actual matchable size.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <
> vladimir.medvedkin@intel.com> wrote:
> > > > > Hi Abdullah,
> > > > >
> > > > > Could you please tell more about use cases where this API may be
> useful?
> > > > >
> > > > > >a new API to get the hidden key count in the hash table if the
> rcu qsbr is enabled
> > > > >
> > > > > Here in commit message and down below in doxygen comments, I think
> this
> > > > > statement should be more specific because rcu can be created with
> > > > > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> > > > >
> > > > > Also, new API must be reflected in release notes
> > > > >
> > > > > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > > > > This patch introduce a new API to get the hidden key count in
> the hash
> > > > > > table if the rcu qsbr is enabled. When using rte_hash_count with
> rcu
> > > > > > qsbr enabled, it will return the number of elements that are not
> in the
> > > > > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number
> of
> > > > > > elements in the defer queue will not be counted and freed.
> Therefore I
> > > > > > added a new API to get the number of hidden (defer queue)
> elements
> > > > > > in the hash table. Then the user can calculate the total number
> of
> > > > > > elements that are available in the hash table.
> > > > > >
> > > > > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > > > > >
> > > > > > ---
> > > > > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > > > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > > > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > > > > ---
> > > > > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > > > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > > > > lib/hash/version.map | 1 +
> > > > > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > > > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > > > > lib/rcu/version.map | 1 +
> > > > > > 6 files changed, 43 insertions(+)
> > > > > >
> > > > > > diff --git a/lib/hash/rte_cuckoo_hash.c
> b/lib/hash/rte_cuckoo_hash.c
> > > > > > index 70456754c4..3553f3efc7 100644
> > > > > > --- a/lib/hash/rte_cuckoo_hash.c
> > > > > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > > > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash
> *h)
> > > > > > return h->entries;
> > > > > > }
> > > > > >
> > > > > > +int32_t
> > > > > > +rte_hash_dq_count(const struct rte_hash *h)
> > > > > > +{
> > > > > > + if (h->dq == NULL)
> > > > > input arguments must be checked since this is a public API, the
> same is
> > > > > true for rte_rcu_qsbr_dq_count()
> > > > > > + return -EINVAL;
> > > > > why not just return 0?
> > > > > > +
> > > > > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > > > > +}
> > > > > > +
> > > > > > int32_t
> > > > > > rte_hash_count(const struct rte_hash *h)
> > > > > > {
> > > > > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > > > > index 7ecc021111..8ea97e297d 100644
> > > > > > --- a/lib/hash/rte_hash.h
> > > > > > +++ b/lib/hash/rte_hash.h
> > > > > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > > > > void
> > > > > > rte_hash_reset(struct rte_hash *h);
> > > > > >
> > > > > > +
> > > > > > +/**
> > > > > > + * Return the number of records in the defer queue of the hash
> table
> > > > > > + * if RCU is enabled.
> > > > > > + * @param h
> > > > > > + * Hash table to query from
> > > > > > + * @return
> > > > > > + * - -EINVAL if parameters are invalid
> > > > > > + * - A value indicating how many records were inserted in the
> table.
> > > > > did you mean how many records are kept in defer queue?
> > > > > > + */
> > > > > > +int32_t
> > > > > > +rte_hash_dq_count(const struct rte_hash *h);
> > > > > > +
> > > > > > /**
> > > > > > * Return the number of keys in the hash table
> > > > > > * @param h
> > > > > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > > > > index 6b2afebf6b..7f7b158cf1 100644
> > > > > > --- a/lib/hash/version.map
> > > > > > +++ b/lib/hash/version.map
> > > > > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > > > > rte_hash_add_key_with_hash;
> > > > > > rte_hash_add_key_with_hash_data;
> > > > > > rte_hash_count;
> > > > > > + rte_hash_dq_count;
> > > > > new API must introduced as an experimental API. The same is true
> for
> > > > > rte_rcu_qsbr_dq_count()
> > > > > > rte_hash_crc32_alg;
> > > > > > rte_hash_crc_set_alg;
> > > > > > rte_hash_create;
> > > > > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > > > > index bd0b83be0c..89f8da4c4c 100644
> > > > > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > > > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > > > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct
> rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > +/**
> > > > > > + * Return the number of entries in a defer queue.
> > > > > > + */
> > > > > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > > > > +{
> > > > Please validate dq here.
> > > >
> > > > > > + return rte_ring_count(dq->r);
> > > > > > +}
> > > > > > +
> > > > > > /* Delete a defer queue. */
> > > > > > int
> > > > > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > > > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > > > > index 23c9f89805..ed5a590edd 100644
> > > > > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > > > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > > > > @@ -794,6 +794,17 @@ int
> > > > > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned
> int n,
> > > > > > unsigned int *freed, unsigned int *pending, unsigned int
> *available);
> > > > > >
> > > > > > +/**
> > > > > > + * Return the number of entries in a defer queue.
> > > > > > + *
> > > > > > + * @param dq
> > > > > > + * Defer queue.
> > > > > > + * @return
> > > > > > + * The number of entries in the defer queue.
> > > > > > + */
> > > > > > +unsigned int
> > > > > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> > > > Agree on the need for this API in RCU
> > > >
> > > > > > +
> > > > > > /**
> > > > > > * Delete a defer queue.
> > > > > > *
> > > > > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > > > > index 982ffd59d9..f410ab41e7 100644
> > > > > > --- a/lib/rcu/version.map
> > > > > > +++ b/lib/rcu/version.map
> > > > > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > > > > rte_rcu_qsbr_dq_create;
> > > > > > rte_rcu_qsbr_dq_delete;
> > > > > > rte_rcu_qsbr_dq_enqueue;
> > > > > > + rte_rcu_qsbr_dq_count;
> > > > > > rte_rcu_qsbr_dq_reclaim;
> > > > > > rte_rcu_qsbr_dump;
> > > > > > rte_rcu_qsbr_get_memsize;
> > > > >
> > > > > --
> > > > > Regards,
> > > > > Vladimir
> > > > >
> > > >
> > >
> >
>
>
[-- Attachment #2: Type: text/html, Size: 16797 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-28 11:44 ` Abdullah Ömer Yamaç
@ 2024-02-28 14:51 ` Honnappa Nagarahalli
2024-03-02 20:22 ` Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Honnappa Nagarahalli @ 2024-02-28 14:51 UTC (permalink / raw)
To: Abdullah Ömer Yamaç
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
> On Feb 28, 2024, at 5:44 AM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> While I was implementing the new API, I realized one issue, and it would be good to discuss it here. First of all rte_rcu_qsbr_dq_reclaim function checks the state of the qsbr values. It means that all threads should report the quiescent states. It conflicts with my aim.
>
> Let's think about below scenario:
> Eight threads use a hash table and periodically report their quiescent states. One additional thread (main thread) periodically reports the hash size. I implemented the reclaim function in that thread. I mean, the main thread calls reclaim before the rte_hash_count.
>
> Here is the exceptional case that I couldn't retrieve the correct hash size:
> Assume that 6 of 8 threads reported quiescent states and 2 of them are still working on some process and haven't reported quiescent states yet. The main thread calls reclaim functions every time, but elements in dq will not be freed because 2 of the worker threads haven't reported their states (especially if they are waiting for some packets). So, my first proposed method is more suitable for this case. Any idea?
If 2 out of 8 threads have not reported their quiescent state then the elements that have not been acknowledged by those 2 threads cannot be reclaimed and cannot be allocated for further use. Using this you can calculate the most accurate number of entries in the hash table available for allocation.
>
> On Thu, Feb 22, 2024 at 7:44 PM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 22, 2024, at 6:39 AM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> >
> > As a final decision, I will add a new hash API that forces the reclaim. Is it ok for everyone?
> Ack from my side
>
> >
> > On Thu, Feb 22, 2024 at 5:37 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> >
> >
> > > On Feb 21, 2024, at 3:51 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> > >
> > >
> > >
> > > On Wed, Feb 21, 2024 at 6:24 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> > >
> > >
> > > > On Feb 20, 2024, at 12:58 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> > > >
> > > > I appreciate that you gave me suggestions and comments. I will make changes according to all your recommendations, but before that, I want to make everyone's minds clear. Then, I will apply modifications.
> > > >
> > > > On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> > > >
> > > >
> > > > > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > Let me explain a use case;
> > > > >
> > > > > I have a hash table whose key value is IP addresses, and data (let's say the username of the IP) is related to the IP address. The key point is matching these data with flows. Flows are dynamic, and this hash table is dynamic, as well; both can change anytime. For example, when a flow starts, we look up the hash table with the corresponding IP and retrieve the username. We need to hold this username until the flow terminates, although we removed this IP key from the hash table (multithread). That's why we have RCU and defer queue is necessary for high performance. In my application, I need to know the number of IP-username entries. These numbers can be calculated by rte_hash_count - defer queue size.
> > > > The entries in the defer queue are not reclaimed (there is a probability that all of them can be reclaimed) and hence they are not available for allocation. So, rte_hash_count - defer queue size might not give you the correct number you are expecting.
> > > >
> > > > Currently, there is no API in hash library that forces a reclaim. Does it makes sense to have an API that just does the reclaim (and returns the number of entries pending in the defer queue)? A call to rte_hash_count should provide the exact count you are looking for.
> > > > You are right; no API in the hash library forces a reclaim. In my application, I periodically call rte_count to retrieve hash size, and this data is shown in my GUI. So that means I need to call regularly reclaim. I am trying to figure out which is better, calling reclaim or retrieving the defer queue size. Any comment about this?
> > > Retrieving the defer queue size will be cheaper. However, calling the reclaim API will ensure the entries are freed hence providing an accurate number. Calling the reclaim API on an empty defer queue does not consume many cycles. If needed we could add a check for empty defer queue in the reclaim API and return early.
> > >
> > > I am also wondering if a reclaim API in hash library is needed. Why not call rte_rcu_qsbr_dq_reclaim API from the application?
> > > The reason is simple. struct rte_hash *h is an internal structure and we cannot access the h->dq. So it is not possible to call reclaim.
> > Ack. This will be just a wrapper around the rte_rcu_qsbr_dq_reclaim.
> >
> > >
> > >
> > > > >
> > > > > I think if you need a non-blocking and multithreaded hash table, an RCU-enabled hash table is necessary. Also, this API is necessary if you need to get the actual matchable size.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <vladimir.medvedkin@intel.com> wrote:
> > > > > Hi Abdullah,
> > > > >
> > > > > Could you please tell more about use cases where this API may be useful?
> > > > >
> > > > > >a new API to get the hidden key count in the hash table if the rcu qsbr is enabled
> > > > >
> > > > > Here in commit message and down below in doxygen comments, I think this
> > > > > statement should be more specific because rcu can be created with
> > > > > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> > > > >
> > > > > Also, new API must be reflected in release notes
> > > > >
> > > > > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > > > > This patch introduce a new API to get the hidden key count in the hash
> > > > > > table if the rcu qsbr is enabled. When using rte_hash_count with rcu
> > > > > > qsbr enabled, it will return the number of elements that are not in the
> > > > > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the number of
> > > > > > elements in the defer queue will not be counted and freed. Therefore I
> > > > > > added a new API to get the number of hidden (defer queue) elements
> > > > > > in the hash table. Then the user can calculate the total number of
> > > > > > elements that are available in the hash table.
> > > > > >
> > > > > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > > > > >
> > > > > > ---
> > > > > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > > > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > > > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > > > > ---
> > > > > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > > > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > > > > lib/hash/version.map | 1 +
> > > > > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > > > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > > > > lib/rcu/version.map | 1 +
> > > > > > 6 files changed, 43 insertions(+)
> > > > > >
> > > > > > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > > > > > index 70456754c4..3553f3efc7 100644
> > > > > > --- a/lib/hash/rte_cuckoo_hash.c
> > > > > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > > > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash *h)
> > > > > > return h->entries;
> > > > > > }
> > > > > >
> > > > > > +int32_t
> > > > > > +rte_hash_dq_count(const struct rte_hash *h)
> > > > > > +{
> > > > > > + if (h->dq == NULL)
> > > > > input arguments must be checked since this is a public API, the same is
> > > > > true for rte_rcu_qsbr_dq_count()
> > > > > > + return -EINVAL;
> > > > > why not just return 0?
> > > > > > +
> > > > > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > > > > +}
> > > > > > +
> > > > > > int32_t
> > > > > > rte_hash_count(const struct rte_hash *h)
> > > > > > {
> > > > > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > > > > index 7ecc021111..8ea97e297d 100644
> > > > > > --- a/lib/hash/rte_hash.h
> > > > > > +++ b/lib/hash/rte_hash.h
> > > > > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > > > > void
> > > > > > rte_hash_reset(struct rte_hash *h);
> > > > > >
> > > > > > +
> > > > > > +/**
> > > > > > + * Return the number of records in the defer queue of the hash table
> > > > > > + * if RCU is enabled.
> > > > > > + * @param h
> > > > > > + * Hash table to query from
> > > > > > + * @return
> > > > > > + * - -EINVAL if parameters are invalid
> > > > > > + * - A value indicating how many records were inserted in the table.
> > > > > did you mean how many records are kept in defer queue?
> > > > > > + */
> > > > > > +int32_t
> > > > > > +rte_hash_dq_count(const struct rte_hash *h);
> > > > > > +
> > > > > > /**
> > > > > > * Return the number of keys in the hash table
> > > > > > * @param h
> > > > > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > > > > index 6b2afebf6b..7f7b158cf1 100644
> > > > > > --- a/lib/hash/version.map
> > > > > > +++ b/lib/hash/version.map
> > > > > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > > > > rte_hash_add_key_with_hash;
> > > > > > rte_hash_add_key_with_hash_data;
> > > > > > rte_hash_count;
> > > > > > + rte_hash_dq_count;
> > > > > new API must introduced as an experimental API. The same is true for
> > > > > rte_rcu_qsbr_dq_count()
> > > > > > rte_hash_crc32_alg;
> > > > > > rte_hash_crc_set_alg;
> > > > > > rte_hash_create;
> > > > > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > > > > index bd0b83be0c..89f8da4c4c 100644
> > > > > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > > > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > > > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > +/**
> > > > > > + * Return the number of entries in a defer queue.
> > > > > > + */
> > > > > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > > > > +{
> > > > Please validate dq here.
> > > >
> > > > > > + return rte_ring_count(dq->r);
> > > > > > +}
> > > > > > +
> > > > > > /* Delete a defer queue. */
> > > > > > int
> > > > > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > > > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > > > > index 23c9f89805..ed5a590edd 100644
> > > > > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > > > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > > > > @@ -794,6 +794,17 @@ int
> > > > > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > > > unsigned int *freed, unsigned int *pending, unsigned int *available);
> > > > > >
> > > > > > +/**
> > > > > > + * Return the number of entries in a defer queue.
> > > > > > + *
> > > > > > + * @param dq
> > > > > > + * Defer queue.
> > > > > > + * @return
> > > > > > + * The number of entries in the defer queue.
> > > > > > + */
> > > > > > +unsigned int
> > > > > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> > > > Agree on the need for this API in RCU
> > > >
> > > > > > +
> > > > > > /**
> > > > > > * Delete a defer queue.
> > > > > > *
> > > > > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > > > > index 982ffd59d9..f410ab41e7 100644
> > > > > > --- a/lib/rcu/version.map
> > > > > > +++ b/lib/rcu/version.map
> > > > > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > > > > rte_rcu_qsbr_dq_create;
> > > > > > rte_rcu_qsbr_dq_delete;
> > > > > > rte_rcu_qsbr_dq_enqueue;
> > > > > > + rte_rcu_qsbr_dq_count;
> > > > > > rte_rcu_qsbr_dq_reclaim;
> > > > > > rte_rcu_qsbr_dump;
> > > > > > rte_rcu_qsbr_get_memsize;
> > > > >
> > > > > --
> > > > > Regards,
> > > > > Vladimir
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] lib/hash,lib/rcu: feature hidden key count in hash
2024-02-28 14:51 ` Honnappa Nagarahalli
@ 2024-03-02 20:22 ` Abdullah Ömer Yamaç
2024-03-02 21:08 ` [PATCH v2] lib/hash: feature reclaim defer queue Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-03-02 20:22 UTC (permalink / raw)
To: Honnappa Nagarahalli
Cc: Medvedkin, Vladimir, dev, Yipeng Wang, Sameh Gobriel,
Bruce Richardson, thomas, nd
[-- Attachment #1: Type: text/plain, Size: 13430 bytes --]
Sorry for the late reply. I understood what you mean. I will create only
the reclaim API for the hash library. Thanks for the explanation.
On Wed, Feb 28, 2024 at 5:51 PM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:
>
>
> > On Feb 28, 2024, at 5:44 AM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> >
> > While I was implementing the new API, I realized one issue, and it would
> be good to discuss it here. First of all rte_rcu_qsbr_dq_reclaim function
> checks the state of the qsbr values. It means that all threads should
> report the quiescent states. It conflicts with my aim.
> >
> > Let's think about below scenario:
> > Eight threads use a hash table and periodically report their quiescent
> states. One additional thread (main thread) periodically reports the hash
> size. I implemented the reclaim function in that thread. I mean, the main
> thread calls reclaim before the rte_hash_count.
> >
> > Here is the exceptional case that I couldn't retrieve the correct hash
> size:
> > Assume that 6 of 8 threads reported quiescent states and 2 of them are
> still working on some process and haven't reported quiescent states yet.
> The main thread calls reclaim functions every time, but elements in dq will
> not be freed because 2 of the worker threads haven't reported their states
> (especially if they are waiting for some packets). So, my first proposed
> method is more suitable for this case. Any idea?
> If 2 out of 8 threads have not reported their quiescent state then the
> elements that have not been acknowledged by those 2 threads cannot be
> reclaimed and cannot be allocated for further use. Using this you can
> calculate the most accurate number of entries in the hash table available
> for allocation.
>
> >
> > On Thu, Feb 22, 2024 at 7:44 PM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> >
> >
> > > On Feb 22, 2024, at 6:39 AM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> > >
> > > As a final decision, I will add a new hash API that forces the
> reclaim. Is it ok for everyone?
> > Ack from my side
> >
> > >
> > > On Thu, Feb 22, 2024 at 5:37 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> > >
> > >
> > > > On Feb 21, 2024, at 3:51 PM, Abdullah Ömer Yamaç <
> aomeryamac@gmail.com> wrote:
> > > >
> > > >
> > > >
> > > > On Wed, Feb 21, 2024 at 6:24 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> > > >
> > > >
> > > > > On Feb 20, 2024, at 12:58 PM, Abdullah Ömer Yamaç <
> aomeryamac@gmail.com> wrote:
> > > > >
> > > > > I appreciate that you gave me suggestions and comments. I will
> make changes according to all your recommendations, but before that, I want
> to make everyone's minds clear. Then, I will apply modifications.
> > > > >
> > > > > On Tue, Feb 20, 2024 at 2:35 AM Honnappa Nagarahalli <
> Honnappa.Nagarahalli@arm.com> wrote:
> > > > >
> > > > >
> > > > > > On Feb 19, 2024, at 3:28 PM, Abdullah Ömer Yamaç <
> aomeryamac@gmail.com> wrote:
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > Let me explain a use case;
> > > > > >
> > > > > > I have a hash table whose key value is IP addresses, and data
> (let's say the username of the IP) is related to the IP address. The key
> point is matching these data with flows. Flows are dynamic, and this hash
> table is dynamic, as well; both can change anytime. For example, when a
> flow starts, we look up the hash table with the corresponding IP and
> retrieve the username. We need to hold this username until the flow
> terminates, although we removed this IP key from the hash table
> (multithread). That's why we have RCU and defer queue is necessary for high
> performance. In my application, I need to know the number of IP-username
> entries. These numbers can be calculated by rte_hash_count - defer queue
> size.
> > > > > The entries in the defer queue are not reclaimed (there is a
> probability that all of them can be reclaimed) and hence they are not
> available for allocation. So, rte_hash_count - defer queue size might not
> give you the correct number you are expecting.
> > > > >
> > > > > Currently, there is no API in hash library that forces a reclaim.
> Does it makes sense to have an API that just does the reclaim (and returns
> the number of entries pending in the defer queue)? A call to rte_hash_count
> should provide the exact count you are looking for.
> > > > > You are right; no API in the hash library forces a reclaim. In my
> application, I periodically call rte_count to retrieve hash size, and this
> data is shown in my GUI. So that means I need to call regularly reclaim. I
> am trying to figure out which is better, calling reclaim or retrieving the
> defer queue size. Any comment about this?
> > > > Retrieving the defer queue size will be cheaper. However, calling
> the reclaim API will ensure the entries are freed hence providing an
> accurate number. Calling the reclaim API on an empty defer queue does not
> consume many cycles. If needed we could add a check for empty defer queue
> in the reclaim API and return early.
> > > >
> > > > I am also wondering if a reclaim API in hash library is needed. Why
> not call rte_rcu_qsbr_dq_reclaim API from the application?
> > > > The reason is simple. struct rte_hash *h is an internal structure
> and we cannot access the h->dq. So it is not possible to call reclaim.
> > > Ack. This will be just a wrapper around the rte_rcu_qsbr_dq_reclaim.
> > >
> > > >
> > > >
> > > > > >
> > > > > > I think if you need a non-blocking and multithreaded hash table,
> an RCU-enabled hash table is necessary. Also, this API is necessary if you
> need to get the actual matchable size.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Mon, Feb 19, 2024 at 8:36 PM Medvedkin, Vladimir <
> vladimir.medvedkin@intel.com> wrote:
> > > > > > Hi Abdullah,
> > > > > >
> > > > > > Could you please tell more about use cases where this API may be
> useful?
> > > > > >
> > > > > > >a new API to get the hidden key count in the hash table if the
> rcu qsbr is enabled
> > > > > >
> > > > > > Here in commit message and down below in doxygen comments, I
> think this
> > > > > > statement should be more specific because rcu can be created
> with
> > > > > > RTE_HASH_QSBR_MODE_SYNC mode i.e. without defer queue.
> > > > > >
> > > > > > Also, new API must be reflected in release notes
> > > > > >
> > > > > > On 07/02/2024 15:33, Abdullah Ömer Yamaç wrote:
> > > > > > > This patch introduce a new API to get the hidden key count in
> the hash
> > > > > > > table if the rcu qsbr is enabled. When using rte_hash_count
> with rcu
> > > > > > > qsbr enabled, it will return the number of elements that are
> not in the
> > > > > > > free queue. Unless rte_rcu_qsbr_dq_reclaim is called, the
> number of
> > > > > > > elements in the defer queue will not be counted and freed.
> Therefore I
> > > > > > > added a new API to get the number of hidden (defer queue)
> elements
> > > > > > > in the hash table. Then the user can calculate the total
> number of
> > > > > > > elements that are available in the hash table.
> > > > > > >
> > > > > > > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > > > > > >
> > > > > > > ---
> > > > > > > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > > > > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > > > > > > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > > > > > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > > > > > > ---
> > > > > > > lib/hash/rte_cuckoo_hash.c | 9 +++++++++
> > > > > > > lib/hash/rte_hash.h | 13 +++++++++++++
> > > > > > > lib/hash/version.map | 1 +
> > > > > > > lib/rcu/rte_rcu_qsbr.c | 8 ++++++++
> > > > > > > lib/rcu/rte_rcu_qsbr.h | 11 +++++++++++
> > > > > > > lib/rcu/version.map | 1 +
> > > > > > > 6 files changed, 43 insertions(+)
> > > > > > >
> > > > > > > diff --git a/lib/hash/rte_cuckoo_hash.c
> b/lib/hash/rte_cuckoo_hash.c
> > > > > > > index 70456754c4..3553f3efc7 100644
> > > > > > > --- a/lib/hash/rte_cuckoo_hash.c
> > > > > > > +++ b/lib/hash/rte_cuckoo_hash.c
> > > > > > > @@ -555,6 +555,15 @@ rte_hash_max_key_id(const struct rte_hash
> *h)
> > > > > > > return h->entries;
> > > > > > > }
> > > > > > >
> > > > > > > +int32_t
> > > > > > > +rte_hash_dq_count(const struct rte_hash *h)
> > > > > > > +{
> > > > > > > + if (h->dq == NULL)
> > > > > > input arguments must be checked since this is a public API, the
> same is
> > > > > > true for rte_rcu_qsbr_dq_count()
> > > > > > > + return -EINVAL;
> > > > > > why not just return 0?
> > > > > > > +
> > > > > > > + return rte_rcu_qsbr_dq_count(h->dq);
> > > > > > > +}
> > > > > > > +
> > > > > > > int32_t
> > > > > > > rte_hash_count(const struct rte_hash *h)
> > > > > > > {
> > > > > > > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > > > > > > index 7ecc021111..8ea97e297d 100644
> > > > > > > --- a/lib/hash/rte_hash.h
> > > > > > > +++ b/lib/hash/rte_hash.h
> > > > > > > @@ -193,6 +193,19 @@ rte_hash_free(struct rte_hash *h);
> > > > > > > void
> > > > > > > rte_hash_reset(struct rte_hash *h);
> > > > > > >
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * Return the number of records in the defer queue of the
> hash table
> > > > > > > + * if RCU is enabled.
> > > > > > > + * @param h
> > > > > > > + * Hash table to query from
> > > > > > > + * @return
> > > > > > > + * - -EINVAL if parameters are invalid
> > > > > > > + * - A value indicating how many records were inserted in
> the table.
> > > > > > did you mean how many records are kept in defer queue?
> > > > > > > + */
> > > > > > > +int32_t
> > > > > > > +rte_hash_dq_count(const struct rte_hash *h);
> > > > > > > +
> > > > > > > /**
> > > > > > > * Return the number of keys in the hash table
> > > > > > > * @param h
> > > > > > > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > > > > > > index 6b2afebf6b..7f7b158cf1 100644
> > > > > > > --- a/lib/hash/version.map
> > > > > > > +++ b/lib/hash/version.map
> > > > > > > @@ -9,6 +9,7 @@ DPDK_24 {
> > > > > > > rte_hash_add_key_with_hash;
> > > > > > > rte_hash_add_key_with_hash_data;
> > > > > > > rte_hash_count;
> > > > > > > + rte_hash_dq_count;
> > > > > > new API must introduced as an experimental API. The same is true
> for
> > > > > > rte_rcu_qsbr_dq_count()
> > > > > > > rte_hash_crc32_alg;
> > > > > > > rte_hash_crc_set_alg;
> > > > > > > rte_hash_create;
> > > > > > > diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
> > > > > > > index bd0b83be0c..89f8da4c4c 100644
> > > > > > > --- a/lib/rcu/rte_rcu_qsbr.c
> > > > > > > +++ b/lib/rcu/rte_rcu_qsbr.c
> > > > > > > @@ -450,6 +450,14 @@ rte_rcu_qsbr_dq_reclaim(struct
> rte_rcu_qsbr_dq *dq, unsigned int n,
> > > > > > > return 0;
> > > > > > > }
> > > > > > >
> > > > > > > +/**
> > > > > > > + * Return the number of entries in a defer queue.
> > > > > > > + */
> > > > > > > +unsigned int rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq)
> > > > > > > +{
> > > > > Please validate dq here.
> > > > >
> > > > > > > + return rte_ring_count(dq->r);
> > > > > > > +}
> > > > > > > +
> > > > > > > /* Delete a defer queue. */
> > > > > > > int
> > > > > > > rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
> > > > > > > diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h
> > > > > > > index 23c9f89805..ed5a590edd 100644
> > > > > > > --- a/lib/rcu/rte_rcu_qsbr.h
> > > > > > > +++ b/lib/rcu/rte_rcu_qsbr.h
> > > > > > > @@ -794,6 +794,17 @@ int
> > > > > > > rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq, unsigned
> int n,
> > > > > > > unsigned int *freed, unsigned int *pending, unsigned int
> *available);
> > > > > > >
> > > > > > > +/**
> > > > > > > + * Return the number of entries in a defer queue.
> > > > > > > + *
> > > > > > > + * @param dq
> > > > > > > + * Defer queue.
> > > > > > > + * @return
> > > > > > > + * The number of entries in the defer queue.
> > > > > > > + */
> > > > > > > +unsigned int
> > > > > > > +rte_rcu_qsbr_dq_count(struct rte_rcu_qsbr_dq *dq);
> > > > > Agree on the need for this API in RCU
> > > > >
> > > > > > > +
> > > > > > > /**
> > > > > > > * Delete a defer queue.
> > > > > > > *
> > > > > > > diff --git a/lib/rcu/version.map b/lib/rcu/version.map
> > > > > > > index 982ffd59d9..f410ab41e7 100644
> > > > > > > --- a/lib/rcu/version.map
> > > > > > > +++ b/lib/rcu/version.map
> > > > > > > @@ -5,6 +5,7 @@ DPDK_24 {
> > > > > > > rte_rcu_qsbr_dq_create;
> > > > > > > rte_rcu_qsbr_dq_delete;
> > > > > > > rte_rcu_qsbr_dq_enqueue;
> > > > > > > + rte_rcu_qsbr_dq_count;
> > > > > > > rte_rcu_qsbr_dq_reclaim;
> > > > > > > rte_rcu_qsbr_dump;
> > > > > > > rte_rcu_qsbr_get_memsize;
> > > > > >
> > > > > > --
> > > > > > Regards,
> > > > > > Vladimir
> > > > > >
> > > > >
> > > >
> > >
> >
>
>
[-- Attachment #2: Type: text/html, Size: 18760 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2] lib/hash: feature reclaim defer queue
2024-03-02 20:22 ` Abdullah Ömer Yamaç
@ 2024-03-02 21:08 ` Abdullah Ömer Yamaç
2024-03-02 21:27 ` Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-03-02 21:08 UTC (permalink / raw)
To: dev; +Cc: Abdullah Ömer Yamaç, Honnappa Nagarahalli
This patch adds a new feature to the hash library to allow the user to
reclaim the defer queue. This is useful when the user wants to force
reclaim resources that are not being used. This API is only available
if the RCU is enabled.
Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
Acked-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
lib/hash/rte_hash.h | 14 ++++++++++++++
lib/hash/version.map | 7 +++++++
3 files changed, 44 insertions(+)
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 9cf94645f6..254fa80cc5 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1588,6 +1588,27 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
return 0;
}
+int
+rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h)
+{
+ int ret;
+
+ if (h->hash_rcu_cfg == NULL || h->dq == NULL) {
+ rte_errno = EINVAL;
+ return -1;
+ }
+
+ ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size, NULL, NULL, NULL);
+ if (ret != 0) {
+ HASH_LOG(ERR,
+ "%s: could not reclaim the defer queue in hash table",
+ __func__);
+ return -1;
+ }
+
+ return 0;
+}
+
static inline void
remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
unsigned int i)
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..c119477d50 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -674,6 +674,21 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
*/
int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
+/**
+ * Reclaim resources from the defer queue.
+ * This API reclaim the resources from the defer queue if rcu is enabled.
+ *
+ * @param h
+ * the hash object to reclaim resources
+ * @return
+ * On success - 0
+ * On error - 1 with error code set in rte_errno.
+ * Possible rte_errno codes are:
+ * - EINVAL - invalid pointer or invalid rcu mode
+ */
+__rte_experimental
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6b2afebf6b..cec0e8fc67 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -48,3 +48,10 @@ DPDK_24 {
local: *;
};
+
+EXPERIMENTAL {
+ global:
+
+ #added in 24.1
+ rte_hash_rcu_qsbr_dq_reclaim;
+}
\ No newline at end of file
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2] lib/hash: feature reclaim defer queue
2024-03-02 21:08 ` [PATCH v2] lib/hash: feature reclaim defer queue Abdullah Ömer Yamaç
@ 2024-03-02 21:27 ` Abdullah Ömer Yamaç
2024-03-03 19:14 ` Honnappa Nagarahalli
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-03-02 21:27 UTC (permalink / raw)
To: dev; +Cc: Abdullah Ömer Yamaç, Honnappa Nagarahalli
This patch adds a new feature to the hash library to allow the user to
reclaim the defer queue. This is useful when the user wants to force
reclaim resources that are not being used. This API is only available
if the RCU is enabled.
Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
Acked-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
lib/hash/rte_hash.h | 14 ++++++++++++++
lib/hash/version.map | 7 +++++++
3 files changed, 44 insertions(+)
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 9cf94645f6..254fa80cc5 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1588,6 +1588,27 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
return 0;
}
+int
+rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h)
+{
+ int ret;
+
+ if (h->hash_rcu_cfg == NULL || h->dq == NULL) {
+ rte_errno = EINVAL;
+ return -1;
+ }
+
+ ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size, NULL, NULL, NULL);
+ if (ret != 0) {
+ HASH_LOG(ERR,
+ "%s: could not reclaim the defer queue in hash table",
+ __func__);
+ return -1;
+ }
+
+ return 0;
+}
+
static inline void
remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
unsigned int i)
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..c119477d50 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -674,6 +674,21 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
*/
int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
+/**
+ * Reclaim resources from the defer queue.
+ * This API reclaim the resources from the defer queue if rcu is enabled.
+ *
+ * @param h
+ * the hash object to reclaim resources
+ * @return
+ * On success - 0
+ * On error - 1 with error code set in rte_errno.
+ * Possible rte_errno codes are:
+ * - EINVAL - invalid pointer or invalid rcu mode
+ */
+__rte_experimental
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6b2afebf6b..cec0e8fc67 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -48,3 +48,9 @@ DPDK_24 {
local: *;
};
+
+EXPERIMENTAL {
+ global:
+
+ rte_hash_rcu_qsbr_dq_reclaim;
+}
\ No newline at end of file
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] lib/hash: feature reclaim defer queue
2024-03-02 21:27 ` Abdullah Ömer Yamaç
@ 2024-03-03 19:14 ` Honnappa Nagarahalli
2024-03-04 8:27 ` Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Honnappa Nagarahalli @ 2024-03-03 19:14 UTC (permalink / raw)
To: Abdullah Ömer Yamaç; +Cc: dev, nd
Hello Abdullah,
Thank you for the patch, few comments inline.
The short commit log could be changed as follows:
"lib/hash: add defer queue reclaim API”
> On Mar 2, 2024, at 3:27 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> This patch adds a new feature to the hash library to allow the user to
> reclaim the defer queue. This is useful when the user wants to force
> reclaim resources that are not being used. This API is only available
> if the RCU is enabled.
>
> Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> Acked-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Please add this only after you get an explicit Ack on the patch.
> ---
> lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
> lib/hash/rte_hash.h | 14 ++++++++++++++
> lib/hash/version.map | 7 +++++++
> 3 files changed, 44 insertions(+)
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 9cf94645f6..254fa80cc5 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -1588,6 +1588,27 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
> return 0;
> }
>
> +int
> +rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h)
We need to add freed, pending and available parameters to this API. I think this information will be helpful for the users. For ex: in your use case, you could use the pending value to calculate the available hash entries.
> +{
> + int ret;
> +
> + if (h->hash_rcu_cfg == NULL || h->dq == NULL) {
We can skip NULL check for h->dq as the RCU reclaim API makes the same check.
> + rte_errno = EINVAL;
> + return -1;
> + }
> +
> + ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size, NULL, NULL, NULL);
> + if (ret != 0) {
> + HASH_LOG(ERR,
> + "%s: could not reclaim the defer queue in hash table",
> + __func__);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static inline void
> remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
> unsigned int i)
> diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> index 7ecc021111..c119477d50 100644
> --- a/lib/hash/rte_hash.h
> +++ b/lib/hash/rte_hash.h
> @@ -674,6 +674,21 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
> */
> int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
>
> +/**
> + * Reclaim resources from the defer queue.
> + * This API reclaim the resources from the defer queue if rcu is enabled.
> + *
> + * @param h
> + * the hash object to reclaim resources
> + * @return
> + * On success - 0
> + * On error - 1 with error code set in rte_errno.
> + * Possible rte_errno codes are:
> + * - EINVAL - invalid pointer or invalid rcu mode
We can remove the ‘invalid rcd mode’.
> + */
> +__rte_experimental
> +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/hash/version.map b/lib/hash/version.map
> index 6b2afebf6b..cec0e8fc67 100644
> --- a/lib/hash/version.map
> +++ b/lib/hash/version.map
> @@ -48,3 +48,9 @@ DPDK_24 {
>
> local: *;
> };
> +
> +EXPERIMENTAL {
> + global:
> +
> + rte_hash_rcu_qsbr_dq_reclaim;
> +}
> \ No newline at end of file
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] lib/hash: feature reclaim defer queue
2024-03-03 19:14 ` Honnappa Nagarahalli
@ 2024-03-04 8:27 ` Abdullah Ömer Yamaç
2024-03-04 21:58 ` Honnappa Nagarahalli
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-03-04 8:27 UTC (permalink / raw)
To: Honnappa Nagarahalli; +Cc: dev, nd
[-- Attachment #1: Type: text/plain, Size: 3932 bytes --]
Just one more question.
On Sun, Mar 3, 2024 at 10:14 PM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:
> Hello Abdullah,
> Thank you for the patch, few comments inline.
>
> The short commit log could be changed as follows:
>
> "lib/hash: add defer queue reclaim API”
>
> > On Mar 2, 2024, at 3:27 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
> >
> > This patch adds a new feature to the hash library to allow the user to
> > reclaim the defer queue. This is useful when the user wants to force
> > reclaim resources that are not being used. This API is only available
> > if the RCU is enabled.
> >
> > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > Acked-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Please add this only after you get an explicit Ack on the patch.
>
> > ---
> > lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
> > lib/hash/rte_hash.h | 14 ++++++++++++++
> > lib/hash/version.map | 7 +++++++
> > 3 files changed, 44 insertions(+)
> >
> > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > index 9cf94645f6..254fa80cc5 100644
> > --- a/lib/hash/rte_cuckoo_hash.c
> > +++ b/lib/hash/rte_cuckoo_hash.c
> > @@ -1588,6 +1588,27 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct
> rte_hash_rcu_config *cfg)
> > return 0;
> > }
> >
> > +int
> > +rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h)
> We need to add freed, pending and available parameters to this API. I
> think this information will be helpful for the users. For ex: in your use
> case, you could use the pending value to calculate the available hash
> entries.
>
> The second parameter, "Maximum number of resources to free.", should be
available also? I set this value to " h->hash_rcu_cfg->max_reclaim_size",
but it can be a parameter in addition to the above parameters
> > +{
> > + int ret;
> > +
> > + if (h->hash_rcu_cfg == NULL || h->dq == NULL) {
> We can skip NULL check for h->dq as the RCU reclaim API makes the same
> check.
>
> > + rte_errno = EINVAL;
> > + return -1;
> > + }
> > +
> > + ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size, NULL, NULL, NULL);
> > + if (ret != 0) {
> > + HASH_LOG(ERR,
> > + "%s: could not reclaim the defer queue in hash table",
> > + __func__);
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static inline void
> > remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
> > unsigned int i)
> > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > index 7ecc021111..c119477d50 100644
> > --- a/lib/hash/rte_hash.h
> > +++ b/lib/hash/rte_hash.h
> > @@ -674,6 +674,21 @@ rte_hash_iterate(const struct rte_hash *h, const
> void **key, void **data, uint32
> > */
> > int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config
> *cfg);
> >
> > +/**
> > + * Reclaim resources from the defer queue.
> > + * This API reclaim the resources from the defer queue if rcu is
> enabled.
> > + *
> > + * @param h
> > + * the hash object to reclaim resources
> > + * @return
> > + * On success - 0
> > + * On error - 1 with error code set in rte_errno.
> > + * Possible rte_errno codes are:
> > + * - EINVAL - invalid pointer or invalid rcu mode
> We can remove the ‘invalid rcd mode’.
>
> > + */
> > +__rte_experimental
> > +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h);
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > index 6b2afebf6b..cec0e8fc67 100644
> > --- a/lib/hash/version.map
> > +++ b/lib/hash/version.map
> > @@ -48,3 +48,9 @@ DPDK_24 {
> >
> > local: *;
> > };
> > +
> > +EXPERIMENTAL {
> > + global:
> > +
> > + rte_hash_rcu_qsbr_dq_reclaim;
> > +}
> > \ No newline at end of file
> > --
> > 2.34.1
> >
>
>
[-- Attachment #2: Type: text/html, Size: 5125 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2] lib/hash: feature reclaim defer queue
2024-03-04 8:27 ` Abdullah Ömer Yamaç
@ 2024-03-04 21:58 ` Honnappa Nagarahalli
2024-03-06 8:55 ` [PATCH v3] lib/hash: add defer queue reclaim API Abdullah Ömer Yamaç
` (2 more replies)
0 siblings, 3 replies; 30+ messages in thread
From: Honnappa Nagarahalli @ 2024-03-04 21:58 UTC (permalink / raw)
To: Abdullah Ömer Yamaç; +Cc: dev, nd
> On Mar 4, 2024, at 2:27 AM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> Just one more question.
>
> On Sun, Mar 3, 2024 at 10:14 PM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> Hello Abdullah,
> Thank you for the patch, few comments inline.
>
> The short commit log could be changed as follows:
>
> "lib/hash: add defer queue reclaim API”
>
> > On Mar 2, 2024, at 3:27 PM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> >
> > This patch adds a new feature to the hash library to allow the user to
> > reclaim the defer queue. This is useful when the user wants to force
> > reclaim resources that are not being used. This API is only available
> > if the RCU is enabled.
> >
> > Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> > Acked-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Please add this only after you get an explicit Ack on the patch.
>
> > ---
> > lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
> > lib/hash/rte_hash.h | 14 ++++++++++++++
> > lib/hash/version.map | 7 +++++++
> > 3 files changed, 44 insertions(+)
> >
> > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > index 9cf94645f6..254fa80cc5 100644
> > --- a/lib/hash/rte_cuckoo_hash.c
> > +++ b/lib/hash/rte_cuckoo_hash.c
> > @@ -1588,6 +1588,27 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
> > return 0;
> > }
> >
> > +int
> > +rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h)
> We need to add freed, pending and available parameters to this API. I think this information will be helpful for the users. For ex: in your use case, you could use the pending value to calculate the available hash entries.
>
> The second parameter, "Maximum number of resources to free.", should be available also? I set this value to " h->hash_rcu_cfg->max_reclaim_size", but it can be a parameter in addition to the above parameters
We don’t have to expose that in the API. The user has provided that already in max_reclaim_size. So, using "h->hash_rcu_cfg->max_reclaim_size” is enough.
> > +{
> > + int ret;
> > +
> > + if (h->hash_rcu_cfg == NULL || h->dq == NULL) {
> We can skip NULL check for h->dq as the RCU reclaim API makes the same check.
>
> > + rte_errno = EINVAL;
> > + return -1;
> > + }
> > +
> > + ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size, NULL, NULL, NULL);
> > + if (ret != 0) {
> > + HASH_LOG(ERR,
> > + "%s: could not reclaim the defer queue in hash table",
> > + __func__);
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static inline void
> > remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
> > unsigned int i)
> > diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> > index 7ecc021111..c119477d50 100644
> > --- a/lib/hash/rte_hash.h
> > +++ b/lib/hash/rte_hash.h
> > @@ -674,6 +674,21 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
> > */
> > int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
> >
> > +/**
> > + * Reclaim resources from the defer queue.
> > + * This API reclaim the resources from the defer queue if rcu is enabled.
> > + *
> > + * @param h
> > + * the hash object to reclaim resources
> > + * @return
> > + * On success - 0
> > + * On error - 1 with error code set in rte_errno.
> > + * Possible rte_errno codes are:
> > + * - EINVAL - invalid pointer or invalid rcu mode
> We can remove the ‘invalid rcd mode’.
>
> > + */
> > +__rte_experimental
> > +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h);
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > index 6b2afebf6b..cec0e8fc67 100644
> > --- a/lib/hash/version.map
> > +++ b/lib/hash/version.map
> > @@ -48,3 +48,9 @@ DPDK_24 {
> >
> > local: *;
> > };
> > +
> > +EXPERIMENTAL {
> > + global:
> > +
> > + rte_hash_rcu_qsbr_dq_reclaim;
> > +}
> > \ No newline at end of file
> > --
> > 2.34.1
> >
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3] lib/hash: add defer queue reclaim API
2024-03-04 21:58 ` Honnappa Nagarahalli
@ 2024-03-06 8:55 ` Abdullah Ömer Yamaç
2024-03-06 8:59 ` Abdullah Ömer Yamaç
2024-03-06 10:13 ` Abdullah Ömer Yamaç
2 siblings, 0 replies; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-03-06 8:55 UTC (permalink / raw)
To: dev; +Cc: Abdullah Ömer Yamaç
This patch adds a new feature to the hash library to allow the user to
reclaim the defer queue. This is useful when the user wants to force
reclaim resources that are not being used. This API is only available
if the RCU is enabled.
Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
---
lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
lib/hash/rte_hash.h | 28 +++++++++++++++++++++++++++-
lib/hash/version.map | 6 ++++++
3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 9cf94645f6..ecc124f1fc 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
return 0;
}
+int
+rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending, unsigned int *available)
+{
+ int ret;
+
+ if (h->hash_rcu_cfg == NULL) {
+ rte_errno = EINVAL;
+ return -1;
+ }
+
+ ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
+ freed, pending, available);
+ if (ret != 0) {
+ HASH_LOG(ERR,
+ "%s: could not reclaim the defer queue in hash table",
+ __func__);
+ return -1;
+ }
+
+ return 0;
+}
+
static inline void
remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
unsigned int i)
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..ad13792dbb 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -631,7 +631,7 @@ rte_hash_lookup_with_hash_bulk_data(const struct rte_hash *h,
*/
int
rte_hash_lookup_bulk(const struct rte_hash *h, const void **keys,
- uint32_t num_keys, int32_t *positions);
+ uint32_t num_keys, int32_t *positions);
/**
* Iterate through the hash table, returning key-value pairs.
@@ -674,6 +674,32 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
*/
int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
+/**
+ * Reclaim resources from the defer queue.
+ * This API reclaim the resources from the defer queue if rcu is enabled.
+ *
+ * @param h
+ * The hash object to reclaim resources.
+ * @param n
+ * Maximum number of resources to free.
+ * @param freed
+ * Number of resources that were freed.
+ * @param pending
+ * Number of resources pending on the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @param available
+ * Number of resources that can be added to the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @return
+ * On success - 0
+ * On error - 1 with error code set in rte_errno.
+ * Possible rte_errno codes are:
+ * - EINVAL - invalid pointer
+ */
+__rte_experimental
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending, unsigned int *available);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6b2afebf6b..fac7f81e6f 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -48,3 +48,9 @@ DPDK_24 {
local: *;
};
+
+EXPERIMENTAL {
+ global:
+
+ rte_hash_rcu_qsbr_dq_reclaim;
+};
\ No newline at end of file
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3] lib/hash: add defer queue reclaim API
2024-03-04 21:58 ` Honnappa Nagarahalli
2024-03-06 8:55 ` [PATCH v3] lib/hash: add defer queue reclaim API Abdullah Ömer Yamaç
@ 2024-03-06 8:59 ` Abdullah Ömer Yamaç
2024-03-06 10:13 ` Abdullah Ömer Yamaç
2 siblings, 0 replies; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-03-06 8:59 UTC (permalink / raw)
To: dev; +Cc: Abdullah Ömer Yamaç
This patch adds a new feature to the hash library to allow the user to
reclaim the defer queue. This is useful when the user wants to force
reclaim resources that are not being used. This API is only available
if the RCU is enabled.
Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
---
lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
lib/hash/rte_hash.h | 28 +++++++++++++++++++++++++++-
lib/hash/version.map | 6 ++++++
3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 9cf94645f6..ecc124f1fc 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
return 0;
}
+int
+rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending, unsigned int *available)
+{
+ int ret;
+
+ if (h->hash_rcu_cfg == NULL) {
+ rte_errno = EINVAL;
+ return -1;
+ }
+
+ ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
+ freed, pending, available);
+ if (ret != 0) {
+ HASH_LOG(ERR,
+ "%s: could not reclaim the defer queue in hash table",
+ __func__);
+ return -1;
+ }
+
+ return 0;
+}
+
static inline void
remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
unsigned int i)
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..ad13792dbb 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -674,6 +674,32 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
*/
int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
+/**
+ * Reclaim resources from the defer queue.
+ * This API reclaim the resources from the defer queue if rcu is enabled.
+ *
+ * @param h
+ * The hash object to reclaim resources.
+ * @param n
+ * Maximum number of resources to free.
+ * @param freed
+ * Number of resources that were freed.
+ * @param pending
+ * Number of resources pending on the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @param available
+ * Number of resources that can be added to the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @return
+ * On success - 0
+ * On error - 1 with error code set in rte_errno.
+ * Possible rte_errno codes are:
+ * - EINVAL - invalid pointer
+ */
+__rte_experimental
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending, unsigned int *available);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6b2afebf6b..fac7f81e6f 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -48,3 +48,9 @@ DPDK_24 {
local: *;
};
+
+EXPERIMENTAL {
+ global:
+
+ rte_hash_rcu_qsbr_dq_reclaim;
+};
\ No newline at end of file
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3] lib/hash: add defer queue reclaim API
2024-03-04 21:58 ` Honnappa Nagarahalli
2024-03-06 8:55 ` [PATCH v3] lib/hash: add defer queue reclaim API Abdullah Ömer Yamaç
2024-03-06 8:59 ` Abdullah Ömer Yamaç
@ 2024-03-06 10:13 ` Abdullah Ömer Yamaç
2024-03-14 7:04 ` Abdullah Ömer Yamaç
2 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-03-06 10:13 UTC (permalink / raw)
To: dev; +Cc: Abdullah Ömer Yamaç
This patch adds a new feature to the hash library to allow the user to
reclaim the defer queue. This is useful when the user wants to force
reclaim resources that are not being used. This API is only available
if the RCU is enabled.
Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
---
lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
lib/hash/rte_hash.h | 24 ++++++++++++++++++++++++
lib/hash/version.map | 6 ++++++
3 files changed, 53 insertions(+)
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 9cf94645f6..1c360fa38b 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
return 0;
}
+int
+rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending, unsigned int *available)
+{
+ int ret;
+
+ if (h->hash_rcu_cfg == NULL) {
+ rte_errno = EINVAL;
+ return -1;
+ }
+
+ ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
+ freed, pending, available);
+ if (ret != 0) {
+ HASH_LOG(ERR,
+ "%s: could not reclaim the defer queue in hash table",
+ __func__);
+ return -1;
+ }
+
+ return 0;
+}
+
static inline void
remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
unsigned int i)
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..edfa262aca 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -674,6 +674,30 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
*/
int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
+/**
+ * Reclaim resources from the defer queue.
+ * This API reclaim the resources from the defer queue if rcu is enabled.
+ *
+ * @param h
+ * The hash object to reclaim resources.
+ * @param freed
+ * Number of resources that were freed.
+ * @param pending
+ * Number of resources pending on the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @param available
+ * Number of resources that can be added to the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @return
+ * On success - 0
+ * On error - 1 with error code set in rte_errno.
+ * Possible rte_errno codes are:
+ * - EINVAL - invalid pointer
+ */
+__rte_experimental
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending, unsigned int *available);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6b2afebf6b..fac7f81e6f 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -48,3 +48,9 @@ DPDK_24 {
local: *;
};
+
+EXPERIMENTAL {
+ global:
+
+ rte_hash_rcu_qsbr_dq_reclaim;
+};
\ No newline at end of file
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] lib/hash: add defer queue reclaim API
2024-03-06 10:13 ` Abdullah Ömer Yamaç
@ 2024-03-14 7:04 ` Abdullah Ömer Yamaç
2024-04-04 10:11 ` Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-03-14 7:04 UTC (permalink / raw)
To: dev; +Cc: Honnappa Nagarahalli
[-- Attachment #1: Type: text/plain, Size: 3592 bytes --]
Hello,
Is there any other comment on this?
On Wed, Mar 6, 2024 at 1:13 PM Abdullah Ömer Yamaç <aomeryamac@gmail.com>
wrote:
> This patch adds a new feature to the hash library to allow the user to
> reclaim the defer queue. This is useful when the user wants to force
> reclaim resources that are not being used. This API is only available
> if the RCU is enabled.
>
> Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> ---
> lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
> lib/hash/rte_hash.h | 24 ++++++++++++++++++++++++
> lib/hash/version.map | 6 ++++++
> 3 files changed, 53 insertions(+)
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 9cf94645f6..1c360fa38b 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct
> rte_hash_rcu_config *cfg)
> return 0;
> }
>
> +int
> +rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> + unsigned int *pending, unsigned int *available)
> +{
> + int ret;
> +
> + if (h->hash_rcu_cfg == NULL) {
> + rte_errno = EINVAL;
> + return -1;
> + }
> +
> + ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size,
> + freed, pending,
> available);
> + if (ret != 0) {
> + HASH_LOG(ERR,
> + "%s: could not reclaim the defer queue in hash
> table",
> + __func__);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static inline void
> remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
> unsigned int i)
> diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> index 7ecc021111..edfa262aca 100644
> --- a/lib/hash/rte_hash.h
> +++ b/lib/hash/rte_hash.h
> @@ -674,6 +674,30 @@ rte_hash_iterate(const struct rte_hash *h, const void
> **key, void **data, uint32
> */
> int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config
> *cfg);
>
> +/**
> + * Reclaim resources from the defer queue.
> + * This API reclaim the resources from the defer queue if rcu is enabled.
> + *
> + * @param h
> + * The hash object to reclaim resources.
> + * @param freed
> + * Number of resources that were freed.
> + * @param pending
> + * Number of resources pending on the defer queue.
> + * This number might not be accurate if multi-thread safety is
> configured.
> + * @param available
> + * Number of resources that can be added to the defer queue.
> + * This number might not be accurate if multi-thread safety is
> configured.
> + * @return
> + * On success - 0
> + * On error - 1 with error code set in rte_errno.
> + * Possible rte_errno codes are:
> + * - EINVAL - invalid pointer
> + */
> +__rte_experimental
> +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> + unsigned int *pending, unsigned int *available);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/hash/version.map b/lib/hash/version.map
> index 6b2afebf6b..fac7f81e6f 100644
> --- a/lib/hash/version.map
> +++ b/lib/hash/version.map
> @@ -48,3 +48,9 @@ DPDK_24 {
>
> local: *;
> };
> +
> +EXPERIMENTAL {
> + global:
> +
> + rte_hash_rcu_qsbr_dq_reclaim;
> +};
> \ No newline at end of file
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 4333 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] lib/hash: add defer queue reclaim API
2024-03-14 7:04 ` Abdullah Ömer Yamaç
@ 2024-04-04 10:11 ` Abdullah Ömer Yamaç
2024-04-05 2:11 ` Honnappa Nagarahalli
2024-04-15 11:26 ` [PATCH v4] " Abdullah Ömer Yamaç
0 siblings, 2 replies; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-04-04 10:11 UTC (permalink / raw)
To: dev; +Cc: Honnappa Nagarahalli, Thomas Monjalon
[-- Attachment #1: Type: text/plain, Size: 3853 bytes --]
Hello,
Could you check the last commit?
Thanks
On Thu, Mar 14, 2024 at 10:04 AM Abdullah Ömer Yamaç <aomeryamac@gmail.com>
wrote:
> Hello,
> Is there any other comment on this?
>
> On Wed, Mar 6, 2024 at 1:13 PM Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> wrote:
>
>> This patch adds a new feature to the hash library to allow the user to
>> reclaim the defer queue. This is useful when the user wants to force
>> reclaim resources that are not being used. This API is only available
>> if the RCU is enabled.
>>
>> Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
>> ---
>> lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
>> lib/hash/rte_hash.h | 24 ++++++++++++++++++++++++
>> lib/hash/version.map | 6 ++++++
>> 3 files changed, 53 insertions(+)
>>
>> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
>> index 9cf94645f6..1c360fa38b 100644
>> --- a/lib/hash/rte_cuckoo_hash.c
>> +++ b/lib/hash/rte_cuckoo_hash.c
>> @@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct
>> rte_hash_rcu_config *cfg)
>> return 0;
>> }
>>
>> +int
>> +rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
>> + unsigned int *pending, unsigned int *available)
>> +{
>> + int ret;
>> +
>> + if (h->hash_rcu_cfg == NULL) {
>> + rte_errno = EINVAL;
>> + return -1;
>> + }
>> +
>> + ret = rte_rcu_qsbr_dq_reclaim(h->dq,
>> h->hash_rcu_cfg->max_reclaim_size,
>> + freed, pending,
>> available);
>> + if (ret != 0) {
>> + HASH_LOG(ERR,
>> + "%s: could not reclaim the defer queue in hash
>> table",
>> + __func__);
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static inline void
>> remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
>> unsigned int i)
>> diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
>> index 7ecc021111..edfa262aca 100644
>> --- a/lib/hash/rte_hash.h
>> +++ b/lib/hash/rte_hash.h
>> @@ -674,6 +674,30 @@ rte_hash_iterate(const struct rte_hash *h, const
>> void **key, void **data, uint32
>> */
>> int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config
>> *cfg);
>>
>> +/**
>> + * Reclaim resources from the defer queue.
>> + * This API reclaim the resources from the defer queue if rcu is enabled.
>> + *
>> + * @param h
>> + * The hash object to reclaim resources.
>> + * @param freed
>> + * Number of resources that were freed.
>> + * @param pending
>> + * Number of resources pending on the defer queue.
>> + * This number might not be accurate if multi-thread safety is
>> configured.
>> + * @param available
>> + * Number of resources that can be added to the defer queue.
>> + * This number might not be accurate if multi-thread safety is
>> configured.
>> + * @return
>> + * On success - 0
>> + * On error - 1 with error code set in rte_errno.
>> + * Possible rte_errno codes are:
>> + * - EINVAL - invalid pointer
>> + */
>> +__rte_experimental
>> +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
>> + unsigned int *pending, unsigned int *available);
>> +
>> #ifdef __cplusplus
>> }
>> #endif
>> diff --git a/lib/hash/version.map b/lib/hash/version.map
>> index 6b2afebf6b..fac7f81e6f 100644
>> --- a/lib/hash/version.map
>> +++ b/lib/hash/version.map
>> @@ -48,3 +48,9 @@ DPDK_24 {
>>
>> local: *;
>> };
>> +
>> +EXPERIMENTAL {
>> + global:
>> +
>> + rte_hash_rcu_qsbr_dq_reclaim;
>> +};
>> \ No newline at end of file
>> --
>> 2.34.1
>>
>>
[-- Attachment #2: Type: text/html, Size: 4773 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] lib/hash: add defer queue reclaim API
2024-04-04 10:11 ` Abdullah Ömer Yamaç
@ 2024-04-05 2:11 ` Honnappa Nagarahalli
2024-04-15 11:26 ` [PATCH v4] " Abdullah Ömer Yamaç
1 sibling, 0 replies; 30+ messages in thread
From: Honnappa Nagarahalli @ 2024-04-05 2:11 UTC (permalink / raw)
To: Abdullah Ömer Yamaç; +Cc: dev, thomas, nd
Apologies for the late reply. Looks good, few nits inline.
Can you please some simple unit tests?
> On Apr 4, 2024, at 5:11 AM, Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> Hello,
> Could you check the last commit?
> Thanks
>
> On Thu, Mar 14, 2024 at 10:04 AM Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> Hello,
> Is there any other comment on this?
>
> On Wed, Mar 6, 2024 at 1:13 PM Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> This patch adds a new feature to the hash library to allow the user to
> reclaim the defer queue. This is useful when the user wants to force
> reclaim resources that are not being used. This API is only available
> if the RCU is enabled.
>
> Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> ---
> lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
> lib/hash/rte_hash.h | 24 ++++++++++++++++++++++++
> lib/hash/version.map | 6 ++++++
> 3 files changed, 53 insertions(+)
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 9cf94645f6..1c360fa38b 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
> return 0;
> }
>
> +int
> +rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> + unsigned int *pending, unsigned int *available)
> +{
> + int ret;
> +
> + if (h->hash_rcu_cfg == NULL) {
Please check for ‘h’ being NULL.
> + rte_errno = EINVAL;
> + return -1;
Return value should be positive 1 as per the API definition below
> + }
> +
> + ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
> + freed, pending, available);
> + if (ret != 0) {
> + HASH_LOG(ERR,
> + "%s: could not reclaim the defer queue in hash table",
> + __func__);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static inline void
> remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
> unsigned int i)
> diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> index 7ecc021111..edfa262aca 100644
> --- a/lib/hash/rte_hash.h
> +++ b/lib/hash/rte_hash.h
> @@ -674,6 +674,30 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
> */
> int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
>
> +/**
> + * Reclaim resources from the defer queue.
> + * This API reclaim the resources from the defer queue if rcu is enabled.
> + *
> + * @param h
> + * The hash object to reclaim resources.
> + * @param freed
> + * Number of resources that were freed.
> + * @param pending
> + * Number of resources pending on the defer queue.
> + * This number might not be accurate if multi-thread safety is configured.
> + * @param available
> + * Number of resources that can be added to the defer queue.
> + * This number might not be accurate if multi-thread safety is configured.
> + * @return
> + * On success - 0
> + * On error - 1 with error code set in rte_errno.
> + * Possible rte_errno codes are:
> + * - EINVAL - invalid pointer
> + */
> +__rte_experimental
> +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> + unsigned int *pending, unsigned int *available);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/hash/version.map b/lib/hash/version.map
> index 6b2afebf6b..fac7f81e6f 100644
> --- a/lib/hash/version.map
> +++ b/lib/hash/version.map
> @@ -48,3 +48,9 @@ DPDK_24 {
>
> local: *;
> };
> +
> +EXPERIMENTAL {
> + global:
> +
> + rte_hash_rcu_qsbr_dq_reclaim;
> +};
> \ No newline at end of file
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4] lib/hash: add defer queue reclaim API
2024-04-04 10:11 ` Abdullah Ömer Yamaç
2024-04-05 2:11 ` Honnappa Nagarahalli
@ 2024-04-15 11:26 ` Abdullah Ömer Yamaç
2024-04-23 13:49 ` Abdullah Ömer Yamaç
2024-04-23 21:24 ` Stephen Hemminger
1 sibling, 2 replies; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-04-15 11:26 UTC (permalink / raw)
To: dev; +Cc: Abdullah Ömer Yamaç
This patch adds a new feature to the hash library to allow the user to
reclaim the defer queue. This is useful when the user wants to force
reclaim resources that are not being used. This API is only available
if the RCU is enabled.
Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
---
app/test/test_hash.c | 97 ++++++++++++++++++++++++++++++++++++++
lib/hash/rte_cuckoo_hash.c | 23 +++++++++
lib/hash/rte_hash.h | 24 ++++++++++
lib/hash/version.map | 7 +++
4 files changed, 151 insertions(+)
diff --git a/app/test/test_hash.c b/app/test/test_hash.c
index d586878a22..ebeda8c322 100644
--- a/app/test/test_hash.c
+++ b/app/test/test_hash.c
@@ -2183,6 +2183,100 @@ test_hash_rcu_qsbr_sync_mode(uint8_t ext_bkt)
}
+/*
+ * rte_hash_rcu_qsbr_dq_reclaim unit test.
+ */
+static int
+test_hash_rcu_qsbr_dq_reclaim(void)
+{
+ size_t sz;
+ int32_t status;
+ unsigned int total_entries = 8;
+ unsigned int freed, pending, available;
+ uint32_t reclaim_keys[8] = {10, 11, 12, 13, 14, 15, 16, 17};
+ struct rte_hash_rcu_config rcu_cfg = {0};
+ struct rte_hash_parameters hash_params = {
+ .name = "test_hash_rcu_qsbr_dq_reclaim",
+ .entries = total_entries,
+ .key_len = sizeof(uint32_t),
+ .hash_func = NULL,
+ .hash_func_init_val = 0,
+ .socket_id = 0,
+ };
+
+ hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
+
+ g_qsv = NULL;
+ g_handle = NULL;
+
+ printf("\n# Running RCU QSBR DQ mode, reclaim defer queue functional test\n");
+
+ g_handle = rte_hash_create(&hash_params);
+ RETURN_IF_ERROR_RCU_QSBR(g_handle == NULL, "Hash creation failed");
+
+ /* Create RCU QSBR variable */
+ sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
+ g_qsv = (struct rte_rcu_qsbr *)rte_zmalloc_socket(NULL, sz,
+ RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY);
+ RETURN_IF_ERROR_RCU_QSBR(g_qsv == NULL,
+ "RCU QSBR variable creation failed");
+
+ status = rte_rcu_qsbr_init(g_qsv, RTE_MAX_LCORE);
+ RETURN_IF_ERROR_RCU_QSBR(status != 0,
+ "RCU QSBR variable initialization failed");
+
+ rcu_cfg.v = g_qsv;
+ rcu_cfg.dq_size = total_entries;
+ rcu_cfg.mode = RTE_HASH_QSBR_MODE_DQ;
+
+ /* Attach RCU QSBR to hash table */
+ status = rte_hash_rcu_qsbr_add(g_handle, &rcu_cfg);
+ RETURN_IF_ERROR_RCU_QSBR(status != 0,
+ "Attach RCU QSBR to hash table failed");
+
+ /* Register pseudo reader */
+ status = rte_rcu_qsbr_thread_register(g_qsv, 0);
+ RETURN_IF_ERROR_RCU_QSBR(status != 0,
+ "RCU QSBR thread registration failed");
+ rte_rcu_qsbr_thread_online(g_qsv, 0);
+
+ /* Fill half of the hash table */
+ for (size_t i = 0; i < total_entries / 2; i++)
+ status = rte_hash_add_key(g_handle, &reclaim_keys[i]);
+
+ /* Lookup inserted elements*/
+ for (size_t i = 0; i < total_entries / 2; i++)
+ rte_hash_lookup(g_handle, &reclaim_keys[i]);
+
+ /* Try to put these elements into the defer queue*/
+ for (size_t i = 0; i < total_entries / 2; i++)
+ rte_hash_del_key(g_handle, &reclaim_keys[i]);
+
+ /* Reader quiescent */
+ rte_rcu_qsbr_quiescent(g_qsv, 0);
+
+ status = rte_hash_add_key(g_handle, &reclaim_keys[0]);
+ RETURN_IF_ERROR_RCU_QSBR(status < 0,
+ "failed to add key (pos[%u]=%d)", 0,
+ status);
+
+ /* This should be (total_entries / 2) + 1 (last add) */
+ unsigned int hash_size = rte_hash_count(g_handle);
+
+ /* Freed size should be (total_entries / 2) */
+ rte_hash_rcu_qsbr_dq_reclaim(g_handle, &freed, &pending, &available);
+
+ rte_hash_free(g_handle);
+ rte_free(g_qsv);
+
+ if (hash_size != (total_entries / 2 + 1) || freed != (total_entries / 2)) {
+ printf("Failed to reclaim defer queue\n");
+ return -1;
+ }
+
+ return 0;
+}
+
/*
* Do all unit and performance tests.
*/
@@ -2261,6 +2355,9 @@ test_hash(void)
if (test_hash_rcu_qsbr_sync_mode(1) < 0)
return -1;
+ if (test_hash_rcu_qsbr_dq_reclaim() < 0)
+ return -1;
+
return 0;
}
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 9cf94645f6..4a44aadd9a 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
return 0;
}
+int
+rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending, unsigned int *available)
+{
+ int ret;
+
+ if (h == NULL || h->hash_rcu_cfg == NULL) {
+ rte_errno = EINVAL;
+ return 1;
+ }
+
+ ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
+ freed, pending, available);
+ if (ret != 0) {
+ HASH_LOG(ERR,
+ "%s: could not reclaim the defer queue in hash table",
+ __func__);
+ return 1;
+ }
+
+ return 0;
+}
+
static inline void
remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
unsigned int i)
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..edfa262aca 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -674,6 +674,30 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
*/
int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
+/**
+ * Reclaim resources from the defer queue.
+ * This API reclaim the resources from the defer queue if rcu is enabled.
+ *
+ * @param h
+ * The hash object to reclaim resources.
+ * @param freed
+ * Number of resources that were freed.
+ * @param pending
+ * Number of resources pending on the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @param available
+ * Number of resources that can be added to the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @return
+ * On success - 0
+ * On error - 1 with error code set in rte_errno.
+ * Possible rte_errno codes are:
+ * - EINVAL - invalid pointer
+ */
+__rte_experimental
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending, unsigned int *available);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6f4bcdb71b..d348dd9196 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -53,3 +53,10 @@ INTERNAL {
rte_thash_gfni_stub;
rte_thash_gfni_bulk_stub;
};
+
+EXPERIMENTAL {
+ global:
+
+ # added in 24.07
+ rte_hash_rcu_qsbr_dq_reclaim;
+};
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] lib/hash: add defer queue reclaim API
2024-04-15 11:26 ` [PATCH v4] " Abdullah Ömer Yamaç
@ 2024-04-23 13:49 ` Abdullah Ömer Yamaç
2024-04-23 21:24 ` Stephen Hemminger
1 sibling, 0 replies; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-04-23 13:49 UTC (permalink / raw)
To: dev; +Cc: Honnappa Nagarahalli, Thomas Monjalon
[-- Attachment #1: Type: text/plain, Size: 8233 bytes --]
Hello, is there any other comment on this patch? Thanks
On Mon, Apr 15, 2024 at 2:26 PM Abdullah Ömer Yamaç <aomeryamac@gmail.com>
wrote:
> This patch adds a new feature to the hash library to allow the user to
> reclaim the defer queue. This is useful when the user wants to force
> reclaim resources that are not being used. This API is only available
> if the RCU is enabled.
>
> Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> ---
> app/test/test_hash.c | 97 ++++++++++++++++++++++++++++++++++++++
> lib/hash/rte_cuckoo_hash.c | 23 +++++++++
> lib/hash/rte_hash.h | 24 ++++++++++
> lib/hash/version.map | 7 +++
> 4 files changed, 151 insertions(+)
>
> diff --git a/app/test/test_hash.c b/app/test/test_hash.c
> index d586878a22..ebeda8c322 100644
> --- a/app/test/test_hash.c
> +++ b/app/test/test_hash.c
> @@ -2183,6 +2183,100 @@ test_hash_rcu_qsbr_sync_mode(uint8_t ext_bkt)
>
> }
>
> +/*
> + * rte_hash_rcu_qsbr_dq_reclaim unit test.
> + */
> +static int
> +test_hash_rcu_qsbr_dq_reclaim(void)
> +{
> + size_t sz;
> + int32_t status;
> + unsigned int total_entries = 8;
> + unsigned int freed, pending, available;
> + uint32_t reclaim_keys[8] = {10, 11, 12, 13, 14, 15, 16, 17};
> + struct rte_hash_rcu_config rcu_cfg = {0};
> + struct rte_hash_parameters hash_params = {
> + .name = "test_hash_rcu_qsbr_dq_reclaim",
> + .entries = total_entries,
> + .key_len = sizeof(uint32_t),
> + .hash_func = NULL,
> + .hash_func_init_val = 0,
> + .socket_id = 0,
> + };
> +
> + hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
> +
> + g_qsv = NULL;
> + g_handle = NULL;
> +
> + printf("\n# Running RCU QSBR DQ mode, reclaim defer queue
> functional test\n");
> +
> + g_handle = rte_hash_create(&hash_params);
> + RETURN_IF_ERROR_RCU_QSBR(g_handle == NULL, "Hash creation failed");
> +
> + /* Create RCU QSBR variable */
> + sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
> + g_qsv = (struct rte_rcu_qsbr *)rte_zmalloc_socket(NULL, sz,
> + RTE_CACHE_LINE_SIZE,
> SOCKET_ID_ANY);
> + RETURN_IF_ERROR_RCU_QSBR(g_qsv == NULL,
> + "RCU QSBR
> variable creation failed");
> +
> + status = rte_rcu_qsbr_init(g_qsv, RTE_MAX_LCORE);
> + RETURN_IF_ERROR_RCU_QSBR(status != 0,
> + "RCU QSBR
> variable initialization failed");
> +
> + rcu_cfg.v = g_qsv;
> + rcu_cfg.dq_size = total_entries;
> + rcu_cfg.mode = RTE_HASH_QSBR_MODE_DQ;
> +
> + /* Attach RCU QSBR to hash table */
> + status = rte_hash_rcu_qsbr_add(g_handle, &rcu_cfg);
> + RETURN_IF_ERROR_RCU_QSBR(status != 0,
> + "Attach RCU QSBR
> to hash table failed");
> +
> + /* Register pseudo reader */
> + status = rte_rcu_qsbr_thread_register(g_qsv, 0);
> + RETURN_IF_ERROR_RCU_QSBR(status != 0,
> + "RCU QSBR thread
> registration failed");
> + rte_rcu_qsbr_thread_online(g_qsv, 0);
> +
> + /* Fill half of the hash table */
> + for (size_t i = 0; i < total_entries / 2; i++)
> + status = rte_hash_add_key(g_handle, &reclaim_keys[i]);
> +
> + /* Lookup inserted elements*/
> + for (size_t i = 0; i < total_entries / 2; i++)
> + rte_hash_lookup(g_handle, &reclaim_keys[i]);
> +
> + /* Try to put these elements into the defer queue*/
> + for (size_t i = 0; i < total_entries / 2; i++)
> + rte_hash_del_key(g_handle, &reclaim_keys[i]);
> +
> + /* Reader quiescent */
> + rte_rcu_qsbr_quiescent(g_qsv, 0);
> +
> + status = rte_hash_add_key(g_handle, &reclaim_keys[0]);
> + RETURN_IF_ERROR_RCU_QSBR(status < 0,
> + "failed to add
> key (pos[%u]=%d)", 0,
> + status);
> +
> + /* This should be (total_entries / 2) + 1 (last add) */
> + unsigned int hash_size = rte_hash_count(g_handle);
> +
> + /* Freed size should be (total_entries / 2) */
> + rte_hash_rcu_qsbr_dq_reclaim(g_handle, &freed, &pending,
> &available);
> +
> + rte_hash_free(g_handle);
> + rte_free(g_qsv);
> +
> + if (hash_size != (total_entries / 2 + 1) || freed !=
> (total_entries / 2)) {
> + printf("Failed to reclaim defer queue\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> /*
> * Do all unit and performance tests.
> */
> @@ -2261,6 +2355,9 @@ test_hash(void)
> if (test_hash_rcu_qsbr_sync_mode(1) < 0)
> return -1;
>
> + if (test_hash_rcu_qsbr_dq_reclaim() < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 9cf94645f6..4a44aadd9a 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct
> rte_hash_rcu_config *cfg)
> return 0;
> }
>
> +int
> +rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> + unsigned int *pending, unsigned
> int *available)
> +{
> + int ret;
> +
> + if (h == NULL || h->hash_rcu_cfg == NULL) {
> + rte_errno = EINVAL;
> + return 1;
> + }
> +
> + ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size,
> + freed,
> pending, available);
> + if (ret != 0) {
> + HASH_LOG(ERR,
> + "%s: could not reclaim the defer queue in
> hash table",
> + __func__);
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> static inline void
> remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
> unsigned int i)
> diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> index 7ecc021111..edfa262aca 100644
> --- a/lib/hash/rte_hash.h
> +++ b/lib/hash/rte_hash.h
> @@ -674,6 +674,30 @@ rte_hash_iterate(const struct rte_hash *h, const void
> **key, void **data, uint32
> */
> int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config
> *cfg);
>
> +/**
> + * Reclaim resources from the defer queue.
> + * This API reclaim the resources from the defer queue if rcu is enabled.
> + *
> + * @param h
> + * The hash object to reclaim resources.
> + * @param freed
> + * Number of resources that were freed.
> + * @param pending
> + * Number of resources pending on the defer queue.
> + * This number might not be accurate if multi-thread safety is
> configured.
> + * @param available
> + * Number of resources that can be added to the defer queue.
> + * This number might not be accurate if multi-thread safety is
> configured.
> + * @return
> + * On success - 0
> + * On error - 1 with error code set in rte_errno.
> + * Possible rte_errno codes are:
> + * - EINVAL - invalid pointer
> + */
> +__rte_experimental
> +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> + unsigned int *pending, unsigned int *available);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/hash/version.map b/lib/hash/version.map
> index 6f4bcdb71b..d348dd9196 100644
> --- a/lib/hash/version.map
> +++ b/lib/hash/version.map
> @@ -53,3 +53,10 @@ INTERNAL {
> rte_thash_gfni_stub;
> rte_thash_gfni_bulk_stub;
> };
> +
> +EXPERIMENTAL {
> + global:
> +
> + # added in 24.07
> + rte_hash_rcu_qsbr_dq_reclaim;
> +};
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 9860 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] lib/hash: add defer queue reclaim API
2024-04-15 11:26 ` [PATCH v4] " Abdullah Ömer Yamaç
2024-04-23 13:49 ` Abdullah Ömer Yamaç
@ 2024-04-23 21:24 ` Stephen Hemminger
2024-04-25 14:03 ` Abdullah Ömer Yamaç
1 sibling, 1 reply; 30+ messages in thread
From: Stephen Hemminger @ 2024-04-23 21:24 UTC (permalink / raw)
To: Abdullah Ömer Yamaç; +Cc: dev
On Mon, 15 Apr 2024 11:26:02 +0000
Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
> + ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
> + freed, pending, available);
Indention here is odd. I would expect "freed," to line up right under h->dq.
Since rte_rcu_qsbrs_dq_reclaim logs error on invalid parameters, this function should as well.
Total indent fixes:
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 4a44aadd9a..e1ea810024 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1590,21 +1590,20 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
int
rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
- unsigned int *pending, unsigned int *available)
+ unsigned int *pending, unsigned int *available)
{
int ret;
if (h == NULL || h->hash_rcu_cfg == NULL) {
+ HASH_LOG(ERR, "Invalid input parameter");
rte_errno = EINVAL;
return 1;
}
ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
- freed, pending, available);
+ freed, pending, available);
if (ret != 0) {
- HASH_LOG(ERR,
- "%s: could not reclaim the defer queue in hash table",
- __func__);
+ HASH_LOG(ERR, "%s: could not reclaim the defer queue in hash table", __func__);
return 1;
}
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] lib/hash: add defer queue reclaim API
2024-04-23 21:24 ` Stephen Hemminger
@ 2024-04-25 14:03 ` Abdullah Ömer Yamaç
2024-04-27 19:54 ` [PATCH v5] " Abdullah Ömer Yamaç
0 siblings, 1 reply; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-04-25 14:03 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev
[-- Attachment #1: Type: text/plain, Size: 2055 bytes --]
Thanks for the comments. This is due to the tab size, and I will fix them.
On Wed, Apr 24, 2024 at 12:24 AM Stephen Hemminger <
stephen@networkplumber.org> wrote:
> On Mon, 15 Apr 2024 11:26:02 +0000
> Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> > + ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size,
> > + freed,
> pending, available);
>
> Indention here is odd. I would expect "freed," to line up right under
> h->dq.
> Since rte_rcu_qsbrs_dq_reclaim logs error on invalid parameters, this
> function should as well.
>
> Total indent fixes:
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 4a44aadd9a..e1ea810024 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -1590,21 +1590,20 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct
> rte_hash_rcu_config *cfg)
>
> int
> rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> - unsigned int *pending, unsigned
> int *available)
> + unsigned int *pending, unsigned int
> *available)
> {
> int ret;
>
> if (h == NULL || h->hash_rcu_cfg == NULL) {
> + HASH_LOG(ERR, "Invalid input parameter");
> rte_errno = EINVAL;
> return 1;
> }
>
> ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size,
> - freed,
> pending, available);
> + freed, pending, available);
> if (ret != 0) {
> - HASH_LOG(ERR,
> - "%s: could not reclaim the defer queue in
> hash table",
> - __func__);
> + HASH_LOG(ERR, "%s: could not reclaim the defer queue in
> hash table", __func__);
> return 1;
> }
>
>
[-- Attachment #2: Type: text/html, Size: 2751 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5] lib/hash: add defer queue reclaim API
2024-04-25 14:03 ` Abdullah Ömer Yamaç
@ 2024-04-27 19:54 ` Abdullah Ömer Yamaç
0 siblings, 0 replies; 30+ messages in thread
From: Abdullah Ömer Yamaç @ 2024-04-27 19:54 UTC (permalink / raw)
To: dev; +Cc: stephen, Abdullah Ömer Yamaç
This patch adds a new feature to the hash library to allow the user to
reclaim the defer queue. This is useful when the user wants to force
reclaim resources that are not being used. This API is only available
if the RCU is enabled.
Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
---
app/test/test_hash.c | 90 ++++++++++++++++++++++++++++++++++++++
lib/hash/rte_cuckoo_hash.c | 21 +++++++++
lib/hash/rte_hash.h | 25 +++++++++++
lib/hash/version.map | 7 +++
4 files changed, 143 insertions(+)
diff --git a/app/test/test_hash.c b/app/test/test_hash.c
index d586878a22..e763d0503f 100644
--- a/app/test/test_hash.c
+++ b/app/test/test_hash.c
@@ -2183,6 +2183,93 @@ test_hash_rcu_qsbr_sync_mode(uint8_t ext_bkt)
}
+/*
+ * rte_hash_rcu_qsbr_dq_reclaim unit test.
+ */
+static int test_hash_rcu_qsbr_dq_reclaim(void)
+{
+ size_t sz;
+ int32_t status;
+ unsigned int total_entries = 8;
+ unsigned int freed, pending, available;
+ uint32_t reclaim_keys[8] = {10, 11, 12, 13, 14, 15, 16, 17};
+ struct rte_hash_rcu_config rcu_cfg = {0};
+ struct rte_hash_parameters hash_params = {
+ .name = "test_hash_rcu_qsbr_dq_reclaim",
+ .entries = total_entries,
+ .key_len = sizeof(uint32_t),
+ .hash_func = NULL,
+ .hash_func_init_val = 0,
+ .socket_id = 0,
+ };
+
+ hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
+
+ g_qsv = NULL;
+ g_handle = NULL;
+
+ printf("\n# Running RCU QSBR DQ mode, reclaim defer queue functional test\n");
+
+ g_handle = rte_hash_create(&hash_params);
+ RETURN_IF_ERROR_RCU_QSBR(g_handle == NULL, "Hash creation failed");
+
+ /* Create RCU QSBR variable */
+ sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
+ g_qsv = (struct rte_rcu_qsbr *)rte_zmalloc_socket(NULL, sz, RTE_CACHE_LINE_SIZE,
+ SOCKET_ID_ANY);
+ RETURN_IF_ERROR_RCU_QSBR(g_qsv == NULL, "RCU QSBR variable creation failed");
+
+ status = rte_rcu_qsbr_init(g_qsv, RTE_MAX_LCORE);
+ RETURN_IF_ERROR_RCU_QSBR(status != 0, "RCU QSBR variable initialization failed");
+
+ rcu_cfg.v = g_qsv;
+ rcu_cfg.dq_size = total_entries;
+ rcu_cfg.mode = RTE_HASH_QSBR_MODE_DQ;
+
+ /* Attach RCU QSBR to hash table */
+ status = rte_hash_rcu_qsbr_add(g_handle, &rcu_cfg);
+ RETURN_IF_ERROR_RCU_QSBR(status != 0, "Attach RCU QSBR to hash table failed");
+
+ /* Register pseudo reader */
+ status = rte_rcu_qsbr_thread_register(g_qsv, 0);
+ RETURN_IF_ERROR_RCU_QSBR(status != 0, "RCU QSBR thread registration failed");
+ rte_rcu_qsbr_thread_online(g_qsv, 0);
+
+ /* Fill half of the hash table */
+ for (size_t i = 0; i < total_entries / 2; i++)
+ status = rte_hash_add_key(g_handle, &reclaim_keys[i]);
+
+ /* Lookup inserted elements*/
+ for (size_t i = 0; i < total_entries / 2; i++)
+ rte_hash_lookup(g_handle, &reclaim_keys[i]);
+
+ /* Try to put these elements into the defer queue*/
+ for (size_t i = 0; i < total_entries / 2; i++)
+ rte_hash_del_key(g_handle, &reclaim_keys[i]);
+
+ /* Reader quiescent */
+ rte_rcu_qsbr_quiescent(g_qsv, 0);
+
+ status = rte_hash_add_key(g_handle, &reclaim_keys[0]);
+ RETURN_IF_ERROR_RCU_QSBR(status < 0, "failed to add key (pos[%u]=%d)", 0, status);
+
+ /* This should be (total_entries / 2) + 1 (last add) */
+ unsigned int hash_size = rte_hash_count(g_handle);
+
+ /* Freed size should be (total_entries / 2) */
+ rte_hash_rcu_qsbr_dq_reclaim(g_handle, &freed, &pending, &available);
+
+ rte_hash_free(g_handle);
+ rte_free(g_qsv);
+
+ if (hash_size != (total_entries / 2 + 1) || freed != (total_entries / 2)) {
+ printf("Failed to reclaim defer queue\n");
+ return -1;
+ }
+
+ return 0;
+}
+
/*
* Do all unit and performance tests.
*/
@@ -2261,6 +2348,9 @@ test_hash(void)
if (test_hash_rcu_qsbr_sync_mode(1) < 0)
return -1;
+ if (test_hash_rcu_qsbr_dq_reclaim() < 0)
+ return -1;
+
return 0;
}
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 9cf94645f6..52ccada12a 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1588,6 +1588,27 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
return 0;
}
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed, unsigned int *pending,
+ unsigned int *available)
+{
+ int ret;
+
+ if (h == NULL || h->hash_rcu_cfg == NULL) {
+ HASH_LOG(ERR, "Invalid input parameter");
+ rte_errno = EINVAL;
+ return 1;
+ }
+
+ ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size, freed, pending,
+ available);
+ if (ret != 0) {
+ HASH_LOG(ERR, "%s: could not reclaim the defer queue in hash table", __func__);
+ return 1;
+ }
+
+ return 0;
+}
+
static inline void
remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
unsigned int i)
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..e1e0375fd5 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -674,6 +674,31 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
*/
int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
+/**
+ * Reclaim resources from the defer queue.
+ * This API reclaim the resources from the defer queue if rcu is enabled.
+ *
+ * @param h
+ * The hash object to reclaim resources.
+ * @param freed
+ * Number of resources that were freed.
+ * @param pending
+ * Number of resources pending on the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @param available
+ * Number of resources that can be added to the defer queue.
+ * This number might not be accurate if multi-thread safety is configured.
+ * @return
+ * On success - 0
+ * On error - 1 with error code set in rte_errno.
+ * Possible rte_errno codes are:
+ * - EINVAL - invalid pointer
+ */
+__rte_experimental
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+ unsigned int *pending,
+ unsigned int *available);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6f4bcdb71b..d348dd9196 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -53,3 +53,10 @@ INTERNAL {
rte_thash_gfni_stub;
rte_thash_gfni_bulk_stub;
};
+
+EXPERIMENTAL {
+ global:
+
+ # added in 24.07
+ rte_hash_rcu_qsbr_dq_reclaim;
+};
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2024-04-27 19:54 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-07 15:33 [PATCH] lib/hash,lib/rcu: feature hidden key count in hash Abdullah Ömer Yamaç
2024-02-16 12:41 ` Thomas Monjalon
2024-02-19 17:36 ` Medvedkin, Vladimir
2024-02-19 21:28 ` Abdullah Ömer Yamaç
2024-02-19 23:35 ` Honnappa Nagarahalli
2024-02-20 18:58 ` Abdullah Ömer Yamaç
2024-02-21 3:23 ` Honnappa Nagarahalli
2024-02-21 21:51 ` Abdullah Ömer Yamaç
2024-02-22 2:37 ` Honnappa Nagarahalli
2024-02-22 12:39 ` Abdullah Ömer Yamaç
2024-02-22 16:44 ` Honnappa Nagarahalli
2024-02-28 11:44 ` Abdullah Ömer Yamaç
2024-02-28 14:51 ` Honnappa Nagarahalli
2024-03-02 20:22 ` Abdullah Ömer Yamaç
2024-03-02 21:08 ` [PATCH v2] lib/hash: feature reclaim defer queue Abdullah Ömer Yamaç
2024-03-02 21:27 ` Abdullah Ömer Yamaç
2024-03-03 19:14 ` Honnappa Nagarahalli
2024-03-04 8:27 ` Abdullah Ömer Yamaç
2024-03-04 21:58 ` Honnappa Nagarahalli
2024-03-06 8:55 ` [PATCH v3] lib/hash: add defer queue reclaim API Abdullah Ömer Yamaç
2024-03-06 8:59 ` Abdullah Ömer Yamaç
2024-03-06 10:13 ` Abdullah Ömer Yamaç
2024-03-14 7:04 ` Abdullah Ömer Yamaç
2024-04-04 10:11 ` Abdullah Ömer Yamaç
2024-04-05 2:11 ` Honnappa Nagarahalli
2024-04-15 11:26 ` [PATCH v4] " Abdullah Ömer Yamaç
2024-04-23 13:49 ` Abdullah Ömer Yamaç
2024-04-23 21:24 ` Stephen Hemminger
2024-04-25 14:03 ` Abdullah Ömer Yamaç
2024-04-27 19:54 ` [PATCH v5] " Abdullah Ömer Yamaç
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).