RE: [PATCH v7] app/testpmd: monitor state of primary process when using secondary

["Varghese, Vipin" <Vipin.Varghese@amd.com>]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Khadem,

Thank you for sharing but few thoughts here

> -----Original Message-----
> From: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> Sent: Monday, August 4, 2025 5:03 PM
> To: stephen@networkplumber.org; thomas@monjalon.net; Yigit, Ferruh
> <Ferruh.Yigit@amd.com>; andrew.rybchenko@oktetlabs.ru
> Cc: dev@dpdk.org; stable@dpdk.org; Khadem Ullah
> <14pwcse1224@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: stable@dpdk.org
>
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> Signed-off-by: Stephen Hemminger <stephen@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.Varghese@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