[PATCH 0/7] Verify C++ compatibility of public headers

[PATCH 0/7] Verify C++ compatibility of public headers

[Bruce Richardson]

This patchset expands upon the exiting chkincs infrastructure to build a
C++ test application as well as the existing C test app, which helps
catch any build errors when the public DPDK headers are included in C++
code. The rest of this patchset fixes the errors caught by this
additional check - mostly missing casts, but also one use of "new" as a
variable name, and one missing closing brace for an 'extern "C"' block.

NOTE: the most complicated changes are in the 6th patch, where there
were linux headers included which were not C++ compatible. This patch
moves the inclusion of those headers to non-public locations in the code
to remove those issues, but I am uncertain what the knock-on impacts of
this change may be. Therefore, I ask particular review from vhost/vdpa
maintainers to see if there is a better solution to this issue

Bruce Richardson (7):
  eal: fix header build with C++
  eventdev: fix header build with C++
  graph: add explicit cast to allow C++ build
  ipsec: : add explicit cast to allow C++ build
  table: add explicit casts for C++ compatibility
  vhost: remove non-C++ compatible includes
  buildtools/chkincs: test headers for C++ compatibility

 buildtools/chkincs/main.cpp              |  4 +++
 buildtools/chkincs/meson.build           | 20 +++++++++++++++
 drivers/net/vhost/rte_eth_vhost.c        |  2 ++
 drivers/vdpa/ifc/ifcvf_vdpa.c            |  2 ++
 drivers/vdpa/mlx5/mlx5_vdpa.h            |  1 +
 lib/eal/include/generic/rte_ticketlock.h | 14 +++++------
 lib/eal/include/rte_trace_point.h        |  2 +-
 lib/eventdev/rte_event_timer_adapter.h   |  3 +++
 lib/eventdev/rte_eventdev.h              |  2 +-
 lib/graph/rte_graph_worker.h             |  2 +-
 lib/ipsec/rte_ipsec_group.h              |  4 +--
 lib/table/rte_table_hash_func.h          | 32 ++++++++++++------------
 lib/vhost/rte_vhost.h                    |  4 ---
 13 files changed, 60 insertions(+), 32 deletions(-)
 create mode 100644 buildtools/chkincs/main.cpp

--
2.32.0

[PATCH 1/7] eal: fix header build with C++

[Bruce Richardson]

C++ files could not include some headers because:

* "new" is a keyword in C++, so can't be a variable name
* there is no automatic casting to/from void *

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/include/generic/rte_ticketlock.h | 14 +++++++-------
 lib/eal/include/rte_trace_point.h        |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/eal/include/generic/rte_ticketlock.h b/lib/eal/include/generic/rte_ticketlock.h
index c1b8808f51..693c67b517 100644
--- a/lib/eal/include/generic/rte_ticketlock.h
+++ b/lib/eal/include/generic/rte_ticketlock.h
@@ -91,13 +91,13 @@ rte_ticketlock_unlock(rte_ticketlock_t *tl)
 static inline int
 rte_ticketlock_trylock(rte_ticketlock_t *tl)
 {
-	rte_ticketlock_t old, new;
-	old.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
-	new.tickets = old.tickets;
-	new.s.next++;
-	if (old.s.next == old.s.current) {
-		if (__atomic_compare_exchange_n(&tl->tickets, &old.tickets,
-		    new.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
+	rte_ticketlock_t oldl, newl;
+	oldl.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
+	newl.tickets = oldl.tickets;
+	newl.s.next++;
+	if (oldl.s.next == oldl.s.current) {
+		if (__atomic_compare_exchange_n(&tl->tickets, &oldl.tickets,
+		    newl.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
 			return 1;
 	}
 
diff --git a/lib/eal/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
index e226f073f7..0f8700974f 100644
--- a/lib/eal/include/rte_trace_point.h
+++ b/lib/eal/include/rte_trace_point.h
@@ -370,7 +370,7 @@ do { \
 do { \
 	if (unlikely(in == NULL)) \
 		return; \
-	rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
+	rte_strscpy((char *)mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 } while (0)
 
-- 
2.32.0

[PATCH 2/7] eventdev: fix header build with C++

[Bruce Richardson]

The eventdev headers had issues when used from C++

* Missing closing "}" for the extern "C" block
* No automatic casting to/from void *

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eventdev/rte_event_timer_adapter.h | 3 +++
 lib/eventdev/rte_eventdev.h            | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index 1551741820..1fe4dd8e8f 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -678,4 +678,7 @@ rte_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
 	return adapter->cancel_burst(adapter, evtims, nb_evtims);
 }
 
+#ifdef __cplusplus
+}
+#endif
 #endif /* __RTE_EVENT_TIMER_ADAPTER_H__ */
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index eef47d8acc..25fb7c89dd 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -1805,7 +1805,7 @@ __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
 		return 0;
 	}
 #endif
-	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, fn);
+	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, (void *)fn);
 	/*
 	 * Allow zero cost non burst mode routine invocation if application
 	 * requests nb_events as const one
-- 
2.32.0

Re: [PATCH 2/7] eventdev: fix header build with C++

[Jerin Jacob]

On Fri, Feb 4, 2022 at 11:12 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> The eventdev headers had issues when used from C++
>
> * Missing closing "}" for the extern "C" block
> * No automatic casting to/from void *
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>


> ---
>  lib/eventdev/rte_event_timer_adapter.h | 3 +++
>  lib/eventdev/rte_eventdev.h            | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
> index 1551741820..1fe4dd8e8f 100644
> --- a/lib/eventdev/rte_event_timer_adapter.h
> +++ b/lib/eventdev/rte_event_timer_adapter.h
> @@ -678,4 +678,7 @@ rte_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
>         return adapter->cancel_burst(adapter, evtims, nb_evtims);
>  }
>
> +#ifdef __cplusplus
> +}
> +#endif
>  #endif /* __RTE_EVENT_TIMER_ADAPTER_H__ */
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index eef47d8acc..25fb7c89dd 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -1805,7 +1805,7 @@ __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
>                 return 0;
>         }
>  #endif
> -       rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, fn);
> +       rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, (void *)fn);
>         /*
>          * Allow zero cost non burst mode routine invocation if application
>          * requests nb_events as const one
> --
> 2.32.0
>

[PATCH 3/7] graph: add explicit cast to allow C++ build

[Bruce Richardson]

C++ does not have automatic casting to/from void pointers, so need
explicit cast if header is to be included in C++ code

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/graph/rte_graph_worker.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
index eef77f732a..0c0b9c095a 100644
--- a/lib/graph/rte_graph_worker.h
+++ b/lib/graph/rte_graph_worker.h
@@ -155,7 +155,7 @@ rte_graph_walk(struct rte_graph *graph)
 	 *	+-----+ <= cir_start + mask
 	 */
 	while (likely(head != graph->tail)) {
-		node = RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
+		node = (struct rte_node *)RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
 		RTE_ASSERT(node->fence == RTE_GRAPH_FENCE);
 		objs = node->objs;
 		rte_prefetch0(objs);
-- 
2.32.0

Re: [PATCH 3/7] graph: add explicit cast to allow C++ build

[Stephen Hemminger]

On Fri,  4 Feb 2022 17:42:05 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> C++ does not have automatic casting to/from void pointers, so need
> explicit cast if header is to be included in C++ code
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/graph/rte_graph_worker.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
> index eef77f732a..0c0b9c095a 100644
> --- a/lib/graph/rte_graph_worker.h
> +++ b/lib/graph/rte_graph_worker.h
> @@ -155,7 +155,7 @@ rte_graph_walk(struct rte_graph *graph)
>  	 *	+-----+ <= cir_start + mask
>  	 */
>  	while (likely(head != graph->tail)) {
> -		node = RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
> +		node = (struct rte_node *)RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
>  		RTE_ASSERT(node->fence == RTE_GRAPH_FENCE);
>  		objs = node->objs;
>  		rte_prefetch0(objs);

Having to do this raises the other question.
Why is this code inline? It should not be in critical path.

Re: [PATCH 3/7] graph: add explicit cast to allow C++ build

[Jerin Jacob]

On Fri, Feb 4, 2022 at 11:42 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Fri,  4 Feb 2022 17:42:05 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > C++ does not have automatic casting to/from void pointers, so need
> > explicit cast if header is to be included in C++ code
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/graph/rte_graph_worker.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
> > index eef77f732a..0c0b9c095a 100644
> > --- a/lib/graph/rte_graph_worker.h
> > +++ b/lib/graph/rte_graph_worker.h
> > @@ -155,7 +155,7 @@ rte_graph_walk(struct rte_graph *graph)
> >        *      +-----+ <= cir_start + mask
> >        */
> >       while (likely(head != graph->tail)) {
> > -             node = RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
> > +             node = (struct rte_node *)RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
> >               RTE_ASSERT(node->fence == RTE_GRAPH_FENCE);
> >               objs = node->objs;
> >               rte_prefetch0(objs);
>
> Having to do this raises the other question.
> Why is this code inline? It should not be in critical path.

rte_graph_walk() is a fastpath function.Slowpath and fasthpath
functions are separated by
rte_graph.h and rte_graph_worker.h

Acked-by: Jerin Jacob <jerinj@marvell.com>

[PATCH 4/7] ipsec: add explicit cast to allow C++ build

[Bruce Richardson]

C++ does not have automatic casting to/from void pointers, so need
explicit cast if header is to be included in C++ code

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/ipsec/rte_ipsec_group.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ipsec/rte_ipsec_group.h b/lib/ipsec/rte_ipsec_group.h
index 60ab297710..62c2bd7217 100644
--- a/lib/ipsec/rte_ipsec_group.h
+++ b/lib/ipsec/rte_ipsec_group.h
@@ -49,10 +49,10 @@ rte_ipsec_ses_from_crypto(const struct rte_crypto_op *cop)

 	if (cop->sess_type == RTE_CRYPTO_OP_SECURITY_SESSION) {
 		ss = cop->sym[0].sec_session;
-		return (void *)(uintptr_t)ss->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)ss->opaque_data;
 	} else if (cop->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
 		cs = cop->sym[0].session;
-		return (void *)(uintptr_t)cs->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)cs->opaque_data;
 	}
 	return NULL;
 }
--
2.32.0

[PATCH 5/7] table: add explicit casts for C++ compatibility

[Bruce Richardson]

Since C++ doesn't support automatic casting from void * to other types,
we need to explicitly add the casts to any header files in DPDK.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/table/rte_table_hash_func.h | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/table/rte_table_hash_func.h b/lib/table/rte_table_hash_func.h
index c4c35cc06a..a962ec2f68 100644
--- a/lib/table/rte_table_hash_func.h
+++ b/lib/table/rte_table_hash_func.h
@@ -58,8 +58,8 @@ static inline uint64_t
 rte_table_hash_crc_key8(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t crc0;
 
 	crc0 = rte_crc32_u64(seed, k[0] & m[0]);
@@ -72,8 +72,8 @@ static inline uint64_t
 rte_table_hash_crc_key16(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, crc0, crc1;
 
 	k0 = k[0] & m[0];
@@ -91,8 +91,8 @@ static inline uint64_t
 rte_table_hash_crc_key24(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1;
 
 	k0 = k[0] & m[0];
@@ -113,8 +113,8 @@ static inline uint64_t
 rte_table_hash_crc_key32(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -139,8 +139,8 @@ static inline uint64_t
 rte_table_hash_crc_key40(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -165,8 +165,8 @@ static inline uint64_t
 rte_table_hash_crc_key48(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -192,8 +192,8 @@ static inline uint64_t
 rte_table_hash_crc_key56(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
 
 	k0 = k[0] & m[0];
@@ -222,8 +222,8 @@ static inline uint64_t
 rte_table_hash_crc_key64(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
 
 	k0 = k[0] & m[0];
-- 
2.32.0

[PATCH 6/7] vhost: remove non-C++ compatible includes

[Bruce Richardson]

Some of the linux header includes are explicitly noted as being
incompatible with C++. However, these headers can included by C files
directly, or by internal headers, to avoid polluting the public DPDK
headers with non-C++ safe includes.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 2 ++
 drivers/vdpa/ifc/ifcvf_vdpa.c     | 2 ++
 drivers/vdpa/mlx5/mlx5_vdpa.h     | 1 +
 lib/vhost/rte_vhost.h             | 4 ----
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 070f0e6dfd..cd2afe3100 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -16,6 +16,8 @@
 #include <rte_kvargs.h>
 #include <rte_vhost.h>
 #include <rte_spinlock.h>
+#include <linux/vhost.h>
+#include <linux/virtio_net.h>

 #include "rte_eth_vhost.h"

diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
index 3853c4cf7e..cbd190ea8d 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -24,6 +24,8 @@
 #include <rte_kvargs.h>
 #include <rte_devargs.h>

+#include <linux/vhost_types.h>
+
 #include "base/ifcvf.h"

 RTE_LOG_REGISTER(ifcvf_vdpa_logtype, pmd.vdpa.ifcvf, NOTICE);
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.h b/drivers/vdpa/mlx5/mlx5_vdpa.h
index 22617924ea..a7fa4356dd 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa.h
+++ b/drivers/vdpa/mlx5/mlx5_vdpa.h
@@ -6,6 +6,7 @@
 #define RTE_PMD_MLX5_VDPA_H_

 #include <linux/virtio_net.h>
+#include <linux/vhost_types.h>
 #include <sys/queue.h>

 #ifdef PEDANTIC
diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index b454c05868..0a21177199 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -21,10 +21,6 @@
 extern "C" {
 #endif

-/* These are not C++-aware. */
-#include <linux/vhost.h>
-#include <linux/virtio_ring.h>
-#include <linux/virtio_net.h>

 #define RTE_VHOST_USER_CLIENT		(1ULL << 0)
 #define RTE_VHOST_USER_NO_RECONNECT	(1ULL << 1)
--
2.32.0

Re: [PATCH 6/7] vhost: remove non-C++ compatible includes

[Bruce Richardson]

On Fri, Feb 04, 2022 at 05:42:08PM +0000, Bruce Richardson wrote:
> Some of the linux header includes are explicitly noted as being
> incompatible with C++. However, these headers can included by C files
> directly, or by internal headers, to avoid polluting the public DPDK
> headers with non-C++ safe includes.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

CI is reporting build issues with this patch on examples, something I'm not
surprised to see. I will wait for maintainer feedback on best approach
before respinning patchset.

/Bruce

RE: [PATCH 6/7] vhost: remove non-C++ compatible includes

[Wang, Xiao W]

Hi Bruce,

> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Saturday, February 5, 2022 2:19 AM
> To: dev@dpdk.org
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> <chenbo.xia@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>; Matan
> Azrad <matan@nvidia.com>; Viacheslav Ovsiienko
> <viacheslavo@nvidia.com>
> Subject: Re: [PATCH 6/7] vhost: remove non-C++ compatible includes
> 
> On Fri, Feb 04, 2022 at 05:42:08PM +0000, Bruce Richardson wrote:
> > Some of the linux header includes are explicitly noted as being
> > incompatible with C++. However, these headers can included by C files
> > directly, or by internal headers, to avoid polluting the public DPDK
> > headers with non-C++ safe includes.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> 
> CI is reporting build issues with this patch on examples, something I'm not
> surprised to see. I will wait for maintainer feedback on best approach
> before respinning patchset.
> 
> /Bruce

Could we move these c++ incompatible linux headers into
#ifndef __cplusplus
...
#endif.
Then we just need to change rte_vhost.h file, and don't break build for the drivers and samples.

BRs,
Xiao

Re: [PATCH 6/7] vhost: remove non-C++ compatible includes

[Bruce Richardson]

On Wed, Feb 09, 2022 at 09:10:36AM +0000, Wang, Xiao W wrote:
> Hi Bruce,
> 
> > -----Original Message----- From: Richardson, Bruce
> > <bruce.richardson@intel.com> Sent: Saturday, February 5, 2022 2:19 AM
> > To: dev@dpdk.org Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia,
> > Chenbo <chenbo.xia@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>;
> > Matan Azrad <matan@nvidia.com>; Viacheslav Ovsiienko
> > <viacheslavo@nvidia.com> Subject: Re: [PATCH 6/7] vhost: remove non-C++
> > compatible includes
> >
> > On Fri, Feb 04, 2022 at 05:42:08PM +0000, Bruce Richardson wrote:
> > > Some of the linux header includes are explicitly noted as being
> > > incompatible with C++. However, these headers can included by C files
> > > directly, or by internal headers, to avoid polluting the public DPDK
> > > headers with non-C++ safe includes.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
> >
> > CI is reporting build issues with this patch on examples, something I'm
> > not surprised to see. I will wait for maintainer feedback on best
> > approach before respinning patchset.
> >
> > /Bruce
> 
> Could we move these c++ incompatible linux headers into #ifndef
> __cplusplus ...  #endif.  Then we just need to change rte_vhost.h file,
> and don't break build for the drivers and samples.
>
I was thinking that something similar would work, but it's not the most
elegant, and strikes me as a bit of a hack. If it's generally acceptable,
though, I'm happy enough to respin the patch with that fix.

/Bruce

[PATCH 7/7] buildtools/chkincs: test headers for C++ compatibility

[Bruce Richardson]

Add support for checking each of our headers for issues when included in
a C++ file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/chkincs/main.cpp    |  4 ++++
 buildtools/chkincs/meson.build | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 buildtools/chkincs/main.cpp

diff --git a/buildtools/chkincs/main.cpp b/buildtools/chkincs/main.cpp
new file mode 100644
index 0000000000..d25bb8852a
--- /dev/null
+++ b/buildtools/chkincs/main.cpp
@@ -0,0 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+int main(void) { return 0; }
diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 5ffca89761..beabcd55d8 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -28,3 +28,23 @@ executable('chkincs', sources,
         dependencies: deps,
         link_whole: dpdk_static_libraries + dpdk_drivers,
         install: false)
+
+# run tests for c++ builds also
+if not add_languages('cpp', required: false)
+    subdir_done()
+endif
+
+gen_cpp_files = generator(gen_c_file_for_header,
+        output: '@BASENAME@.cpp',
+        arguments: ['@INPUT@', '@OUTPUT@'])
+
+cpp_sources = files('main.cpp')
+cpp_sources += gen_cpp_files.process(dpdk_chkinc_headers)
+
+executable('chkincs-cpp', cpp_sources,
+        cpp_args: ['-include', 'rte_config.h', cflags],
+        link_args: dpdk_extra_ldflags,
+        include_directories: includes,
+        dependencies: deps,
+        link_whole: dpdk_static_libraries + dpdk_drivers,
+        install: false)
-- 
2.32.0

[PATCH v2 0/7] Verify C++ compatibility of public headers

[Bruce Richardson]

This patchset expands upon the exiting chkincs infrastructure to build a
C++ test application as well as the existing C test app, which helps
catch any build errors when the public DPDK headers are included in C++
code. The rest of this patchset fixes the errors caught by this
additional check - mostly missing casts, but also one use of "new" as a
variable name, and one missing closing brace for an 'extern "C"' block.

V2:
* Changed patch 6 to have a minimally-invasive fix for C++ without
  affecting existing C code.
* Added fixlines and Cc stable for all patches which are fixes

Bruce Richardson (7):
  eal: fix header build with C++
  eventdev: fix header build with C++
  graph: fix missing explicit cast for C++ build
  ipsec: fix missing explicit cast for C++ build
  table: fix missing explicit casts for C++ build
  vhost: fix incompatible header includes for C++
  buildtools/chkincs: test headers for C++ compatibility

 buildtools/chkincs/main.cpp              |  4 +++
 buildtools/chkincs/meson.build           | 20 +++++++++++++++
 lib/eal/include/generic/rte_ticketlock.h | 14 +++++------
 lib/eal/include/rte_trace_point.h        |  2 +-
 lib/eventdev/rte_event_timer_adapter.h   |  3 +++
 lib/eventdev/rte_eventdev.h              |  2 +-
 lib/graph/rte_graph_worker.h             |  2 +-
 lib/ipsec/rte_ipsec_group.h              |  4 +--
 lib/table/rte_table_hash_func.h          | 32 ++++++++++++------------
 lib/vhost/rte_vhost.h                    |  2 ++
 10 files changed, 57 insertions(+), 28 deletions(-)
 create mode 100644 buildtools/chkincs/main.cpp

--
2.32.0

[PATCH v2 1/7] eal: fix header build with C++

[Bruce Richardson]

C++ files could not include some headers because:

* "new" is a keyword in C++, so can't be a variable name
* there is no automatic casting to/from void *

Fixes: 184104fc6121 ("ticketlock: introduce fair ticket based locking")
Fixes: ebaee6409702 ("trace: simplify trace point headers")
Cc: joyce.kong@arm.com
Cc: david.marchand@redhat.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/include/generic/rte_ticketlock.h | 14 +++++++-------
 lib/eal/include/rte_trace_point.h        |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/eal/include/generic/rte_ticketlock.h b/lib/eal/include/generic/rte_ticketlock.h
index c1b8808f51..693c67b517 100644
--- a/lib/eal/include/generic/rte_ticketlock.h
+++ b/lib/eal/include/generic/rte_ticketlock.h
@@ -91,13 +91,13 @@ rte_ticketlock_unlock(rte_ticketlock_t *tl)
 static inline int
 rte_ticketlock_trylock(rte_ticketlock_t *tl)
 {
-	rte_ticketlock_t old, new;
-	old.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
-	new.tickets = old.tickets;
-	new.s.next++;
-	if (old.s.next == old.s.current) {
-		if (__atomic_compare_exchange_n(&tl->tickets, &old.tickets,
-		    new.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
+	rte_ticketlock_t oldl, newl;
+	oldl.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
+	newl.tickets = oldl.tickets;
+	newl.s.next++;
+	if (oldl.s.next == oldl.s.current) {
+		if (__atomic_compare_exchange_n(&tl->tickets, &oldl.tickets,
+		    newl.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
 			return 1;
 	}
 
diff --git a/lib/eal/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
index e226f073f7..0f8700974f 100644
--- a/lib/eal/include/rte_trace_point.h
+++ b/lib/eal/include/rte_trace_point.h
@@ -370,7 +370,7 @@ do { \
 do { \
 	if (unlikely(in == NULL)) \
 		return; \
-	rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
+	rte_strscpy((char *)mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 } while (0)
 
-- 
2.32.0

Re: [PATCH v2 1/7] eal: fix header build with C++

[David Marchand]

On Thu, Feb 10, 2022 at 1:03 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> C++ files could not include some headers because:
>
> * "new" is a keyword in C++, so can't be a variable name
> * there is no automatic casting to/from void *
>
> Fixes: 184104fc6121 ("ticketlock: introduce fair ticket based locking")
> Fixes: ebaee6409702 ("trace: simplify trace point headers")

rte_strcpy was moved in this commit.
This is more about:
Fixes: 032a7e5499a0 ("trace: implement provider payload")

> Cc: joyce.kong@arm.com
> Cc: david.marchand@redhat.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/eal/include/generic/rte_ticketlock.h | 14 +++++++-------
>  lib/eal/include/rte_trace_point.h        |  2 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/lib/eal/include/generic/rte_ticketlock.h b/lib/eal/include/generic/rte_ticketlock.h
> index c1b8808f51..693c67b517 100644
> --- a/lib/eal/include/generic/rte_ticketlock.h
> +++ b/lib/eal/include/generic/rte_ticketlock.h
> @@ -91,13 +91,13 @@ rte_ticketlock_unlock(rte_ticketlock_t *tl)
>  static inline int
>  rte_ticketlock_trylock(rte_ticketlock_t *tl)
>  {
> -       rte_ticketlock_t old, new;
> -       old.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
> -       new.tickets = old.tickets;
> -       new.s.next++;
> -       if (old.s.next == old.s.current) {
> -               if (__atomic_compare_exchange_n(&tl->tickets, &old.tickets,
> -                   new.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
> +       rte_ticketlock_t oldl, newl;
> +       oldl.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
> +       newl.tickets = oldl.tickets;
> +       newl.s.next++;
> +       if (oldl.s.next == oldl.s.current) {
> +               if (__atomic_compare_exchange_n(&tl->tickets, &oldl.tickets,
> +                   newl.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
>                         return 1;
>         }
>
> diff --git a/lib/eal/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
> index e226f073f7..0f8700974f 100644
> --- a/lib/eal/include/rte_trace_point.h
> +++ b/lib/eal/include/rte_trace_point.h
> @@ -370,7 +370,7 @@ do { \
>  do { \
>         if (unlikely(in == NULL)) \
>                 return; \
> -       rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
> +       rte_strscpy((char *)mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
>         mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
>  } while (0)
>
> --
> 2.32.0
>

lgtm, thanks.


-- 
David Marchand

Re: [PATCH v2 1/7] eal: fix header build with C++

[Bruce Richardson]

On Thu, Feb 10, 2022 at 01:57:09PM +0100, David Marchand wrote:
> On Thu, Feb 10, 2022 at 1:03 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > C++ files could not include some headers because:
> >
> > * "new" is a keyword in C++, so can't be a variable name
> > * there is no automatic casting to/from void *
> >
> > Fixes: 184104fc6121 ("ticketlock: introduce fair ticket based locking")
> > Fixes: ebaee6409702 ("trace: simplify trace point headers")
> 
> rte_strcpy was moved in this commit.
> This is more about:
> Fixes: 032a7e5499a0 ("trace: implement provider payload")
> 
Thank you, I missed that even though I did some searching beyond the commit
I referenced, which I suspected was only moving things!
Unfortunately, I didn't see this before I did up a v3 to fix the CI issues.
If necessary I'll do a v4 to correct, once I check there are no other CI
problems.

/Bruce

[PATCH v2 2/7] eventdev: fix header build with C++

[Bruce Richardson]

The eventdev headers had issues when used from C++

* Missing closing "}" for the extern "C" block
* No automatic casting to/from void *

Fixes: a6562f6d6f8e ("eventdev: introduce event timer adapter")
Fixes: 32e326869ed6 ("eventdev: add tracepoints")
Cc: erik.g.carrillo@intel.com
Cc: skori@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 lib/eventdev/rte_event_timer_adapter.h | 3 +++
 lib/eventdev/rte_eventdev.h            | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index 1551741820..1fe4dd8e8f 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -678,4 +678,7 @@ rte_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
 	return adapter->cancel_burst(adapter, evtims, nb_evtims);
 }
 
+#ifdef __cplusplus
+}
+#endif
 #endif /* __RTE_EVENT_TIMER_ADAPTER_H__ */
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index eef47d8acc..25fb7c89dd 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -1805,7 +1805,7 @@ __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
 		return 0;
 	}
 #endif
-	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, fn);
+	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, (void *)fn);
 	/*
 	 * Allow zero cost non burst mode routine invocation if application
 	 * requests nb_events as const one
-- 
2.32.0

[PATCH v2 3/7] graph: fix missing explicit cast for C++ build

[Bruce Richardson]

C++ does not have automatic casting to/from void pointers, so need
explicit cast if header is to be included in C++ code

Fixes: 40d4f51403ec ("graph: implement fastpath routines")
Cc: jerinj@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 lib/graph/rte_graph_worker.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
index eef77f732a..0c0b9c095a 100644
--- a/lib/graph/rte_graph_worker.h
+++ b/lib/graph/rte_graph_worker.h
@@ -155,7 +155,7 @@ rte_graph_walk(struct rte_graph *graph)
 	 *	+-----+ <= cir_start + mask
 	 */
 	while (likely(head != graph->tail)) {
-		node = RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
+		node = (struct rte_node *)RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
 		RTE_ASSERT(node->fence == RTE_GRAPH_FENCE);
 		objs = node->objs;
 		rte_prefetch0(objs);
-- 
2.32.0

[PATCH v2 4/7] ipsec: fix missing explicit cast for C++ build

[Bruce Richardson]

C++ does not have automatic casting to/from void pointers, so need
explicit cast if header is to be included in C++ code

Fixes: f901d9c82688 ("ipsec: add helpers to group completed crypto-ops")
Cc: konstantin.ananyev@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/ipsec/rte_ipsec_group.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ipsec/rte_ipsec_group.h b/lib/ipsec/rte_ipsec_group.h
index 60ab297710..62c2bd7217 100644
--- a/lib/ipsec/rte_ipsec_group.h
+++ b/lib/ipsec/rte_ipsec_group.h
@@ -49,10 +49,10 @@ rte_ipsec_ses_from_crypto(const struct rte_crypto_op *cop)
 
 	if (cop->sess_type == RTE_CRYPTO_OP_SECURITY_SESSION) {
 		ss = cop->sym[0].sec_session;
-		return (void *)(uintptr_t)ss->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)ss->opaque_data;
 	} else if (cop->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
 		cs = cop->sym[0].session;
-		return (void *)(uintptr_t)cs->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)cs->opaque_data;
 	}
 	return NULL;
 }
-- 
2.32.0

RE: [PATCH v2 4/7] ipsec: fix missing explicit cast for C++ build

[Ananyev, Konstantin]

> C++ does not have automatic casting to/from void pointers, so need
> explicit cast if header is to be included in C++ code
> 
> Fixes: f901d9c82688 ("ipsec: add helpers to group completed crypto-ops")
> Cc: konstantin.ananyev@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/ipsec/rte_ipsec_group.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/ipsec/rte_ipsec_group.h b/lib/ipsec/rte_ipsec_group.h
> index 60ab297710..62c2bd7217 100644
> --- a/lib/ipsec/rte_ipsec_group.h
> +++ b/lib/ipsec/rte_ipsec_group.h
> @@ -49,10 +49,10 @@ rte_ipsec_ses_from_crypto(const struct rte_crypto_op *cop)
> 
>  	if (cop->sess_type == RTE_CRYPTO_OP_SECURITY_SESSION) {
>  		ss = cop->sym[0].sec_session;
> -		return (void *)(uintptr_t)ss->opaque_data;
> +		return (struct rte_ipsec_session *)(uintptr_t)ss->opaque_data;
>  	} else if (cop->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
>  		cs = cop->sym[0].session;
> -		return (void *)(uintptr_t)cs->opaque_data;
> +		return (struct rte_ipsec_session *)(uintptr_t)cs->opaque_data;
>  	}
>  	return NULL;
>  }
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.32.0

[PATCH v2 5/7] table: fix missing explicit casts for C++ build

[Bruce Richardson]

Since C++ doesn't support automatic casting from void * to other types,
we need to explicitly add the casts to any header files in DPDK.

Fixes: ea7be0a0386e ("lib/librte_table: add hash function headers")
Cc: kevin.laatz@intel.com
Cc: cristian.dumitrescu@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/table/rte_table_hash_func.h | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/table/rte_table_hash_func.h b/lib/table/rte_table_hash_func.h
index c4c35cc06a..a962ec2f68 100644
--- a/lib/table/rte_table_hash_func.h
+++ b/lib/table/rte_table_hash_func.h
@@ -58,8 +58,8 @@ static inline uint64_t
 rte_table_hash_crc_key8(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t crc0;
 
 	crc0 = rte_crc32_u64(seed, k[0] & m[0]);
@@ -72,8 +72,8 @@ static inline uint64_t
 rte_table_hash_crc_key16(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, crc0, crc1;
 
 	k0 = k[0] & m[0];
@@ -91,8 +91,8 @@ static inline uint64_t
 rte_table_hash_crc_key24(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1;
 
 	k0 = k[0] & m[0];
@@ -113,8 +113,8 @@ static inline uint64_t
 rte_table_hash_crc_key32(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -139,8 +139,8 @@ static inline uint64_t
 rte_table_hash_crc_key40(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -165,8 +165,8 @@ static inline uint64_t
 rte_table_hash_crc_key48(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -192,8 +192,8 @@ static inline uint64_t
 rte_table_hash_crc_key56(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
 
 	k0 = k[0] & m[0];
@@ -222,8 +222,8 @@ static inline uint64_t
 rte_table_hash_crc_key64(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
 
 	k0 = k[0] & m[0];
-- 
2.32.0

[PATCH v2 6/7] vhost: fix incompatible header includes for C++

[Bruce Richardson]

The virtio kernel header includes are already noted as being
incompatible with C++. We can ensure that the header is safe for
inclusion in C++ code by not including those headers during C++ builds.
While not ideal, this does ensure that all DPDK headers can be included
in C++ code without errors.

Fixes: f8904d563691 ("vhost: fix header for strict compilation flags")
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/vhost/rte_vhost.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index b454c05868..2acb31df2d 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -21,10 +21,12 @@
 extern "C" {
 #endif
 
+#ifndef __cplusplus
 /* These are not C++-aware. */
 #include <linux/vhost.h>
 #include <linux/virtio_ring.h>
 #include <linux/virtio_net.h>
+#endif
 
 #define RTE_VHOST_USER_CLIENT		(1ULL << 0)
 #define RTE_VHOST_USER_NO_RECONNECT	(1ULL << 1)
-- 
2.32.0

[PATCH v2 7/7] buildtools/chkincs: test headers for C++ compatibility

[Bruce Richardson]

Add support for checking each of our headers for issues when included in
a C++ file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/chkincs/main.cpp    |  4 ++++
 buildtools/chkincs/meson.build | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 buildtools/chkincs/main.cpp

diff --git a/buildtools/chkincs/main.cpp b/buildtools/chkincs/main.cpp
new file mode 100644
index 0000000000..d25bb8852a
--- /dev/null
+++ b/buildtools/chkincs/main.cpp
@@ -0,0 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+int main(void) { return 0; }
diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 5ffca89761..beabcd55d8 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -28,3 +28,23 @@ executable('chkincs', sources,
         dependencies: deps,
         link_whole: dpdk_static_libraries + dpdk_drivers,
         install: false)
+
+# run tests for c++ builds also
+if not add_languages('cpp', required: false)
+    subdir_done()
+endif
+
+gen_cpp_files = generator(gen_c_file_for_header,
+        output: '@BASENAME@.cpp',
+        arguments: ['@INPUT@', '@OUTPUT@'])
+
+cpp_sources = files('main.cpp')
+cpp_sources += gen_cpp_files.process(dpdk_chkinc_headers)
+
+executable('chkincs-cpp', cpp_sources,
+        cpp_args: ['-include', 'rte_config.h', cflags],
+        link_args: dpdk_extra_ldflags,
+        include_directories: includes,
+        dependencies: deps,
+        link_whole: dpdk_static_libraries + dpdk_drivers,
+        install: false)
-- 
2.32.0

[PATCH v3 0/7] Verify C++ compatibility of public headers

[Bruce Richardson]

This patchset expands upon the exiting chkincs infrastructure to build a
C++ test application as well as the existing C test app, which helps
catch any build errors when the public DPDK headers are included in C++
code. The rest of this patchset fixes the errors caught by this
additional check - mostly missing casts, but also one use of "new" as a
variable name, and one missing closing brace for an 'extern "C"' block.

V3:
* update linux-build script to have cpp_args=-m32 for 32-bit builds

V2:
* Changed patch 6 to have a minimally-invasive fix for C++ without
  affecting existing C code.
* Added fixlines and Cc stable for all patches which are fixes

Bruce Richardson (7):
  eal: fix header build with C++
  eventdev: fix header build with C++
  graph: fix missing explicit cast for C++ build
  ipsec: fix missing explicit cast for C++ build
  table: fix missing explicit casts for C++ build
  vhost: fix incompatible header includes for C++
  buildtools/chkincs: test headers for C++ compatibility

 .ci/linux-build.sh                       |  1 +
 buildtools/chkincs/main.cpp              |  4 +++
 buildtools/chkincs/meson.build           | 20 +++++++++++++++
 lib/eal/include/generic/rte_ticketlock.h | 14 +++++------
 lib/eal/include/rte_trace_point.h        |  2 +-
 lib/eventdev/rte_event_timer_adapter.h   |  3 +++
 lib/eventdev/rte_eventdev.h              |  2 +-
 lib/graph/rte_graph_worker.h             |  2 +-
 lib/ipsec/rte_ipsec_group.h              |  4 +--
 lib/table/rte_table_hash_func.h          | 32 ++++++++++++------------
 lib/vhost/rte_vhost.h                    |  2 ++
 11 files changed, 58 insertions(+), 28 deletions(-)
 create mode 100644 buildtools/chkincs/main.cpp

--
2.32.0

[PATCH v3 1/7] eal: fix header build with C++

[Bruce Richardson]

C++ files could not include some headers because:

* "new" is a keyword in C++, so can't be a variable name
* there is no automatic casting to/from void *

Fixes: 184104fc6121 ("ticketlock: introduce fair ticket based locking")
Fixes: ebaee6409702 ("trace: simplify trace point headers")
Cc: joyce.kong@arm.com
Cc: david.marchand@redhat.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/include/generic/rte_ticketlock.h | 14 +++++++-------
 lib/eal/include/rte_trace_point.h        |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/eal/include/generic/rte_ticketlock.h b/lib/eal/include/generic/rte_ticketlock.h
index c1b8808f51..693c67b517 100644
--- a/lib/eal/include/generic/rte_ticketlock.h
+++ b/lib/eal/include/generic/rte_ticketlock.h
@@ -91,13 +91,13 @@ rte_ticketlock_unlock(rte_ticketlock_t *tl)
 static inline int
 rte_ticketlock_trylock(rte_ticketlock_t *tl)
 {
-	rte_ticketlock_t old, new;
-	old.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
-	new.tickets = old.tickets;
-	new.s.next++;
-	if (old.s.next == old.s.current) {
-		if (__atomic_compare_exchange_n(&tl->tickets, &old.tickets,
-		    new.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
+	rte_ticketlock_t oldl, newl;
+	oldl.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
+	newl.tickets = oldl.tickets;
+	newl.s.next++;
+	if (oldl.s.next == oldl.s.current) {
+		if (__atomic_compare_exchange_n(&tl->tickets, &oldl.tickets,
+		    newl.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
 			return 1;
 	}
 
diff --git a/lib/eal/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
index e226f073f7..0f8700974f 100644
--- a/lib/eal/include/rte_trace_point.h
+++ b/lib/eal/include/rte_trace_point.h
@@ -370,7 +370,7 @@ do { \
 do { \
 	if (unlikely(in == NULL)) \
 		return; \
-	rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
+	rte_strscpy((char *)mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 } while (0)
 
-- 
2.32.0

[PATCH v3 2/7] eventdev: fix header build with C++

[Bruce Richardson]

The eventdev headers had issues when used from C++

* Missing closing "}" for the extern "C" block
* No automatic casting to/from void *

Fixes: a6562f6d6f8e ("eventdev: introduce event timer adapter")
Fixes: 32e326869ed6 ("eventdev: add tracepoints")
Cc: erik.g.carrillo@intel.com
Cc: skori@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 lib/eventdev/rte_event_timer_adapter.h | 3 +++
 lib/eventdev/rte_eventdev.h            | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index 1551741820..1fe4dd8e8f 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -678,4 +678,7 @@ rte_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
 	return adapter->cancel_burst(adapter, evtims, nb_evtims);
 }
 
+#ifdef __cplusplus
+}
+#endif
 #endif /* __RTE_EVENT_TIMER_ADAPTER_H__ */
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index eef47d8acc..25fb7c89dd 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -1805,7 +1805,7 @@ __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
 		return 0;
 	}
 #endif
-	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, fn);
+	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, (void *)fn);
 	/*
 	 * Allow zero cost non burst mode routine invocation if application
 	 * requests nb_events as const one
-- 
2.32.0

[PATCH v3 3/7] graph: fix missing explicit cast for C++ build

[Bruce Richardson]

C++ does not have automatic casting to/from void pointers, so need
explicit cast if header is to be included in C++ code

Fixes: 40d4f51403ec ("graph: implement fastpath routines")
Cc: jerinj@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 lib/graph/rte_graph_worker.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
index eef77f732a..0c0b9c095a 100644
--- a/lib/graph/rte_graph_worker.h
+++ b/lib/graph/rte_graph_worker.h
@@ -155,7 +155,7 @@ rte_graph_walk(struct rte_graph *graph)
 	 *	+-----+ <= cir_start + mask
 	 */
 	while (likely(head != graph->tail)) {
-		node = RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
+		node = (struct rte_node *)RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
 		RTE_ASSERT(node->fence == RTE_GRAPH_FENCE);
 		objs = node->objs;
 		rte_prefetch0(objs);
-- 
2.32.0

[PATCH v3 4/7] ipsec: fix missing explicit cast for C++ build

[Bruce Richardson]

C++ does not have automatic casting to/from void pointers, so need
explicit cast if header is to be included in C++ code

Fixes: f901d9c82688 ("ipsec: add helpers to group completed crypto-ops")
Cc: konstantin.ananyev@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/ipsec/rte_ipsec_group.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ipsec/rte_ipsec_group.h b/lib/ipsec/rte_ipsec_group.h
index 60ab297710..62c2bd7217 100644
--- a/lib/ipsec/rte_ipsec_group.h
+++ b/lib/ipsec/rte_ipsec_group.h
@@ -49,10 +49,10 @@ rte_ipsec_ses_from_crypto(const struct rte_crypto_op *cop)
 
 	if (cop->sess_type == RTE_CRYPTO_OP_SECURITY_SESSION) {
 		ss = cop->sym[0].sec_session;
-		return (void *)(uintptr_t)ss->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)ss->opaque_data;
 	} else if (cop->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
 		cs = cop->sym[0].session;
-		return (void *)(uintptr_t)cs->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)cs->opaque_data;
 	}
 	return NULL;
 }
-- 
2.32.0

[PATCH v3 5/7] table: fix missing explicit casts for C++ build

[Bruce Richardson]

Since C++ doesn't support automatic casting from void * to other types,
we need to explicitly add the casts to any header files in DPDK.

Fixes: ea7be0a0386e ("lib/librte_table: add hash function headers")
Cc: kevin.laatz@intel.com
Cc: cristian.dumitrescu@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/table/rte_table_hash_func.h | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/table/rte_table_hash_func.h b/lib/table/rte_table_hash_func.h
index c4c35cc06a..a962ec2f68 100644
--- a/lib/table/rte_table_hash_func.h
+++ b/lib/table/rte_table_hash_func.h
@@ -58,8 +58,8 @@ static inline uint64_t
 rte_table_hash_crc_key8(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t crc0;
 
 	crc0 = rte_crc32_u64(seed, k[0] & m[0]);
@@ -72,8 +72,8 @@ static inline uint64_t
 rte_table_hash_crc_key16(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, crc0, crc1;
 
 	k0 = k[0] & m[0];
@@ -91,8 +91,8 @@ static inline uint64_t
 rte_table_hash_crc_key24(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1;
 
 	k0 = k[0] & m[0];
@@ -113,8 +113,8 @@ static inline uint64_t
 rte_table_hash_crc_key32(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -139,8 +139,8 @@ static inline uint64_t
 rte_table_hash_crc_key40(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -165,8 +165,8 @@ static inline uint64_t
 rte_table_hash_crc_key48(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -192,8 +192,8 @@ static inline uint64_t
 rte_table_hash_crc_key56(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
 
 	k0 = k[0] & m[0];
@@ -222,8 +222,8 @@ static inline uint64_t
 rte_table_hash_crc_key64(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
 
 	k0 = k[0] & m[0];
-- 
2.32.0

[PATCH v3 6/7] vhost: fix incompatible header includes for C++

[Bruce Richardson]

The virtio kernel header includes are already noted as being
incompatible with C++. We can ensure that the header is safe for
inclusion in C++ code by not including those headers during C++ builds.
While not ideal, this does ensure that all DPDK headers can be included
in C++ code without errors.

Fixes: f8904d563691 ("vhost: fix header for strict compilation flags")
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/vhost/rte_vhost.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index b454c05868..2acb31df2d 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -21,10 +21,12 @@
 extern "C" {
 #endif
 
+#ifndef __cplusplus
 /* These are not C++-aware. */
 #include <linux/vhost.h>
 #include <linux/virtio_ring.h>
 #include <linux/virtio_net.h>
+#endif
 
 #define RTE_VHOST_USER_CLIENT		(1ULL << 0)
 #define RTE_VHOST_USER_NO_RECONNECT	(1ULL << 1)
-- 
2.32.0

[PATCH v3 7/7] buildtools/chkincs: test headers for C++ compatibility

[Bruce Richardson]

Add support for checking each of our headers for issues when included in
a C++ file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 .ci/linux-build.sh             |  1 +
 buildtools/chkincs/main.cpp    |  4 ++++
 buildtools/chkincs/meson.build | 20 ++++++++++++++++++++
 3 files changed, 25 insertions(+)
 create mode 100644 buildtools/chkincs/main.cpp

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index c10c1a8ab5..67d68535e0 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -74,6 +74,7 @@ fi
 
 if [ "$BUILD_32BIT" = "true" ]; then
     OPTS="$OPTS -Dc_args=-m32 -Dc_link_args=-m32"
+    OPTS="$OPTS -Dcpp_args=-m32 -Dcpp_link_args=-m32"
     export PKG_CONFIG_LIBDIR="/usr/lib32/pkgconfig"
 fi
 
diff --git a/buildtools/chkincs/main.cpp b/buildtools/chkincs/main.cpp
new file mode 100644
index 0000000000..d25bb8852a
--- /dev/null
+++ b/buildtools/chkincs/main.cpp
@@ -0,0 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+int main(void) { return 0; }
diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 5ffca89761..beabcd55d8 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -28,3 +28,23 @@ executable('chkincs', sources,
         dependencies: deps,
         link_whole: dpdk_static_libraries + dpdk_drivers,
         install: false)
+
+# run tests for c++ builds also
+if not add_languages('cpp', required: false)
+    subdir_done()
+endif
+
+gen_cpp_files = generator(gen_c_file_for_header,
+        output: '@BASENAME@.cpp',
+        arguments: ['@INPUT@', '@OUTPUT@'])
+
+cpp_sources = files('main.cpp')
+cpp_sources += gen_cpp_files.process(dpdk_chkinc_headers)
+
+executable('chkincs-cpp', cpp_sources,
+        cpp_args: ['-include', 'rte_config.h', cflags],
+        link_args: dpdk_extra_ldflags,
+        include_directories: includes,
+        dependencies: deps,
+        link_whole: dpdk_static_libraries + dpdk_drivers,
+        install: false)
-- 
2.32.0

Re: [PATCH v3 7/7] buildtools/chkincs: test headers for C++ compatibility

[David Marchand]

On Thu, Feb 10, 2022 at 3:05 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> Add support for checking each of our headers for issues when included in
> a C++ file.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  .ci/linux-build.sh             |  1 +
>  buildtools/chkincs/main.cpp    |  4 ++++
>  buildtools/chkincs/meson.build | 20 ++++++++++++++++++++
>  3 files changed, 25 insertions(+)
>  create mode 100644 buildtools/chkincs/main.cpp
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index c10c1a8ab5..67d68535e0 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -74,6 +74,7 @@ fi
>
>  if [ "$BUILD_32BIT" = "true" ]; then
>      OPTS="$OPTS -Dc_args=-m32 -Dc_link_args=-m32"
> +    OPTS="$OPTS -Dcpp_args=-m32 -Dcpp_link_args=-m32"
>      export PKG_CONFIG_LIBDIR="/usr/lib32/pkgconfig"
>  fi
>

Even though check_includes=true is not forced for 32bits in
test-meson-builds.sh, I sometimes enable more checks for some targets.
Can this change be applied to test-meson-builds.sh too?

https://git.dpdk.org/dpdk/tree/devtools/test-meson-builds.sh#n249


-- 
David Marchand

Re: [PATCH v3 7/7] buildtools/chkincs: test headers for C++ compatibility

[Bruce Richardson]

On Thu, Feb 10, 2022 at 03:37:04PM +0100, David Marchand wrote:
> On Thu, Feb 10, 2022 at 3:05 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > Add support for checking each of our headers for issues when included in
> > a C++ file.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  .ci/linux-build.sh             |  1 +
> >  buildtools/chkincs/main.cpp    |  4 ++++
> >  buildtools/chkincs/meson.build | 20 ++++++++++++++++++++
> >  3 files changed, 25 insertions(+)
> >  create mode 100644 buildtools/chkincs/main.cpp
> >
> > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> > index c10c1a8ab5..67d68535e0 100755
> > --- a/.ci/linux-build.sh
> > +++ b/.ci/linux-build.sh
> > @@ -74,6 +74,7 @@ fi
> >
> >  if [ "$BUILD_32BIT" = "true" ]; then
> >      OPTS="$OPTS -Dc_args=-m32 -Dc_link_args=-m32"
> > +    OPTS="$OPTS -Dcpp_args=-m32 -Dcpp_link_args=-m32"
> >      export PKG_CONFIG_LIBDIR="/usr/lib32/pkgconfig"
> >  fi
> >
> 
> Even though check_includes=true is not forced for 32bits in
> test-meson-builds.sh, I sometimes enable more checks for some targets.
> Can this change be applied to test-meson-builds.sh too?
> 
> https://git.dpdk.org/dpdk/tree/devtools/test-meson-builds.sh#n249
> 
Yes, will do.

[PATCH v4 0/7] Verify C++ compatibility of public headers

[Bruce Richardson]

This patchset expands upon the exiting chkincs infrastructure to build a
C++ test application as well as the existing C test app, which helps
catch any build errors when the public DPDK headers are included in C++
code. The rest of this patchset fixes the errors caught by this
additional check - mostly missing casts, but also one use of "new" as a
variable name, and one missing closing brace for an 'extern "C"' block.

V4:
* add g++-multilib to packages to install for github checks to ensure 32-bit
  builds work
* correct the fixline for patch 1
* add extra cpp_args and cpp_link_args for 32-bit builds in test-meson-builds
  so that includes checks can pass there if enabled.

V3:
* update linux-build script to have cpp_args=-m32 for 32-bit builds

V2:
* Changed patch 6 to have a minimally-invasive fix for C++ without
  affecting existing C code.
* Added fixlines and Cc stable for all patches which are fixes
                         *** SUBJECT HERE ***

*** BLURB HERE ***

Bruce Richardson (7):
  eal: fix header build with C++
  eventdev: fix header build with C++
  graph: fix missing explicit cast for C++ build
  ipsec: fix missing explicit cast for C++ build
  table: fix missing explicit casts for C++ build
  vhost: fix incompatible header includes for C++
  buildtools/chkincs: test headers for C++ compatibility

 .ci/linux-build.sh                       |  1 +
 .github/workflows/build.yml              |  2 +-
 buildtools/chkincs/main.cpp              |  4 +++
 buildtools/chkincs/meson.build           | 20 +++++++++++++++
 devtools/test-meson-builds.sh            |  3 ++-
 lib/eal/include/generic/rte_ticketlock.h | 14 +++++------
 lib/eal/include/rte_trace_point.h        |  2 +-
 lib/eventdev/rte_event_timer_adapter.h   |  3 +++
 lib/eventdev/rte_eventdev.h              |  2 +-
 lib/graph/rte_graph_worker.h             |  2 +-
 lib/ipsec/rte_ipsec_group.h              |  4 +--
 lib/table/rte_table_hash_func.h          | 32 ++++++++++++------------
 lib/vhost/rte_vhost.h                    |  2 ++
 13 files changed, 61 insertions(+), 30 deletions(-)
 create mode 100644 buildtools/chkincs/main.cpp

--
2.32.0

[PATCH v4 1/7] eal: fix header build with C++

[Bruce Richardson]

C++ files could not include some headers because:

* "new" is a keyword in C++, so can't be a variable name
* there is no automatic casting to/from void *

Fixes: 184104fc6121 ("ticketlock: introduce fair ticket based locking")
Fixes: 032a7e5499a0 ("trace: implement provider payload")
Cc: joyce.kong@arm.com
Cc: jerinj@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/include/generic/rte_ticketlock.h | 14 +++++++-------
 lib/eal/include/rte_trace_point.h        |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/eal/include/generic/rte_ticketlock.h b/lib/eal/include/generic/rte_ticketlock.h
index c1b8808f51..693c67b517 100644
--- a/lib/eal/include/generic/rte_ticketlock.h
+++ b/lib/eal/include/generic/rte_ticketlock.h
@@ -91,13 +91,13 @@ rte_ticketlock_unlock(rte_ticketlock_t *tl)
 static inline int
 rte_ticketlock_trylock(rte_ticketlock_t *tl)
 {
-	rte_ticketlock_t old, new;
-	old.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
-	new.tickets = old.tickets;
-	new.s.next++;
-	if (old.s.next == old.s.current) {
-		if (__atomic_compare_exchange_n(&tl->tickets, &old.tickets,
-		    new.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
+	rte_ticketlock_t oldl, newl;
+	oldl.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
+	newl.tickets = oldl.tickets;
+	newl.s.next++;
+	if (oldl.s.next == oldl.s.current) {
+		if (__atomic_compare_exchange_n(&tl->tickets, &oldl.tickets,
+		    newl.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
 			return 1;
 	}
 
diff --git a/lib/eal/include/rte_trace_point.h b/lib/eal/include/rte_trace_point.h
index e226f073f7..0f8700974f 100644
--- a/lib/eal/include/rte_trace_point.h
+++ b/lib/eal/include/rte_trace_point.h
@@ -370,7 +370,7 @@ do { \
 do { \
 	if (unlikely(in == NULL)) \
 		return; \
-	rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
+	rte_strscpy((char *)mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 	mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
 } while (0)
 
-- 
2.32.0

RE: [PATCH v4 1/7] eal: fix header build with C++

[Joyce Kong]

<snip>

> Subject: [PATCH v4 1/7] eal: fix header build with C++
>
> C++ files could not include some headers because:
>
> * "new" is a keyword in C++, so can't be a variable name
> * there is no automatic casting to/from void *
>
> Fixes: 184104fc6121 ("ticketlock: introduce fair ticket based locking")
> Fixes: 032a7e5499a0 ("trace: implement provider payload")
> Cc: joyce.kong@arm.com
> Cc: jerinj@marvell.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Joyce Kong <joyce.kong@arm.com>

> ---
>  lib/eal/include/generic/rte_ticketlock.h | 14 +++++++-------
>  lib/eal/include/rte_trace_point.h        |  2 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/lib/eal/include/generic/rte_ticketlock.h
> b/lib/eal/include/generic/rte_ticketlock.h
> index c1b8808f51..693c67b517 100644
> --- a/lib/eal/include/generic/rte_ticketlock.h
> +++ b/lib/eal/include/generic/rte_ticketlock.h
> @@ -91,13 +91,13 @@ rte_ticketlock_unlock(rte_ticketlock_t *tl)  static
> inline int  rte_ticketlock_trylock(rte_ticketlock_t *tl)  {
> -     rte_ticketlock_t old, new;
> -     old.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
> -     new.tickets = old.tickets;
> -     new.s.next++;
> -     if (old.s.next == old.s.current) {
> -             if (__atomic_compare_exchange_n(&tl->tickets, &old.tickets,
> -                 new.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
> +     rte_ticketlock_t oldl, newl;
> +     oldl.tickets = __atomic_load_n(&tl->tickets, __ATOMIC_RELAXED);
> +     newl.tickets = oldl.tickets;
> +     newl.s.next++;
> +     if (oldl.s.next == oldl.s.current) {
> +             if (__atomic_compare_exchange_n(&tl->tickets, &oldl.tickets,
> +                 newl.tickets, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))
>                       return 1;
>       }
>
> diff --git a/lib/eal/include/rte_trace_point.h
> b/lib/eal/include/rte_trace_point.h
> index e226f073f7..0f8700974f 100644
> --- a/lib/eal/include/rte_trace_point.h
> +++ b/lib/eal/include/rte_trace_point.h
> @@ -370,7 +370,7 @@ do { \
>  do { \
>       if (unlikely(in == NULL)) \
>               return; \
> -     rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \
> +     rte_strscpy((char *)mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX);
> \
>       mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX);
> \  } while (0)
>
> --
> 2.32.0

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH v4 2/7] eventdev: fix header build with C++

[Bruce Richardson]

The eventdev headers had issues when used from C++

* Missing closing "}" for the extern "C" block
* No automatic casting to/from void *

Fixes: a6562f6d6f8e ("eventdev: introduce event timer adapter")
Fixes: 32e326869ed6 ("eventdev: add tracepoints")
Cc: erik.g.carrillo@intel.com
Cc: skori@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 lib/eventdev/rte_event_timer_adapter.h | 3 +++
 lib/eventdev/rte_eventdev.h            | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index 1551741820..1fe4dd8e8f 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -678,4 +678,7 @@ rte_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
 	return adapter->cancel_burst(adapter, evtims, nb_evtims);
 }
 
+#ifdef __cplusplus
+}
+#endif
 #endif /* __RTE_EVENT_TIMER_ADAPTER_H__ */
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index eef47d8acc..25fb7c89dd 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -1805,7 +1805,7 @@ __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
 		return 0;
 	}
 #endif
-	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, fn);
+	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, (void *)fn);
 	/*
 	 * Allow zero cost non burst mode routine invocation if application
 	 * requests nb_events as const one
-- 
2.32.0

[PATCH v4 3/7] graph: fix missing explicit cast for C++ build

[Bruce Richardson]

C++ does not have automatic casting to/from void pointers, so need
explicit cast if header is to be included in C++ code

Fixes: 40d4f51403ec ("graph: implement fastpath routines")
Cc: jerinj@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 lib/graph/rte_graph_worker.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
index eef77f732a..0c0b9c095a 100644
--- a/lib/graph/rte_graph_worker.h
+++ b/lib/graph/rte_graph_worker.h
@@ -155,7 +155,7 @@ rte_graph_walk(struct rte_graph *graph)
 	 *	+-----+ <= cir_start + mask
 	 */
 	while (likely(head != graph->tail)) {
-		node = RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
+		node = (struct rte_node *)RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
 		RTE_ASSERT(node->fence == RTE_GRAPH_FENCE);
 		objs = node->objs;
 		rte_prefetch0(objs);
-- 
2.32.0

[PATCH v4 4/7] ipsec: fix missing explicit cast for C++ build

[Bruce Richardson]

C++ does not have automatic casting to/from void pointers, so need
explicit cast if header is to be included in C++ code

Fixes: f901d9c82688 ("ipsec: add helpers to group completed crypto-ops")
Cc: konstantin.ananyev@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 lib/ipsec/rte_ipsec_group.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ipsec/rte_ipsec_group.h b/lib/ipsec/rte_ipsec_group.h
index 60ab297710..62c2bd7217 100644
--- a/lib/ipsec/rte_ipsec_group.h
+++ b/lib/ipsec/rte_ipsec_group.h
@@ -49,10 +49,10 @@ rte_ipsec_ses_from_crypto(const struct rte_crypto_op *cop)
 
 	if (cop->sess_type == RTE_CRYPTO_OP_SECURITY_SESSION) {
 		ss = cop->sym[0].sec_session;
-		return (void *)(uintptr_t)ss->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)ss->opaque_data;
 	} else if (cop->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
 		cs = cop->sym[0].session;
-		return (void *)(uintptr_t)cs->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)cs->opaque_data;
 	}
 	return NULL;
 }
-- 
2.32.0

[PATCH v4 5/7] table: fix missing explicit casts for C++ build

[Bruce Richardson]

Since C++ doesn't support automatic casting from void * to other types,
we need to explicitly add the casts to any header files in DPDK.

Fixes: ea7be0a0386e ("lib/librte_table: add hash function headers")
Cc: kevin.laatz@intel.com
Cc: cristian.dumitrescu@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/table/rte_table_hash_func.h | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/table/rte_table_hash_func.h b/lib/table/rte_table_hash_func.h
index c4c35cc06a..a962ec2f68 100644
--- a/lib/table/rte_table_hash_func.h
+++ b/lib/table/rte_table_hash_func.h
@@ -58,8 +58,8 @@ static inline uint64_t
 rte_table_hash_crc_key8(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t crc0;
 
 	crc0 = rte_crc32_u64(seed, k[0] & m[0]);
@@ -72,8 +72,8 @@ static inline uint64_t
 rte_table_hash_crc_key16(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, crc0, crc1;
 
 	k0 = k[0] & m[0];
@@ -91,8 +91,8 @@ static inline uint64_t
 rte_table_hash_crc_key24(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1;
 
 	k0 = k[0] & m[0];
@@ -113,8 +113,8 @@ static inline uint64_t
 rte_table_hash_crc_key32(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -139,8 +139,8 @@ static inline uint64_t
 rte_table_hash_crc_key40(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -165,8 +165,8 @@ static inline uint64_t
 rte_table_hash_crc_key48(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3;
 
 	k0 = k[0] & m[0];
@@ -192,8 +192,8 @@ static inline uint64_t
 rte_table_hash_crc_key56(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
 
 	k0 = k[0] & m[0];
@@ -222,8 +222,8 @@ static inline uint64_t
 rte_table_hash_crc_key64(void *key, void *mask, __rte_unused uint32_t key_size,
 	uint64_t seed)
 {
-	uint64_t *k = key;
-	uint64_t *m = mask;
+	uint64_t *k = (uint64_t *)key;
+	uint64_t *m = (uint64_t *)mask;
 	uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
 
 	k0 = k[0] & m[0];
-- 
2.32.0

[PATCH v4 6/7] vhost: fix incompatible header includes for C++

[Bruce Richardson]

The virtio kernel header includes are already noted as being
incompatible with C++. We can ensure that the header is safe for
inclusion in C++ code by not including those headers during C++ builds.
While not ideal, this does ensure that all DPDK headers can be included
in C++ code without errors.

Fixes: f8904d563691 ("vhost: fix header for strict compilation flags")
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/vhost/rte_vhost.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index b454c05868..2acb31df2d 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -21,10 +21,12 @@
 extern "C" {
 #endif
 
+#ifndef __cplusplus
 /* These are not C++-aware. */
 #include <linux/vhost.h>
 #include <linux/virtio_ring.h>
 #include <linux/virtio_net.h>
+#endif
 
 #define RTE_VHOST_USER_CLIENT		(1ULL << 0)
 #define RTE_VHOST_USER_NO_RECONNECT	(1ULL << 1)
-- 
2.32.0

[PATCH v4 7/7] buildtools/chkincs: test headers for C++ compatibility

[Bruce Richardson]

Add support for checking each of our headers for issues when included in
a C++ file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 .ci/linux-build.sh             |  1 +
 .github/workflows/build.yml    |  2 +-
 buildtools/chkincs/main.cpp    |  4 ++++
 buildtools/chkincs/meson.build | 20 ++++++++++++++++++++
 devtools/test-meson-builds.sh  |  3 ++-
 5 files changed, 28 insertions(+), 2 deletions(-)
 create mode 100644 buildtools/chkincs/main.cpp

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index c10c1a8ab5..67d68535e0 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -74,6 +74,7 @@ fi
 
 if [ "$BUILD_32BIT" = "true" ]; then
     OPTS="$OPTS -Dc_args=-m32 -Dc_link_args=-m32"
+    OPTS="$OPTS -Dcpp_args=-m32 -Dcpp_link_args=-m32"
     export PKG_CONFIG_LIBDIR="/usr/lib32/pkgconfig"
 fi
 
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 6cf997d6ee..d30cfd08d7 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -116,7 +116,7 @@ jobs:
           libdw-dev
     - name: Install i386 cross compiling packages
       if: env.BUILD_32BIT == 'true'
-      run: sudo apt install -y gcc-multilib
+      run: sudo apt install -y gcc-multilib g++-multilib
     - name: Install aarch64 cross compiling packages
       if: env.AARCH64 == 'true'
       run: sudo apt install -y gcc-aarch64-linux-gnu libc6-dev-arm64-cross
diff --git a/buildtools/chkincs/main.cpp b/buildtools/chkincs/main.cpp
new file mode 100644
index 0000000000..d25bb8852a
--- /dev/null
+++ b/buildtools/chkincs/main.cpp
@@ -0,0 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+int main(void) { return 0; }
diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 5ffca89761..beabcd55d8 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -28,3 +28,23 @@ executable('chkincs', sources,
         dependencies: deps,
         link_whole: dpdk_static_libraries + dpdk_drivers,
         install: false)
+
+# run tests for c++ builds also
+if not add_languages('cpp', required: false)
+    subdir_done()
+endif
+
+gen_cpp_files = generator(gen_c_file_for_header,
+        output: '@BASENAME@.cpp',
+        arguments: ['@INPUT@', '@OUTPUT@'])
+
+cpp_sources = files('main.cpp')
+cpp_sources += gen_cpp_files.process(dpdk_chkinc_headers)
+
+executable('chkincs-cpp', cpp_sources,
+        cpp_args: ['-include', 'rte_config.h', cflags],
+        link_args: dpdk_extra_ldflags,
+        include_directories: includes,
+        dependencies: deps,
+        link_whole: dpdk_static_libraries + dpdk_drivers,
+        install: false)
diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 4ed61328b9..c07fd16fdc 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -246,7 +246,8 @@ if check_cc_flags '-m32' ; then
 		export PKG_CONFIG_LIBDIR='/usr/lib/pkgconfig'
 	fi
 	target_override='i386-pc-linux-gnu'
-	build build-32b cc ABI -Dc_args='-m32' -Dc_link_args='-m32'
+	build build-32b cc ABI -Dc_args='-m32' -Dc_link_args='-m32' \
+			-Dcpp_args='-m32' -Dcpp_link_args='-m32'
 	target_override=
 	unset PKG_CONFIG_LIBDIR
 fi
-- 
2.32.0

Re: [PATCH v4 7/7] buildtools/chkincs: test headers for C++ compatibility

[Thomas Monjalon]

10/02/2022 16:42, Bruce Richardson:
> +executable('chkincs-cpp', cpp_sources,
> +        cpp_args: ['-include', 'rte_config.h', cflags],
> +        link_args: dpdk_extra_ldflags,
> +        include_directories: includes,
> +        dependencies: deps,
> +        link_whole: dpdk_static_libraries + dpdk_drivers,
> +        install: false)

devtools/test-meson-builds.sh is failing on build-x86-generic
at link stage:
	/usr/bin/ld: cannot find -lIPSec_MB
That's probably because there is no pkg-config file for this lib,
and it is not installed in a standard directory,
so my setup is adding -L in LDFLAGS.

Re: [PATCH v4 7/7] buildtools/chkincs: test headers for C++ compatibility

[Bruce Richardson]

On Thu, Feb 10, 2022 at 10:58:37PM +0100, Thomas Monjalon wrote:
> 10/02/2022 16:42, Bruce Richardson:
> > +executable('chkincs-cpp', cpp_sources, +        cpp_args: ['-include',
> > 'rte_config.h', cflags], +        link_args: dpdk_extra_ldflags, +
> > include_directories: includes, +        dependencies: deps, +
> > link_whole: dpdk_static_libraries + dpdk_drivers, +        install:
> > false)
> 
> devtools/test-meson-builds.sh is failing on build-x86-generic at link
> stage: /usr/bin/ld: cannot find -lIPSec_MB That's probably because there
> is no pkg-config file for this lib, and it is not installed in a standard
> directory, so my setup is adding -L in LDFLAGS.
> 
The test-meson-build.sh script works fine for me on my system with the
ipsec_mb libraries being found ok [installed in standard path]. Can you
share how you are adding the extra library paths, is it via environment or
via c_link_args?

/Bruce

Re: [PATCH v4 7/7] buildtools/chkincs: test headers for C++ compatibility

[Bruce Richardson]

On Fri, Feb 11, 2022 at 10:31:38AM +0000, Bruce Richardson wrote:
> On Thu, Feb 10, 2022 at 10:58:37PM +0100, Thomas Monjalon wrote:
> > 10/02/2022 16:42, Bruce Richardson:
> > > +executable('chkincs-cpp', cpp_sources, +        cpp_args: ['-include',
> > > 'rte_config.h', cflags], +        link_args: dpdk_extra_ldflags, +
> > > include_directories: includes, +        dependencies: deps, +
> > > link_whole: dpdk_static_libraries + dpdk_drivers, +        install:
> > > false)
> > 
> > devtools/test-meson-builds.sh is failing on build-x86-generic at link
> > stage: /usr/bin/ld: cannot find -lIPSec_MB That's probably because there
> > is no pkg-config file for this lib, and it is not installed in a standard
> > directory, so my setup is adding -L in LDFLAGS.
> > 
> The test-meson-build.sh script works fine for me on my system with the
> ipsec_mb libraries being found ok [installed in standard path]. Can you
> share how you are adding the extra library paths, is it via environment or
> via c_link_args?
> 
Actually, I just realised that these chkincs binaries don't need to be
linked with all these libraries at all, since they don't actually call any
functions and are just meant to check includes. Let me test with removing
all the "link_whole:" parameters and if that works, submit it as a new
patch. That might solve your issues here - again depending on how your
environment is set up.

/Bruce

Re: [PATCH v4 7/7] buildtools/chkincs: test headers for C++ compatibility

[Thomas Monjalon]

11/02/2022 11:57, Bruce Richardson:
> On Fri, Feb 11, 2022 at 10:31:38AM +0000, Bruce Richardson wrote:
> > On Thu, Feb 10, 2022 at 10:58:37PM +0100, Thomas Monjalon wrote:
> > > 10/02/2022 16:42, Bruce Richardson:
> > > > +executable('chkincs-cpp', cpp_sources, +        cpp_args: ['-include',
> > > > 'rte_config.h', cflags], +        link_args: dpdk_extra_ldflags, +
> > > > include_directories: includes, +        dependencies: deps, +
> > > > link_whole: dpdk_static_libraries + dpdk_drivers, +        install:
> > > > false)
> > > 
> > > devtools/test-meson-builds.sh is failing on build-x86-generic at link
> > > stage: /usr/bin/ld: cannot find -lIPSec_MB That's probably because there
> > > is no pkg-config file for this lib, and it is not installed in a standard
> > > directory, so my setup is adding -L in LDFLAGS.
> > > 
> > The test-meson-build.sh script works fine for me on my system with the
> > ipsec_mb libraries being found ok [installed in standard path]. Can you
> > share how you are adding the extra library paths, is it via environment or
> > via c_link_args?
> > 
> Actually, I just realised that these chkincs binaries don't need to be
> linked with all these libraries at all, since they don't actually call any
> functions and are just meant to check includes. Let me test with removing
> all the "link_whole:" parameters and if that works, submit it as a new
> patch. That might solve your issues here - again depending on how your
> environment is set up.

As I said, it is configured via LDFLAGS environment variable:
	export LDFLAGS=-L$aesnidep

Re: [PATCH v4 0/7] Verify C++ compatibility of public headers

[Thomas Monjalon]

10/02/2022 16:42, Bruce Richardson:
> Bruce Richardson (7):
>   eal: fix header build with C++
>   eventdev: fix header build with C++
>   graph: fix missing explicit cast for C++ build
>   ipsec: fix missing explicit cast for C++ build
>   table: fix missing explicit casts for C++ build
>   vhost: fix incompatible header includes for C++

Fixes applied, thanks.

>   buildtools/chkincs: test headers for C++ compatibility

This last patch is not merged because of a compilation issue.

[PATCH v5 0/2] Verify C++ compatibility of public headers

[Bruce Richardson]

NOTE: despite significant differences in patchset, submitting this as v5
to keep history and threading rather than as new v1

This patchset expands upon the exiting chkincs infrastructure to build a
C++ test application as well as the existing C test app, which helps
catch any build errors when the public DPDK headers are included in C++
code.

V5:
* Dropped first 6 patches which contained fixes now already applied.
* Added new patch to slim-down the linkage of existing chkincs binary
* New patch to add CPP checks also only does mimimal linkage

V4:
* add g++-multilib to packages to install for github checks to ensure 32-bit
  builds work
* correct the fixline for patch 1
* add extra cpp_args and cpp_link_args for 32-bit builds in test-meson-builds
  so that includes checks can pass there if enabled.

V3:
* update linux-build script to have cpp_args=-m32 for 32-bit builds

V2:
* Changed patch 6 to have a minimally-invasive fix for C++ without
  affecting existing C code.
* Added fixlines and Cc stable for all patches which are fixes

Bruce Richardson (2):
  buildtools/chkincs: remove unnecesary linkage
  buildtools/chkincs: test headers for C++ compatibility

 .ci/linux-build.sh             |  1 +
 .github/workflows/build.yml    |  2 +-
 buildtools/chkincs/main.cpp    |  4 ++++
 buildtools/chkincs/meson.build | 21 +++++++++++++++++++--
 devtools/test-meson-builds.sh  |  3 ++-
 5 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 buildtools/chkincs/main.cpp

--
2.32.0

[PATCH v5 1/2] buildtools/chkincs: remove unnecesary linkage

[Bruce Richardson]

The chkincs binary does not actually call any functions in either libs
or drivers, so we can simplify the linkage of it to just using shared
linkage of the libraries (via meson dependencies). This means a slightly
faster link time as well as making the chkincs binary much, much
smaller.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/chkincs/meson.build | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 5ffca89761..7ea136ff95 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -19,12 +19,11 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
 
 deps = []
 foreach l:enabled_libs
-    deps += get_variable('static_rte_' + l)
+    deps += get_variable('shared_rte_' + l)
 endforeach
 
 executable('chkincs', sources,
         c_args: cflags,
         include_directories: includes,
         dependencies: deps,
-        link_whole: dpdk_static_libraries + dpdk_drivers,
         install: false)
-- 
2.32.0

[PATCH v5 2/2] buildtools/chkincs: test headers for C++ compatibility

[Bruce Richardson]

Add support for checking each of our headers for issues when included in
a C++ file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 .ci/linux-build.sh             |  1 +
 .github/workflows/build.yml    |  2 +-
 buildtools/chkincs/main.cpp    |  4 ++++
 buildtools/chkincs/meson.build | 18 ++++++++++++++++++
 devtools/test-meson-builds.sh  |  3 ++-
 5 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 buildtools/chkincs/main.cpp

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index c10c1a8ab5..67d68535e0 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -74,6 +74,7 @@ fi
 
 if [ "$BUILD_32BIT" = "true" ]; then
     OPTS="$OPTS -Dc_args=-m32 -Dc_link_args=-m32"
+    OPTS="$OPTS -Dcpp_args=-m32 -Dcpp_link_args=-m32"
     export PKG_CONFIG_LIBDIR="/usr/lib32/pkgconfig"
 fi
 
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 6cf997d6ee..d30cfd08d7 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -116,7 +116,7 @@ jobs:
           libdw-dev
     - name: Install i386 cross compiling packages
       if: env.BUILD_32BIT == 'true'
-      run: sudo apt install -y gcc-multilib
+      run: sudo apt install -y gcc-multilib g++-multilib
     - name: Install aarch64 cross compiling packages
       if: env.AARCH64 == 'true'
       run: sudo apt install -y gcc-aarch64-linux-gnu libc6-dev-arm64-cross
diff --git a/buildtools/chkincs/main.cpp b/buildtools/chkincs/main.cpp
new file mode 100644
index 0000000000..d25bb8852a
--- /dev/null
+++ b/buildtools/chkincs/main.cpp
@@ -0,0 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+int main(void) { return 0; }
diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 7ea136ff95..790f700619 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -27,3 +27,21 @@ executable('chkincs', sources,
         include_directories: includes,
         dependencies: deps,
         install: false)
+
+# run tests for c++ builds also
+if not add_languages('cpp', required: false)
+    subdir_done()
+endif
+
+gen_cpp_files = generator(gen_c_file_for_header,
+        output: '@BASENAME@.cpp',
+        arguments: ['@INPUT@', '@OUTPUT@'])
+
+cpp_sources = files('main.cpp')
+cpp_sources += gen_cpp_files.process(dpdk_chkinc_headers)
+
+executable('chkincs-cpp', cpp_sources,
+        cpp_args: ['-include', 'rte_config.h', cflags],
+        include_directories: includes,
+        dependencies: deps,
+        install: false)
diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 4ed61328b9..c07fd16fdc 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -246,7 +246,8 @@ if check_cc_flags '-m32' ; then
 		export PKG_CONFIG_LIBDIR='/usr/lib/pkgconfig'
 	fi
 	target_override='i386-pc-linux-gnu'
-	build build-32b cc ABI -Dc_args='-m32' -Dc_link_args='-m32'
+	build build-32b cc ABI -Dc_args='-m32' -Dc_link_args='-m32' \
+			-Dcpp_args='-m32' -Dcpp_link_args='-m32'
 	target_override=
 	unset PKG_CONFIG_LIBDIR
 fi
-- 
2.32.0

Re: [PATCH v5 0/2] Verify C++ compatibility of public headers

[Thomas Monjalon]

11/02/2022 12:36, Bruce Richardson:
> NOTE: despite significant differences in patchset, submitting this as v5
> to keep history and threading rather than as new v1
> 
> This patchset expands upon the exiting chkincs infrastructure to build a
> C++ test application as well as the existing C test app, which helps
> catch any build errors when the public DPDK headers are included in C++
> code.
> 
> V5:
> * Dropped first 6 patches which contained fixes now already applied.
> * Added new patch to slim-down the linkage of existing chkincs binary
> * New patch to add CPP checks also only does mimimal linkage

This version works fine in my environment.
Applied, thanks.