[v4,3/3] doc: add enqueue callback APIs

Message ID 1603619090-118652-4-git-send-email-abhinandan.gujjar@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series support enqueue callbacks on cryptodev |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Gujjar, Abhinandan S Oct. 25, 2020, 9:44 a.m. UTC
  Add enqueue callback support for cryptodev library

Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
---
 doc/guides/prog_guide/cryptodev_lib.rst | 22 ++++++++++++++++++++++
 doc/guides/rel_notes/release_20_11.rst  |  5 +++++
 2 files changed, 27 insertions(+)
  

Comments

Akhil Goyal Oct. 26, 2020, 7:08 p.m. UTC | #1
> 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.

> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@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?

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.

> +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?

> +
> +.. 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.
> 
>  Removed Items
>  -------------
> --
> 1.9.1
  
Gujjar, Abhinandan S Oct. 27, 2020, 3:52 a.m. UTC | #2
Hi Akhil,

Please find few comments inline.

Thanks
Abhinandan

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Tuesday, October 27, 2020 12:39 AM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Doherty, Declan <declan.doherty@intel.com>;
> Honnappa.Nagarahalli@arm.com; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: Vangati, Narender <narender.vangati@intel.com>; jerinj@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.gujjar@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
  
Ananyev, Konstantin Oct. 27, 2020, 12:51 p.m. UTC | #3
> 
> Add enqueue callback support for cryptodev library
> 
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@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().
> +Multiple callback functions can be added for a given queue pair.
> +
> +.. 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);


Please add explicit statement that all installed callbacks wouldn't suruvive dev_configure().
So it is user responsibility to: remove them before dev_configure() and (re)install after.
With that in place:
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 
>  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.
> 
>  Removed Items
>  -------------
> --
> 1.9.1
  
Gujjar, Abhinandan S Oct. 27, 2020, 5:17 p.m. UTC | #4
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Tuesday, October 27, 2020 6:22 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Doherty, Declan <declan.doherty@intel.com>; akhil.goyal@nxp.com;
> Honnappa.Nagarahalli@arm.com
> Cc: Vangati, Narender <narender.vangati@intel.com>; jerinj@marvell.com
> Subject: RE: [v4 3/3] doc: add enqueue callback APIs
> 
> 
> 
> >
> > Add enqueue callback support for cryptodev library
> >
> > Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@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().
> > +Multiple callback functions can be added for a given queue pair.
> > +
> > +.. 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);
> 
> 
> Please add explicit statement that all installed callbacks wouldn't suruvive
> dev_configure().
> So it is user responsibility to: remove them before dev_configure() and
> (re)install after.
> With that in place:
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Ok. I will update the documentation and send a next version of patch.
> 
> >
> >  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.
> >
> >  Removed Items
> >  -------------
> > --
> > 1.9.1
  

Patch

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().
+Multiple callback functions can be added for a given queue pair.
+
+.. 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.
 
 Removed Items
 -------------