Hi Shally/Ahkil, > -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Verma, Shally > Sent: Tuesday, January 30, 2018 7:56 AM > To: Akhil Goyal <akhil.go...@nxp.com>; De Lara Guarch, Pablo > <pablo.de.lara.gua...@intel.com>; Trahe, Fiona <fiona.tr...@intel.com>; > hemant.agra...@nxp.com; Doherty, Declan <declan.dohe...@intel.com>; > Griffin, John <john.grif...@intel.com>; Jain, Deepak K > <deepak.k.j...@intel.com>; j...@semihalf.com; t...@semihalf.com; > d...@marvell.com; nsams...@marvell.com; jianbo....@arm.com; Jacob, > Jerin <jerin.jacobkollanukka...@cavium.com>; Athreya, Narayana Prasad > <narayanaprasad.athr...@cavium.com>; Murthy, Nidadavolu > <nidadavolu.mur...@cavium.com> > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] doc: announce ABI change for crypto info > struct > > I do see current cryptodev unit testcase (inside \test dir) uses > info.sym.max_nb_sessions param for session mempool_create. So, such > testcases change are also in proposal?
Yes, for these tests, we can just define a macro in the tests, instead of using the info structure. > > Another point, we recently submitted an RFC patch on lib/cryptodev with > asymmetric crypto support > (https://dpdk.org/dev/patchwork/patch/34308/) which is awaiting review > and these fields have role to play there. > So, could this change be please viewed in conjunction with asym RFC? Do you need it for asymmetric? Anyway, this would remove the symmetric function and structures, not applicable for you. > > Thanks > Shally > > > -----Original Message----- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Akhil Goyal > > Sent: 29 January 2018 14:57 > > To: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com>; Trahe, > > Fiona <fiona.tr...@intel.com>; hemant.agra...@nxp.com; Doherty, > Declan > > <declan.dohe...@intel.com>; Griffin, John <john.grif...@intel.com>; > > Jain, Deepak K <deepak.k.j...@intel.com>; j...@semihalf.com; > > t...@semihalf.com; d...@marvell.com; nsams...@marvell.com; > > jianbo....@arm.com; Jacob, Jerin > <jerin.jacobkollanukka...@cavium.com> > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] doc: announce ABI change for crypto > > info struct > > > > Hi Pablo/Fiona, > > > > On 1/26/2018 4:38 PM, De Lara Guarch, Pablo wrote: > > > > > > > > >> -----Original Message----- > > >> From: Trahe, Fiona > > >> Sent: Friday, January 26, 2018 10:45 AM > > >> To: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com>; > > >> akhil.go...@nxp.com; hemant.agra...@nxp.com; Doherty, Declan > > >> <declan.dohe...@intel.com>; jerin.ja...@intel.com; Griffin, John > > >> <john.grif...@intel.com>; Jain, Deepak K <deepak.k.j...@intel.com>; > > >> j...@semihalf.com; t...@semihalf.com; d...@marvell.com; > > >> nsams...@marvell.com; jianbo....@arm.com > > >> Cc: dev@dpdk.org; Trahe, Fiona <fiona.tr...@intel.com> > > >> Subject: RE: [PATCH] doc: announce ABI change for crypto info > > >> struct > > >> > > >> Hi Pablo, > > >> > > >>> -----Original Message----- > > >>> From: De Lara Guarch, Pablo > > >>> Sent: Friday, January 26, 2018 9:04 AM > > >>> To: akhil.go...@nxp.com; hemant.agra...@nxp.com; Doherty, > Declan > > >>> <declan.dohe...@intel.com>; jerin.ja...@intel.com; Trahe, Fiona > > >>> <fiona.tr...@intel.com>; Griffin, John <john.grif...@intel.com>; > > >>> Jain, Deepak K <deepak.k.j...@intel.com>; j...@semihalf.com; > > >>> t...@semihalf.com; d...@marvell.com; nsams...@marvell.com; > > >>> jianbo....@arm.com > > >>> Cc: dev@dpdk.org; De Lara Guarch, Pablo > > >>> <pablo.de.lara.gua...@intel.com> > > >>> Subject: [PATCH] doc: announce ABI change for crypto info struct > > >>> > > >>> Since the API changes made in 17.08, the session mempool is not > > >>> created anymore in each crypto device. > > >>> Therefore, there is no need to have, in the cryptodev info > > >>> structure, the maximum number of sessions supported per device > and > > >>> per queue pair. > > >>> > > >>> Signed-off-by: Pablo de Lara <pablo.de.lara.gua...@intel.com> > > >>> --- > > >>> doc/guides/rel_notes/deprecation.rst | 5 +++++ > > >>> 1 file changed, 5 insertions(+) > > >>> > > >>> diff --git a/doc/guides/rel_notes/deprecation.rst > > >>> b/doc/guides/rel_notes/deprecation.rst > > >>> index d59ad5988..5588ba7c1 100644 > > >>> --- a/doc/guides/rel_notes/deprecation.rst > > >>> +++ b/doc/guides/rel_notes/deprecation.rst > > >>> @@ -59,3 +59,8 @@ Deprecation Notices > > >>> be added between the producer and consumer structures. The > > >>> size of > > >> the > > >>> structure and the offset of the fields will remain the same on > > >>> platforms with 64B cache line, but will change on other platforms. > > >>> + > > >>> +* cryptodev: The structure ``sym``, including its fields > > >>> +``max_nb_sessions`` > > >>> + and ``max_nb_sessions_per_qp``, in structure > > >>> +``rte_cryptodev_info``, > > >>> + will be removed in 18.05, as these fields are not relevant > > >>> +anymore > > >>> + since the session mempool is not internal in the crypto device > > >> anymore. > > >>> -- > > >> [Fiona] max_nb_sessions must be also removed from struct > > >> rte_cryptodev_pmd_init_params > > > > > > Good point. Since this structure is internal, I guess we don't need > > > a > > deprecation notice for it, > > > but I will remove it in the patch for 18.05. > > > > > >> Regards deprecation of max_nb_sessions from both structs: > > >> Acked-by: Fiona Trahe <fiona.tr...@intel.com> > > >> > > >> If removing the max_nb_sessions_per_qp, then the following > > >> functions should also be deprecated. > > >> rte_cryptodev_queue_pair_attach_sym_session > > >> rte_cryptodev_queue_pair_detach_sym_session > > >> These and the max_nb_session_per_qp were added here at request of > > NXP: > > >> http://dpdk.org/ml/archives/dev/2017-March/060740.html > > > > > > Akhil, do you agree on this change? > > > > > > > We recently did some changes in the driver to take care of the > > dependency for limit on max_nb_sessions_per_qp, but it is not removed > > completely. We will need to look into this. But sending the > > deprecation notice at this moment is fine. If something comes up, will > > let you know later. Looks good to me. Could you ack this if you agree with it? Thanks, Pablo > > > > -Akhil
