public inbox for dev@dpdk.org
 help / color / mirror / Atom feed
* [PATCH] eal: clean runtime directory files on exit
@ 2026-03-06  6:08 Mounika Kalamandala
  2026-03-06 16:34 ` Stephen Hemminger
  2026-03-06 16:42 ` Stephen Hemminger
  0 siblings, 2 replies; 3+ messages in thread
From: Mounika Kalamandala @ 2026-03-06  6:08 UTC (permalink / raw)
  To: dev, bruce.richardson, aman.deep.singh; +Cc: shaiq.wani, Mounika Kalamandala

Add cleanup of EAL runtime directory artifacts during rte_eal_cleanup().
This removes runtime files created during initialization and ensures
that no stale files remain under /var/run/dpdk/<file-prefix> after the
application terminates.

Update the documentation in multi_proc_support.rst to describe the
secondary process lifetime requirement when the primary process exits.

Signed-off-by: Mounika Kalamandala <mounikax.kalamandala@intel.com>
---
 doc/guides/prog_guide/multi_proc_support.rst | 11 ++++++++++-
 lib/eal/linux/eal.c                          |  4 ++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/multi_proc_support.rst b/doc/guides/prog_guide/multi_proc_support.rst
index a73918a5da..2297f3d79f 100644
--- a/doc/guides/prog_guide/multi_proc_support.rst
+++ b/doc/guides/prog_guide/multi_proc_support.rst
@@ -1,5 +1,5 @@
 ..  SPDX-License-Identifier: BSD-3-Clause
-    Copyright(c) 2010-2014 Intel Corporation.
+    Copyright(c) 2010-2026 Intel Corporation.
 
 Multi-process Support
 =====================
@@ -30,6 +30,15 @@ after a primary process has already configured the hugepage shared memory for th
     Secondary processes which requires access to physical devices in Primary process, must
     be passed with the same allow and block options.
 
+    When the primary process exits, all secondary processes must be terminated,
+    and then restarted after the primary process is re-run to ensure
+    proper re-initialization of shared resources.
+    If secondary processes continue running after the primary process
+    exits, they may experience reduced functionality. In particular,
+    inter-process communication, new memory allocations, and other
+    shared infrastructure will no longer be available, which may lead
+    to unexpected errors.
+
 To support these two process types, and other multi-process setups described later,
 two additional command-line parameters are available to the EAL:
 
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index d848de03d8..18acaf6b9e 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -989,6 +989,10 @@ rte_eal_cleanup(void)
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
 	rte_eal_malloc_heap_cleanup();
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
+			!internal_conf->no_shconf &&
+			eal_clean_runtime_dir() < 0)
+		EAL_LOG(WARNING, "Cannot clean runtime directory on exit");
 	eal_cleanup_config(internal_conf);
 	eal_lcore_var_cleanup();
 	rte_eal_log_cleanup();
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] eal: clean runtime directory files on exit
  2026-03-06  6:08 [PATCH] eal: clean runtime directory files on exit Mounika Kalamandala
@ 2026-03-06 16:34 ` Stephen Hemminger
  2026-03-06 16:42 ` Stephen Hemminger
  1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2026-03-06 16:34 UTC (permalink / raw)
  To: Mounika Kalamandala; +Cc: dev, bruce.richardson, aman.deep.singh, shaiq.wani

On Fri,  6 Mar 2026 06:08:22 +0000
Mounika Kalamandala <mounikax.kalamandala@intel.com> wrote:

> +    When the primary process exits, all secondary processes must be terminated,
> +    and then restarted after the primary process is re-run to ensure
> +    proper re-initialization of shared resources.
> +    If secondary processes continue running after the primary process
> +    exits, they may experience reduced functionality. In particular,
> +    inter-process communication, new memory allocations, and other
> +    shared infrastructure will no longer be available, which may lead
> +    to unexpected errors.
> +

This wording is a little too soft. There are absolutely no guarantees about
functionality when primary exits. Driver may crash, hardware may be rest.
I would choose stronger language.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] eal: clean runtime directory files on exit
  2026-03-06  6:08 [PATCH] eal: clean runtime directory files on exit Mounika Kalamandala
  2026-03-06 16:34 ` Stephen Hemminger
@ 2026-03-06 16:42 ` Stephen Hemminger
  1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2026-03-06 16:42 UTC (permalink / raw)
  To: Mounika Kalamandala; +Cc: dev, bruce.richardson, aman.deep.singh, shaiq.wani

On Fri,  6 Mar 2026 06:08:22 +0000
Mounika Kalamandala <mounikax.kalamandala@intel.com> wrote:

> Add cleanup of EAL runtime directory artifacts during rte_eal_cleanup().
> This removes runtime files created during initialization and ensures
> that no stale files remain under /var/run/dpdk/<file-prefix> after the
> application terminates.
> 
> Update the documentation in multi_proc_support.rst to describe the
> secondary process lifetime requirement when the primary process exits.
> 
> Signed-off-by: Mounika Kalamandala <mounikax.kalamandala@intel.com>
> ---

AI has more corrections here. It does raise the point that primary
process does know if secondary is still running. Also the more unfortunate
and common case happens when primary crashes.

Normally eal_clean_runtime_dir() is called on eal_init, not exit.
Also, clean_runtime_dir does a test flock() of the file and will
fail if it is in use, so it may leave things on shutdown.

Overall, this patch is a no from me.
---

**Patch: eal: clean runtime directory files on exit**

## Errors

**1. Runtime directory cleanup while secondary processes may still be running (potential correctness issue, ~60% confidence)**

The code unconditionally calls `eal_clean_runtime_dir()` in the primary process cleanup path. While the new documentation states secondaries "must be terminated" before the primary exits, the code does not enforce or verify this. If a secondary process is still running when the primary calls `rte_eal_cleanup()`, this removes runtime files (the config, hugepage info, socket files) from under the secondary process, which could cause crashes, SIGBUS on hugepage access, or silent corruption. The documentation addition describes the requirement but the code provides no defense.

Consider at minimum logging a warning if secondary processes are detected as still active, or skipping the directory cleanup when secondaries are still connected. A best-effort check via the mp socket or `/proc` scan for processes with the same file-prefix would make this safer.

**2. Ordering: `eal_clean_runtime_dir()` placed after memory detach**

`eal_clean_runtime_dir()` is called after `rte_eal_memory_detach()` and `rte_eal_malloc_heap_cleanup()`. This means the hugepage mapping files are already detached from this process, which is fine for the primary itself. However, the runtime directory also contains the mp socket and config file. Verify that `eal_clean_runtime_dir()` does not rely on any state cleaned up by the preceding two calls. If it only uses `internal_conf->runtime_dir` (a stack/heap string, not hugepage-backed), the ordering is acceptable — but this deserves confirmation since the function wasn't previously called at this point in the sequence.

## Warnings

**1. Documentation says "must be terminated" but the patch creates a new failure mode rather than preventing it**

The commit message says the patch "ensures that no stale files remain," but it also creates a new hazard: a clean `rte_eal_cleanup()` in the primary now destroys the runtime directory that running secondaries depend on. Previously, stale files were a nuisance; now, a primary exiting with secondaries still running actively destroys their runtime state. The commit message and documentation should acknowledge this behavioral change more clearly, since operators who previously relied on secondaries surviving a primary restart will now face a harder failure.

**2. RST indentation suggests this is inside the existing `.. note::` block — verify placement**

The added text is indented with 4 spaces, matching the indentation of the preceding `.. note::` directive content. If intentional, this is fine. But the content describes a general lifecycle requirement, not just a note about device access options. It might be clearer as a standalone paragraph or its own admonition (e.g., `.. warning::`) rather than appended to the note about allow/block options.

**3. Commit body starts with "Add" — consider whether this needs a "why"**

The commit body describes *what* the patch does but not *why*. What problem was observed with stale runtime files? Under what circumstances do they cause issues? A sentence of motivation (e.g., "After unclean shutdown, stale files in /var/run/dpdk prevent re-initialization") would help reviewers and future readers understand the necessity.

## Info

The subject line is 42 characters, well within the 60-character limit. Tag ordering and format are correct. Copyright year update to 2026 is within valid range.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-06 16:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-06  6:08 [PATCH] eal: clean runtime directory files on exit Mounika Kalamandala
2026-03-06 16:34 ` Stephen Hemminger
2026-03-06 16:42 ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox