[dpdk-dev] [PATCH] ethdev: fix secondary process change share memory
Jeff Guo
jia.guo at intel.com
Fri Jan 10 08:30:57 CET 2020
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 <fangtonghao at 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
More information about the dev
mailing list