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

Reply via email to