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 D0226A0C47; Tue, 12 Oct 2021 13:52:40 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5905841141; Tue, 12 Oct 2021 13:52:40 +0200 (CEST) Received: from new3-smtp.messagingengine.com (new3-smtp.messagingengine.com [66.111.4.229]) by mails.dpdk.org (Postfix) with ESMTP id 85D0340E0F for ; Tue, 12 Oct 2021 13:52:38 +0200 (CEST) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.nyi.internal (Postfix) with ESMTP id DEECC580423; Tue, 12 Oct 2021 07:52:36 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Tue, 12 Oct 2021 07:52:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm2; bh= jjCftm1VpjixHiazP1SCj0YlNescbGezctTRDONLxy0=; b=r+awTEOmiDxT9lC8 8zI9/R8Dv9zS2+/tPooRmQzNnhDd2lImhO2GMtHVI2inKwPNl03yS3awWAw2MPA2 HikmK/LywnMs7QuR0XODr/U3/kgoCfffXMWZECLgv+bPQWRXECAQdx01Kkn2+7w7 v2OcKNSOqQP+4z9D59WnSLEMRBqOPpqW5RZxCcI203EC6KTShrn+Jf8SOYix4awu mRKDNdzv/11SDaagFkhGMVKYRWBqHMg5BziJ7niRBWpjwGentbO9ckVBzpJTr361 lplJlklMnjz/UMZxG/sTjvhfN5FrkOGxbE0aXlCpQSzmfNVt+yf0ebVQ1PnArGT0 oO3kVQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=jjCftm1VpjixHiazP1SCj0YlNescbGezctTRDONLx y0=; b=kb7KT3XWJjh2ntBEyZkDj2HKcKaGhh32YKa5rVirtnUi1tjSygt+J14TF Joj2FMizDqIEOT+wRq3RWGvdjp/CggnHFBrVjRwe+0QbYN1n7uzj/zlLGA9kheyv QZ5C9iGPAbFZeOYNw2sPr5O6d/uKXJ2zPAVeS2p6iD4LePTOBtaoP4PIlvQSyAAh 7SOv0JetT+6iK3LkbS2QJUcIqZEJXHzpOGlXEWEHz7Bw4Ueh4M1mp3pRFxZhFjow 6t4cRMxnSfQhHfs+CKABXI4GfGzxFjG/z7A9N3bO285StFyGwH+0M9pEQ4vOAq5l zqUG9no+PFrM/2u1x6rM8oAn6McOA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrvddtkedggedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtqhertddttdejnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepkeethedtieevhfeigeejleegudefjeehkeekteeuveeiuedvveeu tdejveehveetnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 12 Oct 2021 07:52:32 -0400 (EDT) From: Thomas Monjalon To: "Kinsella, Ray" , Akhil Goyal , "dev@dpdk.org" , Anoob Joseph Cc: "david.marchand@redhat.com" , "hemant.agrawal@nxp.com" , "pablo.de.lara.guarch@intel.com" , "fiona.trahe@intel.com" , "declan.doherty@intel.com" , "matan@nvidia.com" , "g.singh@nxp.com" , "roy.fan.zhang@intel.com" , "jianjay.zhou@huawei.com" , "asomalap@amd.com" , "ruifeng.wang@arm.com" , "konstantin.ananyev@intel.com" , "radu.nicolau@intel.com" , "ajit.khaparde@broadcom.com" , Nagadheeraj Rottela , Ankur Dwivedi , "ciara.power@intel.com" , Stephen Hemminger , "Yigit, Ferruh" Date: Tue, 12 Oct 2021 13:52:28 +0200 Message-ID: <2844039.NtWzsPphL5@thomas> In-Reply-To: References: <20210731181327.660296-1-gakhil@marvell.com> <84e597ee-ab57-1a2c-889c-68d04e58a12d@ashroe.eu> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v2 1/3] cryptodev: remove LIST_END enumerators 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 Sender: "dev" 12/10/2021 13:34, Anoob Joseph: > From: Kinsella, Ray > > On 12/10/2021 11:50, Anoob Joseph wrote: > > > From: Akhil Goyal > > >>> On 08/10/2021 21:45, Akhil Goyal wrote: > > >>>> Remove *_LIST_END enumerators from asymmetric crypto lib to avoid > > >>>> ABI breakage for every new addition in enums. > > >>>> > > >>>> Signed-off-by: Akhil Goyal > > >>>> --- > > >>>> - } else if (xform->xform_type >=3D > > >>> RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END > > >>>> + } else if (xform->xform_type > RTE_CRYPTO_ASYM_XFORM_ECPM [...] > > >>> > > >>> So I am not sure that this is an improvement. Indeed, it is not an improvement. > > >>> The cryptodev issue we had, was that _LIST_END was being used to > > >>> size arrays. > > >>> And that broke when new algorithms got added. Is that an issue, in = this > > case? > > >> > > >> Yes we did this same exercise for symmetric crypto enums earlier. > > >> Asym enums were left as it was experimental at that point. > > >> They are still experimental, but thought of making this uniform > > >> throughout DPDK enums. > > >> > > >>> > > >>> I am not sure that swapping out _LIST_END, and then littering the > > >>> code with RTE_CRYPTO_ASYM_XFORM_ECPM and > > >>> RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE, is an improvement > > >> here. > > >>> > > >>> My 2c is that from an ABI PoV RTE_CRYPTO_ASYM_OP_LIST_END is not > > >>> better or worse, than > > RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE? > > >>> > > >>> Interested to hear other thoughts. > > >> > > >> I don=E2=80=99t have any better solution for avoiding ABI issues for= now. > > >> The change is for avoiding ABI breakage. But we can drop this patch > > >> For now as asym is still experimental. > > > > > > [Anoob] Having LIST_END would preclude new additions to asymmetric al= gos? > > If yes, then I would suggest we address it now. > >=20 > > Not at all - but it can be problematic, if two versions of DPDK disagre= e with the > > value of LIST_END. > >=20 > > > Looking at the "problematic changes", we only have 2-3 application & > > > PMD changes. For unit test application, we could may be do something > > > like, > >=20 > > The essental functionality not that different, I am just not sure that = the verbosity > > below is helping. > > What you are really trying to guard against is people using LIST_END to= size > > arrays. >=20 > [Anoob] Our problem is application using LIST_END (which comes from libra= ry) to determine the number of iterations for the loop. My suggestion is to= modify the UT such that, we could use RTE_DIM(types) (which comes from app= lication) to determine iterations of loop. This would solve the problem, ri= ght? The problem is not the application. Are you asking the app to define DPDK types? The problem is in DPDK API. We must not suggest a size for enums. If we really need a size, then it must be explicit and updated in the lib b= inary (through a function) when the size increases. > > > - for (i =3D 0; i < RTE_CRYPTO_ASYM_OP_LIST_END; i++) { > > > + enum rte_crypto_asym_op_type types[] =3D { > > > + RTE_CRYPTO_ASYM_OP_ENCRYPT, > > > + RTE_CRYPTO_ASYM_OP_DECRYPT, > > > + RTE_CRYPTO_ASYM_OP_SIGN, > > > + RTE_CRYPTO_ASYM_OP_VERIFY, > > > + RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERA= TE, > > > + RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERAT= E, > > > + RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMP= UTE, > > > + }; > > > + for (i =3D 0; i <=3D RTE_DIM(types); i++) { > > > if (tc.modex.xform_type =3D=3D RTE_CRYPTO_ASY= M_XFORM_RSA) { > > > - if (tc.rsa_data.op_type_flags & (1 <<= i)) { > > > + if (tc.rsa_data.op_type_flags & (1 << > > > + types[i])) { > > > if (tc.rsa_data.key_exp) { > > > status =3D test_crypt= odev_asym_op( > > > &testsuite_pa= rams, &tc, > > > - test_msg, ses= sionless, i, > > > + test_msg, > > > + sessionless, types[i], > > > RTE_RSA_KEY_T= YPE_EXP); > > > } > > > if (status) > > > break; > > > - if (tc.rsa_data.key_qt && (i = =3D=3D > > > + if (tc.rsa_data.key_qt && > > > + (types[i] =3D=3D > > > RTE_CRYPTO_AS= YM_OP_DECRYPT || > > > - i =3D=3D RTE_= CRYPTO_ASYM_OP_SIGN)) { > > > + types[i] =3D= =3D > > > + RTE_CRYPTO_ASYM_OP_SIGN)) { > > > status =3D test_crypt= odev_asym_op( > > > &testsuite_pa= rams, > > > - &tc, test_msg= , sessionless, i, > > > + &tc, test_msg, > > > + sessionless, types[i], > > > RTE_RSA_KET_T= YPE_QT); > > > } > > > if (status) > > > > > > This way, application would only use the ones which it is designed to= work > > with. For QAT driver changes, we could have an overload if condition (i= f alg =3D=3D x > > || alg =3D y || ...) to get the same effect.