thanks for your correction I will rewrite my commit log and send email again
方统浩50450 邮箱:fangtong...@sangfor.com.cn 签名由 网易邮箱大师 定制 On 01/10/2020 15:30, Jeff Guo wrote: hi, tonghao On 1/9/2020 8:27 PM, Fang TongHao wrote: > Hi all,I am from Sangfor Tech.I found a bug when using DPDK in > multiprocess scenario.The secondary process enters > "rte_eth_dev_pci_copy_info" function when initializing.Then it > sets the value of struct "rte_eth_dev_data.dev_flags" to zero, > but this struct is shared by primary process and secondary > process, and the value change is unexpected by primary process. > This may cause very serious damage.I think > the secondary process should not enter "rte_eth_dev_pci_copy_info" > function or changes the value of struct "rte_eth_dev_data.dev_flags" > in shared memory. > I fixed this bug by adding an if-statement to forbid the secondary > process changing the above-mentioned value. > Thansk, All. i think the format of commit log should be refined to be more formal like as below. what do you think? ethdev: XXXXXXXXX XXXXXXXX > Signed-off-by: Fang TongHao <fangtong...@sangfor.com.cn> if it is a fix, suggest to add the line as "Fixes: XXXXXXXX ("ethdev: XXXXXXX") to trace it. > --- > lib/librte_ethdev/rte_ethdev_pci.h | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/lib/librte_ethdev/rte_ethdev_pci.h > b/lib/librte_ethdev/rte_ethdev_pci.h > index ccdbb46ec..916de8a14 100644 > --- a/lib/librte_ethdev/rte_ethdev_pci.h > +++ b/lib/librte_ethdev/rte_ethdev_pci.h > @@ -59,15 +59,16 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev, > } > > eth_dev->intr_handle = &pci_dev->intr_handle; > - > - eth_dev->data->dev_flags = 0; > - if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC) > - eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC; > - if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_RMV) > - eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_RMV; > - > - eth_dev->data->kdrv = pci_dev->kdrv; > - eth_dev->data->numa_node = pci_dev->device.numa_node; > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > + eth_dev->data->dev_flags = 0; > + if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC) > + eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC; > + if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_RMV) > + eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_RMV; > + > + eth_dev->data->kdrv = pci_dev->kdrv; > + eth_dev->data->numa_node = pci_dev->device.numa_node; From the change log, you said that "rte_eth_dev_data.dev_flags" should not be touched by secondary process, but you don't mention about data->kdrv and data->numa_node, could you also explain them in the log if they need to process as the same. > + } > } > > static inline int