From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C0A5F42F83; Wed, 2 Aug 2023 10:58:08 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9322940DDB; Wed, 2 Aug 2023 10:58:08 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 57C7C4021D for ; Wed, 2 Aug 2023 10:58:07 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 2E5BA20634; Wed, 2 Aug 2023 10:58:07 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v1] mempool: fix some errors in html api Date: Wed, 2 Aug 2023 10:58:05 +0200 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87AB7@smartserver.smartshare.dk> In-Reply-To: <20230703061743.10576-1-rma.ma@jaguarmicro.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v1] mempool: fix some errors in html api Thread-Index: AdmtdiRqoOl179lrTjq5Tb4+JGtEswXnu/bg References: <20230703061743.10576-1-rma.ma@jaguarmicro.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Rma Ma" , "dpdk-dev" , "Olivier Matz" , "Andrew Rybchenko" Cc: "Ashwin Sekhar T K" , "Pavan Nikhilesh" , "Harman Kalra" , "Hemant Agrawal" , "Sachin Saxena" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org +CC various mempool driver maintainers > From: Rma Ma [mailto:rma.ma@jaguarmicro.com] > Sent: Monday, 3 July 2023 08.18 >=20 > This patch fix some error descriptions of > return value in mempool api which affect in html api. >=20 > Signed-off-by: Rma Ma > --- > lib/mempool/rte_mempool.h | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) >=20 > diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h > index 160975a7e7..d4d707533a 100644 > --- a/lib/mempool/rte_mempool.h > +++ b/lib/mempool/rte_mempool.h > @@ -1610,7 +1610,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. > * > @@ -1624,7 +1624,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, > @@ -1646,7 +1646,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. > * > @@ -1658,7 +1658,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) > @@ -1677,7 +1677,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. > * > @@ -1687,7 +1687,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) > -- > 2.17.1 Unfortunately, it is not that simple... Here is the list of where mempool drivers are registered: https://elixir.bootlin.com/dpdk/v23.07/C/ident/RTE_MEMPOOL_REGISTER_OPS Some of the mempool drivers return -ENOBUFS, e.g. the ring driver and = the stack driver: https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/ring/rte_me= mpool_ring.c#L145 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/ring/rte_me= mpool_ring.c#L48 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/stack/rte_m= empool_stack.c#L78 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/stack/rte_m= empool_stack.c#L59 However, I found one mempool driver (Marvell cnxk) that returns -ENOENT: https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/cnxk/cn10k_= hwpool_ops.c#L261 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/cnxk/cn10k_= hwpool_ops.c#L69 And one mempool driver (Cavium OCTEON TX) returns -ENOMEM: https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/octeontx/rt= e_mempool_octeontx.c#L194 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/octeontx/rt= e_mempool_octeontx.c#L111 One mempool driver (NXP dpaa) returns -ENOBUFS, or (if requesting too = many objects) -1: https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/dpaa/dpaa_m= empool.c#L351 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/dpaa/dpaa_m= empool.c#L225 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/dpaa/dpaa_m= empool.c#L257 And one mempool driver (NXP dpaa2) returns -ENOBUFS, or in rare cases = -ENOENT or -1: https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/dpaa2/dpaa2= _hw_mempool.c#L471 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/dpaa2/dpaa2= _hw_mempool.c#L373 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/dpaa2/dpaa2= _hw_mempool.c#L336 https://elixir.bootlin.com/dpdk/v23.07/source/drivers/mempool/dpaa2/dpaa2= _hw_mempool.c#L342 The root cause for this misery is the general lack of documentation of = callbacks in DPDK (not just in the mempool library!), which leaves the = callback implementers unaware of what to return. E.g. the mempool driver = enqueue and dequeue callback types have no descriptions of their return = values: https://elixir.bootlin.com/dpdk/v23.07/source/lib/mempool/rte_mempool.h#L= 467 https://elixir.bootlin.com/dpdk/v23.07/source/lib/mempool/rte_mempool.h#L= 473 So in theory, the mempool drivers are free to return any value... = Initially, I didn't think any mempool drivers would return -1 and set = rte_errno, but apparently they do (except the driver doesn't set = rte_errno when returning -1)! On a techboard meeting not long ago, Tyler mentioned the lack of = consistency in return value conventions as an general annoyance. Now = having looked at these mempool drivers, I realize that the situation is = much worse than I would imagine in my worst nightmare! So, how do we want to fix this: 1. Specify the allowed return values in the documentation for the = mempool library callback types, and require the mempool drivers to = follow the updated specification? 2. Update mempool API documentation to list the many currently possible = error return values (-ENOBUFS, -ENOENT, -ENOMEM, and -EPERM (=3D-1))? 3. Update mempool API documentation to say something along the lines of = "negative return value indicates error; rte_errno is not set if -1"? 4. Switch to a general convention of returning -1 and setting rte_errno = in all DPDK APIs, starting with the mempool library? However; this would = still require a documented list of possible rte_errno values set by the = functions - essentially the problem would remain the same: "possible = return values" vs. "possible rte_errno values". Personally, I am in strong favor of any option that tightens the API = specification and treats non-conformance as bugs.