DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] eal: enforce internal API check with Clang
@ 2026-10-14 20:16 Thomas Monjalon
  2026-10-15  8:54 ` Burakov, Anatoly
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Thomas Monjalon @ 2026-10-14 20:16 UTC (permalink / raw)
  To: dev; +Cc: stable, Bruce Richardson

The __rte_internal macro is intended to prevent calls
to internal API functions unless ALLOW_INTERNAL_API is defined.
Unfortunately the check misses some calls to internal API
when built with GCC or Clang.
Clang can be better to diagnose such cases.

Clang 14 and later support the error attribute
and therefore select it instead of diagnose_if for __rte_internal.
Unlike diagnose_if, the error attribute only rejects
internal API calls that remain after optimization.
Internal API calls can therefore go easily unnoticed,
making internal API checks less strict with recent Clang.

Prefer diagnose_if when available to keep strong Clang diagnostics.
Retain the error attribute as a fallback for GCC.

Fixes: 4ab63cd60ceb ("eal: fix internal ABI tag with clang")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/eal/include/rte_compat.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index 97c1540bd0..b62e49d676 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -32,17 +32,7 @@ __attribute__((section(".text.experimental")))
 #define __has_attribute(x) 0
 #endif
 
-#if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
-
-#ifdef RTE_TOOLCHAIN_MSVC
-#define __rte_internal
-#else
-#define __rte_internal \
-__attribute__((error("Symbol is not public ABI"), \
-section(".text.internal")))
-#endif
-
-#elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
+#if !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For Clang */
 
 #ifdef RTE_TOOLCHAIN_MSVC
 #define __rte_internal
@@ -55,6 +45,16 @@ section(".text.internal"))) \
 _Pragma("GCC diagnostic pop")
 #endif
 
+#elif !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
+
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
+#define __rte_internal \
+__attribute__((error("Symbol is not public ABI"), \
+section(".text.internal")))
+#endif
+
 #else
 
 #ifdef RTE_TOOLCHAIN_MSVC
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] eal: enforce internal API check with Clang
  2026-10-14 20:16 [PATCH] eal: enforce internal API check with Clang Thomas Monjalon
@ 2026-10-15  8:54 ` Burakov, Anatoly
  2026-10-15 10:57 ` David Marchand
  2026-10-15 11:29 ` [PATCH v2 1/3] buildtools/chkincs: relax checks on driver headers David Marchand
  2 siblings, 0 replies; 9+ messages in thread
From: Burakov, Anatoly @ 2026-10-15  8:54 UTC (permalink / raw)
  To: Thomas Monjalon, dev; +Cc: stable, Bruce Richardson

On 10/14/2026 10:16 PM, Thomas Monjalon wrote:
> The __rte_internal macro is intended to prevent calls
> to internal API functions unless ALLOW_INTERNAL_API is defined.
> Unfortunately the check misses some calls to internal API
> when built with GCC or Clang.
> Clang can be better to diagnose such cases.
> 
> Clang 14 and later support the error attribute
> and therefore select it instead of diagnose_if for __rte_internal.
> Unlike diagnose_if, the error attribute only rejects
> internal API calls that remain after optimization.
> Internal API calls can therefore go easily unnoticed,
> making internal API checks less strict with recent Clang.
> 
> Prefer diagnose_if when available to keep strong Clang diagnostics.
> Retain the error attribute as a fallback for GCC.
> 
> Fixes: 4ab63cd60ceb ("eal: fix internal ABI tag with clang")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] eal: enforce internal API check with Clang
  2026-10-14 20:16 [PATCH] eal: enforce internal API check with Clang Thomas Monjalon
  2026-10-15  8:54 ` Burakov, Anatoly
@ 2026-10-15 10:57 ` David Marchand
  2026-10-15 11:29 ` [PATCH v2 1/3] buildtools/chkincs: relax checks on driver headers David Marchand
  2 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2026-10-15 10:57 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, stable, Bruce Richardson, Burakov, Anatoly

On Wed, 14 Oct 2026 at 22:16, Thomas Monjalon <thomas@monjalon.net> wrote:
>
> The __rte_internal macro is intended to prevent calls
> to internal API functions unless ALLOW_INTERNAL_API is defined.
> Unfortunately the check misses some calls to internal API
> when built with GCC or Clang.
> Clang can be better to diagnose such cases.
>
> Clang 14 and later support the error attribute
> and therefore select it instead of diagnose_if for __rte_internal.
> Unlike diagnose_if, the error attribute only rejects
> internal API calls that remain after optimization.
> Internal API calls can therefore go easily unnoticed,
> making internal API checks less strict with recent Clang.
>
> Prefer diagnose_if when available to keep strong Clang diagnostics.
> Retain the error attribute as a fallback for GCC.
>
> Fixes: 4ab63cd60ceb ("eal: fix internal ABI tag with clang")
> Cc: stable@dpdk.org
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

What a mess this header has become :-).

I have a little cleanup to propose, and this patch actually does not
pass the chkinc test with recent clang now, as it raises warnings that
were not seen so far.
I'll post a v2, as we discussed offlist.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] buildtools/chkincs: relax checks on driver headers
  2026-10-14 20:16 [PATCH] eal: enforce internal API check with Clang Thomas Monjalon
  2026-10-15  8:54 ` Burakov, Anatoly
  2026-10-15 10:57 ` David Marchand
@ 2026-10-15 11:29 ` David Marchand
  2026-10-15 11:29   ` [PATCH v2 2/3] eal: simplify internal/experimental tags definitions David Marchand
  2026-10-15 11:29   ` [PATCH v2 3/3] eal: enforce internal API check with Clang 14 David Marchand
  2 siblings, 2 replies; 9+ messages in thread
From: David Marchand @ 2026-10-15 11:29 UTC (permalink / raw)
  To: dev; +Cc: thomas, bruce.richardson, anatoly.burakov

Driver headers are common consumers of internal APIs, like for example
the ethdev_pci.h header consumed by net PCI drivers, which calls
internal symbols for allocating/releasing a ethdev port.

Checking that a driver header compiles without allowing internal APIs is
then highly likely to fail.

Relax the check and stop expecting driver headers could compile without
ALLOW_INTERNAL_API.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 buildtools/chkincs/meson.build | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 9a84c1cc25..df674f74ee 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -49,19 +49,13 @@ sources_drivers = files('main.c')
 sources_drivers += gen_c_files.process(dpdk_drivers_headers)
 
 executable('chkincs-drv', sources_drivers,
-        c_args: cflags,
-        include_directories: includes + includes_drivers,
-        dependencies: deps + deps_drivers,
-        install: false)
-
-executable('chkincs-drv-exp', sources_drivers,
-        c_args: [cflags, '-DALLOW_EXPERIMENTAL_API'],
+        c_args: [cflags, '-DALLOW_INTERNAL_API'],
         include_directories: includes + includes_drivers,
         dependencies: deps + deps_drivers,
         install: false)
 
 executable('chkincs-drv-all', sources_drivers,
-        c_args: [cflags, '-DALLOW_EXPERIMENTAL_API', '-DALLOW_INTERNAL_API'],
+        c_args: [cflags, '-DALLOW_INTERNAL_API', '-DALLOW_EXPERIMENTAL_API'],
         include_directories: includes + includes_drivers,
         dependencies: deps + deps_drivers,
         install: false)
@@ -101,20 +95,14 @@ cpp_sources_drivers = files('main.cpp')
 cpp_sources_drivers += gen_cpp_files.process(dpdk_drivers_headers)
 
 executable('chkincs-drv-cpp', cpp_sources_drivers,
-        cpp_args: ['-include', 'rte_config.h', cflags],
-        include_directories: includes + includes_drivers,
-        dependencies: deps + deps_drivers,
-        install: false)
-
-executable('chkincs-drv-cpp-exp', cpp_sources_drivers,
-        cpp_args: ['-include', 'rte_config.h', cflags, '-DALLOW_EXPERIMENTAL_API'],
+        cpp_args: ['-include', 'rte_config.h', cflags, '-DALLOW_INTERNAL_API'],
         include_directories: includes + includes_drivers,
         dependencies: deps + deps_drivers,
         install: false)
 
 executable('chkincs-drv-cpp-all', cpp_sources_drivers,
-        cpp_args: ['-include', 'rte_config.h', cflags, '-DALLOW_EXPERIMENTAL_API',
-                   '-DALLOW_INTERNAL_API'],
+        cpp_args: ['-include', 'rte_config.h', cflags, '-DALLOW_INTERNAL_API',
+                   '-DALLOW_EXPERIMENTAL_API'],
         include_directories: includes + includes_drivers,
         dependencies: deps + deps_drivers,
         install: false)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] eal: simplify internal/experimental tags definitions
  2026-10-15 11:29 ` [PATCH v2 1/3] buildtools/chkincs: relax checks on driver headers David Marchand
@ 2026-10-15 11:29   ` David Marchand
  2026-10-15 11:29   ` [PATCH v2 3/3] eal: enforce internal API check with Clang 14 David Marchand
  1 sibling, 0 replies; 9+ messages in thread
From: David Marchand @ 2026-10-15 11:29 UTC (permalink / raw)
  To: dev; +Cc: thomas, bruce.richardson, anatoly.burakov

The tags definitions have become hard to read after adding support for
various compilers.

Let's reorder the definitions:
- put MSVC first, as it offers neither the section attribute nor helps
  with detecting wrong use of symbols,
- then for each tag
  - start with error/diagnostic tweaks for the respective !ALLOW cases,
  - if no tweak is available, and for the ALLOW cases, fallback to
    moving the symbol to a section,

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/include/rte_compat.h | 39 +++++++++++++++---------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index 97c1540bd0..cb7836a190 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -6,64 +6,57 @@
 #ifndef _RTE_COMPAT_H_
 #define _RTE_COMPAT_H_
 
-#ifndef ALLOW_EXPERIMENTAL_API
-
 #ifdef RTE_TOOLCHAIN_MSVC
+
 #define __rte_experimental
-#else
+#define __rte_internal
+
+#else /* !RTE_TOOLCHAIN_MSVC */
+
+#ifndef ALLOW_EXPERIMENTAL_API
+
 #define __rte_experimental \
 __attribute__((deprecated("Symbol is not yet part of stable ABI"), \
 section(".text.experimental")))
-#endif
 
-#else
+#endif /* !ALLOW_EXPERIMENTAL_API */
 
-#ifdef RTE_TOOLCHAIN_MSVC
-#define __rte_experimental
-#else
+#ifndef __rte_experimental
 #define __rte_experimental \
 __attribute__((section(".text.experimental")))
 #endif
 
-#endif
+#ifndef ALLOW_INTERNAL_API
 
 #ifndef __has_attribute
 /* if no has_attribute assume no support for attribute too */
 #define __has_attribute(x) 0
 #endif
 
-#if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
+#if __has_attribute(error) /* For GCC */
 
-#ifdef RTE_TOOLCHAIN_MSVC
-#define __rte_internal
-#else
 #define __rte_internal \
 __attribute__((error("Symbol is not public ABI"), \
 section(".text.internal")))
-#endif
 
-#elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
+#elif __has_attribute(diagnose_if) /* For clang */
 
-#ifdef RTE_TOOLCHAIN_MSVC
-#define __rte_internal
-#else
 #define __rte_internal \
 _Pragma("GCC diagnostic push") \
 _Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
 section(".text.internal"))) \
 _Pragma("GCC diagnostic pop")
+
 #endif
 
-#else
+#endif /* !ALLOW_INTERNAL_API */
 
-#ifdef RTE_TOOLCHAIN_MSVC
-#define __rte_internal
-#else
+#ifndef __rte_internal
 #define __rte_internal \
 __attribute__((section(".text.internal")))
 #endif
 
-#endif
+#endif /* !RTE_TOOLCHAIN_MSVC */
 
 #endif /* _RTE_COMPAT_H_ */
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] eal: enforce internal API check with Clang 14
  2026-10-15 11:29 ` [PATCH v2 1/3] buildtools/chkincs: relax checks on driver headers David Marchand
  2026-10-15 11:29   ` [PATCH v2 2/3] eal: simplify internal/experimental tags definitions David Marchand
@ 2026-10-15 11:29   ` David Marchand
  2026-10-15 11:31     ` Burakov, Anatoly
  2026-10-15 11:40     ` David Marchand
  1 sibling, 2 replies; 9+ messages in thread
From: David Marchand @ 2026-10-15 11:29 UTC (permalink / raw)
  To: dev; +Cc: thomas, bruce.richardson, anatoly.burakov, stable

From: Thomas Monjalon <thomas@monjalon.net>

The __rte_internal macro is intended to prevent calls
to internal API functions unless ALLOW_INTERNAL_API is defined.
Unfortunately the check misses some calls to internal API
when built with GCC or Clang.
Clang can be better to diagnose such cases.

Clang 14 and later support the error attribute
and therefore select it instead of diagnose_if for __rte_internal.
Unlike diagnose_if, the error attribute only rejects
internal API calls that remain after optimization.
Internal API calls can therefore go easily unnoticed,
making internal API checks less strict with recent Clang.

Prefer diagnose_if when available to keep strong Clang diagnostics.
Retain the error attribute as a fallback for GCC.

Fixes: 4ab63cd60ceb ("eal: fix internal ABI tag with clang")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/eal/include/rte_compat.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index cb7836a190..89c3465245 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -33,13 +33,7 @@ __attribute__((section(".text.experimental")))
 #define __has_attribute(x) 0
 #endif
 
-#if __has_attribute(error) /* For GCC */
-
-#define __rte_internal \
-__attribute__((error("Symbol is not public ABI"), \
-section(".text.internal")))
-
-#elif __has_attribute(diagnose_if) /* For clang */
+#if __has_attribute(diagnose_if) /* For Clang */
 
 #define __rte_internal \
 _Pragma("GCC diagnostic push") \
@@ -48,6 +42,12 @@ __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
 section(".text.internal"))) \
 _Pragma("GCC diagnostic pop")
 
+#elif __has_attribute(error) /* For GCC */
+
+#define __rte_internal \
+__attribute__((error("Symbol is not public ABI"), \
+section(".text.internal")))
+
 #endif
 
 #endif /* !ALLOW_INTERNAL_API */
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/3] eal: enforce internal API check with Clang 14
  2026-10-15 11:29   ` [PATCH v2 3/3] eal: enforce internal API check with Clang 14 David Marchand
@ 2026-10-15 11:31     ` Burakov, Anatoly
  2026-10-15 11:38       ` David Marchand
  2026-10-15 11:40     ` David Marchand
  1 sibling, 1 reply; 9+ messages in thread
From: Burakov, Anatoly @ 2026-10-15 11:31 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: thomas, bruce.richardson, stable

On 10/15/2026 1:29 PM, David Marchand wrote:
> From: Thomas Monjalon <thomas@monjalon.net>
> 
> The __rte_internal macro is intended to prevent calls
> to internal API functions unless ALLOW_INTERNAL_API is defined.
> Unfortunately the check misses some calls to internal API
> when built with GCC or Clang.
> Clang can be better to diagnose such cases.
> 
> Clang 14 and later support the error attribute
> and therefore select it instead of diagnose_if for __rte_internal.
> Unlike diagnose_if, the error attribute only rejects
> internal API calls that remain after optimization.
> Internal API calls can therefore go easily unnoticed,
> making internal API checks less strict with recent Clang.
> 
> Prefer diagnose_if when available to keep strong Clang diagnostics.
> Retain the error attribute as a fallback for GCC.
> 
> Fixes: 4ab63cd60ceb ("eal: fix internal ABI tag with clang")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---

Missing my ack

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/3] eal: enforce internal API check with Clang 14
  2026-10-15 11:31     ` Burakov, Anatoly
@ 2026-10-15 11:38       ` David Marchand
  0 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2026-10-15 11:38 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, thomas, bruce.richardson, stable

On Thu, 15 Oct 2026 at 13:32, Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 10/15/2026 1:29 PM, David Marchand wrote:
> > From: Thomas Monjalon <thomas@monjalon.net>
> >
> > The __rte_internal macro is intended to prevent calls
> > to internal API functions unless ALLOW_INTERNAL_API is defined.
> > Unfortunately the check misses some calls to internal API
> > when built with GCC or Clang.
> > Clang can be better to diagnose such cases.
> >
> > Clang 14 and later support the error attribute
> > and therefore select it instead of diagnose_if for __rte_internal.
> > Unlike diagnose_if, the error attribute only rejects
> > internal API calls that remain after optimization.
> > Internal API calls can therefore go easily unnoticed,
> > making internal API checks less strict with recent Clang.
> >
> > Prefer diagnose_if when available to keep strong Clang diagnostics.
> > Retain the error attribute as a fallback for GCC.
> >
> > Fixes: 4ab63cd60ceb ("eal: fix internal ABI tag with clang")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
>
> Missing my ack
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

I changed the original patch a bit, and well, I had trouble with all
those nested #ifdef ;-), so I preferred not preserving the ack.
Thanks for checking again.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/3] eal: enforce internal API check with Clang 14
  2026-10-15 11:29   ` [PATCH v2 3/3] eal: enforce internal API check with Clang 14 David Marchand
  2026-10-15 11:31     ` Burakov, Anatoly
@ 2026-10-15 11:40     ` David Marchand
  1 sibling, 0 replies; 9+ messages in thread
From: David Marchand @ 2026-10-15 11:40 UTC (permalink / raw)
  To: thomas; +Cc: dev, bruce.richardson, anatoly.burakov, stable, Aaron Conole

On Thu, 15 Oct 2026 at 13:29, David Marchand <david.marchand@redhat.com> wrote:
>
> From: Thomas Monjalon <thomas@monjalon.net>
>
> The __rte_internal macro is intended to prevent calls
> to internal API functions unless ALLOW_INTERNAL_API is defined.
> Unfortunately the check misses some calls to internal API
> when built with GCC or Clang.
> Clang can be better to diagnose such cases.
>
> Clang 14 and later support the error attribute
> and therefore select it instead of diagnose_if for __rte_internal.
> Unlike diagnose_if, the error attribute only rejects
> internal API calls that remain after optimization.
> Internal API calls can therefore go easily unnoticed,
> making internal API checks less strict with recent Clang.
>
> Prefer diagnose_if when available to keep strong Clang diagnostics.
> Retain the error attribute as a fallback for GCC.
>
> Fixes: 4ab63cd60ceb ("eal: fix internal ABI tag with clang")
> Cc: stable@dpdk.org
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Just a note about CI.
It seems we have some trouble with the ovsrobot but this series passes
GHA in my repo:
https://github.com/david-marchand/dpdk/actions/runs/38725255308


-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-15 11:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14 20:16 [PATCH] eal: enforce internal API check with Clang Thomas Monjalon
2026-10-15  8:54 ` Burakov, Anatoly
2026-10-15 10:57 ` David Marchand
2026-10-15 11:29 ` [PATCH v2 1/3] buildtools/chkincs: relax checks on driver headers David Marchand
2026-10-15 11:29   ` [PATCH v2 2/3] eal: simplify internal/experimental tags definitions David Marchand
2026-10-15 11:29   ` [PATCH v2 3/3] eal: enforce internal API check with Clang 14 David Marchand
2026-10-15 11:31     ` Burakov, Anatoly
2026-10-15 11:38       ` David Marchand
2026-10-15 11:40     ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox