[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

[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

[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

[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

[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

[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

[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

[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 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>

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 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 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 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>

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 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

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 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

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

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 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

[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

[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

[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

[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

[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

[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

[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

[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

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 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 03/10] test/mempool: fix test without stack driver

[Marat Khalili]

Looks good.

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

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 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

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

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 05/10] cmdline: fix port list parsing

[Marat Khalili]

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

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.