[dpdk-dev,v3,3/4] cryptodev: rework dependency on vdev header

Message ID 20171006083934.34819-4-jblunck@infradead.org (mailing list archive)
State Rejected, archived
Delegated to: Pablo de Lara Guarch
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Jan Blunck Oct. 6, 2017, 8:39 a.m. UTC
  The rte_cryptodev_vdev_pmd_init() is a helper for vdev-based drivers.
By moving the helper to the header we don't require rte_vdev.h at
build-time of the librte_cryptodev library. This is a preparation to
move the vdev bus into a standalone library.

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_cryptodev/rte_cryptodev_pmd.c       | 29 ------------------------
 lib/librte_cryptodev/rte_cryptodev_vdev.h      | 31 +++++++++++++++++++++++---
 lib/librte_cryptodev/rte_cryptodev_version.map |  1 -
 3 files changed, 28 insertions(+), 33 deletions(-)
  

Comments

De Lara Guarch, Pablo Oct. 10, 2017, 8:47 a.m. UTC | #1
> -----Original Message-----
> From: Jan Blunck [mailto:jblunck@gmail.com] On Behalf Of Jan Blunck
> Sent: Friday, October 6, 2017 9:40 AM
> To: dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH v3 3/4] cryptodev: rework dependency on vdev header
> 
> The rte_cryptodev_vdev_pmd_init() is a helper for vdev-based drivers.
> By moving the helper to the header we don't require rte_vdev.h at build-
> time of the librte_cryptodev library. This is a preparation to move the vdev
> bus into a standalone library.
> 
> Signed-off-by: Jan Blunck <jblunck@infradead.org>

This looks good, but could you also remove the deprecation notice from the previous release?
Apart from that:

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
  
Jan Blunck Oct. 10, 2017, 11:29 a.m. UTC | #2
On Tue, Oct 10, 2017 at 10:47 AM, De Lara Guarch, Pablo
<pablo.de.lara.guarch@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Jan Blunck [mailto:jblunck@gmail.com] On Behalf Of Jan Blunck
>> Sent: Friday, October 6, 2017 9:40 AM
>> To: dev@dpdk.org
>> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>
>> Subject: [PATCH v3 3/4] cryptodev: rework dependency on vdev header
>>
>> The rte_cryptodev_vdev_pmd_init() is a helper for vdev-based drivers.
>> By moving the helper to the header we don't require rte_vdev.h at build-
>> time of the librte_cryptodev library. This is a preparation to move the vdev
>> bus into a standalone library.
>>
>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>
> This looks good, but could you also remove the deprecation notice from the previous release?
> Apart from that:
>

I wondered about that too. The message said that it will be static
from 17.11 on and I wonder if we should actually keep it into the
notes for this release.
  
De Lara Guarch, Pablo Oct. 10, 2017, 12:49 p.m. UTC | #3
> -----Original Message-----

> From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of Jan

> Blunck

> Sent: Tuesday, October 10, 2017 12:30 PM

> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>

> Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>

> Subject: Re: [PATCH v3 3/4] cryptodev: rework dependency on vdev header

> 

> On Tue, Oct 10, 2017 at 10:47 AM, De Lara Guarch, Pablo

> <pablo.de.lara.guarch@intel.com> wrote:

> >

> >

> >> -----Original Message-----

> >> From: Jan Blunck [mailto:jblunck@gmail.com] On Behalf Of Jan Blunck

> >> Sent: Friday, October 6, 2017 9:40 AM

> >> To: dev@dpdk.org

> >> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo

> >> <pablo.de.lara.guarch@intel.com>

> >> Subject: [PATCH v3 3/4] cryptodev: rework dependency on vdev header

> >>

> >> The rte_cryptodev_vdev_pmd_init() is a helper for vdev-based drivers.

> >> By moving the helper to the header we don't require rte_vdev.h at

> >> build- time of the librte_cryptodev library. This is a preparation to

> >> move the vdev bus into a standalone library.

> >>

> >> Signed-off-by: Jan Blunck <jblunck@infradead.org>

> >

> > This looks good, but could you also remove the deprecation notice from

> the previous release?

> > Apart from that:

> >

> 

> I wondered about that too. The message said that it will be static from

> 17.11 on and I wonder if we should actually keep it into the notes for this

> release.


Well, actually this change should be added in release notes (under API changes, I think),
and removed from deprecation.rst.

Thanks,
Pablo
  
De Lara Guarch, Pablo Oct. 11, 2017, 2:45 p.m. UTC | #4
Hi Jan,

> -----Original Message-----

> From: De Lara Guarch, Pablo

> Sent: Tuesday, October 10, 2017 1:50 PM

> To: Jan Blunck <jblunck@infradead.org>

> Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>

> Subject: RE: [PATCH v3 3/4] cryptodev: rework dependency on vdev header

> 

> 

> 

> > -----Original Message-----

> > From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of Jan

> > Blunck

> > Sent: Tuesday, October 10, 2017 12:30 PM

> > To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>

> > Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>

> > Subject: Re: [PATCH v3 3/4] cryptodev: rework dependency on vdev

> > header

> >

> > On Tue, Oct 10, 2017 at 10:47 AM, De Lara Guarch, Pablo

> > <pablo.de.lara.guarch@intel.com> wrote:

> > >

> > >

> > >> -----Original Message-----

> > >> From: Jan Blunck [mailto:jblunck@gmail.com] On Behalf Of Jan Blunck

> > >> Sent: Friday, October 6, 2017 9:40 AM

> > >> To: dev@dpdk.org

> > >> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch,

> > >> Pablo <pablo.de.lara.guarch@intel.com>

> > >> Subject: [PATCH v3 3/4] cryptodev: rework dependency on vdev

> header

> > >>

> > >> The rte_cryptodev_vdev_pmd_init() is a helper for vdev-based drivers.

> > >> By moving the helper to the header we don't require rte_vdev.h at

> > >> build- time of the librte_cryptodev library. This is a preparation

> > >> to move the vdev bus into a standalone library.

> > >>

> > >> Signed-off-by: Jan Blunck <jblunck@infradead.org>

> > >

> > > This looks good, but could you also remove the deprecation notice

> > > from

> > the previous release?

> > > Apart from that:

> > >

> >

> > I wondered about that too. The message said that it will be static

> > from

> > 17.11 on and I wonder if we should actually keep it into the notes for

> > this release.

> 

> Well, actually this change should be added in release notes (under API

> changes, I think), and removed from deprecation.rst.


Are you going to send a new patchset soon, so it can make RC1?

Thanks!
Pablo

> 

> Thanks,

> Pablo
  

Patch

diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.c b/lib/librte_cryptodev/rte_cryptodev_pmd.c
index ec6eeffa1..eaeddf74b 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.c
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.c
@@ -75,35 +75,6 @@  rte_cryptodev_vdev_parse_integer_arg(const char *key __rte_unused,
 	return 0;
 }
 
-struct rte_cryptodev *
-rte_cryptodev_vdev_pmd_init(const char *name, size_t dev_private_size,
-		int socket_id, struct rte_vdev_device *vdev)
-{
-	struct rte_cryptodev *cryptodev;
-
-	/* allocate device structure */
-	cryptodev = rte_cryptodev_pmd_allocate(name, socket_id);
-	if (cryptodev == NULL)
-		return NULL;
-
-	/* allocate private device structure */
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		cryptodev->data->dev_private =
-				rte_zmalloc_socket("cryptodev device private",
-						dev_private_size,
-						RTE_CACHE_LINE_SIZE,
-						socket_id);
-
-		if (cryptodev->data->dev_private == NULL)
-			rte_panic("Cannot allocate memzone for private device"
-					" data");
-	}
-
-	cryptodev->device = &vdev->device;
-
-	return cryptodev;
-}
-
 int
 rte_cryptodev_vdev_parse_init_params(struct rte_crypto_vdev_init_params *params,
 		const char *input_args)
diff --git a/lib/librte_cryptodev/rte_cryptodev_vdev.h b/lib/librte_cryptodev/rte_cryptodev_vdev.h
index 94ab9d33d..c6bafd01b 100644
--- a/lib/librte_cryptodev/rte_cryptodev_vdev.h
+++ b/lib/librte_cryptodev/rte_cryptodev_vdev.h
@@ -36,7 +36,7 @@ 
 #include <rte_vdev.h>
 #include <inttypes.h>
 
-#include "rte_cryptodev.h"
+#include "rte_cryptodev_pmd.h"
 
 #define RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS	8
 #define RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS	2048
@@ -78,9 +78,34 @@  struct rte_crypto_vdev_init_params {
  *   - Cryptodev pointer if device is successfully created.
  *   - NULL if device cannot be created.
  */
-struct rte_cryptodev *
+static inline struct rte_cryptodev *
 rte_cryptodev_vdev_pmd_init(const char *name, size_t dev_private_size,
-		int socket_id, struct rte_vdev_device *vdev);
+		int socket_id, struct rte_vdev_device *dev)
+{
+	struct rte_cryptodev *cryptodev;
+
+	/* allocate device structure */
+	cryptodev = rte_cryptodev_pmd_allocate(name, socket_id);
+	if (cryptodev == NULL)
+		return NULL;
+
+	/* allocate private device structure */
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		cryptodev->data->dev_private =
+				rte_zmalloc_socket("cryptodev device private",
+						dev_private_size,
+						RTE_CACHE_LINE_SIZE,
+						socket_id);
+
+		if (cryptodev->data->dev_private == NULL)
+			rte_panic("Cannot allocate memzone for private device"
+					" data");
+	}
+
+	cryptodev->device = &dev->device;
+
+	return cryptodev;
+}
 
 /**
  * @internal
diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
index e9ba88ac5..abdbbdada 100644
--- a/lib/librte_cryptodev/rte_cryptodev_version.map
+++ b/lib/librte_cryptodev/rte_cryptodev_version.map
@@ -74,7 +74,6 @@  DPDK_17.08 {
 	rte_cryptodev_sym_session_init;
 	rte_cryptodev_sym_session_clear;
 	rte_cryptodev_vdev_parse_init_params;
-	rte_cryptodev_vdev_pmd_init;
 	rte_crypto_aead_algorithm_strings;
 	rte_crypto_aead_operation_strings;