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 AFE82A0C47; Tue, 12 Oct 2021 17:06:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 44EA34111D; Tue, 12 Oct 2021 17:06:49 +0200 (CEST) Received: from new1-smtp.messagingengine.com (new1-smtp.messagingengine.com [66.111.4.221]) by mails.dpdk.org (Postfix) with ESMTP id BF948410FC for ; Tue, 12 Oct 2021 17:06:47 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailnew.nyi.internal (Postfix) with ESMTP id 3C4EF580B77; Tue, 12 Oct 2021 11:06:47 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Tue, 12 Oct 2021 11:06:47 -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= aRJhM6YlRqjIq7QGJqiDqAvt6XBnf00fClu+FtO44Lc=; b=OpoXvsK7kHPfwwN5 E1657RGDu6crAP/RGF0bvTA0mbAUwuf1+21FGYiOPsP/Us1fttZx9UD2EUiyL0i+ t1kvRHGQxoqKp3w4U3KjZtds5yeCn/vQljP3Wo/YTFJHk9F33nERjt2r6LzWUpBd 9aUMHsKkJndWGvGKFbmnZbsX/O2qDNl5kwP0K1zSk0MRGkNPVoMsa9YL6dWm0X8g apH3Ln/63cENUhABs4WQaTdTaJC1Kx/F7jXiZDxDB5rHz9iXf55MiegtCHF+O//q N/SIHvf8nBnDXNRDnP3oewK96z0nCYwHuZ5Ipx85AHhG9kGerWgIhsxZ1SIwAMH1 xFWZGg== 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=aRJhM6YlRqjIq7QGJqiDqAvt6XBnf00fClu+FtO44 Lc=; b=kKhc0IaPV5iKpTfMle1t0WV00tK48nZgOXV2mYdFFZTRgpaSeQSPH4Y8y ytmkS8ZONLE8KLm66yCZlg312PRl8pYmRM55o8yUzCGIJOaiCnAYRQpaaEJIsQZ8 UM4B2Dtir/We6GzCm1tBqwnYi4gfaZrv4iScYqN4yjoupucMan1mSESYa3dcVvlc N5bvq9YfLwP93ZQx8EgZDgcFED+PKc1+awXd7GTId33N6Iw8I+9HKsTOC244Y17C 0n2q5nnPpLedapc+YDtvWrY2KC+fulI9MdzBNSg1UHUQbMfUMRZS+wt1gZR21ZtN WZg8bqp3v5yQ39yM1Q2UvvdgjSFaQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrvddtkedgkedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtqhertddttdejnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepkeethedtieevhfeigeejleegudefjeehkeekteeuveeiuedvveeu tdejveehveetnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 12 Oct 2021 11:06:41 -0400 (EDT) From: Thomas Monjalon To: Anoob Joseph , Akhil Goyal , "dev@dpdk.org" , "Kinsella, Ray" 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" , "bruce.richardson@intel.com" Date: Tue, 12 Oct 2021 17:06:39 +0200 Message-ID: <1716871.KAiom3yBSL@thomas> In-Reply-To: <805733d0-38d7-3e51-e317-b3e4ce7ed165@ashroe.eu> References: <20210731181327.660296-1-gakhil@marvell.com> <805733d0-38d7-3e51-e317-b3e4ce7ed165@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 16:47, Kinsella, Ray: > On 12/10/2021 15:18, Anoob Joseph wrote: > > From: Thomas Monjalon > >> 12/10/2021 15:38, Anoob Joseph: > >>> From: Thomas Monjalon > >>>> 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 > >>>> algos? > >>>>>> If yes, then I would suggest we address it now. > >>>>>> > >>>>>> Not at all - but it can be problematic, if two versions of DPDK > >>>>>> disagree with the value of LIST_END. > >>>>>> > >>>>>>> Looking at the "problematic changes", we only have 2-3 > >>>>>>> application & PMD changes. For unit test application, we could > >>>>>>> may be do something like, > >>>>>> > >>>>>> 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. > >>>>> > >>>>> [Anoob] Our problem is application using LIST_END (which comes > >>>>> from library) > >>>> 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 application) to determine iterations of loop. This would solve = the > >> problem, right? > >>>> > >>>> The problem is not the application. > >>>> Are you asking the app to define DPDK types? > >>> > >>> [Anoob] I didn't understand how you concluded that. > >> > >> Because you define a specific array in the test app. > >> > >>> The app is supposed to test "n" asymmetric features supported by DPDK. > >> Currently, it does that by looping from 0 to LIST_END which happens to= give you > >> the first n features. Now, if we add any new asymmetric feature, LIST_= END > >> value would change. Isn't that the very reason why we removed LIST_END= from > >> symmetric library and applications? > >> > >> Yes > >> > >>> Now coming to what I proposed, the app is supposed to test "n" asymme= tric > >> features. LIST_END helps in doing the loops. If we remove LIST_END, th= en > >> application will not be in a position to do a loop. My suggestion is, = we list the > >> types that are supposed to be tested by the app, and let that array be= used as > >> feature list. > >>> > >>> PS: Just to reiterate, my proposal is just a local array which would = hold DPDK > >> defined RTE enum values for the features that would be tested by this > >> app/function. > >> > >> I am more concerned by the general case than the test app. > >> I think a function returning a number is more app-friendly. > >=20 > > [Anoob] Indeed. But there are 3 LIST_ENDs removed with this patch. Do y= ou propose 3 new APIs to just get max number?=20 >=20 > 1 API returning a single "info" structure perhaps - as being the most ext= ensible? Or 3 iterators (foreach construct). Instead of just returning a size, we can have an iterator for each enum which needs to be iterated. =46eel free to consider the alternative which fits the best in cryptodev.