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

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

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

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

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