[PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

In secondary processes, accessing 'dev->data->dev_private' directly can
cause a segmentation fault if the primary process has exited or the
shared memory is unavailable.

This patch adds a check for dev/data/dev_private and uses
rte_mem_virt2phy to ensure the pointer is still valid.

Fixes: bdad90d12ec8 ('ethdev: change device info get callback to return int')
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 lib/ethdev/rte_ethdev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..03ef446a96 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4079,6 +4079,15 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 
 	if (dev->dev_ops->dev_infos_get == NULL)
 		return -ENOTSUP;
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		(dev == NULL || dev->data == NULL ||
+		dev->data->dev_private == NULL ||
+		rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
 	if (diag != 0) {
 		/* Cleanup already filled in device information */
-- 
2.43.0

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Tue, 22 Jul 2025 07:54:39 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> +		(dev == NULL || dev->data == NULL ||
> +		dev->data->dev_private == NULL ||

dev can't be NULL and checking it here will cause a Coverity warning.

There are many other ethdev calls that will fail if primary dies.
stats, xstats, rx/tx burst, ...

I don't think it is good idea to add checks here.

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]

Hi Stephen,
Can we add only the check that fixes the segfault, or do you mean that it
should be fixed at the PMD level?

Best regards,
Khadem

On Tue, Jul 22, 2025, 18:39 Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Tue, 22 Jul 2025 07:54:39 -0400
> Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>
> > +     if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> > +             (dev == NULL || dev->data == NULL ||
> > +             dev->data->dev_private == NULL ||
>
> dev can't be NULL and checking it here will cause a Coverity warning.
>
> There are many other ethdev calls that will fail if primary dies.
> stats, xstats, rx/tx burst, ...
>
> I don't think it is good idea to add checks here.
>

[-- Attachment #2: Type: text/html, Size: 1290 bytes --]

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Tue, 22 Jul 2025 19:30:41 +0500
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> Hi Stephen,
> Can we add only the check that fixes the segfault, or do you mean that it
> should be fixed at the PMD level?
> 
> Best regards,
> Khadem
> 
> On Tue, Jul 22, 2025, 18:39 Stephen Hemminger <stephen@networkplumber.org>
> wrote:
> 
> > On Tue, 22 Jul 2025 07:54:39 -0400
> > Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
> >  
> > > +     if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> > > +             (dev == NULL || dev->data == NULL ||
> > > +             dev->data->dev_private == NULL ||  
> >
> > dev can't be NULL and checking it here will cause a Coverity warning.
> >
> > There are many other ethdev calls that will fail if primary dies.
> > stats, xstats, rx/tx burst, ...
> >
> > I don't think it is good idea to add checks here.
> >  

It needs to be fixed at the documentation level.
Make sure and document what applications need to do. Rather than adding
more checks in ethdev.

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]

Thanks for the follow up.
Understood. That makes sense. However, I’d like to highlight that
applications should ideally be robust and interactive enough to handle all
edge cases where a segfault or unexpected error might occur. While clear
documentation is certainly important, relying solely on it may not be
sufficient. In my view, potential segfaults should be handled explicitly in
code to ensure stability and to prevent silent failures, especially in
production environments.


On Tue, Jul 22, 2025, 20:42 Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Tue, 22 Jul 2025 19:30:41 +0500
> Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>
> > Hi Stephen,
> > Can we add only the check that fixes the segfault, or do you mean that it
> > should be fixed at the PMD level?
> >
> > Best regards,
> > Khadem
> >
> > On Tue, Jul 22, 2025, 18:39 Stephen Hemminger <
> stephen@networkplumber.org>
> > wrote:
> >
> > > On Tue, 22 Jul 2025 07:54:39 -0400
> > > Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
> > >
> > > > +     if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> > > > +             (dev == NULL || dev->data == NULL ||
> > > > +             dev->data->dev_private == NULL ||
> > >
> > > dev can't be NULL and checking it here will cause a Coverity warning.
> > >
> > > There are many other ethdev calls that will fail if primary dies.
> > > stats, xstats, rx/tx burst, ...
> > >
> > > I don't think it is good idea to add checks here.
> > >
>
> It needs to be fixed at the documentation level.
> Make sure and document what applications need to do. Rather than adding
> more checks in ethdev.
>

[-- Attachment #2: Type: text/html, Size: 2510 bytes --]

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Bruce Richardson]

On Tue, Jul 22, 2025 at 09:01:42PM +0500, Khadem Ullah wrote:
>    Thanks for the follow up.
>    Understood. That makes sense. However, I’d like to highlight that
>    applications should ideally be robust and interactive enough to handle
>    all edge cases where a segfault or unexpected error might occur. While
>    clear documentation is certainly important, relying solely on it may
>    not be sufficient. In my view, potential segfaults should be handled
>    explicitly in code to ensure stability and to prevent silent failures,
>    especially in production environments.
> 
In fairness, where stability is the main concern, I'd generally recommend
avoiding multi-process entirely. Or, if it has to be used, the primary
process should be a minimal slim one, that sets up the ports and memory and
thereafter sleeps so that it should never crash unexpectedly!

Even with that, if any secondary process dies, you'll still have all the
buffers in use by that secondary process leaked, so for any multiprocess
system the only safe behaviour for the system is to restart all processes
if any process unexpectedly terminate.

/Bruce

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

Agree, but I think it's also a good practice to guard against known cases
that are prone to crashes.

On Tue, Jul 22, 2025 at 9:14 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Tue, Jul 22, 2025 at 09:01:42PM +0500, Khadem Ullah wrote:
> >    Thanks for the follow up.
> >    Understood. That makes sense. However, I’d like to highlight that
> >    applications should ideally be robust and interactive enough to handle
> >    all edge cases where a segfault or unexpected error might occur. While
> >    clear documentation is certainly important, relying solely on it may
> >    not be sufficient. In my view, potential segfaults should be handled
> >    explicitly in code to ensure stability and to prevent silent failures,
> >    especially in production environments.
> >
> In fairness, where stability is the main concern, I'd generally recommend
> avoiding multi-process entirely. Or, if it has to be used, the primary
> process should be a minimal slim one, that sets up the ports and memory and
> thereafter sleeps so that it should never crash unexpectedly!
>
> Even with that, if any secondary process dies, you'll still have all the
> buffers in use by that secondary process leaked, so for any multiprocess
> system the only safe behaviour for the system is to restart all processes
> if any process unexpectedly terminate.
>
> /Bruce
>


-- 
Engr. Khadem Ullah,
Software Engineer,
Dreambig Semiconductor Inc
https://dreambigsemi.com/

[-- Attachment #2: Type: text/html, Size: 2532 bytes --]

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Tue, 22 Jul 2025 22:04:32 +0500
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> Agree, but I think it's also a good practice to guard against known cases
> that are prone to crashes.


Right but DPDK chooses performance over API safety.
For example rx/tx burst doesn't check args.

The point is that as a library, if application is doing something wrong
returning error doesn't always help.

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

Right, but performance and reliability are both important. While DPDK
rightly prioritizes performance, some level of reliability should still be
ensured, especially to catch known issues that could lead to instability.

On Tue, Jul 22, 2025, 22:38 Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Tue, 22 Jul 2025 22:04:32 +0500
> Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>
> > Agree, but I think it's also a good practice to guard against known cases
> > that are prone to crashes.
>
>
> Right but DPDK chooses performance over API safety.
> For example rx/tx burst doesn't check args.
>
> The point is that as a library, if application is doing something wrong
> returning error doesn't always help.
>

[-- Attachment #2: Type: text/html, Size: 1164 bytes --]

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Tue, 22 Jul 2025 22:53:08 +0500
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> Right, but performance and reliability are both important. While DPDK
> rightly prioritizes performance, some level of reliability should still be
> ensured, especially to catch known issues that could lead to instability.
> 
> On Tue, Jul 22, 2025, 22:38 Stephen Hemminger <stephen@networkplumber.org>
> wrote:
> 
> > On Tue, 22 Jul 2025 22:04:32 +0500
> > Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
> >  
> > > Agree, but I think it's also a good practice to guard against known cases
> > > that are prone to crashes.  
> >
> >
> > Right but DPDK chooses performance over API safety.
> > For example rx/tx burst doesn't check args.
> >
> > The point is that as a library, if application is doing something wrong
> > returning error doesn't always help.
> >  

The problem is that all those values dev->data and private are shared
between processes without any locking. If the API's are going to MP safe
then they would require locking. The DPDK has made an explicit decision
to not use locking in ethdev control or data path.

You can get away with checking for dev->data being NULL on x86 where
there is data consistency. But on weakly ordered platforms that is not going
to work.

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 1746 bytes --]

Right, those structures are accessed within each PMD, and I believe this
shouldn't pose issues even on weakly ordered systems. As long as the checks
are implemented using proper DPDK APIs, we can ensure correctness by
enforcing appropriate memory ordering where needed, without introducing
full locking overhead.

On Tue, Jul 22, 2025, 23:21 Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Tue, 22 Jul 2025 22:53:08 +0500
> Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>
> > Right, but performance and reliability are both important. While DPDK
> > rightly prioritizes performance, some level of reliability should still
> be
> > ensured, especially to catch known issues that could lead to instability.
> >
> > On Tue, Jul 22, 2025, 22:38 Stephen Hemminger <
> stephen@networkplumber.org>
> > wrote:
> >
> > > On Tue, 22 Jul 2025 22:04:32 +0500
> > > Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
> > >
> > > > Agree, but I think it's also a good practice to guard against known
> cases
> > > > that are prone to crashes.
> > >
> > >
> > > Right but DPDK chooses performance over API safety.
> > > For example rx/tx burst doesn't check args.
> > >
> > > The point is that as a library, if application is doing something wrong
> > > returning error doesn't always help.
> > >
>
> The problem is that all those values dev->data and private are shared
> between processes without any locking. If the API's are going to MP safe
> then they would require locking. The DPDK has made an explicit decision
> to not use locking in ethdev control or data path.
>
> You can get away with checking for dev->data being NULL on x86 where
> there is data consistency. But on weakly ordered platforms that is not
> going
> to work.
>

[-- Attachment #2: Type: text/html, Size: 2546 bytes --]

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Ivan Malov]

There is a difference between control path and data path. Always has been. Yes,
on data path, DPDK has historically sought better performance, but on the slow
path, checks have typically been implemented, even in the flow API, with the
only exception being "asynchronous flow" APIs, which are meant to be fast-path.

Yes, the idea to have a "secondary process reference counter" in 'rte_device'
to be either guarded with its own lock or accessed atomically by 'rte_dev_probe'
and 'rte_dev_remove' (to increment and decrement/check respectively) as well as
by 'rte_eth_dev_close' and 'rte_eth_dev_reset' (to decrement/check) may not be
a hill to die on, to be honest, and might be wrong, so I have no strong opinion.

What scares me most in this idea is that, one may still end up with certain
entry points overlooked, rendering the whole effort worthless.

Thank you.

On Tue, 22 Jul 2025, Stephen Hemminger wrote:

> On Tue, 22 Jul 2025 22:53:08 +0500
> Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>
>> Right, but performance and reliability are both important. While DPDK
>> rightly prioritizes performance, some level of reliability should still be
>> ensured, especially to catch known issues that could lead to instability.
>>
>> On Tue, Jul 22, 2025, 22:38 Stephen Hemminger <stephen@networkplumber.org>
>> wrote:
>>
>>> On Tue, 22 Jul 2025 22:04:32 +0500
>>> Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>>>
>>>> Agree, but I think it's also a good practice to guard against known cases
>>>> that are prone to crashes.
>>>
>>>
>>> Right but DPDK chooses performance over API safety.
>>> For example rx/tx burst doesn't check args.
>>>
>>> The point is that as a library, if application is doing something wrong
>>> returning error doesn't always help.
>>>
>
> The problem is that all those values dev->data and private are shared
> between processes without any locking. If the API's are going to MP safe
> then they would require locking. The DPDK has made an explicit decision
> to not use locking in ethdev control or data path.
>
> You can get away with checking for dev->data being NULL on x86 where
> there is data consistency. But on weakly ordered platforms that is not going
> to work.
>

Re: [PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Tue, 22 Jul 2025 23:05:06 +0400 (+04)
Ivan Malov <ivan.malov@arknetworks.am> wrote:

> There is a difference between control path and data path. Always has been. Yes,
> on data path, DPDK has historically sought better performance, but on the slow
> path, checks have typically been implemented, even in the flow API, with the
> only exception being "asynchronous flow" APIs, which are meant to be fast-path.
> 
> Yes, the idea to have a "secondary process reference counter" in 'rte_device'
> to be either guarded with its own lock or accessed atomically by 'rte_dev_probe'
> and 'rte_dev_remove' (to increment and decrement/check respectively) as well as
> by 'rte_eth_dev_close' and 'rte_eth_dev_reset' (to decrement/check) may not be
> a hill to die on, to be honest, and might be wrong, so I have no strong opinion.
> 
> What scares me most in this idea is that, one may still end up with certain
> entry points overlooked, rendering the whole effort worthless.
> 

Please don't top post.

The DPDK control has (up to now) assumed that control operations are only
done from a single thread on each port. There is also the issue of hotplug
but that is separate. For example, if two threads start and stop the
same port bad thing happen and NIC driver's break. 

This is not well documented and a section needs to go into programmer's guide
thread safety. The whole thread safety section is out of date, and doesn't
reference RCU when it should. It also doesn't cover hot plug or weird secondary
processes that fork.

There is also the issue of how primary/secondary monitoring work.
Right now the secondary monitors primary by periodically polling a lock file.
This inherently a racy method and leads to problems. It needs to be redesigned
to use a blocking method something like spawning a thread in secondary
that uses some part of the existing Unix domain IPC to get notification
when primary crashes or wants to exit. Ideally it would support synchronous
handshake with all primaries and asynchronous case when primary crashes.

The point is that bandaid's in the ethdev layer won't fix it well
enough.

[PATCH] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

In secondary processes, directly accessing 'dev->data->dev_private' can
cause a segmentation fault if the primary process has exited or if the
shared memory is no longer accessible.

This patch adds a safety check using rte_mem_virt2phy(), with an
unlikely() branch hint to minimize performance impact in the fast path.
This ensures 'dev_private' is still valid before accessing it.

Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 lib/ethdev/rte_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..ef5dc55f2e 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 
 	if (dev->dev_ops->dev_infos_get == NULL)
 		return -ENOTSUP;
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
 	if (diag != 0) {
 		/* Cleanup already filled in device information */
-- 
2.43.0

[PATCH v2] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

In secondary processes, directly accessing 'dev->data->dev_private' can
cause a segmentation fault if the primary process has exited or if the
shared memory is no longer accessible.

This patch adds a safety check using rte_mem_virt2phy(), with an
unlikely() branch hint to minimize performance impact in the fast path.
This ensures 'dev_private' is still valid before accessing it.

Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 lib/ethdev/rte_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..ef5dc55f2e 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 
 	if (dev->dev_ops->dev_infos_get == NULL)
 		return -ENOTSUP;
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
 	if (diag != 0) {
 		/* Cleanup already filled in device information */
-- 
2.43.0

Re: [PATCH v2] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]

Secondary application not only breaking on device closing,
it's also getting segfault when we do "show device info all" from secondary
after primary closes:

testpmd> show device info all

********************* Infos for device 0000:03:00.0 *********************
Bus name: pci
Bus information: vendor_id=15b3, device_id=101d
Driver name: mlx5_pci
Devargs:
Connect to socket: 0

Segmentation fault (core dumped)

This patch prevents these crashes and it seems that these fixes should
be in PMD along with the ethdev layer. Some more checks will be added
in the next version to prevent "show device info all" crash.



On Wed, Jul 23, 2025 at 9:50 AM Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
wrote:

> In secondary processes, directly accessing 'dev->data->dev_private' can
> cause a segmentation fault if the primary process has exited or if the
> shared memory is no longer accessible.
>
> This patch adds a safety check using rte_mem_virt2phy(), with an
> unlikely() branch hint to minimize performance impact in the fast path.
> This ensures 'dev_private' is still valid before accessing it.
>
> Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return
> int")
> Cc: stable@dpdk.org
>
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> ---
>  lib/ethdev/rte_ethdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index dd7c00bc94..ef5dc55f2e 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct
> rte_eth_dev_info *dev_info)
>
>         if (dev->dev_ops->dev_infos_get == NULL)
>                 return -ENOTSUP;
> +       if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> +               unlikely(rte_mem_virt2phy(dev->data->dev_private) ==
> RTE_BAD_PHYS_ADDR)) {
> +                       RTE_ETHDEV_LOG_LINE(ERR,
> +                       "Secondary: dev_private not accessible (primary
> exited?)");
> +                       rte_errno = ENODEV;
> +                       return -rte_errno;
> +       }
>         diag = dev->dev_ops->dev_infos_get(dev, dev_info);
>         if (diag != 0) {
>                 /* Cleanup already filled in device information */
> --
> 2.43.0
>
>

-- 
Engr. Khadem Ullah,
Software Engineer,
Dreambig Semiconductor Inc
https://dreambigsemi.com/

[-- Attachment #2: Type: text/html, Size: 3832 bytes --]

Re: [PATCH v2] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 2773 bytes --]

Please check v3 of this patch.

On Wed, Jul 23, 2025 at 5:19 PM Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
wrote:

> Secondary application not only breaking on device closing,
> it's also getting segfault when we do "show device info all" from secondary
> after primary closes:
>
> testpmd> show device info all
>
> ********************* Infos for device 0000:03:00.0 *********************
> Bus name: pci
> Bus information: vendor_id=15b3, device_id=101d
> Driver name: mlx5_pci
> Devargs:
> Connect to socket: 0
>
> Segmentation fault (core dumped)
>
> This patch prevents these crashes and it seems that these fixes should be in PMD along with the ethdev layer. Some more checks will be added in the next version to prevent "show device info all" crash.
>
>
>
> On Wed, Jul 23, 2025 at 9:50 AM Khadem Ullah <
> 14pwcse1224@uetpeshawar.edu.pk> wrote:
>
>> In secondary processes, directly accessing 'dev->data->dev_private' can
>> cause a segmentation fault if the primary process has exited or if the
>> shared memory is no longer accessible.
>>
>> This patch adds a safety check using rte_mem_virt2phy(), with an
>> unlikely() branch hint to minimize performance impact in the fast path.
>> This ensures 'dev_private' is still valid before accessing it.
>>
>> Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return
>> int")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
>> ---
>>  lib/ethdev/rte_ethdev.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index dd7c00bc94..ef5dc55f2e 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct
>> rte_eth_dev_info *dev_info)
>>
>>         if (dev->dev_ops->dev_infos_get == NULL)
>>                 return -ENOTSUP;
>> +       if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
>> +               unlikely(rte_mem_virt2phy(dev->data->dev_private) ==
>> RTE_BAD_PHYS_ADDR)) {
>> +                       RTE_ETHDEV_LOG_LINE(ERR,
>> +                       "Secondary: dev_private not accessible (primary
>> exited?)");
>> +                       rte_errno = ENODEV;
>> +                       return -rte_errno;
>> +       }
>>         diag = dev->dev_ops->dev_infos_get(dev, dev_info);
>>         if (diag != 0) {
>>                 /* Cleanup already filled in device information */
>> --
>> 2.43.0
>>
>>
>
> --
> Engr. Khadem Ullah,
> Software Engineer,
> Dreambig Semiconductor Inc
> https://dreambigsemi.com/
>


-- 
Engr. Khadem Ullah,
Software Engineer,
Dreambig Semiconductor Inc
https://dreambigsemi.com/

[-- Attachment #2: Type: text/html, Size: 4968 bytes --]

Re: [PATCH v2] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Ivan Malov]

[-- Attachment #1: Type: text/plain, Size: 3233 bytes --]

Hi Khadem,

On Wed, 23 Jul 2025, Khadem Ullah wrote:

> Please check v3 of this patch.

Checked. In the meantime, you're not overlooking to mark previous versions [1]
with status 'SUPERSEDED' in DPDK Patchwork, are you?

[1] https://patches.dpdk.org/project/dpdk/patch/20250723045022.1580829-1-14pwcse1224@uetpeshawar.edu.pk/

> 
> On Wed, Jul 23, 2025 at 5:19 PM Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
> 
> Secondary application not only breaking on device closing,
> it's also getting segfault when we do "show device info all" from secondary
> after primary closes:
> testpmd> show device info all
> ********************* Infos for device 0000:03:00.0 *********************
> Bus name: pci
> Bus information: vendor_id=15b3, device_id=101d
> Driver name: mlx5_pci
> Devargs:
> Connect to socket: 0
> Segmentation fault (core dumped)
> This patch prevents these crashes and it seems that these fixes should be in PMD along with the ethdev layer. Some more checks will be added in the next version to prevent "show device i
> nfo all" crash.
> 
> 
> On Wed, Jul 23, 2025 at 9:50 AM Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>       In secondary processes, directly accessing 'dev->data->dev_private' can
>       cause a segmentation fault if the primary process has exited or if the
>       shared memory is no longer accessible.
>
>       This patch adds a safety check using rte_mem_virt2phy(), with an
>       unlikely() branch hint to minimize performance impact in the fast path.
>       This ensures 'dev_private' is still valid before accessing it.
>
>       Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
>       Cc: stable@dpdk.org
>
>       Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
>       ---
>        lib/ethdev/rte_ethdev.c | 7 +++++++
>        1 file changed, 7 insertions(+)
>
>       diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>       index dd7c00bc94..ef5dc55f2e 100644
>       --- a/lib/ethdev/rte_ethdev.c
>       +++ b/lib/ethdev/rte_ethdev.c
>       @@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>
>               if (dev->dev_ops->dev_infos_get == NULL)
>                       return -ENOTSUP;
>       +       if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
>       +               unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
>       +                       RTE_ETHDEV_LOG_LINE(ERR,
>       +                       "Secondary: dev_private not accessible (primary exited?)");
>       +                       rte_errno = ENODEV;
>       +                       return -rte_errno;
>       +       }
>               diag = dev->dev_ops->dev_infos_get(dev, dev_info);
>               if (diag != 0) {
>                       /* Cleanup already filled in device information */
>       --
>       2.43.0
> 
> 
> 
> --
> Engr. Khadem Ullah,
> Software Engineer,
> Dreambig Semiconductor Inc
> https://dreambigsemi.com/
> 
> 
> 
> --
> Engr. Khadem Ullah,
> Software Engineer,
> Dreambig Semiconductor Inc
> https://dreambigsemi.com/
> 
>

Re: [PATCH v2] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 3522 bytes --]

Thanks, no.
Can, I ?

On Wed, Jul 23, 2025, 18:24 Ivan Malov <ivan.malov@arknetworks.am> wrote:

> Hi Khadem,
>
> On Wed, 23 Jul 2025, Khadem Ullah wrote:
>
> > Please check v3 of this patch.
>
> Checked. In the meantime, you're not overlooking to mark previous versions
> [1]
> with status 'SUPERSEDED' in DPDK Patchwork, are you?
>
> [1]
> https://patches.dpdk.org/project/dpdk/patch/20250723045022.1580829-1-14pwcse1224@uetpeshawar.edu.pk/
>
> >
> > On Wed, Jul 23, 2025 at 5:19 PM Khadem Ullah <
> 14pwcse1224@uetpeshawar.edu.pk> wrote:
> >
> > Secondary application not only breaking on device closing,
> > it's also getting segfault when we do "show device info all" from
> secondary
> > after primary closes:
> > testpmd> show device info all
> > ********************* Infos for device 0000:03:00.0 *********************
> > Bus name: pci
> > Bus information: vendor_id=15b3, device_id=101d
> > Driver name: mlx5_pci
> > Devargs:
> > Connect to socket: 0
> > Segmentation fault (core dumped)
> > This patch prevents these crashes and it seems that these fixes should
> be in PMD along with the ethdev layer. Some more checks will be added in
> the next version to prevent "show device i
> > nfo all" crash.
> >
> >
> > On Wed, Jul 23, 2025 at 9:50 AM Khadem Ullah <
> 14pwcse1224@uetpeshawar.edu.pk> wrote:
> >       In secondary processes, directly accessing
> 'dev->data->dev_private' can
> >       cause a segmentation fault if the primary process has exited or if
> the
> >       shared memory is no longer accessible.
> >
> >       This patch adds a safety check using rte_mem_virt2phy(), with an
> >       unlikely() branch hint to minimize performance impact in the fast
> path.
> >       This ensures 'dev_private' is still valid before accessing it.
> >
> >       Fixes: bdad90d12ec8 ("ethdev: change device info get callback to
> return int")
> >       Cc: stable@dpdk.org
> >
> >       Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> >       ---
> >        lib/ethdev/rte_ethdev.c | 7 +++++++
> >        1 file changed, 7 insertions(+)
> >
> >       diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> >       index dd7c00bc94..ef5dc55f2e 100644
> >       --- a/lib/ethdev/rte_ethdev.c
> >       +++ b/lib/ethdev/rte_ethdev.c
> >       @@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id,
> struct rte_eth_dev_info *dev_info)
> >
> >               if (dev->dev_ops->dev_infos_get == NULL)
> >                       return -ENOTSUP;
> >       +       if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> >       +               unlikely(rte_mem_virt2phy(dev->data->dev_private)
> == RTE_BAD_PHYS_ADDR)) {
> >       +                       RTE_ETHDEV_LOG_LINE(ERR,
> >       +                       "Secondary: dev_private not accessible
> (primary exited?)");
> >       +                       rte_errno = ENODEV;
> >       +                       return -rte_errno;
> >       +       }
> >               diag = dev->dev_ops->dev_infos_get(dev, dev_info);
> >               if (diag != 0) {
> >                       /* Cleanup already filled in device information */
> >       --
> >       2.43.0
> >
> >
> >
> > --
> > Engr. Khadem Ullah,
> > Software Engineer,
> > Dreambig Semiconductor Inc
> > https://dreambigsemi.com/
> >
> >
> >
> > --
> > Engr. Khadem Ullah,
> > Software Engineer,
> > Dreambig Semiconductor Inc
> > https://dreambigsemi.com/
> >
> >

[-- Attachment #2: Type: text/html, Size: 5182 bytes --]

Re: [PATCH v2] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Ivan Malov]

[-- Attachment #1: Type: text/plain, Size: 4393 bytes --]

On Wed, 23 Jul 2025, Khadem Ullah wrote:

> Thanks, no. Can, I ?

Typically, the patch author (as indicated in their respective e-mail address)
can sign up for patchwork account, sign in and mark any of their own patches as
superseded, in order to help the delegated reviewers tell new from outdated.

Thank you.

> 
> On Wed, Jul 23, 2025, 18:24 Ivan Malov <ivan.malov@arknetworks.am> wrote:
>       Hi Khadem,
>
>       On Wed, 23 Jul 2025, Khadem Ullah wrote:
>
>       > Please check v3 of this patch.
>
>       Checked. In the meantime, you're not overlooking to mark previous versions [1]
>       with status 'SUPERSEDED' in DPDK Patchwork, are you?
>
>       [1] https://patches.dpdk.org/project/dpdk/patch/20250723045022.1580829-1-14pwcse1224@uetpeshawar.edu.pk/
>
>       >
>       > On Wed, Jul 23, 2025 at 5:19 PM Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>       >
>       > Secondary application not only breaking on device closing,
>       > it's also getting segfault when we do "show device info all" from secondary
>       > after primary closes:
>       > testpmd> show device info all
>       > ********************* Infos for device 0000:03:00.0 *********************
>       > Bus name: pci
>       > Bus information: vendor_id=15b3, device_id=101d
>       > Driver name: mlx5_pci
>       > Devargs:
>       > Connect to socket: 0
>       > Segmentation fault (core dumped)
>       > This patch prevents these crashes and it seems that these fixes should be in PMD along with the ethdev layer. Some more checks will be added in the next version to prevent
>       "show device i
>       > nfo all" crash.
>       >
>       >
>       > On Wed, Jul 23, 2025 at 9:50 AM Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>       >       In secondary processes, directly accessing 'dev->data->dev_private' can
>       >       cause a segmentation fault if the primary process has exited or if the
>       >       shared memory is no longer accessible.
>       >
>       >       This patch adds a safety check using rte_mem_virt2phy(), with an
>       >       unlikely() branch hint to minimize performance impact in the fast path.
>       >       This ensures 'dev_private' is still valid before accessing it.
>       >
>       >       Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
>       >       Cc: stable@dpdk.org
>       >
>       >       Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
>       >       ---
>       >        lib/ethdev/rte_ethdev.c | 7 +++++++
>       >        1 file changed, 7 insertions(+)
>       >
>       >       diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>       >       index dd7c00bc94..ef5dc55f2e 100644
>       >       --- a/lib/ethdev/rte_ethdev.c
>       >       +++ b/lib/ethdev/rte_ethdev.c
>       >       @@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>       >
>       >               if (dev->dev_ops->dev_infos_get == NULL)
>       >                       return -ENOTSUP;
>       >       +       if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
>       >       +               unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
>       >       +                       RTE_ETHDEV_LOG_LINE(ERR,
>       >       +                       "Secondary: dev_private not accessible (primary exited?)");
>       >       +                       rte_errno = ENODEV;
>       >       +                       return -rte_errno;
>       >       +       }
>       >               diag = dev->dev_ops->dev_infos_get(dev, dev_info);
>       >               if (diag != 0) {
>       >                       /* Cleanup already filled in device information */
>       >       --
>       >       2.43.0
>       >
>       >
>       >
>       > --
>       > Engr. Khadem Ullah,
>       > Software Engineer,
>       > Dreambig Semiconductor Inc
>       > https://dreambigsemi.com/
>       >
>       >
>       >
>       > --
>       > Engr. Khadem Ullah,
>       > Software Engineer,
>       > Dreambig Semiconductor Inc
>       > https://dreambigsemi.com/
>       >
>       >
> 
> 
>

[PATCH] [PATCH v3] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

In secondary processes, directly accessing 'dev->data->dev_private' can
cause a segmentation fault if the primary process has exited or if the
shared memory is no longer accessible.

Secondary application not only breaking on device closing,
but also getting segfault when we do "show device info all" from secondary
after primary closes.

This patch adds safety checks while using rte_mem_virt2phy(), with an
unlikely() branch hint to minimize performance impact in the fast path.
This ensures 'dev_private' is still valid before accessing it.

Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 lib/ethdev/rte_ethdev.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..343e156a4f 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 
 	if (dev->dev_ops->dev_infos_get == NULL)
 		return -ENOTSUP;
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
 	if (diag != 0) {
 		/* Cleanup already filled in device information */
@@ -4307,7 +4314,13 @@ rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 			port_id);
 		return -EINVAL;
 	}
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		(dev->data->mac_addrs == NULL)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
 
 	rte_eth_trace_macaddr_get(port_id, mac_addr);
-- 
2.43.0

Re: [PATCH] [PATCH v3] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Ivan Malov]

Hi Khadem,

On Wed, 23 Jul 2025, Khadem Ullah wrote:

> In secondary processes, directly accessing 'dev->data->dev_private' can
> cause a segmentation fault if the primary process has exited or if the
> shared memory is no longer accessible.
>
> Secondary application not only breaking on device closing,
> but also getting segfault when we do "show device info all" from secondary
> after primary closes.
>
> This patch adds safety checks while using rte_mem_virt2phy(), with an
> unlikely() branch hint to minimize performance impact in the fast path.
> This ensures 'dev_private' is still valid before accessing it.
>
> Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
> Cc: stable@dpdk.org
>
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> ---
> lib/ethdev/rte_ethdev.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index dd7c00bc94..343e156a4f 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>
> 	if (dev->dev_ops->dev_infos_get == NULL)
> 		return -ENOTSUP;
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> +		unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
> +			RTE_ETHDEV_LOG_LINE(ERR,
> +			"Secondary: dev_private not accessible (primary exited?)");
> +			rte_errno = ENODEV;
> +			return -rte_errno;
> +	}
> 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
> 	if (diag != 0) {
> 		/* Cleanup already filled in device information */
> @@ -4307,7 +4314,13 @@ rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
> 			port_id);
> 		return -EINVAL;
> 	}
> -
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> +		(dev->data->mac_addrs == NULL)) {
> +			RTE_ETHDEV_LOG_LINE(ERR,
> +			"Secondary: dev_private not accessible (primary exited?)");
> +			rte_errno = ENODEV;
> +			return -rte_errno;
> +	}
> 	rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
>
> 	rte_eth_trace_macaddr_get(port_id, mac_addr);

I see one more API has been augmented with the check. But community members may
still argue this is not robust, as many other APIs will also fail. So, even if
the task was to augment as many APIs as possible with the check, then the check
would still be required to be factorised/generalised somehow. What do you think?

Please also note that there are already macro invocations in many of these APIs,
for example, RTE_ETH_VALID_PORTID_OR_ERR_RET. Could be convenient.

Thank you.

> -- 
> 2.43.0
>
>

Re: [PATCH] [PATCH v3] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 3294 bytes --]

Hi Ivan, agree. I think we can atleast currently guard all the known
crashes.

Sure, I will check the macro and get back to you.

Thank you!

On Wed, Jul 23, 2025, 18:19 Ivan Malov <ivan.malov@arknetworks.am> wrote:

> Hi Khadem,
>
> On Wed, 23 Jul 2025, Khadem Ullah wrote:
>
> > In secondary processes, directly accessing 'dev->data->dev_private' can
> > cause a segmentation fault if the primary process has exited or if the
> > shared memory is no longer accessible.
> >
> > Secondary application not only breaking on device closing,
> > but also getting segfault when we do "show device info all" from
> secondary
> > after primary closes.
> >
> > This patch adds safety checks while using rte_mem_virt2phy(), with an
> > unlikely() branch hint to minimize performance impact in the fast path.
> > This ensures 'dev_private' is still valid before accessing it.
> >
> > Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return
> int")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> > ---
> > lib/ethdev/rte_ethdev.c | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> > index dd7c00bc94..343e156a4f 100644
> > --- a/lib/ethdev/rte_ethdev.c
> > +++ b/lib/ethdev/rte_ethdev.c
> > @@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct
> rte_eth_dev_info *dev_info)
> >
> >       if (dev->dev_ops->dev_infos_get == NULL)
> >               return -ENOTSUP;
> > +     if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> > +             unlikely(rte_mem_virt2phy(dev->data->dev_private) ==
> RTE_BAD_PHYS_ADDR)) {
> > +                     RTE_ETHDEV_LOG_LINE(ERR,
> > +                     "Secondary: dev_private not accessible (primary
> exited?)");
> > +                     rte_errno = ENODEV;
> > +                     return -rte_errno;
> > +     }
> >       diag = dev->dev_ops->dev_infos_get(dev, dev_info);
> >       if (diag != 0) {
> >               /* Cleanup already filled in device information */
> > @@ -4307,7 +4314,13 @@ rte_eth_macaddr_get(uint16_t port_id, struct
> rte_ether_addr *mac_addr)
> >                       port_id);
> >               return -EINVAL;
> >       }
> > -
> > +     if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> > +             (dev->data->mac_addrs == NULL)) {
> > +                     RTE_ETHDEV_LOG_LINE(ERR,
> > +                     "Secondary: dev_private not accessible (primary
> exited?)");
> > +                     rte_errno = ENODEV;
> > +                     return -rte_errno;
> > +     }
> >       rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
> >
> >       rte_eth_trace_macaddr_get(port_id, mac_addr);
>
> I see one more API has been augmented with the check. But community
> members may
> still argue this is not robust, as many other APIs will also fail. So,
> even if
> the task was to augment as many APIs as possible with the check, then the
> check
> would still be required to be factorised/generalised somehow. What do you
> think?
>
> Please also note that there are already macro invocations in many of these
> APIs,
> for example, RTE_ETH_VALID_PORTID_OR_ERR_RET. Could be convenient.
>
> Thank you.
>
> > --
> > 2.43.0
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 4493 bytes --]

Re: [PATCH] [PATCH v3] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Wed, 23 Jul 2025 18:34:04 +0500
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> Hi Ivan, agree. I think we can atleast currently guard all the known
> crashes.
> 
> Sure, I will check the macro and get back to you.
> 
> Thank you!
> 
> On Wed, Jul 23, 2025, 18:19 Ivan Malov <ivan.malov@arknetworks.am> wrote:
> 
> > Hi Khadem,
> >
> > On Wed, 23 Jul 2025, Khadem Ullah wrote:
> >  
> > > In secondary processes, directly accessing 'dev->data->dev_private' can
> > > cause a segmentation fault if the primary process has exited or if the
> > > shared memory is no longer accessible.
> > >
> > > Secondary application not only breaking on device closing,
> > > but also getting segfault when we do "show device info all" from  
> > secondary  
> > > after primary closes.
> > >
> > > This patch adds safety checks while using rte_mem_virt2phy(), with an
> > > unlikely() branch hint to minimize performance impact in the fast path.
> > > This ensures 'dev_private' is still valid before accessing it.
> > >
> > > Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return  
> > int")  
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> > > ---
> > > lib/ethdev/rte_ethdev.c | 15 ++++++++++++++-
> > > 1 file changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> > > index dd7c00bc94..343e156a4f 100644
> > > --- a/lib/ethdev/rte_ethdev.c
> > > +++ b/lib/ethdev/rte_ethdev.c
> > > @@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct  
> > rte_eth_dev_info *dev_info)  
> > >
> > >       if (dev->dev_ops->dev_infos_get == NULL)
> > >               return -ENOTSUP;
> > > +     if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> > > +             unlikely(rte_mem_virt2phy(dev->data->dev_private) ==  
> > RTE_BAD_PHYS_ADDR)) {  
> > > +                     RTE_ETHDEV_LOG_LINE(ERR,
> > > +                     "Secondary: dev_private not accessible (primary  
> > exited?)");  
> > > +                     rte_errno = ENODEV;
> > > +                     return -rte_errno;
> > > +     }
> > >       diag = dev->dev_ops->dev_infos_get(dev, dev_info);
> > >       if (diag != 0) {
> > >               /* Cleanup already filled in device information */
> > > @@ -4307,7 +4314,13 @@ rte_eth_macaddr_get(uint16_t port_id, struct  
> > rte_ether_addr *mac_addr)  
> > >                       port_id);
> > >               return -EINVAL;
> > >       }
> > > -
> > > +     if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> > > +             (dev->data->mac_addrs == NULL)) {
> > > +                     RTE_ETHDEV_LOG_LINE(ERR,
> > > +                     "Secondary: dev_private not accessible (primary  
> > exited?)");  
> > > +                     rte_errno = ENODEV;
> > > +                     return -rte_errno;
> > > +     }
> > >       rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
> > >
> > >       rte_eth_trace_macaddr_get(port_id, mac_addr);  
> >
> > I see one more API has been augmented with the check. But community
> > members may
> > still argue this is not robust, as many other APIs will also fail. So,
> > even if
> > the task was to augment as many APIs as possible with the check, then the
> > check
> > would still be required to be factorised/generalised somehow. What do you
> > think?
> >
> > Please also note that there are already macro invocations in many of these
> > APIs,
> > for example, RTE_ETH_VALID_PORTID_OR_ERR_RET. Could be convenient.
> >
> > Thank you.
> >  
> > > --
> > > 2.43.0
> > >
> > >  
> >  

No top posting.

How are you monitoring the primary? Lets fix that

Re: [PATCH] [PATCH v3] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

Hi Stephen,

Thanks for the feedback.

This patch doesn’t monitor the primary process, it only adds basic checks to prevent crashes when the secondary 
process calls certain APIs after the primary has exited. Right now, I think, only specific apps like `dumpcap` 
use `enable_primary_monitor`, but most (like `testpmd`) don’t monitor the primary at all.

I understand your point about improving the whole primary-secondary monitoring system using a better method
 like Unix domain sockets instead of lock files. That would definitely be a cleaner and more reliable solution, 
and I’d be glad to help with that in future work.

However, even with better monitoring, we would still need the checks in this patch. They’re basic safety checks 
that help avoid crashes in any case, for example, when the API is called with null or invalid data. So I see 
this patch as a necessary short-term fix that doesn’t replace, but supports, any long-term redesign.

Best regards,
Khadem

Re: [PATCH] [PATCH v3] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=yes, Size: 250 bytes --]

Hi Stephen,

Thanks for the feedback.

I’ve submitted v4 of this patch, which includes a mechanism to monitor the primary testpmd process from the secondary testpmd. This helps prevent crashes when the primary testpmd exits.

Best regards,
Khadem

[PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

In secondary processes, directly accessing 'dev->data->dev_private' can
cause a segmentation fault if the primary process has exited or if the
shared memory is no longer accessible.

The secondary process not only crashes on device close but also segfaults
when executing commands like "show device info all" after the primary has
exited.

This patch adds a mechanism in testpmd to monitor the primary process from the
secondary using `rte_eal_primary_proc_alive()`, and introduces safety
checks before dereferencing 'dev_private'. The `rte_mem_virt2phy()` call
is also guarded to avoid access to invalid memory regions, with
`unlikely()` hints to minimize impact in the fast path.

Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 app/test-pmd/testpmd.c  | 56 ++++++++++++++++++++++++++++++++++++++---
 lib/ethdev/rte_ethdev.c | 15 ++++++++++-
 2 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bb88555328..25e55de281 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -101,13 +101,17 @@
 uint16_t verbose_level = 0; /**< Silent by default. */
 int testpmd_logtype; /**< Log type for testpmd logs */
 
+static volatile uint8_t quit_signal;
+
+/* 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
@@ -2281,7 +2285,7 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 			fc->total_cycles += tsc - prev_tsc;
 			prev_tsc = tsc;
 		}
-	} while (! fc->stopped);
+	} while (!fc->stopped && !quit_signal);
 }
 
 static int
@@ -4329,9 +4333,51 @@ static void
 signal_handler(int signum __rte_unused)
 {
 	f_quit = 1;
+	quit_signal = 1;
 	prompt_exit();
 }
 
+
+/* Alarm signal handler, used to check that primary process */
+static void
+monitor_primary(void *arg __rte_unused)
+{
+	if (rte_atomic_load_explicit(&quit_signal, rte_memory_order_relaxed))
+		return;
+
+	if (rte_eal_primary_proc_alive(NULL)) {
+		rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+	} else {
+		fprintf(stderr,
+			"Primary process is no longer active, exiting...\n");
+		quit_signal = 1;
+		f_quit = 1;
+		prompt_exit();
+	}
+}
+
+/* Setup handler to check when primary exits. */
+static void
+enable_primary_monitor(void)
+{
+	int ret;
+
+	/* Once primary exits, so will pdump. */
+	ret = rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Fail to enable monitor:%d\n", ret);
+}
+
+static void
+disable_primary_monitor(void)
+{
+	int ret;
+
+	ret = rte_eal_alarm_cancel(monitor_primary, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Fail to disable monitor:%d\n", ret);
+}
+
 int
 main(int argc, char** argv)
 {
@@ -4362,7 +4408,8 @@ main(int argc, char** argv)
 	if (diag < 0)
 		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
 			 rte_strerror(rte_errno));
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		enable_primary_monitor();
 	/* allocate port structures, and init them */
 	init_port();
 
@@ -4571,7 +4618,8 @@ main(int argc, char** argv)
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE, "Cannot unregister for ethdev events");
 
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		disable_primary_monitor();
 	ret = rte_eal_cleanup();
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE,
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..343e156a4f 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 
 	if (dev->dev_ops->dev_infos_get == NULL)
 		return -ENOTSUP;
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
 	if (diag != 0) {
 		/* Cleanup already filled in device information */
@@ -4307,7 +4314,13 @@ rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 			port_id);
 		return -EINVAL;
 	}
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		(dev->data->mac_addrs == NULL)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
 
 	rte_eth_trace_macaddr_get(port_id, mac_addr);
-- 
2.43.0

Re: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Fri, 25 Jul 2025 08:55:26 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> In secondary processes, directly accessing 'dev->data->dev_private' can
> cause a segmentation fault if the primary process has exited or if the
> shared memory is no longer accessible.
> 
> The secondary process not only crashes on device close but also segfaults
> when executing commands like "show device info all" after the primary has
> exited.
> 
> This patch adds a mechanism in testpmd to monitor the primary process from the
> secondary using `rte_eal_primary_proc_alive()`, and introduces safety
> checks before dereferencing 'dev_private'. The `rte_mem_virt2phy()` call
> is also guarded to avoid access to invalid memory regions, with
> `unlikely()` hints to minimize impact in the fast path.
> 
> Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> ---
>  app/test-pmd/testpmd.c  | 56 ++++++++++++++++++++++++++++++++++++++---
>  lib/ethdev/rte_ethdev.c | 15 ++++++++++-
>  2 files changed, 66 insertions(+), 5 deletions(-)


The test-pmd monitoring is good, and should have done back when secondary
support was added. Please submit just that part of the patch and add a Fixes tag.
You can use the f_quit flag like the signal handler.

The use of volatile and standard atomic overlap.
Testpmd is older code so just uses volatile so you don't need to use
rte_atomic there. Just set f_quit in the alarm handler and avoid restarting alarm
if primary exits.

In future, want to add something like:
	rte_eal_primary_proc_notifier(void (*cb)(void *), void *);
and use some variation of pidfd or inotify to call the function when primary
dies either by termination or abort.

The ethdev part is not necessary, and really won't address the race; leave it out.

Re: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

Hi Stephen,

Many Thanks for the feedback.
I have addressed all the comments on the patch, I have removed the unnecessary check in ethdev, the only one check in rte_ethdev seems to be a neccessary one, as without it, it can still generate segfault while secondary exists after primary.

Best regards,
Khadem

Re: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Tue, 29 Jul 2025 01:42:29 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> Hi Stephen,
> 
> Many Thanks for the feedback.
> I have addressed all the comments on the patch, I have removed the unnecessary check in ethdev, the only one check in rte_ethdev seems to be a neccessary one, as without it, it can still generate segfault while secondary exists after primary.
> 
> Best regards,
> Khadem

Did a bunch more testing, and the problem is much deeper than your initial observation.

If primary process exits, the data structures all over secondary process are complete
garbage. Lots of use after free, linked lists that point to freed data ...

The root cause is that when secondary process starts, it requests current device
state (buses, devices, etc) from primary. When primary process exits it removes
all devices; but there is no mechanism to tell secondary!

For now, the only thing I can think of is to have the secondary process monitor
in testpmd do an immediate exit, no cleanups.

Re: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

Hi Stephen,

Many thanks for diving deeper into the issue and sharing the insights.

I agree. given that the primary tears down all the devices on exit, the secondary is left with dangling pointers and inconsistent state. Without a mechanism to notify or synchronize that teardown, I think it is the only option for the secondary to exit immediately, without attempting any cleanups, once it detects the primary has exited.


Best regards,
Khadem

RE: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Varghese, Vipin]

[Public]

Since somehow the email are split it is difficult to see the indexing

>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Stephen,
>
> Many thanks for diving deeper into the issue and sharing the insights.
>
> I agree. given that the primary tears down all the devices on exit, the secondary is
> left with dangling pointers and inconsistent state.

Secondary if PMD should not be attempting to cleanup (especially for ethdev_ptr shared from primary) I agree to that.
But should not be covered in library or PMD. While cleanup for other needs to be done properly

 Without a mechanism to notify or synchronize that teardown,

You already have health check added in the code for identify if primay is still alive or not..
There used to be MP thread spawned which actually piggy back the communication. Is this broken? Can you please point to git-blame where MP thread does not communicate or notify the cleanup or synchronise (lets fix it asap).


 I think it is the only option for the secondary to exit
> immediately, without attempting any cleanups, once it detects the primary has
> exited.
>
>
> Best regards,
> Khadem

Re: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

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

> [Public]
> 
> Since somehow the email are split it is difficult to see the indexing
> 
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > Hi Stephen,
> >
> > Many thanks for diving deeper into the issue and sharing the insights.
> >
> > I agree. given that the primary tears down all the devices on exit, the secondary is
> > left with dangling pointers and inconsistent state.  
> 
> Secondary if PMD should not be attempting to cleanup (especially for ethdev_ptr shared from primary) I agree to that.
> But should not be covered in library or PMD. While cleanup for other needs to be done properly
> 
>  Without a mechanism to notify or synchronize that teardown,
> 
> You already have health check added in the code for identify if primay is still alive or not..
> There used to be MP thread spawned which actually piggy back the communication. Is this broken? Can you please point to git

There is no easy way to handle the case where primary crashes; leaving secondary process with
pointers to dead data.

RE: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Varghese, Vipin]

[Public]

Snipped

> > Since somehow the email are split it is difficult to see the indexing
> >
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > Hi Stephen,
> > >
> > > Many thanks for diving deeper into the issue and sharing the insights.
> > >
> > > I agree. given that the primary tears down all the devices on exit,
> > > the secondary is left with dangling pointers and inconsistent state.
> >
> > Secondary if PMD should not be attempting to cleanup (especially for ethdev_ptr
> shared from primary) I agree to that.
> > But should not be covered in library or PMD. While cleanup for other
> > needs to be done properly
> >
> >  Without a mechanism to notify or synchronize that teardown,
> >
> > You already have health check added in the code for identify if primay is still alive
> or not..
> > There used to be MP thread spawned which actually piggy back the
> > communication. Is this broken? Can you please point to git
>
> There is no easy way to handle the case where primary crashes; leaving
> secondary process with pointers to dead data.

Thank you for sharing, but as shared the other threads (this topic has 3), the reason for the patch is because once the primary is dead it causes secondary to crash (segment fault).

As shared in earlier email at least till 22.11 LTS and 23.03 (as it was my last testing with multi-process) I did not encounter these.

@Khadem Ullah can you please share which version of DPDK you are noticing this failure?

Re: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]

Hi Vipin,

These crashes are specific to the testpmd primary and testpmd
secondary application.
Please check
https://patches.dpdk.org/project/dpdk/patch/20250804113322.53650-1-14pwcse1224@uetpeshawar.edu.pk/
The details are provided over there.

Regards,
Khadem

On Mon, Aug 11, 2025 at 3:19 PM Varghese, Vipin <Vipin.Varghese@amd.com>
wrote:

> [Public]
>
> Snipped
>
> > > Since somehow the email are split it is difficult to see the indexing
> > >
> > > >
> > > > Caution: This message originated from an External Source. Use proper
> > > > caution when opening attachments, clicking links, or responding.
> > > >
> > > >
> > > > Hi Stephen,
> > > >
> > > > Many thanks for diving deeper into the issue and sharing the
> insights.
> > > >
> > > > I agree. given that the primary tears down all the devices on exit,
> > > > the secondary is left with dangling pointers and inconsistent state.
> > >
> > > Secondary if PMD should not be attempting to cleanup (especially for
> ethdev_ptr
> > shared from primary) I agree to that.
> > > But should not be covered in library or PMD. While cleanup for other
> > > needs to be done properly
> > >
> > >  Without a mechanism to notify or synchronize that teardown,
> > >
> > > You already have health check added in the code for identify if primay
> is still alive
> > or not..
> > > There used to be MP thread spawned which actually piggy back the
> > > communication. Is this broken? Can you please point to git
> >
> > There is no easy way to handle the case where primary crashes; leaving
> > secondary process with pointers to dead data.
>
> Thank you for sharing, but as shared the other threads (this topic has 3),
> the reason for the patch is because once the primary is dead it causes
> secondary to crash (segment fault).
>
> As shared in earlier email at least till 22.11 LTS and 23.03 (as it was my
> last testing with multi-process) I did not encounter these.
>
> @Khadem Ullah can you please share which version of DPDK you are noticing
> this failure?
>


-- 
Engr. Khadem Ullah,
Software Engineer,
Dreambig Semiconductor Inc
https://dreambigsemi.com/

[-- Attachment #2: Type: text/html, Size: 3545 bytes --]

RE: [PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Varghese, Vipin]

[-- Attachment #1: Type: text/plain, Size: 370 bytes --]

[Public]

Snipped

Hi Vipin,
These crashes are specific to the testpmd primary and testpmd secondary application.
Please check https://patches.dpdk.org/project/dpdk/patch/20250804113322.53650-1-14pwcse1224@uetpeshawar.edu.pk/
The details are provided over there.

Are you Killing the primary or closing it. Both have different effect.
Please clarify
snipped

[-- Attachment #2: Type: text/html, Size: 3296 bytes --]

[PATCH] app/testpmd: fix segfault in secondary process by monitoring primary

[Khadem Ullah]

In secondary processes, directly accessing 'dev->data->dev_private' can
cause a segmentation fault if the primary process has exited or if the
shared memory is no longer accessible. If a secondary process attempts
to query device information (e.g., via testpmd), a NULL dereference
may occur due to missing shared data.

The secondary process not only crashes on device close but also segfaults
when executing commands like "show device info all" after the primary has
exited.

This patch adds a mechanism in testpmd to monitor the primary process from the
secondary using `rte_eal_primary_proc_alive()`, and exits secondary process
when primary has exited.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 app/test-pmd/testpmd.c  | 49 +++++++++++++++++++++++++++++++++++++----
 lib/ethdev/rte_ethdev.c |  8 ++++++-
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bb88555328..ad780ba754 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
@@ -2281,7 +2283,7 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 			fc->total_cycles += tsc - prev_tsc;
 			prev_tsc = tsc;
 		}
-	} while (! fc->stopped);
+	} while (!fc->stopped && !f_quit);
 }
 
 static int
@@ -4332,6 +4334,43 @@ signal_handler(int signum __rte_unused)
 	prompt_exit();
 }
 
+
+/* 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 {
+		fprintf(stderr,
+			"Primary process is no longer active, exiting...\n");
+		f_quit = 1;
+		prompt_exit();
+	}
+}
+
+/* Setup handler to check when primary exits. */
+static void
+enable_primary_monitor(void)
+{
+	int ret;
+
+	/* Once primary exits, so will pdump. */
+	ret = rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Fail to enable monitor:%d\n", ret);
+}
+
+static void
+disable_primary_monitor(void)
+{
+	int ret;
+
+	ret = rte_eal_alarm_cancel(monitor_primary, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Fail to disable monitor:%d\n", ret);
+}
+
 int
 main(int argc, char** argv)
 {
@@ -4362,7 +4401,8 @@ main(int argc, char** argv)
 	if (diag < 0)
 		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
 			 rte_strerror(rte_errno));
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		enable_primary_monitor();
 	/* allocate port structures, and init them */
 	init_port();
 
@@ -4571,7 +4611,8 @@ main(int argc, char** argv)
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE, "Cannot unregister for ethdev events");
 
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		disable_primary_monitor();
 	ret = rte_eal_cleanup();
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE,
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..5c73397164 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 
 	if (dev->dev_ops->dev_infos_get == NULL)
 		return -ENOTSUP;
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
 	if (diag != 0) {
 		/* Cleanup already filled in device information */
@@ -4307,7 +4314,6 @@ rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 			port_id);
 		return -EINVAL;
 	}
-
 	rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
 
 	rte_eth_trace_macaddr_get(port_id, mac_addr);
-- 
2.43.0

[PATCH] [PATCH v4] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Khadem Ullah]

In secondary processes, directly accessing 'dev->data->dev_private' can
cause a segmentation fault if the primary process has exited or if the
shared memory is no longer accessible.

The secondary process not only crashes on device close but also segfaults
when executing commands like "show device info all" after the primary has
exited.

This patch adds a mechanism in testpmd to monitor the primary process from the
secondary using `rte_eal_primary_proc_alive()`, and introduces safety
checks before dereferencing 'dev_private'. The `rte_mem_virt2phy()` call
is also guarded to avoid access to invalid memory regions, with
`unlikely()` hints to minimize impact in the fast path.

Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 app/test-pmd/testpmd.c  | 56 ++++++++++++++++++++++++++++++++++++++---
 lib/ethdev/rte_ethdev.c | 15 ++++++++++-
 2 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bb88555328..25e55de281 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -101,13 +101,17 @@
 uint16_t verbose_level = 0; /**< Silent by default. */
 int testpmd_logtype; /**< Log type for testpmd logs */
 
+static volatile uint8_t quit_signal;
+
+/* 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
@@ -2281,7 +2285,7 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 			fc->total_cycles += tsc - prev_tsc;
 			prev_tsc = tsc;
 		}
-	} while (! fc->stopped);
+	} while (!fc->stopped && !quit_signal);
 }
 
 static int
@@ -4329,9 +4333,51 @@ static void
 signal_handler(int signum __rte_unused)
 {
 	f_quit = 1;
+	quit_signal = 1;
 	prompt_exit();
 }
 
+
+/* Alarm signal handler, used to check that primary process */
+static void
+monitor_primary(void *arg __rte_unused)
+{
+	if (rte_atomic_load_explicit(&quit_signal, rte_memory_order_relaxed))
+		return;
+
+	if (rte_eal_primary_proc_alive(NULL)) {
+		rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+	} else {
+		fprintf(stderr,
+			"Primary process is no longer active, exiting...\n");
+		quit_signal = 1;
+		f_quit = 1;
+		prompt_exit();
+	}
+}
+
+/* Setup handler to check when primary exits. */
+static void
+enable_primary_monitor(void)
+{
+	int ret;
+
+	/* Once primary exits, so will pdump. */
+	ret = rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Fail to enable monitor:%d\n", ret);
+}
+
+static void
+disable_primary_monitor(void)
+{
+	int ret;
+
+	ret = rte_eal_alarm_cancel(monitor_primary, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Fail to disable monitor:%d\n", ret);
+}
+
 int
 main(int argc, char** argv)
 {
@@ -4362,7 +4408,8 @@ main(int argc, char** argv)
 	if (diag < 0)
 		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
 			 rte_strerror(rte_errno));
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		enable_primary_monitor();
 	/* allocate port structures, and init them */
 	init_port();
 
@@ -4571,7 +4618,8 @@ main(int argc, char** argv)
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE, "Cannot unregister for ethdev events");
 
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		disable_primary_monitor();
 	ret = rte_eal_cleanup();
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE,
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..343e156a4f 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 
 	if (dev->dev_ops->dev_infos_get == NULL)
 		return -ENOTSUP;
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
 	if (diag != 0) {
 		/* Cleanup already filled in device information */
@@ -4307,7 +4314,13 @@ rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 			port_id);
 		return -EINVAL;
 	}
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		(dev->data->mac_addrs == NULL)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
 
 	rte_eth_trace_macaddr_get(port_id, mac_addr);
-- 
2.43.0

[PATCH] [PATCH v5] app/testpmd: fix segfault in secondary process by monitoring primary

[Khadem Ullah]

In secondary processes, directly accessing 'dev->data->dev_private' can
cause a segmentation fault if the primary process has exited or if the
shared memory is no longer accessible. If a secondary process attempts
to query device information (e.g., via testpmd), a NULL dereference
may occur due to missing shared data.

The secondary process not only crashes on device close but also segfaults
when executing commands like "show device info all" after the primary has
exited.

This patch adds a mechanism in testpmd to monitor the primary process from the
secondary using `rte_eal_primary_proc_alive()`, and exits secondary process
when primary has exited.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 app/test-pmd/testpmd.c  | 49 +++++++++++++++++++++++++++++++++++++----
 lib/ethdev/rte_ethdev.c |  8 ++++++-
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bb88555328..ad780ba754 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
@@ -2281,7 +2283,7 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 			fc->total_cycles += tsc - prev_tsc;
 			prev_tsc = tsc;
 		}
-	} while (! fc->stopped);
+	} while (!fc->stopped && !f_quit);
 }
 
 static int
@@ -4332,6 +4334,43 @@ signal_handler(int signum __rte_unused)
 	prompt_exit();
 }
 
+
+/* 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 {
+		fprintf(stderr,
+			"Primary process is no longer active, exiting...\n");
+		f_quit = 1;
+		prompt_exit();
+	}
+}
+
+/* Setup handler to check when primary exits. */
+static void
+enable_primary_monitor(void)
+{
+	int ret;
+
+	/* Once primary exits, so will pdump. */
+	ret = rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Fail to enable monitor:%d\n", ret);
+}
+
+static void
+disable_primary_monitor(void)
+{
+	int ret;
+
+	ret = rte_eal_alarm_cancel(monitor_primary, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Fail to disable monitor:%d\n", ret);
+}
+
 int
 main(int argc, char** argv)
 {
@@ -4362,7 +4401,8 @@ main(int argc, char** argv)
 	if (diag < 0)
 		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
 			 rte_strerror(rte_errno));
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		enable_primary_monitor();
 	/* allocate port structures, and init them */
 	init_port();
 
@@ -4571,7 +4611,8 @@ main(int argc, char** argv)
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE, "Cannot unregister for ethdev events");
 
-
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		disable_primary_monitor();
 	ret = rte_eal_cleanup();
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE,
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..5c73397164 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4079,6 +4079,13 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 
 	if (dev->dev_ops->dev_infos_get == NULL)
 		return -ENOTSUP;
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+		unlikely(rte_mem_virt2phy(dev->data->dev_private) == RTE_BAD_PHYS_ADDR)) {
+			RTE_ETHDEV_LOG_LINE(ERR,
+			"Secondary: dev_private not accessible (primary exited?)");
+			rte_errno = ENODEV;
+			return -rte_errno;
+	}
 	diag = dev->dev_ops->dev_infos_get(dev, dev_info);
 	if (diag != 0) {
 		/* Cleanup already filled in device information */
@@ -4307,7 +4314,6 @@ rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 			port_id);
 		return -EINVAL;
 	}
-
 	rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
 
 	rte_eth_trace_macaddr_get(port_id, mac_addr);
-- 
2.43.0

Re: [PATCH] [PATCH v5] app/testpmd: fix segfault in secondary process by monitoring primary

[Stephen Hemminger]

On Tue, 29 Jul 2025 02:39:19 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> In secondary processes, directly accessing 'dev->data->dev_private' can
> cause a segmentation fault if the primary process has exited or if the
> shared memory is no longer accessible. If a secondary process attempts
> to query device information (e.g., via testpmd), a NULL dereference
> may occur due to missing shared data.
> 
> The secondary process not only crashes on device close but also segfaults
> when executing commands like "show device info all" after the primary has
> exited.
> 
> This patch adds a mechanism in testpmd to monitor the primary process from the
> secondary using `rte_eal_primary_proc_alive()`, and exits secondary process
> when primary has exited.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> ---

When is testpmd doing a query after it detects primary exit?
That is the problem still.

Re: [PATCH] [PATCH v5] app/testpmd: fix segfault in secondary process by monitoring primary

[Stephen Hemminger]

What about this simpler and more direct approach.


From d3a85e214e8ba9a89e2b154480a8fc76a67c0f68 Mon Sep 17 00:00:00 2001
From: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
Date: Tue, 29 Jul 2025 08:59:15 -0700
Subject: [PATCH] app/testpmd: monitor state of primary process when using
 secondary

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 using `rte_eal_primary_proc_alive()`.
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 | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bb88555328..7c22046efb 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,36 @@ signal_handler(int signum __rte_unused)
 	prompt_exit();
 }
 
+/* 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);
+	}
+}
+
+/* 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);
+}
+
 int
 main(int argc, char** argv)
 {
@@ -4363,6 +4395,10 @@ main(int argc, char** argv)
 		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
 			 rte_strerror(rte_errno));
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+	    enable_primary_monitor() < 0)
+		rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
+
 	/* allocate port structures, and init them */
 	init_port();
 
@@ -4556,6 +4592,9 @@ main(int argc, char** argv)
 		}
 	}
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		disable_primary_monitor();
+
 	pmd_test_exit();
 
 #ifdef RTE_LIB_PDUMP
-- 
2.47.2

Re: [PATCH] [PATCH v5] app/testpmd: fix segfault in secondary process by monitoring primary

[Khadem Ullah]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 307 bytes --]

Hi Stephen,

Many thanks for the fix and for the suggestion that the secondary should exit 
immediately without attempting any cleanups once it detects the primary has exited.

I completely agree. I’ll supersede this patch with a new version that incorporates 
both of our changes.

Best regards,
Khadem

RE: [PATCH] [PATCH v5] app/testpmd: fix segfault in secondary process by monitoring primary

[Varghese, Vipin]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Khadem,

Snipped

>
>
> Hi Stephen,
>
> Many thanks for the fix and for the suggestion that the secondary should exit
> immediately without attempting any cleanups once it detects the primary has exited.

Can you please explain why cleanup of secondary resources are not to be done, If there is specific memory like external hugepage backed mempool none of the callback will be called. Also releasing internal stack memory and heap of secondary is not cleaned.

Is there specific reason why you do not want to clean? If yes, can you please point to the right direction what is breaking?

>
> I completely agree. IâEUR(tm)ll supersede this patch with a new version that
> incorporates both of our changes.
>
> Best regards,
> Khadem

Re: [PATCH] [PATCH v5] app/testpmd: fix segfault in secondary process by monitoring primary

[Stephen Hemminger]

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

> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Khadem,
> 
> Snipped
> 
> >
> >
> > Hi Stephen,
> >
> > Many thanks for the fix and for the suggestion that the secondary should exit
> > immediately without attempting any cleanups once it detects the primary has exited.  
> 
> Can you please explain why cleanup of secondary resources are not to be done, If there is specific memory like external hugepage backed mempool none of the callback will be called. Also releasing internal stack memory and heap of secondary is not cleaned.
> 
> Is there specific reason why you do not want to clean? If yes, can you please point to the right direction what is breaking?

What happens is that both primary and secondary process have a list head
that lists devices (like ethdev).  The ethdev's shared part lives in huge pages.
When primary exits, it calls the cleanup which frees the list (local part) and
the shared part.  When secondary calls cleanup, its local part now points to
freed data in shared huge pages. That leads to all sorts of issues.


There is no need for secondary to do any cleanup, all the local memory disappears when
process exits.

Also, there is no locking between primary and secondary process.

RE: [PATCH] [PATCH v5] app/testpmd: fix segfault in secondary process by monitoring primary

[Varghese, Vipin]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Stephen,

Snipped

> > Hi Khadem,
> >
> > Snipped
> >
> > >
> > >
> > > Hi Stephen,
> > >
> > > Many thanks for the fix and for the suggestion that the secondary
> > > should exit immediately without attempting any cleanups once it detects the
> primary has exited.
> >
> > Can you please explain why cleanup of secondary resources are not to be done,
> If there is specific memory like external hugepage backed mempool none of the
> callback will be called. Also releasing internal stack memory and heap of secondary
> is not cleaned.

@Khadem Ullah can you please share if secondary application is not able allocate external memory and run.
Can you please confirm running the same?

> >
> > Is there specific reason why you do not want to clean? If yes, can you please
> point to the right direction what is breaking?
>
> What happens is that both primary and secondary process have a list head that
> lists devices (like ethdev).  The ethdev's shared part lives in huge pages.
> When primary exits, it calls the cleanup which frees the list (local part) and the
> shared part.  When secondary calls cleanup, its local part now points to freed data
> in shared huge pages. That leads to all sorts of issues.
>
>
> There is no need for secondary to do any cleanup, all the local memory disappears
> when process exits.

As shared in other email threads, the reason for the patch as far I followed `at segfault of primary secondary crashes, @Khadem Ullah can you please share the details.`

>
> Also, there is no locking between primary and secondary process.

Re: [PATCH] [PATCH v5] app/testpmd: fix segfault in secondary process by monitoring primary

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 176 bytes --]

Hi Vipin,
Please check
https://patches.dpdk.org/project/dpdk/patch/20250804113322.53650-1-14pwcse1224@uetpeshawar.edu.pk/
The details are provided over there.

Regards,
Khadem

[-- Attachment #2: Type: text/html, Size: 404 bytes --]

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

[Khadem Ullah]

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 | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bb88555328..7c22046efb 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,36 @@ signal_handler(int signum __rte_unused)
 	prompt_exit();
 }
 
+/* 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);
+	}
+}
+
+/* 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);
+}
+
 int
 main(int argc, char** argv)
 {
@@ -4363,6 +4395,10 @@ main(int argc, char** argv)
 		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
 			 rte_strerror(rte_errno));
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+	    enable_primary_monitor() < 0)
+		rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
+
 	/* allocate port structures, and init them */
 	init_port();
 
@@ -4556,6 +4592,9 @@ main(int argc, char** argv)
 		}
 	}
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		disable_primary_monitor();
+
 	pmd_test_exit();
 
 #ifdef RTE_LIB_PDUMP
-- 
2.43.0

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

[Khadem Ullah]

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 | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bb88555328..7c22046efb 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,36 @@ signal_handler(int signum __rte_unused)
 	prompt_exit();
 }
 
+/* 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);
+	}
+}
+
+/* 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);
+}
+
 int
 main(int argc, char** argv)
 {
@@ -4363,6 +4395,10 @@ main(int argc, char** argv)
 		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
 			 rte_strerror(rte_errno));
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+	    enable_primary_monitor() < 0)
+		rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
+
 	/* allocate port structures, and init them */
 	init_port();
 
@@ -4556,6 +4592,9 @@ main(int argc, char** argv)
 		}
 	}
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		disable_primary_monitor();
+
 	pmd_test_exit();
 
 #ifdef RTE_LIB_PDUMP
-- 
2.43.0

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

[Stephen Hemminger]

On Wed, 30 Jul 2025 02:08:18 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

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

The problem with this patch, is the build now fails on Windows
where there is no primary/secondary process support.

Either add stubs for Windows or #ifdef the code in test-pmd
as necessary.

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

[Khadem Ullah]

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 | 45 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bb88555328..f9dab2106a 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,36 @@ signal_handler(int signum __rte_unused)
 	prompt_exit();
 }
 
+/* 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);
+	}
+}
+
+/* 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);
+}
+
 int
 main(int argc, char** argv)
 {
@@ -4363,6 +4395,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 +4594,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

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

[Khadem Ullah]

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);
+	}
+}
+
+/* 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

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

[Stephen Hemminger]

On Mon,  4 Aug 2025 07:33:22 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

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

Queued to next-net for 25.11.

Wonder if Windows plans to implement primary-secondary.

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

[fengchengwen]

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 8/4/2025 7:33 PM, Khadem Ullah wrote:
> 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>

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

[Varghese, Vipin]

[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

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

[Khadem Ullah]

Hi Vipin Varghese, 
Thank you for your feedback! 

Yes we need graceful exit, directly calling eal_cleanup from secondary was not 
working, we can do the following workaround to use `eal_cleanup`.

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b7affa6da9..5ef6008a3c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -4347,7 +4347,12 @@ monitor_primary(void *arg __rte_unused)
                 * is no longer valid. Calling any cleanup code is going to
                 * run into use after free.
                 */
+               int ret;
                fprintf(stderr, "\nPrimary process is no longer active, exiting...\n");
+               ret = rte_eal_cleanup();
+               if (ret != 0)
+               rte_exit(EXIT_FAILURE,
+                        "EAL cleanup failed: %s\n", strerror(-ret));
                exit(EXIT_FAILURE);
        }
 }
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 52efb8626b..087a4b1137 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1333,9 +1333,12 @@ rte_eal_cleanup(void)
 #ifdef VFIO_PRESENT
        vfio_mp_sync_cleanup();
 #endif
-       rte_mp_channel_cleanup();
+       if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+               rte_mp_channel_cleanup();
+               rte_eal_alarm_cleanup();
+       }
+
        eal_bus_cleanup();
-       rte_eal_alarm_cleanup();
        rte_trace_save();
        eal_trace_fini();
        eal_mp_dev_hotplug_cleanup();

If Stephen and others are agree, I can supersede this patch with 
the newer ones containing cleanup changes.

Thank you, 
Best Regards,
Khadem 

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

[Varghese, Vipin]

[Public]

Hi Khadem,

Snipped

>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Vipin Varghese,
> Thank you for your feedback!
>
> Yes we need graceful exit, directly calling eal_cleanup from secondary was not
> working, we can do the following workaround to use `eal_cleanup`.
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> b7affa6da9..5ef6008a3c 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -4347,7 +4347,12 @@ monitor_primary(void *arg __rte_unused)
>                  * is no longer valid. Calling any cleanup code is going to
>                  * run into use after free.
>                  */
> +               int ret;
>                 fprintf(stderr, "\nPrimary process is no longer active, exiting...\n");
> +               ret = rte_eal_cleanup();
> +               if (ret != 0)
> +               rte_exit(EXIT_FAILURE,
> +                        "EAL cleanup failed: %s\n", strerror(-ret));
>                 exit(EXIT_FAILURE);
>         }
>  }
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c index 52efb8626b..087a4b1137
> 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -1333,9 +1333,12 @@ rte_eal_cleanup(void)  #ifdef VFIO_PRESENT
>         vfio_mp_sync_cleanup();
>  #endif
> -       rte_mp_channel_cleanup();
> +       if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +               rte_mp_channel_cleanup();
> +               rte_eal_alarm_cleanup();
> +       }
> +
>         eal_bus_cleanup();
> -       rte_eal_alarm_cleanup();
>         rte_trace_save();
>         eal_trace_fini();
>         eal_mp_dev_hotplug_cleanup();
>
> If Stephen and others are agree, I can supersede this patch with the newer ones
> containing cleanup changes.

In the email follow up earlier, there is mention from your end as soon as primary is down secondary goes segment fault.
As far I recollect testing for multiprocess using dpdk 22.11 and 23.03 the behavior were different.

1. Primary can exit or segfault.
2. Secondary can still continue to use mbuf and pmd calls without fail.

But if you are seeing there difference in behviour, can you please let us know which version onwards it changed.

With respect to synchronization, when dpdk library-based application is started there is also a MP thread that were created.
Are you sharing that this is no longer functional?

Note: I am trying to understand the behavior observed at your end.


>
> Thank you,
> Best Regards,
> Khadem

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

[Khadem Ullah]

The crashes are on 22.11, 23.03, 24.11, it is on all dpdk stable versions and 25.07 as well. 
Please first close primary testpmd before secondary testpmd application 
and try to close secondary or execute any of the following commands, 

"show device info all
show port stats all 
show port xstats all 
set fwd rxonly 
set fwd txonly 
start
etc"

We are all agree that these crashes exists. First we were tried 
to prevent the crashes at PMD level, but it was not possible to 
add checks in each PMD. Then we tried to add safety checks 
in ethdev layer, and it was not suitable as with primary closing
all reference to device information (pointers) would lead crashes.

Then we agreed on secondary process monitoring for primary process exiting. 
and it is now resolved on application level, i.e. on testpmd. 

Now, this solution is working perfectly. We can add eal_cleanup for 
gracefull exit. 

Best Regards, 
Khadem 

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

[Stephen Hemminger]

On Fri,  8 Aug 2025 07:49:09 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> The crashes are on 22.11, 23.03, 24.11, it is on all dpdk stable versions and 25.07 as well. 
> Please first close primary testpmd before secondary testpmd application 
> and try to close secondary or execute any of the following commands, 
> 
> "show device info all
> show port stats all 
> show port xstats all 
> set fwd rxonly 
> set fwd txonly 
> start
> etc"
> 
> We are all agree that these crashes exists. First we were tried 
> to prevent the crashes at PMD level, but it was not possible to 
> add checks in each PMD. Then we tried to add safety checks 
> in ethdev layer, and it was not suitable as with primary closing
> all reference to device information (pointers) would lead crashes.
> 
> Then we agreed on secondary process monitoring for primary process exiting. 
> and it is now resolved on application level, i.e. on testpmd. 
> 
> Now, this solution is working perfectly. We can add eal_cleanup for 
> gracefull exit. 
> 
> Best Regards, 
> Khadem 

Maybe this quick picture would help explain the data structures

                               │                                                 
                               │            Huge pages (shared)                                     
                               │                                                 
            rte_eth_devices[]  │                                                 
                               │                                                 
             ┌────────┐        │                                                 
Primary      │        ┼────────┼───┐                                             
Process      │        │        │   │                                             
             ┌────────┐        │   │                                             
             │        ┼────┐   │   │                                             
             │        │    │   │   │              rte_eth_dev_data               
             └────────┘    │   │   │            ┌─────────────────┐              
                           │   │   │            │                 │              
                           │   │   └───────────►│                 │              
                           │   │                │              ───┼─────────────►
                           │   │        ┌───────►                 │              
                           │   │        │       │                 │              
                           │   │        │       └─────────────────┘              
                           │   │        │                                        
                           │   │        │       ┌─────────────────┐              
                           │   │        │       │                 │              
                           └───┼────────┼───────►                 │              
                               │        │       │            ─────┼────────────► 
            rte_eth_devices    │        │  ┌────►                 │              
  Secondary ┌────────┐         │        │  │    │                 │              
  Process   │        ┼─────────┼────────┘  │    └─────────────────┘              
            │        │         │           │                                     
            ┌────────┐         │           │                                     
            │        ┼─────────┼───────────┘                                     
            │        │         │                                                 
            └────────┘         │                                                 
                               │                                                 
                                                                         

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

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 164 bytes --]

Now, it is much clearer from the diagram that we do not need any cleanup
 in the secondary process if the primary has already been done!
Thanks again for the help!

[-- Attachment #2: Type: text/html, Size: 235 bytes --]

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

[Varghese, Vipin]

[Public]

Hi Stepehen,

Thank you for sharing

Snipped

>
>
> On Fri,  8 Aug 2025 07:49:09 -0400
> Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:
>
> > The crashes are on 22.11, 23.03, 24.11, it is on all dpdk stable versions and 25.07
> as well.
> > Please first close primary testpmd before secondary testpmd
> > application and try to close secondary or execute any of the following
> > commands,
> >
> > "show device info all
> > show port stats all
> > show port xstats all
> > set fwd rxonly
> > set fwd txonly
> > start
> > etc"
> >
> > We are all agree that these crashes exists. First we were tried to
> > prevent the crashes at PMD level, but it was not possible to add
> > checks in each PMD. Then we tried to add safety checks in ethdev
> > layer, and it was not suitable as with primary closing all reference
> > to device information (pointers) would lead crashes.
> >
> > Then we agreed on secondary process monitoring for primary process exiting.
> > and it is now resolved on application level, i.e. on testpmd.
> >
> > Now, this solution is working perfectly. We can add eal_cleanup for
> > gracefull exit.
> >
> > Best Regards,
> > Khadem
>
> Maybe this quick picture would help explain the data structures
>
>                                │
>                                │            Huge pages (shared)
>                                │
>             rte_eth_devices[]  │
>                                │
>              ┌────────┐        │
> Primary      │        ┼────────┼───┐
> Process      │        │        │   │
>              ┌────────┐        │   │
>              │        ┼────┐   │   │
>              │        │    │   │   │              rte_eth_dev_data
>              └────────┘    │   │   │            ┌─────────────────┐
>                            │   │   │            │                 │
>                            │   │   └───────────►│                 │
>                            │   │                │              ───┼─────────────►
>                            │   │        ┌───────►                 │
>                            │   │        │       │                 │
>                            │   │        │       └─────────────────┘
>                            │   │        │
>                            │   │        │       ┌─────────────────┐
>                            │   │        │       │                 │
>                            └───┼────────┼───────►                 │
>                                │        │       │            ─────┼────────────►
>             rte_eth_devices    │        │  ┌────►                 │
>   Secondary ┌────────┐         │        │  │    │                 │
>   Process   │        ┼─────────┼────────┘  │
> └─────────────────┘
>             │        │         │           │
>             ┌────────┐         │           │
>             │        ┼─────────┼───────────┘
>             │        │         │
>             └────────┘         │
>                                │

Definitely something in way `rte_eth_Dev_data` is changed in some release.
Earlier when secondary comes up, the memory shared for rte_eth_dev_data were probed by secondary to get physical device into local memory. Then all virtual devices under secondary were added.

Hence adding physical or virtual device in primary did not reflect back to secondary after rte_eal_inti completed by secondary.
I am still trying to figure out why @Khadem Ullah mentioned

```
Please first close primary testpmd before secondary testpmd application and try to close secondary or execute any of the following commands,

"show device info all
show port stats all
show port xstats all
set fwd rxonly
set fwd txonly
start
etc"
```

`I think the reason is because, in all the testing we use SIGKILL to kill the primary, and not close or shutdown which will trigger the cleanup.
The thumb rule or understanding, if you are shutting down primary always shut down secondaries first.`

@Khadem Ullah I am open to be available to slack for you where you can recreate the issue (as you have working setup) with the version deployed.
Please do let me know.

Regards
Vipin Varghese

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

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 893 bytes --]

Hi,
You can create a simple setup with only running testpmd with primary and
another testpmd with secondary and then see the results.

No, the reason for the primary process termination could be close or
shutdown (ctrl + c or ctrl + q) or any other.

The reason is clear: "In secondary processes, accessing
'dev->data->dev_private' or &dev->data->mac_addrs directly can
cause a segmentation fault if the primary process has exited or the
shared memory is unavailable."

The effect is throughput the life-cycle of secondary process existence in
dpdk after primary,
You can check
https://patches.dpdk.org/project/dpdk/patch/20250721073851.963141-1-14pwcse1224@uetpeshawar.edu.pk/
https://patches.dpdk.org/project/dpdk/patch/20250721105522.1019515-1-14pwcse1224@uetpeshawar.edu.pk/

I think most of the details have already been provided.
The community agrees with the current solution.

Thanks.

[-- Attachment #2: Type: text/html, Size: 1385 bytes --]

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

[Varghese, Vipin]

[-- Attachment #1: Type: text/plain, Size: 3407 bytes --]

[Public]

HI,

Snipped

Hi,
You can create a simple setup with only running testpmd with primary and another testpmd with secondary and then see the results.

No, the reason for the primary process termination could be close or shutdown (ctrl + c or ctrl + q) or any other.
The reason is clear: "In secondary processes, accessing 'dev->data->dev_private' or &dev->data->mac_addrs directly can
cause a segmentation fault if the primary process has exited or the
shared memory is unavailable."
The effect is throughput the life-cycle of secondary process existence in dpdk after primary,
You can check
https://patches.dpdk.org/project/dpdk/patch/20250721073851.963141-1-14pwcse1224@uetpeshawar.edu.pk/
https://patches.dpdk.org/project/dpdk/patch/20250721105522.1019515-1-14pwcse1224@uetpeshawar.edu.pk/

I think most of the details have already been provided.
The community agrees with the current solution.
Thanks.


There are couple of corrections that needs to shared to the current understanding you are sharing


  1.  Please find DPDK Documentation on mutli process as 4. Multi-process Support — Data Plane Development Kit 25.07.0 documentation<https://doc.dpdk.org/guides/prog_guide/multi_proc_support.html>

2.    From your ` This patch only adds stability and robustness to primary-secondary applications.` this is not true

3.    ` any other application which provides primary-secondary support could also follow the same model` - actually by default the eal args supports mutli process. Only by specifying `--single-process` this is disabled

4.    ` the reason for the primary process termination could be close or shutdown (ctrl + c or ctrl + q)` DPDK testpmd and a few examples like L2fwd, eventdev, l3fwd and other uses SIGACTION to check gracefully exit

  1.  In all graceful exit `rte_eal_cleanup is invoked`. Hence any secondary (as shared in my previous emails) will have dangling pointers.
  2.  For customer till 2023 and internal testing (as mentioned in my email) any abrupt kill of primary does not corrupt the secondary rte_eth_dev_list.

From patch perspective
https://patches.dpdk.org/project/dpdk/patch/20250721073851.963141-1-14pwcse1224@uetpeshawar.edu.pk/
what Stephen shared is correct, PMD are not incharge of process aliveness. It is correct hence NAK

net/ice: fix segfault in secondary process by validating dev_private pointer - Patchwork<https://patches.dpdk.org/project/dpdk/patch/20250721105522.1019515-1-14pwcse1224@uetpeshawar.edu.pk/>
trying to do in ICE driver should be NAK

From solution perspective, in my earlier email I clearly asked why have not explored multi-process communication thread. Link to it is same as above under section ` 4.4. Communication between multiple processes`

You could always design when ` (ctrl + c or ctrl + q)` primary can send a broadcast to all secondaries to stop eth, event, bbdev or any such execution or simply exit.
This is what we designed and shared as working logic.



Note: You have also mentioned ` The community agrees with the current solution. ` which paints a picture of

  1.  Only there is one way to solve this problem.
  2.  Since I did not agree to your thought process I am not community.
Personally, I am not happy with such biases, DPDK had been supporting and nurturing open healthy conversation and platform. I am not aware this was changed.



[-- Attachment #2: Type: text/html, Size: 15193 bytes --]

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

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

Hi Vipin,
sorry for any confusion. I am only trying to explain that these patches are
in its final state (waiting to upstream). You have provided your valuable
insights and
I am very thankful for that. There was much discussion about these patches
from start to finish.

Already mentioned that with Stephen and other community members, the
discussion was started earlier and I have provided all the information
to you in a singla email.
 "First we tried
to prevent the crashes at PMD level, but it was not possible to
add checks in each PMD. Then we tried to add safety checks
in the ethdev layer, and it was not suitable as with primary closing
all reference to device information (pointers) would lead to crashes."

@Stephen Hemminger <stephen@networkplumber.org>  agreed to add a
primary-secondary monitoring mechanism.

I am always open to healthy conversation!

Thanks & Best Regards,
Khadem

[-- Attachment #2: Type: text/html, Size: 1229 bytes --]

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

[Varghese, Vipin]

[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]

[Public]

Hi Khadem,

Please do not use `sorry for any confusion`. Very clearly, I shared what works and why your case is different.
Snipped

Hi Vipin,
sorry for any confusion. I am only trying to explain that these patches are in its final state (waiting to upstream).

[VV] it is not about the state, but of the quality and state.

You have provided your valuable insights and
I am very thankful for that. There was much discussion about these patches from start to finish.

Already mentioned that with Stephen and other community members, the discussion was started earlier and I have provided all the information
to you in a singla email.
 "First we tried
to prevent the crashes at PMD level, but it was not possible to
add checks in each PMD. Then we tried to add safety checks
in the ethdev layer, and it was not suitable as with primary closing
all reference to device information (pointers) would lead to crashes."
[VV] I did also share I checked these eemails, none of them discussed use for multi process communication as a trigger or why it is not useful.
@Stephen Hemminger<mailto:stephen@networkplumber.org>  agreed to add a primary-secondary monitoring mechanism.

I am always open to healthy conversation!

I will rest it here, I clearly understand your previous content ` The community agrees with the current solution. `

[-- Attachment #2: Type: text/html, Size: 5294 bytes --]

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

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 213 bytes --]

Hi Vipin,
Thank you for understanding!
As Stephen mentioned:
"There is no easy way to handle the case where primary crashes; leaving
secondary process with invalid data (dangling pointers)."

Best regards,
Khadem

[-- Attachment #2: Type: text/html, Size: 558 bytes --]

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

[Varghese, Vipin]

[AMD Official Use Only - AMD Internal Distribution Only]

Thank you for sharing the details in this thread,


Snipped
>
>
> The crashes are on 22.11, 23.03, 24.11, it is on all dpdk stable versions and 25.07
> as well.
> Please first close primary testpmd before secondary testpmd application and try to
> close secondary or execute any of the following commands,

Here is my problem, these were working at start of 2023. So something has definitely changed.

>
> "show device info all
> show port stats all
> show port xstats all
> set fwd rxonly
> set fwd txonly
> start
> etc"

Have you started the rx-tx of the device in primary or secondary applications?

>
> We are all agree that these crashes exists. First we were tried to prevent the
> crashes at PMD level, but it was not possible to add checks in each PMD. Then we
> tried to add safety checks in ethdev layer, and it was not suitable as with primary
> closing all reference to device information (pointers) would lead crashes.
>
> Then we agreed on secondary process monitoring for primary process exiting.
> and it is now resolved on application level, i.e. on testpmd.

This is where I am not clear. You have attempted the fix in `testpmd`.
Please help in understanding the cause and impact

1. is it only occurring in testpmd?
2. does other applications (Example) also faces this issue?

I still remember, where solutions were designed for network customers where primary allocates and manages memory.
While secondary does rx and tx burst. We ended up in creating service thread in secondary to check aliveness of primary.

>
> Now, this solution is working perfectly. We can add eal_cleanup for gracefull exit.

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

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

Hi Vipin,
Glad you got the reason.
I have only checked it in testpmd, any other application which provides
primary-secondary support could also follow the same model.
This patch only adds stability and robustness to primary-secondary
applications. Primary applications can be closed, quit or shutdown
by various reasons while secondary end-up with dangling pointers.
Without the primary application, the secondary could not do any rx or tx.

[-- Attachment #2: Type: text/html, Size: 584 bytes --]

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

[Varghese, Vipin]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

[Public]

Hi Khadem

snipped

Hi Vipin,
Glad you got the reason.
I have only checked it in testpmd, any other application which provides primary-secondary support could also follow the same model.
This patch only adds stability and robustness to primary-secondary applications. Primary applications can be closed, quit or shutdown
by various reasons while secondary end-up with dangling pointers.
Without the primary application, the secondary could not do any rx or tx.


Can you please answer to the previous question `are you SIGKILL or doing graceful shutdown to primary`? IBoth are different behaviour

[-- Attachment #2: Type: text/html, Size: 3503 bytes --]

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

[Stephen Hemminger]

On Fri,  8 Aug 2025 01:44:52 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

>                  * run into use after free.
>                  */
> +               int ret;
>                 fprintf(stderr, "\nPrimary process is no longer active, exiting...\n");
> +               ret = rte_eal_cleanup();
> +               if (ret != 0)
> +               rte_exit(EXIT_FAILURE,
> +                        "EAL cleanup failed: %s\n", strerror(-ret));
>                 exit(EXIT_FAILURE);

No, that eal_cleanup references freed data when run in secondary.
That is the problem this is trying to solve.

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

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 126 bytes --]

Hi Stephen,
Thanks for the feedback.
So, it means the patch is perfect in the current state (i.e., v7)?

Best Regards,
Khadem

[-- Attachment #2: Type: text/html, Size: 251 bytes --]

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

[Stephen Hemminger]

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.

[PATCH v3] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Khadem Ullah]

When the secondary process testpmd application running any fwd_engine
and after that primary has exited will cause a crash.

This patch forces secondary process forwarding lcores
in fwd_engine to stop before the primary process exits.

Fixes: a550baf24af90f ("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      | 19 ++++++++++++++++---
 lib/eal/common/hotplug_mp.c |  3 +++
 lib/eal/common/hotplug_mp.h |  2 ++
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0f58a31eb5..eda3a0264d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 #ifndef RTE_EXEC_ENV_WINDOWS
 #include <sys/mman.h>
+#include "hotplug_mp.h"
 #endif
 #include <sys/types.h>
 #include <errno.h>
@@ -103,7 +104,7 @@ 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)
+#define MONITOR_INTERVAL (500)
 
 /* use main core for command line ? */
 uint8_t interactive = 0;
@@ -2563,7 +2564,8 @@ stop_packet_forwarding(void)
 	for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++)
 		fwd_lcores[lc_id]->stopped = 1;
 	printf("\nWaiting for lcores to finish...\n");
-	rte_eal_mp_wait_lcore();
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_eal_mp_wait_lcore();
 	port_fwd_end = cur_fwd_config.fwd_eng->port_fwd_end;
 	if (port_fwd_end != NULL) {
 		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
@@ -3634,6 +3636,11 @@ pmd_test_exit(void)
 			printf("\nStopping port %d...\n", pt_id);
 			fflush(stdout);
 			stop_port(pt_id);
+			printf("Stopping secondary process...\n");
+			struct eal_dev_mp_req req;
+			memset(&req, 0, sizeof(req));
+			req.t = EAL_DEV_REQ_TYPE_STOP;
+			eal_dev_hotplug_request_to_secondary(&req);
 		}
 		RTE_ETH_FOREACH_DEV(pt_id) {
 			printf("\nShutting down port %d...\n", pt_id);
@@ -4371,7 +4378,13 @@ static void
 monitor_primary(void *arg __rte_unused)
 {
 	if (rte_eal_primary_proc_alive(NULL)) {
-		rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+		if (f_exit) {
+			stop_packet_forwarding();
+			fprintf(stderr, "\nPrimary process is no longer active, exiting...\n");
+			exit(EXIT_FAILURE);
+		} else {
+			rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+		}
 	} else {
 		/*
 		 * If primary process exits, then all the device information
diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index 17089ca3db..538e6eaf4e 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -239,6 +239,9 @@ static void __handle_primary_request(void *param)
 	memset(&mp_resp, 0, sizeof(mp_resp));
 
 	switch (req->t) {
+	case EAL_DEV_REQ_TYPE_STOP:
+		f_exit = 1;
+		break;
 	case EAL_DEV_REQ_TYPE_ATTACH:
 	case EAL_DEV_REQ_TYPE_DETACH_ROLLBACK:
 		ret = local_dev_probe(req->devargs, &dev);
diff --git a/lib/eal/common/hotplug_mp.h b/lib/eal/common/hotplug_mp.h
index 7221284286..b0755bcf51 100644
--- a/lib/eal/common/hotplug_mp.h
+++ b/lib/eal/common/hotplug_mp.h
@@ -9,6 +9,7 @@
 
 #define EAL_DEV_MP_ACTION_REQUEST      "eal_dev_mp_request"
 #define EAL_DEV_MP_ACTION_RESPONSE     "eal_dev_mp_response"
+volatile uint8_t f_exit;
 
 #define EAL_DEV_MP_DEV_NAME_MAX_LEN RTE_DEV_NAME_MAX_LEN
 #define EAL_DEV_MP_BUS_NAME_MAX_LEN 32
@@ -19,6 +20,7 @@ enum eal_dev_req_type {
 	EAL_DEV_REQ_TYPE_DETACH,
 	EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK,
 	EAL_DEV_REQ_TYPE_DETACH_ROLLBACK,
+	EAL_DEV_REQ_TYPE_STOP,
 };
 
 struct eal_dev_mp_req {
-- 
2.27.0

[PATCH v3] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Khadem Ullah]

When the secondary process testpmd application running any fwd_engine
and after that primary has exited will cause a crash.

This patch forces secondary process forwarding lcores
in fwd_engine to stop before the primary process exits.

Fixes: a550baf24af90f ("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      | 25 ++++++++++++++++++++++---
 lib/eal/common/hotplug_mp.c |  3 +++
 lib/eal/common/hotplug_mp.h |  2 ++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0f58a31eb5..f385bf920d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 #ifndef RTE_EXEC_ENV_WINDOWS
 #include <sys/mman.h>
+#include "hotplug_mp.h"
 #endif
 #include <sys/types.h>
 #include <errno.h>
@@ -103,7 +104,7 @@ 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)
+#define MONITOR_INTERVAL (500)
 
 /* use main core for command line ? */
 uint8_t interactive = 0;
@@ -2563,7 +2564,12 @@ stop_packet_forwarding(void)
 	for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++)
 		fwd_lcores[lc_id]->stopped = 1;
 	printf("\nWaiting for lcores to finish...\n");
-	rte_eal_mp_wait_lcore();
+#ifndef RTE_EXEC_ENV_WINDOWS
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_eal_mp_wait_lcore();
+#else
+		rte_eal_mp_wait_lcore();
+#endif
 	port_fwd_end = cur_fwd_config.fwd_eng->port_fwd_end;
 	if (port_fwd_end != NULL) {
 		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
@@ -3634,6 +3640,13 @@ pmd_test_exit(void)
 			printf("\nStopping port %d...\n", pt_id);
 			fflush(stdout);
 			stop_port(pt_id);
+#ifndef RTE_EXEC_ENV_WINDOWS
+			printf("Stopping secondary process...\n");
+			struct eal_dev_mp_req req;
+			memset(&req, 0, sizeof(req));
+			req.t = EAL_DEV_REQ_TYPE_STOP;
+			eal_dev_hotplug_request_to_secondary(&req);
+#endif
 		}
 		RTE_ETH_FOREACH_DEV(pt_id) {
 			printf("\nShutting down port %d...\n", pt_id);
@@ -4371,7 +4384,13 @@ static void
 monitor_primary(void *arg __rte_unused)
 {
 	if (rte_eal_primary_proc_alive(NULL)) {
-		rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+		if (f_exit_sec) {
+			stop_packet_forwarding();
+			fprintf(stderr, "\nPrimary process is no longer active, exiting...\n");
+			exit(EXIT_FAILURE);
+		} else {
+			rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+		}
 	} else {
 		/*
 		 * If primary process exits, then all the device information
diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index 17089ca3db..2574a95168 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -239,6 +239,9 @@ static void __handle_primary_request(void *param)
 	memset(&mp_resp, 0, sizeof(mp_resp));
 
 	switch (req->t) {
+	case EAL_DEV_REQ_TYPE_STOP:
+		f_exit_sec = 1;
+		break;
 	case EAL_DEV_REQ_TYPE_ATTACH:
 	case EAL_DEV_REQ_TYPE_DETACH_ROLLBACK:
 		ret = local_dev_probe(req->devargs, &dev);
diff --git a/lib/eal/common/hotplug_mp.h b/lib/eal/common/hotplug_mp.h
index 7221284286..4f7b6d8f58 100644
--- a/lib/eal/common/hotplug_mp.h
+++ b/lib/eal/common/hotplug_mp.h
@@ -9,6 +9,7 @@
 
 #define EAL_DEV_MP_ACTION_REQUEST      "eal_dev_mp_request"
 #define EAL_DEV_MP_ACTION_RESPONSE     "eal_dev_mp_response"
+volatile uint8_t f_exit_sec;
 
 #define EAL_DEV_MP_DEV_NAME_MAX_LEN RTE_DEV_NAME_MAX_LEN
 #define EAL_DEV_MP_BUS_NAME_MAX_LEN 32
@@ -19,6 +20,7 @@ enum eal_dev_req_type {
 	EAL_DEV_REQ_TYPE_DETACH,
 	EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK,
 	EAL_DEV_REQ_TYPE_DETACH_ROLLBACK,
+	EAL_DEV_REQ_TYPE_STOP,
 };
 
 struct eal_dev_mp_req {
-- 
2.27.0

[PATCH v3] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Khadem Ullah]

When the secondary process testpmd application running any fwd_engine
and after that primary has exited will cause a crash.

This patch forces secondary process forwarding lcores
in fwd_engine to stop before the primary process exits.

Fixes: a550baf24af90f ("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      | 25 ++++++++++++++++++++++---
 lib/eal/common/hotplug_mp.c |  4 ++++
 lib/eal/common/hotplug_mp.h |  2 ++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0f58a31eb5..f385bf920d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 #ifndef RTE_EXEC_ENV_WINDOWS
 #include <sys/mman.h>
+#include "hotplug_mp.h"
 #endif
 #include <sys/types.h>
 #include <errno.h>
@@ -103,7 +104,7 @@ 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)
+#define MONITOR_INTERVAL (500)
 
 /* use main core for command line ? */
 uint8_t interactive = 0;
@@ -2563,7 +2564,12 @@ stop_packet_forwarding(void)
 	for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++)
 		fwd_lcores[lc_id]->stopped = 1;
 	printf("\nWaiting for lcores to finish...\n");
-	rte_eal_mp_wait_lcore();
+#ifndef RTE_EXEC_ENV_WINDOWS
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_eal_mp_wait_lcore();
+#else
+		rte_eal_mp_wait_lcore();
+#endif
 	port_fwd_end = cur_fwd_config.fwd_eng->port_fwd_end;
 	if (port_fwd_end != NULL) {
 		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
@@ -3634,6 +3640,13 @@ pmd_test_exit(void)
 			printf("\nStopping port %d...\n", pt_id);
 			fflush(stdout);
 			stop_port(pt_id);
+#ifndef RTE_EXEC_ENV_WINDOWS
+			printf("Stopping secondary process...\n");
+			struct eal_dev_mp_req req;
+			memset(&req, 0, sizeof(req));
+			req.t = EAL_DEV_REQ_TYPE_STOP;
+			eal_dev_hotplug_request_to_secondary(&req);
+#endif
 		}
 		RTE_ETH_FOREACH_DEV(pt_id) {
 			printf("\nShutting down port %d...\n", pt_id);
@@ -4371,7 +4384,13 @@ static void
 monitor_primary(void *arg __rte_unused)
 {
 	if (rte_eal_primary_proc_alive(NULL)) {
-		rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+		if (f_exit_sec) {
+			stop_packet_forwarding();
+			fprintf(stderr, "\nPrimary process is no longer active, exiting...\n");
+			exit(EXIT_FAILURE);
+		} else {
+			rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+		}
 	} else {
 		/*
 		 * If primary process exits, then all the device information
diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index 17089ca3db..d1913c50db 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -15,6 +15,7 @@
 #include "eal_private.h"
 
 #define MP_TIMEOUT_S 5 /**< 5 seconds timeouts */
+uint8_t f_exit_sec;
 
 struct mp_reply_bundle {
 	struct rte_mp_msg msg;
@@ -239,6 +240,9 @@ static void __handle_primary_request(void *param)
 	memset(&mp_resp, 0, sizeof(mp_resp));
 
 	switch (req->t) {
+	case EAL_DEV_REQ_TYPE_STOP:
+		f_exit_sec = 1;
+		break;
 	case EAL_DEV_REQ_TYPE_ATTACH:
 	case EAL_DEV_REQ_TYPE_DETACH_ROLLBACK:
 		ret = local_dev_probe(req->devargs, &dev);
diff --git a/lib/eal/common/hotplug_mp.h b/lib/eal/common/hotplug_mp.h
index 7221284286..a33e3cbcc5 100644
--- a/lib/eal/common/hotplug_mp.h
+++ b/lib/eal/common/hotplug_mp.h
@@ -9,6 +9,7 @@
 
 #define EAL_DEV_MP_ACTION_REQUEST      "eal_dev_mp_request"
 #define EAL_DEV_MP_ACTION_RESPONSE     "eal_dev_mp_response"
+extern uint8_t f_exit_sec;
 
 #define EAL_DEV_MP_DEV_NAME_MAX_LEN RTE_DEV_NAME_MAX_LEN
 #define EAL_DEV_MP_BUS_NAME_MAX_LEN 32
@@ -19,6 +20,7 @@ enum eal_dev_req_type {
 	EAL_DEV_REQ_TYPE_DETACH,
 	EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK,
 	EAL_DEV_REQ_TYPE_DETACH_ROLLBACK,
+	EAL_DEV_REQ_TYPE_STOP,
 };
 
 struct eal_dev_mp_req {
-- 
2.27.0

[PATCH v3] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Khadem Ullah]

When the secondary process testpmd application running any fwd_engine
and after that primary has exited will cause a crash.

This patch forces secondary process forwarding lcores
in fwd_engine to stop before the primary process exits.

Fixes: a550baf24af90f ("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      | 27 ++++++++++++++++++++++++---
 lib/eal/common/hotplug_mp.c |  4 ++++
 lib/eal/common/hotplug_mp.h |  2 ++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0f58a31eb5..bf2f578b80 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -75,6 +75,7 @@
 #endif
 
 #include "testpmd.h"
+#include "hotplug_mp.h"
 
 #ifndef MAP_HUGETLB
 /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
@@ -103,7 +104,7 @@ 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)
+#define MONITOR_INTERVAL (500)
 
 /* use main core for command line ? */
 uint8_t interactive = 0;
@@ -2563,7 +2564,12 @@ stop_packet_forwarding(void)
 	for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++)
 		fwd_lcores[lc_id]->stopped = 1;
 	printf("\nWaiting for lcores to finish...\n");
-	rte_eal_mp_wait_lcore();
+#ifndef RTE_EXEC_ENV_WINDOWS
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_eal_mp_wait_lcore();
+#else
+		rte_eal_mp_wait_lcore();
+#endif
 	port_fwd_end = cur_fwd_config.fwd_eng->port_fwd_end;
 	if (port_fwd_end != NULL) {
 		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
@@ -3634,6 +3640,15 @@ pmd_test_exit(void)
 			printf("\nStopping port %d...\n", pt_id);
 			fflush(stdout);
 			stop_port(pt_id);
+#ifndef RTE_EXEC_ENV_WINDOWS
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			printf("Stopping secondary process...\n");
+			struct eal_dev_mp_req req;
+			memset(&req, 0, sizeof(req));
+			req.t = EAL_DEV_REQ_TYPE_STOP;
+			eal_dev_hotplug_request_to_secondary(&req);
+		}
+#endif
 		}
 		RTE_ETH_FOREACH_DEV(pt_id) {
 			printf("\nShutting down port %d...\n", pt_id);
@@ -4371,7 +4386,13 @@ static void
 monitor_primary(void *arg __rte_unused)
 {
 	if (rte_eal_primary_proc_alive(NULL)) {
-		rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+		if (f_exit_sec) {
+			stop_packet_forwarding();
+			fprintf(stderr, "\nPrimary process is no longer active, exiting...\n");
+			exit(EXIT_FAILURE);
+		} else {
+			rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
+		}
 	} else {
 		/*
 		 * If primary process exits, then all the device information
diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index 17089ca3db..d1913c50db 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -15,6 +15,7 @@
 #include "eal_private.h"
 
 #define MP_TIMEOUT_S 5 /**< 5 seconds timeouts */
+uint8_t f_exit_sec;
 
 struct mp_reply_bundle {
 	struct rte_mp_msg msg;
@@ -239,6 +240,9 @@ static void __handle_primary_request(void *param)
 	memset(&mp_resp, 0, sizeof(mp_resp));
 
 	switch (req->t) {
+	case EAL_DEV_REQ_TYPE_STOP:
+		f_exit_sec = 1;
+		break;
 	case EAL_DEV_REQ_TYPE_ATTACH:
 	case EAL_DEV_REQ_TYPE_DETACH_ROLLBACK:
 		ret = local_dev_probe(req->devargs, &dev);
diff --git a/lib/eal/common/hotplug_mp.h b/lib/eal/common/hotplug_mp.h
index 7221284286..a33e3cbcc5 100644
--- a/lib/eal/common/hotplug_mp.h
+++ b/lib/eal/common/hotplug_mp.h
@@ -9,6 +9,7 @@
 
 #define EAL_DEV_MP_ACTION_REQUEST      "eal_dev_mp_request"
 #define EAL_DEV_MP_ACTION_RESPONSE     "eal_dev_mp_response"
+extern uint8_t f_exit_sec;
 
 #define EAL_DEV_MP_DEV_NAME_MAX_LEN RTE_DEV_NAME_MAX_LEN
 #define EAL_DEV_MP_BUS_NAME_MAX_LEN 32
@@ -19,6 +20,7 @@ enum eal_dev_req_type {
 	EAL_DEV_REQ_TYPE_DETACH,
 	EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK,
 	EAL_DEV_REQ_TYPE_DETACH_ROLLBACK,
+	EAL_DEV_REQ_TYPE_STOP,
 };
 
 struct eal_dev_mp_req {
-- 
2.27.0

Re: [PATCH v3] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Stephen Hemminger]

On Mon, 15 Sep 2025 04:25:13 -0700
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> +#ifndef RTE_EXEC_ENV_WINDOWS
> +		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +			printf("Stopping secondary process...\n");
> +			struct eal_dev_mp_req req;
> +			memset(&req, 0, sizeof(req));
> +			req.t = EAL_DEV_REQ_TYPE_STOP;
> +			eal_dev_hotplug_request_to_secondary(&req);
> +		}
> +#endif
>  		}

This could be improved:
	1. The structure can just be iniatialised as:
		struct eal_dev_mp_req req = { .t = EAL_DEV_REQ_TYPE_STOP };
	2. Check the result of sending request to secondary

	3. This is really a different MP request path, please don't
	   overload hotplug for this. Register a new type with rte_mp_action_register()

        4. If done right, you should be able to drop the #ifdef around WINDOWS.
           For now, there will be no secondary on Windows so the request to send
           will just succeed as a nop.


The other issue is that stopping the secondary would be faster if you
used existing f_stop flag to break out of the forwarding loop.

[PATCH v4] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Khadem Ullah]

When the secondary process testpmd application running any fwd_engine
and after that primary has exited will cause a crash.

This patch forces secondary process forwarding lcores
in fwd_engine to stop before the primary process exits.

Fixes: a550baf24af90f ("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      | 57 ++++++++++++++++++++++++++++++++++---
 app/test-pmd/testpmd.h      |  1 -
 lib/eal/common/hotplug_mp.h |  1 +
 3 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0f58a31eb5..474c97ed1c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -75,6 +75,7 @@
 #endif
 
 #include "testpmd.h"
+#include "hotplug_mp.h"
 
 #ifndef MAP_HUGETLB
 /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
@@ -103,7 +104,7 @@ 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)
+#define MONITOR_INTERVAL (500)
 
 /* use main core for command line ? */
 uint8_t interactive = 0;
@@ -238,6 +239,7 @@ unsigned int xstats_display_num; /**< Size of extended statistics to show */
  */
 volatile uint8_t f_quit;
 uint8_t cl_quit; /* Quit testpmd from cmdline. */
+volatile uint8_t f_exit_sec;
 
 /*
  * Max Rx frame size, set by '--max-pkt-len' parameter.
@@ -2563,7 +2565,12 @@ stop_packet_forwarding(void)
 	for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++)
 		fwd_lcores[lc_id]->stopped = 1;
 	printf("\nWaiting for lcores to finish...\n");
-	rte_eal_mp_wait_lcore();
+#ifndef RTE_EXEC_ENV_WINDOWS
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_eal_mp_wait_lcore();
+#else
+		rte_eal_mp_wait_lcore();
+#endif
 	port_fwd_end = cur_fwd_config.fwd_eng->port_fwd_end;
 	if (port_fwd_end != NULL) {
 		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
@@ -3630,6 +3637,19 @@ pmd_test_exit(void)
 #endif
 	if (ports != NULL) {
 		no_link_check = 1;
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			struct eal_dev_mp_req req = { .t = EAL_DEV_REQ_TYPE_STOP,
+				.result = 0};
+			struct rte_mp_msg mp_req;
+			memset(&mp_req, 0, sizeof(mp_req));
+			snprintf(mp_req.name, sizeof(mp_req.name), "eal_dev_mp_stop_req");
+			memcpy(mp_req.param, &req, sizeof(req));
+			mp_req.len_param = sizeof(req);
+			struct rte_mp_reply reply;
+			struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
+			printf("Primary: Sending 'stop_req' request to secondary...\n");
+			rte_mp_request_sync(&mp_req, &reply, &ts);
+		}
 		RTE_ETH_FOREACH_DEV(pt_id) {
 			printf("\nStopping port %d...\n", pt_id);
 			fflush(stdout);
@@ -4365,12 +4385,35 @@ signal_handler(int signum __rte_unused)
 	prompt_exit();
 }
 
+static int
+handle_eal_dev_request(const struct rte_mp_msg *request, const void *peer)
+{
+	const struct eal_dev_mp_req *req =
+		(const struct eal_dev_mp_req *)request->param;
+	struct eal_dev_mp_req reply_data;
+	struct rte_mp_msg reply_msg;
+
+	printf("Received message from primary\n");
+	if (req->t == EAL_DEV_REQ_TYPE_STOP) {
+		f_quit = 1;
+		reply_data.result = 0;
+	} else {
+		reply_data.result = -1;
+	}
+	memset(&reply_msg, 0, sizeof(reply_msg));
+	strlcpy(reply_msg.name, request->name, sizeof(reply_msg.name));
+	memcpy(reply_msg.param, &reply_data, sizeof(reply_data));
+	rte_mp_reply(&reply_msg, peer);
+	printf("Sent stop reply to primary\n");
+	return 0;
+}
+
 #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)) {
+	if (rte_eal_primary_proc_alive(NULL) && (!f_quit)) {
 		rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL);
 	} else {
 		/*
@@ -4378,6 +4421,7 @@ monitor_primary(void *arg __rte_unused)
 		 * is no longer valid. Calling any cleanup code is going to
 		 * run into use after free.
 		 */
+		stop_packet_forwarding();
 		fprintf(stderr, "\nPrimary process is no longer active, exiting...\n");
 		exit(EXIT_FAILURE);
 	}
@@ -4432,6 +4476,9 @@ main(int argc, char** argv)
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
 	    enable_primary_monitor() < 0)
 		rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
+	else if (rte_mp_action_register("eal_dev_mp_stop_req",
+				handle_eal_dev_request) != 0)
+		rte_exit(EXIT_FAILURE, "Failed to register message action\n");
 #endif
 
 	/* allocate port structures, and init them */
@@ -4628,8 +4675,10 @@ main(int argc, char** argv)
 	}
 
 #ifndef RTE_EXEC_ENV_WINDOWS
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
 		disable_primary_monitor();
+		rte_mp_action_unregister("eal_dev_mp_stop_req");
+	}
 #endif
 
 	pmd_test_exit();
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 96ac88a29c..79adb82f74 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -34,7 +34,6 @@
 #define RTE_PORT_HANDLING       (uint16_t)3
 
 extern uint8_t cl_quit;
-extern volatile uint8_t f_quit;
 
 /*
  * It is used to allocate the memory for hash key.
diff --git a/lib/eal/common/hotplug_mp.h b/lib/eal/common/hotplug_mp.h
index 7221284286..bfea2906c0 100644
--- a/lib/eal/common/hotplug_mp.h
+++ b/lib/eal/common/hotplug_mp.h
@@ -19,6 +19,7 @@ enum eal_dev_req_type {
 	EAL_DEV_REQ_TYPE_DETACH,
 	EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK,
 	EAL_DEV_REQ_TYPE_DETACH_ROLLBACK,
+	EAL_DEV_REQ_TYPE_STOP,
 };
 
 struct eal_dev_mp_req {
-- 
2.27.0

Re: [PATCH v4] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Stephen Hemminger]

On Wed, 17 Sep 2025 04:36:01 -0700
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> When the secondary process testpmd application running any fwd_engine
> and after that primary has exited will cause a crash.
> 
> This patch forces secondary process forwarding lcores
> in fwd_engine to stop before the primary process exits.
> 
> Fixes: a550baf24af90f ("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>
> ---

There are still several issue left:
- using hotplug mp structs
- sending reply before fully exited.

Try this. It worked for simple quit and killing primary testpmd
with an ILL signal.



diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0f58a31eb5..4c949adeda 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -46,6 +46,7 @@
 #include <rte_ether.h>
 #include <rte_ethdev.h>
 #include <rte_dev.h>
+#include <rte_stdatomic.h>
 #include <rte_string_fns.h>
 #include <rte_pci.h>
 #ifdef RTE_NET_IXGBE
@@ -3609,6 +3610,83 @@ detach_devargs(char *identifier)
 	rte_devargs_reset(&da);
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
+
+enum testpmd_req_type {
+	TESTPMD_REQ_TYPE_EXIT,
+};
+
+struct testpmd_mp_req {
+	enum testpmd_req_type t;
+};
+
+struct testpmd_mp_resp {
+	int result;
+};
+
+#define TESTPMD_MP	"mp_testpmd"
+
+/* Send reply to this peer when testpmd exits */
+static RTE_ATOMIC(const char *) primary_name;
+
+static void
+reply_to_primary(const char *peer, int result)
+{
+	struct rte_mp_msg reply = { };
+	struct testpmd_mp_resp *resp = (struct testpmd_mp_resp *) &reply.param;
+
+	strlcpy(reply.name, TESTPMD_MP, RTE_MP_MAX_NAME_LEN);
+	reply.len_param = sizeof(*resp);
+	resp->result = result;
+
+	printf("Replying %d to primary\n", result);
+	fflush(stdout);
+
+	if (rte_mp_reply(&reply, peer) < 0)
+		printf("Failed to send response to primary:%s", strerror(rte_errno));
+}
+
+/* Primary process is exiting, stop secondary process */
+static void
+pmd_notify_secondary(void)
+{
+	struct testpmd_mp_req request = {
+		.t = TESTPMD_REQ_TYPE_EXIT,
+	};
+	struct rte_mp_msg mp_req = {
+		.name = TESTPMD_MP,
+		.len_param = sizeof(&request),
+	};
+	struct rte_mp_reply reply;
+	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
+
+	printf("\nPrimary: Sending 'stop_req' request to secondary...\n");
+	fflush(stdout);
+
+	memcpy(mp_req.param, &request, sizeof(request));
+	rte_mp_request_sync(&mp_req, &reply, &ts);
+}
+
+static int
+handle_testpmd_request(const struct rte_mp_msg *request, const void *peer)
+{
+	const struct testpmd_mp_req *req = (const struct testpmd_mp_req *)request->param;
+
+	if (req->t == TESTPMD_REQ_TYPE_EXIT) {
+		printf("\nReceived notification of primary exiting\n");
+		fflush(stdout);
+
+		/* Response is sent after forwarding loop exits */
+		rte_atomic_store_explicit(&primary_name, peer, rte_memory_order_relaxed);
+
+		kill(getpid(), SIGINT);
+	} else {
+		reply_to_primary(peer, -EINVAL);
+	}
+	return 0;
+}
+#endif
+
 void
 pmd_test_exit(void)
 {
@@ -3620,6 +3698,10 @@ pmd_test_exit(void)
 		stop_packet_forwarding();
 
 #ifndef RTE_EXEC_ENV_WINDOWS
+	/* Tell secondary to exit */
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		pmd_notify_secondary();
+
 	for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
 		if (mempools[i]) {
 			if (mp_alloc_type == MP_ALLOC_ANON)
@@ -3628,6 +3710,7 @@ pmd_test_exit(void)
 		}
 	}
 #endif
+
 	if (ports != NULL) {
 		no_link_check = 1;
 		RTE_ETH_FOREACH_DEV(pt_id) {
@@ -4429,9 +4512,12 @@ main(int argc, char** argv)
 			 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");
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		if (enable_primary_monitor() < 0)
+			rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
+		if (rte_mp_action_register(TESTPMD_MP, handle_testpmd_request) < 0)
+		    rte_exit(EXIT_FAILURE, "Failed to register message action\n");
+	}
 #endif
 
 	/* allocate port structures, and init them */
@@ -4628,12 +4714,23 @@ main(int argc, char** argv)
 	}
 
 #ifndef RTE_EXEC_ENV_WINDOWS
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
 		disable_primary_monitor();
+		rte_mp_action_unregister(TESTPMD_MP);
+	}
 #endif
-
 	pmd_test_exit();
 
+#ifndef RTE_EXEC_ENV_WINDOWS
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		const char *peer
+			= rte_atomic_exchange_explicit(&primary_name, NULL, rte_memory_order_relaxed);
+		if (peer)
+			reply_to_primary(peer, 0);
+	}
+#endif
+
+
 #ifdef RTE_LIB_PDUMP
 	/* uninitialize packet capture framework */
 	rte_pdump_uninit();
-- 
2.47.3

[PATCH v5] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Khadem Ullah]

When the secondary process testpmd application running any fwd_engine
and after that primary has exited will cause a crash.

This patch forces secondary process forwarding lcores
in fwd_engine to stop before the primary process exits.

Fixes: a550baf24af90f ("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 | 104 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0f58a31eb5..6c97026962 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3609,6 +3609,83 @@ detach_devargs(char *identifier)
 	rte_devargs_reset(&da);
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
+
+enum testpmd_req_type {
+	TESTPMD_REQ_TYPE_EXIT,
+};
+
+struct testpmd_mp_req {
+	enum testpmd_req_type t;
+};
+
+struct testpmd_mp_resp {
+	int result;
+};
+
+#define TESTPMD_MP	"mp_testpmd"
+
+/* Send reply to this peer when testpmd exits */
+static RTE_ATOMIC(const char *) primary_name;
+
+static void
+reply_to_primary(const char *peer, int result)
+{
+	struct rte_mp_msg reply = { };
+	struct testpmd_mp_resp *resp = (struct testpmd_mp_resp *) &reply.param;
+
+	strlcpy(reply.name, TESTPMD_MP, RTE_MP_MAX_NAME_LEN);
+	reply.len_param = sizeof(*resp);
+	resp->result = result;
+
+	printf("Replying %d to primary\n", result);
+	fflush(stdout);
+
+	if (rte_mp_reply(&reply, peer) < 0)
+		printf("Failed to send response to primary:%s", strerror(rte_errno));
+}
+
+/* Primary process is exiting, stop secondary process */
+static void
+pmd_notify_secondary(void)
+{
+	struct testpmd_mp_req request = {
+		.t = TESTPMD_REQ_TYPE_EXIT,
+	};
+	struct rte_mp_msg mp_req = {
+		.name = TESTPMD_MP,
+		.len_param = sizeof(request),
+	};
+	struct rte_mp_reply reply;
+	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
+
+	printf("\nPrimary: Sending 'stop_req' request to secondary...\n");
+	fflush(stdout);
+
+	memcpy(mp_req.param, &request, sizeof(request));
+	rte_mp_request_sync(&mp_req, &reply, &ts);
+}
+
+static int
+handle_testpmd_request(const struct rte_mp_msg *request, const void *peer)
+{
+	const struct testpmd_mp_req *req = (const struct testpmd_mp_req *)request->param;
+
+	if (req->t == TESTPMD_REQ_TYPE_EXIT) {
+		printf("\nReceived notification of primary exiting\n");
+		fflush(stdout);
+
+		/* Response is sent after forwarding loop exits */
+		rte_atomic_store_explicit(&primary_name, peer, rte_memory_order_relaxed);
+
+		kill(getpid(), SIGINT);
+	} else {
+		reply_to_primary(peer, -EINVAL);
+	}
+	return 0;
+}
+#endif
+
 void
 pmd_test_exit(void)
 {
@@ -3620,6 +3697,10 @@ pmd_test_exit(void)
 		stop_packet_forwarding();
 
 #ifndef RTE_EXEC_ENV_WINDOWS
+	/* Tell secondary to exit */
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		pmd_notify_secondary();
+
 	for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
 		if (mempools[i]) {
 			if (mp_alloc_type == MP_ALLOC_ANON)
@@ -3628,6 +3709,7 @@ pmd_test_exit(void)
 		}
 	}
 #endif
+
 	if (ports != NULL) {
 		no_link_check = 1;
 		RTE_ETH_FOREACH_DEV(pt_id) {
@@ -4429,9 +4511,12 @@ main(int argc, char** argv)
 			 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");
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		if (enable_primary_monitor() < 0)
+			rte_exit(EXIT_FAILURE, "Cannot setup primary monitor");
+		if (rte_mp_action_register(TESTPMD_MP, handle_testpmd_request) < 0)
+			rte_exit(EXIT_FAILURE, "Failed to register message action\n");
+	}
 #endif
 
 	/* allocate port structures, and init them */
@@ -4628,12 +4713,23 @@ main(int argc, char** argv)
 	}
 
 #ifndef RTE_EXEC_ENV_WINDOWS
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
 		disable_primary_monitor();
+		rte_mp_action_unregister(TESTPMD_MP);
+	}
 #endif
 
 	pmd_test_exit();
 
+#ifndef RTE_EXEC_ENV_WINDOWS
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		const char *peer = rte_atomic_exchange_explicit(&primary_name, NULL,
+				rte_memory_order_relaxed);
+		if (peer)
+			reply_to_primary(peer, 0);
+	}
+#endif
+
 #ifdef RTE_LIB_PDUMP
 	/* uninitialize packet capture framework */
 	rte_pdump_uninit();
-- 
2.27.0

Re: [PATCH v5] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Stephen Hemminger]

On Wed, 17 Sep 2025 23:47:03 -0700
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> @@ -3628,6 +3709,7 @@ pmd_test_exit(void)
>  		}
>  	}
>  #endif
> +
>  	if (ports != NULL) {
>  		no_link_check = 1;
>  		RTE_ETH_FOREACH_DEV(pt_id) {

I will drop that extra newline... Sorry

> +
> +static int
> +handle_testpmd_request(const struct rte_mp_msg *request, const void *peer)
> +{
> +	const struct testpmd_mp_req *req = (const struct testpmd_mp_req *)request->param;
> +
> +	if (req->t == TESTPMD_REQ_TYPE_EXIT) {
> +		printf("\nReceived notification of primary exiting\n");
> +		fflush(stdout);
> +
> +		/* Response is sent after forwarding loop exits */
> +		rte_atomic_store_explicit(&primary_name, peer, rte_memory_order_relaxed);
> +
> +		kill(getpid(), SIGINT);

Needed to use kill() here to force end of read when testpmd is in interactive mode.
Only other option would be closing stdin, but that seemed less safe.

Re: [PATCH v5] app/testpmd: stop secondary process fwd_lcores during primary teardown

[Stephen Hemminger]

On Wed, 17 Sep 2025 23:47:03 -0700
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> When the secondary process testpmd application running any fwd_engine
> and after that primary has exited will cause a crash.
> 
> This patch forces secondary process forwarding lcores
> in fwd_engine to stop before the primary process exits.
> 
> Fixes: a550baf24af90f ("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>

Queued to next-net

Re: [PATCH v2] lib/ethdev: fix segfault in secondary process by validating dev_private pointer

[Stephen Hemminger]

On Wed, 23 Jul 2025 00:50:22 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> In secondary processes, directly accessing 'dev->data->dev_private' can
> cause a segmentation fault if the primary process has exited or if the
> shared memory is no longer accessible.
> 
> This patch adds a safety check using rte_mem_virt2phy(), with an
> unlikely() branch hint to minimize performance impact in the fast path.
> This ensures 'dev_private' is still valid before accessing it.
> 
> Fixes: bdad90d12ec8 ("ethdev: change device info get callback to return int")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> ---

No still a band-aid solution, and it is depending on side effects.

Is your secondary process monitoring for primary process exiting?

If it is, then lets fix the monitoring path.