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

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 8 Aug 2025 03:23:37 +0000
"Varghese, Vipin" <Vipin.Varghese@amd.com> wrote:

> [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>

Eal_cleanup will reference data that has already been freed by the primary.
That is the problem.