* RE: [PATCH v2] doc, eal, devtools: discourage new __rte_always_inline
From: Konstantin Ananyev @ 2026-06-02 7:07 UTC (permalink / raw)
To: Stephen Hemminger, dev@dpdk.org; +Cc: Thomas Monjalon
In-Reply-To: <20260601172104.311909-1-stephen@networkplumber.org>
> Modern compilers at -O2 make good inlining decisions for small
> static inline functions; forced inlining via __rte_always_inline
> should be reserved for cases where it is required for correctness
> or for documented measured performance reasons.
>
> Document the policy in the coding style guide and add a
> checkpatches.sh entry that flags when new uses of the attribute
> are introduced. Checkpatches is not an absolute blocker to
> acceptance, only an indicator that more review is needed.
>
> Add additional comments about use of __rte_always_inline,
> __rte_noinline, __rte_hot, and __rte_cold to the rte_common.h
> to aid developers.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> devtools/checkpatches.sh | 8 +++++
> doc/guides/contributing/coding_style.rst | 26 ++++++++++++++++-
> lib/eal/include/rte_common.h | 37 ++++++++++++++++++++++--
> 3 files changed, 68 insertions(+), 3 deletions(-)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index f5dd77443f..2a3d364178 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -137,6 +137,14 @@ check_forbidden_additions() { # <patch>
> -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> "$1" || res=1
>
> + # forbid new use of __rte_always_inline
> + awk -v FOLDERS="lib drivers app examples" \
> + -v EXPRESSIONS='\\<__rte_always_inline\\>' \
> + -v RET_ON_FAIL=1 \
> + -v MESSAGE='Adding __rte_always_inline; prefer plain inline' \
> + -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> + "$1" || res=1
> +
> # refrain from using compiler __rte_atomic_thread_fence()
> # It should be avoided on x86 for SMP case.
> awk -v FOLDERS="lib drivers app examples" \
> diff --git a/doc/guides/contributing/coding_style.rst
> b/doc/guides/contributing/coding_style.rst
> index 243a3c2959..8f9e77d1c1 100644
> --- a/doc/guides/contributing/coding_style.rst
> +++ b/doc/guides/contributing/coding_style.rst
> @@ -747,11 +747,35 @@ Static Variables and Functions
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> * All functions and variables that are local to a file must be declared as ``static``
> because it can often help the compiler to do some optimizations (such as, inlining
> the code).
> -* Functions that should be inlined should to be declared as ``static inline`` and
> can be defined in a .c or a .h file.
> +* Functions that should be inlined should be declared as ``static inline`` and can
> be defined in a .c or a .h file.
>
> .. note::
> Static functions defined in a header file must be declared as ``static
> inline`` in order to prevent compiler warnings about the function being unused.
>
> +Use of ``__rte_always_inline``
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The ``__rte_always_inline`` attribute forces the compiler to inline a function
> regardless of its size or call-graph heuristics.
> +Prefer plain ``inline`` (or no annotation at all for static functions) and let the
> compiler decide.
> +Modern compilers at ``-O2`` make good inlining decisions for small ``static
> inline`` functions in headers,
> +and forced inlining can hurt performance by inflating function bodies, increasing
> register pressure, and overriding profile-guided optimization.
> +
> +``__rte_always_inline`` should only be used when one of the following applies:
> +
> +* The function contains ``__rte_constant()`` checks that gate a constant-folded
> fast path,
> + and the optimization is lost if the function is not inlined into the caller.
> + Examples include byte-order helpers and length-dispatched copy/compare
> routines.
> +
> +* The function wraps inline assembly or a compiler intrinsic whose correctness
> depends on being inlined into the caller's register context (for example, intrinsics
> requiring a compile-time constant argument).
> +
> +* Measurement on a representative workload shows that the annotation is
> required to retain performance, and the reason is documented in the commit
> message that introduces it.
> +
> +Each use must be justified at the point it is introduced. Adding
> ``__rte_always_inline`` because nearby code uses it is not a justification;
> +if the constant or intrinsic that requires inlining is several call levels up the call
> chain,
> +restructure the code rather than annotating the entire chain.
> +
> +The complementary attribute ``__rte_noinline`` is useful for explicitly marking
> cold paths (error handling, initialization, slow-path fallbacks) where outlining the
> function can reduce instruction-cache pressure on the hot path.
> +
> Const Attribute
> ~~~~~~~~~~~~~~~
>
> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> index 71415346cf..e358be7fcf 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -482,7 +482,22 @@ static void __attribute__((destructor(RTE_PRIO(prio)),
> used)) func(void)
> #endif
>
> /**
> - * Force a function to be inlined
> + * Force a function to be inlined, regardless of the compiler's size and
> + * call-graph heuristics.
> + *
> + * Prefer plain @c inline (or no annotation on a static function) and let the
> compiler decide.
> + * Modern compilers at -O2 inline small static functions well,
> + * and forcing it can hurt by inflating call sites, raising register pressure,
> + * and overriding profile-guided optimization.
> + *
> + * Reserve this attribute for cases where inlining is required for
> + * correctness, or for a documented and measured performance reason, e.g.
> + * - a constant-folded fast path gated by @c __rte_constant() that is lost
> + * unless the function is inlined into the caller;
> + * - a wrapper around inline asm or an intrinsic that needs a
> + * compile-time-constant argument from the caller's context.
> + *
> + * See the DPDK coding style guide for the full policy.
> */
> #ifdef RTE_TOOLCHAIN_MSVC
> #define __rte_always_inline __forceinline
> @@ -491,7 +506,11 @@ static void __attribute__((destructor(RTE_PRIO(prio)),
> used)) func(void)
> #endif
>
> /**
> - * Force a function to be noinlined
> + * Force a function not to be inlined.
> + *
> + * Useful for explicitly outlining cold paths such as error handling,
> + * initialization, slow-path fallbacks, so they do not bloat the hot path
> + * or add to its instruction-cache footprint.
> */
> #ifdef RTE_TOOLCHAIN_MSVC
> #define __rte_noinline __declspec(noinline)
> @@ -501,6 +520,12 @@ static void __attribute__((destructor(RTE_PRIO(prio)),
> used)) func(void)
>
> /**
> * Hint function in the hot path
> + *
> + * The compiler may optimize the function more aggressively, treat calls
> + * to it as likely for branch prediction, and group it with other hot
> + * functions to improve instruction-cache locality. This affects code
> + * placement and prediction, not inlining; combine with an inline
> + * annotation if both are wanted.
> */
> #ifdef RTE_TOOLCHAIN_MSVC
> #define __rte_hot
> @@ -510,6 +535,14 @@ static void __attribute__((destructor(RTE_PRIO(prio)),
> used)) func(void)
>
> /**
> * Hint function in the cold path
> + *
> + * The compiler optimizes the function for size rather than speed,
> + * marks branches that reach it as unlikely, and may move it to a separate
> + * section to keep it off the hot path and reduce instruction-cache
> + * pressure there.
> + *
> + * Suitable for error handling, logging, and setup/teardown code.
> + * Functions marked @c __rte_noreturn are already treated as cold.
> */
> #ifdef RTE_TOOLCHAIN_MSVC
> #define __rte_cold
> --
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> 2.53.0
^ permalink raw reply
* RE: [PATCH v2] app/test-pmd: add generic PROG action parser support
From: Konstantin Ananyev @ 2026-06-02 7:35 UTC (permalink / raw)
To: Ajmera, Megha, Stephen Hemminger
Cc: Richardson, Bruce, Dumitrescu, Cristian, Shetty, Praveen,
Singh, Aman Deep, dev@dpdk.org
In-Reply-To: <MW4PR11MB589273F3A331E94D0876900D97152@MW4PR11MB5892.namprd11.prod.outlook.com>
> > For my own curiosity: how user can define his own PROG action?
> > Is it supposed to be programmed and uplodaded to the NIC by some external
> tool
> > (P4 compiler)?
> > Or does it refer to the set of some predefined functions that given firmware
> > supports?
> > Or ... ?
> > Sorry for probably naive questions, but I found is nearly zero information inside
> > DPDK docs about how PROG action supposed to work.
>
> PROG in rte_flow is currently a vendor-defined action interface (name +
> arguments), not a generic DPDK programming model by itself.
>
> So in practice:
>
> 1. DPDK/testpmd does not define or upload user code to NIC on its own.
> 2. testpmd just passes the prog name/argument/payload to the PMD via
> rte_flow.
> 3. What that name/arguments means is NIC/firmware specific.
> 4. Depending on device support, it may map to predefined firmware-exposed
> functions
Ok, so it is predefined by the FW.
Then shouldn't CPFL PG contain a list of supported PROGs and their arguments?
Again, some tests/examples for it with testpmd/DTS.
Or that's all will be the next step (patch-series)?
> 5. If the PMD/firmware does not recognize it, flow validate/create will fail.
^ permalink raw reply
* RE: [PATCH] ring: avoid extra store at move head
From: Konstantin Ananyev @ 2026-06-02 9:02 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev@dpdk.org
In-Reply-To: <20260601152313.1946d438@phoenix.local>
> > C11 __rte_ring_headtail_move_head_mt() uses output
> > parameter: 'uint32_t *old_head' directly within CAS operation.
> > In x86_64 that cause gcc to generate extra instructions to
> > store return value of CAS (eax) within 'old_head' memory location,
> > even when CAS was not successful and another attempt should be
> > performed. In some cases, even extra branch can be observed.
> > To be more specific the code like that is generated:
> > // start of 'do { } while();' loop
> > .L2
> > ...
> > lock cmpxchgl %r8d, (%rdi)
> > jne .L17 //
> > .L1: // <---- successful completion of CAS, finish
> > movl %edx, %eax
> > ret
> > .L17: // <---- unsuccessful completion of CAS, repeat
> > movl %eax, (%r9)
> > jmp .L2
> >
> > In constrast, x86 specific version that uses
> > __sync_bool_compare_and_swap() doesn't exibit such problem,
> > as __sync_bool_compare_and_swap() doesn't update the 'old_head'
> > with new value, and we have to re-read it explicitly on each iteration.
> >
> > Overcome that problem by using local variable 'head' inside the loop,
> > and updaing '*old_head' value only at exit.
> > With such change gcc manages to avoid extra store(/branch).
> >
> > Depends-on: series-38225 ("deprecate rte_atomicNN family")
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> > ---
>
> I used the standard ring perf tests and ran 10 times via:
> ! /bin/bash
> if [ -z "$1" ]; then
> echo "Usage $0 version"
> exit 1
> fi
>
> VERSION=$1
> for i in $(seq 1 10); do
> sudo DPDK_TEST=ring_perf_autotest \
> ./build/app/dpdk-test -l 2-5 -n 4 --no-pci --file-prefix=run$i \
> > ~/DPDK/ring_perf_results/${VERSION}_run${i}.log 2>&1
> echo "${VERSION} run $i done"
> done
>
>
> Then had Claude compare results:
>
> Key metric (two physical cores legacy MP/MC bulk n=128):
> main: 5.380 cycles/elem
> sync-bool: 5.377 cycles/elem (-0.07%)
> avoid-store: 5.892 cycles/elem (+9.52%) ← regresses
>
>
> Looking at the dissassembly of ring_enqueue_bulk:
>
> The inner loop of main and sync-bool versions is:
> mov 0x80(%rdi),%r11d ; load d->head via displacement
> mov 0x104(%rdi),%ebx ; load s->tail
> add %ecx,%ebx
> sub %r11d,%ebx
> cmp %ebx,%r12d
> jae [exit]
> lea (%r8,%r11,1),%r13d ; new_head = old_head + n
> mov %r11d,%eax ; expected → eax
> lock cmpxchg %r13d,0x80(%rdi) ; ← displacement addressing
> jne [retry] ; ← direct jne, eax preserved
>
> Using atomic_compare_exchange and your patch:
> mov 0x38(%rdi),%r10d
> mov 0x80(%rdi),%eax ; load d->head directly into %eax
> lea 0x80(%rdi),%rcx ; ← MATERIALIZE &d->head into %rcx
> lea -0x1(%r8),%r12d
> mov 0x104(%rdi),%r11d
> add %r10d,%r11d
> sub %eax,%r11d
> cmp %r11d,%r12d
> jae [exit]
> lea (%r8,%rax,1),%r13d ; new_head
> lock cmpxchg %r13d,(%rcx) ; ← INDIRECT addressing via %rcx
> mov %eax,%ebx ; ← EXTRA: save post-CAS %eax to %ebx
> jne [retry]
>
> Bottom line: good idea but still fighting with Gcc optimizer here.
Thanks for trying.
On my box (AMD EPYC 9534) with same test, there is no much difference between all of them:
use-sync-bool: 2.2273
use-c11-current-version: 2.2422
use-c11-patched: 2.2431
Anyway, -10% on some boxes - that's probably good enough reason to keep specific version
for __rte_ring_headtail_move_head_mt().
My ask would be to have some special macro for it, so users can enable/disable it via
'meson setup' at will.
Konstantin
^ permalink raw reply
* RE: [PATCH] app/crypto-perf: support ML DSA
From: Akhil Goyal @ 2026-06-02 9:02 UTC (permalink / raw)
To: Pratik Senapati, dev@dpdk.org
Cc: Anoob Joseph, Gowrishankar Muthukrishnan, kai.ji@intel.com
In-Reply-To: <20260528075853.2206573-2-psenapati@marvell.com>
Please send this patch as a series with ML KEM support so that CI can run successfully.
^ permalink raw reply
* RE: [PATCH] crypto/openssl: fix use-after-free bug and cleanup
From: Akhil Goyal @ 2026-06-02 9:03 UTC (permalink / raw)
To: Pratik Senapati, dev@dpdk.org
Cc: Anoob Joseph, Gowrishankar Muthukrishnan, kai.ji@intel.com,
stable@dpdk.org
In-Reply-To: <20260528075822.2206494-1-psenapati@marvell.com>
> -----Original Message-----
> From: Pratik Senapati <psenapati@marvell.com>
> Sent: Thursday, May 28, 2026 1:28 PM
> To: dev@dpdk.org
> Cc: Akhil Goyal <gakhil@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>; kai.ji@intel.com;
> stable@dpdk.org
> Subject: [PATCH] crypto/openssl: fix use-after-free bug and cleanup
>
> params is freed before it is used by
> EVP_PKEY_decapsulate_init() causing a
> use-after-free issue. Pass NULL to
> EVP_PKEY_decapsulate_init() instead of params
> to avoid it.
>
> Add resource cleanup for all error paths in the ML-KEM
> decapsulate handler and consolidate cleanup into
> two goto labels err_pkey and err_decap.
>
> Fixes: 5f761d7b60 ("crypto/openssl: support ML-KEM and ML-DSA")
> Cc: stable@dpdk.org
> Signed-off-by: Pratik Senapati <psenapati@marvell.com>
> ---
Fix compilation.
^ permalink raw reply
* [RFC PATCH 0/3] remove unneeded build options
From: Bruce Richardson @ 2026-06-02 9:08 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
Having unnecessary build options just makes things confusing for
the user, and expands the set of potential builds. This set
makes 3 build options no-ops, and marks them as deprecated for
future removal.
Bruce Richardson (3):
build: deprecate kernel dir option
build: deprecate standalone tests option
build: deprecate HPET build option
app/meson.build | 6 +-----
app/test/test.c | 7 -------
app/test/test_timer.c | 5 +++++
config/meson.build | 2 +-
doc/guides/linux_gsg/enable_func.rst | 2 --
doc/guides/rel_notes/deprecation.rst | 7 +++++++
lib/eal/linux/meson.build | 5 +----
meson_options.txt | 6 +++---
8 files changed, 18 insertions(+), 22 deletions(-)
--
2.53.0
^ permalink raw reply
* [RFC PATCH 1/3] build: deprecate kernel dir option
From: Bruce Richardson @ 2026-06-02 9:08 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
In-Reply-To: <20260602090847.905721-1-bruce.richardson@intel.com>
The kernel_dir option does nothing, so it can be deprecated and removed
in future.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
doc/guides/rel_notes/deprecation.rst | 4 ++++
meson_options.txt | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 17f90a6352..ed1dda6008 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -17,6 +17,10 @@ Other API and ABI deprecation notices are to be posted below.
Deprecation Notices
-------------------
+* build: The following meson build options are deprecated and will be removed in a future release:
+
+ - ``kernel_dir``: option unused as Linux kernel drivers are in a separate ``dpdk-kmods`` repository.
+
* kvargs: The function ``rte_kvargs_process`` will get a new parameter
for returning key match count. It will ease handling of no-match case.
diff --git a/meson_options.txt b/meson_options.txt
index e28d24054c..7bd5ebc084 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -33,7 +33,7 @@ option('ibverbs_link', type: 'combo', choices : ['static', 'shared', 'dlopen'],
option('include_subdir_arch', type: 'string', value: '', description:
'subdirectory where to install arch-dependent headers')
option('kernel_dir', type: 'string', value: '', description:
- 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir or $kernel_dir/build. Modules will be installed in /lib/modules.')
+ '[Deprecated] Value unused. Previously, path to the kernel for building kernel modules.')
option('machine', type: 'string', value: 'auto', description:
'Alias of cpu_instruction_set.')
option('max_ethports', type: 'integer', value: 32, description:
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 2/3] build: deprecate standalone tests option
From: Bruce Richardson @ 2026-06-02 9:08 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
In-Reply-To: <20260602090847.905721-1-bruce.richardson@intel.com>
The unit tests can be disabled in DPDK in a couple of ways. Firstly,
there is the original "tests" boolean option - when set to false, the
tests are skipped as part of the build. However, the newer
enable_apps/disable_apps options also can be used to disable the tests
[you can disable using the "enable" option by omitting "tests" from the
list to enable].
This duplication of functionality is unnecessary, so let's remove the
standalone tests option.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
app/meson.build | 6 +-----
doc/guides/rel_notes/deprecation.rst | 1 +
meson_options.txt | 2 +-
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/app/meson.build b/app/meson.build
index 1798db3ae4..52f704ec94 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -17,6 +17,7 @@ apps = [
'graph',
'pdump',
'proc-info',
+ 'test',
'test-acl',
'test-bbdev',
'test-cmdline',
@@ -35,11 +36,6 @@ apps = [
'test-security-perf',
]
-if get_option('tests')
-# build the auto test app if enabled.
- apps += 'test'
-endif
-
default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
default_ldflags = []
if get_option('default_library') == 'static' and not is_windows
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index ed1dda6008..15459851d0 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -20,6 +20,7 @@ Deprecation Notices
* build: The following meson build options are deprecated and will be removed in a future release:
- ``kernel_dir``: option unused as Linux kernel drivers are in a separate ``dpdk-kmods`` repository.
+ - ``tests``: disabling tests can be achieved by using the ``enable_apps`` / ``disable_apps`` options instead.
* kvargs: The function ``rte_kvargs_process`` will get a new parameter
for returning key match count. It will ease handling of no-match case.
diff --git a/meson_options.txt b/meson_options.txt
index 7bd5ebc084..323a3901e1 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -55,6 +55,6 @@ option('enable_stdatomic', type: 'boolean', value: false, description:
option('enable_trace_fp', type: 'boolean', value: false, description:
'enable fast path trace points.')
option('tests', type: 'boolean', value: true, description:
- 'build unit tests')
+ '[Deprecated] Value unused. Add "tests" to disable_apps setting to disable the unit tests')
option('use_hpet', type: 'boolean', value: false, description:
'use HPET timer in EAL')
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 3/3] build: deprecate HPET build option
From: Bruce Richardson @ 2026-06-02 9:08 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
In-Reply-To: <20260602090847.905721-1-bruce.richardson@intel.com>
We can enable the building of the HPET code by default on Linux, since
the timers are not used - or even initialized - by default. Instead an
app needs to explicitly call rte_eal_hpet_init() to use the HPET timer
APIs. Therefore, let's simplify the user experience by deprecating the
option "use_hpet" and make it a no-op.
To avoid issue with the dpdk-test binary which was trying to initialize
the hpet on startup, move the hpet init call to the timer autotest - the
only place where it was used.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
app/test/test.c | 7 -------
app/test/test_timer.c | 5 +++++
config/meson.build | 2 +-
doc/guides/linux_gsg/enable_func.rst | 2 --
doc/guides/rel_notes/deprecation.rst | 2 ++
lib/eal/linux/meson.build | 5 +----
meson_options.txt | 2 +-
7 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/app/test/test.c b/app/test/test.c
index 58ef52f312..99d6e90f8b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -180,13 +180,6 @@ main(int argc, char **argv)
goto out;
}
-#ifdef RTE_LIBEAL_USE_HPET
- if (rte_eal_hpet_init(1) < 0)
-#endif
- RTE_LOG(INFO, APP,
- "HPET is not enabled, using TSC as default timer\n");
-
-
char *dpdk_test = getenv("DPDK_TEST");
if (dpdk_test && strlen(dpdk_test) > 0)
diff --git a/app/test/test_timer.c b/app/test/test_timer.c
index c936843ddc..dcb26c4395 100644
--- a/app/test/test_timer.c
+++ b/app/test/test_timer.c
@@ -501,6 +501,11 @@ static int
timer_sanity_check(void)
{
#ifdef RTE_LIBEAL_USE_HPET
+ if (rte_eal_hpet_init(1) < 0) {
+ printf("HPET is not enabled, using TSC as default timer\n");
+ return 0;
+ }
+
if (eal_timer_source != EAL_TIMER_HPET) {
printf("Not using HPET, can't sanity check timer sources\n");
return 0;
diff --git a/config/meson.build b/config/meson.build
index 9ba7b9a338..6012a3c243 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -381,7 +381,7 @@ endforeach
# set other values pulled from the build options
dpdk_conf.set('RTE_MAX_ETHPORTS', get_option('max_ethports'))
-dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))
+dpdk_conf.set('RTE_LIBEAL_USE_HPET', is_linux)
dpdk_conf.set('RTE_ENABLE_STDATOMIC', get_option('enable_stdatomic'))
dpdk_conf.set('RTE_ENABLE_TRACE_FP', get_option('enable_trace_fp'))
dpdk_conf.set('RTE_PKTMBUF_HEADROOM', get_option('pkt_mbuf_headroom'))
diff --git a/doc/guides/linux_gsg/enable_func.rst b/doc/guides/linux_gsg/enable_func.rst
index 4e1a939f35..1d5b030172 100644
--- a/doc/guides/linux_gsg/enable_func.rst
+++ b/doc/guides/linux_gsg/enable_func.rst
@@ -176,8 +176,6 @@ To enable HPET support in DPDK:
Note that this may involve doing a kernel rebuild,
as many common linux distributions do *not* have this setting
enabled by default in their kernel builds.
-#. Enable DPDK support for HPET by using the build-time meson option ``use_hpet``,
- for example, ``meson configure -Duse_hpet=true``
For an application to use the ``rte_get_hpet_cycles()`` and ``rte_get_hpet_hz()`` API calls,
and optionally to make the HPET the default time source for the rte_timer library,
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 15459851d0..42ad118c92 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -21,6 +21,8 @@ Deprecation Notices
- ``kernel_dir``: option unused as Linux kernel drivers are in a separate ``dpdk-kmods`` repository.
- ``tests``: disabling tests can be achieved by using the ``enable_apps`` / ``disable_apps`` options instead.
+ - ``use_hpet``: option unused as HPET is only supported on Linux
+ and is runtime-enabled using ``rte_eal_hpet_init()`` API.
* kvargs: The function ``rte_kvargs_process`` will get a new parameter
for returning key match count. It will ease handling of no-match case.
diff --git a/lib/eal/linux/meson.build b/lib/eal/linux/meson.build
index 29ba313218..8b0519c9a1 100644
--- a/lib/eal/linux/meson.build
+++ b/lib/eal/linux/meson.build
@@ -15,14 +15,11 @@ sources += files(
'eal_memory.c',
'eal_thread.c',
'eal_timer.c',
+ 'eal_timer_hpet.c',
'eal_vfio.c',
'eal_vfio_mp_sync.c',
)
-if dpdk_conf.get('RTE_LIBEAL_USE_HPET')
- sources += files('eal_timer_hpet.c')
-endif
-
deps += ['kvargs', 'telemetry']
if has_libnuma
dpdk_conf.set10('RTE_EAL_NUMA_AWARE_HUGEPAGES', true)
diff --git a/meson_options.txt b/meson_options.txt
index 323a3901e1..242de6681f 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -57,4 +57,4 @@ option('enable_trace_fp', type: 'boolean', value: false, description:
option('tests', type: 'boolean', value: true, description:
'[Deprecated] Value unused. Add "tests" to disable_apps setting to disable the unit tests')
option('use_hpet', type: 'boolean', value: false, description:
- 'use HPET timer in EAL')
+ '[Deprecated] Value unused. HPET can be runtime-enabled on Linux using rte_eal_hpet_init().')
--
2.53.0
^ permalink raw reply related
* RE: [PATCH] ring: avoid extra store at move head
From: Morten Brørup @ 2026-06-02 9:21 UTC (permalink / raw)
To: Konstantin Ananyev, Stephen Hemminger; +Cc: dev
In-Reply-To: <580828b0214b4e1fb22e2f6d4079eadf@huawei.com>
> > Then had Claude compare results:
> >
> > Key metric (two physical cores legacy MP/MC bulk n=128):
> > main: 5.380 cycles/elem
> > sync-bool: 5.377 cycles/elem (-0.07%)
> > avoid-store: 5.892 cycles/elem (+9.52%) ← regresses
> >
> >
> > Looking at the dissassembly of ring_enqueue_bulk:
> >
> > The inner loop of main and sync-bool versions is:
> > mov 0x80(%rdi),%r11d ; load d->head via displacement
> > mov 0x104(%rdi),%ebx ; load s->tail
> > add %ecx,%ebx
> > sub %r11d,%ebx
> > cmp %ebx,%r12d
> > jae [exit]
> > lea (%r8,%r11,1),%r13d ; new_head = old_head + n
> > mov %r11d,%eax ; expected → eax
> > lock cmpxchg %r13d,0x80(%rdi) ; ← displacement addressing
> > jne [retry] ; ← direct jne, eax preserved
> >
> > Using atomic_compare_exchange and your patch:
> > mov 0x38(%rdi),%r10d
> > mov 0x80(%rdi),%eax ; load d->head directly into %eax
> > lea 0x80(%rdi),%rcx ; ← MATERIALIZE &d->head into
> %rcx
> > lea -0x1(%r8),%r12d
> > mov 0x104(%rdi),%r11d
> > add %r10d,%r11d
> > sub %eax,%r11d
> > cmp %r11d,%r12d
> > jae [exit]
> > lea (%r8,%rax,1),%r13d ; new_head
> > lock cmpxchg %r13d,(%rcx) ; ← INDIRECT addressing via %rcx
> > mov %eax,%ebx ; ← EXTRA: save post-CAS %eax to
> %ebx
> > jne [retry]
> >
> > Bottom line: good idea but still fighting with Gcc optimizer here.
>
> Thanks for trying.
> On my box (AMD EPYC 9534) with same test, there is no much difference
> between all of them:
> use-sync-bool: 2.2273
> use-c11-current-version: 2.2422
> use-c11-patched: 2.2431
> Anyway, -10% on some boxes - that's probably good enough reason to keep
> specific version
> for __rte_ring_headtail_move_head_mt().
> My ask would be to have some special macro for it, so users can
> enable/disable it via 'meson setup' at will.
This seems very exotic as a meson command line option.
Either put it in rte_config.h, or make it CPU specific.
^ permalink raw reply
* RE: [RFC PATCH 1/3] build: deprecate kernel dir option
From: Morten Brørup @ 2026-06-02 9:24 UTC (permalink / raw)
To: Bruce Richardson, dev
In-Reply-To: <20260602090847.905721-2-bruce.richardson@intel.com>
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Tuesday, 2 June 2026 11.09
>
> The kernel_dir option does nothing, so it can be deprecated and removed
> in future.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
Good cleanup.
Acked-by: Morten Brørup <mb@smartsharesystems.com>
^ permalink raw reply
* RE: [RFC PATCH 2/3] build: deprecate standalone tests option
From: Morten Brørup @ 2026-06-02 9:26 UTC (permalink / raw)
To: Bruce Richardson, dev
In-Reply-To: <20260602090847.905721-3-bruce.richardson@intel.com>
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Tuesday, 2 June 2026 11.09
>
> The unit tests can be disabled in DPDK in a couple of ways. Firstly,
> there is the original "tests" boolean option - when set to false, the
> tests are skipped as part of the build. However, the newer
> enable_apps/disable_apps options also can be used to disable the tests
> [you can disable using the "enable" option by omitting "tests" from the
> list to enable].
>
> This duplication of functionality is unnecessary, so let's remove the
> standalone tests option.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
Good cleanup.
Acked-by: Morten Brørup <mb@smartsharesystems.com>
^ permalink raw reply
* Re: [PATCH] gpu/metax: add new driver for Metax GPU
From: Thomas Monjalon @ 2026-06-02 10:01 UTC (permalink / raw)
To: 许玲燕; +Cc: dev, eagostini
In-Reply-To: <6c18e957-c1c2-4b43-9b2c-b36424c1e9af.lingyan.xu@metax-tech.com>
Hello,
01/06/2026 07:47, 许玲燕:
> I am writing to propose a new driver for the Metax GPU,
How do you access the GPU?
Are you using a specific library or kernel module?
> which I believe will significantly enhance our support
> and performance for this hardware.
> The patch attached includes the initial implementation of the driver,
> with key features such as:
>
> * Basic initialization and configuration
> * Memory management and allocation
> * Core functionality for rendering and compute tasks
I am familiar with connecting compute tasks of a GPU
with DPDK networking, but I'm surprised by the rendering functionality.
Do you mean graphical rendering of data coming from the network?
> Please review the code and let me know if you have any feedback or suggestions.
> I am more than happy to make any necessary adjustments and improvements.
Thank you for working on this.
I recommend following this guide to introduce a new driver:
https://doc.dpdk.org/guides/contributing/new_driver.html
^ permalink raw reply
* RE: [RFC PATCH 3/3] build: deprecate HPET build option
From: Morten Brørup @ 2026-06-02 10:47 UTC (permalink / raw)
To: Bruce Richardson, dev
In-Reply-To: <20260602090847.905721-4-bruce.richardson@intel.com>
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Tuesday, 2 June 2026 11.09
>
> We can enable the building of the HPET code by default on Linux, since
> the timers are not used - or even initialized - by default. Instead an
> app needs to explicitly call rte_eal_hpet_init() to use the HPET timer
> APIs. Therefore, let's simplify the user experience by deprecating the
> option "use_hpet" and make it a no-op.
>
> To avoid issue with the dpdk-test binary which was trying to initialize
> the hpet on startup, move the hpet init call to the timer autotest -
> the
> only place where it was used.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
Careful!
I think this patch has unintended side effects:
On Linux, it unconditionally enables HPET (and sets RTE_LIBEAL_USE_HPET), which was previously disabled by default.
So, if some Linux applications use #ifdef RTE_LIBEAL_USE_HPET, they will now tell DPDK to use that timer instead of the TSC.
We can fix the apps/examples in the DPDK repo, but it will potentially change behavior of DPDK user's applications.
I'm not opposed to unconditionally enabling HPET ability in DPDK itself on Linux.
But I'm worried about side effects of unconditionally enabling #ifdef RTE_LIBEAL_USE_HPET in Linux user applications.
^ permalink raw reply
* Re: [RFC PATCH 3/3] build: deprecate HPET build option
From: David Marchand @ 2026-06-02 11:39 UTC (permalink / raw)
To: Morten Brørup, Bruce Richardson; +Cc: dev
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35F658D8@smartserver.smartshare.dk>
On Tue, 2 Jun 2026 at 12:47, Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Tuesday, 2 June 2026 11.09
> >
> > We can enable the building of the HPET code by default on Linux, since
> > the timers are not used - or even initialized - by default. Instead an
> > app needs to explicitly call rte_eal_hpet_init() to use the HPET timer
> > APIs. Therefore, let's simplify the user experience by deprecating the
> > option "use_hpet" and make it a no-op.
> >
> > To avoid issue with the dpdk-test binary which was trying to initialize
> > the hpet on startup, move the hpet init call to the timer autotest -
> > the
> > only place where it was used.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
>
> Careful!
> I think this patch has unintended side effects:
>
> On Linux, it unconditionally enables HPET (and sets RTE_LIBEAL_USE_HPET), which was previously disabled by default.
>
> So, if some Linux applications use #ifdef RTE_LIBEAL_USE_HPET, they will now tell DPDK to use that timer instead of the TSC.
> We can fix the apps/examples in the DPDK repo, but it will potentially change behavior of DPDK user's applications.
>
> I'm not opposed to unconditionally enabling HPET ability in DPDK itself on Linux.
> But I'm worried about side effects of unconditionally enabling #ifdef RTE_LIBEAL_USE_HPET in Linux user applications.
I don't see a functional impact.
There may be an impact on performance?
But users can switch to rte_get_tsc_cycles() to avoid the added branch.
On the other hand, did you consider dropping HPET altogether?
--
David Marchand
^ permalink raw reply
* RE: [PATCH 1/2] crypto/ipsec_mb: allow aesni_mb and aesni_gcm vdevs on Arm
From: Hemant Agrawal @ 2026-06-02 12:09 UTC (permalink / raw)
To: Wathsala Vithanage, Kai Ji, Pablo de Lara
Cc: dev@dpdk.org, nd@arm.com, Paul.Elliott@arm.com,
Dhruv.Tripathi@arm.com, Shebu.VargheseKuriakose@arm.com
In-Reply-To: <20260529205512.1985844-1-wathsala.vithanage@arm.com>
> -----Original Message-----
> From: Wathsala Vithanage <wathsala.vithanage@arm.com>
> Sent: 30 May 2026 02:25
> To: Kai Ji <kai.ji@intel.com>; Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Paul.Elliott@arm.com;
> Dhruv.Tripathi@arm.com; Shebu.VargheseKuriakose@arm.com; Wathsala
> Vithanage <wathsala.vithanage@arm.com>
> Subject: [PATCH 1/2] crypto/ipsec_mb: allow aesni_mb and aesni_gcm vdevs
> on Arm
>
> Extend Arm PMD gating in ipsec_mb_create() to permit
> IPSEC_MB_PMD_TYPE_AESNI_MB and IPSEC_MB_PMD_TYPE_AESNI_GCM in
> addition to existing SNOW3G and ZUC.
>
> This removes -ENOTSUP rejection for crypto_aesni_mb and crypto_aesni_gcm
> on Arm, enabling these vdevs to probe and run when backed by a compatible
> ipsec-mb library.
>
> Signed-off-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
> ---
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
^ permalink raw reply
* RE: [PATCH 2/2] doc: update Arm IPsec-MB references for cryptodev PMDs
From: Hemant Agrawal @ 2026-06-02 12:10 UTC (permalink / raw)
To: Wathsala Vithanage, Kai Ji, Pablo de Lara
Cc: dev@dpdk.org, nd@arm.com, paul.elliott@arm.com,
dhruv.tripathi@arm.com, shebu.vargheseKuriakose@arm.com
In-Reply-To: <20260529210404.1986896-1-wathsala.vithanage@arm.com>
> -----Original Message-----
> From: Wathsala Vithanage <wathsala.vithanage@arm.com>
> Sent: 30 May 2026 02:34
> To: Kai Ji <kai.ji@intel.com>; Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; paul.elliott@arm.com;
> dhruv.tripathi@arm.com; shebu.vargheseKuriakose@arm.com; Wathsala
> Vithanage <wathsala.vithanage@arm.com>
> Subject: [PATCH 2/2] doc: update Arm IPsec-MB references for cryptodev
> PMDs
>
> Document Arm/ARM64 IPsec-MB source and supported tag level for
> aesni_mb, aesni_gcm, snow3g, and zuc cryptodev guides.
>
> - Add Arm library reference to aesni_mb and aesni_gcm docs.
> - Update snow3g and zuc IPsec-MB library tag to SECLIB-IPSEC-2026.05.30.
>
> Signed-off-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
> ---
> doc/guides/cryptodevs/aesni_gcm.rst | 4 ++++
> doc/guides/cryptodevs/aesni_mb.rst | 4 ++++
> doc/guides/cryptodevs/snow3g.rst | 2 +-
> doc/guides/cryptodevs/zuc.rst | 2 +-
> 4 files changed, 10 insertions(+), 2 deletions(-)
>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
^ permalink raw reply
* Re: [RFC PATCH 3/3] build: deprecate HPET build option
From: Bruce Richardson @ 2026-06-02 12:27 UTC (permalink / raw)
To: Morten Brørup; +Cc: dev
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35F658D8@smartserver.smartshare.dk>
On Tue, Jun 02, 2026 at 12:47:00PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Tuesday, 2 June 2026 11.09
> >
> > We can enable the building of the HPET code by default on Linux, since
> > the timers are not used - or even initialized - by default. Instead an
> > app needs to explicitly call rte_eal_hpet_init() to use the HPET timer
> > APIs. Therefore, let's simplify the user experience by deprecating the
> > option "use_hpet" and make it a no-op.
> >
> > To avoid issue with the dpdk-test binary which was trying to initialize
> > the hpet on startup, move the hpet init call to the timer autotest -
> > the
> > only place where it was used.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
>
> Careful!
> I think this patch has unintended side effects:
>
> On Linux, it unconditionally enables HPET (and sets RTE_LIBEAL_USE_HPET), which was previously disabled by default.
>
> So, if some Linux applications use #ifdef RTE_LIBEAL_USE_HPET, they will now tell DPDK to use that timer instead of the TSC.
> We can fix the apps/examples in the DPDK repo, but it will potentially change behavior of DPDK user's applications.
>
> I'm not opposed to unconditionally enabling HPET ability in DPDK itself on Linux.
> But I'm worried about side effects of unconditionally enabling #ifdef RTE_LIBEAL_USE_HPET in Linux user applications.
>
Good point, I hadn't considered if end applications had code guarded by
USE_HPET.
However, even if HPET support is build-enabled, it still requires apps to
explicitly opt-in a) to use it and then b) to make it the default for timer
operations. That means that if apps have got the code present to use HPET
by default, then they must have a valid reason for doing so and have
explicitly opted in to doing so by adding the hpet API calls. It's not
possible for apps to "accidentally" start using HPET. [I suppose there may
be a risk from old/legacy code in apps, maybe where hpet was used in the
past and forgotten about, but otherwise I can't see there being an issue.]
More review and testing is welcome though, to ensure I'm not actually
missing something here...
/Bruce
^ permalink raw reply
* Re: [RFC PATCH 3/3] build: deprecate HPET build option
From: Bruce Richardson @ 2026-06-02 12:37 UTC (permalink / raw)
To: David Marchand; +Cc: Morten Brørup, dev
In-Reply-To: <CAJFAV8x5_JihAXe_bruriVu01b5wmF4fJ7vD42yFfNwpJQwDtQ@mail.gmail.com>
On Tue, Jun 02, 2026 at 01:39:42PM +0200, David Marchand wrote:
> On Tue, 2 Jun 2026 at 12:47, Morten Brørup <mb@smartsharesystems.com> wrote:
> >
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Tuesday, 2 June 2026 11.09
> > >
> > > We can enable the building of the HPET code by default on Linux, since
> > > the timers are not used - or even initialized - by default. Instead an
> > > app needs to explicitly call rte_eal_hpet_init() to use the HPET timer
> > > APIs. Therefore, let's simplify the user experience by deprecating the
> > > option "use_hpet" and make it a no-op.
> > >
> > > To avoid issue with the dpdk-test binary which was trying to initialize
> > > the hpet on startup, move the hpet init call to the timer autotest -
> > > the
> > > only place where it was used.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> >
> > Careful!
> > I think this patch has unintended side effects:
> >
> > On Linux, it unconditionally enables HPET (and sets RTE_LIBEAL_USE_HPET), which was previously disabled by default.
> >
> > So, if some Linux applications use #ifdef RTE_LIBEAL_USE_HPET, they will now tell DPDK to use that timer instead of the TSC.
> > We can fix the apps/examples in the DPDK repo, but it will potentially change behavior of DPDK user's applications.
> >
> > I'm not opposed to unconditionally enabling HPET ability in DPDK itself on Linux.
> > But I'm worried about side effects of unconditionally enabling #ifdef RTE_LIBEAL_USE_HPET in Linux user applications.
>
> I don't see a functional impact.
>
> There may be an impact on performance?
> But users can switch to rte_get_tsc_cycles() to avoid the added branch.
>
I'd be very surprised if there was an impact from that. Any timers we have
already take a measurable number of cycles, so the extra branch is going to
be unnoticable IMHO.
>
> On the other hand, did you consider dropping HPET altogether?
>
I did, but that I considered a bit riskier and harder to do, as we don't
know who might actually be using it. This was a simple way to get rid of
the unnecessary option without impacting apps.
IIRC the main reason we had HPET off by default was that lots of distro
kernels in the past had the necessary MMAP_HPET support disabled by
default, making it awkward to use - the user had to compile the kernel.
However, I was pleasantly surprised to discover that Ubuntu has it enabled
by default so the HPET now works out of the box, and it's interesting to
see the timer_autotest doing TSC and HPET clock comparisons.
/Bruce
^ permalink raw reply
* RE: [RFC PATCH 3/3] build: deprecate HPET build option
From: Morten Brørup @ 2026-06-02 12:41 UTC (permalink / raw)
To: David Marchand, Bruce Richardson; +Cc: dev
In-Reply-To: <CAJFAV8x5_JihAXe_bruriVu01b5wmF4fJ7vD42yFfNwpJQwDtQ@mail.gmail.com>
> On the other hand, did you consider dropping HPET altogether?
Google AI says that - on modern CPUs - the HPET has no advantages over the TSC.
This supports David's idea.
Suggestion:
Rewrite HPET APIs as a shim to TSC, and deprecate the HPET APIs.
-Morten
^ permalink raw reply
* [PATCH] graph/conn: fix memory leak on socket init failure
From: Alexey Simakov @ 2026-06-02 13:12 UTC (permalink / raw)
To: Sunil Kumar Kori
Cc: Alexey Simakov, Rakesh Kudurumalla, Nithin Dabilpuram,
Jerin Jacob, dev, sdl.dpdk
In conn_init(), conn_free() does not release the buffer allocated
for conn structure, resulting in a memory leak when server socket
creation fails.
Add missing free(c->buf) call to conn_free().
Fixes: 3f90eda5b7fb ("app/graph: add telnet connectivity")
Cc: skori@marvell.com
Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
---
app/graph/conn.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/app/graph/conn.c b/app/graph/conn.c
index c5e1c1ae1b..e73e929a08 100644
--- a/app/graph/conn.c
+++ b/app/graph/conn.c
@@ -197,6 +197,7 @@ conn_free(struct conn *c)
free(c->msg_in);
free(c->prompt);
free(c->welcome);
+ free(c->buf);
free(c);
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v4 06/10] net/mlx5: support selective Rx
From: Stephen Hemminger @ 2026-06-02 13:53 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev, Gregory Etelson, Dariusz Sosnowski, Viacheslav Ovsiienko,
Bing Zhao, Ori Kam, Suanming Mou, Matan Azrad
In-Reply-To: <20260529133522.2646044-7-thomas@monjalon.net>
On Fri, 29 May 2026 15:34:00 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:
> From: Gregory Etelson <getelson@nvidia.com>
>
> Selective Rx may save some PCI bandwidth.
> Implement selective Rx in the (quite slow) scalar SPRQ Rx path
> mlx5_rx_burst() where the performance impact
> of the added condition branches is acceptable.
> Other Rx functions do not support this feature.
> When using selective Rx, mlx5_rx_burst will be selected.
>
> A null Memory Region (MR) is always allocated
> at shared device context initialization.
> The selective Rx capability is not advertised
> if this special MR allocation fails.
>
> For each Rx segment configured with a NULL mempool,
> a "null mbuf" is created.
> It is a fake mbuf allocated outside any mempool,
> used as a placeholder in the Rx ring.
> The null MR lkey is used in the WQE for these segments
> so the NIC writes received data to a discard buffer.
> The mbuf data room size is resolved from the first segment having a pool.
> For null segments, the buffer length is from the last seen pool,
> so that the WQE stride size remains consistent.
>
> In mlx5_rx_burst, discarded segments are not chained
> into the packet mbuf list, NB_SEGS is decremented accordingly,
> and no replacement buffer is allocated.
> A separate data_seg_len accumulator tracks the total length
> of delivered segments only.
> The packet length is adjusted to reflect only the data
> actually delivered to the application.
>
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
AI review with Opus 4.8 and High setting found one issue:
Patch 6: net/mlx5: support selective Rx
Error: NULL pointer dereference when the first configured Rx segment is a
discard segment (mp == NULL).
In mlx5_rx_burst() the head mbuf and the chain tail are tracked like this:
if (pkt) {
if (rep->pool)
NEXT(tail) = rep;
else
--NB_SEGS(pkt);
}
...
if (seg->pool) {
tail = seg;
...
}
tail is only ever assigned inside "if (seg->pool)", and pkt is set to the
first processed segment unconditionally (pkt = seg in the !pkt block, no
pool guard). So if the first segment of a packet is a discard segment:
pkt becomes the null_mbuf (pool == NULL), tail stays NULL;
on the next (real) segment, rep->pool is set, so NEXT(tail) = rep executes with tail == NULL -> write through NULL.
Even without the crash, returning the pool-less null_mbuf as the packet
head is wrong: the application later frees it back to a NULL pool.
This is reachable, not theoretical. testpmd (patch 3) inserts a leading
mp==NULL segment whenever the first offset is > 0 (seg_offset > next_offset
with next_offset starting at 0), ethdev check_split (patch 2) now permits a
leading NULL mp, and mlx5_rxq_new() accepts it (first_mp is just the first
non-NULL pool; there is no requirement that rxseg[0].mp != NULL). The DTS
cases selective_rx_payload_only (rxoffs=[34]) and selective_rx_two_segments
(rxoffs=[14,...]) in patch 10 configure exactly this layout, and
mlx5_selective_rx_enabled() forces the scalar mlx5_rx_burst path, so the
buggy path is the one that runs.
Trace for rxoffs=34 / rxpkts=payload (segments: discard[0,34) real[34,290)
discard[290,max)):
iter0 (discard head): pkt == NULL, seg->pool == NULL -> pkt = null_mbuf,
tail not set; len(290) > DATA_LEN(34) -> ++NB_SEGS, continue.
iter1 (real seg): pkt set, rep->pool != NULL -> NEXT(tail==NULL)=rep.
Suggested fix: a discard segment must not become the packet head/tail.
Either reject rxseg[0].mp == NULL in mlx5_rxq_new() (cleanest, matches the
"deliver last N bytes" case being unsupported here), or make the data path
skip leading discard segments without assigning them to pkt and only set
pkt/tail on the first segment with a pool. If leading discard is intended
to be supported, the head selection and NEXT(tail) linking both need to
account for tail == NULL.
The same head/tail assumption also means a packet that falls entirely
within a leading discard segment would be returned with a NULL-pool head;
fixing the above covers that too.
^ permalink raw reply
* Re: [PATCH v3] net/intel: optimize for fast-free hint
From: Bruce Richardson @ 2026-06-02 15:36 UTC (permalink / raw)
To: Loftus, Ciara; +Cc: dev@dpdk.org, mb@smartsharesystems.com
In-Reply-To: <DM3PPF7D18F34A17BA228196DB8843D7D6F8E092@DM3PPF7D18F34A1.namprd11.prod.outlook.com>
On Thu, May 28, 2026 at 02:23:00PM +0100, Loftus, Ciara wrote:
> > Subject: [PATCH v3] net/intel: optimize for fast-free hint
> >
>
> snip
>
> > diff --git a/drivers/net/intel/common/tx_scalar.h
> > b/drivers/net/intel/common/tx_scalar.h
> > index 9fcd2e4733..d27df34dfa 100644
> > --- a/drivers/net/intel/common/tx_scalar.h
> > +++ b/drivers/net/intel/common/tx_scalar.h
> > @@ -197,16 +197,64 @@ ci_tx_xmit_cleanup(struct ci_tx_queue *txq)
> > const uint16_t rs_idx = (last_desc_cleaned == nb_tx_desc - 1) ?
> > 0 :
> > (last_desc_cleaned + 1) >> txq->log2_rs_thresh;
> > - uint16_t desc_to_clean_to = (rs_idx << txq->log2_rs_thresh) + (txq-
> > >tx_rs_thresh - 1);
> > + const uint16_t dd_idx = txq->rs_last_id[rs_idx];
> > + const uint16_t first_to_clean = rs_idx << txq->log2_rs_thresh;
> >
> > - /* Check if descriptor is done */
> > - if ((txd[txq->rs_last_id[rs_idx]].cmd_type_offset_bsz &
> > - rte_cpu_to_le_64(CI_TXD_QW1_DTYPE_M)) !=
> > -
> > rte_cpu_to_le_64(CI_TX_DESC_DTYPE_DESC_DONE))
> > + /* Check if descriptor is done - all drivers use 0xF as done value in bits
> > 3:0 */
> > + if ((txd[dd_idx].cmd_type_offset_bsz &
> > rte_cpu_to_le_64(CI_TXD_QW1_DTYPE_M)) !=
> > + rte_cpu_to_le_64(CI_TX_DESC_DTYPE_DESC_DONE))
> > + /* Descriptor not yet processed by hardware */
> > return -1;
> >
> > + /* DD bit is set, descriptors are done. Now free the mbufs. */
> > + /* Note: nb_tx_desc is guaranteed to be a multiple of tx_rs_thresh,
> > + * validated during queue setup. This means cleanup never wraps
> > around
> > + * the ring within a single burst (e.g., ring=256, rs_thresh=32 gives
> > + * bursts of 0-31, 32-63, ..., 224-255).
> > + */
> > + const uint16_t nb_to_clean = txq->tx_rs_thresh;
> > + struct ci_tx_entry *sw_ring = txq->sw_ring;
> > +
> > + /* fast_free_mp is NULL only when the fast free is disabled*/
> > + if (txq->fast_free_mp != NULL) {
> > + /* FAST_FREE path: mbufs are already reset, just return to
> > pool */
> > + struct rte_mbuf *free[CI_TX_MAX_FREE_BUF_SZ];
> > + uint16_t nb_free = 0;
> > +
> > + /* Get cached mempool pointer, or cache it on first use */
> > + struct rte_mempool *mp =
> > + likely(txq->fast_free_mp != (void *)UINTPTR_MAX) ?
> > + txq->fast_free_mp :
> > + (txq->fast_free_mp = sw_ring[dd_idx].mbuf->pool);
>
> Is the mbuf you are dereferencing here guaranteed to be non NULL?
>
Good point. It almost certainly is, but checking through the logic of the
code, the one possible error condition is where we have the last segment of
the packet being a large segment using TSO which is to be split across
multiple data descriptors. In that case (only) the mbuf pointer can be
NULL. In all other cases, it should be valid, because the DD bit can only
be set on a data descriptor.
Rather than trying to fix that issue here on cleanup, I think the better
solution is to adjust the large segment handling so that the last
descriptor of the segment, rather than the first, has the mbuf pointer.
That way we can always know that the descriptor that has DD set on it will
have a valid mbuf. Will implement this as a separate patch in v4.
/Bruce
^ permalink raw reply
* [PATCH v4 0/2] net/intel: optimize for fast-free hint
From: Bruce Richardson @ 2026-06-02 15:45 UTC (permalink / raw)
To: dev; +Cc: mb, ciara.loftus, Bruce Richardson
In-Reply-To: <20260123112032.2174361-1-bruce.richardson@intel.com>
When the fast-free hint is provided to the driver we know that the mbufs
have refcnt of 1 and are from the same mempool. Therefore, we can
optimize a bit for this case even in the scalar path of our drivers.
---
v4:
* add precursor patch to adjust mbuf pointers so that the DD bit
is written to a descriptor with a valid mbuf pointer associated
with it.
v3:
* used mbuf_raw_free_bulk rather than mempool function directly
* check for fast_free via mp pointer rather than flags
* remove unnecessary prefetches
V2: Fix issues with original submission:
* missed check for NULL mbufs
* fixed issue with freeing directly from sw_ring in scalar path which
doesn't work as thats not a flag array of pointers
* fixed missing null assignment in case of large segments for TSO
Bruce Richardson (2):
net/intel: write mbuf for last Tx desc of segment
net/intel: optimize for fast-free hint
drivers/net/intel/common/tx.h | 21 ++++--
drivers/net/intel/common/tx_scalar.h | 98 +++++++++++++++++++++-------
2 files changed, 90 insertions(+), 29 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH v4 1/2] net/intel: write mbuf for last Tx desc of segment
From: Bruce Richardson @ 2026-06-02 15:45 UTC (permalink / raw)
To: dev; +Cc: mb, ciara.loftus, Bruce Richardson
In-Reply-To: <20260602154513.1079865-1-bruce.richardson@intel.com>
When a single mbuf segment has more data than can be handled by a single
Tx data descriptor in the TSO case, adjust how the storing of mbufs is
being done. Rather than putting the mbuf pointer in the first slot for
that segment, store it in the last slot instead. This guarantees for us
that the descriptor for which we have desc-done (DD) writeback always
has a valid mbuf associated with it.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/common/tx_scalar.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/intel/common/tx_scalar.h b/drivers/net/intel/common/tx_scalar.h
index 9fcd2e4733..7809bd53e8 100644
--- a/drivers/net/intel/common/tx_scalar.h
+++ b/drivers/net/intel/common/tx_scalar.h
@@ -509,7 +509,6 @@ ci_xmit_pkts(struct ci_tx_queue *txq,
if (txe->mbuf)
rte_pktmbuf_free_seg(txe->mbuf);
- txe->mbuf = m_seg;
/* Setup TX Descriptor */
/* Calculate segment length, using IPsec callback if provided */
@@ -528,6 +527,7 @@ ci_xmit_pkts(struct ci_tx_queue *txq,
((uint64_t)CI_MAX_DATA_PER_TXD << CI_TXD_QW1_TX_BUF_SZ_S) |
((uint64_t)td_tag << CI_TXD_QW1_L2TAG1_S);
write_txd(txd, buf_dma_addr, cmd_type_offset_bsz);
+ txe->mbuf = NULL;
buf_dma_addr += CI_MAX_DATA_PER_TXD;
slen -= CI_MAX_DATA_PER_TXD;
@@ -548,6 +548,7 @@ ci_xmit_pkts(struct ci_tx_queue *txq,
((uint64_t)slen << CI_TXD_QW1_TX_BUF_SZ_S) |
((uint64_t)td_tag << CI_TXD_QW1_L2TAG1_S);
write_txd(txd, buf_dma_addr, cmd_type_offset_bsz);
+ txe->mbuf = m_seg;
tx_id = txe->next_id;
txe = txn;
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox