[PATCH] cryptodev: fix C++ include

[PATCH] cryptodev: fix C++ include

[Thomas Monjalon]

Some cryptodev functions were not included in the extern "C" block,
so it is moved to start before.

An include is also moved to avoid being part of this block.

Fixes: 719834a6849e ("use C linkage where appropriate in headers")
Cc: stable@dpdk.org

Reported-by: Zhigang Hu <zhigang.hu@intel.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 .mailmap                      |  1 +
 lib/cryptodev/rte_cryptodev.h | 10 +++++-----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/.mailmap b/.mailmap
index 2bf38f9e8c..1e4bb06d6e 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1794,6 +1794,7 @@ Zhenghua Zhou <zhenghuax.zhou@intel.com>
 Zhenning Xiao <zhenning.xiao@intel.com>
 Zhe Tao <zhe.tao@intel.com>
 Zhichao Zeng <zhichaox.zeng@intel.com>
+Zhigang Hu <zhigang.hu@intel.com>
 Zhigang Lu <zlu@ezchip.com>
 Zhiguang He <hezhiguang3@huawei.com>
 Zhihong Peng <zhihongx.peng@intel.com>
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index c64d2f83a0..31a7edb189 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -21,6 +21,11 @@
 #include <rte_rcu_qsbr.h>
 
 #include "rte_cryptodev_trace_fp.h"
+#include "rte_cryptodev_core.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
 
 /**
  * @internal Logtype used for cryptodev related messages.
@@ -1928,11 +1933,6 @@ int rte_cryptodev_remove_deq_callback(uint8_t dev_id,
 				      uint16_t qp_id,
 				      struct rte_cryptodev_cb *cb);
 
-#include <rte_cryptodev_core.h>
-
-#ifdef __cplusplus
-extern "C" {
-#endif
 /**
  *
  * Dequeue a burst of processed crypto operations from a queue on the crypto
-- 
2.47.1

RE: [EXTERNAL] [PATCH] cryptodev: fix C++ include

[Akhil Goyal]

> Some cryptodev functions were not included in the extern "C" block,
> so it is moved to start before.
> 
> An include is also moved to avoid being part of this block.
> 
> Fixes: 719834a6849e ("use C linkage where appropriate in headers")
> Cc: stable@dpdk.org
> 
> Reported-by: Zhigang Hu <zhigang.hu@intel.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  .mailmap                      |  1 +
>  lib/cryptodev/rte_cryptodev.h | 10 +++++-----
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index 2bf38f9e8c..1e4bb06d6e 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -1794,6 +1794,7 @@ Zhenghua Zhou <zhenghuax.zhou@intel.com>
>  Zhenning Xiao <zhenning.xiao@intel.com>
>  Zhe Tao <zhe.tao@intel.com>
>  Zhichao Zeng <zhichaox.zeng@intel.com>
> +Zhigang Hu <zhigang.hu@intel.com>
>  Zhigang Lu <zlu@ezchip.com>
>  Zhiguang He <hezhiguang3@huawei.com>
>  Zhihong Peng <zhihongx.peng@intel.com>
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index c64d2f83a0..31a7edb189 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -21,6 +21,11 @@
>  #include <rte_rcu_qsbr.h>
> 
>  #include "rte_cryptodev_trace_fp.h"
> +#include "rte_cryptodev_core.h"

Fix is ok but rte_cryptodev_core.h should not be moved up.
It is added in the middle to segregate the fast path APIs.
And it is an internal header which cannot be included by app directly.
I think the same schema is followed in ethdev as well.

> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> 
>  /**
>   * @internal Logtype used for cryptodev related messages.
> @@ -1928,11 +1933,6 @@ int rte_cryptodev_remove_deq_callback(uint8_t
> dev_id,
>  				      uint16_t qp_id,
>  				      struct rte_cryptodev_cb *cb);
> 
> -#include <rte_cryptodev_core.h>
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
>  /**
>   *
>   * Dequeue a burst of processed crypto operations from a queue on the crypto
> --
> 2.47.1

Re: [EXTERNAL] [PATCH] cryptodev: fix C++ include

[Thomas Monjalon]

19/12/2024 13:24, Akhil Goyal:
> > Some cryptodev functions were not included in the extern "C" block,
> > so it is moved to start before.
> > 
> > An include is also moved to avoid being part of this block.
[...]
> > --- a/lib/cryptodev/rte_cryptodev.h
> > +++ b/lib/cryptodev/rte_cryptodev.h
> > @@ -21,6 +21,11 @@
> >  #include <rte_rcu_qsbr.h>
> > 
> >  #include "rte_cryptodev_trace_fp.h"
> > +#include "rte_cryptodev_core.h"
> 
> Fix is ok but rte_cryptodev_core.h should not be moved up.
> It is added in the middle to segregate the fast path APIs.
> And it is an internal header which cannot be included by app directly.
> I think the same schema is followed in ethdev as well.

Indeed, this is how it's done in ethdev:

#ifdef __cplusplus
}
#endif

#include <rte_ethdev_core.h>

#ifdef __cplusplus
extern "C" {
#endif

I'll do the same in v2.

[PATCH v2] cryptodev: fix C++ include

[Thomas Monjalon]

Some cryptodev functions were not included in an extern "C" block.

There are 2 blocks, the second one being fast path inline functions,
preceded with an include of the required rte_cryptodev_core.h file.

Fixes: 719834a6849e ("use C linkage where appropriate in headers")
Cc: stable@dpdk.org

Reported-by: Zhigang Hu <zhigang.hu@intel.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v2: keep rte_cryptodev_core.h include at the same place
---
 .mailmap                      |  1 +
 lib/cryptodev/rte_cryptodev.h | 12 ++++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/.mailmap b/.mailmap
index 2bf38f9e8c..1e4bb06d6e 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1794,6 +1794,7 @@ Zhenghua Zhou <zhenghuax.zhou@intel.com>
 Zhenning Xiao <zhenning.xiao@intel.com>
 Zhe Tao <zhe.tao@intel.com>
 Zhichao Zeng <zhichaox.zeng@intel.com>
+Zhigang Hu <zhigang.hu@intel.com>
 Zhigang Lu <zlu@ezchip.com>
 Zhiguang He <hezhiguang3@huawei.com>
 Zhihong Peng <zhihongx.peng@intel.com>
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index c64d2f83a0..071ff3dbdf 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -22,6 +22,10 @@
 
 #include "rte_cryptodev_trace_fp.h"
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 /**
  * @internal Logtype used for cryptodev related messages.
  */
@@ -1928,11 +1932,16 @@ int rte_cryptodev_remove_deq_callback(uint8_t dev_id,
 				      uint16_t qp_id,
 				      struct rte_cryptodev_cb *cb);
 
-#include <rte_cryptodev_core.h>
+#ifdef __cplusplus
+}
+#endif
+
+#include "rte_cryptodev_core.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
+
 /**
  *
  * Dequeue a burst of processed crypto operations from a queue on the crypto
@@ -2125,7 +2134,6 @@ rte_cryptodev_qp_depth_used(uint8_t dev_id, uint16_t qp_id)
 	return rc;
 }
 
-
 #ifdef __cplusplus
 }
 #endif
-- 
2.47.1

Re: [PATCH v2] cryptodev: fix C++ include

[David Marchand]

On Thu, Dec 19, 2024 at 2:31 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> Some cryptodev functions were not included in an extern "C" block.
>
> There are 2 blocks, the second one being fast path inline functions,
> preceded with an include of the required rte_cryptodev_core.h file.
>
> Fixes: 719834a6849e ("use C linkage where appropriate in headers")
> Cc: stable@dpdk.org
>
> Reported-by: Zhigang Hu <zhigang.hu@intel.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

I wonder if there is also an issue with
lib/cryptodev/rte_crypto_asym.h, for symbols
rte_crypto_asym_ke_strings and rte_crypto_asym_op_strings.
But at least this patch looks fine to me.

Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

Re: [PATCH v2] cryptodev: fix C++ include

[Mattias Rönnblom]

On 2024-12-19 14:30, Thomas Monjalon wrote:
> Some cryptodev functions were not included in an extern "C" block.
> 
> There are 2 blocks, the second one being fast path inline functions,
> preceded with an include of the required rte_cryptodev_core.h file.
> 
> Fixes: 719834a6849e ("use C linkage where appropriate in headers")
> Cc: stable@dpdk.org
> 
> Reported-by: Zhigang Hu <zhigang.hu@intel.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v2: keep rte_cryptodev_core.h include at the same place
> ---
>   .mailmap                      |  1 +
>   lib/cryptodev/rte_cryptodev.h | 12 ++++++++++--
>   2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index 2bf38f9e8c..1e4bb06d6e 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -1794,6 +1794,7 @@ Zhenghua Zhou <zhenghuax.zhou@intel.com>
>   Zhenning Xiao <zhenning.xiao@intel.com>
>   Zhe Tao <zhe.tao@intel.com>
>   Zhichao Zeng <zhichaox.zeng@intel.com>
> +Zhigang Hu <zhigang.hu@intel.com>
>   Zhigang Lu <zlu@ezchip.com>
>   Zhiguang He <hezhiguang3@huawei.com>
>   Zhihong Peng <zhihongx.peng@intel.com>
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index c64d2f83a0..071ff3dbdf 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -22,6 +22,10 @@
>   
>   #include "rte_cryptodev_trace_fp.h"
>   
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
>   /**
>    * @internal Logtype used for cryptodev related messages.
>    */
> @@ -1928,11 +1932,16 @@ int rte_cryptodev_remove_deq_callback(uint8_t dev_id,
>   				      uint16_t qp_id,
>   				      struct rte_cryptodev_cb *cb);
>   
> -#include <rte_cryptodev_core.h>
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#include "rte_cryptodev_core.h"
>   
>   #ifdef __cplusplus
>   extern "C" {
>   #endif
> +
>   /**
>    *
>    * Dequeue a burst of processed crypto operations from a queue on the crypto
> @@ -2125,7 +2134,6 @@ rte_cryptodev_qp_depth_used(uint8_t dev_id, uint16_t qp_id)
>   	return rc;
>   }
>   
> -
>   #ifdef __cplusplus
>   }
>   #endif

Reviewed-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>

Re: [PATCH v2] cryptodev: fix C++ include

[Mattias Rönnblom]

On 2024-12-19 16:37, David Marchand wrote:
> On Thu, Dec 19, 2024 at 2:31 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> Some cryptodev functions were not included in an extern "C" block.
>>
>> There are 2 blocks, the second one being fast path inline functions,
>> preceded with an include of the required rte_cryptodev_core.h file.
>>
>> Fixes: 719834a6849e ("use C linkage where appropriate in headers")
>> Cc: stable@dpdk.org
>>
>> Reported-by: Zhigang Hu <zhigang.hu@intel.com>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> I wonder if there is also an issue with
> lib/cryptodev/rte_crypto_asym.h, for symbols
> rte_crypto_asym_ke_strings and rte_crypto_asym_op_strings.

+1

> But at least this patch looks fine to me.
> 
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> 
>