From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2A957A00BE; Mon, 27 Apr 2020 13:44:54 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 078031D16A; Mon, 27 Apr 2020 13:44:54 +0200 (CEST) Received: from qrelay37.mxroute.com (qrelay37.mxroute.com [172.82.139.37]) by dpdk.org (Postfix) with ESMTP id 2EED61D15A for ; Mon, 27 Apr 2020 13:44:52 +0200 (CEST) Received: from filter004.mxroute.com ([149.28.56.236] 149.28.56.236.vultr.com) (Authenticated sender: mN4UYu2MZsgR) by qrelay37.mxroute.com (ZoneMTA) with ESMTPA id 171bb7436420000d83.002 for ; Mon, 27 Apr 2020 11:44:47 +0000 X-Zone-Loop: 9fa8020d16b0d48e7ae124ff456b4adefc551e3f20eb X-Originating-IP: [149.28.56.236] Received: from galaxy.mxroute.com (unknown [23.92.70.113]) by filter004.mxroute.com (Postfix) with ESMTPS id CFC153EA09; Mon, 27 Apr 2020 11:44:41 +0000 (UTC) Received: from [192.198.151.44] by galaxy.mxroute.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.91) (envelope-from ) id 1jT1lj-00033h-0P; Mon, 27 Apr 2020 07:17:47 -0400 To: Thomas Monjalon , dev@dpdk.org Cc: david.marchand@redhat.com, Olivier Matz , Neil Horman , John McNamara , Marko Kovacevic , Xiaoyun Li , Jingjing Wu , Andrew Rybchenko References: <20200117145754.11682-4-olivier.matz@6wind.com> <20200425222338.2787083-1-thomas@monjalon.net> From: Ray Kinsella Autocrypt: addr=mdr@ashroe.eu; keydata= mQINBFv8B3wBEAC+5ImcgbIvadt3axrTnt7Sxch3FsmWTTomXfB8YiuHT8KL8L/bFRQSL1f6 ASCHu3M89EjYazlY+vJUWLr0BhK5t/YI7bQzrOuYrl9K94vlLwzD19s/zB/g5YGGR5plJr0s JtJsFGEvF9LL3e+FKMRXveQxBB8A51nAHfwG0WSyx53d61DYz7lp4/Y4RagxaJoHp9lakn8j HV2N6rrnF+qt5ukj5SbbKWSzGg5HQF2t0QQ5tzWhCAKTfcPlnP0GymTBfNMGOReWivi3Qqzr S51Xo7hoGujUgNAM41sxpxmhx8xSwcQ5WzmxgAhJ/StNV9cb3HWIoE5StCwQ4uXOLplZNGnS uxNdegvKB95NHZjRVRChg/uMTGpg9PqYbTIFoPXjuk27sxZLRJRrueg4tLbb3HM39CJwSB++ YICcqf2N+GVD48STfcIlpp12/HI+EcDSThzfWFhaHDC0hyirHxJyHXjnZ8bUexI/5zATn/ux TpMbc/vicJxeN+qfaVqPkCbkS71cHKuPluM3jE8aNCIBNQY1/j87k5ELzg3qaesLo2n1krBH bKvFfAmQuUuJT84/IqfdVtrSCTabvDuNBDpYBV0dGbTwaRfE7i+LiJJclUr8lOvHUpJ4Y6a5 0cxEPxm498G12Z3NoY/mP5soItPIPtLR0rA0fage44zSPwp6cQARAQABtBxSYXkgS2luc2Vs bGEgPG1kckBhc2hyb2UuZXU+iQJUBBMBCAA+FiEEcDUDlKDJaDuJlfZfdJdaH/sCCpsFAlv8 B3wCGyMFCQlmAYAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQdJdaH/sCCptdtRAAl0oE msa+djBVYLIsax+0f8acidtWg2l9f7kc2hEjp9h9aZCpPchQvhhemtew/nKavik3RSnLTAyn B3C/0GNlmvI1l5PFROOgPZwz4xhJKGN7jOsRrbkJa23a8ly5UXwF3Vqnlny7D3z+7cu1qq/f VRK8qFyWkAb+xgqeZ/hTcbJUWtW+l5Zb+68WGEp8hB7TuJLEWb4+VKgHTpQ4vElYj8H3Z94a 04s2PJMbLIZSgmKDASnyrKY0CzTpPXx5rSJ1q+B1FCsfepHLqt3vKSALa3ld6bJ8fSJtDUJ7 JLiU8dFZrywgDIVme01jPbjJtUScW6jONLvhI8Z2sheR71UoKqGomMHNQpZ03ViVWBEALzEt TcjWgJFn8yAmxqM4nBnZ+hE3LbMo34KCHJD4eg18ojDt3s9VrDLa+V9fNxUHPSib9FD9UX/1 +nGfU/ZABmiTuUDM7WZdXri7HaMpzDRJUKI6b+/uunF8xH/h/MHW16VuMzgI5dkOKKv1LejD dT5mA4R+2zBS+GsM0oa2hUeX9E5WwjaDzXtVDg6kYq8YvEd+m0z3M4e6diFeLS77/sAOgaYL 92UcoKD+Beym/fVuC6/55a0e12ksTmgk5/ZoEdoNQLlVgd2INtvnO+0k5BJcn66ZjKn3GbEC VqFbrnv1GnA58nEInRCTzR1k26h9nmS5Ag0EW/wHfAEQAMth1vHr3fOZkVOPfod3M6DkQir5 xJvUW5EHgYUjYCPIa2qzgIVVuLDqZgSCCinyooG5dUJONVHj3nCbITCpJp4eB3PI84RPfDcC hf/V34N/Gx5mTeoymSZDBmXT8YtvV/uJvn+LvHLO4ZJdvq5ZxmDyxfXFmkm3/lLw0+rrNdK5 pt6OnVlCqEU9tcDBezjUwDtOahyV20XqxtUttN4kQWbDRkhT+HrA9WN9l2HX91yEYC+zmF1S OhBqRoTPLrR6g4sCWgFywqztpvZWhyIicJipnjac7qL/wRS+wrWfsYy6qWLIV80beN7yoa6v ccnuy4pu2uiuhk9/edtlmFE4dNdoRf7843CV9k1yRASTlmPkU59n0TJbw+okTa9fbbQgbIb1 pWsAuicRHyLUIUz4f6kPgdgty2FgTKuPuIzJd1s8s6p2aC1qo+Obm2gnBTduB+/n1Jw+vKpt 07d+CKEKu4CWwvZZ8ktJJLeofi4hMupTYiq+oMzqH+V1k6QgNm0Da489gXllU+3EFC6W1qKj tkvQzg2rYoWeYD1Qn8iXcO4Fpk6wzylclvatBMddVlQ6qrYeTmSbCsk+m2KVrz5vIyja0o5Y yfeN29s9emXnikmNfv/dA5fpi8XCANNnz3zOfA93DOB9DBf0TQ2/OrSPGjB3op7RCfoPBZ7u AjJ9dM7VABEBAAGJAjwEGAEIACYWIQRwNQOUoMloO4mV9l90l1of+wIKmwUCW/wHfAIbDAUJ CWYBgAAKCRB0l1of+wIKm3KlD/9w/LOG5rtgtCUWPl4B3pZvGpNym6XdK8cop9saOnE85zWf u+sKWCrxNgYkYP7aZrYMPwqDvilxhbTsIJl5HhPgpTO1b0i+c0n1Tij3EElj5UCg3q8mEc17 c+5jRrY3oz77g7E3oPftAjaq1ybbXjY4K32o3JHFR6I8wX3m9wJZJe1+Y+UVrrjY65gZFxcA thNVnWKErarVQGjeNgHV4N1uF3pIx3kT1N4GSnxhoz4Bki91kvkbBhUgYfNflGURfZT3wIKK +d50jd7kqRouXUCzTdzmDh7jnYrcEFM4nvyaYu0JjSS5R672d9SK5LVIfWmoUGzqD4AVmUW8 pcv461+PXchuS8+zpltR9zajl72Q3ymlT4BTAQOlCWkD0snBoKNUB5d2EXPNV13nA0qlm4U2 GpROfJMQXjV6fyYRvttKYfM5xYKgRgtP0z5lTAbsjg9WFKq0Fndh7kUlmHjuAIwKIV4Tzo75 QO2zC0/NTaTjmrtiXhP+vkC4pcrOGNsbHuaqvsc/ZZ0siXyYsqbctj/sCd8ka2r94u+c7o4l BGaAm+FtwAfEAkXHu4y5Phuv2IRR+x1wTey1U1RaEPgN8xq0LQ1OitX4t2mQwjdPihZQBCnZ wzOrkbzlJMNrMKJpEgulmxAHmYJKgvZHXZXtLJSejFjR0GdHJcL5rwVOMWB8cg== Message-ID: <6bd397c6-3012-ae10-af9a-eabee96d5c92@ashroe.eu> Date: Mon, 27 Apr 2020 12:44:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200425222338.2787083-1-thomas@monjalon.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-AuthUser: mdr@ashroe.eu Subject: Re: [dpdk-dev] [PATCH v3] mempool: return 0 if area is too small on populate X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 25/04/2020 23:23, Thomas Monjalon wrote: > From: Olivier Matz > > Change rte_mempool_populate_iova() and rte_mempool_populate_iova() to > return 0 instead of -EINVAL when there is not enough room to store one > object, as it can be helpful for applications to distinguish this > specific case. > > As this is an ABI change, use symbol versioning to preserve old > behavior for binary applications. > > Signed-off-by: Olivier Matz > --- > > changes in v3: > - rebase > - remove deprecation notice > - notify API change in release notes > - fix ABI version from 20.0.1 to 20.0.2 (should be 21 maybe) yes, should be v21. > > This v3 cannot be merged because of a false positive ABI check: > > 2 Removed functions: > 'function int rte_mempool_populate_iova(rte_mempool*, char*, rte_iova_t, size_t, rte_mempool_memchunk_free_cb_t*, void*)' {rte_mempool_populate_iova@@DPDK_20.0} > 'function int rte_mempool_populate_virt(rte_mempool*, char*, size_t, size_t, rte_mempool_memchunk_free_cb_t*, void*)' {rte_mempool_populate_virt@@DPDK_20.0} Well it's not really a false positive, as the v20 symbols are now missing. See notes on VERSION_SYMBOL. > > --- > doc/guides/rel_notes/deprecation.rst | 5 -- > doc/guides/rel_notes/release_20_05.rst | 4 ++ > examples/ntb/ntb_fwd.c | 2 +- > lib/librte_mempool/meson.build | 2 + > lib/librte_mempool/rte_mempool.c | 77 ++++++++++++++++++---- > lib/librte_mempool/rte_mempool.h | 14 ++-- > lib/librte_mempool/rte_mempool_version.map | 7 ++ > 7 files changed, 90 insertions(+), 21 deletions(-) > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst > index 1339f54f5f..20aa745b77 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -65,11 +65,6 @@ Deprecation Notices > structure would be made internal (or removed if all dependencies are cleared) > in future releases. > > -* mempool: starting from v20.05, the API of rte_mempool_populate_iova() > - and rte_mempool_populate_virt() will change to return 0 instead > - of -EINVAL when there is not enough room to store one object. The ABI > - will be preserved until 20.11. > - > * ethdev: the legacy filter API, including > ``rte_eth_dev_filter_supported()``, ``rte_eth_dev_filter_ctrl()`` as well > as filter types MACVLAN, ETHERTYPE, FLEXIBLE, SYN, NTUPLE, TUNNEL, FDIR, > diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst > index b124c3f287..ab20a7d021 100644 > --- a/doc/guides/rel_notes/release_20_05.rst > +++ b/doc/guides/rel_notes/release_20_05.rst > @@ -241,6 +241,10 @@ API Changes > Also, make sure to start the actual text at the margin. > ========================================================= > > +* mempool: The API of ``rte_mempool_populate_iova()`` and > + ``rte_mempool_populate_virt()`` changed to return 0 instead of -EINVAL > + when there is not enough room to store one object. > + > > ABI Changes > ----------- > diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c > index d49189e175..eba8ebf9fa 100644 > --- a/examples/ntb/ntb_fwd.c > +++ b/examples/ntb/ntb_fwd.c > @@ -1319,7 +1319,7 @@ ntb_mbuf_pool_create(uint16_t mbuf_seg_size, uint32_t nb_mbuf, > mz->len - ntb_info.ntb_hdr_size, > ntb_mempool_mz_free, > (void *)(uintptr_t)mz); > - if (ret < 0) { > + if (ret <= 0) { > rte_memzone_free(mz); > rte_mempool_free(mp); > return NULL; > diff --git a/lib/librte_mempool/meson.build b/lib/librte_mempool/meson.build > index a6e861cbfc..7dbe6b9bea 100644 > --- a/lib/librte_mempool/meson.build > +++ b/lib/librte_mempool/meson.build > @@ -9,6 +9,8 @@ foreach flag: extra_flags > endif > endforeach > > +use_function_versioning = true > + > sources = files('rte_mempool.c', 'rte_mempool_ops.c', > 'rte_mempool_ops_default.c', 'mempool_trace_points.c') > headers = files('rte_mempool.h', 'rte_mempool_trace.h', > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > index 0be8f9f59d..edbdafaafb 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > > #include "rte_mempool.h" > #include "rte_mempool_trace.h" > @@ -303,12 +304,17 @@ mempool_ops_alloc_once(struct rte_mempool *mp) > return 0; > } > > +__vsym int > +rte_mempool_populate_iova_v20_0_2(struct rte_mempool *mp, char *vaddr, > + rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, > + void *opaque); > + > /* Add objects in the pool, using a physically contiguous memory > * zone. Return the number of objects added, or a negative value > * on error. > */ > -static int > -__rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, > +__vsym int > +rte_mempool_populate_iova_v20_0_2(struct rte_mempool *mp, char *vaddr, > rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, > void *opaque) > { > @@ -366,14 +372,27 @@ __rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, > return ret; > } > > -int > -rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, > +BIND_DEFAULT_SYMBOL(rte_mempool_populate_iova, _v20_0_2, 20.0.2); as discussed v21. > +MAP_STATIC_SYMBOL( > + int rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, > + rte_iova_t iova, size_t len, > + rte_mempool_memchunk_free_cb_t *free_cb, > + void *opaque), > + rte_mempool_populate_iova_v20_0_2); > + > +__vsym int > +rte_mempool_populate_iova_v20(struct rte_mempool *mp, char *vaddr, > + rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, > + void *opaque); > + > +__vsym int > +rte_mempool_populate_iova_v20(struct rte_mempool *mp, char *vaddr, > rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, > void *opaque) > { > int ret; > > - ret = __rte_mempool_populate_iova(mp, vaddr, iova, len, free_cb, > + ret = rte_mempool_populate_iova_v20_0_2(mp, vaddr, iova, len, free_cb, > opaque); > if (ret == 0) > ret = -EINVAL; > @@ -381,6 +400,7 @@ rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, > rte_mempool_trace_populate_iova(mp, vaddr, iova, len, free_cb, opaque); > return ret; > } > +VERSION_SYMBOL(rte_mempool_populate_iova, _v20_0, 20.0); this should be VERSION_SYMBOL(rte_mempool_populate_iova, _v20, 20.0); > > static rte_iova_t > get_iova(void *addr) > @@ -395,11 +415,16 @@ get_iova(void *addr) > return ms->iova + RTE_PTR_DIFF(addr, ms->addr); > } > > +__vsym int > +rte_mempool_populate_virt_v20_0_2(struct rte_mempool *mp, char *addr, > + size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb, > + void *opaque); > + > /* Populate the mempool with a virtual area. Return the number of > * objects added, or a negative value on error. > */ > -int > -rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, > +__vsym int > +rte_mempool_populate_virt_v20_0_2(struct rte_mempool *mp, char *addr, > size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb, > void *opaque) > { > @@ -432,7 +457,7 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, > break; > } > > - ret = __rte_mempool_populate_iova(mp, addr + off, iova, > + ret = rte_mempool_populate_iova_v20_0_2(mp, addr + off, iova, > phys_len, free_cb, opaque); > if (ret == 0) > continue; > @@ -443,9 +468,6 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, > cnt += ret; > } > > - if (cnt == 0) > - return -EINVAL; > - > rte_mempool_trace_populate_virt(mp, addr, len, pg_sz, free_cb, opaque); > return cnt; > > @@ -453,6 +475,35 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, > rte_mempool_free_memchunks(mp); > return ret; > } > +BIND_DEFAULT_SYMBOL(rte_mempool_populate_virt, _v20_0_2, 20.0.2); as discussed v21. > +MAP_STATIC_SYMBOL( > + int rte_mempool_populate_virt(struct rte_mempool *mp, > + char *addr, size_t len, size_t pg_sz, > + rte_mempool_memchunk_free_cb_t *free_cb, > + void *opaque), > + rte_mempool_populate_virt_v20_0_2); > + > +__vsym int > +rte_mempool_populate_virt_v20(struct rte_mempool *mp, char *addr, > + size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb, > + void *opaque); > + > +__vsym int > +rte_mempool_populate_virt_v20(struct rte_mempool *mp, char *addr, > + size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb, > + void *opaque) > +{ > + int ret; > + > + ret = rte_mempool_populate_virt_v20_0_2(mp, addr, len, pg_sz, > + free_cb, opaque); > + > + if (ret == 0) > + ret = -EINVAL; > + > + return ret; > +} > +VERSION_SYMBOL(rte_mempool_populate_virt, _v20_0, 20.0); this should be VERSION_SYMBOL(rte_mempool_populate_virt, _v20, 20.0); > > /* Get the minimal page size used in a mempool before populating it. */ > int > @@ -609,6 +660,8 @@ rte_mempool_populate_default(struct rte_mempool *mp) > mz->len, pg_sz, > rte_mempool_memchunk_mz_free, > (void *)(uintptr_t)mz); > + if (ret == 0) /* should not happen */ > + ret = -ENOBUFS; > if (ret < 0) { > rte_memzone_free(mz); > goto fail; > @@ -701,6 +754,8 @@ rte_mempool_populate_anon(struct rte_mempool *mp) > > ret = rte_mempool_populate_virt(mp, addr, size, getpagesize(), > rte_mempool_memchunk_anon_free, addr); > + if (ret == 0) /* should not happen */ > + ret = -ENOBUFS; > if (ret < 0) { > rte_errno = -ret; > goto fail; > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h > index 6e0573ea42..652d19f9f1 100644 > --- a/lib/librte_mempool/rte_mempool.h > +++ b/lib/librte_mempool/rte_mempool.h > @@ -1112,9 +1112,12 @@ rte_mempool_free(struct rte_mempool *mp); > * @param opaque > * An opaque argument passed to free_cb. > * @return > - * The number of objects added on success. > + * The number of objects added on success (strictly positive). > * On error, the chunk is not added in the memory list of the > - * mempool and a negative errno is returned. > + * mempool the following code is returned: > + * (0): not enough room in chunk for one object. > + * (-ENOSPC): mempool is already populated. > + * (-ENOMEM): allocation failure. > */ > int rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, > rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, > @@ -1139,9 +1142,12 @@ int rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, > * @param opaque > * An opaque argument passed to free_cb. > * @return > - * The number of objects added on success. > + * The number of objects added on success (strictly positive). > * On error, the chunk is not added in the memory list of the > - * mempool and a negative errno is returned. > + * mempool the following code is returned: > + * (0): not enough room in chunk for one object. > + * (-ENOSPC): mempool is already populated. > + * (-ENOMEM): allocation failure. > */ > int > rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, > diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map > index 695dd6e04f..9e58093665 100644 > --- a/lib/librte_mempool/rte_mempool_version.map > +++ b/lib/librte_mempool/rte_mempool_version.map > @@ -31,6 +31,13 @@ DPDK_20.0 { > local: *; > }; > > +DPDK_20.0.2 { as discussed DPDK_21 > + global: > + > + rte_mempool_populate_iova; > + rte_mempool_populate_virt; > +} DPDK_20.0; > + > EXPERIMENTAL { > global: > >