[PATCH] mempool: fix the description of some function return values

[PATCH] mempool: fix the description of some function return values

[Zhiheng Chen]

In rte_mempool_ring.c, the committer uses the symbol ENOBUFS to describe the return value of function common_ring_sc_dequeue, but in rte_mempool.h, the symbol ENOENT is used to describe the return value of function rte_mempool_get. If the user of dpdk uses the symbol ENOENT as the judgment condition of the return value, it may cause some abnormal phenomena in their own programs, such as when the mempool space is exhausted.

Fixes: ea5dd2744b90 ("mempool: cache optimisations")

Signed-off-by: Zhiheng Chen <chenzhiheng0227@gmail.com>
---
 lib/mempool/rte_mempool.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 1e7a3c1527..2a7d3455ef 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -1521,7 +1521,7 @@ rte_mempool_do_generic_get(struct rte_mempool *mp, void **obj_table,
  * Get several objects from the mempool.
  *
  * If cache is enabled, objects will be retrieved first from cache,
- * subsequently from the common pool. Note that it can return -ENOENT when
+ * subsequently from the common pool. Note that it can return -ENOBUFS when
  * the local cache and common pool are empty, even if cache from other
  * lcores are full.
  *
@@ -1535,7 +1535,7 @@ rte_mempool_do_generic_get(struct rte_mempool *mp, void **obj_table,
  *   A pointer to a mempool cache structure. May be NULL if not needed.
  * @return
  *   - 0: Success; objects taken.
- *   - -ENOENT: Not enough entries in the mempool; no object is retrieved.
+ *   - -ENOBUFS: Not enough entries in the mempool; no object is retrieved.
  */
 static __rte_always_inline int
 rte_mempool_generic_get(struct rte_mempool *mp, void **obj_table,
@@ -1557,7 +1557,7 @@ rte_mempool_generic_get(struct rte_mempool *mp, void **obj_table,
  * mempool creation time (see flags).
  *
  * If cache is enabled, objects will be retrieved first from cache,
- * subsequently from the common pool. Note that it can return -ENOENT when
+ * subsequently from the common pool. Note that it can return -ENOBUFS when
  * the local cache and common pool are empty, even if cache from other
  * lcores are full.
  *
@@ -1569,7 +1569,7 @@ rte_mempool_generic_get(struct rte_mempool *mp, void **obj_table,
  *   The number of objects to get from the mempool to obj_table.
  * @return
  *   - 0: Success; objects taken
- *   - -ENOENT: Not enough entries in the mempool; no object is retrieved.
+ *   - -ENOBUFS: Not enough entries in the mempool; no object is retrieved.
  */
 static __rte_always_inline int
 rte_mempool_get_bulk(struct rte_mempool *mp, void **obj_table, unsigned int n)
@@ -1588,7 +1588,7 @@ rte_mempool_get_bulk(struct rte_mempool *mp, void **obj_table, unsigned int n)
  * mempool creation (see flags).
  *
  * If cache is enabled, objects will be retrieved first from cache,
- * subsequently from the common pool. Note that it can return -ENOENT when
+ * subsequently from the common pool. Note that it can return -ENOBUFS when
  * the local cache and common pool are empty, even if cache from other
  * lcores are full.
  *
@@ -1598,7 +1598,7 @@ rte_mempool_get_bulk(struct rte_mempool *mp, void **obj_table, unsigned int n)
  *   A pointer to a void * pointer (object) that will be filled.
  * @return
  *   - 0: Success; objects taken.
- *   - -ENOENT: Not enough entries in the mempool; no object is retrieved.
+ *   - -ENOBUFS: Not enough entries in the mempool; no object is retrieved.
  */
 static __rte_always_inline int
 rte_mempool_get(struct rte_mempool *mp, void **obj_p)
@@ -1777,7 +1777,7 @@ void rte_mempool_list_dump(FILE *f);
  *   The pointer to the mempool matching the name, or NULL if not found.
  *   NULL on error
  *   with rte_errno set appropriately. Possible rte_errno values include:
- *    - ENOENT - required entry not available to return.
+ *    - ENOBUFS - required entry not available to return.
  *
  */
 struct rte_mempool *rte_mempool_lookup(const char *name);
-- 
2.32.0

RE: [PATCH] mempool: fix the description of some function return values

[Morten Brørup]

> From: Zhiheng Chen [mailto:chenzhiheng0227@gmail.com]
> Sent: Saturday, 18 December 2021 18.14
> 
> In rte_mempool_ring.c, the committer uses the symbol ENOBUFS to
> describe the return value of function common_ring_sc_dequeue, but in
> rte_mempool.h, the symbol ENOENT is used to describe the return value
> of function rte_mempool_get. If the user of dpdk uses the symbol ENOENT
> as the judgment condition of the return value, it may cause some
> abnormal phenomena in their own programs, such as when the mempool
> space is exhausted.
> 
> Fixes: ea5dd2744b90 ("mempool: cache optimisations")
> 
> Signed-off-by: Zhiheng Chen <chenzhiheng0227@gmail.com>
> ---
>  lib/mempool/rte_mempool.h | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
> index 1e7a3c1527..2a7d3455ef 100644
> --- a/lib/mempool/rte_mempool.h
> +++ b/lib/mempool/rte_mempool.h
> @@ -1521,7 +1521,7 @@ rte_mempool_do_generic_get(struct rte_mempool
> *mp, void **obj_table,
>   * Get several objects from the mempool.
>   *
>   * If cache is enabled, objects will be retrieved first from cache,
> - * subsequently from the common pool. Note that it can return -ENOENT
> when
> + * subsequently from the common pool. Note that it can return -ENOBUFS
> when
>   * the local cache and common pool are empty, even if cache from other
>   * lcores are full.
>   *
> @@ -1535,7 +1535,7 @@ rte_mempool_do_generic_get(struct rte_mempool
> *mp, void **obj_table,
>   *   A pointer to a mempool cache structure. May be NULL if not
> needed.
>   * @return
>   *   - 0: Success; objects taken.
> - *   - -ENOENT: Not enough entries in the mempool; no object is
> retrieved.
> + *   - -ENOBUFS: Not enough entries in the mempool; no object is
> retrieved.
>   */
>  static __rte_always_inline int
>  rte_mempool_generic_get(struct rte_mempool *mp, void **obj_table,
> @@ -1557,7 +1557,7 @@ rte_mempool_generic_get(struct rte_mempool *mp,
> void **obj_table,
>   * mempool creation time (see flags).
>   *
>   * If cache is enabled, objects will be retrieved first from cache,
> - * subsequently from the common pool. Note that it can return -ENOENT
> when
> + * subsequently from the common pool. Note that it can return -ENOBUFS
> when
>   * the local cache and common pool are empty, even if cache from other
>   * lcores are full.
>   *
> @@ -1569,7 +1569,7 @@ rte_mempool_generic_get(struct rte_mempool *mp,
> void **obj_table,
>   *   The number of objects to get from the mempool to obj_table.
>   * @return
>   *   - 0: Success; objects taken
> - *   - -ENOENT: Not enough entries in the mempool; no object is
> retrieved.
> + *   - -ENOBUFS: Not enough entries in the mempool; no object is
> retrieved.
>   */
>  static __rte_always_inline int
>  rte_mempool_get_bulk(struct rte_mempool *mp, void **obj_table,
> unsigned int n)
> @@ -1588,7 +1588,7 @@ rte_mempool_get_bulk(struct rte_mempool *mp, void
> **obj_table, unsigned int n)
>   * mempool creation (see flags).
>   *
>   * If cache is enabled, objects will be retrieved first from cache,
> - * subsequently from the common pool. Note that it can return -ENOENT
> when
> + * subsequently from the common pool. Note that it can return -ENOBUFS
> when
>   * the local cache and common pool are empty, even if cache from other
>   * lcores are full.
>   *
> @@ -1598,7 +1598,7 @@ rte_mempool_get_bulk(struct rte_mempool *mp, void
> **obj_table, unsigned int n)
>   *   A pointer to a void * pointer (object) that will be filled.
>   * @return
>   *   - 0: Success; objects taken.
> - *   - -ENOENT: Not enough entries in the mempool; no object is
> retrieved.
> + *   - -ENOBUFS: Not enough entries in the mempool; no object is
> retrieved.
>   */
>  static __rte_always_inline int
>  rte_mempool_get(struct rte_mempool *mp, void **obj_p)

Good catch! These functions call some underlying functions, and the descriptions of those functions don't mention -ENOENT or -ENOBUFS; they only say <0 to indicate error. Please consider updating the underlying functions too, so the chain of descriptions is complete. The description of this function should not be based on code inspection of its underlying public functions, it should be based on the descriptions of its underlying functions. Alternatively, describe the return value like the underlying functions: <0 means error returned from dequeue function.

> @@ -1777,7 +1777,7 @@ void rte_mempool_list_dump(FILE *f);
>   *   The pointer to the mempool matching the name, or NULL if not
> found.
>   *   NULL on error
>   *   with rte_errno set appropriately. Possible rte_errno values
> include:
> - *    - ENOENT - required entry not available to return.
> + *    - ENOBUFS - required entry not available to return.
>   *
>   */
>  struct rte_mempool *rte_mempool_lookup(const char *name);

This one is using ENOENT, so don't change the description. Ref:
http://code.dpdk.org/dpdk/latest/source/lib/mempool/rte_mempool.c#L1356

RE: [PATCH] mempool: fix the description of some function return values

[zhiheng chen]

[-- Attachment #1: Type: text/plain, Size: 79 bytes --]

Thanks for your suggestions and time, I will update the underlying functions.


[-- Attachment #2: Type: text/html, Size: 467 bytes --]