[v3] Fixes: ethdev: secondary process change shared memory

Message ID 20200117020804.1290-1-fangtonghao@sangfor.com.cn (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v3] Fixes: ethdev: secondary process change shared memory |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

方统浩50450 Jan. 17, 2020, 2:08 a.m. UTC
  Fixes the secondary process changed shared memory
in "rte_eth_copy_pci_info" function.In that function
only primary can update the value of "eth_dev->data"
which shared by primary and secondary.

Signed-off-by: Fang TongHao <fangtonghao@sangfor.com.cn>
---
 lib/librte_ethdev/rte_ethdev_pci.h | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)
  

Comments

Andrew Rybchenko Jan. 17, 2020, 8:33 a.m. UTC | #1
On 1/17/20 5:08 AM, Fang TongHao wrote:

Summary does not comply with [1].

> Fixes the secondary process changed shared memory
> in "rte_eth_copy_pci_info" function.In that function
> only primary can update the value of "eth_dev->data"
> which shared by primary and secondary.

Consider:
Avoid overwriting device flags and other information in device
data stored in shared memory when a secondary process
probes PCI device.

Fixes tag and CC to stable is required in accordance with [2].

> Signed-off-by: Fang TongHao <fangtonghao@sangfor.com.cn>

[1]
https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line
[2]
https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-body
  
Ferruh Yigit Jan. 17, 2020, 5:58 p.m. UTC | #2
On 1/17/2020 8:33 AM, Andrew Rybchenko wrote:
> On 1/17/20 5:08 AM, Fang TongHao wrote:
> 
> Summary does not comply with [1].
> 
>> Fixes the secondary process changed shared memory
>> in "rte_eth_copy_pci_info" function.In that function
>> only primary can update the value of "eth_dev->data"
>> which shared by primary and secondary.
> 
> Consider:
> Avoid overwriting device flags and other information in device
> data stored in shared memory when a secondary process
> probes PCI device.
> 
> Fixes tag and CC to stable is required in accordance with [2].
> 
>> Signed-off-by: Fang TongHao <fangtonghao@sangfor.com.cn>
> 
> [1]
> https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line
> [2]
> https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-body
> 

For sake of having the patch in -rc1, I have updated as suggested while merging,
commit log become as following:

    ethdev: fix secondary process memory overwrite

    Avoid overwriting device flags and other information in device
    data stored in shared memory when a secondary process
    probes PCI device.

    Fixes: 494adb7f63f2 ("ethdev: add device fields from PCI layer")
    Cc: stable@dpdk.org

    Signed-off-by: Fang TongHao <fangtonghao@sangfor.com.cn>
    Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
    Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>


And I have added a not the the function comment as following:
   * Copy pci device info to the Ethernet device data.
 + * Shared memory (eth_dev->data) only updated by primary process, so it is safe
 + * to call this function from both primary and secondary processes.
  *


Applied to dpdk-next-net/master, thanks.
  

Patch

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