Re: [dpdk-stable] [dpdk-dev] [PATCH v2] Fixes: ethdev: secondary process change shared memory

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/13/2020 5:03 AM, Fang TongHao wrote:
> Secondary process calls “rte_eth_dev_pci_allocate”
> function and enters rte_eth_copy_pci_info function
> when initializing.Then it sets the value of struct
> "rte_eth_dev_data.dev_flags" to zero and reset it,
> but this struct is shared by primary process and
> secondary process.To fix this bug,by adding an
> if-statement to forbid the secondaryprocess changing
> the above-mentioned value.

Hi Fang,

Thanks for the fix, I agree with the problem statement, but not sure if this
should be handled in the helper function or in the place where the function is
called. Helper function is simple on what it does, do we need to put the primary
process logic in it.

Can you please give more details of the bug you have encounter, is it seen by a
specific PMD?

Thanks,
ferruh

> 
> Signed-off-by: Fang TongHao <fangtonghao@sangfor.com.cn>
> ---
>  lib/librte_ethdev/rte_ethdev_pci.h | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev_pci.h b/lib/librte_ethdev/rte_ethdev_pci.h
> index ccdbb46ec..e7dae0545 100644
> --- a/lib/librte_ethdev/rte_ethdev_pci.h
> +++ b/lib/librte_ethdev/rte_ethdev_pci.h
> @@ -60,14 +60,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;
> +	}
>  }
>  
>  static inline int
>