[dpdk-dev] crypto/scheduler: fix include of local headers

Message ID 20170328105816.22502-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Headers

Checks

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

Commit Message

Bruce Richardson March 28, 2017, 10:58 a.m. UTC
  When a C file for a library/driver is including the public header files for
that library, those need to be included as local includes using quotes
rather than angle-brackets. Without doing so, parallel builds can fail, as
the compiler will only look for those headers in the global include folder
rather than locally, and the build system does not enforce that the headers
for a lib are installed before the rest of the lib is compiled.

Fixes: 097ab0bac017 ("crypto/scheduler: add API")
Fixes: 503e9c5afb38 ("crypto/scheduler: register as vdev driver")
Fixes: 31439ee72b2c ("crypto/scheduler: add API implementations")

CC: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/crypto/scheduler/rte_cryptodev_scheduler.c | 2 +-
 drivers/crypto/scheduler/rte_cryptodev_scheduler.h | 2 +-
 drivers/crypto/scheduler/scheduler_pmd.c           | 2 +-
 drivers/crypto/scheduler/scheduler_pmd_private.h   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
  

Comments

Bruce Richardson March 28, 2017, 11:04 a.m. UTC | #1
On Tue, Mar 28, 2017 at 11:58:16AM +0100, Bruce Richardson wrote:
> When a C file for a library/driver is including the public header files for
> that library, those need to be included as local includes using quotes
> rather than angle-brackets. Without doing so, parallel builds can fail, as
> the compiler will only look for those headers in the global include folder
> rather than locally, and the build system does not enforce that the headers
> for a lib are installed before the rest of the lib is compiled.
> 
> Fixes: 097ab0bac017 ("crypto/scheduler: add API")
> Fixes: 503e9c5afb38 ("crypto/scheduler: register as vdev driver")
> Fixes: 31439ee72b2c ("crypto/scheduler: add API implementations")
> 
> CC: stable@dpdk.org
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  drivers/crypto/scheduler/rte_cryptodev_scheduler.c | 2 +-
>  drivers/crypto/scheduler/rte_cryptodev_scheduler.h | 2 +-
>  drivers/crypto/scheduler/scheduler_pmd.c           | 2 +-
>  drivers/crypto/scheduler/scheduler_pmd_private.h   | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
Two follow-up points:

* Even though this is a crypto patch, I think it should go in main tree
  as builds are broken right now if you turn on the PMD and use a large
  -j setting. (i.e. it's broken for me!! :-) )
* Is there a reason why this PMD is disabled by default? It doesn't
  appear to have any external dependencies and if we turn it on by
  default we should be able to catch issues like this a lot quicker.

Regards,
/Bruce
  
Thomas Monjalon March 28, 2017, 12:04 p.m. UTC | #2
2017-03-28 12:04, Bruce Richardson:
> On Tue, Mar 28, 2017 at 11:58:16AM +0100, Bruce Richardson wrote:
> > When a C file for a library/driver is including the public header files for
> > that library, those need to be included as local includes using quotes
> > rather than angle-brackets. Without doing so, parallel builds can fail, as
> > the compiler will only look for those headers in the global include folder
> > rather than locally, and the build system does not enforce that the headers
> > for a lib are installed before the rest of the lib is compiled.
> > 
> > Fixes: 097ab0bac017 ("crypto/scheduler: add API")
> > Fixes: 503e9c5afb38 ("crypto/scheduler: register as vdev driver")
> > Fixes: 31439ee72b2c ("crypto/scheduler: add API implementations")
> > 
> > CC: stable@dpdk.org
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  drivers/crypto/scheduler/rte_cryptodev_scheduler.c | 2 +-
> >  drivers/crypto/scheduler/rte_cryptodev_scheduler.h | 2 +-
> >  drivers/crypto/scheduler/scheduler_pmd.c           | 2 +-
> >  drivers/crypto/scheduler/scheduler_pmd_private.h   | 2 +-
> >  4 files changed, 4 insertions(+), 4 deletions(-)
> > 
> Two follow-up points:
> 
> * Even though this is a crypto patch, I think it should go in main tree
>   as builds are broken right now if you turn on the PMD and use a large
>   -j setting. (i.e. it's broken for me!! :-) )

Applied, thanks for catching

> * Is there a reason why this PMD is disabled by default? It doesn't
>   appear to have any external dependencies and if we turn it on by
>   default we should be able to catch issues like this a lot quicker.

I forgot to check this PMD. I was not compiling it either.
I agree it should be enabled by default, or at least enabled in test-build.sh.
  

Patch

diff --git a/drivers/crypto/scheduler/rte_cryptodev_scheduler.c b/drivers/crypto/scheduler/rte_cryptodev_scheduler.c
index 11e8143..2f49ad4 100644
--- a/drivers/crypto/scheduler/rte_cryptodev_scheduler.c
+++ b/drivers/crypto/scheduler/rte_cryptodev_scheduler.c
@@ -32,9 +32,9 @@ 
 #include <rte_reorder.h>
 #include <rte_cryptodev.h>
 #include <rte_cryptodev_pmd.h>
-#include <rte_cryptodev_scheduler.h>
 #include <rte_malloc.h>
 
+#include "rte_cryptodev_scheduler.h"
 #include "scheduler_pmd_private.h"
 
 /** update the scheduler pmd's capability with attaching device's
diff --git a/drivers/crypto/scheduler/rte_cryptodev_scheduler.h b/drivers/crypto/scheduler/rte_cryptodev_scheduler.h
index 7ef44e7..98ab8f2 100644
--- a/drivers/crypto/scheduler/rte_cryptodev_scheduler.h
+++ b/drivers/crypto/scheduler/rte_cryptodev_scheduler.h
@@ -34,7 +34,7 @@ 
 #ifndef _RTE_CRYPTO_SCHEDULER_H
 #define _RTE_CRYPTO_SCHEDULER_H
 
-#include <rte_cryptodev_scheduler_operations.h>
+#include "rte_cryptodev_scheduler_operations.h"
 
 #ifdef __cplusplus
 extern "C" {
diff --git a/drivers/crypto/scheduler/scheduler_pmd.c b/drivers/crypto/scheduler/scheduler_pmd.c
index eeafbe6..7efdc91 100644
--- a/drivers/crypto/scheduler/scheduler_pmd.c
+++ b/drivers/crypto/scheduler/scheduler_pmd.c
@@ -37,8 +37,8 @@ 
 #include <rte_malloc.h>
 #include <rte_cpuflags.h>
 #include <rte_reorder.h>
-#include <rte_cryptodev_scheduler.h>
 
+#include "rte_cryptodev_scheduler.h"
 #include "scheduler_pmd_private.h"
 
 struct scheduler_init_params {
diff --git a/drivers/crypto/scheduler/scheduler_pmd_private.h b/drivers/crypto/scheduler/scheduler_pmd_private.h
index ac4690e..9a1409c 100644
--- a/drivers/crypto/scheduler/scheduler_pmd_private.h
+++ b/drivers/crypto/scheduler/scheduler_pmd_private.h
@@ -36,7 +36,7 @@ 
 
 #include <rte_hash.h>
 #include <rte_reorder.h>
-#include <rte_cryptodev_scheduler.h>
+#include "rte_cryptodev_scheduler.h"
 
 /**< Maximum number of bonded devices per devices */
 #ifndef MAX_SLAVES_NUM