DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
@ 2020-06-24 14:26 Arek Kusztal
  2020-07-02 17:50 ` Akhil Goyal
  2020-07-08 13:37 ` Thomas Monjalon
  0 siblings, 2 replies; 7+ messages in thread
From: Arek Kusztal @ 2020-06-24 14:26 UTC (permalink / raw)
  To: dev; +Cc: akhil.goyal, fiona.trahe, arkadiuszx.kusztal

From: Fiona Trahe <fiona.trahe@intel.com>

This patch adds function that can check if queue pair
was already setup. This may be useful when dealing with
multi process approach in cryptodev.

Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
v2:
- changed return values
- added function to map file

 lib/librte_cryptodev/rte_cryptodev.c           | 29 ++++++++++++++++++++++++++
 lib/librte_cryptodev/rte_cryptodev.h           | 17 +++++++++++++++
 lib/librte_cryptodev/rte_cryptodev_version.map |  3 +++
 3 files changed, 49 insertions(+)

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index e37b83a..6f556c3 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -1080,6 +1080,35 @@ rte_cryptodev_close(uint8_t dev_id)
 }
 
 int
+rte_cryptodev_get_qp_status(uint8_t dev_id, uint16_t queue_pair_id)
+{
+	struct rte_cryptodev *dev;
+
+	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
+		return -EINVAL;
+	}
+
+	dev = &rte_crypto_devices[dev_id];
+	if (queue_pair_id >= dev->data->nb_queue_pairs) {
+		CDEV_LOG_ERR("Invalid queue_pair_id=%d", queue_pair_id);
+		return -EINVAL;
+	}
+	void **qps = dev->data->queue_pairs;
+
+	if (qps[queue_pair_id])	{
+		CDEV_LOG_DEBUG("qp %d on dev %d is initialised",
+			queue_pair_id, dev_id);
+		return 1;
+	}
+
+	CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+		queue_pair_id, dev_id);
+
+	return 0;
+}
+
+int
 rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
 		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id)
 
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index 4aaee73..7b3ebc2 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -727,6 +727,23 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
 		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
 
 /**
+ * Get the status of queue pairs setup on a specific crypto device
+ *
+ * @param	dev_id		Crypto device identifier.
+ * @param	queue_pair_id	The index of the queue pairs to set up. The
+ *				value must be in the range [0, nb_queue_pair
+ *				- 1] previously supplied to
+ *				rte_cryptodev_configure().
+ * @return
+ *   - 0: qp was not configured
+ *	 - 1: qp was configured
+ *	 - -EINVAL: device was not configured
+ */
+__rte_experimental
+int
+rte_cryptodev_get_qp_status(uint8_t dev_id, uint16_t queue_pair_id);
+
+/**
  * Get the number of queue pairs on a specific crypto device
  *
  * @param	dev_id		Crypto device identifier.
diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
index 07a2d2f..a7a78dc 100644
--- a/lib/librte_cryptodev/rte_cryptodev_version.map
+++ b/lib/librte_cryptodev/rte_cryptodev_version.map
@@ -103,4 +103,7 @@ EXPERIMENTAL {
 	__rte_cryptodev_trace_asym_session_clear;
 	__rte_cryptodev_trace_dequeue_burst;
 	__rte_cryptodev_trace_enqueue_burst;
+
+	# added in 20.08
+	rte_cryptodev_get_qp_status;
 };
-- 
2.1.0


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

* Re: [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
  2020-06-24 14:26 [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup Arek Kusztal
@ 2020-07-02 17:50 ` Akhil Goyal
  2020-07-03 14:08   ` Kusztal, ArkadiuszX
  2020-07-08 13:37 ` Thomas Monjalon
  1 sibling, 1 reply; 7+ messages in thread
From: Akhil Goyal @ 2020-07-02 17:50 UTC (permalink / raw)
  To: Arek Kusztal, dev; +Cc: fiona.trahe

Hi Arek/Fiona,
> 
> From: Fiona Trahe <fiona.trahe@intel.com>
> 
> This patch adds function that can check if queue pair
> was already setup. This may be useful when dealing with
> multi process approach in cryptodev.
> 
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
> v2:
> - changed return values
> - added function to map file
> 
Please mark the previous versions as superseded in the patchwork.
I see that previous version had 2 patches one was acked and so you skipped that in this version.
Or you do not want that patch?
http://patches.dpdk.org/patch/71212/

Those patches could have been sent separately but if you are sending the patches in a series,
Then next version should have all the patches unless you want to drop some of the patches.

Acked-by: Akhil Goyal <akhil.goyal@nxp.com>


Regards,
Akhil

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

* Re: [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
  2020-07-02 17:50 ` Akhil Goyal
@ 2020-07-03 14:08   ` Kusztal, ArkadiuszX
  2020-07-04 20:11     ` Akhil Goyal
  0 siblings, 1 reply; 7+ messages in thread
From: Kusztal, ArkadiuszX @ 2020-07-03 14:08 UTC (permalink / raw)
  To: Akhil Goyal, dev; +Cc: Trahe, Fiona

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Thursday, July 2, 2020 7:51 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [PATCH v2] cryptodev: add function to check if qp was setup
> 
> Hi Arek/Fiona,
> >
> > From: Fiona Trahe <fiona.trahe@intel.com>
> >
> > This patch adds function that can check if queue pair was already
> > setup. This may be useful when dealing with multi process approach in
> > cryptodev.
> >
> > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > ---
> > v2:
> > - changed return values
> > - added function to map file
> >
> Please mark the previous versions as superseded in the patchwork.
> I see that previous version had 2 patches one was acked and so you skipped
> that in this version.
> Or you do not want that patch?
> http://patches.dpdk.org/patch/71212/
[Arek] - sorry for that, both patches should be included, should I send v2 with this one ? Or both with v3?
> 
> Those patches could have been sent separately but if you are sending the
> patches in a series, Then next version should have all the patches unless you
> want to drop some of the patches.
> 
> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
> 
> 
> Regards,
> Akhil

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

* Re: [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
  2020-07-03 14:08   ` Kusztal, ArkadiuszX
@ 2020-07-04 20:11     ` Akhil Goyal
  0 siblings, 0 replies; 7+ messages in thread
From: Akhil Goyal @ 2020-07-04 20:11 UTC (permalink / raw)
  To: Kusztal, ArkadiuszX, dev; +Cc: Trahe, Fiona

> > Hi Arek/Fiona,
> > >
> > > From: Fiona Trahe <fiona.trahe@intel.com>
> > >
> > > This patch adds function that can check if queue pair was already
> > > setup. This may be useful when dealing with multi process approach in
> > > cryptodev.
> > >
> > > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > > ---
> > > v2:
> > > - changed return values
> > > - added function to map file
> > >
> > Please mark the previous versions as superseded in the patchwork.
> > I see that previous version had 2 patches one was acked and so you skipped
> > that in this version.
> > Or you do not want that patch?
> >
> http://patches.dpdk.org/patch/71212/
> [Arek] - sorry for that, both patches should be included, should I send v2 with
> this one ? Or both with v3?

Please take care of this in future. I have applied both.
> >
> > Those patches could have been sent separately but if you are sending the
> > patches in a series, Then next version should have all the patches unless you
> > want to drop some of the patches.
> >
> > Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
> >
> >

Applied to dpdk-next-crypto

Thanks.

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

* Re: [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
  2020-06-24 14:26 [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup Arek Kusztal
  2020-07-02 17:50 ` Akhil Goyal
@ 2020-07-08 13:37 ` Thomas Monjalon
  2020-07-08 14:05   ` Akhil Goyal
  2020-07-08 14:10   ` Trahe, Fiona
  1 sibling, 2 replies; 7+ messages in thread
From: Thomas Monjalon @ 2020-07-08 13:37 UTC (permalink / raw)
  To: akhil.goyal, fiona.trahe, arkadiuszx.kusztal
  Cc: dev, ferruh.yigit, bruce.richardson, orika, jerinj, stephen,
	olivier.matz, hemant.agrawal, mdr

24/06/2020 16:26, Arek Kusztal:
> From: Fiona Trahe <fiona.trahe@intel.com>
> 
> This patch adds function that can check if queue pair
> was already setup. This may be useful when dealing with
> multi process approach in cryptodev.

That's all? No more justification?
No usage in example apps?
No addition in test apps?
Is it needed for the application?

I don't know cryptodev enough, but I can tell with ethdev experience
that we are a lot more demanding when adding a new API in ethdev.
We are still fixing the API errors done years ago in ethdev,
and it is very difficult to deprecate what was used in the past.
I hope my fear is wrong and you are not doing the same errors
as we did in ethdev, it would be a pity.



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

* Re: [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
  2020-07-08 13:37 ` Thomas Monjalon
@ 2020-07-08 14:05   ` Akhil Goyal
  2020-07-08 14:10   ` Trahe, Fiona
  1 sibling, 0 replies; 7+ messages in thread
From: Akhil Goyal @ 2020-07-08 14:05 UTC (permalink / raw)
  To: Thomas Monjalon, fiona.trahe, arkadiuszx.kusztal
  Cc: dev, ferruh.yigit, bruce.richardson, orika, jerinj, stephen,
	olivier.matz, Hemant Agrawal, mdr

Hi Thomas,
> 
> 24/06/2020 16:26, Arek Kusztal:
> > From: Fiona Trahe <fiona.trahe@intel.com>
> >
> > This patch adds function that can check if queue pair
> > was already setup. This may be useful when dealing with
> > multi process approach in cryptodev.
> 
> That's all? No more justification?
> No usage in example apps?
> No addition in test apps?
> Is it needed for the application?
The application is in review stage right now.
http://patches.dpdk.org/patch/72156/

The current patch is a cryptodev patch which should be part of RC1.
The app is targeted for RC2.
The API explanation in rte_cryptodev.h is I think sufficient and the usage will be
Demonstrated in the app.
/**
+ * Get the status of queue pairs setup on a specific crypto device
+ *
+ * @param	dev_id		Crypto device identifier.
+ * @param	queue_pair_id	The index of the queue pairs to set up. The
+ *				value must be in the range [0, nb_queue_pair
+ *				- 1] previously supplied to
+ *				rte_cryptodev_configure().
+ * @return
+ *   - 0: qp was not configured
+ *	 - 1: qp was configured
+ *	 - -EINVAL: device was not configured
+ */
> 
> I don't know cryptodev enough, but I can tell with ethdev experience
> that we are a lot more demanding when adding a new API in ethdev.
> We are still fixing the API errors done years ago in ethdev,
> and it is very difficult to deprecate what was used in the past.
> I hope my fear is wrong and you are not doing the same errors
> as we did in ethdev, it would be a pity.
> 


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

* Re: [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
  2020-07-08 13:37 ` Thomas Monjalon
  2020-07-08 14:05   ` Akhil Goyal
@ 2020-07-08 14:10   ` Trahe, Fiona
  1 sibling, 0 replies; 7+ messages in thread
From: Trahe, Fiona @ 2020-07-08 14:10 UTC (permalink / raw)
  To: Thomas Monjalon, akhil.goyal, Kusztal, ArkadiuszX
  Cc: dev, Yigit, Ferruh, Richardson, Bruce, orika, jerinj, stephen,
	olivier.matz, hemant.agrawal, mdr, Trahe, Fiona



> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, July 8, 2020 2:38 PM
> To: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; orika@mellanox.com; jerinj@marvell.com;
> stephen@networkplumber.org; olivier.matz@6wind.com; hemant.agrawal@nxp.com; mdr@ashroe.eu
> Subject: Re: [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
> 
> 24/06/2020 16:26, Arek Kusztal:
> > From: Fiona Trahe <fiona.trahe@intel.com>
> >
> > This patch adds function that can check if queue pair
> > was already setup. This may be useful when dealing with
> > multi process approach in cryptodev.
> 
> That's all? No more justification?
> No usage in example apps?
> No addition in test apps?
> Is it needed for the application?
> 
> I don't know cryptodev enough, but I can tell with ethdev experience
> that we are a lot more demanding when adding a new API in ethdev.
> We are still fixing the API errors done years ago in ethdev,
> and it is very difficult to deprecate what was used in the past.
> I hope my fear is wrong and you are not doing the same errors
> as we did in ethdev, it would be a pity.

This is used in a new example app which we expect will be applied in rc2.
There are use-cases where the primary process creates and configure the device and queue-pairs
and a secondary process uses the queue-pair on the data-path.
It seems safer to provide the secondary a way to ensure that the qp it's about to use is setup already
rather than it relying on assumptions based on timing or the primary process communicating this to the secondary.
It's quite a simple API and fulfils this purpose.
If anyone wants to propose any improvements to it feedback would be appreciated
https://patches.dpdk.org/patch/72157/


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

end of thread, other threads:[~2020-07-08 14:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24 14:26 [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup Arek Kusztal
2020-07-02 17:50 ` Akhil Goyal
2020-07-03 14:08   ` Kusztal, ArkadiuszX
2020-07-04 20:11     ` Akhil Goyal
2020-07-08 13:37 ` Thomas Monjalon
2020-07-08 14:05   ` Akhil Goyal
2020-07-08 14:10   ` Trahe, Fiona

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).