[PATCH v5 00/10] support reinit flow

[PATCH v5 00/10] support reinit flow

[okaya]

From: Sinan Kaya <okaya@kernel.org>

We want to be able to call rte_eal_init() and rte_eal_cleanup()
APIs back to back for maintanance reasons.

Here is a summary of the code we have seen so far:

1. some code support getting called multiple times by keeping
a static variable.
2. some code initializes once but never clean up after them and
don't have a cleanup API.
3. some code assumes that they only get called once during the
lifecycle of the process.

Most changes in this patch center around following the #1 design
principle.

Why?

It is not always ideal to reinitialize a DPDK process. Memory needs
to be reinitialized, hugetables need to warm up etc.

Limitations:

This sequence could only be done by main lcore, and never ever in a signal handler.
Do not try and trap signals like abort, bus error, illegal instruction and try to
use this for recovery. It is a recipe for failure.


Changed from

v1:
Fix checkpatch warnings

v2:
rebase to most recent DPDK.

v3:
pick up Stephen's "eal: cleanup plugins data" as a pre-requisite
patch.

v4:
fix windows build
rename variables to run_once where possible
move run_once inside the functions where possible

Graham Whyte (1):
  eal: fixes for re-initialization issues

Sinan Kaya (8):
  tailq: skip init if already initialized
  eal_memzone: bail out on initialized
  memseg: init once
  eal_memory: skip initialization
  eal_interrupts: don't reinitialize threads
  eal: initialize worker threads once
  eal: declare the reinit via cleanup/init feature unsupported
  test: remove double registration check

Stephen Hemminger (1):
  eal: cleanup plugins data

 app/test/test_tailq.c               |  5 ---
 lib/eal/common/eal_common_memory.c  |  6 +++
 lib/eal/common/eal_common_memzone.c |  6 +++
 lib/eal/common/eal_common_options.c | 21 +++++++++
 lib/eal/common/eal_common_tailqs.c  | 20 ++++++---
 lib/eal/common/eal_options.h        |  1 +
 lib/eal/common/malloc_heap.c        |  7 +++
 lib/eal/include/rte_eal.h           |  5 +++
 lib/eal/linux/eal.c                 | 66 ++++++++++++++++-------------
 lib/eal/linux/eal_interrupts.c      |  6 +++
 lib/eal/linux/eal_memory.c          | 12 +++++-
 11 files changed, 113 insertions(+), 42 deletions(-)

-- 
2.25.1

[PATCH v5 01/10] eal: cleanup plugins data

[okaya]

From: Stephen Hemminger <stephen@networkplumber.org>

When rte_eal_cleanup is called walk through the list of shared
objects loaded, and close them and free the data structure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_options.c | 14 ++++++++++++++
 lib/eal/common/eal_options.h        |  1 +
 lib/eal/linux/eal.c                 |  2 ++
 3 files changed, 17 insertions(+)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 062f1d8d9c..a509adc78b 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -244,6 +244,20 @@ eal_save_args(int argc, char **argv)
 }
 #endif
 
+void
+eal_plugins_cleanup(void)
+{
+#ifndef RTE_EXEC_ENV_WINDOWS
+	struct shared_driver *solib, *tmp;
+
+	RTE_TAILQ_FOREACH_SAFE(solib, &solib_list, next, tmp) {
+		if (solib->lib_handle)
+			dlclose(solib->lib_handle);
+		free(solib);
+	}
+#endif
+}
+
 static int
 eal_option_device_add(enum rte_devtype type, const char *optarg)
 {
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 3cc9cb6412..ddbaafc4f1 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -105,6 +105,7 @@ int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
 enum rte_proc_type_t eal_proc_type_detect(void);
 int eal_plugins_init(void);
+void eal_plugins_cleanup(void);
 int eal_save_args(int argc, char **argv);
 int handle_eal_info_request(const char *cmd, const char *params __rte_unused,
 		struct rte_tel_data *d);
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index c6efd92014..dee649bab3 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1398,6 +1398,8 @@ rte_eal_cleanup(void)
 	eal_trace_fini();
 	eal_mp_dev_hotplug_cleanup();
 	rte_eal_alarm_cleanup();
+	eal_plugins_cleanup();
+
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
 	rte_eal_malloc_heap_cleanup();
-- 
2.25.1

[PATCH v5 02/10] eal: fixes for re-initialization issues

[okaya]

From: Graham Whyte <grwhyte@microsoft.com>

reinitialize the solib link list and clean the globals holding
state for parsing.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
Signed-off-by: Graham Whyte <grwhyte@microsoft.com>
---
 lib/eal/common/eal_common_options.c | 7 +++++++
 lib/eal/linux/eal.c                 | 4 +++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index a509adc78b..630d06f918 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -256,6 +256,13 @@ eal_plugins_cleanup(void)
 		free(solib);
 	}
 #endif
+
+	/* Reinitialize solib_list */
+	TAILQ_INIT(&solib_list);
+
+	main_lcore_parsed = 0;
+	mem_parsed = 0;
+	core_parsed = 0;
 }
 
 static int
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index dee649bab3..584c78e640 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -79,6 +79,8 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
 int rte_cycles_vmware_tsc_map;
 
 
+static uint32_t run_once;
+
 int
 eal_clean_runtime_dir(void)
 {
@@ -505,6 +507,7 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
 		socket_arg[i] = val;
 	}
 
+	__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 	return 0;
 }
 
@@ -967,7 +970,6 @@ int
 rte_eal_init(int argc, char **argv)
 {
 	int i, fctret, ret;
-	static uint32_t run_once;
 	uint32_t has_run = 0;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 	char thread_name[RTE_MAX_THREAD_NAME_LEN];
-- 
2.25.1

[PATCH v5 03/10] tailq: skip init if already initialized

[okaya]

From: Sinan Kaya <okaya@kernel.org>

Allows tailq to be reinitialied multiple times
by looking up previously registered tailqs

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_tailqs.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/lib/eal/common/eal_common_tailqs.c b/lib/eal/common/eal_common_tailqs.c
index 580fbf24bc..75c0235438 100644
--- a/lib/eal/common/eal_common_tailqs.c
+++ b/lib/eal/common/eal_common_tailqs.c
@@ -73,9 +73,10 @@ rte_eal_tailq_create(const char *name)
 		strlcpy(head->name, name, sizeof(head->name) - 1);
 		TAILQ_INIT(&head->tailq_head);
 		rte_tailqs_count++;
+		return head;
 	}
 
-	return head;
+	return rte_eal_tailq_lookup(name);
 }
 
 /* local register, used to store "early" tailqs before rte_eal_init() and to
@@ -99,7 +100,9 @@ rte_eal_tailq_update(struct rte_tailq_elem *t)
 {
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		/* primary process is the only one that creates */
-		t->head = rte_eal_tailq_create(t->name);
+		t->head = rte_eal_tailq_lookup(t->name);
+		if (t->head == NULL)
+			t->head = rte_eal_tailq_create(t->name);
 	} else {
 		t->head = rte_eal_tailq_lookup(t->name);
 	}
@@ -108,15 +111,13 @@ rte_eal_tailq_update(struct rte_tailq_elem *t)
 int
 rte_eal_tailq_register(struct rte_tailq_elem *t)
 {
-	if (rte_eal_tailq_local_register(t) < 0) {
-		RTE_LOG(ERR, EAL,
-			"%s tailq is already registered\n", t->name);
-		goto error;
-	}
+	rte_eal_tailq_local_register(t);
 
 	/* if a register happens after rte_eal_tailqs_init(), then we can update
 	 * tailq head */
 	if (rte_tailqs_count >= 0) {
+		RTE_LOG(INFO, EAL,
+			"%s tailq is registered\n", t->name);
 		rte_eal_tailq_update(t);
 		if (t->head == NULL) {
 			RTE_LOG(ERR, EAL,
@@ -138,6 +139,11 @@ rte_eal_tailqs_init(void)
 {
 	struct rte_tailq_elem *t;
 
+	if (rte_tailqs_count > 0) {
+		RTE_LOG(INFO, EAL, "tailq already initialized\n");
+		return 0;
+	}
+
 	rte_tailqs_count = 0;
 
 	TAILQ_FOREACH(t, &rte_tailq_elem_head, next) {
-- 
2.25.1

[PATCH v5 04/10] eal_memzone: bail out on initialized

[okaya]

From: Sinan Kaya <okaya@kernel.org>

Initialize memzone once and bail out if someone calls init
multiple times.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_memzone.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 1f3e701499..d2fac4be01 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -425,6 +425,10 @@ rte_eal_memzone_init(void)
 {
 	struct rte_mem_config *mcfg;
 	int ret = 0;
+	static bool run_once;
+
+	if (run_once)
+		return 0;
 
 	/* get pointer to global configuration */
 	mcfg = rte_eal_get_configuration()->mem_config;
@@ -444,6 +448,8 @@ rte_eal_memzone_init(void)
 
 	rte_rwlock_write_unlock(&mcfg->mlock);
 
+	run_once = true;
+
 	return ret;
 }
 
-- 
2.25.1

[PATCH v5 05/10] memseg: init once

[okaya]

From: Sinan Kaya <okaya@kernel.org>

Initialize memory segments just once and bail out if called
multiple times.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/linux/eal_memory.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
index 9b6f08fba8..693a56649b 100644
--- a/lib/eal/linux/eal_memory.c
+++ b/lib/eal/linux/eal_memory.c
@@ -1920,6 +1920,11 @@ rte_eal_memseg_init(void)
 {
 	/* increase rlimit to maximum */
 	struct rlimit lim;
+	int ret;
+	static bool run_once;
+
+	if (run_once)
+		return 0;
 
 #ifndef RTE_EAL_NUMA_AWARE_HUGEPAGES
 	const struct internal_config *internal_conf =
@@ -1948,11 +1953,16 @@ rte_eal_memseg_init(void)
 	}
 #endif
 
-	return rte_eal_process_type() == RTE_PROC_PRIMARY ?
+	ret = rte_eal_process_type() == RTE_PROC_PRIMARY ?
 #ifndef RTE_ARCH_64
 			memseg_primary_init_32() :
 #else
 			memseg_primary_init() :
 #endif
 			memseg_secondary_init();
+
+	if (!ret)
+		run_once = true;
+
+	return ret;
 }
-- 
2.25.1

[PATCH v5 06/10] eal_memory: skip initialization

[okaya]

From: Sinan Kaya <okaya@kernel.org>

Initialize heap area just once.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_memory.c | 6 ++++++
 lib/eal/common/malloc_heap.c       | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/lib/eal/common/eal_common_memory.c b/lib/eal/common/eal_common_memory.c
index d9433db623..1ba2007f76 100644
--- a/lib/eal/common/eal_common_memory.c
+++ b/lib/eal/common/eal_common_memory.c
@@ -1083,6 +1083,10 @@ rte_eal_memory_init(void)
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
 	int retval;
+	static bool run_once;
+
+	if (run_once)
+		return 0;
 
 	RTE_LOG(DEBUG, EAL, "Setting up physically contiguous memory...\n");
 
@@ -1101,6 +1105,8 @@ rte_eal_memory_init(void)
 	if (internal_conf->no_shconf == 0 && rte_eal_memdevice_init() < 0)
 		goto fail;
 
+	run_once = true;
+
 	return 0;
 fail:
 	return -1;
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 6b6cf9174c..5f01735520 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -31,6 +31,7 @@
 #define CONST_MAX(a, b) (a > b ? a : b) /* RTE_MAX is not a constant */
 #define EXTERNAL_HEAP_MIN_SOCKET_ID (CONST_MAX((1 << 8), RTE_MAX_NUMA_NODES))
 
+
 static unsigned
 check_hugepage_sz(unsigned flags, uint64_t hugepage_sz)
 {
@@ -1409,6 +1410,10 @@ rte_eal_malloc_heap_init(void)
 	unsigned int i;
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
+	static bool run_once;
+
+	if (run_once)
+		return 0;
 
 	if (internal_conf->match_allocations)
 		RTE_LOG(DEBUG, EAL, "Hugepages will be freed exactly as allocated.\n");
@@ -1435,6 +1440,8 @@ rte_eal_malloc_heap_init(void)
 		return -1;
 	}
 
+	run_once = true;
+
 	return 0;
 }
 
-- 
2.25.1

[PATCH v5 07/10] eal_interrupts: don't reinitialize threads

[okaya]

From: Sinan Kaya <okaya@kernel.org>

Initialize interrupt thread once and keep a flag
for further init.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/linux/eal_interrupts.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index c9881143be..7adf4076ae 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1174,6 +1174,10 @@ int
 rte_eal_intr_init(void)
 {
 	int ret = 0;
+	static bool run_once;
+
+	if (run_once)
+		return 0;
 
 	/* init the global interrupt source head */
 	TAILQ_INIT(&intr_sources);
@@ -1196,6 +1200,8 @@ rte_eal_intr_init(void)
 			"Failed to create thread for interrupt handling\n");
 	}
 
+	run_once = true;
+
 	return ret;
 }
 
-- 
2.25.1

[PATCH v5 08/10] eal: initialize worker threads once

[okaya]

From: Sinan Kaya <okaya@kernel.org>

Initialize worker threads once and keep a flag
for other init calls.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/linux/eal.c | 60 ++++++++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 584c78e640..1305e1df54 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -80,6 +80,7 @@ int rte_cycles_vmware_tsc_map;
 
 
 static uint32_t run_once;
+static bool worker_initialized;
 
 int
 eal_clean_runtime_dir(void)
@@ -1254,42 +1255,45 @@ rte_eal_init(int argc, char **argv)
 		config->main_lcore, (uintptr_t)pthread_self(), cpuset,
 		ret == 0 ? "" : "...");
 
-	RTE_LCORE_FOREACH_WORKER(i) {
+	if (worker_initialized == false) {
+		RTE_LCORE_FOREACH_WORKER(i) {
 
 		/*
 		 * create communication pipes between main thread
 		 * and children
 		 */
-		if (pipe(lcore_config[i].pipe_main2worker) < 0)
-			rte_panic("Cannot create pipe\n");
-		if (pipe(lcore_config[i].pipe_worker2main) < 0)
-			rte_panic("Cannot create pipe\n");
-
-		lcore_config[i].state = WAIT;
-
-		/* create a thread for each lcore */
-		ret = eal_worker_thread_create(i);
-		if (ret != 0)
-			rte_panic("Cannot create thread\n");
-
-		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, sizeof(thread_name),
-			"rte-worker-%d", i);
-		rte_thread_set_name(lcore_config[i].thread_id, thread_name);
+			if (pipe(lcore_config[i].pipe_main2worker) < 0)
+				rte_panic("Cannot create pipe\n");
+			if (pipe(lcore_config[i].pipe_worker2main) < 0)
+				rte_panic("Cannot create pipe\n");
+
+			lcore_config[i].state = WAIT;
+
+			/* create a thread for each lcore */
+			ret = eal_worker_thread_create(i);
+			if (ret != 0)
+				rte_panic("Cannot create thread\n");
+
+			/* Set thread_name for aid in debugging. */
+			snprintf(thread_name, sizeof(thread_name),
+				"rte-worker-%d", i);
+			rte_thread_set_name(lcore_config[i].thread_id, thread_name);
+
+			ret = rte_thread_set_affinity_by_id(lcore_config[i].thread_id,
+				&lcore_config[i].cpuset);
+			if (ret != 0)
+				rte_panic("Cannot set affinity\n");
+		}
 
-		ret = rte_thread_set_affinity_by_id(lcore_config[i].thread_id,
-			&lcore_config[i].cpuset);
-		if (ret != 0)
-			rte_panic("Cannot set affinity\n");
+		/*
+		 * Launch a dummy function on all worker lcores, so that main lcore
+		 * knows they are all ready when this function returns.
+		 */
+		rte_eal_mp_remote_launch(sync_func, NULL, SKIP_MAIN);
+		rte_eal_mp_wait_lcore();
+		worker_initialized = true;
 	}
 
-	/*
-	 * Launch a dummy function on all worker lcores, so that main lcore
-	 * knows they are all ready when this function returns.
-	 */
-	rte_eal_mp_remote_launch(sync_func, NULL, SKIP_MAIN);
-	rte_eal_mp_wait_lcore();
-
 	/* initialize services so vdevs register service during bus_probe. */
 	ret = rte_service_init();
 	if (ret) {
-- 
2.25.1

[PATCH v5 09/10] eal: declare the reinit via cleanup/init feature unsupported

[okaya]

From: Sinan Kaya <okaya@kernel.org>

Call of rte_eal_init() api after rte_eal_cleanup() possible for
certain application types but this is not an officially supported
feature.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/include/rte_eal.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index 53c4a5519e..348b340f00 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -67,6 +67,11 @@ int rte_eal_iopl_init(void);
  * as possible in the application's main() function.
  * It puts the WORKER lcores in the WAIT state.
  *
+ * @warning
+ * It maybe possisble to call it again after rte_eal_cleanup().
+ * But this usage is dependent on libraries and drivers support which
+ * may not work. Use at your own risk.
+ *
  * @param argc
  *   A non-negative value.  If it is greater than 0, the array members
  *   for argv[0] through argv[argc] (non-inclusive) shall contain pointers
-- 
2.25.1

Re: [PATCH v5 09/10] eal: declare the reinit via cleanup/init feature unsupported

[Stephen Hemminger]

On Thu, 17 Aug 2023 00:28:20 -0400
okaya@kernel.org wrote:

> + * @warning
> + * It maybe possisble to call it again after rte_eal_cleanup().
> + * But this usage is dependent on libraries and drivers support which
> + * may not work. Use at y

Sorry, my spelling error made it in here...

Re: [PATCH v5 00/10] support reinit flow

[Stephen Hemminger]

On Thu, 17 Aug 2023 00:28:11 -0400
okaya@kernel.org wrote:

> From: Sinan Kaya <okaya@kernel.org>
> 
> We want to be able to call rte_eal_init() and rte_eal_cleanup()
> APIs back to back for maintanance reasons.
> 
> Here is a summary of the code we have seen so far:
> 
> 1. some code support getting called multiple times by keeping
> a static variable.
> 2. some code initializes once but never clean up after them and
> don't have a cleanup API.
> 3. some code assumes that they only get called once during the
> lifecycle of the process.
> 
> Most changes in this patch center around following the #1 design
> principle.
> 
> Why?
> 
> It is not always ideal to reinitialize a DPDK process. Memory needs
> to be reinitialized, hugetables need to warm up etc.
> 
> Limitations:
> 
> This sequence could only be done by main lcore, and never ever in a signal handler.
> Do not try and trap signals like abort, bus error, illegal instruction and try to
> use this for recovery. It is a recipe for failure.
> 

This patch series suffers bit rot and does not apply anymore. Needs to be rebase
and resubmit.

There probably needs to be more unit-tests for restart.
Also some documentation for example, for which NIC's does this work?
Probably not all.