Verify C++ compatibility of public headers
cover Verify C++ compatibility of public headers
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++
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 *
--- lib/eal/include/generic/rte_ticketlock.h | 14 +++++++------- lib/eal/include/rte_trace_point.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-)
lib/eal/include/generic/rte_ticketlock.h +7 -7
@@ -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 ; }
lib/eal/include/rte_trace_point.h +1 -1
@@ -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 )
Signature · 1 line
2.32.0
PATCH 2/7 eventdev: fix header build with C++
In reply to Bruce RichardsonThe eventdev headers had issues when used from C++
* Missing closing "}" for the extern "C" block
* No automatic casting to/from void *
--- lib/eventdev/rte_event_timer_adapter.h | 3 +++ lib/eventdev/rte_eventdev.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
lib/eventdev/rte_event_timer_adapter.h +3 -0
@@ -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__ */
lib/eventdev/rte_eventdev.h +1 -1
@@ -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
Signature · 1 line
2.32.0
Re: [PATCH 2/7] eventdev: fix header build with C++
Quoted 7 lines · 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 <[email protected]>
Quoted 33 lines
--- lib/eventdev/rte_event_timer_adapter.h | 3 +++ lib/eventdev/rte_eventdev.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)lib/eventdev/rte_event_timer_adapter.h +3
-0@@ -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__ */ lib/eventdev/rte_eventdev.h +1
-2@@ -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
In reply to Bruce RichardsonC++ does not have automatic casting to/from void pointers, so need explicit cast if header is to be included in C++ code
--- lib/graph/rte_graph_worker.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/graph/rte_graph_worker.h +1 -1
@@ -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);
Signature · 1 line
2.32.0
Re: [PATCH 3/7] graph: add explicit cast to allow C++ build
Quoted 13 lines · On Fri, 4 Feb 2022 17:42:05 +0000 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 <[email protected]>--- lib/graph/rte_graph_worker.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)lib/graph/rte_graph_worker.h +0
-0
lib/graph/rte_graph_worker.h rte_graph_walk(struct rte_graph *graph) · line 161
Quoted 8 lines
lib/graph/rte_graph_worker.h +1
-1@@ -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.
Jerin Jacob
Acked-byRe: [PATCH 3/7] graph: add explicit cast to allow C++ build
Quoted 12 lines · Stephen Hemminger
Quoted 8 lines · On Fri, 4 Feb 2022 17:42:05 +0000 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 <[email protected]>--- lib/graph/rte_graph_worker.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 16 lines
lib/graph/rte_graph_worker.h rte_graph_walk(struct rte_graph *graph) · line 161
Quoted 13 lines
lib/graph/rte_graph_worker.h +1
-1@@ -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 byrte_graph.h and rte_graph_worker.h
PATCH 4/7 ipsec: add explicit cast to allow C++ build
In reply to Bruce RichardsonC++ does not have automatic casting to/from void pointers, so need explicit cast if header is to be included in C++ code
--- lib/ipsec/rte_ipsec_group.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
lib/ipsec/rte_ipsec_group.h +2 -2
@@ -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
In reply to Bruce RichardsonSince C++ doesn't support automatic casting from void * to other types, we need to explicitly add the casts to any header files in DPDK.
--- lib/table/rte_table_hash_func.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
lib/table/rte_table_hash_func.h +16 -16
@@ -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 ];
Signature · 1 line
2.32.0
PATCH 6/7 vhost: remove non-C++ compatible includes
In reply to Bruce RichardsonSome 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.
--- 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(-)
drivers/net/vhost/rte_eth_vhost.c +2 -0
@@ -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"
drivers/vdpa/ifc/ifcvf_vdpa.c +2 -0
@@ -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 );
drivers/vdpa/mlx5/mlx5_vdpa.h +1 -0
@@ -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
lib/vhost/rte_vhost.h +0 -4
@@ -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
Quoted 7 lines · 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 <[email protected]>---
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
Hi Bruce,
Quoted 16 lines
-----Original Message-----
From: Richardson, Bruce <[email protected]>
Sent: Saturday, February 5, 2022 2:19 AM
To: [email protected]
Cc: Maxime Coquelin <[email protected]>; Xia, Chenbo <[email protected]>; Wang, Xiao W <[email protected]>; Matan Azrad <[email protected]>; Viacheslav Ovsiienko <[email protected]>
Subject: Re: [PATCH 6/7] vhost: remove non-C++ compatible includesQuoted 5 lines · 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.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Bruce Richardson <[email protected]>---
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
Quoted 22 lines · Xiao W
Hi Bruce,
Quoted 20 lines
-----Original Message----- From: Richardson, Bruce <[email protected]> Sent: Saturday, February 5, 2022 2:19 AM
To: [email protected] Cc: Maxime Coquelin <[email protected]>; Xia, Chenbo <[email protected]>; Wang, Xiao W <[email protected]>; Matan Azrad <[email protected]>; Viacheslav Ovsiienko <[email protected]> Subject: Re: [PATCH 6/7] vhost: remove non-C++ compatible includesQuoted 6 lines · 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 <[email protected]> ---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.
Quoted 6 lines
Quoted 1 line
/Bruce
Could we move these c++ incompatible linux headers into #ifndef
__cplusplus... #endif. Then we just need to changerte_vhost.hfile, 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
In reply to Bruce RichardsonAdd support for checking each of our headers for issues when included in a C++ file.
--- buildtools/chkincs/main.cpp | 4 ++++ buildtools/chkincs/meson.build | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 buildtools/chkincs/main.cpp
buildtools/chkincs/main.cpp new file +4 -0
@@ -0,0 +1,4 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2021 Intel Corporation + */ +int main(void) { return 0; }
buildtools/chkincs/meson.build +20 -0
@@ -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: '@[email protected]', + 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)
Signature · 1 line
2.32.0
v2 cover Verify C++ compatibility of public headers
In reply to Bruce RichardsonThis 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++
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 *
--- lib/eal/include/generic/rte_ticketlock.h | 14 +++++++------- lib/eal/include/rte_trace_point.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-)
lib/eal/include/generic/rte_ticketlock.h +7 -7
@@ -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 ; }
lib/eal/include/rte_trace_point.h +1 -1
@@ -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 )
Signature · 1 line
2.32.0
Re: [PATCH v2 1/7] eal: fix header build with C++
Quoted 8 lines · 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")
rte_strcpy was moved in this commit.
This is more about:
Quoted 49 lines
Signed-off-by Bruce Richardson <[email protected]>--- lib/eal/include/generic/rte_ticketlock.h | 14 +++++++------- lib/eal/include/rte_trace_point.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-)lib/eal/include/generic/rte_ticketlock.h +7
-7@@ -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 ; }lib/eal/include/rte_trace_point.h +1
-2@@ -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 ) --
Quoted 2 lines
2.32.0
lgtm, thanks.
Signature · 1 line
David Marchand
Re: [PATCH v2 1/7] eal: fix header build with C++
Quoted 8 lines · David Marchand
Quoted 6 lines · 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 *
Quoted 7 lines
Quoted 2 lines
Fixes 184104fc6121 ("ticketlock: introduce fair ticket based locking")Fixes ebaee6409702 ("trace: simplify trace point headers")
rte_strcpywas 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++
In reply to Bruce RichardsonThe eventdev headers had issues when used from C++
* Missing closing "}" for the extern "C" block
* No automatic casting to/from void *
--- lib/eventdev/rte_event_timer_adapter.h | 3 +++ lib/eventdev/rte_eventdev.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
lib/eventdev/rte_event_timer_adapter.h +3 -0
@@ -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__ */
lib/eventdev/rte_eventdev.h +1 -1
@@ -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
Signature · 1 line
2.32.0
PATCH v2 3/7 graph: fix missing explicit cast for C++ build
In reply to Bruce RichardsonC++ does not have automatic casting to/from void pointers, so need explicit cast if header is to be included in C++ code
--- lib/graph/rte_graph_worker.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/graph/rte_graph_worker.h +1 -1
@@ -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);
Signature · 1 line
2.32.0
PATCH v2 4/7 ipsec: fix missing explicit cast for C++ build
In reply to Bruce RichardsonC++ does not have automatic casting to/from void pointers, so need explicit cast if header is to be included in C++ code
--- lib/ipsec/rte_ipsec_group.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
lib/ipsec/rte_ipsec_group.h +2 -2
@@ -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 ; }
Signature · 1 line
2.32.0
RE: [PATCH v2 4/7] ipsec: fix missing explicit cast for C++ build
Quoted 22 lines
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")Signed-off-by Bruce Richardson <[email protected]>--- lib/ipsec/rte_ipsec_group.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)lib/ipsec/rte_ipsec_group.h +0
-0@@ -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;
lib/ipsec/rte_ipsec_group.h rte_ipsec_ses_from_crypto(const struct rte_crypto_op *cop)
Quoted 10 lines
lib/ipsec/rte_ipsec_group.h +2
-3-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 ; } --
Quoted 1 line
2.32.0
PATCH v2 5/7 table: fix missing explicit casts for C++ build
In reply to Bruce RichardsonSince C++ doesn't support automatic casting from void * to other types, we need to explicitly add the casts to any header files in DPDK.
--- lib/table/rte_table_hash_func.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
lib/table/rte_table_hash_func.h +16 -16
@@ -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 ];
Signature · 1 line
2.32.0
PATCH v2 6/7 vhost: fix incompatible header includes for C++
In reply to Bruce RichardsonThe 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.
--- lib/vhost/rte_vhost.h | 2 ++ 1 file changed, 2 insertions(+)
lib/vhost/rte_vhost.h +2 -0
@@ -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)
Signature · 1 line
2.32.0
PATCH v2 7/7 buildtools/chkincs: test headers for C++ compatibility
In reply to Bruce RichardsonAdd support for checking each of our headers for issues when included in a C++ file.
--- buildtools/chkincs/main.cpp | 4 ++++ buildtools/chkincs/meson.build | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 buildtools/chkincs/main.cpp
buildtools/chkincs/main.cpp new file +4 -0
@@ -0,0 +1,4 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2021 Intel Corporation + */ +int main(void) { return 0; }
buildtools/chkincs/meson.build +20 -0
@@ -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: '@[email protected]', + 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)
Signature · 1 line
2.32.0
v3 cover Verify C++ compatibility of public headers
In reply to Bruce RichardsonThis 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++
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 *
--- lib/eal/include/generic/rte_ticketlock.h | 14 +++++++------- lib/eal/include/rte_trace_point.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-)
lib/eal/include/generic/rte_ticketlock.h +7 -7
@@ -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 ; }
lib/eal/include/rte_trace_point.h +1 -1
@@ -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 )
Signature · 1 line
2.32.0
PATCH v3 2/7 eventdev: fix header build with C++
In reply to Bruce RichardsonThe eventdev headers had issues when used from C++
* Missing closing "}" for the extern "C" block
* No automatic casting to/from void *
--- lib/eventdev/rte_event_timer_adapter.h | 3 +++ lib/eventdev/rte_eventdev.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
lib/eventdev/rte_event_timer_adapter.h +3 -0
@@ -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__ */
lib/eventdev/rte_eventdev.h +1 -1
@@ -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
Signature · 1 line
2.32.0
PATCH v3 3/7 graph: fix missing explicit cast for C++ build
In reply to Bruce RichardsonC++ does not have automatic casting to/from void pointers, so need explicit cast if header is to be included in C++ code
--- lib/graph/rte_graph_worker.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/graph/rte_graph_worker.h +1 -1
@@ -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);
Signature · 1 line
2.32.0
PATCH v3 4/7 ipsec: fix missing explicit cast for C++ build
In reply to Bruce RichardsonC++ does not have automatic casting to/from void pointers, so need explicit cast if header is to be included in C++ code
--- lib/ipsec/rte_ipsec_group.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
lib/ipsec/rte_ipsec_group.h +2 -2
@@ -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 ; }
Signature · 1 line
2.32.0
PATCH v3 5/7 table: fix missing explicit casts for C++ build
In reply to Bruce RichardsonSince C++ doesn't support automatic casting from void * to other types, we need to explicitly add the casts to any header files in DPDK.
--- lib/table/rte_table_hash_func.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
lib/table/rte_table_hash_func.h +16 -16
@@ -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 ];
Signature · 1 line
2.32.0
PATCH v3 6/7 vhost: fix incompatible header includes for C++
In reply to Bruce RichardsonThe 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.
--- lib/vhost/rte_vhost.h | 2 ++ 1 file changed, 2 insertions(+)
lib/vhost/rte_vhost.h +2 -0
@@ -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)
Signature · 1 line
2.32.0
PATCH v3 7/7 buildtools/chkincs: test headers for C++ compatibility
In reply to Bruce RichardsonAdd support for checking each of our headers for issues when included in a C++ file.
--- .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
.ci/linux-build.sh +1 -0
@@ -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
buildtools/chkincs/main.cpp new file +4 -0
@@ -0,0 +1,4 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2021 Intel Corporation + */ +int main(void) { return 0; }
buildtools/chkincs/meson.build +20 -0
@@ -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: '@[email protected]', + 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)
Signature · 1 line
2.32.0
Re: [PATCH v3 7/7] buildtools/chkincs: test headers for C++ compatibility
Quoted 17 lines · Bruce Richardson
Add support for checking each of our headers for issues when included in a C++ file.
Signed-off-by Bruce Richardson <[email protected]>--- .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.ci/linux-build.sh +0
-0
.ci/linux-build.sh fi · line 80
Quoted 7 lines
.ci/linux-build.sh +1
-0@@ -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
Signature · 1 line
David Marchand
Re: [PATCH v3 7/7] buildtools/chkincs: test headers for C++ compatibility
Quoted 31 lines · David Marchand
Quoted 17 lines · Bruce Richardson
Add support for checking each of our headers for issues when included in a C++ file.
Signed-off-by Bruce Richardson <[email protected]>--- .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.ci/linux-build.sh +0
-0.ci/linux-build.sh fi · line 80
Quoted 7 lines
.ci/linux-build.sh +1
-0@@ -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" fiEven 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 totest-meson-builds.shtoo?
Yes, will do.
v4 cover Verify C++ compatibility of public headers
In reply to Bruce RichardsonThis 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++
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 *
--- lib/eal/include/generic/rte_ticketlock.h | 14 +++++++------- lib/eal/include/rte_trace_point.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-)
lib/eal/include/generic/rte_ticketlock.h +7 -7
@@ -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 ; }
lib/eal/include/rte_trace_point.h +1 -1
@@ -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 )
Signature · 1 line
2.32.0
RE: [PATCH v4 1/7] eal: fix header build with C++
<snip>
Quoted 7 lines
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 *
Quoted 7 lines
Fixes 184104fc6121 ("ticketlock: introduce fair ticket based locking")Fixes 032a7e5499a0 ("trace: implement provider payload")Signed-off-by Bruce Richardson <[email protected]>
Quoted 45 lines
--- lib/eal/include/generic/rte_ticketlock.h | 14 +++++++------- lib/eal/include/rte_trace_point.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-)lib/eal/include/generic/rte_ticketlock.h +7
-7@@ -91,13 +91,13 @@ rte_ticketlock_unlock(rte_ticketlock_t *tl) staticinline 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 ; }lib/eal/include/rte_trace_point.h +1
-2@@ -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) --
Quoted 1 line
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++
In reply to Bruce RichardsonThe eventdev headers had issues when used from C++
* Missing closing "}" for the extern "C" block
* No automatic casting to/from void *
--- lib/eventdev/rte_event_timer_adapter.h | 3 +++ lib/eventdev/rte_eventdev.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
lib/eventdev/rte_event_timer_adapter.h +3 -0
@@ -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__ */
lib/eventdev/rte_eventdev.h +1 -1
@@ -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
Signature · 1 line
2.32.0
PATCH v4 3/7 graph: fix missing explicit cast for C++ build
In reply to Bruce RichardsonC++ does not have automatic casting to/from void pointers, so need explicit cast if header is to be included in C++ code
--- lib/graph/rte_graph_worker.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/graph/rte_graph_worker.h +1 -1
@@ -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);
Signature · 1 line
2.32.0
PATCH v4 4/7 ipsec: fix missing explicit cast for C++ build
In reply to Bruce RichardsonC++ does not have automatic casting to/from void pointers, so need explicit cast if header is to be included in C++ code
--- lib/ipsec/rte_ipsec_group.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
lib/ipsec/rte_ipsec_group.h +2 -2
@@ -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 ; }
Signature · 1 line
2.32.0
PATCH v4 5/7 table: fix missing explicit casts for C++ build
In reply to Bruce RichardsonSince C++ doesn't support automatic casting from void * to other types, we need to explicitly add the casts to any header files in DPDK.
--- lib/table/rte_table_hash_func.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
lib/table/rte_table_hash_func.h +16 -16
@@ -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 ];
Signature · 1 line
2.32.0
PATCH v4 6/7 vhost: fix incompatible header includes for C++
In reply to Bruce RichardsonThe 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.
--- lib/vhost/rte_vhost.h | 2 ++ 1 file changed, 2 insertions(+)
lib/vhost/rte_vhost.h +2 -0
@@ -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)
Signature · 1 line
2.32.0
PATCH v4 7/7 buildtools/chkincs: test headers for C++ compatibility
In reply to Bruce RichardsonAdd support for checking each of our headers for issues when included in a C++ file.
--- .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
.ci/linux-build.sh +1 -0
@@ -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
.github/workflows/build.yml +1 -1
@@ -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
buildtools/chkincs/main.cpp new file +4 -0
@@ -0,0 +1,4 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2021 Intel Corporation + */ +int main(void) { return 0; }
buildtools/chkincs/meson.build +20 -0
@@ -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: '@[email protected]', + 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)
devtools/test-meson-builds.sh +2 -1
@@ -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
Signature · 1 line
2.32.0
Re: [PATCH v4 7/7] buildtools/chkincs: test headers for C++ compatibility
10/02/2022 16:42, Bruce Richardson:
buildtools/chkincs/meson.build executable('chkincs', sources, · line 50
Quoted 7 lines
buildtools/chkincs/meson.build +7
-0+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
Quoted 7 lines · Thomas Monjalon
10/02/2022 16:42, Bruce Richardson:
Quoted 5 lines
+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)
Quoted 5 lines
devtools/test-meson-builds.shis 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
Quoted 13 lines · Bruce Richardson
Quoted 7 lines · Thomas Monjalon
10/02/2022 16:42, Bruce Richardson:
Quoted 5 lines
+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)Quoted 5 lines
devtools/test-meson-builds.shis 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.
Quoted 5 lines
The
test-meson-build.shscript works fine for me on my system with theipsec_mblibraries being found ok [installed in standard path]. Can you share how you are adding the extra library paths, is it via environment or viac_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
11/02/2022 11:57, Bruce Richardson:
Quoted 19 lines · Bruce Richardson
Quoted 13 lines · Thomas Monjalon
Quoted 7 lines
10/02/2022 16:42, Bruce Richardson:
Quoted 5 lines
+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)Quoted 5 lines
devtools/test-meson-builds.shis 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.Quoted 5 lines
The
test-meson-build.shscript works fine for me on my system with theipsec_mblibraries being found ok [installed in standard path]. Can you share how you are adding the extra library paths, is it via environment or viac_link_args?
Quoted 6 lines
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
10/02/2022 16:42, Bruce Richardson:
Quoted 7 lines
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.
Quoted 1 line
buildtools/chkincs: test headers for C++ compatibility
This last patch is not merged because of a compilation issue.
v5 cover Verify C++ compatibility of public headers
In reply to Bruce RichardsonNOTE: 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
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.
--- buildtools/chkincs/meson.build | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
buildtools/chkincs/meson.build +1 -2
@@ -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)
Signature · 1 line
2.32.0
PATCH v5 2/2 buildtools/chkincs: test headers for C++ compatibility
In reply to Bruce RichardsonAdd support for checking each of our headers for issues when included in a C++ file.
--- .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
.ci/linux-build.sh +1 -0
@@ -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
.github/workflows/build.yml +1 -1
@@ -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
buildtools/chkincs/main.cpp new file +4 -0
@@ -0,0 +1,4 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2021 Intel Corporation + */ +int main(void) { return 0; }
buildtools/chkincs/meson.build +18 -0
@@ -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: '@[email protected]', + 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)
devtools/test-meson-builds.sh +2 -1
@@ -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
Signature · 1 line
2.32.0
11/02/2022 12:36, Bruce Richardson:
Quoted 8 lines
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.
Quoted 4 lines
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.