DPDK patches and discussions
 help / color / mirror / Atom feed
* [RFC PATCH] cryptodev: add return parameter to callback process API
@ 2022-06-24 12:12 Srujana Challa
  2022-07-17 10:46 ` Thomas Monjalon
  0 siblings, 1 reply; 4+ messages in thread
From: Srujana Challa @ 2022-06-24 12:12 UTC (permalink / raw)
  To: gakhil, roy.fan.zhang; +Cc: dev, jerinj, ndabilpuram, anoobj

Adds a return parameter "uint16_t qp_id" to the functions
rte_cryptodev_pmd_callback_process and rte_cryptodev_cb_fn.
The new parameter is used to return queue pair ID to
the application when it gets error interrupt, so that
application can disable and enable the queue pair, to bring
the queue back to normal state.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 lib/cryptodev/cryptodev_pmd.h | 3 ++-
 lib/cryptodev/rte_cryptodev.c | 8 ++++----
 lib/cryptodev/rte_cryptodev.h | 9 ++++++---
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
index 3dcc3cb7ed..f087b58161 100644
--- a/lib/cryptodev/cryptodev_pmd.h
+++ b/lib/cryptodev/cryptodev_pmd.h
@@ -560,6 +560,7 @@ rte_cryptodev_pmd_destroy(struct rte_cryptodev *cryptodev);
  * device.
  *  *
  * @param	dev	Pointer to cryptodev struct
+ * @param	qp_id	To pass qp_id, which got interrupt, to user application.
  * @param	event	Crypto device interrupt event type.
  *
  * @return
@@ -567,7 +568,7 @@ rte_cryptodev_pmd_destroy(struct rte_cryptodev *cryptodev);
  */
 __rte_internal
 void rte_cryptodev_pmd_callback_process(struct rte_cryptodev *dev,
-				enum rte_cryptodev_event_type event);
+			uint16_t qp_id, enum rte_cryptodev_event_type event);
 
 /**
  * @internal
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index 42f3221052..eee208ce2d 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -1689,8 +1689,8 @@ rte_cryptodev_callback_unregister(uint8_t dev_id,
 }
 
 void
-rte_cryptodev_pmd_callback_process(struct rte_cryptodev *dev,
-	enum rte_cryptodev_event_type event)
+rte_cryptodev_pmd_callback_process(struct rte_cryptodev *dev, uint16_t qp_id,
+				   enum rte_cryptodev_event_type event)
 {
 	struct rte_cryptodev_callback *cb_lst;
 	struct rte_cryptodev_callback dev_cb;
@@ -1702,8 +1702,8 @@ rte_cryptodev_pmd_callback_process(struct rte_cryptodev *dev,
 		dev_cb = *cb_lst;
 		cb_lst->active = 1;
 		rte_spinlock_unlock(&rte_cryptodev_cb_lock);
-		dev_cb.cb_fn(dev->data->dev_id, dev_cb.event,
-						dev_cb.cb_arg);
+		dev_cb.cb_fn(dev->data->dev_id, qp_id, dev_cb.event,
+			     dev_cb.cb_arg);
 		rte_spinlock_lock(&rte_cryptodev_cb_lock);
 		cb_lst->active = 0;
 	}
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index 56f459c6a0..6b41262735 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -577,13 +577,16 @@ typedef uint16_t (*rte_cryptodev_callback_fn)(uint16_t dev_id, uint16_t qp_id,
  * software for notification of device events
  *
  * @param	dev_id	Crypto device identifier
+ * @param	qp_id	Return parameter from driver to the application. Driver
+ *			returns queue pair ID when it gets HW error interrupt.
+ *			The application can release and setup the queue
+ *			again, to bring the HW queue back to normal state.
  * @param	event	Crypto device event to register for notification of.
  * @param	cb_arg	User specified parameter to be passed as to passed to
  *			users callback function.
  */
-typedef void (*rte_cryptodev_cb_fn)(uint8_t dev_id,
-		enum rte_cryptodev_event_type event, void *cb_arg);
-
+typedef void (*rte_cryptodev_cb_fn)(uint8_t dev_id, uint16_t qp_id,
+	enum rte_cryptodev_event_type event, void *cb_arg);
 
 /** Crypto Device statistics */
 struct rte_cryptodev_stats {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] cryptodev: add return parameter to callback process API
  2022-06-24 12:12 [RFC PATCH] cryptodev: add return parameter to callback process API Srujana Challa
@ 2022-07-17 10:46 ` Thomas Monjalon
  2022-09-19 12:38   ` [EXT] " Srujana Challa
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Monjalon @ 2022-07-17 10:46 UTC (permalink / raw)
  To: gakhil, Srujana Challa
  Cc: roy.fan.zhang, dev, jerinj, ndabilpuram, anoobj, david.marchand,
	bruce.richardson, konstantin.v.ananyev, matan,
	honnappa.nagarahalli

24/06/2022 14:12, Srujana Challa:
> Adds a return parameter "uint16_t qp_id" to the functions
> rte_cryptodev_pmd_callback_process and rte_cryptodev_cb_fn.
> The new parameter is used to return queue pair ID to
> the application when it gets error interrupt, so that
> application can disable and enable the queue pair, to bring
> the queue back to normal state.

What about other events?

> + * @param	qp_id	Return parameter from driver to the application. Driver
> + *			returns queue pair ID when it gets HW error interrupt.
> + *			The application can release and setup the queue
> + *			again, to bring the HW queue back to normal state.

What will it mean if the event is not related to queues?

>   * @param	event	Crypto device event to register for notification of.
>   * @param	cb_arg	User specified parameter to be passed as to passed to
>   *			users callback function.

Are you going to add a new callback parameter each time
the application needs info about a new event?

In my opinion, it is a very bad idea.
As done in ethdev, you should add a query function specific to the event.

Example: https://git.dpdk.org/dpdk/commit/?id=bc70e5594838
Here, when a threshold is reached, an event RTE_ETH_EVENT_RX_AVAIL_THRESH
is fired, and the application can get more info about what happened
by calling the function rte_eth_rx_avail_thresh_query().
Look at the parameters description:
"
@param[inout] queue_id
 On input starting Rx queue index to search from.
 If the queue_id is bigger than maximum queue ID of the port,
 search is started from 0. So that application can keep calling
 this function to handle all pending events with a simple increment
 of queue_id on the next call.
 On output if return value is 1, Rx queue index with the event pending.
@param[out] avail_thresh
 Location for available descriptors threshold of the found Rx queue.
"



^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [EXT] Re: [RFC PATCH] cryptodev: add return parameter to callback process API
  2022-07-17 10:46 ` Thomas Monjalon
@ 2022-09-19 12:38   ` Srujana Challa
  2022-09-30 19:17     ` Akhil Goyal
  0 siblings, 1 reply; 4+ messages in thread
From: Srujana Challa @ 2022-09-19 12:38 UTC (permalink / raw)
  To: Thomas Monjalon, Akhil Goyal
  Cc: roy.fan.zhang, dev, Jerin Jacob Kollanukkaran,
	Nithin Kumar Dabilpuram, Anoob Joseph, david.marchand,
	bruce.richardson, konstantin.v.ananyev, matan,
	honnappa.nagarahalli

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Sunday, July 17, 2022 4:17 PM
> To: Akhil Goyal <gakhil@marvell.com>; Srujana Challa
> <schalla@marvell.com>
> Cc: roy.fan.zhang@intel.com; dev@dpdk.org; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> david.marchand@redhat.com; bruce.richardson@intel.com;
> konstantin.v.ananyev@yandex.ru; matan@nvidia.com;
> honnappa.nagarahalli@arm.com
> Subject: [EXT] Re: [RFC PATCH] cryptodev: add return parameter to callback
> process API
> 
> External Email
> 
> ----------------------------------------------------------------------
> 24/06/2022 14:12, Srujana Challa:
> > Adds a return parameter "uint16_t qp_id" to the functions
> > rte_cryptodev_pmd_callback_process and rte_cryptodev_cb_fn.
> > The new parameter is used to return queue pair ID to the application
> > when it gets error interrupt, so that application can disable and
> > enable the queue pair, to bring the queue back to normal state.
> 
> What about other events?
> 
> > + * @param	qp_id	Return parameter from driver to the application.
> Driver
> > + *			returns queue pair ID when it gets HW error
> interrupt.
> > + *			The application can release and setup the queue
> > + *			again, to bring the HW queue back to normal state.
> 
> What will it mean if the event is not related to queues?
> 
> >   * @param	event	Crypto device event to register for notification of.
> >   * @param	cb_arg	User specified parameter to be passed as to passed
> to
> >   *			users callback function.
> 
> Are you going to add a new callback parameter each time the application
> needs info about a new event?
> 
> In my opinion, it is a very bad idea.
> As done in ethdev, you should add a query function specific to the event.
> 
> Example: https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__git.dpdk.org_dpdk_commit_-3Fid-
> 3Dbc70e5594838&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=Fj4OoD5hcK
> FpANhTWdwQzjT1Jpf7veC5263T47JVpnc&m=SUn-
> UVCQXX4KwyuDDcIb_PvE4MwkLTimQ3ox7hHcKW7wCq6BzW2849tn1nq2dO
> S1&s=aSpFxjeE4xjxZadI9wxc5AqInIvinSvFfa0NEeRrxBA&e=
> Here, when a threshold is reached, an event
> RTE_ETH_EVENT_RX_AVAIL_THRESH is fired, and the application can get
> more info about what happened by calling the function
> rte_eth_rx_avail_thresh_query().
> Look at the parameters description:
> "
> @param[inout] queue_id
>  On input starting Rx queue index to search from.
>  If the queue_id is bigger than maximum queue ID of the port,  search is
> started from 0. So that application can keep calling  this function to handle all
> pending events with a simple increment  of queue_id on the next call.
>  On output if return value is 1, Rx queue index with the event pending.
> @param[out] avail_thresh
>  Location for available descriptors threshold of the found Rx queue.
> "
>
Agree with your comment. Will work on to implement query API for events. But as of now we only have single error event.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [EXT] Re: [RFC PATCH] cryptodev: add return parameter to callback process API
  2022-09-19 12:38   ` [EXT] " Srujana Challa
@ 2022-09-30 19:17     ` Akhil Goyal
  0 siblings, 0 replies; 4+ messages in thread
From: Akhil Goyal @ 2022-09-30 19:17 UTC (permalink / raw)
  To: Srujana Challa
  Cc: roy.fan.zhang, dev, Jerin Jacob Kollanukkaran,
	Nithin Kumar Dabilpuram, Anoob Joseph, david.marchand,
	bruce.richardson, konstantin.v.ananyev, Thomas Monjalon, matan,
	honnappa.nagarahalli

> > 24/06/2022 14:12, Srujana Challa:
> > > Adds a return parameter "uint16_t qp_id" to the functions
> > > rte_cryptodev_pmd_callback_process and rte_cryptodev_cb_fn.
> > > The new parameter is used to return queue pair ID to the application
> > > when it gets error interrupt, so that application can disable and
> > > enable the queue pair, to bring the queue back to normal state.
> >
> > What about other events?
> >
> > > + * @param	qp_id	Return parameter from driver to the application.
> > Driver
> > > + *			returns queue pair ID when it gets HW error
> > interrupt.
> > > + *			The application can release and setup the queue
> > > + *			again, to bring the HW queue back to normal state.
> >
> > What will it mean if the event is not related to queues?
> >
> > >   * @param	event	Crypto device event to register for notification of.
> > >   * @param	cb_arg	User specified parameter to be passed as to passed
> > to
> > >   *			users callback function.
> >
> > Are you going to add a new callback parameter each time the application
> > needs info about a new event?
> >
> > In my opinion, it is a very bad idea.
> > As done in ethdev, you should add a query function specific to the event.
> >
> > Example: https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__git.dpdk.org_dpdk_commit_-3Fid-
> > 3Dbc70e5594838&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=Fj4OoD5hcK
> > FpANhTWdwQzjT1Jpf7veC5263T47JVpnc&m=SUn-
> > UVCQXX4KwyuDDcIb_PvE4MwkLTimQ3ox7hHcKW7wCq6BzW2849tn1nq2dO
> > S1&s=aSpFxjeE4xjxZadI9wxc5AqInIvinSvFfa0NEeRrxBA&e=
> > Here, when a threshold is reached, an event
> > RTE_ETH_EVENT_RX_AVAIL_THRESH is fired, and the application can get
> > more info about what happened by calling the function
> > rte_eth_rx_avail_thresh_query().
> > Look at the parameters description:
> > "
> > @param[inout] queue_id
> >  On input starting Rx queue index to search from.
> >  If the queue_id is bigger than maximum queue ID of the port,  search is
> > started from 0. So that application can keep calling  this function to handle all
> > pending events with a simple increment  of queue_id on the next call.
> >  On output if return value is 1, Rx queue index with the event pending.
> > @param[out] avail_thresh
> >  Location for available descriptors threshold of the found Rx queue.
> > "
> >
> Agree with your comment. Will work on to implement query API for events. But
> as of now we only have single error event.

There is a deprecation notice for this patch. Since the patch is dropped can we remove that notice?

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-09-30 19:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-24 12:12 [RFC PATCH] cryptodev: add return parameter to callback process API Srujana Challa
2022-07-17 10:46 ` Thomas Monjalon
2022-09-19 12:38   ` [EXT] " Srujana Challa
2022-09-30 19:17     ` Akhil Goyal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).