* out of tree driver builds broken with C++
@ 2022-02-14 9:13 Tyler Retzlaff
2022-02-14 9:22 ` Thomas Monjalon
0 siblings, 1 reply; 14+ messages in thread
From: Tyler Retzlaff @ 2022-02-14 9:13 UTC (permalink / raw)
To: dev, thomas, ocardona; +Cc: roretzla
while the driver api is "internal" we agreed some time ago that drivers
could be built external to the dpdk tree. by enabling the meson setup
option -Denable_driver_sdk=true.
it was agreed that the driver api was internal and would attract no
binary compatibility support which was fine. this change has now
imposed a further restriction that out of tree drivers have to be
authored in C only as non-C++ compatible code will invariably leak into
the internal structures.
you won't allow us to build C++ drivers in the dpdk tree and it seems
now you are preventing building of C++ drivers outside of the tree too.
could we please re-evaluate this.
thanks.
commit 7a335720575507f55b723b1e10bfea7daeba1386
Author: Thomas Monjalon <thomas@monjalon.net>
Date: Wed Sep 15 18:46:35 2021 +0200
lib: remove C++ include guard from private headers
The private headers are compiled internally with a C compiler.
Thus extern "C" declaration is useless in such files.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: out of tree driver builds broken with C++ 2022-02-14 9:13 out of tree driver builds broken with C++ Tyler Retzlaff @ 2022-02-14 9:22 ` Thomas Monjalon 2022-02-14 10:45 ` Bruce Richardson 2022-02-15 6:10 ` Tyler Retzlaff 0 siblings, 2 replies; 14+ messages in thread From: Thomas Monjalon @ 2022-02-14 9:22 UTC (permalink / raw) To: Tyler Retzlaff Cc: dev, ocardona, roretzla, david.marchand, ferruh.yigit, bruce.richardson 14/02/2022 10:13, Tyler Retzlaff: > while the driver api is "internal" we agreed some time ago that drivers > could be built external to the dpdk tree. by enabling the meson setup > option -Denable_driver_sdk=true. > > it was agreed that the driver api was internal and would attract no > binary compatibility support which was fine. this change has now > imposed a further restriction that out of tree drivers have to be > authored in C only as non-C++ compatible code will invariably leak into > the internal structures. > > you won't allow us to build C++ drivers in the dpdk tree and it seems > now you are preventing building of C++ drivers outside of the tree too. That's the problem of non-written assumptions, they are unknown or forgotten. Did we agree to support out-of-tree drivers in C++? We really need to make things clear and written in documentation. > could we please re-evaluate this. Yes we can re-evaluate. What is the list of impacted files? > commit 7a335720575507f55b723b1e10bfea7daeba1386 > Author: Thomas Monjalon <thomas@monjalon.net> > Date: Wed Sep 15 18:46:35 2021 +0200 > > lib: remove C++ include guard from private headers > > The private headers are compiled internally with a C compiler. > Thus extern "C" declaration is useless in such files. > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 9:22 ` Thomas Monjalon @ 2022-02-14 10:45 ` Bruce Richardson 2022-02-14 11:19 ` Thomas Monjalon 2022-02-15 6:10 ` Tyler Retzlaff 1 sibling, 1 reply; 14+ messages in thread From: Bruce Richardson @ 2022-02-14 10:45 UTC (permalink / raw) To: Thomas Monjalon Cc: Tyler Retzlaff, dev, ocardona, roretzla, david.marchand, ferruh.yigit On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > 14/02/2022 10:13, Tyler Retzlaff: > > while the driver api is "internal" we agreed some time ago that drivers > > could be built external to the dpdk tree. by enabling the meson setup > > option -Denable_driver_sdk=true. > > > > it was agreed that the driver api was internal and would attract no > > binary compatibility support which was fine. this change has now > > imposed a further restriction that out of tree drivers have to be > > authored in C only as non-C++ compatible code will invariably leak into > > the internal structures. > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > now you are preventing building of C++ drivers outside of the tree too. > > That's the problem of non-written assumptions, they are unknown or forgotten. > Did we agree to support out-of-tree drivers in C++? > > We really need to make things clear and written in documentation. > > > could we please re-evaluate this. > > Yes we can re-evaluate. > What is the list of impacted files? > Hacking meson files a bit, the list of SDK header files is reported as below. /Bruce Message: SDK headers: Message: ethdev_driver.h Message: ethdev_pci.h Message: ethdev_vdev.h Message: cryptodev_pmd.h Message: eventdev_pmd.h Message: eventdev_pmd_pci.h Message: eventdev_pmd_vdev.h Message: eventdev_trace.h Message: event_timer_adapter_pmd.h Message: rte_dmadev_pmd.h Message: vdpa_driver.h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 10:45 ` Bruce Richardson @ 2022-02-14 11:19 ` Thomas Monjalon 2022-02-14 11:43 ` Bruce Richardson 0 siblings, 1 reply; 14+ messages in thread From: Thomas Monjalon @ 2022-02-14 11:19 UTC (permalink / raw) To: Bruce Richardson Cc: dev, Tyler Retzlaff, dev, ocardona, roretzla, david.marchand, ferruh.yigit 14/02/2022 11:45, Bruce Richardson: > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > 14/02/2022 10:13, Tyler Retzlaff: > > > while the driver api is "internal" we agreed some time ago that drivers > > > could be built external to the dpdk tree. by enabling the meson setup > > > option -Denable_driver_sdk=true. > > > > > > it was agreed that the driver api was internal and would attract no > > > binary compatibility support which was fine. this change has now > > > imposed a further restriction that out of tree drivers have to be > > > authored in C only as non-C++ compatible code will invariably leak into > > > the internal structures. > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > now you are preventing building of C++ drivers outside of the tree too. > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > Did we agree to support out-of-tree drivers in C++? > > > > We really need to make things clear and written in documentation. > > > > > could we please re-evaluate this. > > > > Yes we can re-evaluate. > > What is the list of impacted files? > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > /Bruce > > Message: SDK headers: > Message: ethdev_driver.h > Message: ethdev_pci.h > Message: ethdev_vdev.h > Message: cryptodev_pmd.h > Message: eventdev_pmd.h > Message: eventdev_pmd_pci.h > Message: eventdev_pmd_vdev.h > Message: eventdev_trace.h > Message: event_timer_adapter_pmd.h > Message: rte_dmadev_pmd.h > Message: vdpa_driver.h I see no harm in supporting C++ include of these headers. Any objection? Could we have a test in chkincs for the SDK headers? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 11:19 ` Thomas Monjalon @ 2022-02-14 11:43 ` Bruce Richardson 2022-02-14 15:03 ` Bruce Richardson 0 siblings, 1 reply; 14+ messages in thread From: Bruce Richardson @ 2022-02-14 11:43 UTC (permalink / raw) To: Thomas Monjalon Cc: dev, Tyler Retzlaff, ocardona, roretzla, david.marchand, ferruh.yigit On Mon, Feb 14, 2022 at 12:19:24PM +0100, Thomas Monjalon wrote: > 14/02/2022 11:45, Bruce Richardson: > > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > > 14/02/2022 10:13, Tyler Retzlaff: > > > > while the driver api is "internal" we agreed some time ago that drivers > > > > could be built external to the dpdk tree. by enabling the meson setup > > > > option -Denable_driver_sdk=true. > > > > > > > > it was agreed that the driver api was internal and would attract no > > > > binary compatibility support which was fine. this change has now > > > > imposed a further restriction that out of tree drivers have to be > > > > authored in C only as non-C++ compatible code will invariably leak into > > > > the internal structures. > > > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > > now you are preventing building of C++ drivers outside of the tree too. > > > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > > Did we agree to support out-of-tree drivers in C++? > > > > > > We really need to make things clear and written in documentation. > > > > > > > could we please re-evaluate this. > > > > > > Yes we can re-evaluate. > > > What is the list of impacted files? > > > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > > > /Bruce > > > > Message: SDK headers: > > Message: ethdev_driver.h > > Message: ethdev_pci.h > > Message: ethdev_vdev.h > > Message: cryptodev_pmd.h > > Message: eventdev_pmd.h > > Message: eventdev_pmd_pci.h > > Message: eventdev_pmd_vdev.h > > Message: eventdev_trace.h > > Message: event_timer_adapter_pmd.h > > Message: rte_dmadev_pmd.h > > Message: vdpa_driver.h > > I see no harm in supporting C++ include of these headers. > Any objection? > > Could we have a test in chkincs for the SDK headers? > Yes. It may make things a little more complicated, though, as it seems these headers also have a tendency to rely on some driver headers - specifically bus driver headers. /Bruce ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 11:43 ` Bruce Richardson @ 2022-02-14 15:03 ` Bruce Richardson 2022-02-14 17:24 ` Stephen Hemminger 0 siblings, 1 reply; 14+ messages in thread From: Bruce Richardson @ 2022-02-14 15:03 UTC (permalink / raw) To: Thomas Monjalon Cc: dev, Tyler Retzlaff, ocardona, roretzla, david.marchand, ferruh.yigit On Mon, Feb 14, 2022 at 11:43:35AM +0000, Bruce Richardson wrote: > On Mon, Feb 14, 2022 at 12:19:24PM +0100, Thomas Monjalon wrote: > > 14/02/2022 11:45, Bruce Richardson: > > > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > > > 14/02/2022 10:13, Tyler Retzlaff: > > > > > while the driver api is "internal" we agreed some time ago that drivers > > > > > could be built external to the dpdk tree. by enabling the meson setup > > > > > option -Denable_driver_sdk=true. > > > > > > > > > > it was agreed that the driver api was internal and would attract no > > > > > binary compatibility support which was fine. this change has now > > > > > imposed a further restriction that out of tree drivers have to be > > > > > authored in C only as non-C++ compatible code will invariably leak into > > > > > the internal structures. > > > > > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > > > now you are preventing building of C++ drivers outside of the tree too. > > > > > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > > > Did we agree to support out-of-tree drivers in C++? > > > > > > > > We really need to make things clear and written in documentation. > > > > > > > > > could we please re-evaluate this. > > > > > > > > Yes we can re-evaluate. > > > > What is the list of impacted files? > > > > > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > > > > > /Bruce > > > > > > Message: SDK headers: > > > Message: ethdev_driver.h > > > Message: ethdev_pci.h > > > Message: ethdev_vdev.h > > > Message: cryptodev_pmd.h > > > Message: eventdev_pmd.h > > > Message: eventdev_pmd_pci.h > > > Message: eventdev_pmd_vdev.h > > > Message: eventdev_trace.h > > > Message: event_timer_adapter_pmd.h > > > Message: rte_dmadev_pmd.h > > > Message: vdpa_driver.h > > > > I see no harm in supporting C++ include of these headers. > > Any objection? > > > > Could we have a test in chkincs for the SDK headers? > > > Yes. It may make things a little more complicated, though, as it seems > these headers also have a tendency to rely on some driver headers - > specifically bus driver headers. > Working on a patch to add these to checks. However, just running a c++ compile does not check for valid 'extern "C"' blocks. Adding the following to chkincs/meson.build as a basic sanity check throws up a larger list of files to be looked at. Diff: +# check for extern C in files, since this is not detected as an error by the compiler +grep = find_program('grep', required: false) +if grep.found() + errlist = run_command([grep, '--files-without-match', '^extern "C"', dpdk_chkinc_headers], + check: true, capture: true).stdout().split() + if errlist != [] + error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- '.join(errlist)) + endif +endif + Output: ../buildtools/chkincs/meson.build:45:8: ERROR: Problem encountered: Files missing C++ 'extern "C"' guards: - /home/bruce/dpdk.org/lib/telemetry/rte_telemetry.h - /home/bruce/dpdk.org/lib/eal/include/rte_bitops.h - /home/bruce/dpdk.org/lib/eal/include/rte_branch_prediction.h - /home/bruce/dpdk.org/lib/eal/include/rte_compat.h - /home/bruce/dpdk.org/lib/eal/include/rte_hypervisor.h - /home/bruce/dpdk.org/lib/eal/include/rte_keepalive.h - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_feature_defs.h - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_features.h - /home/bruce/dpdk.org/lib/eal/include/rte_time.h - /home/bruce/dpdk.org/lib/eal/include/rte_trace_point_register.h - /home/bruce/dpdk.org/lib/eal/linux/include/rte_os.h - /home/bruce/dpdk.org/lib/ethdev/rte_dev_info.h - /home/bruce/dpdk.org/lib/ethdev/ethdev_driver.h - /home/bruce/dpdk.org/lib/ethdev/ethdev_pci.h - /home/bruce/dpdk.org/lib/ethdev/ethdev_vdev.h - /home/bruce/dpdk.org/lib/metrics/rte_metrics_telemetry.h - /home/bruce/dpdk.org/lib/acl/rte_acl_osdep.h - /home/bruce/dpdk.org/lib/bpf/bpf_def.h - /home/bruce/dpdk.org/lib/compressdev/rte_compressdev_internal.h - /home/bruce/dpdk.org/lib/cryptodev/cryptodev_pmd.h - /home/bruce/dpdk.org/lib/eventdev/rte_event_ring.h - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd.h - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_pci.h - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_vdev.h - /home/bruce/dpdk.org/lib/kni/rte_kni_common.h - /home/bruce/dpdk.org/lib/vhost/rte_vdpa.h - /home/bruce/dpdk.org/lib/vhost/rte_vhost_async.h - /home/bruce/dpdk.org/lib/vhost/rte_vhost_crypto.h - /home/bruce/dpdk.org/lib/vhost/vdpa_driver.h Regards, /Bruce ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 15:03 ` Bruce Richardson @ 2022-02-14 17:24 ` Stephen Hemminger 2022-02-14 17:49 ` Thomas Monjalon 2022-02-14 18:03 ` Bruce Richardson 0 siblings, 2 replies; 14+ messages in thread From: Stephen Hemminger @ 2022-02-14 17:24 UTC (permalink / raw) To: Bruce Richardson Cc: Thomas Monjalon, dev, Tyler Retzlaff, ocardona, roretzla, david.marchand, ferruh.yigit On Mon, 14 Feb 2022 15:03:43 +0000 Bruce Richardson <bruce.richardson@intel.com> wrote: > On Mon, Feb 14, 2022 at 11:43:35AM +0000, Bruce Richardson wrote: > > On Mon, Feb 14, 2022 at 12:19:24PM +0100, Thomas Monjalon wrote: > > > 14/02/2022 11:45, Bruce Richardson: > > > > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > > > > 14/02/2022 10:13, Tyler Retzlaff: > > > > > > while the driver api is "internal" we agreed some time ago that drivers > > > > > > could be built external to the dpdk tree. by enabling the meson setup > > > > > > option -Denable_driver_sdk=true. > > > > > > > > > > > > it was agreed that the driver api was internal and would attract no > > > > > > binary compatibility support which was fine. this change has now > > > > > > imposed a further restriction that out of tree drivers have to be > > > > > > authored in C only as non-C++ compatible code will invariably leak into > > > > > > the internal structures. > > > > > > > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > > > > now you are preventing building of C++ drivers outside of the tree too. > > > > > > > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > > > > Did we agree to support out-of-tree drivers in C++? > > > > > > > > > > We really need to make things clear and written in documentation. > > > > > > > > > > > could we please re-evaluate this. > > > > > > > > > > Yes we can re-evaluate. > > > > > What is the list of impacted files? > > > > > > > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > > > > > > > /Bruce > > > > > > > > Message: SDK headers: > > > > Message: ethdev_driver.h > > > > Message: ethdev_pci.h > > > > Message: ethdev_vdev.h > > > > Message: cryptodev_pmd.h > > > > Message: eventdev_pmd.h > > > > Message: eventdev_pmd_pci.h > > > > Message: eventdev_pmd_vdev.h > > > > Message: eventdev_trace.h > > > > Message: event_timer_adapter_pmd.h > > > > Message: rte_dmadev_pmd.h > > > > Message: vdpa_driver.h > > > > > > I see no harm in supporting C++ include of these headers. > > > Any objection? > > > > > > Could we have a test in chkincs for the SDK headers? > > > > > Yes. It may make things a little more complicated, though, as it seems > > these headers also have a tendency to rely on some driver headers - > > specifically bus driver headers. > > > > Working on a patch to add these to checks. However, just running a c++ > compile does not check for valid 'extern "C"' blocks. Adding the following > to chkincs/meson.build as a basic sanity check throws up a larger list of > files to be looked at. > > Diff: > > +# check for extern C in files, since this is not detected as an error by the compiler > +grep = find_program('grep', required: false) > +if grep.found() > + errlist = run_command([grep, '--files-without-match', '^extern "C"', dpdk_chkinc_headers], > + check: true, capture: true).stdout().split() > + if errlist != [] > + error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- '.join(errlist)) > + endif > +endif > + > > Output: > ../buildtools/chkincs/meson.build:45:8: ERROR: Problem encountered: Files missing C++ 'extern "C"' guards: > - /home/bruce/dpdk.org/lib/telemetry/rte_telemetry.h > - /home/bruce/dpdk.org/lib/eal/include/rte_bitops.h > - /home/bruce/dpdk.org/lib/eal/include/rte_branch_prediction.h > - /home/bruce/dpdk.org/lib/eal/include/rte_compat.h > - /home/bruce/dpdk.org/lib/eal/include/rte_hypervisor.h > - /home/bruce/dpdk.org/lib/eal/include/rte_keepalive.h > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_feature_defs.h > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_features.h > - /home/bruce/dpdk.org/lib/eal/include/rte_time.h > - /home/bruce/dpdk.org/lib/eal/include/rte_trace_point_register.h > - /home/bruce/dpdk.org/lib/eal/linux/include/rte_os.h > - /home/bruce/dpdk.org/lib/ethdev/rte_dev_info.h > - /home/bruce/dpdk.org/lib/ethdev/ethdev_driver.h > - /home/bruce/dpdk.org/lib/ethdev/ethdev_pci.h > - /home/bruce/dpdk.org/lib/ethdev/ethdev_vdev.h > - /home/bruce/dpdk.org/lib/metrics/rte_metrics_telemetry.h > - /home/bruce/dpdk.org/lib/acl/rte_acl_osdep.h > - /home/bruce/dpdk.org/lib/bpf/bpf_def.h > - /home/bruce/dpdk.org/lib/compressdev/rte_compressdev_internal.h > - /home/bruce/dpdk.org/lib/cryptodev/cryptodev_pmd.h > - /home/bruce/dpdk.org/lib/eventdev/rte_event_ring.h > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd.h > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_pci.h > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_vdev.h > - /home/bruce/dpdk.org/lib/kni/rte_kni_common.h > - /home/bruce/dpdk.org/lib/vhost/rte_vdpa.h > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_async.h > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_crypto.h > - /home/bruce/dpdk.org/lib/vhost/vdpa_driver.h > > Regards, > /Bruce The actual C++ guards are small and have no impact therefore we should accept patches as needed. Having more guards than needed is just noise. But the responsibility of telling DPDK project where they are required should fall to any out-of-tree users who care. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 17:24 ` Stephen Hemminger @ 2022-02-14 17:49 ` Thomas Monjalon 2022-02-14 18:21 ` Stephen Hemminger 2022-02-14 18:03 ` Bruce Richardson 1 sibling, 1 reply; 14+ messages in thread From: Thomas Monjalon @ 2022-02-14 17:49 UTC (permalink / raw) To: Stephen Hemminger Cc: Bruce Richardson, dev, Tyler Retzlaff, ocardona, roretzla, david.marchand, ferruh.yigit 14/02/2022 18:t24, Stephen Hemminger: > On Mon, 14 Feb 2022 15:03:43 +0000 > Bruce Richardson <bruce.richardson@intel.com> wrote: > > > On Mon, Feb 14, 2022 at 11:43:35AM +0000, Bruce Richardson wrote: > > > On Mon, Feb 14, 2022 at 12:19:24PM +0100, Thomas Monjalon wrote: > > > > 14/02/2022 11:45, Bruce Richardson: > > > > > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > > > > > 14/02/2022 10:13, Tyler Retzlaff: > > > > > > > while the driver api is "internal" we agreed some time ago that drivers > > > > > > > could be built external to the dpdk tree. by enabling the meson setup > > > > > > > option -Denable_driver_sdk=true. > > > > > > > > > > > > > > it was agreed that the driver api was internal and would attract no > > > > > > > binary compatibility support which was fine. this change has now > > > > > > > imposed a further restriction that out of tree drivers have to be > > > > > > > authored in C only as non-C++ compatible code will invariably leak into > > > > > > > the internal structures. > > > > > > > > > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > > > > > now you are preventing building of C++ drivers outside of the tree too. > > > > > > > > > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > > > > > Did we agree to support out-of-tree drivers in C++? > > > > > > > > > > > > We really need to make things clear and written in documentation. > > > > > > > > > > > > > could we please re-evaluate this. > > > > > > > > > > > > Yes we can re-evaluate. > > > > > > What is the list of impacted files? > > > > > > > > > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > > > > > > > > > /Bruce > > > > > > > > > > Message: SDK headers: > > > > > Message: ethdev_driver.h > > > > > Message: ethdev_pci.h > > > > > Message: ethdev_vdev.h > > > > > Message: cryptodev_pmd.h > > > > > Message: eventdev_pmd.h > > > > > Message: eventdev_pmd_pci.h > > > > > Message: eventdev_pmd_vdev.h > > > > > Message: eventdev_trace.h > > > > > Message: event_timer_adapter_pmd.h > > > > > Message: rte_dmadev_pmd.h > > > > > Message: vdpa_driver.h > > > > > > > > I see no harm in supporting C++ include of these headers. > > > > Any objection? > > > > > > > > Could we have a test in chkincs for the SDK headers? > > > > > > > Yes. It may make things a little more complicated, though, as it seems > > > these headers also have a tendency to rely on some driver headers - > > > specifically bus driver headers. > > > > > > > Working on a patch to add these to checks. However, just running a c++ > > compile does not check for valid 'extern "C"' blocks. Adding the following > > to chkincs/meson.build as a basic sanity check throws up a larger list of > > files to be looked at. > > > > Diff: > > > > +# check for extern C in files, since this is not detected as an error by the compiler > > +grep = find_program('grep', required: false) > > +if grep.found() > > + errlist = run_command([grep, '--files-without-match', '^extern "C"', dpdk_chkinc_headers], > > + check: true, capture: true).stdout().split() > > + if errlist != [] > > + error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- '.join(errlist)) > > + endif > > +endif > > + > > > > Output: > > ../buildtools/chkincs/meson.build:45:8: ERROR: Problem encountered: Files missing C++ 'extern "C"' guards: > > - /home/bruce/dpdk.org/lib/telemetry/rte_telemetry.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_bitops.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_branch_prediction.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_compat.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_hypervisor.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_keepalive.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_feature_defs.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_features.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_time.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_trace_point_register.h > > - /home/bruce/dpdk.org/lib/eal/linux/include/rte_os.h > > - /home/bruce/dpdk.org/lib/ethdev/rte_dev_info.h > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_driver.h > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_pci.h > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_vdev.h > > - /home/bruce/dpdk.org/lib/metrics/rte_metrics_telemetry.h > > - /home/bruce/dpdk.org/lib/acl/rte_acl_osdep.h > > - /home/bruce/dpdk.org/lib/bpf/bpf_def.h > > - /home/bruce/dpdk.org/lib/compressdev/rte_compressdev_internal.h > > - /home/bruce/dpdk.org/lib/cryptodev/cryptodev_pmd.h > > - /home/bruce/dpdk.org/lib/eventdev/rte_event_ring.h > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd.h > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_pci.h > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_vdev.h > > - /home/bruce/dpdk.org/lib/kni/rte_kni_common.h > > - /home/bruce/dpdk.org/lib/vhost/rte_vdpa.h > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_async.h > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_crypto.h > > - /home/bruce/dpdk.org/lib/vhost/vdpa_driver.h > > > > Regards, > > /Bruce > > The actual C++ guards are small and have no impact therefore we > should accept patches as needed. Having more guards than needed is > just noise. > > But the responsibility of telling DPDK project where they are required > should fall to any out-of-tree users who care. Sorry Stephen I don't understand. Please could you rephrase your comment? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 17:49 ` Thomas Monjalon @ 2022-02-14 18:21 ` Stephen Hemminger 2022-02-14 18:32 ` Tyler Retzlaff 0 siblings, 1 reply; 14+ messages in thread From: Stephen Hemminger @ 2022-02-14 18:21 UTC (permalink / raw) To: Thomas Monjalon Cc: Bruce Richardson, dev, Tyler Retzlaff, ocardona, roretzla, david.marchand, ferruh.yigit On Mon, 14 Feb 2022 18:49:48 +0100 Thomas Monjalon <thomas@monjalon.net> wrote: > 14/02/2022 18:t24, Stephen Hemminger: > > On Mon, 14 Feb 2022 15:03:43 +0000 > > Bruce Richardson <bruce.richardson@intel.com> wrote: > > > > > On Mon, Feb 14, 2022 at 11:43:35AM +0000, Bruce Richardson wrote: > > > > On Mon, Feb 14, 2022 at 12:19:24PM +0100, Thomas Monjalon wrote: > > > > > 14/02/2022 11:45, Bruce Richardson: > > > > > > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > > > > > > 14/02/2022 10:13, Tyler Retzlaff: > > > > > > > > while the driver api is "internal" we agreed some time ago that drivers > > > > > > > > could be built external to the dpdk tree. by enabling the meson setup > > > > > > > > option -Denable_driver_sdk=true. > > > > > > > > > > > > > > > > it was agreed that the driver api was internal and would attract no > > > > > > > > binary compatibility support which was fine. this change has now > > > > > > > > imposed a further restriction that out of tree drivers have to be > > > > > > > > authored in C only as non-C++ compatible code will invariably leak into > > > > > > > > the internal structures. > > > > > > > > > > > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > > > > > > now you are preventing building of C++ drivers outside of the tree too. > > > > > > > > > > > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > > > > > > Did we agree to support out-of-tree drivers in C++? > > > > > > > > > > > > > > We really need to make things clear and written in documentation. > > > > > > > > > > > > > > > could we please re-evaluate this. > > > > > > > > > > > > > > Yes we can re-evaluate. > > > > > > > What is the list of impacted files? > > > > > > > > > > > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > > > > > > > > > > > /Bruce > > > > > > > > > > > > Message: SDK headers: > > > > > > Message: ethdev_driver.h > > > > > > Message: ethdev_pci.h > > > > > > Message: ethdev_vdev.h > > > > > > Message: cryptodev_pmd.h > > > > > > Message: eventdev_pmd.h > > > > > > Message: eventdev_pmd_pci.h > > > > > > Message: eventdev_pmd_vdev.h > > > > > > Message: eventdev_trace.h > > > > > > Message: event_timer_adapter_pmd.h > > > > > > Message: rte_dmadev_pmd.h > > > > > > Message: vdpa_driver.h > > > > > > > > > > I see no harm in supporting C++ include of these headers. > > > > > Any objection? > > > > > > > > > > Could we have a test in chkincs for the SDK headers? > > > > > > > > > Yes. It may make things a little more complicated, though, as it seems > > > > these headers also have a tendency to rely on some driver headers - > > > > specifically bus driver headers. > > > > > > > > > > Working on a patch to add these to checks. However, just running a c++ > > > compile does not check for valid 'extern "C"' blocks. Adding the following > > > to chkincs/meson.build as a basic sanity check throws up a larger list of > > > files to be looked at. > > > > > > Diff: > > > > > > +# check for extern C in files, since this is not detected as an error by the compiler > > > +grep = find_program('grep', required: false) > > > +if grep.found() > > > + errlist = run_command([grep, '--files-without-match', '^extern "C"', dpdk_chkinc_headers], > > > + check: true, capture: true).stdout().split() > > > + if errlist != [] > > > + error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- '.join(errlist)) > > > + endif > > > +endif > > > + > > > > > > Output: > > > ../buildtools/chkincs/meson.build:45:8: ERROR: Problem encountered: Files missing C++ 'extern "C"' guards: > > > - /home/bruce/dpdk.org/lib/telemetry/rte_telemetry.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_bitops.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_branch_prediction.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_compat.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_hypervisor.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_keepalive.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_feature_defs.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_features.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_time.h > > > - /home/bruce/dpdk.org/lib/eal/include/rte_trace_point_register.h > > > - /home/bruce/dpdk.org/lib/eal/linux/include/rte_os.h > > > - /home/bruce/dpdk.org/lib/ethdev/rte_dev_info.h > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_driver.h > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_pci.h > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_vdev.h > > > - /home/bruce/dpdk.org/lib/metrics/rte_metrics_telemetry.h > > > - /home/bruce/dpdk.org/lib/acl/rte_acl_osdep.h > > > - /home/bruce/dpdk.org/lib/bpf/bpf_def.h > > > - /home/bruce/dpdk.org/lib/compressdev/rte_compressdev_internal.h > > > - /home/bruce/dpdk.org/lib/cryptodev/cryptodev_pmd.h > > > - /home/bruce/dpdk.org/lib/eventdev/rte_event_ring.h > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd.h > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_pci.h > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_vdev.h > > > - /home/bruce/dpdk.org/lib/kni/rte_kni_common.h > > > - /home/bruce/dpdk.org/lib/vhost/rte_vdpa.h > > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_async.h > > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_crypto.h > > > - /home/bruce/dpdk.org/lib/vhost/vdpa_driver.h > > > > > > Regards, > > > /Bruce > > > > The actual C++ guards are small and have no impact therefore we > > should accept patches as needed. Having more guards than needed is > > just noise. > > > > But the responsibility of telling DPDK project where they are required > > should fall to any out-of-tree users who care. > > Sorry Stephen I don't understand. > Please could you rephrase your comment? Since there are no in-tree DPDK drivers that use C++ it is an extra effort to add infrastructure to test if headers are broken. As new features get added to the DPDK, it doesn't make sense for the upstream DPDK to guarantee that no driver API changed in a way that breaks C++. But if a breakage happens in C++ driver, the upstream DPDK is willing to accept patches to fix it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 18:21 ` Stephen Hemminger @ 2022-02-14 18:32 ` Tyler Retzlaff 2022-02-14 18:40 ` Tyler Retzlaff 0 siblings, 1 reply; 14+ messages in thread From: Tyler Retzlaff @ 2022-02-14 18:32 UTC (permalink / raw) To: Stephen Hemminger Cc: Thomas Monjalon, Bruce Richardson, dev, ocardona, roretzla, david.marchand, ferruh.yigit On Mon, Feb 14, 2022 at 10:21:20AM -0800, Stephen Hemminger wrote: > On Mon, 14 Feb 2022 18:49:48 +0100 > Thomas Monjalon <thomas@monjalon.net> wrote: > > > 14/02/2022 18:t24, Stephen Hemminger: > > > On Mon, 14 Feb 2022 15:03:43 +0000 > > > Bruce Richardson <bruce.richardson@intel.com> wrote: > > > > > > > On Mon, Feb 14, 2022 at 11:43:35AM +0000, Bruce Richardson wrote: > > > > > On Mon, Feb 14, 2022 at 12:19:24PM +0100, Thomas Monjalon wrote: > > > > > > 14/02/2022 11:45, Bruce Richardson: > > > > > > > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > > > > > > > 14/02/2022 10:13, Tyler Retzlaff: > > > > > > > > > while the driver api is "internal" we agreed some time ago that drivers > > > > > > > > > could be built external to the dpdk tree. by enabling the meson setup > > > > > > > > > option -Denable_driver_sdk=true. > > > > > > > > > > > > > > > > > > it was agreed that the driver api was internal and would attract no > > > > > > > > > binary compatibility support which was fine. this change has now > > > > > > > > > imposed a further restriction that out of tree drivers have to be > > > > > > > > > authored in C only as non-C++ compatible code will invariably leak into > > > > > > > > > the internal structures. > > > > > > > > > > > > > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > > > > > > > now you are preventing building of C++ drivers outside of the tree too. > > > > > > > > > > > > > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > > > > > > > Did we agree to support out-of-tree drivers in C++? > > > > > > > > > > > > > > > > We really need to make things clear and written in documentation. > > > > > > > > > > > > > > > > > could we please re-evaluate this. > > > > > > > > > > > > > > > > Yes we can re-evaluate. > > > > > > > > What is the list of impacted files? > > > > > > > > > > > > > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > > > > > > > > > > > > > /Bruce > > > > > > > > > > > > > > Message: SDK headers: > > > > > > > Message: ethdev_driver.h > > > > > > > Message: ethdev_pci.h > > > > > > > Message: ethdev_vdev.h > > > > > > > Message: cryptodev_pmd.h > > > > > > > Message: eventdev_pmd.h > > > > > > > Message: eventdev_pmd_pci.h > > > > > > > Message: eventdev_pmd_vdev.h > > > > > > > Message: eventdev_trace.h > > > > > > > Message: event_timer_adapter_pmd.h > > > > > > > Message: rte_dmadev_pmd.h > > > > > > > Message: vdpa_driver.h > > > > > > > > > > > > I see no harm in supporting C++ include of these headers. > > > > > > Any objection? > > > > > > > > > > > > Could we have a test in chkincs for the SDK headers? > > > > > > > > > > > Yes. It may make things a little more complicated, though, as it seems > > > > > these headers also have a tendency to rely on some driver headers - > > > > > specifically bus driver headers. > > > > > > > > > > > > > Working on a patch to add these to checks. However, just running a c++ > > > > compile does not check for valid 'extern "C"' blocks. Adding the following > > > > to chkincs/meson.build as a basic sanity check throws up a larger list of > > > > files to be looked at. > > > > > > > > Diff: > > > > > > > > +# check for extern C in files, since this is not detected as an error by the compiler > > > > +grep = find_program('grep', required: false) > > > > +if grep.found() > > > > + errlist = run_command([grep, '--files-without-match', '^extern "C"', dpdk_chkinc_headers], > > > > + check: true, capture: true).stdout().split() > > > > + if errlist != [] > > > > + error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- '.join(errlist)) > > > > + endif > > > > +endif > > > > + > > > > > > > > Output: > > > > ../buildtools/chkincs/meson.build:45:8: ERROR: Problem encountered: Files missing C++ 'extern "C"' guards: > > > > - /home/bruce/dpdk.org/lib/telemetry/rte_telemetry.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_bitops.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_branch_prediction.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_compat.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_hypervisor.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_keepalive.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_feature_defs.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_features.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_time.h > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_trace_point_register.h > > > > - /home/bruce/dpdk.org/lib/eal/linux/include/rte_os.h > > > > - /home/bruce/dpdk.org/lib/ethdev/rte_dev_info.h > > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_driver.h > > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_pci.h > > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_vdev.h > > > > - /home/bruce/dpdk.org/lib/metrics/rte_metrics_telemetry.h > > > > - /home/bruce/dpdk.org/lib/acl/rte_acl_osdep.h > > > > - /home/bruce/dpdk.org/lib/bpf/bpf_def.h > > > > - /home/bruce/dpdk.org/lib/compressdev/rte_compressdev_internal.h > > > > - /home/bruce/dpdk.org/lib/cryptodev/cryptodev_pmd.h > > > > - /home/bruce/dpdk.org/lib/eventdev/rte_event_ring.h > > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd.h > > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_pci.h > > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_vdev.h > > > > - /home/bruce/dpdk.org/lib/kni/rte_kni_common.h > > > > - /home/bruce/dpdk.org/lib/vhost/rte_vdpa.h > > > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_async.h > > > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_crypto.h > > > > - /home/bruce/dpdk.org/lib/vhost/vdpa_driver.h > > > > > > > > Regards, > > > > /Bruce > > > > > > The actual C++ guards are small and have no impact therefore we > > > should accept patches as needed. Having more guards than needed is > > > just noise. > > > > > > But the responsibility of telling DPDK project where they are required > > > should fall to any out-of-tree users who care. > > > > Sorry Stephen I don't understand. > > Please could you rephrase your comment? > > Since there are no in-tree DPDK drivers that use C++ it is an extra > effort to add infrastructure to test if headers are broken. As new > features get added to the DPDK, it doesn't make sense for the upstream > DPDK to guarantee that no driver API changed in a way that breaks C++. > But if a breakage happens in C++ driver, the upstream DPDK is willing > to accept patches to fix it. that is completely untenable. to have a policy where the consumers using c++ have to detect the non-compatible changes and then react by submitting patches to fix upstream is overly burdensome. it is far less overhead to prevent the non-compatible code creaping into the headers in the first place rather than pushing it down to the users of dpdk. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 18:32 ` Tyler Retzlaff @ 2022-02-14 18:40 ` Tyler Retzlaff 0 siblings, 0 replies; 14+ messages in thread From: Tyler Retzlaff @ 2022-02-14 18:40 UTC (permalink / raw) To: Stephen Hemminger Cc: Thomas Monjalon, Bruce Richardson, dev, ocardona, roretzla, david.marchand, ferruh.yigit On Mon, Feb 14, 2022 at 10:32:24AM -0800, Tyler Retzlaff wrote: > On Mon, Feb 14, 2022 at 10:21:20AM -0800, Stephen Hemminger wrote: > > On Mon, 14 Feb 2022 18:49:48 +0100 > > Thomas Monjalon <thomas@monjalon.net> wrote: > > > > > 14/02/2022 18:t24, Stephen Hemminger: > > > > On Mon, 14 Feb 2022 15:03:43 +0000 > > > > Bruce Richardson <bruce.richardson@intel.com> wrote: > > > > > > > > > On Mon, Feb 14, 2022 at 11:43:35AM +0000, Bruce Richardson wrote: > > > > > > On Mon, Feb 14, 2022 at 12:19:24PM +0100, Thomas Monjalon wrote: > > > > > > > 14/02/2022 11:45, Bruce Richardson: > > > > > > > > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > > > > > > > > 14/02/2022 10:13, Tyler Retzlaff: > > > > > > > > > > while the driver api is "internal" we agreed some time ago that drivers > > > > > > > > > > could be built external to the dpdk tree. by enabling the meson setup > > > > > > > > > > option -Denable_driver_sdk=true. > > > > > > > > > > > > > > > > > > > > it was agreed that the driver api was internal and would attract no > > > > > > > > > > binary compatibility support which was fine. this change has now > > > > > > > > > > imposed a further restriction that out of tree drivers have to be > > > > > > > > > > authored in C only as non-C++ compatible code will invariably leak into > > > > > > > > > > the internal structures. > > > > > > > > > > > > > > > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > > > > > > > > now you are preventing building of C++ drivers outside of the tree too. > > > > > > > > > > > > > > > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > > > > > > > > Did we agree to support out-of-tree drivers in C++? > > > > > > > > > > > > > > > > > > We really need to make things clear and written in documentation. > > > > > > > > > > > > > > > > > > > could we please re-evaluate this. > > > > > > > > > > > > > > > > > > Yes we can re-evaluate. > > > > > > > > > What is the list of impacted files? > > > > > > > > > > > > > > > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > > > > > > > > > > > > > > > /Bruce > > > > > > > > > > > > > > > > Message: SDK headers: > > > > > > > > Message: ethdev_driver.h > > > > > > > > Message: ethdev_pci.h > > > > > > > > Message: ethdev_vdev.h > > > > > > > > Message: cryptodev_pmd.h > > > > > > > > Message: eventdev_pmd.h > > > > > > > > Message: eventdev_pmd_pci.h > > > > > > > > Message: eventdev_pmd_vdev.h > > > > > > > > Message: eventdev_trace.h > > > > > > > > Message: event_timer_adapter_pmd.h > > > > > > > > Message: rte_dmadev_pmd.h > > > > > > > > Message: vdpa_driver.h > > > > > > > > > > > > > > I see no harm in supporting C++ include of these headers. > > > > > > > Any objection? > > > > > > > > > > > > > > Could we have a test in chkincs for the SDK headers? > > > > > > > > > > > > > Yes. It may make things a little more complicated, though, as it seems > > > > > > these headers also have a tendency to rely on some driver headers - > > > > > > specifically bus driver headers. > > > > > > > > > > > > > > > > Working on a patch to add these to checks. However, just running a c++ > > > > > compile does not check for valid 'extern "C"' blocks. Adding the following > > > > > to chkincs/meson.build as a basic sanity check throws up a larger list of > > > > > files to be looked at. > > > > > > > > > > Diff: > > > > > > > > > > +# check for extern C in files, since this is not detected as an error by the compiler > > > > > +grep = find_program('grep', required: false) > > > > > +if grep.found() > > > > > + errlist = run_command([grep, '--files-without-match', '^extern "C"', dpdk_chkinc_headers], > > > > > + check: true, capture: true).stdout().split() > > > > > + if errlist != [] > > > > > + error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- '.join(errlist)) > > > > > + endif > > > > > +endif > > > > > + > > > > > > > > > > Output: > > > > > ../buildtools/chkincs/meson.build:45:8: ERROR: Problem encountered: Files missing C++ 'extern "C"' guards: > > > > > - /home/bruce/dpdk.org/lib/telemetry/rte_telemetry.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_bitops.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_branch_prediction.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_compat.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_hypervisor.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_keepalive.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_feature_defs.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_features.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_time.h > > > > > - /home/bruce/dpdk.org/lib/eal/include/rte_trace_point_register.h > > > > > - /home/bruce/dpdk.org/lib/eal/linux/include/rte_os.h > > > > > - /home/bruce/dpdk.org/lib/ethdev/rte_dev_info.h > > > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_driver.h > > > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_pci.h > > > > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_vdev.h > > > > > - /home/bruce/dpdk.org/lib/metrics/rte_metrics_telemetry.h > > > > > - /home/bruce/dpdk.org/lib/acl/rte_acl_osdep.h > > > > > - /home/bruce/dpdk.org/lib/bpf/bpf_def.h > > > > > - /home/bruce/dpdk.org/lib/compressdev/rte_compressdev_internal.h > > > > > - /home/bruce/dpdk.org/lib/cryptodev/cryptodev_pmd.h > > > > > - /home/bruce/dpdk.org/lib/eventdev/rte_event_ring.h > > > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd.h > > > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_pci.h > > > > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_vdev.h > > > > > - /home/bruce/dpdk.org/lib/kni/rte_kni_common.h > > > > > - /home/bruce/dpdk.org/lib/vhost/rte_vdpa.h > > > > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_async.h > > > > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_crypto.h > > > > > - /home/bruce/dpdk.org/lib/vhost/vdpa_driver.h > > > > > > > > > > Regards, > > > > > /Bruce > > > > > > > > The actual C++ guards are small and have no impact therefore we > > > > should accept patches as needed. Having more guards than needed is > > > > just noise. > > > > > > > > But the responsibility of telling DPDK project where they are required > > > > should fall to any out-of-tree users who care. > > > > > > Sorry Stephen I don't understand. > > > Please could you rephrase your comment? > > > > Since there are no in-tree DPDK drivers that use C++ it is an extra > > effort to add infrastructure to test if headers are broken. As new > > features get added to the DPDK, it doesn't make sense for the upstream > > DPDK to guarantee that no driver API changed in a way that breaks C++. > > But if a breakage happens in C++ driver, the upstream DPDK is willing > > to accept patches to fix it. > > that is completely untenable. to have a policy where the consumers using > c++ have to detect the non-compatible changes and then react by > submitting patches to fix upstream is overly burdensome. > > it is far less overhead to prevent the non-compatible code creaping into > the headers in the first place rather than pushing it down to the users > of dpdk. s/creaping/creeping/ additionally on the topic of extra effort you should consider the perpetual submission and review of patches has non-zero overhead. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 17:24 ` Stephen Hemminger 2022-02-14 17:49 ` Thomas Monjalon @ 2022-02-14 18:03 ` Bruce Richardson 1 sibling, 0 replies; 14+ messages in thread From: Bruce Richardson @ 2022-02-14 18:03 UTC (permalink / raw) To: Stephen Hemminger Cc: Thomas Monjalon, dev, Tyler Retzlaff, ocardona, roretzla, david.marchand, ferruh.yigit On Mon, Feb 14, 2022 at 09:24:35AM -0800, Stephen Hemminger wrote: > On Mon, 14 Feb 2022 15:03:43 +0000 > Bruce Richardson <bruce.richardson@intel.com> wrote: > > > On Mon, Feb 14, 2022 at 11:43:35AM +0000, Bruce Richardson wrote: > > > On Mon, Feb 14, 2022 at 12:19:24PM +0100, Thomas Monjalon wrote: > > > > 14/02/2022 11:45, Bruce Richardson: > > > > > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > > > > > 14/02/2022 10:13, Tyler Retzlaff: > > > > > > > while the driver api is "internal" we agreed some time ago that drivers > > > > > > > could be built external to the dpdk tree. by enabling the meson setup > > > > > > > option -Denable_driver_sdk=true. > > > > > > > > > > > > > > it was agreed that the driver api was internal and would attract no > > > > > > > binary compatibility support which was fine. this change has now > > > > > > > imposed a further restriction that out of tree drivers have to be > > > > > > > authored in C only as non-C++ compatible code will invariably leak into > > > > > > > the internal structures. > > > > > > > > > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > > > > > now you are preventing building of C++ drivers outside of the tree too. > > > > > > > > > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > > > > > Did we agree to support out-of-tree drivers in C++? > > > > > > > > > > > > We really need to make things clear and written in documentation. > > > > > > > > > > > > > could we please re-evaluate this. > > > > > > > > > > > > Yes we can re-evaluate. > > > > > > What is the list of impacted files? > > > > > > > > > > > Hacking meson files a bit, the list of SDK header files is reported as below. > > > > > > > > > > /Bruce > > > > > > > > > > Message: SDK headers: > > > > > Message: ethdev_driver.h > > > > > Message: ethdev_pci.h > > > > > Message: ethdev_vdev.h > > > > > Message: cryptodev_pmd.h > > > > > Message: eventdev_pmd.h > > > > > Message: eventdev_pmd_pci.h > > > > > Message: eventdev_pmd_vdev.h > > > > > Message: eventdev_trace.h > > > > > Message: event_timer_adapter_pmd.h > > > > > Message: rte_dmadev_pmd.h > > > > > Message: vdpa_driver.h > > > > > > > > I see no harm in supporting C++ include of these headers. > > > > Any objection? > > > > > > > > Could we have a test in chkincs for the SDK headers? > > > > > > > Yes. It may make things a little more complicated, though, as it seems > > > these headers also have a tendency to rely on some driver headers - > > > specifically bus driver headers. > > > > > > > Working on a patch to add these to checks. However, just running a c++ > > compile does not check for valid 'extern "C"' blocks. Adding the following > > to chkincs/meson.build as a basic sanity check throws up a larger list of > > files to be looked at. > > > > Diff: > > > > +# check for extern C in files, since this is not detected as an error by the compiler > > +grep = find_program('grep', required: false) > > +if grep.found() > > + errlist = run_command([grep, '--files-without-match', '^extern "C"', dpdk_chkinc_headers], > > + check: true, capture: true).stdout().split() > > + if errlist != [] > > + error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- '.join(errlist)) > > + endif > > +endif > > + > > > > Output: > > ../buildtools/chkincs/meson.build:45:8: ERROR: Problem encountered: Files missing C++ 'extern "C"' guards: > > - /home/bruce/dpdk.org/lib/telemetry/rte_telemetry.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_bitops.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_branch_prediction.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_compat.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_hypervisor.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_keepalive.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_feature_defs.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_pci_dev_features.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_time.h > > - /home/bruce/dpdk.org/lib/eal/include/rte_trace_point_register.h > > - /home/bruce/dpdk.org/lib/eal/linux/include/rte_os.h > > - /home/bruce/dpdk.org/lib/ethdev/rte_dev_info.h > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_driver.h > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_pci.h > > - /home/bruce/dpdk.org/lib/ethdev/ethdev_vdev.h > > - /home/bruce/dpdk.org/lib/metrics/rte_metrics_telemetry.h > > - /home/bruce/dpdk.org/lib/acl/rte_acl_osdep.h > > - /home/bruce/dpdk.org/lib/bpf/bpf_def.h > > - /home/bruce/dpdk.org/lib/compressdev/rte_compressdev_internal.h > > - /home/bruce/dpdk.org/lib/cryptodev/cryptodev_pmd.h > > - /home/bruce/dpdk.org/lib/eventdev/rte_event_ring.h > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd.h > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_pci.h > > - /home/bruce/dpdk.org/lib/eventdev/eventdev_pmd_vdev.h > > - /home/bruce/dpdk.org/lib/kni/rte_kni_common.h > > - /home/bruce/dpdk.org/lib/vhost/rte_vdpa.h > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_async.h > > - /home/bruce/dpdk.org/lib/vhost/rte_vhost_crypto.h > > - /home/bruce/dpdk.org/lib/vhost/vdpa_driver.h > > > > Regards, > > /Bruce > > The actual C++ guards are small and have no impact therefore we > should accept patches as needed. Having more guards than needed is > just noise. > > But the responsibility of telling DPDK project where they are required > should fall to any out-of-tree users who care. The guards in the above files are all needed, I believe. The scan for chkincs only runs on header files that are public, or part of the driver_sdk set. Internal headers aren't scanned and don't need the guards. /Bruce ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-14 9:22 ` Thomas Monjalon 2022-02-14 10:45 ` Bruce Richardson @ 2022-02-15 6:10 ` Tyler Retzlaff 2022-02-15 8:55 ` Thomas Monjalon 1 sibling, 1 reply; 14+ messages in thread From: Tyler Retzlaff @ 2022-02-15 6:10 UTC (permalink / raw) To: Thomas Monjalon Cc: dev, ocardona, roretzla, david.marchand, ferruh.yigit, bruce.richardson On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > 14/02/2022 10:13, Tyler Retzlaff: > > while the driver api is "internal" we agreed some time ago that drivers > > could be built external to the dpdk tree. by enabling the meson setup > > option -Denable_driver_sdk=true. > > > > it was agreed that the driver api was internal and would attract no > > binary compatibility support which was fine. this change has now > > imposed a further restriction that out of tree drivers have to be > > authored in C only as non-C++ compatible code will invariably leak into > > the internal structures. > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > now you are preventing building of C++ drivers outside of the tree too. > > That's the problem of non-written assumptions, they are unknown or forgotten. > Did we agree to support out-of-tree drivers in C++? > > We really need to make things clear and written in documentation. how / where would you prefer this be documented? thanks ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: out of tree driver builds broken with C++ 2022-02-15 6:10 ` Tyler Retzlaff @ 2022-02-15 8:55 ` Thomas Monjalon 0 siblings, 0 replies; 14+ messages in thread From: Thomas Monjalon @ 2022-02-15 8:55 UTC (permalink / raw) To: Tyler Retzlaff Cc: dev, ocardona, roretzla, david.marchand, ferruh.yigit, bruce.richardson 15/02/2022 07:10, Tyler Retzlaff: > On Mon, Feb 14, 2022 at 10:22:08AM +0100, Thomas Monjalon wrote: > > 14/02/2022 10:13, Tyler Retzlaff: > > > while the driver api is "internal" we agreed some time ago that drivers > > > could be built external to the dpdk tree. by enabling the meson setup > > > option -Denable_driver_sdk=true. > > > > > > it was agreed that the driver api was internal and would attract no > > > binary compatibility support which was fine. this change has now > > > imposed a further restriction that out of tree drivers have to be > > > authored in C only as non-C++ compatible code will invariably leak into > > > the internal structures. > > > > > > you won't allow us to build C++ drivers in the dpdk tree and it seems > > > now you are preventing building of C++ drivers outside of the tree too. > > > > That's the problem of non-written assumptions, they are unknown or forgotten. > > Did we agree to support out-of-tree drivers in C++? > > > > We really need to make things clear and written in documentation. > > how / where would you prefer this be documented? I think we could add a general explanation about driver SDK in doc/guides/contributing/design.rst ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-02-15 8:55 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-14 9:13 out of tree driver builds broken with C++ Tyler Retzlaff 2022-02-14 9:22 ` Thomas Monjalon 2022-02-14 10:45 ` Bruce Richardson 2022-02-14 11:19 ` Thomas Monjalon 2022-02-14 11:43 ` Bruce Richardson 2022-02-14 15:03 ` Bruce Richardson 2022-02-14 17:24 ` Stephen Hemminger 2022-02-14 17:49 ` Thomas Monjalon 2022-02-14 18:21 ` Stephen Hemminger 2022-02-14 18:32 ` Tyler Retzlaff 2022-02-14 18:40 ` Tyler Retzlaff 2022-02-14 18:03 ` Bruce Richardson 2022-02-15 6:10 ` Tyler Retzlaff 2022-02-15 8:55 ` Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).