The question for the modification against (lib/cryptodev/rte_cryptodev.h) in commit 719834a6

The question for the modification against (lib/cryptodev/rte_cryptodev.h) in commit 719834a6

[Hu, Zhigang]

[-- Attachment #1.1: Type: text/plain, Size: 740 bytes --]

Hi, experts,

My C++ program calls some functions defined in lib/cryptodev/rte_cryptodev.h. After upgrading dpdk from 23.11 to 24.11, my C++ program failed to be complied.  It looks like that the cimmit 719834a6 caused my issue.

It seems that the commit 719834a6 intends to exclude all head files from the
#ifdef __cplusplus
extern "C" {
#endif
......
#ifdef __cplusplus
}
#endif
Unfortunately some function declarations are also excluded wrongly since these declarations are before the last head file. Below picture is an example. The function in the red circle can't be linked by C++ complier. Would you please help to check this issue?

Best regards!
Hu Zhigang
    
[cid:bbf5bf01-e72e-42b0-8b1a-fc627fbb1dc5]

[-- Attachment #1.2: Type: text/html, Size: 4833 bytes --]

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 90739 bytes --]

[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: The question for the modification against (lib/cryptodev/rte_cryptodev.h) in commit 719834a6

[Thomas Monjalon]

19/12/2024 11:41, Hu, Zhigang:
> Hi, experts,
> 
> My C++ program calls some functions defined in lib/cryptodev/rte_cryptodev.h. After upgrading dpdk from 23.11 to 24.11, my C++ program failed to be complied.  It looks like that the cimmit 719834a6 caused my issue.

Yes, you're right.
Thanks for reporting.

Please could you try this fix?
https://patches.dpdk.org/project/dpdk/patch/20241219114555.406331-1-thomas@monjalon.net/

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