[PATCH] eal/linux: redefine the name for rte_fbarray_init()

[PATCH] eal/linux: redefine the name for rte_fbarray_init()

[Congjie Zhou]

add the current time into the name

Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
---

When multiple secondary processes run in different containers, names
identified by PIDs are not unique due to the pid namespace.
Add current time to redefine a unique name.

 lib/eal/linux/eal_memalloc.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index e354efc..3fae534 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -16,6 +16,7 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <setjmp.h>
+#include <inttypes.h>
 #ifdef F_ADD_SEALS /* if file sealing is supported, so is memfd */
 #include <linux/memfd.h>
 #define MEMFD_SUPPORTED
@@ -1430,6 +1431,18 @@ eal_memalloc_sync_with_primary(void)
 	return 0;
 }
 
+static uint64_t
+current_time(void)
+{
+	struct timespec ts;
+	if (clock_gettime(CLOCK_MONOTONIC_RAW, &ts) == -1) {
+		EAL_LOG(ERR, "Fail to get current time");
+		return -1ULL;
+	}
+	uint64_t time_ns = (uint64_t)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
+	return time_ns;
+}
+
 static int
 secondary_msl_create_walk(const struct rte_memseg_list *msl,
 		void *arg __rte_unused)
@@ -1447,8 +1460,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 	local_msl = &local_memsegs[msl_idx];
 
 	/* create distinct fbarrays for each secondary */
-	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
-		primary_msl->memseg_arr.name, getpid());
+	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%"PRIx64,
+		primary_msl->memseg_arr.name, getpid(), current_time());
 
 	ret = rte_fbarray_init(&local_msl->memseg_arr, name,
 		primary_msl->memseg_arr.len,
-- 
2.34.1

Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()

[Stephen Hemminger]

On Thu, 14 Nov 2024 16:37:10 +0800
Congjie Zhou <zcjie0802@qq.com> wrote:

>  
> +static uint64_t
> +current_time(void)
> +{
> +	struct timespec ts;
> +	if (clock_gettime(CLOCK_MONOTONIC_RAW, &ts) == -1) {
> +		EAL_LOG(ERR, "Fail to get current time");
> +		return -1ULL;
> +	}
> +	uint64_t time_ns = (uint64_t)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
> +	return time_ns;
> +}
> +
>  static int
>  secondary_msl_create_walk(const struct rte_memseg_list *msl,
>  		void *arg __rte_unused)
> @@ -1447,8 +1460,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
>  	local_msl = &local_memsegs[msl_idx];
>  
>  	/* create distinct fbarrays for each secondary */
> -	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
> -		primary_msl->memseg_arr.name, getpid());
> +	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%"PRIx64,
> +		primary_msl->memseg_arr.name, getpid(), current_time());
>  

In general DPDK uses tsc instead of monotonic time, since it is faster and platform
independent (ie Windows).

Why not use use a global counter instead?

Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()

[Stephen Hemminger]

What about using thread id instead?

From d1687ffbf865ba0b2d64c35acd602ca43329691e Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 14 Nov 2024 08:48:54 -0800
Subject: [PATCH] eal: fix fbarray name with multiple secondary processes

When multiple secondary processes run in different containers, names
identified by PIDs are not unique due to the pid namespace.
Add current thread id to the name to be unique.

Fixes: 046aa5c4477b ("mem: add memalloc init stage")
Cc: anatoly.burakov@intel.com

Reported-by: Congjie Zhou <zcjie0802@qq.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/linux/eal_memalloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index e354efc95d..776260e14f 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -1447,8 +1447,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 	local_msl = &local_memsegs[msl_idx];
 
 	/* create distinct fbarrays for each secondary */
-	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
-		primary_msl->memseg_arr.name, getpid());
+	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%i",
+		 primary_msl->memseg_arr.name, getpid(), rte_sys_gettid());
 
 	ret = rte_fbarray_init(&local_msl->memseg_arr, name,
 		primary_msl->memseg_arr.len,
-- 
2.45.2

Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()

[Zhou congjie]

On Thu, 14 Nov 2024, Stephen Hemminger wrote:

> 
> In general DPDK uses tsc instead of monotonic time, since it is faster and platform
> independent (ie Windows).
> 
> Why not use use a global counter instead?
> 

I have tried tsc, but it only works on x86 and does not function on other archs.

A global counter seems to be a good idea.
 

Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()

[Zhou congjie]

On Thu, 14 Nov 2024, Stephen Hemminger wrote:
 
> 
> In general DPDK uses tsc instead of monotonic time, since it is faster and platform
> independent (ie Windows).
> 
> Why not use use a global counter instead?
> 

I have tried tsc, but it only works on x86 and does not function on other 
architectures.

A global counter seems to be a good idea.

Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()

[Zhou congjie]

On Thu, 14 Nov 2024, Stephen Hemminger wrote:

> Date: Fri, 15 Nov 2024 01:06:27
> From: Stephen Hemminger <stephen@networkplumber.org>
> To: zcjie0802 <zcjie0802@qq.com>
> Cc: dev <dev@dpdk.org>, anatoly.burakov <anatoly.burakov@intel.com>
> Subject: Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()
> 
> What about using thread id instead?
> 
> From d1687ffbf865ba0b2d64c35acd602ca43329691e Mon Sep 17 00:00:00 2001
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Thu, 14 Nov 2024 08:48:54 -0800
> Subject: [PATCH] eal: fix fbarray name with multiple secondary processes
> 
> When multiple secondary processes run in different containers, names
> identified by PIDs are not unique due to the pid namespace.
> Add current thread id to the name to be unique.
> 
> Fixes: 046aa5c4477b ("mem: add memalloc init stage")
> Cc: anatoly.burakov@intel.com
> 
> Reported-by: Congjie Zhou <zcjie0802@qq.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/eal/linux/eal_memalloc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
> index e354efc95d..776260e14f 100644
> --- a/lib/eal/linux/eal_memalloc.c
> +++ b/lib/eal/linux/eal_memalloc.c
> @@ -1447,8 +1447,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
>  	local_msl = &local_memsegs[msl_idx];
>  
>  	/* create distinct fbarrays for each secondary */
> -	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
> -		primary_msl->memseg_arr.name, getpid());
> +	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%i",
> +		 primary_msl->memseg_arr.name, getpid(), rte_sys_gettid());
>  
>  	ret = rte_fbarray_init(&local_msl->memseg_arr, name,
>  		primary_msl->memseg_arr.len,
> 

As far as know, the tid is pid independent, so programs in different PID 
namespaces may have the same pid and tid.

For the solution that uses a global counter, I feel it can only be 
implemented by adding variables into rte_config or rte_mem_config, which 
involves modifying multiple files. Adding the current time would be more 
simple. This code is excuted only once when the secondary process is created, so 
it will not cause performance issues.

Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()

[Stephen Hemminger]

On Fri, 15 Nov 2024 10:26:00 +0800
Zhou congjie <zcjie0802@qq.com> wrote:

> On Thu, 14 Nov 2024, Stephen Hemminger wrote:
> 
> > Date: Fri, 15 Nov 2024 01:06:27
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > To: zcjie0802 <zcjie0802@qq.com>
> > Cc: dev <dev@dpdk.org>, anatoly.burakov <anatoly.burakov@intel.com>
> > Subject: Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()
> > 
> > What about using thread id instead?
> > 
> > From d1687ffbf865ba0b2d64c35acd602ca43329691e Mon Sep 17 00:00:00 2001
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Date: Thu, 14 Nov 2024 08:48:54 -0800
> > Subject: [PATCH] eal: fix fbarray name with multiple secondary processes
> > 
> > When multiple secondary processes run in different containers, names
> > identified by PIDs are not unique due to the pid namespace.
> > Add current thread id to the name to be unique.
> > 
> > Fixes: 046aa5c4477b ("mem: add memalloc init stage")
> > Cc: anatoly.burakov@intel.com
> > 
> > Reported-by: Congjie Zhou <zcjie0802@qq.com>
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/eal/linux/eal_memalloc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
> > index e354efc95d..776260e14f 100644
> > --- a/lib/eal/linux/eal_memalloc.c
> > +++ b/lib/eal/linux/eal_memalloc.c
> > @@ -1447,8 +1447,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
> >  	local_msl = &local_memsegs[msl_idx];
> >  
> >  	/* create distinct fbarrays for each secondary */
> > -	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
> > -		primary_msl->memseg_arr.name, getpid());
> > +	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%i",
> > +		 primary_msl->memseg_arr.name, getpid(), rte_sys_gettid());
> >  
> >  	ret = rte_fbarray_init(&local_msl->memseg_arr, name,
> >  		primary_msl->memseg_arr.len,
> >   
> 
> As far as know, the tid is pid independent, so programs in different PID 
> namespaces may have the same pid and tid.
> 
> For the solution that uses a global counter, I feel it can only be 
> implemented by adding variables into rte_config or rte_mem_config, which 
> involves modifying multiple files. Adding the current time would be more 
> simple. This code is excuted only once when the secondary process is created, so 
> it will not cause performance issues.

The combination of thread id and process id will be unique but
there might be issues with some container implementations which will
have overlapping pid's (see Docker --pid flag).

The TSC is available in all of DPDK via rte_get_tsc_cycles()
or use rte_get_timer_cycles().

But you could still have collisions from two containers using hidden pid
doing function at same exact time. Not ever likely, but it is possible.

[PATCH v2] eal/linux: fix fbarray name with multiple secondary processes

[Congjie Zhou]

add the tsc into the name.

Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
---

When multiple secondary processes run in different containers, names
identified by PIDs are not unique due to the pid namespace. 
So Add tsc to redefine a unique name.

v1: use monotonic time to redefine the name
v2: use tsc to redefine the name

 lib/eal/linux/eal_memalloc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index e354efc..367d401 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -16,6 +16,7 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <setjmp.h>
+#include <inttypes.h>
 #ifdef F_ADD_SEALS /* if file sealing is supported, so is memfd */
 #include <linux/memfd.h>
 #define MEMFD_SUPPORTED
@@ -1447,8 +1448,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 	local_msl = &local_memsegs[msl_idx];
 
 	/* create distinct fbarrays for each secondary */
-	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
-		primary_msl->memseg_arr.name, getpid());
+	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%"PRIx64,
+		primary_msl->memseg_arr.name, getpid(), rte_get_tsc_cycles());
 
 	ret = rte_fbarray_init(&local_msl->memseg_arr, name,
 		primary_msl->memseg_arr.len,
-- 
2.34.1

Re: [PATCH v2] eal/linux: fix fbarray name with multiple secondary processes

[Stephen Hemminger]

On Fri, 15 Nov 2024 15:50:08 +0800
Congjie Zhou <zcjie0802@qq.com> wrote:

> add the tsc into the name.
> 
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
> ---
> 
> When multiple secondary processes run in different containers, names
> identified by PIDs are not unique due to the pid namespace. 
> So Add tsc to redefine a unique name.
> 
> v1: use monotonic time to redefine the name
> v2: use tsc to redefine the name
> 
>  lib/eal/linux/eal_memalloc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
> index e354efc..367d401 100644
> --- a/lib/eal/linux/eal_memalloc.c
> +++ b/lib/eal/linux/eal_memalloc.c
> @@ -16,6 +16,7 @@
>  #include <fcntl.h>
>  #include <signal.h>
>  #include <setjmp.h>
> +#include <inttypes.h>
>  #ifdef F_ADD_SEALS /* if file sealing is supported, so is memfd */
>  #include <linux/memfd.h>
>  #define MEMFD_SUPPORTED
> @@ -1447,8 +1448,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
>  	local_msl = &local_memsegs[msl_idx];
>  
>  	/* create distinct fbarrays for each secondary */
> -	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
> -		primary_msl->memseg_arr.name, getpid());
> +	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%"PRIx64,
> +		primary_msl->memseg_arr.name, getpid(), rte_get_tsc_cycles());
>  

Worst case name is now:
'memseg-1048576k-128-128-1234567890-ffffffffffffffff' = 51 characters


Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH v2] eal/linux: fix fbarray name with multiple secondary processes

[Stephen Hemminger]

On Fri, 15 Nov 2024 15:50:08 +0800
Congjie Zhou <zcjie0802@qq.com> wrote:

> diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
> index e354efc..367d401 100644
> --- a/lib/eal/linux/eal_memalloc.c
> +++ b/lib/eal/linux/eal_memalloc.c
> @@ -16,6 +16,7 @@
>  #include <fcntl.h>
>  #include <signal.h>
>  #include <setjmp.h>
> +#include <inttypes.h>
>  #ifdef F_ADD_SEALS /* if file sealing is supported, so is memfd */
>  #include <linux/memfd.h>
>  #define MEMFD_SUPPORTED
> @@ -1447,8 +1448,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
>  	local_msl = &local_memsegs[msl_idx];
>  
>  	/* create distinct fbarrays for each secondary */
> -	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
> -		primary_msl->memseg_arr.name, getpid());
> +	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%"PRIx64,
> +		primary_msl->memseg_arr.name, getpid(), rte_get_tsc_cycles());
>  
>  	ret = rte_fbarray_init(&local_msl->memseg_arr, name,
>  		primary_msl->memseg_arr.len,


Need to include <rte_cycles.h> to get the prototype rte_get_tsc_cycles()

Re: [PATCH v2] eal/linux: fix fbarray name with multiple secondaryprocesses

[Zhou congjie]

On Fri, 15 Nov 2024, Stephen Hemminger wrote:

> Need to include <rte_cycles.h> to get the prototype rte_get_tsc_cycles()

When I compile locally (on x86), it compiles successfully whether or not 
<rte_cycles.h> is included. 

However, cloud testing indicates that compilation fails on other 
architectures when <rte_cycles.h> is not included.
 
Is <rte_cycles.h> the cause of the compilation failure? 
If so, I'm curious why this is the case.

Re: [PATCH v2] eal/linux: fix fbarray name with multiple secondaryprocesses

[Zhou congjie]

On Fri, 15 Nov 2024, Stephen Hemminger wrote:

> Need to include <rte_cycles.h> to get the prototype rte_get_tsc_cycles()


For the code without <rte_cycles.h> could run on x86, I think I have found 
the reason. Through the path <rte_memory.h> -> <rte_eal_memconfig.h> -> 
<rte_spinlock.h>, the <rte_spinlock.h> is included.

<rte_spinlock.h> is independent on each architecture. And on x86 the 
<rte_spinlock.h> includes its own "rte_cycles.h", while other 
architectures do not.

[PATCH v3] eal/linux: fix fbarray name with multiple secondary processes

[Congjie Zhou]

add the tsc into the name.

Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
---

When multiple secondary processes run in different containers, names
identified by PIDs are not unique due to the pid namespace. 
So Add tsc to redefine a unique name.

v1: use monotonic time to redefine the name
v2: use tsc to redefine the name
v3: include <rte_cycles.h> to support all architectures

 lib/eal/linux/eal_memalloc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index e354efc..ae1e024 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -16,6 +16,7 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <setjmp.h>
+#include <inttypes.h>
 #ifdef F_ADD_SEALS /* if file sealing is supported, so is memfd */
 #include <linux/memfd.h>
 #define MEMFD_SUPPORTED
@@ -31,6 +32,7 @@
 #include <rte_log.h>
 #include <rte_eal.h>
 #include <rte_memory.h>
+#include <rte_cycles.h>
 
 #include "eal_filesystem.h"
 #include "eal_internal_cfg.h"
@@ -1447,8 +1449,8 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 	local_msl = &local_memsegs[msl_idx];
 
 	/* create distinct fbarrays for each secondary */
-	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
-		primary_msl->memseg_arr.name, getpid());
+	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i_%"PRIx64,
+		primary_msl->memseg_arr.name, getpid(), rte_get_tsc_cycles());
 
 	ret = rte_fbarray_init(&local_msl->memseg_arr, name,
 		primary_msl->memseg_arr.len,
-- 
2.34.1

Re: [PATCH v3] eal/linux: fix fbarray name with multiple secondary processes

[Stephen Hemminger]

On Sat, 16 Nov 2024 12:07:41 +0800
Congjie Zhou <zcjie0802@qq.com> wrote:

> add the tsc into the name.
> 
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
> ---

This patch seems to have gotten stuck in the unreviewed dead pile.
Looking at it, it addresses a potential problem but there are issues.
Please consider what this AI review feedback had to say and resubmit

Now let me analyze this patch against the AGENTS.md criteria and evaluate the technical correctness.

## Evaluation Against AGENTS.md

### Commit Message Issues

| Check | Status | Details |
|-------|--------|---------|
| Subject ≤60 chars | ✅ | 56 characters |
| Correct prefix | ✅ | `eal/linux:` is correct |
| Lowercase after colon | ✅ | "fix fbarray..." |
| Imperative mood | ✅ | "fix" |
| No trailing period | ✅ | |
| Signed-off-by present | ✅ | |
| Body lines ≤75 chars | ✅ | |

**Warning**: The commit body is weak - "add the tsc into the name." is very terse and doesn't explain the *problem* being solved. The actual explanation is after `---` which won't appear in git history. The body should explain the container PID namespace issue before the Signed-off-by.

### Code Style

| Check | Status | Details |
|-------|--------|---------|
| Include order | ✅ | `inttypes.h` with system includes, `rte_cycles.h` with DPDK includes |
| PRIx64 format | ✅ | Correct portable format specifier |
| Line length | ✅ | Within 100 chars |

---

## Technical Correctness Analysis

### The Problem
In Linux containers with separate PID namespaces, each container's PID numbering starts from 1. If multiple secondary DPDK processes in different containers share the same hugetlbfs mount, the fbarray names (which include PID) can collide.

### Issues with the TSC Solution

1. **TSC is not reliably unique**: Two containers starting at nearly the same instant on the same CPU could get very close or identical TSC values. The TSC counter increments at GHz frequencies, but container startup can be synchronized at millisecond granularity.

2. **TSC synchronization issues**: On multi-socket systems, TSC may not be synchronized across sockets/CPUs. While modern CPUs have `constant_tsc` and `nonstop_tsc`, this isn't universal.

3. **Overhead**: `rte_get_tsc_cycles()` is very cheap, but it adds a dependency on the cycles subsystem being initialized.

4. **Name length**: Adding a 16-character hex value to an already length-limited name (RTE_FBARRAY_NAME_LEN) could cause truncation issues.

---

## Simpler Alternative Solutions

### Option 1: Use `rte_rand()` (Recommended)

```c
#include <rte_random.h>

snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%x",
    primary_msl->memseg_arr.name, rte_rand());
```

**Advantages**:
- Cryptographically random, much lower collision probability
- Already seeded during EAL init
- No timing dependencies
- Simpler code (no PID needed at all)

### Option 2: Use the runtime directory uniqueness

The **proper solution** for containers is to use `--file-prefix` which already exists. Each container should specify a unique file prefix, making this problem disappear entirely. This is a documentation/deployment issue, not a code issue.

### Option 3: Combine with monotonic clock nanoseconds

```c
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%lx",
    primary_msl->memseg_arr.name, 
    (unsigned long)(ts.tv_sec * 1000000000UL + ts.tv_nsec));
```

This was v1 of the patch and is actually fine - CLOCK_MONOTONIC with nanosecond resolution within a container's lifetime is effectively unique.

---

## Verdict

**The patch is technically functional but the solution is suboptimal.** 

The TSC approach works in practice but has theoretical collision risks. A cleaner approach would be:

1. **Best**: Document that containers sharing hugetlbfs should use `--file-prefix`
2. **Code fix if needed**: Use `rte_rand()` instead of TSC - it's simpler and provides stronger uniqueness guarantees

Would you like me to draft an alternative patch using `rte_rand()`?

[PATCH v4] Subject: eal/linux: fix fbarray name collision in containers

[Stephen Hemminger]

When multiple secondary processes run in different containers that
share the same hugetlbfs mount, the fbarray names can collide.
This happens because containers use separate PID namespaces, so
different processes in different containers can have the same PID.

Fix by replacing the PID with a timestamp-based value. The TSC
(timestamp counter) provides sufficient uniqueness since containers
starting at the same CPU cycle is practically impossible - even 1ms
of startup time difference means millions of cycles apart at GHz
frequencies.

Also, reduce the name buffer from PATH_MAX to RTE_FBARRAY_NAME_LEN
since it is only used for the fbarray name.

Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v4 - update comment and commit message
   - reduce name buffer from 4K to 64 to save stack space
     and catch any future format overflow
 lib/eal/linux/eal_memalloc.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index 1e60e21620..cbe22c98d0 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdint.h>
+#include <inttypes.h>
 #include <string.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
@@ -28,6 +29,7 @@
 #include <rte_log.h>
 #include <rte_eal.h>
 #include <rte_memory.h>
+#include <rte_cycles.h>
 
 #include "eal_filesystem.h"
 #include "eal_internal_cfg.h"
@@ -1380,7 +1382,7 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	struct rte_memseg_list *primary_msl, *local_msl;
-	char name[PATH_MAX];
+	char name[RTE_FBARRAY_NAME_LEN];
 	int msl_idx, ret;
 
 	if (msl->external)
@@ -1390,9 +1392,16 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 	primary_msl = &mcfg->memsegs[msl_idx];
 	local_msl = &local_memsegs[msl_idx];
 
-	/* create distinct fbarrays for each secondary */
-	snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
-		primary_msl->memseg_arr.name, getpid());
+	/*
+	 * Create distinct fbarrays for each secondary using TSC for uniqueness,
+	 * since PID is not unique across containers (different PID namespaces).
+	 * The worst case name length is:
+	 * Base name: "memseg-1048576k-99-99" ~21 chars
+	 * Suffix "_<pid>_<16hex>" +24
+	 * Total = 44 < RTE_FBARRAY_NAME_LEN 64
+	 */
+	snprintf(name, sizeof(name), "%s_%"PRIx64,
+		primary_msl->memseg_arr.name, rte_get_tsc_cycles());
 
 	ret = rte_fbarray_init(&local_msl->memseg_arr, name,
 		primary_msl->memseg_arr.len,
-- 
2.51.0

[PATCH v5] Subject: eal/linux: fix fbarray name collision in containers

[Stephen Hemminger]

From: Congjie Zhou <zcjie0802@qq.com>

When multiple secondary processes run in different containers that
share the same hugetlbfs mount, the fbarray names can collide.
This happens because containers use separate PID namespaces, so
different processes in different containers can have the same PID.

Fix by replacing the PID with a timestamp-based value. The TSC
(timestamp counter) provides sufficient uniqueness since containers
starting at the same CPU cycle is practically impossible - even 1ms
of startup time difference means millions of cycles apart at GHz
frequencies.

Also, reduce the name buffer from PATH_MAX to RTE_FBARRAY_NAME_LEN
since it is only used for the fbarray name.

Fixes: 046aa5c4477b ("mem: add memalloc init stage")
Cc: stable@dpdk.org

Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v5 - rebase after format-over fixes

 lib/eal/linux/eal_memalloc.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index 4dee224ac5..70f6335d03 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdint.h>
+#include <inttypes.h>
 #include <string.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
@@ -28,6 +29,7 @@
 #include <rte_log.h>
 #include <rte_eal.h>
 #include <rte_memory.h>
+#include <rte_cycles.h>
 
 #include "eal_filesystem.h"
 #include "eal_internal_cfg.h"
@@ -1387,7 +1389,7 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	struct rte_memseg_list *primary_msl, *local_msl;
-	char name[PATH_MAX];
+	char name[RTE_FBARRAY_NAME_LEN];
 	int msl_idx, ret;
 
 	if (msl->external)
@@ -1397,9 +1399,16 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 	primary_msl = &mcfg->memsegs[msl_idx];
 	local_msl = &local_memsegs[msl_idx];
 
-	/* create distinct fbarrays for each secondary */
-	ret = snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
-		primary_msl->memseg_arr.name, getpid());
+	/*
+	 * Create distinct fbarrays for each secondary using TSC for uniqueness,
+	 * since PID is not unique across containers (different PID namespaces).
+	 * The worst case name length is:
+	 * Base name: "memseg-1048576k-99-99" ~21 chars
+	 * Suffix "_<pid>_<16hex>" +24
+	 * Total = 44 < RTE_FBARRAY_NAME_LEN 64
+	 */
+	ret = snprintf(name, sizeof(name), "%s_%"PRIx64,
+		primary_msl->memseg_arr.name, rte_get_tsc_cycles());
 	if (ret >= RTE_FBARRAY_NAME_LEN) {
 		EAL_LOG(ERR, "fbarray name %s_%i is too long",
 				primary_msl->memseg_arr.name, getpid());
-- 
2.51.0

[PATCH v6] eal/linux: fix fbarray name collision in containers

[Stephen Hemminger]

From: Congjie Zhou <zcjie0802@qq.com>

When multiple secondary processes run in different containers that
share the same hugetlbfs mount, the fbarray names can collide.
This happens because containers use separate PID namespaces, so
different processes in different containers can have the same PID.

Fix by replacing the PID with a timestamp-based value. The TSC
(timestamp counter) provides sufficient uniqueness since containers
starting at the same CPU cycle is practically impossible - even 1ms
of startup time difference means millions of cycles apart at GHz
frequencies.

Also, reduce the name buffer from PATH_MAX to RTE_FBARRAY_NAME_LEN
since it is only used for the fbarray name.

Fixes: 046aa5c4477b ("mem: add memalloc init stage")
Cc: stable@dpdk.org

Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v6 - fix typo in subject
   - add pid to name because if two secondary spawn at once
     in theory could collide.

 lib/eal/linux/eal_memalloc.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index 4dee224ac5..dfbedf4626 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdint.h>
+#include <inttypes.h>
 #include <string.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
@@ -28,6 +29,7 @@
 #include <rte_log.h>
 #include <rte_eal.h>
 #include <rte_memory.h>
+#include <rte_cycles.h>
 
 #include "eal_filesystem.h"
 #include "eal_internal_cfg.h"
@@ -1387,7 +1389,7 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	struct rte_memseg_list *primary_msl, *local_msl;
-	char name[PATH_MAX];
+	char name[RTE_FBARRAY_NAME_LEN];
 	int msl_idx, ret;
 
 	if (msl->external)
@@ -1397,12 +1399,21 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
 	primary_msl = &mcfg->memsegs[msl_idx];
 	local_msl = &local_memsegs[msl_idx];
 
-	/* create distinct fbarrays for each secondary */
-	ret = snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i",
-		primary_msl->memseg_arr.name, getpid());
-	if (ret >= RTE_FBARRAY_NAME_LEN) {
-		EAL_LOG(ERR, "fbarray name %s_%i is too long",
-				primary_msl->memseg_arr.name, getpid());
+	/*
+	 * Create distinct fbarrays for each secondary using TSC for uniqueness,
+	 * since PID is not unique across containers (different PID namespaces).
+	 * The worst case name length is:
+	 * Base name: "memseg-1048576k-99-99" ~21 chars
+	 * Suffix "_<pid>_<16hex>" +24
+	 * Total = 44 < RTE_FBARRAY_NAME_LEN 64
+	 */
+	uint64_t tsc = rte_get_tsc_cycles();
+	pid_t pid = getpid();
+	ret = snprintf(name, sizeof(name), "%s_%d_%"PRIx64,
+		       primary_msl->memseg_arr.name, pid, tsc);
+	if (ret >= (int)sizeof(name)) {
+		EAL_LOG(ERR, "fbarray name \"%s_%d_%"PRIx64"\" is too long",
+			primary_msl->memseg_arr.name, pid, tsc);
 		return -1;
 	}
 
-- 
2.51.0

Re: [PATCH v6] eal/linux: fix fbarray name collision in containers

[David Marchand]

On Fri, 13 Feb 2026 at 23:01, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> From: Congjie Zhou <zcjie0802@qq.com>
>
> When multiple secondary processes run in different containers that
> share the same hugetlbfs mount, the fbarray names can collide.
> This happens because containers use separate PID namespaces, so
> different processes in different containers can have the same PID.
>
> Fix by replacing the PID with a timestamp-based value. The TSC
> (timestamp counter) provides sufficient uniqueness since containers
> starting at the same CPU cycle is practically impossible - even 1ms
> of startup time difference means millions of cycles apart at GHz
> frequencies.
>
> Also, reduce the name buffer from PATH_MAX to RTE_FBARRAY_NAME_LEN
> since it is only used for the fbarray name.
>

Afaics, the filename generation comes from:
524e43c2ad9a ("mem: prepare memseg lists for multiprocess sync")

> Fixes: 046aa5c4477b ("mem: add memalloc init stage")

This change ^^ only moved the point in EAL init where the name was generated.


> Cc: stable@dpdk.org
>
> Signed-off-by: Congjie Zhou <zcjie0802@qq.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Updated comment:
         * Base name: "memseg-1048576k-99-99" ~21 chars
         * Suffix "_<pid>_<16hex>" +24
-        * Total = 44 < RTE_FBARRAY_NAME_LEN 64
+        * Total = ~45 < RTE_FBARRAY_NAME_LEN 64

Applied, thanks.


-- 
David Marchand

Re: [PATCH v6] eal/linux: fix fbarray name collision in containers

[Stephen Hemminger]

On Mon, 16 Feb 2026 18:22:48 +0100
David Marchand <david.marchand@redhat.com> wrote:

> On Fri, 13 Feb 2026 at 23:01, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > From: Congjie Zhou <zcjie0802@qq.com>
> >
> > When multiple secondary processes run in different containers that
> > share the same hugetlbfs mount, the fbarray names can collide.
> > This happens because containers use separate PID namespaces, so
> > different processes in different containers can have the same PID.
> >
> > Fix by replacing the PID with a timestamp-based value. The TSC
> > (timestamp counter) provides sufficient uniqueness since containers
> > starting at the same CPU cycle is practically impossible - even 1ms
> > of startup time difference means millions of cycles apart at GHz
> > frequencies.
> >
> > Also, reduce the name buffer from PATH_MAX to RTE_FBARRAY_NAME_LEN
> > since it is only used for the fbarray name.
> >  
> 
> Afaics, the filename generation comes from:
> 524e43c2ad9a ("mem: prepare memseg lists for multiprocess sync")
> 
> > Fixes: 046aa5c4477b ("mem: add memalloc init stage")  
> 
> This change ^^ only moved the point in EAL init where the name was generated.

Thanks, I didn't go digging back through the history and was more looking
at what to blame.