[AMD Official Use Only - AMD Internal Distribution Only]
Hi Khadem,
Thank you for sharing but few thoughts here
> -----Original Message-----
> From: Khadem Ullah <14pwcse1...@uetpeshawar.edu.pk>
> Sent: Monday, August 4, 2025 5:03 PM
> To: step...@networkplumber.org; tho...@monjalon.net; Yigit, Ferruh
> <ferruh.yi...@amd.com>; andrew.rybche...@oktetlabs.ru
> Cc: dev@dpdk.org; sta...@dpdk.org; Khadem Ullah
> <14pwcse1...@uetpeshawar.edu.pk>
> Subject: [PATCH v7] app/testpmd: monitor state of primary process when using
> secondary
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> In secondary processes, accessing device after primary has exited will cause
> crash.
>
> This patch adds a mechanism in testpmd to monitor the primary process from the
> secondary process.
> When primary process exits it forces secondary to exit avoiding issues from
> cleanup logic.
>
> Fixes: a550baf24af9 ('app/testpmd: support multi-process')
> Cc: sta...@dpdk.org
>
> Signed-off-by: Khadem Ullah <14pwcse1...@uetpeshawar.edu.pk>
> Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
> ---
> app/test-pmd/testpmd.c | 47
> +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> bb88555328..b7affa6da9 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -101,13 +101,15 @@
> uint16_t verbose_level = 0; /**< Silent by default. */ int testpmd_logtype;
> /**< Log
> type for testpmd logs */
>
> +/* Maximum delay for exiting after primary process. */ #define
> +MONITOR_INTERVAL (500 * 1000)
> +
> /* use main core for command line ? */
> uint8_t interactive = 0;
> uint8_t auto_start = 0;
> uint8_t tx_first;
> char cmdline_filename[PATH_MAX] = {0};
> bool echo_cmdline_file;
> -
> /*
> * NUMA support configuration.
> * When set, the NUMA support attempts to dispatch the allocation of the @@ -
> 4332,6 +4334,38 @@ signal_handler(int signum __rte_unused)
> prompt_exit();
> }
>
> +#ifndef RTE_EXEC_ENV_WINDOWS
> +/* Alarm signal handler, used to check that primary process */ static
> +void monitor_primary(void *arg __rte_unused) {
> + if (rte_eal_primary_proc_alive(NULL)) {
> + rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
> + } else {
> + /*
> + * If primary process exits, then all the device information
> + * is no longer valid. Calling any cleanup code is going to
> + * run into use after free.
> + */
> + fprintf(stderr, "\nPrimary process is no longer active,
> exiting...\n");
> + exit(EXIT_FAILURE);
Indeed, the idea for monitoring and finding if primary is still alive.
When we exit, should not we need graceful exit? If yes, we can not simply use
`exit`, but need to use `eal_cleanup`.
Hence I request for rework with cleanup.
NACK: Vipin Varghese <vipin.vargh...@amd.com>
> + }
> +}
> +
> +/* Setup handler to check when primary exits. */ static int
> +enable_primary_monitor(void)
> +{
> + return rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary,
> +NULL); }
> +
> +static void
> +disable_primary_monitor(void)
> +{
> + rte_eal_alarm_cancel(monitor_primary, NULL); } #endif
> +
> int
> main(int argc, char** argv)
> {
> @@ -4363,6 +4397,12 @@ main(int argc, char** argv)
> rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
> rte_strerror(rte_errno));
>
> +#ifndef RTE_EXEC_ENV_WINDOWS
> + if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> + enable_primary_monitor() < 0)
> + rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
> +#endif
> +
> /* allocate port structures, and init them */
> init_port();
>
> @@ -4556,6 +4596,11 @@ main(int argc, char** argv)
> }
> }
>
> +#ifndef RTE_EXEC_ENV_WINDOWS
> + if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> + disable_primary_monitor(); #endif
> +
> pmd_test_exit();
>
> #ifdef RTE_LIB_PDUMP
> --
> 2.43.0
