Hi Akhil, Please find few comments inline.
Thanks Abhinandan > -----Original Message----- > From: Akhil Goyal <akhil.go...@nxp.com> > Sent: Tuesday, October 27, 2020 12:39 AM > To: Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; dev@dpdk.org; > Doherty, Declan <declan.dohe...@intel.com>; > honnappa.nagaraha...@arm.com; Ananyev, Konstantin > <konstantin.anan...@intel.com> > Cc: Vangati, Narender <narender.vang...@intel.com>; jer...@marvell.com > Subject: RE: [v4 3/3] doc: add enqueue callback APIs > > > Subject: [v4 3/3] doc: add enqueue callback APIs > > > > Add enqueue callback support for cryptodev library > > > No need for separate documentation patch. It should be part of patch which is > adding The functionality. Ok > > > Signed-off-by: Abhinandan Gujjar <abhinandan.guj...@intel.com> > > --- > > doc/guides/prog_guide/cryptodev_lib.rst | 22 ++++++++++++++++++++++ > > doc/guides/rel_notes/release_20_11.rst | 5 +++++ > > 2 files changed, 27 insertions(+) > > > > diff --git a/doc/guides/prog_guide/cryptodev_lib.rst > > b/doc/guides/prog_guide/cryptodev_lib.rst > > index 72129e4..bb3de61 100644 > > --- a/doc/guides/prog_guide/cryptodev_lib.rst > > +++ b/doc/guides/prog_guide/cryptodev_lib.rst > > @@ -366,6 +366,28 @@ can never be larger than ``nb_ops``. > > uint16_t rte_cryptodev_dequeue_burst(uint8_t dev_id, uint16_t qp_id, > > struct rte_crypto_op **ops, > > uint16_t nb_ops) > > > > +User callback APIs > > +~~~~~~~~~~~~~~~~~~ > > +The add API configures a callback function to be called for each > > +burst of crypto ops received on a given crypto device queue pair. The > > +return value is a pointer that can be used later to remove the > > +callback using > > rte_cryptodev_remove_enq_callback(). > > Does that mean callback cannot be called for each packet and it will be called > for Each burst only? Yes. This is similar to ethdev's rxtx callback which is called for a burst of packet, where each packet can be processed one after the other. > > The usage of these APIs is not clear with this documentation. Probably you can > add More text/ pseudo code about the sequence of APIs to be used. One of motivation for the callback is mentioned in the cover letter. Since the usage from eventdev world & the callbacks are generic, I didn't not mention. Please let me know, if you think of any text which adds more clarity to the usage. > > > +Multiple callback functions can be added for a given queue pair. > > Can we add callbacks at runtime or is it before the first enqueue only? Yes. I will add some text for this. > > > + > > +.. code-block:: c > > + > > + struct rte_cryptodev_cb * > > + rte_cryptodev_add_enq_callback(uint8_t dev_id, > > + uint16_t qp_id, > > + rte_cryptodev_callback_fn cb_fn, > > + void *cb_arg); > > + > > +The remove API removes a callback function added by > > rte_cryptodev_add_enq_callback(). > > + > > +.. code-block:: c > > + > > + int rte_cryptodev_remove_enq_callback(uint8_t dev_id, > > + uint16_t qp_id, > > + struct rte_cryptodev_cb *cb); > > > > Operation Representation > > ~~~~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/doc/guides/rel_notes/release_20_11.rst > > b/doc/guides/rel_notes/release_20_11.rst > > index 48717ee..7e2fd30 100644 > > --- a/doc/guides/rel_notes/release_20_11.rst > > +++ b/doc/guides/rel_notes/release_20_11.rst > > @@ -285,6 +285,11 @@ New Features > > * Added scatter gather support. > > * Added NIST GCMVS complaint GMAC test method support. > > > > +* **Added enqueue callback APIs for cryptodev library.** > > + > > + Cryptodev is added with enqueue callback APIs to enable > > + applications to add/remove user callbacks which gets called for > > + every enqueue operations. > > Please follow the sequence mentioned. It should be after the other new > features Of cryptodev. Ok > > > > Removed Items > > ------------- > > -- > > 1.9.1