<div dir="auto">Hi Ivan, agree. I think we can atleast currently guard all the known crashes.<div dir="auto"> </div><div dir="auto">Sure, I will check the macro and get back to you. </div><div dir="auto"> </div><div dir="auto">Thank you!</div></div> <div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Wed, Jul 23, 2025, 18:19 Ivan Malov <<a href="mailto:ivan.malov@arknetworks.am">ivan.malov@arknetworks.am</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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: <a href="mailto:stable@dpdk.org" target="_blank" rel="noreferrer">stable@dpdk.org</a> > > Signed-off-by: Khadem Ullah <<a href="mailto:14pwcse1224@uetpeshawar.edu.pk" target="_blank" rel="noreferrer">14pwcse1224@uetpeshawar.edu.pk</a>> > --- > 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 > > </blockquote></div>