All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: David Marchand <david.marchand@redhat.com>
Cc: dev@dpdk.org, bruce.richardson@intel.com
Subject: Re: [PATCH v4 0/9] Limit usage of internal API in tests
Date: Tue, 29 Sep 2026 12:50:00 -0700	[thread overview]
Message-ID: <20260929125000.4800e8a2@phoenix.local> (raw)
In-Reply-To: <20260922094026.2636265-1-david.marchand@redhat.com>

On Tue, 22 Sep 2026 11:40:16 +0200
David Marchand <david.marchand@redhat.com> wrote:

> We had a few bug reports related to internal (and experimental) symbols
> issues during 26.07 development.
> See for example https://bugs.dpdk.org/show_bug.cgi?id=1957 or more
> recently https://bugs.dpdk.org/show_bug.cgi?id=1967.
> 
> To catch such issues earlier in the CI, this series proposes to run
> the unit tests through meson with the ABI reference unit test binary
> against the current ABI libraries and drivers.
> 
> For this to work, some unit tests must be skipped (since meson may
> invoke the ABI reference code with tests that were unknown at the time).
> 
> A few unit tests were directly dereferencing internal structures and are
> reworked so they use public APIs.
> 
> Additionally, unit tests were allowed to use any internal API which has
> hidden a few issues (like a public API backed by internal symbols in the
> hash library).
> So disable the global ALLLOW_INTERNAL_API and move it to code explicitly
> requiring internal API, with the hope it will push us to have better API.

 
I think this causing breakage with minsize build. It is not correct to use
__rte_internal on inline helper functions in header file. I checked and only
thash has that anti-pattern.


In file included from ../lib/hash/rte_thash_gfni.h:13,
                 from ../lib/hash/rte_thash.h:23,
                 from ../app/test/test_thash_perf.c:13:
In function ‘rte_thash_gfni’,
    inlined from ‘run_rss_calc’ at ../app/test/test_thash_perf.c:58:13:
../lib/hash/rte_thash_x86_gfni.h:181:27: error: call to ‘__rte_thash_gfni’ declared with attribute error: Symbol is not public ABI
  181 |         __m512i xor_acc = __rte_thash_gfni(m, tuple, NULL, len);
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘rte_thash_gfni_bulk’,
    inlined from ‘run_rss_calc_bulk’ at ../app/test/test_thash_perf.c:79:3,
    inlined from ‘run_thash_test’ at ../app/test/test_thash_perf.c:122:13:
../lib/hash/rte_thash_x86_gfni.h:213:27: error: call to ‘__rte_thash_gfni’ declared with attribute error: Symbol is not public ABI
  213 |                 xor_acc = __rte_thash_gfni(mtrx, tuple[i], tuple[i + 1], len);
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


AI analysis:

  Pre-existing: the actual bug

  lib/hash/rte_thash_x86_gfni.h:32 and :67 put __rte_internal on two static inline
  helpers, __rte_thash_xor_reduce and __rte_thash_gfni. The public, stable (per
  b9dd86db2a) wrappers rte_thash_gfni() / rte_thash_gfni_bulk() call them. Those
  markers date to 4fd8c4cb0d ("hash: add new Toeplitz hash implementation") —
  they've always been wrong. Any external application including rte_thash.h without
  ALLOW_INTERNAL_API was already broken; 1f1c91397c just made DPDK's own tree hit
  the same wall.

  This is the same class of bug da31ef54c8 ("hash: fix GFNI stubs export") fixed
  for the out-of-line stubs — it missed the inline helpers.

  Why only minsize

  __rte_internal expands to __attribute__((error(...))), which only fires if the
  call survives to codegen:

  -O0 : 6    -O1 : 0    -O2 : 0    -O3 : 0    -Os : 2

  At -O1/-O2/-O3 GCC inlines the helper and the call vanishes. At -Os it declines
  to inline (function too large), at -O0 it never inlines. So the marker was never
  actually enforcing anything at the default optimization levels — it only
  detonates under specific inlining decisions. minsize and a debug build are the
  two configs that expose it.

  Recommended fix

  Drop __rte_internal from those two static inline helpers, rather than restoring
  ALLOW_INTERNAL_API to app/test. __rte_internal on a static inline in an installed
  public header is wrong by construction: there's no exported symbol to protect,
  and it makes the public wrappers unusable from outside DPDK.

  Verified: with those two markers removed, test_thash.c and test_thash_perf.c both
  compile clean at -O0 and -Os (6→0 and 2→0). I grepped the rest of lib/ and
  drivers/ for __rte_internal immediately preceding static inline in public headers
  — these two are the only instances, so a single small patch covers it. Fixes:
  should point at 4fd8c4cb0d, not 1f1c91397c.


  parent reply	other threads:[~2026-09-29 19:50 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-17  9:29 [PATCH 00/10] Limit usage of internal API in tests David Marchand
2026-07-17  9:29 ` [PATCH 01/10] bbdev: fix stats aggregation from queues David Marchand
2026-07-17 17:22   ` Chautru, Nicolas
2026-07-18  7:47     ` David Marchand
2026-07-17  9:29 ` [PATCH 02/10] bbdev: add per-queue statistics API David Marchand
2026-07-17  9:29 ` [PATCH 03/10] hash: fix GFNI stubs export David Marchand
2026-07-17  9:38   ` Bruce Richardson
2026-07-17  9:55   ` Konstantin Ananyev
2026-07-17  9:29 ` [PATCH 04/10] test: uninline helper for forking David Marchand
2026-07-17  9:39   ` Bruce Richardson
2026-07-17  9:30 ` [PATCH 05/10] test/bonding: get MAC address with public API David Marchand
2026-07-17  9:30 ` [PATCH 06/10] test/devargs: check driver presence " David Marchand
2026-07-17  9:55   ` Bruce Richardson
2026-07-17 10:08     ` David Marchand
2026-07-17 10:14       ` Bruce Richardson
2026-07-17 12:34         ` David Marchand
2026-07-17 12:56           ` Thomas Monjalon
2026-07-17  9:30 ` [PATCH 07/10] test/vdev: find device " David Marchand
2026-07-17  9:30 ` [PATCH 08/10] test: limit internal API usage David Marchand
2026-07-17  9:30 ` [PATCH 09/10] ci: make ABI reference generation faster David Marchand
2026-07-17  9:30 ` [PATCH 10/10] ci: run reference unit tests against ABI David Marchand
2026-07-17  9:35 ` [PATCH 00/10] Limit usage of internal API in tests David Marchand
2026-07-23 12:55 ` [PATCH v2 0/9] " David Marchand
2026-07-23 12:55   ` [PATCH v2 1/9] bbdev: add per-queue statistics API David Marchand
2026-09-18 15:29     ` Bruce Richardson
2026-07-23 12:55   ` [PATCH v2 2/9] hash: fix GFNI stubs export David Marchand
2026-07-23 12:55   ` [PATCH v2 3/9] test: uninline helper for forking David Marchand
2026-07-23 12:55   ` [PATCH v2 4/9] test/bonding: get MAC address with public API David Marchand
2026-07-23 13:30     ` Bruce Richardson
2026-07-23 12:55   ` [PATCH v2 5/9] test/devargs: rely on net null driver David Marchand
2026-07-23 13:31     ` Bruce Richardson
2026-07-23 12:55   ` [PATCH v2 6/9] test/vdev: find device with public API David Marchand
2026-09-18 15:30     ` Bruce Richardson
2026-07-23 12:55   ` [PATCH v2 7/9] test: limit internal API usage David Marchand
2026-07-23 13:33     ` Bruce Richardson
2026-07-23 15:04     ` Morten Brørup
2026-07-23 12:55   ` [PATCH v2 8/9] ci: make ABI reference generation faster David Marchand
2026-09-18 15:32     ` Bruce Richardson
2026-07-23 12:55   ` [PATCH v2 9/9] ci: run reference binaries against current ABI David Marchand
2026-09-18 15:41     ` Bruce Richardson
2026-09-21 11:47 ` [PATCH v3 0/9] Limit usage of internal API in tests David Marchand
2026-09-21 11:47   ` [PATCH v3 1/9] bbdev: add per-queue statistics API David Marchand
2026-09-21 11:47   ` [PATCH v3 2/9] hash: fix GFNI stubs export David Marchand
2026-09-21 11:47   ` [PATCH v3 3/9] test: uninline helper for forking David Marchand
2026-09-21 11:47   ` [PATCH v3 4/9] test/bonding: get MAC address with public API David Marchand
2026-09-21 11:47   ` [PATCH v3 5/9] test/devargs: rely on net null driver David Marchand
2026-09-21 16:38     ` Stephen Hemminger
2026-09-21 11:47   ` [PATCH v3 6/9] test/vdev: find device with public API David Marchand
2026-09-21 16:39     ` Stephen Hemminger
2026-09-21 11:47   ` [PATCH v3 7/9] test: limit internal API usage David Marchand
2026-09-21 11:47   ` [PATCH v3 8/9] ci: make ABI reference generation faster David Marchand
2026-09-21 11:47   ` [PATCH v3 9/9] ci: run reference binaries against current ABI David Marchand
2026-09-21 16:39     ` Stephen Hemminger
2026-09-22  7:38       ` David Marchand
2026-09-22  9:40 ` [PATCH v4 0/9] Limit usage of internal API in tests David Marchand
2026-09-22  9:40   ` [PATCH v4 1/9] bbdev: add per-queue statistics API David Marchand
2026-09-22  9:40   ` [PATCH v4 2/9] hash: fix GFNI stubs export David Marchand
2026-09-22  9:40   ` [PATCH v4 3/9] test: uninline helper for forking David Marchand
2026-09-22  9:40   ` [PATCH v4 4/9] test/bonding: get MAC address with public API David Marchand
2026-09-22  9:40   ` [PATCH v4 5/9] test/devargs: rely on net null driver David Marchand
2026-09-22  9:40   ` [PATCH v4 6/9] test/vdev: find device with public API David Marchand
2026-09-22  9:40   ` [PATCH v4 7/9] test: limit internal API usage David Marchand
2026-09-22  9:40   ` [PATCH v4 8/9] ci: make ABI reference generation faster David Marchand
2026-09-22  9:40   ` [PATCH v4 9/9] ci: run reference binaries against current ABI David Marchand
2026-09-22 10:03     ` Bruce Richardson
2026-09-29 13:38   ` [PATCH v4 0/9] Limit usage of internal API in tests David Marchand
2026-09-29 19:50   ` Stephen Hemminger [this message]
2026-09-30  7:32     ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260929125000.4800e8a2@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.