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 3CB0EA00C2; Wed, 22 Apr 2020 10:36:51 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 243E11C1FE; Wed, 22 Apr 2020 10:36:50 +0200 (CEST) Received: from relay0133.mxlogin.com (relay0133.mxlogin.com [199.181.239.133]) by dpdk.org (Postfix) with ESMTP id 4EDC21C0B2 for ; Wed, 22 Apr 2020 10:36:48 +0200 (CEST) Received: from filter003.mxroute.com ([168.235.111.26] 168-235-111-26.cloud.ramnode.com) (Authenticated sender: mN4UYu2MZsgR) by relay0133.mxlogin.com (ZoneMTA) with ESMTPSA id 171a10841930000766.001 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Wed, 22 Apr 2020 08:36:45 +0000 X-Zone-Loop: def217a064bb16ceab29f2aa5456d6d1beb013ce349b X-Originating-IP: [168.235.111.26] Received: from galaxy.mxroute.com (unknown [23.92.70.113]) by filter003.mxroute.com (Postfix) with ESMTPS id 06EC260030; Wed, 22 Apr 2020 08:36:44 +0000 (UTC) Received: from [192.198.151.43] by galaxy.mxroute.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.91) (envelope-from ) id 1jRASV-00053F-4W; Wed, 22 Apr 2020 04:10:15 -0400 From: Ray Kinsella To: "Trahe, Fiona" , Akhil Goyal , "Kusztal, ArkadiuszX" , "dev@dpdk.org" Cc: Thomas Monjalon References: <20200417145924.2052-1-arkadiuszx.kusztal@intel.com> <075c0e6b-0588-9e83-efd8-a4d7d937438e@ashroe.eu> 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: Date: Wed, 22 Apr 2020 09:36:40 +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: <075c0e6b-0588-9e83-efd8-a4d7d937438e@ashroe.eu> 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] cryptodev: version cryptodev info get function 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 20/04/2020 18:26, Ray Kinsella wrote: > > > On 20/04/2020 17:54, Trahe, Fiona wrote: >> Hi Ray, Akhil, >> >> >>> On 20/04/2020 15:22, Akhil Goyal wrote: >>>> >>>> >>>>> >>>>> This patch adds versioned function rte_cryptodev_info_get() >>>>> to prevent some issues with ABI policy. >>>>> Node v21 works in same way as before, returning driver capabilities >>>>> directly to the API caller. These capabilities may include new elements >>>>> not part of the v20 ABI. >>>>> Node v20 function maintains compatibility with v20 ABI releases >>>>> by stripping out elements not supported in v20 ABI. Because >>>>> rte_cryptodev_info_get is called by other API functions, >>>>> rte_cryptodev_sym_capability_get function is versioned the same way. >>>>> >>>>> Signed-off-by: Arek Kusztal >>>>> --- >>>>> v2: >>>>> - changed version numbers of symbols to 20.0.2 >>>>> v3: >>>>> - added v2/v3 informations >>>>> - changed version numbers of symbols to 21 >>>>> - fixed checkpatch issues >>>>> >>>>> This patch depends on following patches: >>>>> >>>>> [1] - "[v3] cryptodev: add chacha20-poly1305 aead algorithm" >>>>> (https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatchwor >>>>> k.dpdk.org%2Fpatch%2F64549%2F&data=02%7C01%7Cakhil.goyal%40nxp. >>>>> com%7Ce6789fd42a5946c128e508d7e2dffe2f%7C686ea1d3bc2b4c6fa92cd99c >>>>> 5c301635%7C0%7C0%7C637227323980059545&sdata=50eQJE7WHTME6d >>>>> qA7Nfk%2B50PVAyJrpKlMw%2BoGtA1%2FTc%3D&reserved=0) >>>> >>>> Please include the dependent patches in a single series in your next version. >>>>> >>>>> lib/librte_cryptodev/rte_cryptodev.c | 143 ++++++++++++++++++++++++- >>>>> lib/librte_cryptodev/rte_cryptodev.h | 39 ++++++- >>>>> lib/librte_cryptodev/rte_cryptodev_version.map | 7 ++ >>>>> 3 files changed, 186 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/lib/librte_cryptodev/rte_cryptodev.c >>>>> b/lib/librte_cryptodev/rte_cryptodev.c >>>>> index 6d1d0e9..b061447 100644 >>>>> --- a/lib/librte_cryptodev/rte_cryptodev.c >>>>> +++ b/lib/librte_cryptodev/rte_cryptodev.c >>>>> @@ -41,6 +41,9 @@ >>>>> #include "rte_cryptodev.h" >>>>> #include "rte_cryptodev_pmd.h" >>>>> >>>>> +#include >>>>> +#include >>>>> + >>>>> static uint8_t nb_drivers; >>>>> >>>>> static struct rte_cryptodev rte_crypto_devices[RTE_CRYPTO_MAX_DEVS]; >>>>> @@ -56,6 +59,14 @@ static struct rte_cryptodev_global cryptodev_globals = { >>>>> /* spinlock for crypto device callbacks */ >>>>> static rte_spinlock_t rte_cryptodev_cb_lock = RTE_SPINLOCK_INITIALIZER; >>>>> >>>>> +static const struct rte_cryptodev_capabilities >>>>> + cryptodev_undefined_capabilities[] = { >>>>> + RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST() >>>>> +}; >>>>> + >>>>> +static struct rte_cryptodev_capabilities >>>>> + *capability_copies[RTE_CRYPTO_MAX_DEVS]; >>>> >>>> Capabilities_copy is a better name as it is copy of many capabilities. >> [Fiona] ok >> >> >>>>> const struct rte_cryptodev_symmetric_capability * >>>>> -rte_cryptodev_sym_capability_get(uint8_t dev_id, >>>>> +rte_cryptodev_sym_capability_get_v20(uint8_t dev_id, >>>> >>>> __vsym Annotation to be used in a declaration of the internal symbol >>>> to signal that it is being used as an implementation of a particular >>>> version of symbol. >> [Fiona] ok >> >> >>>>> + } >>>>> + >>>>> + return NULL; >>>>> + >>>> >>>> Extra line >> [Fiona] ok >> >>>> >>>>> +} >>>>> +VERSION_SYMBOL(rte_cryptodev_sym_capability_get, _v20, 20.0); >>>>> + >>>>> +const struct rte_cryptodev_symmetric_capability * >>>> >>>> __vsym annotation >> [Fiona] ok >> >> >>>> >>>>> +rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, >>>>> const struct rte_cryptodev_sym_capability_idx *idx) >>>>> { >>>>> const struct rte_cryptodev_capabilities *capability; >>>>> @@ -313,6 +360,10 @@ rte_cryptodev_sym_capability_get(uint8_t dev_id, >>>>> return NULL; >>>>> >>>>> } >>>>> +MAP_STATIC_SYMBOL(const struct rte_cryptodev_symmetric_capability * >>>>> + rte_cryptodev_sym_capability_get(uint8_t dev_id, >>>>> + const struct rte_cryptodev_sym_capability_idx *idx), >>>>> + rte_cryptodev_sym_capability_get_v21); >>>>> >>>>> static int >>>>> param_range_check(uint16_t size, const struct rte_crypto_param_range *range) >>>>> @@ -999,6 +1050,13 @@ rte_cryptodev_close(uint8_t dev_id) >>>>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_close, -ENOTSUP); >>>>> retval = (*dev->dev_ops->dev_close)(dev); >>>>> >>>>> + >>>>> + if (capability_copies[dev_id]) { >>>>> + free(capability_copies[dev_id]); >>>>> + capability_copies[dev_id] = NULL; >>>>> + } >>>>> + is_capability_checked[dev_id] = 0; >>>>> + >>>>> if (retval < 0) >>>>> return retval; >>>>> >>>>> @@ -1111,9 +1169,61 @@ rte_cryptodev_stats_reset(uint8_t dev_id) >>>>> (*dev->dev_ops->stats_reset)(dev); >>>>> } >>>>> >>>>> +static void >>>>> +get_v20_capabilities(uint8_t dev_id, struct rte_cryptodev_info *dev_info) >>>>> +{ >>>>> + const struct rte_cryptodev_capabilities *capability; >>>>> + uint8_t found_invalid_capa = 0; >>>>> + uint8_t counter = 0; >>>>> + >>>>> + for (capability = dev_info->capabilities; >>>>> + capability->op != RTE_CRYPTO_OP_TYPE_UNDEFINED; >>>>> + ++capability, ++counter) { >>>>> + if (capability->op == RTE_CRYPTO_OP_TYPE_SYMMETRIC && >>>>> + capability->sym.xform_type == >>>>> + RTE_CRYPTO_SYM_XFORM_AEAD >>>>> + && capability->sym.aead.algo >= >>>>> + RTE_CRYPTO_AEAD_CHACHA20_POLY1305) { >>>>> + found_invalid_capa = 1; >>>>> + counter--; >>>>> + } >>>>> + } >>>>> + is_capability_checked[dev_id] = 1; >>>>> + if (found_invalid_capa) { >>>> >>>> Code becomes unreadable due to indentation which can be avoided. >> [Fiona] ok >> >> >> >>>>> +void >>>>> +rte_cryptodev_info_get_v21(uint8_t dev_id, struct rte_cryptodev_info >>>>> *dev_info); >>>>> +BIND_DEFAULT_SYMBOL(rte_cryptodev_info_get, _v21, 21); >>>> >>>> I am not sure if we need to bind for _v20 also >>>> BIND_DEFAULT_SYMBOL(rte_cryptodev_info_get, _v20, 20); >>> >>> The correct call to VERSION_SYMBOL is already above. >> [Fiona] ok, so won't do this. >> >> >>>> Ray, can you please suggest if it required or not? And what all we need to check? >>> >>> See below. >>> >>>> >>>> The patch is still showing Incompatibilities >>>> NOTICE: ABI may be incompatible, check reports/logs for details. >>>> NOTICE: Incompatible list: librte_cryptodev.so >>> >>> So I looked through the issues it is complaining about, these are here. >>> https://travis-ci.com/github/ovsrobot/dpdk/jobs/320526253#L4540 >>> >>> Basically they all are warnings related to the changes to the enumeration >>> rte_crypto_aead_algorithm. >>> >>> Essentially the new member RTE_CRYPTO_AEAD_CHACHA20_POLY1305. >>> The change to the end value RTE_CRYPTO_AEAD_LIST_END. >>> Members of this type "enum rte_crypto_aead_algorithm algo" are demeeded to also have changed, >>> but they haven't. >>> >>> With the additional work to create the v20 version of rte_cryptodev_info_get. >>> I think all reasonable steps have been been taken here. >> [Fiona] Do we need to change the tool or somehow mark as a false positive? > > Yes, I will take a look at it. So take a look at devtools/libabigail.abignore. Suppressions on 'enum rte_crypto_aead_algorithm' & 'rte_crypto_aead_algorithm_strings', _should_ take care of it. Please check with test_build.sh & test_meson_build.sh with DPDK_ABI_REF_VERSION=v19.11 > >> >> >>>>> /** >>>>> * Register a callback function for specific device id. >>>>> diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map >>>>> b/lib/librte_cryptodev/rte_cryptodev_version.map >>>>> index 6e41b4b..512a4a7 100644 >>>>> --- a/lib/librte_cryptodev/rte_cryptodev_version.map >>>>> +++ b/lib/librte_cryptodev/rte_cryptodev_version.map >>>>> @@ -58,6 +58,13 @@ DPDK_20.0 { >>>>> local: *; >>>>> }; >>>>> >>>>> +DPDK_21 { >>> >>> Should be DPDK_21.0 >> [Fiona] Can you explain why? >> I thought it could go back to a 2-number system with _v21 ABI. >> I thought we'd clarified that DPDK_20.0 is only there due to a mistake, that should have been DPDK_20. >> >> >>>>> + global: >>>>> + rte_cryptodev_info_get; >>>>> + rte_cryptodev_sym_capability_get; >>>>> +} DPDK_20.0; >>>>> + >>>>> + >>>>> EXPERIMENTAL { >>>>> global: >>>>> >>>>> -- >>>>> 2.1.0 >>>>