Hi Shally/Ahkil,

> -----Original Message-----
> From: dev [mailto:[email protected]] On Behalf Of Verma, Shally
> Sent: Tuesday, January 30, 2018 7:56 AM
> To: Akhil Goyal <[email protected]>; De Lara Guarch, Pablo
> <[email protected]>; Trahe, Fiona <[email protected]>;
> [email protected]; Doherty, Declan <[email protected]>;
> Griffin, John <[email protected]>; Jain, Deepak K
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Jacob,
> Jerin <[email protected]>; Athreya, Narayana Prasad
> <[email protected]>; Murthy, Nidadavolu
> <[email protected]>
> Cc: [email protected]
> 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:[email protected]] On Behalf Of Akhil Goyal
> > Sent: 29 January 2018 14:57
> > To: De Lara Guarch, Pablo <[email protected]>; Trahe,
> > Fiona <[email protected]>; [email protected]; Doherty,
> Declan
> > <[email protected]>; Griffin, John <[email protected]>;
> > Jain, Deepak K <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; Jacob, Jerin
> <[email protected]>
> > Cc: [email protected]
> > 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 <[email protected]>;
> > >> [email protected]; [email protected]; Doherty, Declan
> > >> <[email protected]>; [email protected]; Griffin, John
> > >> <[email protected]>; Jain, Deepak K <[email protected]>;
> > >> [email protected]; [email protected]; [email protected];
> > >> [email protected]; [email protected]
> > >> Cc: [email protected]; Trahe, Fiona <[email protected]>
> > >> 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: [email protected]; [email protected]; Doherty,
> Declan
> > >>> <[email protected]>; [email protected]; Trahe, Fiona
> > >>> <[email protected]>; Griffin, John <[email protected]>;
> > >>> Jain, Deepak K <[email protected]>; [email protected];
> > >>> [email protected]; [email protected]; [email protected];
> > >>> [email protected]
> > >>> Cc: [email protected]; De Lara Guarch, Pablo
> > >>> <[email protected]>
> > >>> 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 <[email protected]>
> > >>> ---
> > >>>   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 <[email protected]>
> > >>
> > >> 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

Reply via email to