Re: [dpdk-dev] [PATCH v2 3/4] cryptodev: rework PMD init to not require rte_vdev.h

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

> -----Original Message-----
> From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of Jan
> Blunck
> Sent: Thursday, October 5, 2017 3:53 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 v2 3/4] cryptodev: rework PMD init to not require
> rte_vdev.h
> 
> On Mon, Sep 4, 2017 at 4:32 PM, De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com> wrote:
> > Hi Jan,
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of De Lara Guarch,
> >> Pablo
> >> Sent: Saturday, July 15, 2017 12:05 PM
> >> To: Jan Blunck <jblunck@infradead.org>; dev@dpdk.org
> >> Cc: Doherty, Declan <declan.doherty@intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH v2 3/4] cryptodev: rework PMD init to
> >> not require rte_vdev.h
> >>
> >> Hi
> >>
> >> > -----Original Message-----
> >> > From: Jan Blunck [mailto:jblunck@gmail.com] On Behalf Of Jan Blunck
> >> > Sent: Wednesday, July 12, 2017 8:59 PM
> >> > To: dev@dpdk.org
> >> > Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch,
> >> > Pablo <pablo.de.lara.guarch@intel.com>
> >> > Subject: [PATCH v2 3/4] cryptodev: rework PMD init to not require
> >> > rte_vdev.h
> >> >
> >> > 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>
> >>
> >> I am seeing some clang errors when applying this patch:
> >>
> >> lib/librte_cryptodev/rte_cryptodev_vdev.h:88:14: error: implicit
> >> declaration of function 'rte_cryptodev_pmd_allocate' is invalid in
> >> C99 [- Werror,-Wimplicit-function-declaration]
> >>         cryptodev = rte_cryptodev_pmd_allocate(name, socket_id);
> >>                     ^
> >> lib/librte_cryptodev/rte_cryptodev_vdev.h:88:12: error: incompatible
> >> integer to pointer conversion assigning to 'struct rte_cryptodev *' from
> 'int'
> >> [-Werror,-Wint-conversion]
> >>         cryptodev = rte_cryptodev_pmd_allocate(name, socket_id);
> >>
> 
> Pablo,
> 
> I can not reproduce this. There is already an include for
> rte_cryptodev_pmd_allocate() in rte_cryptodev_vdev.h.

Yes, but you are adding that header file in patch 4, so patch 3 fails.
You should add it in patch 3:

--- a/lib/librte_cryptodev/rte_cryptodev_vdev.h
+++ b/lib/librte_cryptodev/rte_cryptodev_vdev.h
@@ -36,6 +36,7 @@
 #include <rte_vdev.h>
 #include <inttypes.h>

+#include "rte_cryptodev_pmd.h"
 #include "rte_cryptodev.h"

 #define RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS  8

> 
> >> Also, looks like git commit title is not correct, according to check-git-
> log.sh:
> >>
> >> Wrong headline format:
> >>         cryptodev: rework PMD init to not require rte_vdev.h
> >
> 
> This script complains about underscores ...

Yes, could you reword it a bit? Instead of rte_vdev, use vdev header?

> 
> Tell me what you think,
> Jan

Thanks,
Pablo