* [PATCH] eal/linux: redefine the name for rte_fbarray_init()
@ 2024-11-14 8:37 Congjie Zhou
2024-11-14 16:24 ` Stephen Hemminger
0 siblings, 1 reply; 8+ messages in thread
From: Congjie Zhou @ 2024-11-14 8:37 UTC (permalink / raw)
To: dev; +Cc: anatoly.burakov, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()
2024-11-14 8:37 [PATCH] eal/linux: redefine the name for rte_fbarray_init() Congjie Zhou
@ 2024-11-14 16:24 ` Stephen Hemminger
2024-11-14 17:06 ` Stephen Hemminger
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Stephen Hemminger @ 2024-11-14 16:24 UTC (permalink / raw)
To: Congjie Zhou; +Cc: dev, anatoly.burakov
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?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()
2024-11-14 16:24 ` Stephen Hemminger
@ 2024-11-14 17:06 ` Stephen Hemminger
2024-11-15 2:26 ` Zhou congjie
2024-11-15 1:57 ` Zhou congjie
2024-11-15 2:00 ` Zhou congjie
2 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2024-11-14 17:06 UTC (permalink / raw)
To: Congjie Zhou; +Cc: dev, anatoly.burakov
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()
2024-11-14 17:06 ` Stephen Hemminger
@ 2024-11-15 2:26 ` Zhou congjie
2024-11-15 5:32 ` Stephen Hemminger
0 siblings, 1 reply; 8+ messages in thread
From: Zhou congjie @ 2024-11-15 2:26 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, anatoly.burakov
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.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()
2024-11-15 2:26 ` Zhou congjie
@ 2024-11-15 5:32 ` Stephen Hemminger
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2024-11-15 5:32 UTC (permalink / raw)
To: Zhou congjie; +Cc: dev, anatoly.burakov
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()
2024-11-14 16:24 ` Stephen Hemminger
2024-11-14 17:06 ` Stephen Hemminger
@ 2024-11-15 1:57 ` Zhou congjie
2024-11-15 2:00 ` Zhou congjie
2 siblings, 0 replies; 8+ messages in thread
From: Zhou congjie @ 2024-11-15 1:57 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, anatoly.burakov
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] eal/linux: redefine the name for rte_fbarray_init()
2024-11-14 16:24 ` Stephen Hemminger
2024-11-14 17:06 ` Stephen Hemminger
2024-11-15 1:57 ` Zhou congjie
@ 2024-11-15 2:00 ` Zhou congjie
2 siblings, 0 replies; 8+ messages in thread
From: Zhou congjie @ 2024-11-15 2:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, anatoly.burakov
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] eal/linux: redefine the name for rte_fbarray_init()
@ 2024-11-14 8:10 Congjie Zhou
0 siblings, 0 replies; 8+ messages in thread
From: Congjie Zhou @ 2024-11-14 8:10 UTC (permalink / raw)
To: dev; +Cc: anatoly.burakov, 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 | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index e354efc..017bf67 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,17 @@ 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 +1459,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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-11-15 5:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-14 8:37 [PATCH] eal/linux: redefine the name for rte_fbarray_init() Congjie Zhou
2024-11-14 16:24 ` Stephen Hemminger
2024-11-14 17:06 ` Stephen Hemminger
2024-11-15 2:26 ` Zhou congjie
2024-11-15 5:32 ` Stephen Hemminger
2024-11-15 1:57 ` Zhou congjie
2024-11-15 2:00 ` Zhou congjie
-- strict thread matches above, loose matches on Subject: below --
2024-11-14 8:10 Congjie Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).