[PATCH 00/10] Run with UBSan in GHA

[PATCH 00/10] Run with UBSan in GHA

[David Marchand]

This series fixes a number of issues reported by UBSan and adds a simple
job in GHA to avoid introducing undefined behavior in the core
components.
There is way more work/fixes to do if we want to run with a full set of
components, but baby steps first.


-- 
David Marchand

David Marchand (10):
  ci: save ccache on failure
  test/telemetry: fix test calling all commands
  test/mempool: fix test without stack driver
  eal: fix plugin dir walk
  cmdline: fix port list parsing
  cmdline: fix highest bit port list parsing
  tailq: fix cast macro for null pointer
  hash: fix unaligned access in predictable RSS
  stack: fix unaligned accesses on 128-bit
  build: support Undefined Behavior Sanitizer

 .ci/linux-build.sh                   | 27 +++++++++++++++++++++--
 .github/workflows/build.yml          | 11 ++++++++++
 app/test/suites/meson.build          |  3 +--
 app/test/suites/test_telemetry.sh    |  2 +-
 app/test/test_mempool.c              | 32 +++++++++++++++++-----------
 config/meson.build                   | 18 +++++++++++++++-
 devtools/words-case.txt              |  1 +
 lib/cmdline/cmdline_parse_portlist.c | 17 ++++++++++-----
 lib/eal/common/eal_common_options.c  | 15 +++++++++----
 lib/eal/include/rte_tailq.h          |  2 +-
 lib/hash/rte_thash.c                 |  6 +++---
 lib/stack/rte_stack_lf_c11.h         |  8 +++----
 lib/stack/rte_stack_lf_generic.h     |  8 +++----
 13 files changed, 111 insertions(+), 39 deletions(-)

-- 
2.49.0

[PATCH 01/10] ci: save ccache on failure

[David Marchand]

When troubleshooting unit test failures and repeating jobs in GHA,
the absence of ccache makes the whole process way slower.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .github/workflows/build.yml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index e5f17ef6ac..6c4bc664d0 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -164,6 +164,12 @@ jobs:
         chmod o-w $HOME
     - name: Build and test
       run: .ci/linux-build.sh
+    - name: Save ccache on failure
+      if: failure()
+      uses: actions/cache/save@v4
+      with:
+        path: ~/.ccache
+        key: ${{ steps.get_ref_keys.outputs.ccache }}-${{ github.ref }}
     - name: Upload logs on failure
       if: failure()
       uses: actions/upload-artifact@v4
-- 
2.49.0

Re: [PATCH 01/10] ci: save ccache on failure

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> When troubleshooting unit test failures and repeating jobs in GHA,
> the absence of ccache makes the whole process way slower.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

>  .github/workflows/build.yml | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
> index e5f17ef6ac..6c4bc664d0 100644
> --- a/.github/workflows/build.yml
> +++ b/.github/workflows/build.yml
> @@ -164,6 +164,12 @@ jobs:
>          chmod o-w $HOME
>      - name: Build and test
>        run: .ci/linux-build.sh
> +    - name: Save ccache on failure
> +      if: failure()
> +      uses: actions/cache/save@v4
> +      with:
> +        path: ~/.ccache
> +        key: ${{ steps.get_ref_keys.outputs.ccache }}-${{ github.ref }}
>      - name: Upload logs on failure
>        if: failure()
>        uses: actions/upload-artifact@v4

[PATCH 02/10] test/telemetry: fix test calling all commands

[David Marchand]

This test was doing nothing as it could not find the telemetry client
script following the test suite rework.

Caught while looking at UNH unit test logs:

/root/workspace/Generic-Unit-Test-DPDK/dpdk/app/test/suites/test_telemetry.sh:
18: /root/workspace/Generic-Unit-Test-DPDK/dpdk/app/usertools/dpdk-telemetry.py:
not found

Fixes: 9da71dc4f96e ("test: add test case for scripted telemetry commands")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/suites/test_telemetry.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/suites/test_telemetry.sh b/app/test/suites/test_telemetry.sh
index ca6abe266e..20806b43e4 100755
--- a/app/test/suites/test_telemetry.sh
+++ b/app/test/suites/test_telemetry.sh
@@ -7,7 +7,7 @@ which jq || {
     exit 77
 }
 
-rootdir=$(readlink -f $(dirname $(readlink -f $0))/../..)
+rootdir=$(readlink -f $(dirname $(readlink -f $0))/../../..)
 tmpoutput=$(mktemp -t dpdk.test_telemetry.XXXXXX)
 trap "cat $tmpoutput; rm -f $tmpoutput" EXIT
 
-- 
2.49.0

Re: [PATCH 02/10] test/telemetry: fix test calling all commands

[Bruce Richardson]

On Thu, Jun 19, 2025 at 09:10:28AM +0200, David Marchand wrote:
> This test was doing nothing as it could not find the telemetry client
> script following the test suite rework.
> 
> Caught while looking at UNH unit test logs:
> 
> /root/workspace/Generic-Unit-Test-DPDK/dpdk/app/test/suites/test_telemetry.sh:
> 18: /root/workspace/Generic-Unit-Test-DPDK/dpdk/app/usertools/dpdk-telemetry.py:
> not found
> 
> Fixes: 9da71dc4f96e ("test: add test case for scripted telemetry commands")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  app/test/suites/test_telemetry.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test/suites/test_telemetry.sh b/app/test/suites/test_telemetry.sh
> index ca6abe266e..20806b43e4 100755
> --- a/app/test/suites/test_telemetry.sh
> +++ b/app/test/suites/test_telemetry.sh
> @@ -7,7 +7,7 @@ which jq || {
>      exit 77
>  }
>  
> -rootdir=$(readlink -f $(dirname $(readlink -f $0))/../..)
> +rootdir=$(readlink -f $(dirname $(readlink -f $0))/../../..)
>  tmpoutput=$(mktemp -t dpdk.test_telemetry.XXXXXX)
>  trap "cat $tmpoutput; rm -f $tmpoutput" EXIT
>  
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH 02/10] test/telemetry: fix test calling all commands

[David Marchand]

Hello Shai,

On Thu, Jun 19, 2025 at 9:11 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> This test was doing nothing as it could not find the telemetry client
> script following the test suite rework.
>
> Caught while looking at UNH unit test logs:
>
> /root/workspace/Generic-Unit-Test-DPDK/dpdk/app/test/suites/test_telemetry.sh:
> 18: /root/workspace/Generic-Unit-Test-DPDK/dpdk/app/usertools/dpdk-telemetry.py:
> not found
>
> Fixes: 9da71dc4f96e ("test: add test case for scripted telemetry commands")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

This fix is flagged as a failure because telemetry_all hits a 10s
timeout in AWS CI.
https://mails.dpdk.org/archives/test-report/2025-June/887693.html

Please use a 30s timeout (like other CI do for fast-tests), iow pass a
'-t 3' to the meson test command.

Thanks.


-- 
David Marchand

[PATCH 03/10] test/mempool: fix test without stack driver

[David Marchand]

In a minimal build, the mempool/stack driver is disabled.
Separate the code specific to this external driver and rename unrelated
variables.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_mempool.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 61385e096e..63356998fd 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -847,9 +847,11 @@ test_mempool(void)
 	size_t alignment = 0;
 	struct rte_mempool *mp_cache = NULL;
 	struct rte_mempool *mp_nocache = NULL;
-	struct rte_mempool *mp_stack_anon = NULL;
-	struct rte_mempool *mp_stack_mempool_iter = NULL;
+	struct rte_mempool *mp_anon = NULL;
+	struct rte_mempool *mp_mempool_iter = NULL;
+#ifdef RTE_MEMPOOL_STACK
 	struct rte_mempool *mp_stack = NULL;
+#endif
 	struct rte_mempool *default_pool = NULL;
 	struct rte_mempool *mp_alignment = NULL;
 	struct mp_data cb_arg = {
@@ -884,28 +886,28 @@ test_mempool(void)
 	}
 
 	/* create an empty mempool  */
-	mp_stack_anon = rte_mempool_create_empty("test_stack_anon",
+	mp_anon = rte_mempool_create_empty("test_anon",
 		MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE,
 		RTE_MEMPOOL_CACHE_MAX_SIZE, 0,
 		SOCKET_ID_ANY, 0);
 
-	if (mp_stack_anon == NULL)
+	if (mp_anon == NULL)
 		GOTO_ERR(ret, err);
 
 	/* populate an empty mempool */
-	ret = rte_mempool_populate_anon(mp_stack_anon);
+	ret = rte_mempool_populate_anon(mp_anon);
 	printf("%s ret = %d\n", __func__, ret);
 	if (ret < 0)
 		GOTO_ERR(ret, err);
 
 	/* Try to populate when already populated */
-	ret = rte_mempool_populate_anon(mp_stack_anon);
+	ret = rte_mempool_populate_anon(mp_anon);
 	if (ret != 0)
 		GOTO_ERR(ret, err);
 
 	/* create a mempool  */
-	mp_stack_mempool_iter = rte_mempool_create("test_iter_obj",
+	mp_mempool_iter = rte_mempool_create("test_iter_obj",
 		MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE,
 		RTE_MEMPOOL_CACHE_MAX_SIZE, 0,
@@ -913,20 +915,21 @@ test_mempool(void)
 		my_obj_init, NULL,
 		SOCKET_ID_ANY, 0);
 
-	if (mp_stack_mempool_iter == NULL)
+	if (mp_mempool_iter == NULL)
 		GOTO_ERR(ret, err);
 
 	/* test to initialize mempool objects and memory */
-	nb_objs = rte_mempool_obj_iter(mp_stack_mempool_iter, my_obj_init,
+	nb_objs = rte_mempool_obj_iter(mp_mempool_iter, my_obj_init,
 			NULL);
 	if (nb_objs == 0)
 		GOTO_ERR(ret, err);
 
-	nb_mem_chunks = rte_mempool_mem_iter(mp_stack_mempool_iter,
+	nb_mem_chunks = rte_mempool_mem_iter(mp_mempool_iter,
 			test_mp_mem_init, &cb_arg);
 	if (nb_mem_chunks == 0 || cb_arg.ret < 0)
 		GOTO_ERR(ret, err);
 
+#ifdef RTE_MEMPOOL_STACK
 	/* create a mempool with an external handler */
 	mp_stack = rte_mempool_create_empty("test_stack",
 		MEMPOOL_SIZE,
@@ -947,6 +950,7 @@ test_mempool(void)
 		GOTO_ERR(ret, err);
 	}
 	rte_mempool_obj_iter(mp_stack, my_obj_init, NULL);
+#endif /* RTE_MEMPOOL_STACK */
 
 	/* Create a mempool based on Default handler */
 	printf("Testing %s mempool handler\n", default_pool_ops);
@@ -1077,9 +1081,11 @@ test_mempool(void)
 	if (test_mempool_same_name_twice_creation() < 0)
 		GOTO_ERR(ret, err);
 
+#ifdef RTE_MEMPOOL_STACK
 	/* test the stack handler */
 	if (test_mempool_basic(mp_stack, 1) < 0)
 		GOTO_ERR(ret, err);
+#endif
 
 	if (test_mempool_basic(default_pool, 1) < 0)
 		GOTO_ERR(ret, err);
@@ -1105,9 +1111,11 @@ test_mempool(void)
 err:
 	rte_mempool_free(mp_nocache);
 	rte_mempool_free(mp_cache);
-	rte_mempool_free(mp_stack_anon);
-	rte_mempool_free(mp_stack_mempool_iter);
+	rte_mempool_free(mp_anon);
+	rte_mempool_free(mp_mempool_iter);
+#ifdef RTE_MEMPOOL_STACK
 	rte_mempool_free(mp_stack);
+#endif
 	rte_mempool_free(default_pool);
 	rte_mempool_free(mp_alignment);
 
-- 
2.49.0

Re: [PATCH 03/10] test/mempool: fix test without stack driver

[Andrew Rybchenko]

On 6/19/25 10:10 AM, David Marchand wrote:
> In a minimal build, the mempool/stack driver is disabled.
> Separate the code specific to this external driver and rename unrelated
> variables.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

[PATCH 04/10] eal: fix plugin dir walk

[David Marchand]

For '.' and '..' directories (or any short file name),
a out of bound issue occurs.

Caught by UBSan:

EAL: Detected shared linkage of DPDK
../lib/eal/common/eal_common_options.c:420:15: runtime error: index -2
	out of bounds for type 'char[256]'
    #0 0x7f867eedf206 in eal_plugindir_init
	eal_common_options.c
    #1 0x7f867eede58a in eal_plugins_init
	(build/lib/librte_eal.so.25+0xde58a)
	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
    #2 0x7f867efb8587 in rte_eal_init
	(build/lib/librte_eal.so.25+0x1b8587)
	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
    #3 0x55b62360861e in main
	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x9e061e)
	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
    #4 0x7f8667429d8f in __libc_start_call_main
	csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #5 0x7f8667429e3f in __libc_start_main
	csu/../csu/libc-start.c:392:3
    #6 0x55b622d9d444 in _start
	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x175444)
	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/common/eal_common_options.c:420:15 in
	../lib/eal/common/eal_common_options.c:421:15:
	runtime error: index 18446744073709551609 out of bounds
	for type 'char[256]'

Fixes: c57f6e5c604a ("eal: fix plugin loading")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/common/eal_common_options.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 83b6fc7e89..153f807e4f 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -399,6 +399,14 @@ eal_plugins_init(void)
 }
 #else
 
+static bool
+ends_with(const char *str, size_t str_len, const char *tail)
+{
+	size_t tail_len = strlen(tail);
+
+	return str_len >= tail_len && strncmp(&str[str_len - tail_len], tail, tail_len) == 0;
+}
+
 static int
 eal_plugindir_init(const char *path)
 {
@@ -417,13 +425,12 @@ eal_plugindir_init(const char *path)
 	}
 
 	while ((dent = readdir(d)) != NULL) {
+		size_t nlen = strnlen(dent->d_name, sizeof(dent->d_name));
 		struct stat sb;
-		int nlen = strnlen(dent->d_name, sizeof(dent->d_name));
 
 		/* check if name ends in .so or .so.ABI_VERSION */
-		if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 &&
-		    strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)],
-			   ".so."ABI_VERSION) != 0)
+		if (!ends_with(dent->d_name, nlen, ".so") &&
+				!ends_with(dent->d_name, nlen, ".so."ABI_VERSION))
 			continue;
 
 		snprintf(sopath, sizeof(sopath), "%s/%s", path, dent->d_name);
-- 
2.49.0

Re: [PATCH 04/10] eal: fix plugin dir walk

[Bruce Richardson]

On Thu, Jun 19, 2025 at 09:10:30AM +0200, David Marchand wrote:
> For '.' and '..' directories (or any short file name),
> a out of bound issue occurs.
> 
> Caught by UBSan:
> 
> EAL: Detected shared linkage of DPDK
> ../lib/eal/common/eal_common_options.c:420:15: runtime error: index -2
> 	out of bounds for type 'char[256]'
>     #0 0x7f867eedf206 in eal_plugindir_init
> 	eal_common_options.c
>     #1 0x7f867eede58a in eal_plugins_init
> 	(build/lib/librte_eal.so.25+0xde58a)
> 	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
>     #2 0x7f867efb8587 in rte_eal_init
> 	(build/lib/librte_eal.so.25+0x1b8587)
> 	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
>     #3 0x55b62360861e in main
> 	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x9e061e)
> 	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
>     #4 0x7f8667429d8f in __libc_start_call_main
> 	csu/../sysdeps/nptl/libc_start_call_main.h:58:16
>     #5 0x7f8667429e3f in __libc_start_main
> 	csu/../csu/libc-start.c:392:3
>     #6 0x55b622d9d444 in _start
> 	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x175444)
> 	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> 	../lib/eal/common/eal_common_options.c:420:15 in
> 	../lib/eal/common/eal_common_options.c:421:15:
> 	runtime error: index 18446744073709551609 out of bounds
> 	for type 'char[256]'
> 
> Fixes: c57f6e5c604a ("eal: fix plugin loading")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

One thought inline below...

>  lib/eal/common/eal_common_options.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> index 83b6fc7e89..153f807e4f 100644
> --- a/lib/eal/common/eal_common_options.c
> +++ b/lib/eal/common/eal_common_options.c
> @@ -399,6 +399,14 @@ eal_plugins_init(void)
>  }
>  #else
>  
> +static bool
> +ends_with(const char *str, size_t str_len, const char *tail)
> +{
> +	size_t tail_len = strlen(tail);
> +
> +	return str_len >= tail_len && strncmp(&str[str_len - tail_len], tail, tail_len) == 0;
> +}
> +

I wonder if that function is worth renaming to "rte_str_ends_with" and
putting in rte_string_fns.h?


>  static int
>  eal_plugindir_init(const char *path)
>  {
> @@ -417,13 +425,12 @@ eal_plugindir_init(const char *path)
>  	}
>  
>  	while ((dent = readdir(d)) != NULL) {
> +		size_t nlen = strnlen(dent->d_name, sizeof(dent->d_name));
>  		struct stat sb;
> -		int nlen = strnlen(dent->d_name, sizeof(dent->d_name));
>  
>  		/* check if name ends in .so or .so.ABI_VERSION */
> -		if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 &&
> -		    strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)],
> -			   ".so."ABI_VERSION) != 0)
> +		if (!ends_with(dent->d_name, nlen, ".so") &&
> +				!ends_with(dent->d_name, nlen, ".so."ABI_VERSION))
>  			continue;
>  
>  		snprintf(sopath, sizeof(sopath), "%s/%s", path, dent->d_name);
> -- 
> 2.49.0
> 

Re: [PATCH 04/10] eal: fix plugin dir walk

[David Marchand]

On Fri, Jun 20, 2025 at 11:19 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Jun 19, 2025 at 09:10:30AM +0200, David Marchand wrote:
> > For '.' and '..' directories (or any short file name),
> > a out of bound issue occurs.
> >
> > Caught by UBSan:
> >
> > EAL: Detected shared linkage of DPDK
> > ../lib/eal/common/eal_common_options.c:420:15: runtime error: index -2
> >       out of bounds for type 'char[256]'
> >     #0 0x7f867eedf206 in eal_plugindir_init
> >       eal_common_options.c
> >     #1 0x7f867eede58a in eal_plugins_init
> >       (build/lib/librte_eal.so.25+0xde58a)
> >       (BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
> >     #2 0x7f867efb8587 in rte_eal_init
> >       (build/lib/librte_eal.so.25+0x1b8587)
> >       (BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
> >     #3 0x55b62360861e in main
> >       (/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x9e061e)
> >       (BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
> >     #4 0x7f8667429d8f in __libc_start_call_main
> >       csu/../sysdeps/nptl/libc_start_call_main.h:58:16
> >     #5 0x7f8667429e3f in __libc_start_main
> >       csu/../csu/libc-start.c:392:3
> >     #6 0x55b622d9d444 in _start
> >       (/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x175444)
> >       (BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
> > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> >       ../lib/eal/common/eal_common_options.c:420:15 in
> >       ../lib/eal/common/eal_common_options.c:421:15:
> >       runtime error: index 18446744073709551609 out of bounds
> >       for type 'char[256]'
> >
> > Fixes: c57f6e5c604a ("eal: fix plugin loading")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> One thought inline below...
>
> >  lib/eal/common/eal_common_options.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> > index 83b6fc7e89..153f807e4f 100644
> > --- a/lib/eal/common/eal_common_options.c
> > +++ b/lib/eal/common/eal_common_options.c
> > @@ -399,6 +399,14 @@ eal_plugins_init(void)
> >  }
> >  #else
> >
> > +static bool
> > +ends_with(const char *str, size_t str_len, const char *tail)
> > +{
> > +     size_t tail_len = strlen(tail);
> > +
> > +     return str_len >= tail_len && strncmp(&str[str_len - tail_len], tail, tail_len) == 0;
> > +}
> > +
>
> I wonder if that function is worth renaming to "rte_str_ends_with" and
> putting in rte_string_fns.h?

I'll have a look and see if we have potential users in the tree.



-- 
David Marchand

[PATCH 05/10] cmdline: fix port list parsing

[David Marchand]

Doing arithmetics with the NULL pointer is undefined.

Caught by UBSan:

../lib/cmdline/cmdline_parse_portlist.c:40:19: runtime error:
	applying non-zero offset 1 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/cmdline/cmdline_parse_portlist.c:40:19 in

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/cmdline/cmdline_parse_portlist.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
index ef6ce223b5..0c07cc02b5 100644
--- a/lib/cmdline/cmdline_parse_portlist.c
+++ b/lib/cmdline/cmdline_parse_portlist.c
@@ -4,6 +4,7 @@
  * All rights reserved.
  */
 
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -37,10 +38,11 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
 	const char *first, *last;
 	char *end;
 
-	for (first = str, last = first;
-	    first != NULL && last != NULL;
-	    first = last + 1) {
+	if (str == NULL)
+		return 0;
 
+	last = first = str;
+	do {
 		last = strchr(first, ',');
 
 		errno = 0;
@@ -65,7 +67,10 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
 			return -1;
 
 		parse_set_list(pl, ps, pe);
-	}
+		if (last == NULL)
+			break;
+		first = last + 1;
+	} while (true);
 
 	return 0;
 }
-- 
2.49.0

Re: [PATCH 05/10] cmdline: fix port list parsing

[Bruce Richardson]

On Thu, Jun 19, 2025 at 09:10:31AM +0200, David Marchand wrote:
> Doing arithmetics with the NULL pointer is undefined.
> 
> Caught by UBSan:
> 
> ../lib/cmdline/cmdline_parse_portlist.c:40:19: runtime error:
> 	applying non-zero offset 1 to null pointer
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> 	../lib/cmdline/cmdline_parse_portlist.c:40:19 in
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/cmdline/cmdline_parse_portlist.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
> index ef6ce223b5..0c07cc02b5 100644
> --- a/lib/cmdline/cmdline_parse_portlist.c
> +++ b/lib/cmdline/cmdline_parse_portlist.c
> @@ -4,6 +4,7 @@
>   * All rights reserved.
>   */
>  
> +#include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -37,10 +38,11 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
>  	const char *first, *last;
>  	char *end;
>  
> -	for (first = str, last = first;
> -	    first != NULL && last != NULL;
> -	    first = last + 1) {

Maybe I'm a little slow this morning, but I can't see how this is actually
a problem. By my understanding, the check for "first != NULL && last !=
NULL" happens before any increment of "first = last + 1", meaning we are
guaranteed that the last is never null when we increment it.

/Bruce

> +	if (str == NULL)
> +		return 0;
>  
> +	last = first = str;
> +	do {
>  		last = strchr(first, ',');
>  
>  		errno = 0;
> @@ -65,7 +67,10 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
>  			return -1;
>  
>  		parse_set_list(pl, ps, pe);
> -	}
> +		if (last == NULL)
> +			break;
> +		first = last + 1;
> +	} while (true);
>  
>  	return 0;
>  }
> -- 
> 2.49.0
> 

Re: [PATCH 05/10] cmdline: fix port list parsing

[David Marchand]

On Fri, Jun 20, 2025 at 11:59 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Jun 19, 2025 at 09:10:31AM +0200, David Marchand wrote:
> > Doing arithmetics with the NULL pointer is undefined.
> >
> > Caught by UBSan:
> >
> > ../lib/cmdline/cmdline_parse_portlist.c:40:19: runtime error:
> >       applying non-zero offset 1 to null pointer
> > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> >       ../lib/cmdline/cmdline_parse_portlist.c:40:19 in
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  lib/cmdline/cmdline_parse_portlist.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
> > index ef6ce223b5..0c07cc02b5 100644
> > --- a/lib/cmdline/cmdline_parse_portlist.c
> > +++ b/lib/cmdline/cmdline_parse_portlist.c
> > @@ -4,6 +4,7 @@
> >   * All rights reserved.
> >   */
> >
> > +#include <stdbool.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> > @@ -37,10 +38,11 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
> >       const char *first, *last;
> >       char *end;
> >
> > -     for (first = str, last = first;
> > -         first != NULL && last != NULL;
> > -         first = last + 1) {
>
> Maybe I'm a little slow this morning, but I can't see how this is actually
> a problem. By my understanding, the check for "first != NULL && last !=
> NULL" happens before any increment of "first = last + 1", meaning we are
> guaranteed that the last is never null when we increment it.

Well, not sure I follow, but the problem is not at the first
iteration, if this is what you mean.

On the last iteration of the parsing, there is no , left in the string
that is parsed so last = strchr(first, ',') makes last == NULL.
Then the first variable is set to last + 1 *before* evaluating the end
condition.

I removed this patch of the series, rerun the test and I see:

9/75 DPDK:fast-tests / cmdline_autotest               OK              0.22s
09:20:08 DPDK_TEST=cmdline_autotest MALLOC_PERTURB_=169
/home/runner/work/dpdk/dpdk/build/app/dpdk-test --no-huge -m 2048 -d
/home/runner/work/dpdk/dpdk/build/drivers
----------------------------------- output -----------------------------------
stdout:
RTE>>cmdline_autotest
Testind parsing ethernet addresses...
Testind parsing port lists...
Testind parsing numbers...
Testing parsing IP addresses...
Testing parsing strings...
Testing circular buffer...
Testing library functions...
Test OK
RTE>>
stderr:
EAL: Detected CPU lcores: 4
EAL: Detected NUMA nodes: 1
EAL: Detected shared linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
APP: HPET is not enabled, using TSC as default timer
../lib/cmdline/cmdline_parse_portlist.c:44:19: runtime error: applying
non-zero offset 1 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../lib/cmdline/cmdline_parse_portlist.c:44:19 in
------------------------------------------------------------------------------


-- 
David Marchand

Re: [PATCH 05/10] cmdline: fix port list parsing

[Bruce Richardson]

On Mon, Jun 23, 2025 at 11:40:15AM +0200, David Marchand wrote:
> On Fri, Jun 20, 2025 at 11:59 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Thu, Jun 19, 2025 at 09:10:31AM +0200, David Marchand wrote:
> > > Doing arithmetics with the NULL pointer is undefined.
> > >
> > > Caught by UBSan:
> > >
> > > ../lib/cmdline/cmdline_parse_portlist.c:40:19: runtime error:
> > >       applying non-zero offset 1 to null pointer
> > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> > >       ../lib/cmdline/cmdline_parse_portlist.c:40:19 in
> > >
> > > Fixes: af75078fece3 ("first public release")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  lib/cmdline/cmdline_parse_portlist.c | 13 +++++++++----
> > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
> > > index ef6ce223b5..0c07cc02b5 100644
> > > --- a/lib/cmdline/cmdline_parse_portlist.c
> > > +++ b/lib/cmdline/cmdline_parse_portlist.c
> > > @@ -4,6 +4,7 @@
> > >   * All rights reserved.
> > >   */
> > >
> > > +#include <stdbool.h>
> > >  #include <stdio.h>
> > >  #include <stdlib.h>
> > >  #include <string.h>
> > > @@ -37,10 +38,11 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
> > >       const char *first, *last;
> > >       char *end;
> > >
> > > -     for (first = str, last = first;
> > > -         first != NULL && last != NULL;
> > > -         first = last + 1) {
> >
> > Maybe I'm a little slow this morning, but I can't see how this is actually
> > a problem. By my understanding, the check for "first != NULL && last !=
> > NULL" happens before any increment of "first = last + 1", meaning we are
> > guaranteed that the last is never null when we increment it.
> 
> Well, not sure I follow, but the problem is not at the first
> iteration, if this is what you mean.
> 
> On the last iteration of the parsing, there is no , left in the string
> that is parsed so last = strchr(first, ',') makes last == NULL.
> Then the first variable is set to last + 1 *before* evaluating the end
> condition.
> 
> I removed this patch of the series, rerun the test and I see:
> 
> 9/75 DPDK:fast-tests / cmdline_autotest               OK              0.22s
> 09:20:08 DPDK_TEST=cmdline_autotest MALLOC_PERTURB_=169
> /home/runner/work/dpdk/dpdk/build/app/dpdk-test --no-huge -m 2048 -d
> /home/runner/work/dpdk/dpdk/build/drivers
> ----------------------------------- output -----------------------------------
> stdout:
> RTE>>cmdline_autotest
> Testind parsing ethernet addresses...
> Testind parsing port lists...
> Testind parsing numbers...
> Testing parsing IP addresses...
> Testing parsing strings...
> Testing circular buffer...
> Testing library functions...
> Test OK
> RTE>>
> stderr:
> EAL: Detected CPU lcores: 4
> EAL: Detected NUMA nodes: 1
> EAL: Detected shared linkage of DPDK
> EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'VA'
> APP: HPET is not enabled, using TSC as default timer
> ../lib/cmdline/cmdline_parse_portlist.c:44:19: runtime error: applying
> non-zero offset 1 to null pointer
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> ../lib/cmdline/cmdline_parse_portlist.c:44:19 in
> ------------------------------------------------------------------------------
> 
> 
Thanks for the explanation. I was indeed thinking the issue was on the
first iteration only.

With the change to fix this, we can actually make last a local var within
the loop itself. Also, by using a while rather than do-while we can remove
the initial check for str = NULL. Here's an alternate fix that is very
slightly shorter, and limits the scope of "last":

diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
index ebe2a961bb..c65f3b704e 100644
--- a/lib/cmdline/cmdline_parse_portlist.c
+++ b/lib/cmdline/cmdline_parse_portlist.c
@@ -34,14 +34,11 @@ static int
 parse_ports(cmdline_portlist_t *pl, const char *str)
 {
        size_t ps, pe;
-       const char *first, *last;
+       const char *first = str;
        char *end;
 
-       for (first = str, last = first;
-           first != NULL && last != NULL;
-           first = last + 1) {
-
-               last = strchr(first, ',');
+       while (first != NULL) {
+               const char *last = strchr(first, ',');
 
                errno = 0;
                ps = strtoul(first, &end, 10);
@@ -65,6 +62,8 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
                        return -1;
 
                parse_set_list(pl, ps, pe);
+
+               first = (last == NULL ? NULL : last + 1);
        }
 
        return 0;

[PATCH 06/10] cmdline: fix highest bit port list parsing

[David Marchand]

pl->map is a uint32_t.

Caught by UBSan:

../lib/cmdline/cmdline_parse_portlist.c:27:17: runtime error:
	left shift of 1 by 31 places cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/cmdline/cmdline_parse_portlist.c:27:17 in

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/cmdline/cmdline_parse_portlist.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
index 0c07cc02b5..3ef427d32a 100644
--- a/lib/cmdline/cmdline_parse_portlist.c
+++ b/lib/cmdline/cmdline_parse_portlist.c
@@ -11,7 +11,9 @@
 #include <errno.h>
 
 #include <eal_export.h>
+#include <rte_bitops.h>
 #include <rte_string_fns.h>
+
 #include "cmdline_parse.h"
 #include "cmdline_parse_portlist.h"
 
@@ -27,7 +29,7 @@ static void
 parse_set_list(cmdline_portlist_t *pl, size_t low, size_t high)
 {
 	do {
-		pl->map |= (1 << low++);
+		pl->map |= RTE_BIT32(low++);
 	} while (low <= high);
 }
 
-- 
2.49.0

Re: [PATCH 06/10] cmdline: fix highest bit port list parsing

[Bruce Richardson]

On Thu, Jun 19, 2025 at 09:10:32AM +0200, David Marchand wrote:
> pl->map is a uint32_t.
> 
> Caught by UBSan:
> 
> ../lib/cmdline/cmdline_parse_portlist.c:27:17: runtime error:
> 	left shift of 1 by 31 places cannot be represented in type 'int'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> 	../lib/cmdline/cmdline_parse_portlist.c:27:17 in
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/cmdline/cmdline_parse_portlist.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
> index 0c07cc02b5..3ef427d32a 100644
> --- a/lib/cmdline/cmdline_parse_portlist.c
> +++ b/lib/cmdline/cmdline_parse_portlist.c
> @@ -11,7 +11,9 @@
>  #include <errno.h>
>  
>  #include <eal_export.h>
> +#include <rte_bitops.h>
>  #include <rte_string_fns.h>
> +
>  #include "cmdline_parse.h"
>  #include "cmdline_parse_portlist.h"
>  
> @@ -27,7 +29,7 @@ static void
>  parse_set_list(cmdline_portlist_t *pl, size_t low, size_t high)
>  {
>  	do {
> -		pl->map |= (1 << low++);
> +		pl->map |= RTE_BIT32(low++);
>  	} while (low <= high);
>  }
>
While this is correct, the use of "++" in a call to a macro sets off some
alarm bells for me!
Can we put the "++" in the while instead, as "++low"?

/Bruce

Re: [PATCH 06/10] cmdline: fix highest bit port list parsing

[David Marchand]

On Fri, Jun 20, 2025 at 11:22 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Jun 19, 2025 at 09:10:32AM +0200, David Marchand wrote:
> > pl->map is a uint32_t.
> >
> > Caught by UBSan:
> >
> > ../lib/cmdline/cmdline_parse_portlist.c:27:17: runtime error:
> >       left shift of 1 by 31 places cannot be represented in type 'int'
> > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> >       ../lib/cmdline/cmdline_parse_portlist.c:27:17 in
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  lib/cmdline/cmdline_parse_portlist.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
> > index 0c07cc02b5..3ef427d32a 100644
> > --- a/lib/cmdline/cmdline_parse_portlist.c
> > +++ b/lib/cmdline/cmdline_parse_portlist.c
> > @@ -11,7 +11,9 @@
> >  #include <errno.h>
> >
> >  #include <eal_export.h>
> > +#include <rte_bitops.h>
> >  #include <rte_string_fns.h>
> > +
> >  #include "cmdline_parse.h"
> >  #include "cmdline_parse_portlist.h"
> >
> > @@ -27,7 +29,7 @@ static void
> >  parse_set_list(cmdline_portlist_t *pl, size_t low, size_t high)
> >  {
> >       do {
> > -             pl->map |= (1 << low++);
> > +             pl->map |= RTE_BIT32(low++);
> >       } while (low <= high);
> >  }
> >
> While this is correct, the use of "++" in a call to a macro sets off some
> alarm bells for me!
> Can we put the "++" in the while instead, as "++low"?

It would be safer yes.


-- 
David Marchand

[PATCH 07/10] tailq: fix cast macro for null pointer

[David Marchand]

Doing arithmetics with the NULL pointer is undefined.

Caught by UBSan:

../app/test/test_tailq.c:111:9: runtime error:
	member access within null pointer of type 'struct rte_tailq_head'

Fixes: f6b4f6c9c123 ("tailq: use a single cast macro")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/include/rte_tailq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/eal/include/rte_tailq.h b/lib/eal/include/rte_tailq.h
index 89f7ef2134..c23df77d96 100644
--- a/lib/eal/include/rte_tailq.h
+++ b/lib/eal/include/rte_tailq.h
@@ -54,7 +54,7 @@ struct rte_tailq_elem {
  * Return the first tailq entry cast to the right struct.
  */
 #define RTE_TAILQ_CAST(tailq_entry, struct_name) \
-	(struct struct_name *)&(tailq_entry)->tailq_head
+	(tailq_entry == NULL ? NULL : (struct struct_name *)&(tailq_entry)->tailq_head)
 
 /**
  * Utility macro to make looking up a tailqueue for a particular struct easier.
-- 
2.49.0

Re: [PATCH 07/10] tailq: fix cast macro for null pointer

[Bruce Richardson]

On Thu, Jun 19, 2025 at 09:10:33AM +0200, David Marchand wrote:
> Doing arithmetics with the NULL pointer is undefined.
> 
> Caught by UBSan:
> 
> ../app/test/test_tailq.c:111:9: runtime error:
> 	member access within null pointer of type 'struct rte_tailq_head'
> 
> Fixes: f6b4f6c9c123 ("tailq: use a single cast macro")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH 08/10] hash: fix unaligned access in predictable RSS

[David Marchand]

Caught by UBSan:

../lib/hash/rte_thash.c:421:8: runtime error: load of misaligned address
	0x0001816c2da3 for type 'uint32_t' (aka 'unsigned int'),
	which requires 4 byte alignment

Fixes: 28ebff11c2dc ("hash: add predictable RSS")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/hash/rte_thash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
index 6c662bf14f..6d4dbea6d7 100644
--- a/lib/hash/rte_thash.c
+++ b/lib/hash/rte_thash.c
@@ -415,10 +415,10 @@ generate_subkey(struct rte_thash_ctx *ctx, struct thash_lfsr *lfsr,
 static inline uint32_t
 get_subvalue(struct rte_thash_ctx *ctx, uint32_t offset)
 {
-	uint32_t *tmp, val;
+	uint32_t tmp, val;
 
-	tmp = (uint32_t *)(&ctx->hash_key[offset >> 3]);
-	val = rte_be_to_cpu_32(*tmp);
+	memcpy(&tmp, &ctx->hash_key[offset >> 3], sizeof(tmp));
+	val = rte_be_to_cpu_32(tmp);
 	val >>= (TOEPLITZ_HASH_LEN - ((offset & (CHAR_BIT - 1)) +
 		ctx->reta_sz_log));
 
-- 
2.49.0

[PATCH 09/10] stack: fix unaligned accesses on 128-bit

[David Marchand]

Caught by UBSan:

../lib/eal/x86/include/rte_atomic_64.h:206:21: runtime error:
	member access within misaligned address 0x7ffd9c67f228 for
	type 'const rte_int128_t', which requires 16 byte alignment
	0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
	../lib/eal/x86/include/rte_atomic_64.h:206:21: runtime error:
	member access within misaligned address 0x7ffd9c67f228 for type
	'const union rte_int128_t::(anonymous at
	../lib/eal/include/generic/rte_atomic.h:1102:2)', which requires
	16 byte alignment
0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
	../lib/eal/x86/include/rte_atomic_64.h:206:16: runtime error:
	load of misaligned address 0x7ffd9c67f228 for type
	'const uint64_t' (aka 'const unsigned long'), which requires
	16 byte alignment
0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in

Fixes: 3340202f5954 ("stack: add lock-free implementation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/stack/rte_stack_lf_c11.h     | 8 ++++----
 lib/stack/rte_stack_lf_generic.h | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/stack/rte_stack_lf_c11.h b/lib/stack/rte_stack_lf_c11.h
index b97e02d6a1..f674731235 100644
--- a/lib/stack/rte_stack_lf_c11.h
+++ b/lib/stack/rte_stack_lf_c11.h
@@ -63,13 +63,13 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
 			  struct rte_stack_lf_elem *last,
 			  unsigned int num)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success;
 
 	old_head = list->head;
 
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 
 		/* Swing the top pointer to the first element in the list and
 		 * make the last element point to the old top.
@@ -102,7 +102,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 			 void **obj_table,
 			 struct rte_stack_lf_elem **last)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	uint64_t len;
 	int success = 0;
 
@@ -129,7 +129,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 
 	/* Pop num elements */
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 		struct rte_stack_lf_elem *tmp;
 		unsigned int i;
 
diff --git a/lib/stack/rte_stack_lf_generic.h b/lib/stack/rte_stack_lf_generic.h
index cc69e4d168..32f56dffdd 100644
--- a/lib/stack/rte_stack_lf_generic.h
+++ b/lib/stack/rte_stack_lf_generic.h
@@ -36,13 +36,13 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
 			  struct rte_stack_lf_elem *last,
 			  unsigned int num)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success;
 
 	old_head = list->head;
 
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 
 		/* An acquire fence (or stronger) is needed for weak memory
 		 * models to establish a synchronized-with relationship between
@@ -77,7 +77,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 			 void **obj_table,
 			 struct rte_stack_lf_elem **last)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success = 0;
 
 	/* Reserve num elements, if available */
@@ -99,7 +99,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 
 	/* Pop num elements */
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 		struct rte_stack_lf_elem *tmp;
 		unsigned int i;
 
-- 
2.49.0

[PATCH 10/10] build: support Undefined Behavior Sanitizer

[David Marchand]

Enable UBSan in GHA.
There are still a lot of issues so only run unit tests for a "mini"
target.

Building with debugoptimized forces -O2 and consumes too much memory
with UBSan, prefer plain build (iow -O0) even though this hides a number
of build issues.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .ci/linux-build.sh          | 27 +++++++++++++++++++++++++--
 .github/workflows/build.yml |  5 +++++
 app/test/suites/meson.build |  3 +--
 config/meson.build          | 18 +++++++++++++++++-
 devtools/words-case.txt     |  1 +
 5 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index e9272d3931..905c11b1bf 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -44,6 +44,13 @@ catch_coredump() {
     return 1
 }
 
+catch_ubsan() {
+    [ "$UBSAN" = "true" ] || return 0
+    grep -q UndefinedBehaviorSanitizer $2 2>/dev/null || return 0
+    grep -E "($1|UndefinedBehaviorSanitizer)" $2
+    return 1
+}
+
 check_traces() {
     which babeltrace >/dev/null || return 0
     for file in $(sudo find $HOME -name metadata); do
@@ -100,7 +107,6 @@ fi
 
 OPTS="$OPTS -Dplatform=generic"
 OPTS="$OPTS -Ddefault_library=$DEF_LIB"
-OPTS="$OPTS -Dbuildtype=$buildtype"
 if [ "$STDATOMIC" = "true" ]; then
 	OPTS="$OPTS -Denable_stdatomic=true"
 else
@@ -117,13 +123,28 @@ else
 fi
 OPTS="$OPTS -Dlibdir=lib"
 
+buildtype=debugoptimized
+sanitizer=
 if [ "$ASAN" = "true" ]; then
-    OPTS="$OPTS -Db_sanitize=address"
+    sanitizer=${sanitizer:+$sanitizer,}address
+fi
+
+if [ "$UBSAN" = "true" ]; then
+    sanitizer=${sanitizer:+$sanitizer,}undefined
+    if [ "$RUN_TESTS" = "true" ]; then
+        # UBSan takes too much memory with -O2
+        buildtype=plain
+    fi
+fi
+
+if [ -n "$sanitizer" ]; then
+    OPTS="$OPTS -Db_sanitize=$sanitizer"
     if [ "${CC%%clang}" != "$CC" ]; then
         OPTS="$OPTS -Db_lundef=false"
     fi
 fi
 
+OPTS="$OPTS -Dbuildtype=$buildtype"
 OPTS="$OPTS -Dwerror=true"
 
 if [ -d build ]; then
@@ -141,6 +162,7 @@ if [ -z "$cross_file" ]; then
     configure_coredump
     devtools/test-null.sh || failed="true"
     catch_coredump
+    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
     check_traces
     [ "$failed" != "true" ]
 fi
@@ -182,6 +204,7 @@ if [ "$RUN_TESTS" = "true" ]; then
     configure_coredump
     sudo meson test -C build --suite fast-tests -t 3 || failed="true"
     catch_coredump
+    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
     check_traces
     [ "$failed" != "true" ]
 fi
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 6c4bc664d0..4353bb97d8 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -54,6 +54,7 @@ jobs:
       RISCV64: ${{ matrix.config.cross == 'riscv64' }}
       RUN_TESTS: ${{ contains(matrix.config.checks, 'tests') }}
       STDATOMIC: ${{ contains(matrix.config.checks, 'stdatomic') }}
+      UBSAN: ${{ contains(matrix.config.checks, 'ubsan') }}
 
     strategy:
       fail-fast: false
@@ -62,6 +63,10 @@ jobs:
           - os: ubuntu-22.04
             compiler: gcc
             mini: mini
+          - os: ubuntu-22.04
+            compiler: clang
+            mini: mini
+            checks: tests+ubsan
           - os: ubuntu-22.04
             compiler: gcc
             checks: stdatomic
diff --git a/app/test/suites/meson.build b/app/test/suites/meson.build
index e482373330..0cba63ee12 100644
--- a/app/test/suites/meson.build
+++ b/app/test/suites/meson.build
@@ -72,8 +72,7 @@ foreach suite:test_suites
             elif not has_hugepage
                 continue  #skip this tests
             endif
-            if not asan and (get_option('b_sanitize') == 'address'
-                    or get_option('b_sanitize') == 'address,undefined')
+            if not asan and get_option('b_sanitize').contains('address')
                 continue  # skip this test
             endif
 
diff --git a/config/meson.build b/config/meson.build
index f31fef216c..b8a8511a7f 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -499,7 +499,7 @@ if get_option('b_lto')
     endif
 endif
 
-if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address,undefined'
+if get_option('b_sanitize').contains('address')
     if is_windows
         error('ASan is not supported on windows')
     endif
@@ -519,6 +519,22 @@ if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address
     endif
 endif
 
+if get_option('b_sanitize').contains('undefined')
+    if is_windows
+        error('UBSan is not supported on windows')
+    endif
+
+    if cc.get_id() == 'gcc'
+        ubsan_dep = cc.find_library('ubsan', required: true)
+        if (not cc.links('int main(int argc, char *argv[]) { return 0; }',
+                dependencies: ubsan_dep))
+            error('broken dependency, "libubsan"')
+        endif
+        add_project_link_arguments('-lubsan', language: 'c')
+        dpdk_extra_ldflags += '-lubsan'
+    endif
+endif
+
 if get_option('default_library') == 'both'
     error( '''
  Unsupported value "both" for "default_library" option.
diff --git a/devtools/words-case.txt b/devtools/words-case.txt
index bc35c160ba..d315fde8fe 100644
--- a/devtools/words-case.txt
+++ b/devtools/words-case.txt
@@ -105,6 +105,7 @@ TSO
 TTL
 Tx
 uAPI
+UBSan
 UDM
 UDP
 ULP
-- 
2.49.0

Re: [PATCH 10/10] build: support Undefined Behavior Sanitizer

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> Enable UBSan in GHA.
> There are still a lot of issues so only run unit tests for a "mini"
> target.
>
> Building with debugoptimized forces -O2 and consumes too much memory
> with UBSan, prefer plain build (iow -O0) even though this hides a number
> of build issues.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

>  .ci/linux-build.sh          | 27 +++++++++++++++++++++++++--
>  .github/workflows/build.yml |  5 +++++
>  app/test/suites/meson.build |  3 +--
>  config/meson.build          | 18 +++++++++++++++++-
>  devtools/words-case.txt     |  1 +
>  5 files changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index e9272d3931..905c11b1bf 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -44,6 +44,13 @@ catch_coredump() {
>      return 1
>  }
>  
> +catch_ubsan() {
> +    [ "$UBSAN" = "true" ] || return 0
> +    grep -q UndefinedBehaviorSanitizer $2 2>/dev/null || return 0
> +    grep -E "($1|UndefinedBehaviorSanitizer)" $2
> +    return 1
> +}
> +
>  check_traces() {
>      which babeltrace >/dev/null || return 0
>      for file in $(sudo find $HOME -name metadata); do
> @@ -100,7 +107,6 @@ fi
>  
>  OPTS="$OPTS -Dplatform=generic"
>  OPTS="$OPTS -Ddefault_library=$DEF_LIB"
> -OPTS="$OPTS -Dbuildtype=$buildtype"
>  if [ "$STDATOMIC" = "true" ]; then
>  	OPTS="$OPTS -Denable_stdatomic=true"
>  else
> @@ -117,13 +123,28 @@ else
>  fi
>  OPTS="$OPTS -Dlibdir=lib"
>  
> +buildtype=debugoptimized
> +sanitizer=
>  if [ "$ASAN" = "true" ]; then
> -    OPTS="$OPTS -Db_sanitize=address"
> +    sanitizer=${sanitizer:+$sanitizer,}address
> +fi
> +
> +if [ "$UBSAN" = "true" ]; then
> +    sanitizer=${sanitizer:+$sanitizer,}undefined
> +    if [ "$RUN_TESTS" = "true" ]; then
> +        # UBSan takes too much memory with -O2
> +        buildtype=plain
> +    fi
> +fi
> +
> +if [ -n "$sanitizer" ]; then
> +    OPTS="$OPTS -Db_sanitize=$sanitizer"
>      if [ "${CC%%clang}" != "$CC" ]; then
>          OPTS="$OPTS -Db_lundef=false"
>      fi
>  fi
>  
> +OPTS="$OPTS -Dbuildtype=$buildtype"
>  OPTS="$OPTS -Dwerror=true"
>  
>  if [ -d build ]; then
> @@ -141,6 +162,7 @@ if [ -z "$cross_file" ]; then
>      configure_coredump
>      devtools/test-null.sh || failed="true"
>      catch_coredump
> +    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
>      check_traces
>      [ "$failed" != "true" ]
>  fi
> @@ -182,6 +204,7 @@ if [ "$RUN_TESTS" = "true" ]; then
>      configure_coredump
>      sudo meson test -C build --suite fast-tests -t 3 || failed="true"
>      catch_coredump
> +    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
>      check_traces
>      [ "$failed" != "true" ]
>  fi
> diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
> index 6c4bc664d0..4353bb97d8 100644
> --- a/.github/workflows/build.yml
> +++ b/.github/workflows/build.yml
> @@ -54,6 +54,7 @@ jobs:
>        RISCV64: ${{ matrix.config.cross == 'riscv64' }}
>        RUN_TESTS: ${{ contains(matrix.config.checks, 'tests') }}
>        STDATOMIC: ${{ contains(matrix.config.checks, 'stdatomic') }}
> +      UBSAN: ${{ contains(matrix.config.checks, 'ubsan') }}
>  
>      strategy:
>        fail-fast: false
> @@ -62,6 +63,10 @@ jobs:
>            - os: ubuntu-22.04
>              compiler: gcc
>              mini: mini
> +          - os: ubuntu-22.04
> +            compiler: clang
> +            mini: mini
> +            checks: tests+ubsan
>            - os: ubuntu-22.04
>              compiler: gcc
>              checks: stdatomic
> diff --git a/app/test/suites/meson.build b/app/test/suites/meson.build
> index e482373330..0cba63ee12 100644
> --- a/app/test/suites/meson.build
> +++ b/app/test/suites/meson.build
> @@ -72,8 +72,7 @@ foreach suite:test_suites
>              elif not has_hugepage
>                  continue  #skip this tests
>              endif
> -            if not asan and (get_option('b_sanitize') == 'address'
> -                    or get_option('b_sanitize') == 'address,undefined')
> +            if not asan and get_option('b_sanitize').contains('address')
>                  continue  # skip this test
>              endif
>  
> diff --git a/config/meson.build b/config/meson.build
> index f31fef216c..b8a8511a7f 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -499,7 +499,7 @@ if get_option('b_lto')
>      endif
>  endif
>  
> -if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address,undefined'
> +if get_option('b_sanitize').contains('address')
>      if is_windows
>          error('ASan is not supported on windows')
>      endif
> @@ -519,6 +519,22 @@ if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address
>      endif
>  endif
>  
> +if get_option('b_sanitize').contains('undefined')
> +    if is_windows
> +        error('UBSan is not supported on windows')
> +    endif
> +
> +    if cc.get_id() == 'gcc'
> +        ubsan_dep = cc.find_library('ubsan', required: true)
> +        if (not cc.links('int main(int argc, char *argv[]) { return 0; }',
> +                dependencies: ubsan_dep))
> +            error('broken dependency, "libubsan"')
> +        endif
> +        add_project_link_arguments('-lubsan', language: 'c')
> +        dpdk_extra_ldflags += '-lubsan'
> +    endif
> +endif
> +
>  if get_option('default_library') == 'both'
>      error( '''
>   Unsupported value "both" for "default_library" option.
> diff --git a/devtools/words-case.txt b/devtools/words-case.txt
> index bc35c160ba..d315fde8fe 100644
> --- a/devtools/words-case.txt
> +++ b/devtools/words-case.txt
> @@ -105,6 +105,7 @@ TSO
>  TTL
>  Tx
>  uAPI
> +UBSan
>  UDM
>  UDP
>  ULP

[PATCH v2 00/10] Run with UBSan in GHA

[David Marchand]

This series fixes a number of issues reported by UBSan and adds a simple
job in GHA to avoid introducing undefined behavior in the core
components.
There is way more work/fixes to do if we want to run with a full set of
components, but baby steps first.


-- 
David Marchand

Changes since v1:
- small updates on patches 5 and 6,

David Marchand (10):
  ci: save ccache on failure
  test/telemetry: fix test calling all commands
  test/mempool: fix test without stack driver
  eal: fix plugin dir walk
  cmdline: fix port list parsing
  cmdline: fix highest bit port list parsing
  tailq: fix cast macro for null pointer
  hash: fix unaligned access in predictable RSS
  stack: fix unaligned accesses on 128-bit
  build: support Undefined Behavior Sanitizer

 .ci/linux-build.sh                   | 27 +++++++++++++++++++++--
 .github/workflows/build.yml          | 11 ++++++++++
 app/test/suites/meson.build          |  3 +--
 app/test/suites/test_telemetry.sh    |  2 +-
 app/test/test_mempool.c              | 32 +++++++++++++++++-----------
 config/meson.build                   | 18 +++++++++++++++-
 devtools/words-case.txt              |  1 +
 lib/cmdline/cmdline_parse_portlist.c | 15 +++++++------
 lib/eal/common/eal_common_options.c  | 15 +++++++++----
 lib/eal/include/rte_tailq.h          |  2 +-
 lib/hash/rte_thash.c                 |  6 +++---
 lib/stack/rte_stack_lf_c11.h         |  8 +++----
 lib/stack/rte_stack_lf_generic.h     |  8 +++----
 13 files changed, 107 insertions(+), 41 deletions(-)

-- 
2.49.0

[PATCH v2 01/10] ci: save ccache on failure

[David Marchand]

When troubleshooting unit test failures and repeating jobs in GHA,
the absence of ccache makes the whole process way slower.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .github/workflows/build.yml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index e5f17ef6ac..6c4bc664d0 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -164,6 +164,12 @@ jobs:
         chmod o-w $HOME
     - name: Build and test
       run: .ci/linux-build.sh
+    - name: Save ccache on failure
+      if: failure()
+      uses: actions/cache/save@v4
+      with:
+        path: ~/.ccache
+        key: ${{ steps.get_ref_keys.outputs.ccache }}-${{ github.ref }}
     - name: Upload logs on failure
       if: failure()
       uses: actions/upload-artifact@v4
-- 
2.49.0

[PATCH v2 02/10] test/telemetry: fix test calling all commands

[David Marchand]

This test was doing nothing as it could not find the telemetry client
script following the test suite rework.

Caught while looking at UNH unit test logs:

/root/workspace/Generic-Unit-Test-DPDK/dpdk/app/test/suites/test_telemetry.sh:
18: /root/workspace/Generic-Unit-Test-DPDK/dpdk/app/usertools/dpdk-telemetry.py:
not found

Fixes: 9da71dc4f96e ("test: add test case for scripted telemetry commands")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/suites/test_telemetry.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/suites/test_telemetry.sh b/app/test/suites/test_telemetry.sh
index ca6abe266e..20806b43e4 100755
--- a/app/test/suites/test_telemetry.sh
+++ b/app/test/suites/test_telemetry.sh
@@ -7,7 +7,7 @@ which jq || {
     exit 77
 }
 
-rootdir=$(readlink -f $(dirname $(readlink -f $0))/../..)
+rootdir=$(readlink -f $(dirname $(readlink -f $0))/../../..)
 tmpoutput=$(mktemp -t dpdk.test_telemetry.XXXXXX)
 trap "cat $tmpoutput; rm -f $tmpoutput" EXIT
 
-- 
2.49.0

RE: [PATCH v2 02/10] test/telemetry: fix test calling all commands

[Marat Khalili]

Reviewed-by: Marat Khalili <marat.khalili@huawei.com>

Just an idea, in case you have another iteration: could we maybe add a small check that $telemetry_script actually exists and executable to avoid similar situations in the future? Can be as simple as:

test -f "$telemetry_script"
test -x "$telemetry_script"

Due to -e in the first line it should fail the script of any of these tests fail.

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday 23 June 2025 14:53
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Bruce Richardson <bruce.richardson@intel.com>
> Subject: [PATCH v2 02/10] test/telemetry: fix test calling all commands
> 
> This test was doing nothing as it could not find the telemetry client
> script following the test suite rework.
> 
> Caught while looking at UNH unit test logs:
> 
> /root/workspace/Generic-Unit-Test-
> DPDK/dpdk/app/test/suites/test_telemetry.sh:
> 18: /root/workspace/Generic-Unit-Test-DPDK/dpdk/app/usertools/dpdk-
> telemetry.py:
> not found
> 
> Fixes: 9da71dc4f96e ("test: add test case for scripted telemetry commands")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  app/test/suites/test_telemetry.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test/suites/test_telemetry.sh
> b/app/test/suites/test_telemetry.sh
> index ca6abe266e..20806b43e4 100755
> --- a/app/test/suites/test_telemetry.sh
> +++ b/app/test/suites/test_telemetry.sh
> @@ -7,7 +7,7 @@ which jq || {
>      exit 77
>  }
> 
> -rootdir=$(readlink -f $(dirname $(readlink -f $0))/../..)
> +rootdir=$(readlink -f $(dirname $(readlink -f $0))/../../..)
>  tmpoutput=$(mktemp -t dpdk.test_telemetry.XXXXXX)
>  trap "cat $tmpoutput; rm -f $tmpoutput" EXIT
> 
> --
> 2.49.0

Re: [PATCH v2 02/10] test/telemetry: fix test calling all commands

[David Marchand]

On Tue, Jun 24, 2025 at 6:00 PM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> Reviewed-by: Marat Khalili <marat.khalili@huawei.com>
>
> Just an idea, in case you have another iteration: could we maybe add a small check that $telemetry_script actually exists and executable to avoid similar situations in the future? Can be as simple as:
>
> test -f "$telemetry_script"
> test -x "$telemetry_script"
>
> Due to -e in the first line it should fail the script of any of these tests fail.

The problem lies in the use of subshell and pipes and that a failure
is not propagated.
Adding a test only the the telemetry script would not catch other
errors (like for example, if the jq command starts to spew errors).

The most elegant would be to use errtrace and pipefail options, but
the errtrace is a bashism (iow not POSIX), and pipefail is POSIX only
since 2022 and many shell (like dash in Ubuntu 22.04/24.04) don't
implement it.

We could try something like:

diff --git a/app/test/suites/test_telemetry.sh
b/app/test/suites/test_telemetry.sh
index ca6abe266e..a81b4add90 100755
--- a/app/test/suites/test_telemetry.sh
+++ b/app/test/suites/test_telemetry.sh
@@ -15,7 +15,7 @@ call_all_telemetry() {
     telemetry_script=$rootdir/usertools/dpdk-telemetry.py
     echo >$tmpoutput
     echo "Telemetry commands log:" >>$tmpoutput
-    for cmd in $(echo / | $telemetry_script | jq -r '.["/"][]')
+    echo / | $telemetry_script | jq -r '.["/"][]' | while read cmd
     do
         for input in $cmd $cmd,0 $cmd,z
         do
@@ -25,4 +25,5 @@ call_all_telemetry() {
     done
 }

-(sleep 1 && call_all_telemetry && echo quit) | $@
+! set -o | grep -q pipefail || set -o pipefail
+(set -e; ! set -o | grep -q pipefail || set -o pipefail; sleep 1 &&
call_all_telemetry && echo quit) | $@



-- 
David Marchand

RE: [PATCH v2 02/10] test/telemetry: fix test calling all commands

[Marat Khalili]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday 26 June 2025 09:33
> 
> The problem lies in the use of subshell and pipes and that a failure
> is not propagated.
> Adding a test only the the telemetry script would not catch other
> errors (like for example, if the jq command starts to spew errors).

Yes, I agree.

> The most elegant would be to use errtrace and pipefail options, but
> the errtrace is a bashism (iow not POSIX), and pipefail is POSIX only
> since 2022 and many shell (like dash in Ubuntu 22.04/24.04) don't
> implement it.

Yes, also true.

> We could try something like:
> 
> diff --git a/app/test/suites/test_telemetry.sh
> b/app/test/suites/test_telemetry.sh
> index ca6abe266e..a81b4add90 100755
> --- a/app/test/suites/test_telemetry.sh
> +++ b/app/test/suites/test_telemetry.sh
> @@ -15,7 +15,7 @@ call_all_telemetry() {
>      telemetry_script=$rootdir/usertools/dpdk-telemetry.py
>      echo >$tmpoutput
>      echo "Telemetry commands log:" >>$tmpoutput
> -    for cmd in $(echo / | $telemetry_script | jq -r '.["/"][]')
> +    echo / | $telemetry_script | jq -r '.["/"][]' | while read cmd
>      do
>          for input in $cmd $cmd,0 $cmd,z
>          do
> @@ -25,4 +25,5 @@ call_all_telemetry() {
>      done
>  }
> 
> -(sleep 1 && call_all_telemetry && echo quit) | $@
> +! set -o | grep -q pipefail || set -o pipefail
> +(set -e; ! set -o | grep -q pipefail || set -o pipefail; sleep 1 &&
> call_all_telemetry && echo quit) | $@

I 100% agree with the idea, but sadly I'm not familiar with shell scripting enough to suggest or review this diff. Is `for cmd in` always equivalent to `while read cmd`? Is CI ever executing it in bash for our attempt to set pipefail to be justified? Is it idiomatic? I hope someone else here can help with this.

Perhaps this should just be re-written in Python. It depends on a Python script anyway.

Re: [PATCH v2 02/10] test/telemetry: fix test calling all commands

[David Marchand]

On Thu, Jun 26, 2025 at 11:53 AM Marat Khalili <marat.khalili@huawei.com> wrote:
> > @@ -15,7 +15,7 @@ call_all_telemetry() {
> >      telemetry_script=$rootdir/usertools/dpdk-telemetry.py
> >      echo >$tmpoutput
> >      echo "Telemetry commands log:" >>$tmpoutput
> > -    for cmd in $(echo / | $telemetry_script | jq -r '.["/"][]')
> > +    echo / | $telemetry_script | jq -r '.["/"][]' | while read cmd
> >      do
> >          for input in $cmd $cmd,0 $cmd,z
> >          do
> > @@ -25,4 +25,5 @@ call_all_telemetry() {
> >      done
> >  }
> >
> > -(sleep 1 && call_all_telemetry && echo quit) | $@
> > +! set -o | grep -q pipefail || set -o pipefail
> > +(set -e; ! set -o | grep -q pipefail || set -o pipefail; sleep 1 &&
> > call_all_telemetry && echo quit) | $@
>
> I 100% agree with the idea, but sadly I'm not familiar with shell scripting enough to suggest or review this diff. Is `for cmd in` always equivalent to `while read cmd`? Is CI ever executing it in bash for our attempt to set pipefail to be justified? Is it idiomatic? I hope someone else here can help with this.

- From my experiment, the difference between 'for cmd in $(xxx)' and
'xxx | while read cmd' is that an error is not propagated in the
former case.

I suppose it has to do with the for loop, as described in POSIX:
"""
Exit Status

If there is at least one item in the list of items, the exit status of
a for command shall be the exit status of the last compound-list
executed. If there are no items, the exit status shall be zero.
"""

On error of the command, there is no item in the list of the for loop,
so the loop is overall evaluated as a success.


- As far as the CI is concened, the unit tests are run on various
distributions, including Fedora at UNH.
Since the default shell for Fedora is bash, then an error would be caught there.


>
> Perhaps this should just be re-written in Python. It depends on a Python script anyway.

Maybe.
Though if we go that way, we would need some refactoring of the
telemetry client script, defining some python class etc...


-- 
David Marchand

RE: [PATCH v2 02/10] test/telemetry: fix test calling all commands

[Marat Khalili]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday 3 July 2025 15:10
> 
> On Thu, Jun 26, 2025 at 11:53 AM Marat Khalili <marat.khalili@huawei.com>
> wrote:
> > > @@ -15,7 +15,7 @@ call_all_telemetry() {
> > >      telemetry_script=$rootdir/usertools/dpdk-telemetry.py
> > >      echo >$tmpoutput
> > >      echo "Telemetry commands log:" >>$tmpoutput
> > > -    for cmd in $(echo / | $telemetry_script | jq -r '.["/"][]')
> > > +    echo / | $telemetry_script | jq -r '.["/"][]' | while read cmd
> > >      do
> > >          for input in $cmd $cmd,0 $cmd,z
> > >          do
> > > @@ -25,4 +25,5 @@ call_all_telemetry() {
> > >      done
> > >  }
> > >
> > > -(sleep 1 && call_all_telemetry && echo quit) | $@
> > > +! set -o | grep -q pipefail || set -o pipefail
> > > +(set -e; ! set -o | grep -q pipefail || set -o pipefail; sleep 1 &&
> > > call_all_telemetry && echo quit) | $@
> >
> > I 100% agree with the idea, but sadly I'm not familiar with shell scripting
> enough to suggest or review this diff. Is `for cmd in` always equivalent to
> `while read cmd`? Is CI ever executing it in bash for our attempt to set
> pipefail to be justified? Is it idiomatic? I hope someone else here can help
> with this.
> 
> - From my experiment, the difference between 'for cmd in $(xxx)' and
> 'xxx | while read cmd' is that an error is not propagated in the
> former case.
> 
> I suppose it has to do with the for loop, as described in POSIX:
> """
> Exit Status
> 
> If there is at least one item in the list of items, the exit status of
> a for command shall be the exit status of the last compound-list
> executed. If there are no items, the exit status shall be zero.
> """
> 
> On error of the command, there is no item in the list of the for loop,
> so the loop is overall evaluated as a success.

BTW what would be the behaviour of the new code for the empty input?

> - As far as the CI is concened, the unit tests are run on various
> distributions, including Fedora at UNH.
> Since the default shell for Fedora is bash, then an error would be caught
> there.
> 
> 
> >
> > Perhaps this should just be re-written in Python. It depends on a Python
> script anyway.
> 
> Maybe.
> Though if we go that way, we would need some refactoring of the
> telemetry client script, defining some python class etc...

I guess it's a balance between effort and gain, and we are not making it worse (it will still need to pass CI where we can double check the behaviour)... I now support this change, thanks for detailed explanations and research.

Acked-by: Marat Khalili <marat.khalili@huawei.com>

[PATCH v2 03/10] test/mempool: fix test without stack driver

[David Marchand]

In a minimal build, the mempool/stack driver is disabled.
Separate the code specific to this external driver and rename unrelated
variables.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 app/test/test_mempool.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 61385e096e..63356998fd 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -847,9 +847,11 @@ test_mempool(void)
 	size_t alignment = 0;
 	struct rte_mempool *mp_cache = NULL;
 	struct rte_mempool *mp_nocache = NULL;
-	struct rte_mempool *mp_stack_anon = NULL;
-	struct rte_mempool *mp_stack_mempool_iter = NULL;
+	struct rte_mempool *mp_anon = NULL;
+	struct rte_mempool *mp_mempool_iter = NULL;
+#ifdef RTE_MEMPOOL_STACK
 	struct rte_mempool *mp_stack = NULL;
+#endif
 	struct rte_mempool *default_pool = NULL;
 	struct rte_mempool *mp_alignment = NULL;
 	struct mp_data cb_arg = {
@@ -884,28 +886,28 @@ test_mempool(void)
 	}
 
 	/* create an empty mempool  */
-	mp_stack_anon = rte_mempool_create_empty("test_stack_anon",
+	mp_anon = rte_mempool_create_empty("test_anon",
 		MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE,
 		RTE_MEMPOOL_CACHE_MAX_SIZE, 0,
 		SOCKET_ID_ANY, 0);
 
-	if (mp_stack_anon == NULL)
+	if (mp_anon == NULL)
 		GOTO_ERR(ret, err);
 
 	/* populate an empty mempool */
-	ret = rte_mempool_populate_anon(mp_stack_anon);
+	ret = rte_mempool_populate_anon(mp_anon);
 	printf("%s ret = %d\n", __func__, ret);
 	if (ret < 0)
 		GOTO_ERR(ret, err);
 
 	/* Try to populate when already populated */
-	ret = rte_mempool_populate_anon(mp_stack_anon);
+	ret = rte_mempool_populate_anon(mp_anon);
 	if (ret != 0)
 		GOTO_ERR(ret, err);
 
 	/* create a mempool  */
-	mp_stack_mempool_iter = rte_mempool_create("test_iter_obj",
+	mp_mempool_iter = rte_mempool_create("test_iter_obj",
 		MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE,
 		RTE_MEMPOOL_CACHE_MAX_SIZE, 0,
@@ -913,20 +915,21 @@ test_mempool(void)
 		my_obj_init, NULL,
 		SOCKET_ID_ANY, 0);
 
-	if (mp_stack_mempool_iter == NULL)
+	if (mp_mempool_iter == NULL)
 		GOTO_ERR(ret, err);
 
 	/* test to initialize mempool objects and memory */
-	nb_objs = rte_mempool_obj_iter(mp_stack_mempool_iter, my_obj_init,
+	nb_objs = rte_mempool_obj_iter(mp_mempool_iter, my_obj_init,
 			NULL);
 	if (nb_objs == 0)
 		GOTO_ERR(ret, err);
 
-	nb_mem_chunks = rte_mempool_mem_iter(mp_stack_mempool_iter,
+	nb_mem_chunks = rte_mempool_mem_iter(mp_mempool_iter,
 			test_mp_mem_init, &cb_arg);
 	if (nb_mem_chunks == 0 || cb_arg.ret < 0)
 		GOTO_ERR(ret, err);
 
+#ifdef RTE_MEMPOOL_STACK
 	/* create a mempool with an external handler */
 	mp_stack = rte_mempool_create_empty("test_stack",
 		MEMPOOL_SIZE,
@@ -947,6 +950,7 @@ test_mempool(void)
 		GOTO_ERR(ret, err);
 	}
 	rte_mempool_obj_iter(mp_stack, my_obj_init, NULL);
+#endif /* RTE_MEMPOOL_STACK */
 
 	/* Create a mempool based on Default handler */
 	printf("Testing %s mempool handler\n", default_pool_ops);
@@ -1077,9 +1081,11 @@ test_mempool(void)
 	if (test_mempool_same_name_twice_creation() < 0)
 		GOTO_ERR(ret, err);
 
+#ifdef RTE_MEMPOOL_STACK
 	/* test the stack handler */
 	if (test_mempool_basic(mp_stack, 1) < 0)
 		GOTO_ERR(ret, err);
+#endif
 
 	if (test_mempool_basic(default_pool, 1) < 0)
 		GOTO_ERR(ret, err);
@@ -1105,9 +1111,11 @@ test_mempool(void)
 err:
 	rte_mempool_free(mp_nocache);
 	rte_mempool_free(mp_cache);
-	rte_mempool_free(mp_stack_anon);
-	rte_mempool_free(mp_stack_mempool_iter);
+	rte_mempool_free(mp_anon);
+	rte_mempool_free(mp_mempool_iter);
+#ifdef RTE_MEMPOOL_STACK
 	rte_mempool_free(mp_stack);
+#endif
 	rte_mempool_free(default_pool);
 	rte_mempool_free(mp_alignment);
 
-- 
2.49.0

RE: [PATCH v2 03/10] test/mempool: fix test without stack driver

[Marat Khalili]

Looks good.

Reviewed-by: Marat Khalili <marat.khalili@huawei.com>

[PATCH v2 04/10] eal: fix plugin dir walk

[David Marchand]

For '.' and '..' directories (or any short file name),
a out of bound issue occurs.

Caught by UBSan:

EAL: Detected shared linkage of DPDK
../lib/eal/common/eal_common_options.c:420:15: runtime error: index -2
	out of bounds for type 'char[256]'
    #0 0x7f867eedf206 in eal_plugindir_init
	eal_common_options.c
    #1 0x7f867eede58a in eal_plugins_init
	(build/lib/librte_eal.so.25+0xde58a)
	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
    #2 0x7f867efb8587 in rte_eal_init
	(build/lib/librte_eal.so.25+0x1b8587)
	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
    #3 0x55b62360861e in main
	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x9e061e)
	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
    #4 0x7f8667429d8f in __libc_start_call_main
	csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #5 0x7f8667429e3f in __libc_start_main
	csu/../csu/libc-start.c:392:3
    #6 0x55b622d9d444 in _start
	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x175444)
	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/common/eal_common_options.c:420:15 in
	../lib/eal/common/eal_common_options.c:421:15:
	runtime error: index 18446744073709551609 out of bounds
	for type 'char[256]'

Fixes: c57f6e5c604a ("eal: fix plugin loading")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/common/eal_common_options.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 83b6fc7e89..153f807e4f 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -399,6 +399,14 @@ eal_plugins_init(void)
 }
 #else
 
+static bool
+ends_with(const char *str, size_t str_len, const char *tail)
+{
+	size_t tail_len = strlen(tail);
+
+	return str_len >= tail_len && strncmp(&str[str_len - tail_len], tail, tail_len) == 0;
+}
+
 static int
 eal_plugindir_init(const char *path)
 {
@@ -417,13 +425,12 @@ eal_plugindir_init(const char *path)
 	}
 
 	while ((dent = readdir(d)) != NULL) {
+		size_t nlen = strnlen(dent->d_name, sizeof(dent->d_name));
 		struct stat sb;
-		int nlen = strnlen(dent->d_name, sizeof(dent->d_name));
 
 		/* check if name ends in .so or .so.ABI_VERSION */
-		if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 &&
-		    strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)],
-			   ".so."ABI_VERSION) != 0)
+		if (!ends_with(dent->d_name, nlen, ".so") &&
+				!ends_with(dent->d_name, nlen, ".so."ABI_VERSION))
 			continue;
 
 		snprintf(sopath, sizeof(sopath), "%s/%s", path, dent->d_name);
-- 
2.49.0

RE: [PATCH v2 04/10] eal: fix plugin dir walk

[Marat Khalili]

Thank you for doing this.

> +static bool
> +ends_with(const char *str, size_t str_len, const char *tail)

I too think we should have a general ends_with, I for one had to code one just this week. However, I do not think it should support non-null-terminated strings.

> +{
> +	size_t tail_len = strlen(tail);
> +
> +	return str_len >= tail_len && strncmp(&str[str_len - tail_len], tail,
> tail_len) == 0;
> +}

Note that when str is not null-terminated and both str_len and tail_len are zeroes &str[str_len - tail_len] will dereference one character after the end before taking a reference to it again, which would be a UB. (Won't happen in your case of course since your tail is always non-empty, but may happen if this function is moved into a general-use library.)

> @@ -417,13 +425,12 @@ eal_plugindir_init(const char *path)
>  	}
> 
>  	while ((dent = readdir(d)) != NULL) {
> +		size_t nlen = strnlen(dent->d_name, sizeof(dent->d_name));
>  		struct stat sb;
> -		int nlen = strnlen(dent->d_name, sizeof(dent->d_name));
> 
>  		/* check if name ends in .so or .so.ABI_VERSION */
> -		if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 &&
> -		    strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)],
> -			   ".so."ABI_VERSION) != 0)
> +		if (!ends_with(dent->d_name, nlen, ".so") &&
> +				!ends_with(dent->d_name, nlen, ".so."ABI_VERSION))
>  			continue;

I do not think we should try to handle the non-null-terminated dent->d_name case here, I'd just delete nlen and everything related to it. To be super-defensive we could add a check that `memchr(dent->d_name, 0, sizeof(dent->d_name)) != NULL`, but I don't think it's needed.

Re: [PATCH v2 04/10] eal: fix plugin dir walk

[David Marchand]

On Wed, Jun 25, 2025 at 10:43 AM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> Thank you for doing this.
>
> > +static bool
> > +ends_with(const char *str, size_t str_len, const char *tail)
>
> I too think we should have a general ends_with, I for one had to code one just this week. However, I do not think it should support non-null-terminated strings.
>
> > +{
> > +     size_t tail_len = strlen(tail);
> > +
> > +     return str_len >= tail_len && strncmp(&str[str_len - tail_len], tail,
> > tail_len) == 0;
> > +}
>
> Note that when str is not null-terminated and both str_len and tail_len are zeroes &str[str_len - tail_len] will dereference one character after the end before taking a reference to it again, which would be a UB. (Won't happen in your case of course since your tail is always non-empty, but may happen if this function is moved into a general-use library.)

As a generic helper, it would be worth to make it more robust.
Though here, as a fix, I would avoid adding a helper so the backport
can be done without adding a new API.


>
> > @@ -417,13 +425,12 @@ eal_plugindir_init(const char *path)
> >       }
> >
> >       while ((dent = readdir(d)) != NULL) {
> > +             size_t nlen = strnlen(dent->d_name, sizeof(dent->d_name));
> >               struct stat sb;
> > -             int nlen = strnlen(dent->d_name, sizeof(dent->d_name));
> >
> >               /* check if name ends in .so or .so.ABI_VERSION */
> > -             if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 &&
> > -                 strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)],
> > -                        ".so."ABI_VERSION) != 0)
> > +             if (!ends_with(dent->d_name, nlen, ".so") &&
> > +                             !ends_with(dent->d_name, nlen, ".so."ABI_VERSION))
> >                       continue;
>
> I do not think we should try to handle the non-null-terminated dent->d_name case here, I'd just delete nlen and everything related to it. To be super-defensive we could add a check that `memchr(dent->d_name, 0, sizeof(dent->d_name)) != NULL`, but I don't think it's needed.
>

Mm, good point.
I did not reevaluate this part of the code, but it is indeed odd
trying to protect against a non null terminated dent->d_name here.

https://pubs.opengroup.org/onlinepubs/007904875/basedefs/dirent.h.html
"""
The character array d_name is of unspecified size, but the number of
bytes preceding the terminating null byte shall not exceed {NAME_MAX}.
"""

I'll rework this local helper so it assumes null terminated strings.


-- 
David Marchand

[PATCH v2 05/10] cmdline: fix port list parsing

[David Marchand]

Doing arithmetics with the NULL pointer is undefined.

Caught by UBSan:

../lib/cmdline/cmdline_parse_portlist.c:40:19: runtime error:
	applying non-zero offset 1 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/cmdline/cmdline_parse_portlist.c:40:19 in

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v1:
- moved some variable as suggested by Bruce,

---
 lib/cmdline/cmdline_parse_portlist.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
index ef6ce223b5..549f6d9671 100644
--- a/lib/cmdline/cmdline_parse_portlist.c
+++ b/lib/cmdline/cmdline_parse_portlist.c
@@ -33,15 +33,12 @@ parse_set_list(cmdline_portlist_t *pl, size_t low, size_t high)
 static int
 parse_ports(cmdline_portlist_t *pl, const char *str)
 {
+	const char *first = str;
 	size_t ps, pe;
-	const char *first, *last;
 	char *end;
 
-	for (first = str, last = first;
-	    first != NULL && last != NULL;
-	    first = last + 1) {
-
-		last = strchr(first, ',');
+	while (first != NULL) {
+		const char *last = strchr(first, ',');
 
 		errno = 0;
 		ps = strtoul(first, &end, 10);
@@ -65,6 +62,7 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
 			return -1;
 
 		parse_set_list(pl, ps, pe);
+		first = (last == NULL ? NULL : last + 1);
 	}
 
 	return 0;
-- 
2.49.0

Re: [PATCH v2 05/10] cmdline: fix port list parsing

[Bruce Richardson]

On Mon, Jun 23, 2025 at 03:52:35PM +0200, David Marchand wrote:
> Doing arithmetics with the NULL pointer is undefined.
> 
> Caught by UBSan:
> 
> ../lib/cmdline/cmdline_parse_portlist.c:40:19: runtime error:
> 	applying non-zero offset 1 to null pointer
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> 	../lib/cmdline/cmdline_parse_portlist.c:40:19 in
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v1:
> - moved some variable as suggested by Bruce,
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

RE: [PATCH v2 05/10] cmdline: fix port list parsing

[Marat Khalili]

Acked-by: Marat Khalili <marat.khalili@huawei.com>

[PATCH v2 06/10] cmdline: fix highest bit port list parsing

[David Marchand]

pl->map is a uint32_t.

Caught by UBSan:

../lib/cmdline/cmdline_parse_portlist.c:27:17: runtime error:
	left shift of 1 by 31 places cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/cmdline/cmdline_parse_portlist.c:27:17 in

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
Changes since v1:
- moved variable increment out of the macro call,

---
 lib/cmdline/cmdline_parse_portlist.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
index 549f6d9671..3efe4143e3 100644
--- a/lib/cmdline/cmdline_parse_portlist.c
+++ b/lib/cmdline/cmdline_parse_portlist.c
@@ -10,7 +10,9 @@
 #include <errno.h>
 
 #include <eal_export.h>
+#include <rte_bitops.h>
 #include <rte_string_fns.h>
+
 #include "cmdline_parse.h"
 #include "cmdline_parse_portlist.h"
 
@@ -26,7 +28,8 @@ static void
 parse_set_list(cmdline_portlist_t *pl, size_t low, size_t high)
 {
 	do {
-		pl->map |= (1 << low++);
+		pl->map |= RTE_BIT32(low);
+		low++;
 	} while (low <= high);
 }
 
-- 
2.49.0

RE: [PATCH v2 06/10] cmdline: fix highest bit port list parsing

[Marat Khalili]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday 23 June 2025 14:53
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Bruce Richardson <bruce.richardson@intel.com>
> Subject: [PATCH v2 06/10] cmdline: fix highest bit port list parsing
> 
> pl->map is a uint32_t.
> 
> Caught by UBSan:
> 
> ../lib/cmdline/cmdline_parse_portlist.c:27:17: runtime error:
> 	left shift of 1 by 31 places cannot be represented in type 'int'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> 	../lib/cmdline/cmdline_parse_portlist.c:27:17 in
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> Changes since v1:
> - moved variable increment out of the macro call,
> 
> ---
>  lib/cmdline/cmdline_parse_portlist.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/cmdline/cmdline_parse_portlist.c
> b/lib/cmdline/cmdline_parse_portlist.c
> index 549f6d9671..3efe4143e3 100644
> --- a/lib/cmdline/cmdline_parse_portlist.c
> +++ b/lib/cmdline/cmdline_parse_portlist.c
> @@ -10,7 +10,9 @@
>  #include <errno.h>
> 
>  #include <eal_export.h>
> +#include <rte_bitops.h>
>  #include <rte_string_fns.h>
> +
>  #include "cmdline_parse.h"
>  #include "cmdline_parse_portlist.h"
> 
> @@ -26,7 +28,8 @@ static void
>  parse_set_list(cmdline_portlist_t *pl, size_t low, size_t high)
>  {
>  	do {
> -		pl->map |= (1 << low++);
> +		pl->map |= RTE_BIT32(low);
> +		low++;
>  	} while (low <= high);
>  }

Reviewed-by: Marat Khalili <marat.khalili@huawei.com>

[PATCH v2 07/10] tailq: fix cast macro for null pointer

[David Marchand]

Doing arithmetics with the NULL pointer is undefined.

Caught by UBSan:

../app/test/test_tailq.c:111:9: runtime error:
	member access within null pointer of type 'struct rte_tailq_head'

Fixes: f6b4f6c9c123 ("tailq: use a single cast macro")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/include/rte_tailq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/eal/include/rte_tailq.h b/lib/eal/include/rte_tailq.h
index 89f7ef2134..c23df77d96 100644
--- a/lib/eal/include/rte_tailq.h
+++ b/lib/eal/include/rte_tailq.h
@@ -54,7 +54,7 @@ struct rte_tailq_elem {
  * Return the first tailq entry cast to the right struct.
  */
 #define RTE_TAILQ_CAST(tailq_entry, struct_name) \
-	(struct struct_name *)&(tailq_entry)->tailq_head
+	(tailq_entry == NULL ? NULL : (struct struct_name *)&(tailq_entry)->tailq_head)
 
 /**
  * Utility macro to make looking up a tailqueue for a particular struct easier.
-- 
2.49.0

RE: [PATCH v2 07/10] tailq: fix cast macro for null pointer

[Marat Khalili]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday 23 June 2025 14:53
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Bruce Richardson <bruce.richardson@intel.com>; Tyler
> Retzlaff <roretzla@linux.microsoft.com>; Neil Horman
> <nhorman@tuxdriver.com>
> Subject: [PATCH v2 07/10] tailq: fix cast macro for null pointer
> 
> Doing arithmetics with the NULL pointer is undefined.
> 
> Caught by UBSan:
> 
> ../app/test/test_tailq.c:111:9: runtime error:
> 	member access within null pointer of type 'struct rte_tailq_head'
> 
> Fixes: f6b4f6c9c123 ("tailq: use a single cast macro")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/eal/include/rte_tailq.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/eal/include/rte_tailq.h b/lib/eal/include/rte_tailq.h
> index 89f7ef2134..c23df77d96 100644
> --- a/lib/eal/include/rte_tailq.h
> +++ b/lib/eal/include/rte_tailq.h
> @@ -54,7 +54,7 @@ struct rte_tailq_elem {
>   * Return the first tailq entry cast to the right struct.
>   */
>  #define RTE_TAILQ_CAST(tailq_entry, struct_name) \
> -	(struct struct_name *)&(tailq_entry)->tailq_head
> +	(tailq_entry == NULL ? NULL : (struct struct_name *)&(tailq_entry)-
> >tailq_head)
> 
>  /**
>   * Utility macro to make looking up a tailqueue for a particular struct easier.

First tailq_entry is missing parentheses. Also, it is worrying that we now use macro argument twice. E.g. RTE_TAILQ_LOOKUP may become twice slower as a result.

Could we perhaps simplify the macro to `(struct struct_name *)(tailq_entry)`. I tried to find or understand the reasons behind the original construction, but could not.

[PATCH v2 08/10] hash: fix unaligned access in predictable RSS

[David Marchand]

Caught by UBSan:

../lib/hash/rte_thash.c:421:8: runtime error: load of misaligned address
	0x0001816c2da3 for type 'uint32_t' (aka 'unsigned int'),
	which requires 4 byte alignment

Fixes: 28ebff11c2dc ("hash: add predictable RSS")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/hash/rte_thash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
index 6c662bf14f..6d4dbea6d7 100644
--- a/lib/hash/rte_thash.c
+++ b/lib/hash/rte_thash.c
@@ -415,10 +415,10 @@ generate_subkey(struct rte_thash_ctx *ctx, struct thash_lfsr *lfsr,
 static inline uint32_t
 get_subvalue(struct rte_thash_ctx *ctx, uint32_t offset)
 {
-	uint32_t *tmp, val;
+	uint32_t tmp, val;
 
-	tmp = (uint32_t *)(&ctx->hash_key[offset >> 3]);
-	val = rte_be_to_cpu_32(*tmp);
+	memcpy(&tmp, &ctx->hash_key[offset >> 3], sizeof(tmp));
+	val = rte_be_to_cpu_32(tmp);
 	val >>= (TOEPLITZ_HASH_LEN - ((offset & (CHAR_BIT - 1)) +
 		ctx->reta_sz_log));
 
-- 
2.49.0

Re: [PATCH v2 08/10] hash: fix unaligned access in predictable RSS

[Bruce Richardson]

On Mon, Jun 23, 2025 at 03:52:38PM +0200, David Marchand wrote:
> Caught by UBSan:
> 
> ../lib/hash/rte_thash.c:421:8: runtime error: load of misaligned address
> 	0x0001816c2da3 for type 'uint32_t' (aka 'unsigned int'),
> 	which requires 4 byte alignment
> 
> Fixes: 28ebff11c2dc ("hash: add predictable RSS")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/hash/rte_thash.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
> index 6c662bf14f..6d4dbea6d7 100644
> --- a/lib/hash/rte_thash.c
> +++ b/lib/hash/rte_thash.c
> @@ -415,10 +415,10 @@ generate_subkey(struct rte_thash_ctx *ctx, struct thash_lfsr *lfsr,
>  static inline uint32_t
>  get_subvalue(struct rte_thash_ctx *ctx, uint32_t offset)
>  {
> -	uint32_t *tmp, val;
> +	uint32_t tmp, val;
>  
> -	tmp = (uint32_t *)(&ctx->hash_key[offset >> 3]);
> -	val = rte_be_to_cpu_32(*tmp);
> +	memcpy(&tmp, &ctx->hash_key[offset >> 3], sizeof(tmp));
> +	val = rte_be_to_cpu_32(tmp);
>  	val >>= (TOEPLITZ_HASH_LEN - ((offset & (CHAR_BIT - 1)) +
>  		ctx->reta_sz_log));
>  
> -- 
> 2.49.0
> 

RE: [PATCH v2 08/10] hash: fix unaligned access in predictable RSS

[Konstantin Ananyev]

> Caught by UBSan:
> 
> ../lib/hash/rte_thash.c:421:8: runtime error: load of misaligned address
> 	0x0001816c2da3 for type 'uint32_t' (aka 'unsigned int'),
> 	which requires 4 byte alignment
> 
> Fixes: 28ebff11c2dc ("hash: add predictable RSS")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/hash/rte_thash.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
> index 6c662bf14f..6d4dbea6d7 100644
> --- a/lib/hash/rte_thash.c
> +++ b/lib/hash/rte_thash.c
> @@ -415,10 +415,10 @@ generate_subkey(struct rte_thash_ctx *ctx, struct thash_lfsr *lfsr,
>  static inline uint32_t
>  get_subvalue(struct rte_thash_ctx *ctx, uint32_t offset)
>  {
> -	uint32_t *tmp, val;
> +	uint32_t tmp, val;
> 
> -	tmp = (uint32_t *)(&ctx->hash_key[offset >> 3]);
> -	val = rte_be_to_cpu_32(*tmp);
> +	memcpy(&tmp, &ctx->hash_key[offset >> 3], sizeof(tmp));
> +	val = rte_be_to_cpu_32(tmp);

Just wonder do you guys consider it as a real one?
AFAIK, all architectures that we care about do support unaligned load for 32-bit integers.

>  	val >>= (TOEPLITZ_HASH_LEN - ((offset & (CHAR_BIT - 1)) +
>  		ctx->reta_sz_log));
> 
> --
> 2.49.0
> 

Re: [PATCH v2 08/10] hash: fix unaligned access in predictable RSS

[David Marchand]

On Tue, Jul 1, 2025 at 10:36 AM Konstantin Ananyev
<konstantin.ananyev@huawei.com> wrote:
> > Caught by UBSan:
> >
> > ../lib/hash/rte_thash.c:421:8: runtime error: load of misaligned address
> >       0x0001816c2da3 for type 'uint32_t' (aka 'unsigned int'),
> >       which requires 4 byte alignment
> >
> > Fixes: 28ebff11c2dc ("hash: add predictable RSS")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  lib/hash/rte_thash.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
> > index 6c662bf14f..6d4dbea6d7 100644
> > --- a/lib/hash/rte_thash.c
> > +++ b/lib/hash/rte_thash.c
> > @@ -415,10 +415,10 @@ generate_subkey(struct rte_thash_ctx *ctx, struct thash_lfsr *lfsr,
> >  static inline uint32_t
> >  get_subvalue(struct rte_thash_ctx *ctx, uint32_t offset)
> >  {
> > -     uint32_t *tmp, val;
> > +     uint32_t tmp, val;
> >
> > -     tmp = (uint32_t *)(&ctx->hash_key[offset >> 3]);
> > -     val = rte_be_to_cpu_32(*tmp);
> > +     memcpy(&tmp, &ctx->hash_key[offset >> 3], sizeof(tmp));
> > +     val = rte_be_to_cpu_32(tmp);
>
> Just wonder do you guys consider it as a real one?
> AFAIK, all architectures that we care about do support unaligned load for 32-bit integers.

Well this is undefined behavior, regardless of what the architecture support.
And the compiler may end up generating wrong code.

I could revisit this change with the aliasing trick (used for rte_memcpy).


-- 
David Marchand

RE: [PATCH v2 08/10] hash: fix unaligned access in predictable RSS

[Konstantin Ananyev]

> On Tue, Jul 1, 2025 at 10:36 AM Konstantin Ananyev
> <konstantin.ananyev@huawei.com> wrote:
> > > Caught by UBSan:
> > >
> > > ../lib/hash/rte_thash.c:421:8: runtime error: load of misaligned address
> > >       0x0001816c2da3 for type 'uint32_t' (aka 'unsigned int'),
> > >       which requires 4 byte alignment
> > >
> > > Fixes: 28ebff11c2dc ("hash: add predictable RSS")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  lib/hash/rte_thash.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
> > > index 6c662bf14f..6d4dbea6d7 100644
> > > --- a/lib/hash/rte_thash.c
> > > +++ b/lib/hash/rte_thash.c
> > > @@ -415,10 +415,10 @@ generate_subkey(struct rte_thash_ctx *ctx, struct thash_lfsr *lfsr,
> > >  static inline uint32_t
> > >  get_subvalue(struct rte_thash_ctx *ctx, uint32_t offset)
> > >  {
> > > -     uint32_t *tmp, val;
> > > +     uint32_t tmp, val;
> > >
> > > -     tmp = (uint32_t *)(&ctx->hash_key[offset >> 3]);
> > > -     val = rte_be_to_cpu_32(*tmp);
> > > +     memcpy(&tmp, &ctx->hash_key[offset >> 3], sizeof(tmp));
> > > +     val = rte_be_to_cpu_32(tmp);
> >
> > Just wonder do you guys consider it as a real one?
> > AFAIK, all architectures that we care about do support unaligned load for 32-bit integers.
> 
> Well this is undefined behavior, regardless of what the architecture support.
> And the compiler may end up generating wrong code.

Probably, though AFAIK, we do have a lot of code that load 32-bit values from possibly
non-aligned addresses (nearly all packet parsing does that).
I wonder why it only complained only about that one? 
BTW, would our 'unaligned_uint32_t' type help here?

> I could revisit this change with the aliasing trick (used for rte_memcpy).
> 
> 
> --
> David Marchand
> 

[PATCH v2 09/10] stack: fix unaligned accesses on 128-bit

[David Marchand]

Caught by UBSan:

../lib/eal/x86/include/rte_atomic_64.h:206:21: runtime error:
	member access within misaligned address 0x7ffd9c67f228 for
	type 'const rte_int128_t', which requires 16 byte alignment
	0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
	../lib/eal/x86/include/rte_atomic_64.h:206:21: runtime error:
	member access within misaligned address 0x7ffd9c67f228 for type
	'const union rte_int128_t::(anonymous at
	../lib/eal/include/generic/rte_atomic.h:1102:2)', which requires
	16 byte alignment
0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
	../lib/eal/x86/include/rte_atomic_64.h:206:16: runtime error:
	load of misaligned address 0x7ffd9c67f228 for type
	'const uint64_t' (aka 'const unsigned long'), which requires
	16 byte alignment
0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in

Fixes: 3340202f5954 ("stack: add lock-free implementation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/stack/rte_stack_lf_c11.h     | 8 ++++----
 lib/stack/rte_stack_lf_generic.h | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/stack/rte_stack_lf_c11.h b/lib/stack/rte_stack_lf_c11.h
index b97e02d6a1..f674731235 100644
--- a/lib/stack/rte_stack_lf_c11.h
+++ b/lib/stack/rte_stack_lf_c11.h
@@ -63,13 +63,13 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
 			  struct rte_stack_lf_elem *last,
 			  unsigned int num)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success;
 
 	old_head = list->head;
 
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 
 		/* Swing the top pointer to the first element in the list and
 		 * make the last element point to the old top.
@@ -102,7 +102,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 			 void **obj_table,
 			 struct rte_stack_lf_elem **last)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	uint64_t len;
 	int success = 0;
 
@@ -129,7 +129,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 
 	/* Pop num elements */
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 		struct rte_stack_lf_elem *tmp;
 		unsigned int i;
 
diff --git a/lib/stack/rte_stack_lf_generic.h b/lib/stack/rte_stack_lf_generic.h
index cc69e4d168..32f56dffdd 100644
--- a/lib/stack/rte_stack_lf_generic.h
+++ b/lib/stack/rte_stack_lf_generic.h
@@ -36,13 +36,13 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
 			  struct rte_stack_lf_elem *last,
 			  unsigned int num)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success;
 
 	old_head = list->head;
 
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 
 		/* An acquire fence (or stronger) is needed for weak memory
 		 * models to establish a synchronized-with relationship between
@@ -77,7 +77,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 			 void **obj_table,
 			 struct rte_stack_lf_elem **last)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success = 0;
 
 	/* Reserve num elements, if available */
@@ -99,7 +99,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 
 	/* Pop num elements */
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 		struct rte_stack_lf_elem *tmp;
 		unsigned int i;
 
-- 
2.49.0

Re: [PATCH v2 09/10] stack: fix unaligned accesses on 128-bit

[Bruce Richardson]

On Mon, Jun 23, 2025 at 03:52:39PM +0200, David Marchand wrote:
> Caught by UBSan:
> 
> ../lib/eal/x86/include/rte_atomic_64.h:206:21: runtime error:
> 	member access within misaligned address 0x7ffd9c67f228 for
> 	type 'const rte_int128_t', which requires 16 byte alignment
> 	0x7ffd9c67f228: note: pointer points here
>  00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
>               ^
>  00 00 00 00 00 00 00 00  00 00 00 00
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> 	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
> 	../lib/eal/x86/include/rte_atomic_64.h:206:21: runtime error:
> 	member access within misaligned address 0x7ffd9c67f228 for type
> 	'const union rte_int128_t::(anonymous at
> 	../lib/eal/include/generic/rte_atomic.h:1102:2)', which requires
> 	16 byte alignment
> 0x7ffd9c67f228: note: pointer points here
>  00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
>               ^
>  00 00 00 00 00 00 00 00  00 00 00 00
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> 	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
> 	../lib/eal/x86/include/rte_atomic_64.h:206:16: runtime error:
> 	load of misaligned address 0x7ffd9c67f228 for type
> 	'const uint64_t' (aka 'const unsigned long'), which requires
> 	16 byte alignment
> 0x7ffd9c67f228: note: pointer points here
>  00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
>               ^
>  00 00 00 00 00 00 00 00  00 00 00 00
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> 	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
> 
> Fixes: 3340202f5954 ("stack: add lock-free implementation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v2 10/10] build: support Undefined Behavior Sanitizer

[David Marchand]

Enable UBSan in GHA.
There are still a lot of issues so only run unit tests for a "mini"
target.

Building with debugoptimized forces -O2 and consumes too much memory
with UBSan, prefer plain build (iow -O0) even though this hides a number
of build issues.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .ci/linux-build.sh          | 27 +++++++++++++++++++++++++--
 .github/workflows/build.yml |  5 +++++
 app/test/suites/meson.build |  3 +--
 config/meson.build          | 18 +++++++++++++++++-
 devtools/words-case.txt     |  1 +
 5 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index e9272d3931..905c11b1bf 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -44,6 +44,13 @@ catch_coredump() {
     return 1
 }
 
+catch_ubsan() {
+    [ "$UBSAN" = "true" ] || return 0
+    grep -q UndefinedBehaviorSanitizer $2 2>/dev/null || return 0
+    grep -E "($1|UndefinedBehaviorSanitizer)" $2
+    return 1
+}
+
 check_traces() {
     which babeltrace >/dev/null || return 0
     for file in $(sudo find $HOME -name metadata); do
@@ -100,7 +107,6 @@ fi
 
 OPTS="$OPTS -Dplatform=generic"
 OPTS="$OPTS -Ddefault_library=$DEF_LIB"
-OPTS="$OPTS -Dbuildtype=$buildtype"
 if [ "$STDATOMIC" = "true" ]; then
 	OPTS="$OPTS -Denable_stdatomic=true"
 else
@@ -117,13 +123,28 @@ else
 fi
 OPTS="$OPTS -Dlibdir=lib"
 
+buildtype=debugoptimized
+sanitizer=
 if [ "$ASAN" = "true" ]; then
-    OPTS="$OPTS -Db_sanitize=address"
+    sanitizer=${sanitizer:+$sanitizer,}address
+fi
+
+if [ "$UBSAN" = "true" ]; then
+    sanitizer=${sanitizer:+$sanitizer,}undefined
+    if [ "$RUN_TESTS" = "true" ]; then
+        # UBSan takes too much memory with -O2
+        buildtype=plain
+    fi
+fi
+
+if [ -n "$sanitizer" ]; then
+    OPTS="$OPTS -Db_sanitize=$sanitizer"
     if [ "${CC%%clang}" != "$CC" ]; then
         OPTS="$OPTS -Db_lundef=false"
     fi
 fi
 
+OPTS="$OPTS -Dbuildtype=$buildtype"
 OPTS="$OPTS -Dwerror=true"
 
 if [ -d build ]; then
@@ -141,6 +162,7 @@ if [ -z "$cross_file" ]; then
     configure_coredump
     devtools/test-null.sh || failed="true"
     catch_coredump
+    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
     check_traces
     [ "$failed" != "true" ]
 fi
@@ -182,6 +204,7 @@ if [ "$RUN_TESTS" = "true" ]; then
     configure_coredump
     sudo meson test -C build --suite fast-tests -t 3 || failed="true"
     catch_coredump
+    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
     check_traces
     [ "$failed" != "true" ]
 fi
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 6c4bc664d0..4353bb97d8 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -54,6 +54,7 @@ jobs:
       RISCV64: ${{ matrix.config.cross == 'riscv64' }}
       RUN_TESTS: ${{ contains(matrix.config.checks, 'tests') }}
       STDATOMIC: ${{ contains(matrix.config.checks, 'stdatomic') }}
+      UBSAN: ${{ contains(matrix.config.checks, 'ubsan') }}
 
     strategy:
       fail-fast: false
@@ -62,6 +63,10 @@ jobs:
           - os: ubuntu-22.04
             compiler: gcc
             mini: mini
+          - os: ubuntu-22.04
+            compiler: clang
+            mini: mini
+            checks: tests+ubsan
           - os: ubuntu-22.04
             compiler: gcc
             checks: stdatomic
diff --git a/app/test/suites/meson.build b/app/test/suites/meson.build
index e482373330..0cba63ee12 100644
--- a/app/test/suites/meson.build
+++ b/app/test/suites/meson.build
@@ -72,8 +72,7 @@ foreach suite:test_suites
             elif not has_hugepage
                 continue  #skip this tests
             endif
-            if not asan and (get_option('b_sanitize') == 'address'
-                    or get_option('b_sanitize') == 'address,undefined')
+            if not asan and get_option('b_sanitize').contains('address')
                 continue  # skip this test
             endif
 
diff --git a/config/meson.build b/config/meson.build
index f31fef216c..b8a8511a7f 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -499,7 +499,7 @@ if get_option('b_lto')
     endif
 endif
 
-if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address,undefined'
+if get_option('b_sanitize').contains('address')
     if is_windows
         error('ASan is not supported on windows')
     endif
@@ -519,6 +519,22 @@ if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address
     endif
 endif
 
+if get_option('b_sanitize').contains('undefined')
+    if is_windows
+        error('UBSan is not supported on windows')
+    endif
+
+    if cc.get_id() == 'gcc'
+        ubsan_dep = cc.find_library('ubsan', required: true)
+        if (not cc.links('int main(int argc, char *argv[]) { return 0; }',
+                dependencies: ubsan_dep))
+            error('broken dependency, "libubsan"')
+        endif
+        add_project_link_arguments('-lubsan', language: 'c')
+        dpdk_extra_ldflags += '-lubsan'
+    endif
+endif
+
 if get_option('default_library') == 'both'
     error( '''
  Unsupported value "both" for "default_library" option.
diff --git a/devtools/words-case.txt b/devtools/words-case.txt
index bc35c160ba..d315fde8fe 100644
--- a/devtools/words-case.txt
+++ b/devtools/words-case.txt
@@ -105,6 +105,7 @@ TSO
 TTL
 Tx
 uAPI
+UBSan
 UDM
 UDP
 ULP
-- 
2.49.0

[PATCH v3 00/18] Run with UBSan in GHA

[David Marchand]

This series fixes a number of issues reported by UBSan and adds a simple
job in GHA to avoid introducing undefined behavior in the core
components.
There is way more work/fixes to do if we want to run with a full set of
components, but baby steps first.


-- 
David Marchand

Changes since v2:
- simplified plugin fix,
- enabled more coverage,

Changes since v1:
- small updates on patches 5 and 6,

David Marchand (17):
  ci: save ccache on failure
  test/telemetry: fix test calling all commands
  test/mempool: fix test without stack driver
  eal: fix plugin dir walk
  cmdline: fix port list parsing
  cmdline: fix highest bit port list parsing
  tailq: fix cast macro for null pointer
  hash: fix unaligned access in predictable RSS
  stack: fix unaligned accesses on 128-bit
  build: support Undefined Behavior Sanitizer
  test/telemetry: catch errors in subshell
  malloc: fix mp message alignment
  graph: fix unaligned access in stats
  eventdev: fix listing timer adapters with telemetry
  test/power: fix tests without power drivers
  test/raw: fix test without skeleton driver
  ci: extend coverage with UBSan

Marat Khalili (1):
  graph: fix stats query with no node xstats

 .ci/linux-build.sh                     |  38 ++++++++-
 .github/workflows/build.yml            |  11 +++
 .mailmap                               |   1 +
 app/test/meson.build                   |  12 +--
 app/test/suites/meson.build            |   3 +-
 app/test/suites/test_telemetry.sh      |   6 +-
 app/test/test_mempool.c                |  32 +++++---
 config/meson.build                     |  18 ++++-
 devtools/words-case.txt                |   1 +
 lib/cmdline/cmdline_parse_portlist.c   |  15 ++--
 lib/eal/common/eal_common_options.c    |  17 ++--
 lib/eal/common/eal_common_proc.c       |   2 +-
 lib/eal/common/malloc_mp.c             |  18 ++---
 lib/eal/include/rte_tailq.h            |   2 +-
 lib/eventdev/rte_event_timer_adapter.c |   4 +-
 lib/graph/graph_stats.c                | 105 ++++++++++++++-----------
 lib/hash/rte_thash.c                   |   6 +-
 lib/stack/rte_stack_lf_c11.h           |   8 +-
 lib/stack/rte_stack_lf_generic.h       |   8 +-
 19 files changed, 201 insertions(+), 106 deletions(-)

-- 
2.50.0

[PATCH v3 01/18] ci: save ccache on failure

[David Marchand]

When troubleshooting unit test failures and repeating jobs in GHA,
the absence of ccache makes the whole process way slower.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 .github/workflows/build.yml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index e5f17ef6ac..6c4bc664d0 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -164,6 +164,12 @@ jobs:
         chmod o-w $HOME
     - name: Build and test
       run: .ci/linux-build.sh
+    - name: Save ccache on failure
+      if: failure()
+      uses: actions/cache/save@v4
+      with:
+        path: ~/.ccache
+        key: ${{ steps.get_ref_keys.outputs.ccache }}-${{ github.ref }}
     - name: Upload logs on failure
       if: failure()
       uses: actions/upload-artifact@v4
-- 
2.50.0

[PATCH v3 02/18] test/telemetry: fix test calling all commands

[David Marchand]

This test was doing nothing as it could not find the telemetry client
script following the test suite rework.

Caught while looking at UNH unit test logs:

/root/workspace/Generic-Unit-Test-DPDK/dpdk/app/test/suites/test_telemetry.sh:
18: /root/workspace/Generic-Unit-Test-DPDK/dpdk/app/usertools/dpdk-telemetry.py:
not found

Fixes: 9da71dc4f96e ("test: add test case for scripted telemetry commands")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Marat Khalili <marat.khalili@huawei.com>
---
 .mailmap                          | 1 +
 app/test/suites/test_telemetry.sh | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 1ea4f9446d..84a83369e0 100644
--- a/.mailmap
+++ b/.mailmap
@@ -936,6 +936,7 @@ Manish Chopra <manishc@marvell.com>
 Manish Kurup <manish.kurup@broadcom.com>
 Manish Tomar <manish.tomar@nxp.com>
 Mao Jiang <maox.jiang@intel.com>
+Marat Khalili <marat.khalili@huawei.com>
 Marcel Apfelbaum <marcel@redhat.com>
 Marcel Cornu <marcel.d.cornu@intel.com>
 Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
diff --git a/app/test/suites/test_telemetry.sh b/app/test/suites/test_telemetry.sh
index ca6abe266e..20806b43e4 100755
--- a/app/test/suites/test_telemetry.sh
+++ b/app/test/suites/test_telemetry.sh
@@ -7,7 +7,7 @@ which jq || {
     exit 77
 }
 
-rootdir=$(readlink -f $(dirname $(readlink -f $0))/../..)
+rootdir=$(readlink -f $(dirname $(readlink -f $0))/../../..)
 tmpoutput=$(mktemp -t dpdk.test_telemetry.XXXXXX)
 trap "cat $tmpoutput; rm -f $tmpoutput" EXIT
 
-- 
2.50.0

[PATCH v3 03/18] test/mempool: fix test without stack driver

[David Marchand]

In a minimal build, the mempool/stack driver is disabled.
Separate the code specific to this external driver and rename unrelated
variables.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Reviewed-by: Marat Khalili <marat.khalili@huawei.com>
---
 app/test/test_mempool.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 61385e096e..63356998fd 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -847,9 +847,11 @@ test_mempool(void)
 	size_t alignment = 0;
 	struct rte_mempool *mp_cache = NULL;
 	struct rte_mempool *mp_nocache = NULL;
-	struct rte_mempool *mp_stack_anon = NULL;
-	struct rte_mempool *mp_stack_mempool_iter = NULL;
+	struct rte_mempool *mp_anon = NULL;
+	struct rte_mempool *mp_mempool_iter = NULL;
+#ifdef RTE_MEMPOOL_STACK
 	struct rte_mempool *mp_stack = NULL;
+#endif
 	struct rte_mempool *default_pool = NULL;
 	struct rte_mempool *mp_alignment = NULL;
 	struct mp_data cb_arg = {
@@ -884,28 +886,28 @@ test_mempool(void)
 	}
 
 	/* create an empty mempool  */
-	mp_stack_anon = rte_mempool_create_empty("test_stack_anon",
+	mp_anon = rte_mempool_create_empty("test_anon",
 		MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE,
 		RTE_MEMPOOL_CACHE_MAX_SIZE, 0,
 		SOCKET_ID_ANY, 0);
 
-	if (mp_stack_anon == NULL)
+	if (mp_anon == NULL)
 		GOTO_ERR(ret, err);
 
 	/* populate an empty mempool */
-	ret = rte_mempool_populate_anon(mp_stack_anon);
+	ret = rte_mempool_populate_anon(mp_anon);
 	printf("%s ret = %d\n", __func__, ret);
 	if (ret < 0)
 		GOTO_ERR(ret, err);
 
 	/* Try to populate when already populated */
-	ret = rte_mempool_populate_anon(mp_stack_anon);
+	ret = rte_mempool_populate_anon(mp_anon);
 	if (ret != 0)
 		GOTO_ERR(ret, err);
 
 	/* create a mempool  */
-	mp_stack_mempool_iter = rte_mempool_create("test_iter_obj",
+	mp_mempool_iter = rte_mempool_create("test_iter_obj",
 		MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE,
 		RTE_MEMPOOL_CACHE_MAX_SIZE, 0,
@@ -913,20 +915,21 @@ test_mempool(void)
 		my_obj_init, NULL,
 		SOCKET_ID_ANY, 0);
 
-	if (mp_stack_mempool_iter == NULL)
+	if (mp_mempool_iter == NULL)
 		GOTO_ERR(ret, err);
 
 	/* test to initialize mempool objects and memory */
-	nb_objs = rte_mempool_obj_iter(mp_stack_mempool_iter, my_obj_init,
+	nb_objs = rte_mempool_obj_iter(mp_mempool_iter, my_obj_init,
 			NULL);
 	if (nb_objs == 0)
 		GOTO_ERR(ret, err);
 
-	nb_mem_chunks = rte_mempool_mem_iter(mp_stack_mempool_iter,
+	nb_mem_chunks = rte_mempool_mem_iter(mp_mempool_iter,
 			test_mp_mem_init, &cb_arg);
 	if (nb_mem_chunks == 0 || cb_arg.ret < 0)
 		GOTO_ERR(ret, err);
 
+#ifdef RTE_MEMPOOL_STACK
 	/* create a mempool with an external handler */
 	mp_stack = rte_mempool_create_empty("test_stack",
 		MEMPOOL_SIZE,
@@ -947,6 +950,7 @@ test_mempool(void)
 		GOTO_ERR(ret, err);
 	}
 	rte_mempool_obj_iter(mp_stack, my_obj_init, NULL);
+#endif /* RTE_MEMPOOL_STACK */
 
 	/* Create a mempool based on Default handler */
 	printf("Testing %s mempool handler\n", default_pool_ops);
@@ -1077,9 +1081,11 @@ test_mempool(void)
 	if (test_mempool_same_name_twice_creation() < 0)
 		GOTO_ERR(ret, err);
 
+#ifdef RTE_MEMPOOL_STACK
 	/* test the stack handler */
 	if (test_mempool_basic(mp_stack, 1) < 0)
 		GOTO_ERR(ret, err);
+#endif
 
 	if (test_mempool_basic(default_pool, 1) < 0)
 		GOTO_ERR(ret, err);
@@ -1105,9 +1111,11 @@ test_mempool(void)
 err:
 	rte_mempool_free(mp_nocache);
 	rte_mempool_free(mp_cache);
-	rte_mempool_free(mp_stack_anon);
-	rte_mempool_free(mp_stack_mempool_iter);
+	rte_mempool_free(mp_anon);
+	rte_mempool_free(mp_mempool_iter);
+#ifdef RTE_MEMPOOL_STACK
 	rte_mempool_free(mp_stack);
+#endif
 	rte_mempool_free(default_pool);
 	rte_mempool_free(mp_alignment);
 
-- 
2.50.0

RE: [PATCH v3 03/18] test/mempool: fix test without stack driver

[Morten Brørup]

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Tuesday, 8 July 2025 14.28
> 
> In a minimal build, the mempool/stack driver is disabled.
> Separate the code specific to this external driver and rename unrelated
> variables.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Marat Khalili <marat.khalili@huawei.com>
> ---

Acked-by: Morten Brørup <mb@smartsharesystems.com>

[PATCH v3 04/18] eal: fix plugin dir walk

[David Marchand]

For '.' and '..' directories (or any short file name),
a out of bound issue occurs.

Caught by UBSan:

EAL: Detected shared linkage of DPDK
../lib/eal/common/eal_common_options.c:420:15: runtime error: index -2
	out of bounds for type 'char[256]'
    #0 0x7f867eedf206 in eal_plugindir_init
	eal_common_options.c
    #1 0x7f867eede58a in eal_plugins_init
	(build/lib/librte_eal.so.25+0xde58a)
	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
    #2 0x7f867efb8587 in rte_eal_init
	(build/lib/librte_eal.so.25+0x1b8587)
	(BuildId: e7e4a1935e4bacb51c82ab1a84098a27decf3b4c)
    #3 0x55b62360861e in main
	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x9e061e)
	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
    #4 0x7f8667429d8f in __libc_start_call_main
	csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #5 0x7f8667429e3f in __libc_start_main
	csu/../csu/libc-start.c:392:3
    #6 0x55b622d9d444 in _start
	(/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0x175444)
	(BuildId: d821ec918612c83fad8b5ccb6cc518e66bee48cd)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/common/eal_common_options.c:420:15 in
	../lib/eal/common/eal_common_options.c:421:15:
	runtime error: index 18446744073709551609 out of bounds
	for type 'char[256]'

Fixes: c57f6e5c604a ("eal: fix plugin loading")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
Changes since v2:
- removed check on null termination of dirent,

---
 lib/eal/common/eal_common_options.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index f0a9ddeeb7..e6eb04a138 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -399,12 +399,21 @@ eal_plugins_init(void)
 }
 #else
 
+static bool
+ends_with(const char *str, const char *tail)
+{
+	size_t tail_len = strlen(tail);
+	size_t str_len = strlen(str);
+
+	return str_len >= tail_len && strcmp(&str[str_len - tail_len], tail) == 0;
+}
+
 static int
 eal_plugindir_init(const char *path)
 {
-	DIR *d = NULL;
 	struct dirent *dent = NULL;
 	char sopath[PATH_MAX];
+	DIR *d = NULL;
 
 	if (path == NULL || *path == '\0')
 		return 0;
@@ -418,12 +427,8 @@ eal_plugindir_init(const char *path)
 
 	while ((dent = readdir(d)) != NULL) {
 		struct stat sb;
-		int nlen = strnlen(dent->d_name, sizeof(dent->d_name));
 
-		/* check if name ends in .so or .so.ABI_VERSION */
-		if (strcmp(&dent->d_name[nlen - 3], ".so") != 0 &&
-		    strcmp(&dent->d_name[nlen - 4 - strlen(ABI_VERSION)],
-			   ".so."ABI_VERSION) != 0)
+		if (!ends_with(dent->d_name, ".so") && !ends_with(dent->d_name, ".so."ABI_VERSION))
 			continue;
 
 		snprintf(sopath, sizeof(sopath), "%s/%s", path, dent->d_name);
-- 
2.50.0

[PATCH v3 05/18] cmdline: fix port list parsing

[David Marchand]

Doing arithmetic with the NULL pointer is undefined.

Caught by UBSan:

../lib/cmdline/cmdline_parse_portlist.c:40:19: runtime error:
	applying non-zero offset 1 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/cmdline/cmdline_parse_portlist.c:40:19 in

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Marat Khalili <marat.khalili@huawei.com>
---
Changes since v1:
- moved some variable as suggested by Bruce,

---
 lib/cmdline/cmdline_parse_portlist.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
index ef6ce223b5..549f6d9671 100644
--- a/lib/cmdline/cmdline_parse_portlist.c
+++ b/lib/cmdline/cmdline_parse_portlist.c
@@ -33,15 +33,12 @@ parse_set_list(cmdline_portlist_t *pl, size_t low, size_t high)
 static int
 parse_ports(cmdline_portlist_t *pl, const char *str)
 {
+	const char *first = str;
 	size_t ps, pe;
-	const char *first, *last;
 	char *end;
 
-	for (first = str, last = first;
-	    first != NULL && last != NULL;
-	    first = last + 1) {
-
-		last = strchr(first, ',');
+	while (first != NULL) {
+		const char *last = strchr(first, ',');
 
 		errno = 0;
 		ps = strtoul(first, &end, 10);
@@ -65,6 +62,7 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
 			return -1;
 
 		parse_set_list(pl, ps, pe);
+		first = (last == NULL ? NULL : last + 1);
 	}
 
 	return 0;
-- 
2.50.0

[PATCH v3 06/18] cmdline: fix highest bit port list parsing

[David Marchand]

pl->map is a uint32_t.

Caught by UBSan:

../lib/cmdline/cmdline_parse_portlist.c:27:17: runtime error:
	left shift of 1 by 31 places cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/cmdline/cmdline_parse_portlist.c:27:17 in

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Marat Khalili <marat.khalili@huawei.com>
---
Changes since v1:
- moved variable increment out of the macro call,

---
 lib/cmdline/cmdline_parse_portlist.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
index 549f6d9671..3efe4143e3 100644
--- a/lib/cmdline/cmdline_parse_portlist.c
+++ b/lib/cmdline/cmdline_parse_portlist.c
@@ -10,7 +10,9 @@
 #include <errno.h>
 
 #include <eal_export.h>
+#include <rte_bitops.h>
 #include <rte_string_fns.h>
+
 #include "cmdline_parse.h"
 #include "cmdline_parse_portlist.h"
 
@@ -26,7 +28,8 @@ static void
 parse_set_list(cmdline_portlist_t *pl, size_t low, size_t high)
 {
 	do {
-		pl->map |= (1 << low++);
+		pl->map |= RTE_BIT32(low);
+		low++;
 	} while (low <= high);
 }
 
-- 
2.50.0

[PATCH v3 07/18] tailq: fix cast macro for null pointer

[David Marchand]

Doing arithmetic with the NULL pointer is undefined.

Caught by UBSan:

../app/test/test_tailq.c:111:9: runtime error:
	member access within null pointer of type 'struct rte_tailq_head'

Fixes: f6b4f6c9c123 ("tailq: use a single cast macro")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/include/rte_tailq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/eal/include/rte_tailq.h b/lib/eal/include/rte_tailq.h
index 89f7ef2134..c23df77d96 100644
--- a/lib/eal/include/rte_tailq.h
+++ b/lib/eal/include/rte_tailq.h
@@ -54,7 +54,7 @@ struct rte_tailq_elem {
  * Return the first tailq entry cast to the right struct.
  */
 #define RTE_TAILQ_CAST(tailq_entry, struct_name) \
-	(struct struct_name *)&(tailq_entry)->tailq_head
+	(tailq_entry == NULL ? NULL : (struct struct_name *)&(tailq_entry)->tailq_head)
 
 /**
  * Utility macro to make looking up a tailqueue for a particular struct easier.
-- 
2.50.0

[PATCH v3 08/18] hash: fix unaligned access in predictable RSS

[David Marchand]

Caught by UBSan:

../lib/hash/rte_thash.c:421:8: runtime error: load of misaligned address
	0x0001816c2da3 for type 'uint32_t' (aka 'unsigned int'),
	which requires 4 byte alignment

Fixes: 28ebff11c2dc ("hash: add predictable RSS")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/hash/rte_thash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
index 6c662bf14f..6d4dbea6d7 100644
--- a/lib/hash/rte_thash.c
+++ b/lib/hash/rte_thash.c
@@ -415,10 +415,10 @@ generate_subkey(struct rte_thash_ctx *ctx, struct thash_lfsr *lfsr,
 static inline uint32_t
 get_subvalue(struct rte_thash_ctx *ctx, uint32_t offset)
 {
-	uint32_t *tmp, val;
+	uint32_t tmp, val;
 
-	tmp = (uint32_t *)(&ctx->hash_key[offset >> 3]);
-	val = rte_be_to_cpu_32(*tmp);
+	memcpy(&tmp, &ctx->hash_key[offset >> 3], sizeof(tmp));
+	val = rte_be_to_cpu_32(tmp);
 	val >>= (TOEPLITZ_HASH_LEN - ((offset & (CHAR_BIT - 1)) +
 		ctx->reta_sz_log));
 
-- 
2.50.0

Re: [PATCH v3 08/18] hash: fix unaligned access in predictable RSS

[Medvedkin, Vladimir]

Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>

On 7/8/2025 1:28 PM, David Marchand wrote:
> Caught by UBSan:
>
> ../lib/hash/rte_thash.c:421:8: runtime error: load of misaligned address
> 	0x0001816c2da3 for type 'uint32_t' (aka 'unsigned int'),
> 	which requires 4 byte alignment
>
> Fixes: 28ebff11c2dc ("hash: add predictable RSS")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   lib/hash/rte_thash.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
> index 6c662bf14f..6d4dbea6d7 100644
> --- a/lib/hash/rte_thash.c
> +++ b/lib/hash/rte_thash.c
> @@ -415,10 +415,10 @@ generate_subkey(struct rte_thash_ctx *ctx, struct thash_lfsr *lfsr,
>   static inline uint32_t
>   get_subvalue(struct rte_thash_ctx *ctx, uint32_t offset)
>   {
> -	uint32_t *tmp, val;
> +	uint32_t tmp, val;
>   
> -	tmp = (uint32_t *)(&ctx->hash_key[offset >> 3]);
> -	val = rte_be_to_cpu_32(*tmp);
> +	memcpy(&tmp, &ctx->hash_key[offset >> 3], sizeof(tmp));
> +	val = rte_be_to_cpu_32(tmp);
>   	val >>= (TOEPLITZ_HASH_LEN - ((offset & (CHAR_BIT - 1)) +
>   		ctx->reta_sz_log));
>   

-- 
Regards,
Vladimir

[PATCH v3 09/18] stack: fix unaligned accesses on 128-bit

[David Marchand]

Caught by UBSan:

../lib/eal/x86/include/rte_atomic_64.h:206:21: runtime error:
	member access within misaligned address 0x7ffd9c67f228 for
	type 'const rte_int128_t', which requires 16 byte alignment
	0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
	../lib/eal/x86/include/rte_atomic_64.h:206:21: runtime error:
	member access within misaligned address 0x7ffd9c67f228 for type
	'const union rte_int128_t::(anonymous at
	../lib/eal/include/generic/rte_atomic.h:1102:2)', which requires
	16 byte alignment
0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in
	../lib/eal/x86/include/rte_atomic_64.h:206:16: runtime error:
	load of misaligned address 0x7ffd9c67f228 for type
	'const uint64_t' (aka 'const unsigned long'), which requires
	16 byte alignment
0x7ffd9c67f228: note: pointer points here
 00 00 00 00  c0 5d 3e 00 01 00 00 00  01 00 00 00 00 00 00 00
              ^
 00 00 00 00 00 00 00 00  00 00 00 00
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
	../lib/eal/x86/include/rte_atomic_64.h:206:21 in

Fixes: 3340202f5954 ("stack: add lock-free implementation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/stack/rte_stack_lf_c11.h     | 8 ++++----
 lib/stack/rte_stack_lf_generic.h | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/stack/rte_stack_lf_c11.h b/lib/stack/rte_stack_lf_c11.h
index b97e02d6a1..f674731235 100644
--- a/lib/stack/rte_stack_lf_c11.h
+++ b/lib/stack/rte_stack_lf_c11.h
@@ -63,13 +63,13 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
 			  struct rte_stack_lf_elem *last,
 			  unsigned int num)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success;
 
 	old_head = list->head;
 
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 
 		/* Swing the top pointer to the first element in the list and
 		 * make the last element point to the old top.
@@ -102,7 +102,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 			 void **obj_table,
 			 struct rte_stack_lf_elem **last)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	uint64_t len;
 	int success = 0;
 
@@ -129,7 +129,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 
 	/* Pop num elements */
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 		struct rte_stack_lf_elem *tmp;
 		unsigned int i;
 
diff --git a/lib/stack/rte_stack_lf_generic.h b/lib/stack/rte_stack_lf_generic.h
index cc69e4d168..32f56dffdd 100644
--- a/lib/stack/rte_stack_lf_generic.h
+++ b/lib/stack/rte_stack_lf_generic.h
@@ -36,13 +36,13 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
 			  struct rte_stack_lf_elem *last,
 			  unsigned int num)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success;
 
 	old_head = list->head;
 
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 
 		/* An acquire fence (or stronger) is needed for weak memory
 		 * models to establish a synchronized-with relationship between
@@ -77,7 +77,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 			 void **obj_table,
 			 struct rte_stack_lf_elem **last)
 {
-	struct rte_stack_lf_head old_head;
+	alignas(16) struct rte_stack_lf_head old_head;
 	int success = 0;
 
 	/* Reserve num elements, if available */
@@ -99,7 +99,7 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 
 	/* Pop num elements */
 	do {
-		struct rte_stack_lf_head new_head;
+		alignas(16) struct rte_stack_lf_head new_head;
 		struct rte_stack_lf_elem *tmp;
 		unsigned int i;
 
-- 
2.50.0

RE: [PATCH v3 09/18] stack: fix unaligned accesses on 128-bit

[Morten Brørup]

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Tuesday, 8 July 2025 14.28
> 
> Fixes: 3340202f5954 ("stack: add lock-free implementation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

> -	struct rte_stack_lf_head old_head;
> +	alignas(16) struct rte_stack_lf_head old_head;

I don't know if Bruce also commented on this one, like he did on patch 12/18; so at the risk of repeating something already mentioned...

If all uses of struct rte_stack_lf_head must be 16-byte aligned, better fix the root cause by aligning the type rather than every use of it.
Please add such a root cause fix to the 25.11 TODO list, like the alignment of the struct rte_mp_msg type.

[PATCH v3 10/18] build: support Undefined Behavior Sanitizer

[David Marchand]

Enable UBSan in GHA.
There are still a lot of issues so only run unit tests for a "mini"
target.

Building with debugoptimized forces -O2 and consumes too much memory
with UBSan, prefer plain build (iow -O0) even though this hides a number
of build issues.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v2:
- left the mini configuration alone and instead filtered
  enabled component when running unit tests with UBSan,

---
 .ci/linux-build.sh          | 38 +++++++++++++++++++++++++++++++++++--
 .github/workflows/build.yml |  5 +++++
 app/test/suites/meson.build |  3 +--
 config/meson.build          | 18 +++++++++++++++++-
 devtools/words-case.txt     |  1 +
 5 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index e9272d3931..a24820c65b 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -44,6 +44,13 @@ catch_coredump() {
     return 1
 }
 
+catch_ubsan() {
+    [ "$UBSAN" = "true" ] || return 0
+    grep -q UndefinedBehaviorSanitizer $2 2>/dev/null || return 0
+    grep -E "($1|UndefinedBehaviorSanitizer)" $2
+    return 1
+}
+
 check_traces() {
     which babeltrace >/dev/null || return 0
     for file in $(sudo find $HOME -name metadata); do
@@ -100,7 +107,6 @@ fi
 
 OPTS="$OPTS -Dplatform=generic"
 OPTS="$OPTS -Ddefault_library=$DEF_LIB"
-OPTS="$OPTS -Dbuildtype=$buildtype"
 if [ "$STDATOMIC" = "true" ]; then
 	OPTS="$OPTS -Denable_stdatomic=true"
 else
@@ -117,13 +123,39 @@ else
 fi
 OPTS="$OPTS -Dlibdir=lib"
 
+buildtype=debugoptimized
+sanitizer=
 if [ "$ASAN" = "true" ]; then
-    OPTS="$OPTS -Db_sanitize=address"
+    sanitizer=${sanitizer:+$sanitizer,}address
+fi
+
+if [ "$UBSAN" = "true" ]; then
+    sanitizer=${sanitizer:+$sanitizer,}undefined
+    if [ "$RUN_TESTS" = "true" ]; then
+        # UBSan takes too much memory with -O2
+        buildtype=plain
+        # Only enable test binaries
+        OPTS="$OPTS -Denable_apps=test,test-pmd"
+        # Restrict drivers to a minimum set for now
+        OPTS="$OPTS -Denable_drivers=net/null"
+        # There are still known issues in various libraries, disable them for now
+        OPTS="$OPTS -Ddisable_libs=*"
+        # No examples are run
+        OPTS="$OPTS -Dexamples="
+        # The UBSan target takes a good amount of time and headers coverage is done in other
+        # targets already.
+        OPTS="$OPTS -Dcheck_includes=false"
+    fi
+fi
+
+if [ -n "$sanitizer" ]; then
+    OPTS="$OPTS -Db_sanitize=$sanitizer"
     if [ "${CC%%clang}" != "$CC" ]; then
         OPTS="$OPTS -Db_lundef=false"
     fi
 fi
 
+OPTS="$OPTS -Dbuildtype=$buildtype"
 OPTS="$OPTS -Dwerror=true"
 
 if [ -d build ]; then
@@ -141,6 +173,7 @@ if [ -z "$cross_file" ]; then
     configure_coredump
     devtools/test-null.sh || failed="true"
     catch_coredump
+    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
     check_traces
     [ "$failed" != "true" ]
 fi
@@ -182,6 +215,7 @@ if [ "$RUN_TESTS" = "true" ]; then
     configure_coredump
     sudo meson test -C build --suite fast-tests -t 3 || failed="true"
     catch_coredump
+    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
     check_traces
     [ "$failed" != "true" ]
 fi
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 6c4bc664d0..0c8ca23bbb 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -54,6 +54,7 @@ jobs:
       RISCV64: ${{ matrix.config.cross == 'riscv64' }}
       RUN_TESTS: ${{ contains(matrix.config.checks, 'tests') }}
       STDATOMIC: ${{ contains(matrix.config.checks, 'stdatomic') }}
+      UBSAN: ${{ contains(matrix.config.checks, 'ubsan') }}
 
     strategy:
       fail-fast: false
@@ -62,6 +63,10 @@ jobs:
           - os: ubuntu-22.04
             compiler: gcc
             mini: mini
+          - os: ubuntu-22.04
+            compiler: clang
+            library: shared
+            checks: tests+ubsan
           - os: ubuntu-22.04
             compiler: gcc
             checks: stdatomic
diff --git a/app/test/suites/meson.build b/app/test/suites/meson.build
index e482373330..0cba63ee12 100644
--- a/app/test/suites/meson.build
+++ b/app/test/suites/meson.build
@@ -72,8 +72,7 @@ foreach suite:test_suites
             elif not has_hugepage
                 continue  #skip this tests
             endif
-            if not asan and (get_option('b_sanitize') == 'address'
-                    or get_option('b_sanitize') == 'address,undefined')
+            if not asan and get_option('b_sanitize').contains('address')
                 continue  # skip this test
             endif
 
diff --git a/config/meson.build b/config/meson.build
index f31fef216c..b8a8511a7f 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -499,7 +499,7 @@ if get_option('b_lto')
     endif
 endif
 
-if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address,undefined'
+if get_option('b_sanitize').contains('address')
     if is_windows
         error('ASan is not supported on windows')
     endif
@@ -519,6 +519,22 @@ if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address
     endif
 endif
 
+if get_option('b_sanitize').contains('undefined')
+    if is_windows
+        error('UBSan is not supported on windows')
+    endif
+
+    if cc.get_id() == 'gcc'
+        ubsan_dep = cc.find_library('ubsan', required: true)
+        if (not cc.links('int main(int argc, char *argv[]) { return 0; }',
+                dependencies: ubsan_dep))
+            error('broken dependency, "libubsan"')
+        endif
+        add_project_link_arguments('-lubsan', language: 'c')
+        dpdk_extra_ldflags += '-lubsan'
+    endif
+endif
+
 if get_option('default_library') == 'both'
     error( '''
  Unsupported value "both" for "default_library" option.
diff --git a/devtools/words-case.txt b/devtools/words-case.txt
index bc35c160ba..d315fde8fe 100644
--- a/devtools/words-case.txt
+++ b/devtools/words-case.txt
@@ -105,6 +105,7 @@ TSO
 TTL
 Tx
 uAPI
+UBSan
 UDM
 UDP
 ULP
-- 
2.50.0

[PATCH v3 11/18] test/telemetry: catch errors in subshell

[David Marchand]

This script relies on subshell and pipes to prepare a list of commands
to pass to the telemetry script.

However, errors are not propagated to the parent process and the test
may still pass when an error occurs.
There is no POSIX option to cleanly catch all errors, so rely on bash
options (as some CI envs run with bash).

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/suites/test_telemetry.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/test/suites/test_telemetry.sh b/app/test/suites/test_telemetry.sh
index 20806b43e4..3c5b629b63 100755
--- a/app/test/suites/test_telemetry.sh
+++ b/app/test/suites/test_telemetry.sh
@@ -15,7 +15,7 @@ call_all_telemetry() {
     telemetry_script=$rootdir/usertools/dpdk-telemetry.py
     echo >$tmpoutput
     echo "Telemetry commands log:" >>$tmpoutput
-    for cmd in $(echo / | $telemetry_script | jq -r '.["/"][]')
+    echo / | $telemetry_script | jq -r '.["/"][]' | while read cmd
     do
         for input in $cmd $cmd,0 $cmd,z
         do
@@ -25,4 +25,6 @@ call_all_telemetry() {
     done
 }
 
+! set -o | grep -q errtrace || set -o errtrace
+! set -o | grep -q pipefail || set -o pipefail
 (sleep 1 && call_all_telemetry && echo quit) | $@
-- 
2.50.0

[PATCH v3 12/18] malloc: fix mp message alignment

[David Marchand]

Content (param[]) of received multiprocess messages are aligned with
a 4 bytes constraint.

Before patch:
struct mp_msg_internal {
 int type;                                                     /*   0     4 */
 struct rte_mp_msg {
  char name[64];                                               /*   4    64 */
  /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */
  int len_param;                                               /*  68     4 */
  int num_fds;                                                 /*  72     4 */
  /* typedef uint8_t -> __uint8_t */ unsigned char param[256]; /*  76   256 */
  /* --- cacheline 5 boundary (320 bytes) was 12 bytes ago --- */
  int fds[253];                                                /* 332  1012 */
 } msg;                                                        /*   4  1340 */

 /* size: 1344, cachelines: 21, members: 2 */
};

This results in many unaligned accesses for multiprocess malloc requests.

Examples:
../lib/eal/common/malloc_mp.c:308:32: runtime error:
	member access within misaligned address 0x7f7b35df4684 for type
	'const struct malloc_mp_req', which requires 8 byte alignment

../lib/eal/common/malloc_mp.c:158:9: runtime error:
	member access within misaligned address 0x7f36a535bb5c for type
	'const struct malloc_mp_req', which requires 8 byte alignment

../lib/eal/common/malloc_mp.c:171:8: runtime error:
	member access within misaligned address 0x7f4ba65f296c for type
	'struct malloc_mp_req', which requires 8 byte alignment

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/common/eal_common_proc.c |  2 +-
 lib/eal/common/malloc_mp.c       | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index 0dea787e38..3846c7178d 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -62,7 +62,7 @@ enum mp_type {
 
 struct mp_msg_internal {
 	int type;
-	struct rte_mp_msg msg;
+	alignas(8) struct rte_mp_msg msg;
 };
 
 struct async_request_param {
diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
index 9765277f5d..000c7f6b47 100644
--- a/lib/eal/common/malloc_mp.c
+++ b/lib/eal/common/malloc_mp.c
@@ -148,7 +148,7 @@ get_unique_id(void)
 static int
 handle_sync(const struct rte_mp_msg *msg, const void *peer)
 {
-	struct rte_mp_msg reply;
+	alignas(8) struct rte_mp_msg reply;
 	const struct malloc_mp_req *req =
 			(const struct malloc_mp_req *)msg->param;
 	struct malloc_mp_req *resp =
@@ -330,7 +330,7 @@ handle_request(const struct rte_mp_msg *msg, const void *peer __rte_unused)
 	}
 
 	if (ret != 0) {
-		struct rte_mp_msg resp_msg;
+		alignas(8) struct rte_mp_msg resp_msg;
 		struct malloc_mp_req *resp =
 				(struct malloc_mp_req *)resp_msg.param;
 
@@ -351,7 +351,7 @@ handle_request(const struct rte_mp_msg *msg, const void *peer __rte_unused)
 		/* we did not modify the request */
 		free(entry);
 	} else {
-		struct rte_mp_msg sr_msg;
+		alignas(8) struct rte_mp_msg sr_msg;
 		struct malloc_mp_req *sr =
 				(struct malloc_mp_req *)sr_msg.param;
 		struct timespec ts;
@@ -444,7 +444,7 @@ handle_sync_response(const struct rte_mp_msg *request,
 	}
 
 	if (entry->user_req.t == REQ_TYPE_FREE) {
-		struct rte_mp_msg msg;
+		alignas(8) struct rte_mp_msg msg;
 		struct malloc_mp_req *resp = (struct malloc_mp_req *)msg.param;
 
 		memset(&msg, 0, sizeof(msg));
@@ -465,7 +465,7 @@ handle_sync_response(const struct rte_mp_msg *request,
 	} else if (entry->user_req.t == REQ_TYPE_ALLOC &&
 			result == REQ_RESULT_SUCCESS) {
 		struct malloc_heap *heap = entry->alloc_state.heap;
-		struct rte_mp_msg msg;
+		alignas(8) struct rte_mp_msg msg;
 		struct malloc_mp_req *resp =
 				(struct malloc_mp_req *)msg.param;
 
@@ -489,7 +489,7 @@ handle_sync_response(const struct rte_mp_msg *request,
 		free(entry);
 	} else if (entry->user_req.t == REQ_TYPE_ALLOC &&
 			result == REQ_RESULT_FAIL) {
-		struct rte_mp_msg rb_msg;
+		alignas(8) struct rte_mp_msg rb_msg;
 		struct malloc_mp_req *rb =
 				(struct malloc_mp_req *)rb_msg.param;
 		struct timespec ts;
@@ -551,7 +551,7 @@ static int
 handle_rollback_response(const struct rte_mp_msg *request,
 		const struct rte_mp_reply *reply __rte_unused)
 {
-	struct rte_mp_msg msg;
+	alignas(8) struct rte_mp_msg msg;
 	struct malloc_mp_req *resp = (struct malloc_mp_req *)msg.param;
 	const struct malloc_mp_req *mpreq =
 			(const struct malloc_mp_req *)request->param;
@@ -628,7 +628,7 @@ handle_response(const struct rte_mp_msg *msg, const void *peer  __rte_unused)
 int
 request_sync(void)
 {
-	struct rte_mp_msg msg;
+	alignas(8) struct rte_mp_msg msg;
 	struct rte_mp_reply reply;
 	struct malloc_mp_req *req = (struct malloc_mp_req *)msg.param;
 	struct timespec ts;
@@ -697,7 +697,7 @@ request_sync(void)
 int
 request_to_primary(struct malloc_mp_req *user_req)
 {
-	struct rte_mp_msg msg;
+	alignas(8) struct rte_mp_msg msg;
 	struct malloc_mp_req *msg_req = (struct malloc_mp_req *)msg.param;
 	struct mp_request *entry;
 	struct timespec ts;
-- 
2.50.0

Re: [PATCH v3 12/18] malloc: fix mp message alignment

[Bruce Richardson]

On Tue, Jul 08, 2025 at 02:28:16PM +0200, David Marchand wrote:
> Content (param[]) of received multiprocess messages are aligned with
> a 4 bytes constraint.
> 
> Before patch:
> struct mp_msg_internal {
>  int type;                                                     /*   0     4 */
>  struct rte_mp_msg {
>   char name[64];                                               /*   4    64 */
>   /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */
>   int len_param;                                               /*  68     4 */
>   int num_fds;                                                 /*  72     4 */
>   /* typedef uint8_t -> __uint8_t */ unsigned char param[256]; /*  76   256 */
>   /* --- cacheline 5 boundary (320 bytes) was 12 bytes ago --- */
>   int fds[253];                                                /* 332  1012 */
>  } msg;                                                        /*   4  1340 */
> 
>  /* size: 1344, cachelines: 21, members: 2 */
> };
> 
> This results in many unaligned accesses for multiprocess malloc requests.
> 
> Examples:
> ../lib/eal/common/malloc_mp.c:308:32: runtime error:
> 	member access within misaligned address 0x7f7b35df4684 for type
> 	'const struct malloc_mp_req', which requires 8 byte alignment
> 
> ../lib/eal/common/malloc_mp.c:158:9: runtime error:
> 	member access within misaligned address 0x7f36a535bb5c for type
> 	'const struct malloc_mp_req', which requires 8 byte alignment
> 
> ../lib/eal/common/malloc_mp.c:171:8: runtime error:
> 	member access within misaligned address 0x7f4ba65f296c for type
> 	'struct malloc_mp_req', which requires 8 byte alignment
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/eal/common/eal_common_proc.c |  2 +-
>  lib/eal/common/malloc_mp.c       | 18 +++++++++---------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
> index 0dea787e38..3846c7178d 100644
> --- a/lib/eal/common/eal_common_proc.c
> +++ b/lib/eal/common/eal_common_proc.c
> @@ -62,7 +62,7 @@ enum mp_type {
>  
>  struct mp_msg_internal {
>  	int type;
> -	struct rte_mp_msg msg;
> +	alignas(8) struct rte_mp_msg msg;
>  };
>  
>  struct async_request_param {
> diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
> index 9765277f5d..000c7f6b47 100644
> --- a/lib/eal/common/malloc_mp.c
> +++ b/lib/eal/common/malloc_mp.c
> @@ -148,7 +148,7 @@ get_unique_id(void)
>  static int
>  handle_sync(const struct rte_mp_msg *msg, const void *peer)
>  {
> -	struct rte_mp_msg reply;
> +	alignas(8) struct rte_mp_msg reply;
>  	const struct malloc_mp_req *req =
>  			(const struct malloc_mp_req *)msg->param;

This patch seems to have a lot of these definitions with alignas added to
them. Would it be simpler just to put the alignas inside the rte_mp_msg
definition?

More specifically, if its the "uint8_t param" element that needs alignment,
how about changing that specific field to make it aligned?

/Bruce

Re: [PATCH v3 12/18] malloc: fix mp message alignment

[David Marchand]

On Tue, Jul 8, 2025 at 2:44 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Jul 08, 2025 at 02:28:16PM +0200, David Marchand wrote:
> > Content (param[]) of received multiprocess messages are aligned with
> > a 4 bytes constraint.
> >
> > Before patch:
> > struct mp_msg_internal {
> >  int type;                                                     /*   0     4 */
> >  struct rte_mp_msg {
> >   char name[64];                                               /*   4    64 */
> >   /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */
> >   int len_param;                                               /*  68     4 */
> >   int num_fds;                                                 /*  72     4 */
> >   /* typedef uint8_t -> __uint8_t */ unsigned char param[256]; /*  76   256 */
> >   /* --- cacheline 5 boundary (320 bytes) was 12 bytes ago --- */
> >   int fds[253];                                                /* 332  1012 */
> >  } msg;                                                        /*   4  1340 */
> >
> >  /* size: 1344, cachelines: 21, members: 2 */
> > };
> >
> > This results in many unaligned accesses for multiprocess malloc requests.
> >
> > Examples:
> > ../lib/eal/common/malloc_mp.c:308:32: runtime error:
> >       member access within misaligned address 0x7f7b35df4684 for type
> >       'const struct malloc_mp_req', which requires 8 byte alignment
> >
> > ../lib/eal/common/malloc_mp.c:158:9: runtime error:
> >       member access within misaligned address 0x7f36a535bb5c for type
> >       'const struct malloc_mp_req', which requires 8 byte alignment
> >
> > ../lib/eal/common/malloc_mp.c:171:8: runtime error:
> >       member access within misaligned address 0x7f4ba65f296c for type
> >       'struct malloc_mp_req', which requires 8 byte alignment
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  lib/eal/common/eal_common_proc.c |  2 +-
> >  lib/eal/common/malloc_mp.c       | 18 +++++++++---------
> >  2 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
> > index 0dea787e38..3846c7178d 100644
> > --- a/lib/eal/common/eal_common_proc.c
> > +++ b/lib/eal/common/eal_common_proc.c
> > @@ -62,7 +62,7 @@ enum mp_type {
> >
> >  struct mp_msg_internal {
> >       int type;
> > -     struct rte_mp_msg msg;
> > +     alignas(8) struct rte_mp_msg msg;
> >  };
> >
> >  struct async_request_param {
> > diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
> > index 9765277f5d..000c7f6b47 100644
> > --- a/lib/eal/common/malloc_mp.c
> > +++ b/lib/eal/common/malloc_mp.c
> > @@ -148,7 +148,7 @@ get_unique_id(void)
> >  static int
> >  handle_sync(const struct rte_mp_msg *msg, const void *peer)
> >  {
> > -     struct rte_mp_msg reply;
> > +     alignas(8) struct rte_mp_msg reply;
> >       const struct malloc_mp_req *req =
> >                       (const struct malloc_mp_req *)msg->param;
>
> This patch seems to have a lot of these definitions with alignas added to
> them. Would it be simpler just to put the alignas inside the rte_mp_msg
> definition?
>
> More specifically, if its the "uint8_t param" element that needs alignment,
> how about changing that specific field to make it aligned?

We could probably enhance this, but I expect this breaks ABI.

This could be done during 25.11.


-- 
David Marchand

Re: [PATCH v3 12/18] malloc: fix mp message alignment

[Bruce Richardson]

On Tue, Jul 08, 2025 at 02:46:04PM +0200, David Marchand wrote:
> On Tue, Jul 8, 2025 at 2:44 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Tue, Jul 08, 2025 at 02:28:16PM +0200, David Marchand wrote:
> > > Content (param[]) of received multiprocess messages are aligned with
> > > a 4 bytes constraint.
> > >
> > > Before patch:
> > > struct mp_msg_internal {
> > >  int type;                                                     /*   0     4 */
> > >  struct rte_mp_msg {
> > >   char name[64];                                               /*   4    64 */
> > >   /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */
> > >   int len_param;                                               /*  68     4 */
> > >   int num_fds;                                                 /*  72     4 */
> > >   /* typedef uint8_t -> __uint8_t */ unsigned char param[256]; /*  76   256 */
> > >   /* --- cacheline 5 boundary (320 bytes) was 12 bytes ago --- */
> > >   int fds[253];                                                /* 332  1012 */
> > >  } msg;                                                        /*   4  1340 */
> > >
> > >  /* size: 1344, cachelines: 21, members: 2 */
> > > };
> > >
> > > This results in many unaligned accesses for multiprocess malloc requests.
> > >
> > > Examples:
> > > ../lib/eal/common/malloc_mp.c:308:32: runtime error:
> > >       member access within misaligned address 0x7f7b35df4684 for type
> > >       'const struct malloc_mp_req', which requires 8 byte alignment
> > >
> > > ../lib/eal/common/malloc_mp.c:158:9: runtime error:
> > >       member access within misaligned address 0x7f36a535bb5c for type
> > >       'const struct malloc_mp_req', which requires 8 byte alignment
> > >
> > > ../lib/eal/common/malloc_mp.c:171:8: runtime error:
> > >       member access within misaligned address 0x7f4ba65f296c for type
> > >       'struct malloc_mp_req', which requires 8 byte alignment
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  lib/eal/common/eal_common_proc.c |  2 +-
> > >  lib/eal/common/malloc_mp.c       | 18 +++++++++---------
> > >  2 files changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
> > > index 0dea787e38..3846c7178d 100644
> > > --- a/lib/eal/common/eal_common_proc.c
> > > +++ b/lib/eal/common/eal_common_proc.c
> > > @@ -62,7 +62,7 @@ enum mp_type {
> > >
> > >  struct mp_msg_internal {
> > >       int type;
> > > -     struct rte_mp_msg msg;
> > > +     alignas(8) struct rte_mp_msg msg;
> > >  };
> > >
> > >  struct async_request_param {
> > > diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
> > > index 9765277f5d..000c7f6b47 100644
> > > --- a/lib/eal/common/malloc_mp.c
> > > +++ b/lib/eal/common/malloc_mp.c
> > > @@ -148,7 +148,7 @@ get_unique_id(void)
> > >  static int
> > >  handle_sync(const struct rte_mp_msg *msg, const void *peer)
> > >  {
> > > -     struct rte_mp_msg reply;
> > > +     alignas(8) struct rte_mp_msg reply;
> > >       const struct malloc_mp_req *req =
> > >                       (const struct malloc_mp_req *)msg->param;
> >
> > This patch seems to have a lot of these definitions with alignas added to
> > them. Would it be simpler just to put the alignas inside the rte_mp_msg
> > definition?
> >
> > More specifically, if its the "uint8_t param" element that needs alignment,
> > how about changing that specific field to make it aligned?
> 
> We could probably enhance this, but I expect this breaks ABI.
> 
Is the multi-process message type part of the public ABI? I don't believe
we ever guaranteed multiprocess support working across versions of DPDK.

/Bruce

Re: [PATCH v3 12/18] malloc: fix mp message alignment

[David Marchand]

On Tue, Jul 8, 2025 at 3:26 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Jul 08, 2025 at 02:46:04PM +0200, David Marchand wrote:
> > On Tue, Jul 8, 2025 at 2:44 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Tue, Jul 08, 2025 at 02:28:16PM +0200, David Marchand wrote:
> > > > Content (param[]) of received multiprocess messages are aligned with
> > > > a 4 bytes constraint.
> > > >
> > > > Before patch:
> > > > struct mp_msg_internal {
> > > >  int type;                                                     /*   0     4 */
> > > >  struct rte_mp_msg {
> > > >   char name[64];                                               /*   4    64 */
> > > >   /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */
> > > >   int len_param;                                               /*  68     4 */
> > > >   int num_fds;                                                 /*  72     4 */
> > > >   /* typedef uint8_t -> __uint8_t */ unsigned char param[256]; /*  76   256 */
> > > >   /* --- cacheline 5 boundary (320 bytes) was 12 bytes ago --- */
> > > >   int fds[253];                                                /* 332  1012 */
> > > >  } msg;                                                        /*   4  1340 */
> > > >
> > > >  /* size: 1344, cachelines: 21, members: 2 */
> > > > };
> > > >
> > > > This results in many unaligned accesses for multiprocess malloc requests.
> > > >
> > > > Examples:
> > > > ../lib/eal/common/malloc_mp.c:308:32: runtime error:
> > > >       member access within misaligned address 0x7f7b35df4684 for type
> > > >       'const struct malloc_mp_req', which requires 8 byte alignment
> > > >
> > > > ../lib/eal/common/malloc_mp.c:158:9: runtime error:
> > > >       member access within misaligned address 0x7f36a535bb5c for type
> > > >       'const struct malloc_mp_req', which requires 8 byte alignment
> > > >
> > > > ../lib/eal/common/malloc_mp.c:171:8: runtime error:
> > > >       member access within misaligned address 0x7f4ba65f296c for type
> > > >       'struct malloc_mp_req', which requires 8 byte alignment
> > > >
> > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > ---
> > > >  lib/eal/common/eal_common_proc.c |  2 +-
> > > >  lib/eal/common/malloc_mp.c       | 18 +++++++++---------
> > > >  2 files changed, 10 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
> > > > index 0dea787e38..3846c7178d 100644
> > > > --- a/lib/eal/common/eal_common_proc.c
> > > > +++ b/lib/eal/common/eal_common_proc.c
> > > > @@ -62,7 +62,7 @@ enum mp_type {
> > > >
> > > >  struct mp_msg_internal {
> > > >       int type;
> > > > -     struct rte_mp_msg msg;
> > > > +     alignas(8) struct rte_mp_msg msg;
> > > >  };
> > > >
> > > >  struct async_request_param {
> > > > diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
> > > > index 9765277f5d..000c7f6b47 100644
> > > > --- a/lib/eal/common/malloc_mp.c
> > > > +++ b/lib/eal/common/malloc_mp.c
> > > > @@ -148,7 +148,7 @@ get_unique_id(void)
> > > >  static int
> > > >  handle_sync(const struct rte_mp_msg *msg, const void *peer)
> > > >  {
> > > > -     struct rte_mp_msg reply;
> > > > +     alignas(8) struct rte_mp_msg reply;
> > > >       const struct malloc_mp_req *req =
> > > >                       (const struct malloc_mp_req *)msg->param;
> > >
> > > This patch seems to have a lot of these definitions with alignas added to
> > > them. Would it be simpler just to put the alignas inside the rte_mp_msg
> > > definition?
> > >
> > > More specifically, if its the "uint8_t param" element that needs alignment,
> > > how about changing that specific field to make it aligned?
> >
> > We could probably enhance this, but I expect this breaks ABI.
> >
> Is the multi-process message type part of the public ABI? I don't believe
> we ever guaranteed multiprocess support working across versions of DPDK.

An application can register its own callbacks which take a rte_mp_msg as input.
Hum... would it really be an ABI breakage though... need to think.

At least I tested, and the ABI check is complaining.


-- 
David Marchand

[PATCH v3 13/18] graph: fix stats query with no node xstats

[David Marchand]

From: Marat Khalili <marat.khalili@huawei.com>

This was flagged by undefined behaviour sanitizer: memset should not be
called with NULL first argument. (memset requires first argument to be
pointer to a memory object, so passing NULL may result in an undefined
behaviour including among other things optimizer potentially removing
code paths depending on stat->xstat_count being NULL.)

Sanitizer message:

    lib/graph/graph_stats.c:473:2: runtime error: null pointer passed as
    argument 1, which is declared to never be null

Add a check that stat->xstat_cntrs is not zero before the call, since
stat->xstat_count can only be NULL when stat->xstat_cntrs is zero.

Fixes: 070db97e017b ("graph: support node xstats")
Cc: stable@dpdk.org

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 lib/graph/graph_stats.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c
index eac73cbf71..583ad8dbd5 100644
--- a/lib/graph/graph_stats.c
+++ b/lib/graph/graph_stats.c
@@ -470,7 +470,8 @@ cluster_node_arregate_stats(struct cluster_node *cluster, bool dispatch)
 	uint64_t *xstat;
 	uint8_t i;
 
-	memset(stat->xstat_count, 0, sizeof(uint64_t) * stat->xstat_cntrs);
+	if (stat->xstat_cntrs != 0)
+		memset(stat->xstat_count, 0, sizeof(uint64_t) * stat->xstat_cntrs);
 	for (count = 0; count < cluster->nb_nodes; count++) {
 		node = cluster->nodes[count];
 
-- 
2.50.0

[PATCH v3 14/18] graph: fix unaligned access in stats

[David Marchand]

UBSan reports:

    ../lib/graph/graph_stats.c:208:13: runtime error:
            member access within misaligned address 0x000054742c50
            for type 'struct rte_graph_cluster_stats',
            which requires 64 byte alignment

    ../lib/graph/graph_stats.c:257:12: runtime error:
            member access within misaligned address 0x00002219fd30
            for type 'struct rte_graph_cluster_stats',
            which requires 64 byte alignment

The current code goes into various complex (non aligned) reallocations /
memset / memcpy.

Simplify this by computing how many nodes are present in the
cluster of graphes.
Then directly call rte_malloc for the whole stats object.

As a bonus, this change also fixes leaks:
- if any error occurred before call to rte_malloc, since the xstats
  objects stored in the glibc allocated stats object were not freed,
- if an allocation failure occurs, with constructs using
  ptr = realloc(ptr, sz), since the original ptr is lost,

Fixes: af1ae8b6a32c ("graph: implement stats")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/graph/graph_stats.c | 102 +++++++++++++++++++++++-----------------
 1 file changed, 58 insertions(+), 44 deletions(-)

diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c
index 583ad8dbd5..e9b183bf13 100644
--- a/lib/graph/graph_stats.c
+++ b/lib/graph/graph_stats.c
@@ -37,7 +37,6 @@ struct __rte_cache_aligned rte_graph_cluster_stats {
 	int socket_id;
 	bool dispatch;
 	void *cookie;
-	size_t sz;
 
 	struct cluster_node clusters[];
 };
@@ -178,15 +177,55 @@ graph_cluster_stats_cb_dispatch(bool is_first, bool is_last, void *cookie,
 	return graph_cluster_stats_cb(true, is_first, is_last, cookie, stat);
 };
 
+static uint32_t
+cluster_count_nodes(const struct cluster *cluster)
+{
+	rte_node_t *nodes = NULL;
+	uint32_t max_nodes = 0;
+
+	for (unsigned int i = 0; i < cluster->nb_graphs; i++) {
+		struct graph_node *graph_node;
+
+		STAILQ_FOREACH(graph_node, &cluster->graphs[i]->node_list, next) {
+			unsigned int n;
+
+			for (n = 0; n < max_nodes; n++) {
+				if (nodes[n] != graph_node->node->id)
+					continue;
+				break;
+			}
+			if (n == max_nodes) {
+				rte_node_t *new_nodes;
+
+				max_nodes++;
+				new_nodes = realloc(nodes, max_nodes * sizeof(nodes[0]));
+				if (new_nodes == NULL) {
+					free(nodes);
+					return 0;
+				}
+				nodes = new_nodes;
+				nodes[n] = graph_node->node->id;
+			}
+		}
+	}
+	free(nodes);
+
+	return max_nodes;
+}
+
 static struct rte_graph_cluster_stats *
 stats_mem_init(struct cluster *cluster,
 	       const struct rte_graph_cluster_stats_param *prm)
 {
-	size_t sz = sizeof(struct rte_graph_cluster_stats);
 	struct rte_graph_cluster_stats *stats;
 	rte_graph_cluster_stats_cb_t fn;
 	int socket_id = prm->socket_id;
 	uint32_t cluster_node_size;
+	uint32_t max_nodes;
+
+	max_nodes = cluster_count_nodes(cluster);
+	if (max_nodes == 0)
+		return NULL;
 
 	/* Fix up callback */
 	fn = prm->fn;
@@ -203,25 +242,23 @@ stats_mem_init(struct cluster *cluster,
 	cluster_node_size += cluster->nb_graphs * sizeof(struct rte_node *);
 	cluster_node_size = RTE_ALIGN(cluster_node_size, RTE_CACHE_LINE_SIZE);
 
-	stats = realloc(NULL, sz);
+	stats = rte_zmalloc_socket(NULL, sizeof(struct rte_graph_cluster_stats) +
+		max_nodes * cluster_node_size, 0, socket_id);
 	if (stats) {
-		memset(stats, 0, sz);
 		stats->fn = fn;
 		stats->cluster_node_size = cluster_node_size;
 		stats->max_nodes = 0;
 		stats->socket_id = socket_id;
 		stats->cookie = prm->cookie;
-		stats->sz = sz;
 	}
 
 	return stats;
 }
 
 static int
-stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
+stats_mem_populate(struct rte_graph_cluster_stats *stats,
 		   struct rte_graph *graph, struct graph_node *graph_node)
 {
-	struct rte_graph_cluster_stats *stats = *stats_in;
 	rte_node_t id = graph_node->node->id;
 	struct cluster_node *cluster;
 	struct rte_node *node;
@@ -247,21 +284,12 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
 		cluster = RTE_PTR_ADD(cluster, stats->cluster_node_size);
 	}
 
-	/* Hey, it is a new node, allocate space for it in the reel */
-	stats = realloc(stats, stats->sz + stats->cluster_node_size);
-	if (stats == NULL)
-		SET_ERR_JMP(ENOMEM, err, "Realloc failed");
-	*stats_in = NULL;
-
-	/* Clear the new struct cluster_node area */
-	cluster = RTE_PTR_ADD(stats, stats->sz),
-	memset(cluster, 0, stats->cluster_node_size);
 	memcpy(cluster->stat.name, graph_node->node->name, RTE_NODE_NAMESIZE);
 	cluster->stat.id = graph_node->node->id;
 	cluster->stat.hz = rte_get_timer_hz();
 	node = graph_node_id_to_ptr(graph, id);
 	if (node == NULL)
-		SET_ERR_JMP(ENOENT, free, "Failed to find node %s in graph %s",
+		SET_ERR_JMP(ENOENT, err, "Failed to find node %s in graph %s",
 			    graph_node->node->name, graph->name);
 	cluster->nodes[cluster->nb_nodes++] = node;
 	if (graph_node->node->xstats) {
@@ -270,7 +298,7 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
 			sizeof(uint64_t) * graph_node->node->xstats->nb_xstats,
 			RTE_CACHE_LINE_SIZE, stats->socket_id);
 		if (cluster->stat.xstat_count == NULL)
-			SET_ERR_JMP(ENOMEM, free, "Failed to allocate memory node %s graph %s",
+			SET_ERR_JMP(ENOMEM, err, "Failed to allocate memory node %s graph %s",
 				    graph_node->node->name, graph->name);
 
 		cluster->stat.xstat_desc = rte_zmalloc_socket(NULL,
@@ -278,7 +306,7 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
 			RTE_CACHE_LINE_SIZE, stats->socket_id);
 		if (cluster->stat.xstat_desc == NULL) {
 			rte_free(cluster->stat.xstat_count);
-			SET_ERR_JMP(ENOMEM, free, "Failed to allocate memory node %s graph %s",
+			SET_ERR_JMP(ENOMEM, err, "Failed to allocate memory node %s graph %s",
 				    graph_node->node->name, graph->name);
 		}
 
@@ -288,30 +316,20 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
 					RTE_NODE_XSTAT_DESC_SIZE) < 0) {
 				rte_free(cluster->stat.xstat_count);
 				rte_free(cluster->stat.xstat_desc);
-				SET_ERR_JMP(E2BIG, free,
+				SET_ERR_JMP(E2BIG, err,
 					    "Error description overflow node %s graph %s",
 					    graph_node->node->name, graph->name);
 			}
 		}
 	}
 
-	stats->sz += stats->cluster_node_size;
 	stats->max_nodes++;
-	*stats_in = stats;
 
 	return 0;
-free:
-	free(stats);
 err:
 	return -rte_errno;
 }
 
-static void
-stats_mem_fini(struct rte_graph_cluster_stats *stats)
-{
-	free(stats);
-}
-
 static void
 cluster_init(struct cluster *cluster)
 {
@@ -381,10 +399,7 @@ struct rte_graph_cluster_stats *
 rte_graph_cluster_stats_create(const struct rte_graph_cluster_stats_param *prm)
 {
 	struct rte_graph_cluster_stats *stats, *rc = NULL;
-	struct graph_node *graph_node;
 	struct cluster cluster;
-	struct graph *graph;
-	const char *pattern;
 	rte_graph_t i;
 
 	/* Sanity checks */
@@ -402,37 +417,36 @@ rte_graph_cluster_stats_create(const struct rte_graph_cluster_stats_param *prm)
 	graph_spinlock_lock();
 	/* Expand graph pattern and add the graph to the cluster */
 	for (i = 0; i < prm->nb_graph_patterns; i++) {
-		pattern = prm->graph_patterns[i];
-		if (expand_pattern_to_cluster(&cluster, pattern))
+		if (expand_pattern_to_cluster(&cluster, prm->graph_patterns[i]))
 			goto bad_pattern;
 	}
 
 	/* Alloc the stats memory */
 	stats = stats_mem_init(&cluster, prm);
 	if (stats == NULL)
-		SET_ERR_JMP(ENOMEM, bad_pattern, "Failed alloc stats memory");
+		SET_ERR_JMP(ENOMEM, bad_pattern, "Failed rte_malloc for stats memory");
 
 	/* Iterate over M(Graph) x N (Nodes in graph) */
 	for (i = 0; i < cluster.nb_graphs; i++) {
+		struct graph_node *graph_node;
+		struct graph *graph;
+
 		graph = cluster.graphs[i];
 		STAILQ_FOREACH(graph_node, &graph->node_list, next) {
 			struct rte_graph *graph_fp = graph->graph;
-			if (stats_mem_populate(&stats, graph_fp, graph_node))
+			if (stats_mem_populate(stats, graph_fp, graph_node))
 				goto realloc_fail;
 		}
 		if (graph->graph->model == RTE_GRAPH_MODEL_MCORE_DISPATCH)
 			stats->dispatch = true;
 	}
 
-	/* Finally copy to hugepage memory to avoid pressure on rte_realloc */
-	rc = rte_malloc_socket(NULL, stats->sz, 0, stats->socket_id);
-	if (rc)
-		rte_memcpy(rc, stats, stats->sz);
-	else
-		SET_ERR_JMP(ENOMEM, realloc_fail, "rte_malloc failed");
+	rc = stats;
+	stats = NULL;
 
 realloc_fail:
-	stats_mem_fini(stats);
+	if (stats != NULL)
+		rte_graph_cluster_stats_destroy(stats);
 bad_pattern:
 	graph_spinlock_unlock();
 	cluster_fini(&cluster);
-- 
2.50.0

[PATCH v3 15/18] eventdev: fix listing timer adapters with telemetry

[David Marchand]

If no timer adapter is created, listing with telemetry can lead to crash
and is reported by UBSan as an undefined behavior:

../lib/eventdev/rte_event_timer_adapter.c:1418:13:
	runtime error: applying zero offset to null pointer

../lib/eventdev/rte_event_timer_adapter.c:1464:13:
	runtime error: applying zero offset to null pointer

Fixes: 791dfec24d00 ("eventdev/timer: add telemetry")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eventdev/rte_event_timer_adapter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index 06ce478d90..af98b1d9f6 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -1410,7 +1410,7 @@ handle_ta_info(const char *cmd __rte_unused, const char *params,
 
 	adapter_id = atoi(params);
 
-	if (adapter_id >= RTE_EVENT_TIMER_ADAPTER_NUM_MAX) {
+	if (adapters == NULL || adapter_id >= RTE_EVENT_TIMER_ADAPTER_NUM_MAX) {
 		EVTIM_LOG_ERR("Invalid timer adapter id %u", adapter_id);
 		return -EINVAL;
 	}
@@ -1456,7 +1456,7 @@ handle_ta_stats(const char *cmd __rte_unused, const char *params,
 
 	adapter_id = atoi(params);
 
-	if (adapter_id >= RTE_EVENT_TIMER_ADAPTER_NUM_MAX) {
+	if (adapters == NULL || adapter_id >= RTE_EVENT_TIMER_ADAPTER_NUM_MAX) {
 		EVTIM_LOG_ERR("Invalid timer adapter id %u", adapter_id);
 		return -EINVAL;
 	}
-- 
2.50.0

[PATCH v3 16/18] test/power: fix tests without power drivers

[David Marchand]

In the absence of drivers, skip tests instead of failing.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 7d38f51918..79d635b42b 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -145,10 +145,12 @@ source_file_deps = {
     'test_pmd_ring.c': ['net_ring', 'ethdev', 'bus_vdev'],
     'test_pmd_ring_perf.c': ['ethdev', 'net_ring', 'bus_vdev'],
     'test_pmu.c': ['pmu'],
-    'test_power.c': ['power'],
-    'test_power_cpufreq.c': ['power'],
-    'test_power_intel_uncore.c': ['power'],
-    'test_power_kvm_vm.c': ['power'],
+    'test_power.c': ['power', 'power_acpi', 'power_kvm_vm', 'power_intel_pstate',
+        'power_amd_pstate', 'power_cppc'],
+    'test_power_cpufreq.c': ['power', 'power_acpi', 'power_intel_pstate', 'power_amd_pstate',
+        'power_cppc'],
+    'test_power_intel_uncore.c': ['power', 'power_intel_uncore'],
+    'test_power_kvm_vm.c': ['power', 'power_kvm_vm'],
     'test_prefetch.c': [],
     'test_ptr_compress.c': ['ptr_compress'],
     'test_rand_perf.c': [],
-- 
2.50.0

Re: [PATCH v3 16/18] test/power: fix tests without power drivers

[Bruce Richardson]

On Tue, Jul 08, 2025 at 02:28:20PM +0200, David Marchand wrote:
> In the absence of drivers, skip tests instead of failing.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  app/test/meson.build | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 7d38f51918..79d635b42b 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -145,10 +145,12 @@ source_file_deps = {
>      'test_pmd_ring.c': ['net_ring', 'ethdev', 'bus_vdev'],
>      'test_pmd_ring_perf.c': ['ethdev', 'net_ring', 'bus_vdev'],
>      'test_pmu.c': ['pmu'],
> -    'test_power.c': ['power'],
> -    'test_power_cpufreq.c': ['power'],
> -    'test_power_intel_uncore.c': ['power'],
> -    'test_power_kvm_vm.c': ['power'],
> +    'test_power.c': ['power', 'power_acpi', 'power_kvm_vm', 'power_intel_pstate',
> +        'power_amd_pstate', 'power_cppc'],

Is this better done at build time or at runtime? Unfortunately we don't
have support for "or" operations on dependencies, so if even one driver is
missing the whole test file will not be built. I would think it better to
look for the drivers at runtime and return TEST_SKIPPED if not present.
WDYT?

> +    'test_power_cpufreq.c': ['power', 'power_acpi', 'power_intel_pstate', 'power_amd_pstate',
> +        'power_cppc'],
> +    'test_power_intel_uncore.c': ['power', 'power_intel_uncore'],
> +    'test_power_kvm_vm.c': ['power', 'power_kvm_vm'],
>      'test_prefetch.c': [],
>      'test_ptr_compress.c': ['ptr_compress'],
>      'test_rand_perf.c': [],
> -- 
> 2.50.0
> 

Re: [PATCH v3 16/18] test/power: fix tests without power drivers

[David Marchand]

On Tue, Jul 8, 2025 at 2:48 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Jul 08, 2025 at 02:28:20PM +0200, David Marchand wrote:
> > In the absence of drivers, skip tests instead of failing.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  app/test/meson.build | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test/meson.build b/app/test/meson.build
> > index 7d38f51918..79d635b42b 100644
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > @@ -145,10 +145,12 @@ source_file_deps = {
> >      'test_pmd_ring.c': ['net_ring', 'ethdev', 'bus_vdev'],
> >      'test_pmd_ring_perf.c': ['ethdev', 'net_ring', 'bus_vdev'],
> >      'test_pmu.c': ['pmu'],
> > -    'test_power.c': ['power'],
> > -    'test_power_cpufreq.c': ['power'],
> > -    'test_power_intel_uncore.c': ['power'],
> > -    'test_power_kvm_vm.c': ['power'],
> > +    'test_power.c': ['power', 'power_acpi', 'power_kvm_vm', 'power_intel_pstate',
> > +        'power_amd_pstate', 'power_cppc'],
>
> Is this better done at build time or at runtime? Unfortunately we don't
> have support for "or" operations on dependencies, so if even one driver is
> missing the whole test file will not be built. I would think it better to
> look for the drivers at runtime and return TEST_SKIPPED if not present.
> WDYT?

I don't understand the intent behind this test, but test_power.c
requires this list of drivers to be present.

        /* Perform tests for valid environments.*/
        const enum power_management_env envs[] = {PM_ENV_ACPI_CPUFREQ,
                        PM_ENV_KVM_VM,
                        PM_ENV_PSTATE_CPUFREQ,
                        PM_ENV_AMD_PSTATE_CPUFREQ,
                        PM_ENV_CPPC_CPUFREQ};

        unsigned int i;
        for (i = 0; i < RTE_DIM(envs); ++i) {

                /* Test setting a valid environment */
                ret = rte_power_set_env(envs[i]);

From this, I chose to disable all other unit tests.


-- 
David Marchand

Re: [PATCH v3 16/18] test/power: fix tests without power drivers

[Bruce Richardson]

On Tue, Jul 08, 2025 at 02:53:03PM +0200, David Marchand wrote:
> On Tue, Jul 8, 2025 at 2:48 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Tue, Jul 08, 2025 at 02:28:20PM +0200, David Marchand wrote:
> > > In the absence of drivers, skip tests instead of failing.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  app/test/meson.build | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/app/test/meson.build b/app/test/meson.build
> > > index 7d38f51918..79d635b42b 100644
> > > --- a/app/test/meson.build
> > > +++ b/app/test/meson.build
> > > @@ -145,10 +145,12 @@ source_file_deps = {
> > >      'test_pmd_ring.c': ['net_ring', 'ethdev', 'bus_vdev'],
> > >      'test_pmd_ring_perf.c': ['ethdev', 'net_ring', 'bus_vdev'],
> > >      'test_pmu.c': ['pmu'],
> > > -    'test_power.c': ['power'],
> > > -    'test_power_cpufreq.c': ['power'],
> > > -    'test_power_intel_uncore.c': ['power'],
> > > -    'test_power_kvm_vm.c': ['power'],
> > > +    'test_power.c': ['power', 'power_acpi', 'power_kvm_vm', 'power_intel_pstate',
> > > +        'power_amd_pstate', 'power_cppc'],
> >
> > Is this better done at build time or at runtime? Unfortunately we don't
> > have support for "or" operations on dependencies, so if even one driver is
> > missing the whole test file will not be built. I would think it better to
> > look for the drivers at runtime and return TEST_SKIPPED if not present.
> > WDYT?
> 
> I don't understand the intent behind this test, but test_power.c
> requires this list of drivers to be present.
> 
>         /* Perform tests for valid environments.*/
>         const enum power_management_env envs[] = {PM_ENV_ACPI_CPUFREQ,
>                         PM_ENV_KVM_VM,
>                         PM_ENV_PSTATE_CPUFREQ,
>                         PM_ENV_AMD_PSTATE_CPUFREQ,
>                         PM_ENV_CPPC_CPUFREQ};
> 
>         unsigned int i;
>         for (i = 0; i < RTE_DIM(envs); ++i) {
> 
>                 /* Test setting a valid environment */
>                 ret = rte_power_set_env(envs[i]);
> 
> From this, I chose to disable all other unit tests.
> 
Ok, if that's what it needs, so be it.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v3 17/18] test/raw: fix test without skeleton driver

[David Marchand]

Skip the test in the absence of the required skeleton driver.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 79d635b42b..8df8d3edd1 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -154,7 +154,7 @@ source_file_deps = {
     'test_prefetch.c': [],
     'test_ptr_compress.c': ['ptr_compress'],
     'test_rand_perf.c': [],
-    'test_rawdev.c': ['rawdev', 'bus_vdev'],
+    'test_rawdev.c': ['rawdev', 'bus_vdev', 'raw_skeleton'],
     'test_rcu_qsbr.c': ['rcu', 'hash'],
     'test_rcu_qsbr_perf.c': ['rcu', 'hash'],
     'test_reassembly_perf.c': ['net', 'ip_frag'],
-- 
2.50.0

Re: [PATCH v3 17/18] test/raw: fix test without skeleton driver

[Bruce Richardson]

On Tue, Jul 08, 2025 at 02:28:21PM +0200, David Marchand wrote:
> Skip the test in the absence of the required skeleton driver.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  app/test/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 79d635b42b..8df8d3edd1 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -154,7 +154,7 @@ source_file_deps = {
>      'test_prefetch.c': [],
>      'test_ptr_compress.c': ['ptr_compress'],
>      'test_rand_perf.c': [],
> -    'test_rawdev.c': ['rawdev', 'bus_vdev'],
> +    'test_rawdev.c': ['rawdev', 'bus_vdev', 'raw_skeleton'],
>      'test_rcu_qsbr.c': ['rcu', 'hash'],
>      'test_rcu_qsbr_perf.c': ['rcu', 'hash'],
>      'test_reassembly_perf.c': ['net', 'ip_frag'],
> -- 
> 2.50.0
> 

[PATCH v3 18/18] ci: extend coverage with UBSan

[David Marchand]

Enable more libraries, but exclude the ones with currently failing
unit tests.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .ci/linux-build.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index a24820c65b..1b0e01d395 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -139,7 +139,7 @@ if [ "$UBSAN" = "true" ]; then
         # Restrict drivers to a minimum set for now
         OPTS="$OPTS -Denable_drivers=net/null"
         # There are still known issues in various libraries, disable them for now
-        OPTS="$OPTS -Ddisable_libs=*"
+        OPTS="$OPTS -Ddisable_libs=acl,bpf,cfgfile,distributor,fib,lpm,member,ptr_compress,rib,table"
         # No examples are run
         OPTS="$OPTS -Dexamples="
         # The UBSan target takes a good amount of time and headers coverage is done in other
-- 
2.50.0