| Message ID | 1486164969-2566-1-git-send-email-qi.z.zhang@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Ferruh Yigit |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
| ci/Intel compilation | success | Compilation OK |
On 2/3/2017 11:36 PM, Qi Zhang wrote: > dev_flags is wrong overwritten by RTE_ETH_DEV_DETACHABLE after > rte_eth_copy_pci_info. > > Fixes: 22dda618c00c ("pci: separate detaching ethernet ports from PCI devices") > > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com> > --- > drivers/net/bnxt/bnxt_ethdev.c | 2 +- > drivers/net/e1000/igb_ethdev.c | 4 ++-- > drivers/net/fm10k/fm10k_ethdev.c | 2 +- > drivers/net/i40e/i40e_ethdev.c | 2 +- > drivers/net/i40e/i40e_ethdev_vf.c | 2 +- > drivers/net/ixgbe/ixgbe_ethdev.c | 4 ++-- > drivers/net/nfp/nfp_net.c | 2 +- > drivers/net/vmxnet3/vmxnet3_ethdev.c | 2 +- > 8 files changed, 10 insertions(+), 10 deletions(-) > Hi Qi, Thanks for the fix. Is following missed, or intentionally excluded: drivers/net/e1000/em_ethdev.c drivers/net/virtio/virtio_ethdev.c Thanks, ferruh
On 2/7/2017 10:24 AM, Ferruh Yigit wrote: > On 2/3/2017 11:36 PM, Qi Zhang wrote: >> dev_flags is wrong overwritten by RTE_ETH_DEV_DETACHABLE after >> rte_eth_copy_pci_info. >> >> Fixes: 22dda618c00c ("pci: separate detaching ethernet ports from PCI devices") >> >> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com> >> --- >> drivers/net/bnxt/bnxt_ethdev.c | 2 +- >> drivers/net/e1000/igb_ethdev.c | 4 ++-- >> drivers/net/fm10k/fm10k_ethdev.c | 2 +- >> drivers/net/i40e/i40e_ethdev.c | 2 +- >> drivers/net/i40e/i40e_ethdev_vf.c | 2 +- >> drivers/net/ixgbe/ixgbe_ethdev.c | 4 ++-- >> drivers/net/nfp/nfp_net.c | 2 +- >> drivers/net/vmxnet3/vmxnet3_ethdev.c | 2 +- >> 8 files changed, 10 insertions(+), 10 deletions(-) >> > > Hi Qi, > > Thanks for the fix. > > Is following missed, or intentionally excluded: > > drivers/net/e1000/em_ethdev.c > drivers/net/virtio/virtio_ethdev.c Hi Yuanhan, I saw while checking for this, there can be an issue in virtio for this path, can you please check: eth_virtio_dev_init() set-some-eth_dev-dev_flags virtio_init_device() rte_eth_copy_pci_info() eth_dev->data->dev_flags = 0; <---- Reset previous flags Regards, ferruh
On Tue, Feb 07, 2017 at 10:32:32AM +0000, Ferruh Yigit wrote: > On 2/7/2017 10:24 AM, Ferruh Yigit wrote: > > On 2/3/2017 11:36 PM, Qi Zhang wrote: > >> dev_flags is wrong overwritten by RTE_ETH_DEV_DETACHABLE after > >> rte_eth_copy_pci_info. > >> ... > Hi Yuanhan, > > I saw while checking for this, there can be an issue in virtio for this > path, can you please check: > > eth_virtio_dev_init() > set-some-eth_dev-dev_flags > virtio_init_device() > rte_eth_copy_pci_info() > eth_dev->data->dev_flags = 0; <---- Reset previous flags Hi Ferruh, Nice catch! And yes, it's buggy. It may be trivially fixed by setting the orders correctly. But my question is why "rte_eth_copy_pci_info" does a reset on "dev_flags"? It's an *un-documented* side-effect of this function, which could be very error-prone, like this one. --yliu
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 501c97d..aaff3c8 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -1087,7 +1087,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev) RTE_LOG(INFO, PMD, "%s", bnxt_version); rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; bp = eth_dev->data->dev_private; if (bnxt_vf_pciid(pci_dev->id.device_id)) diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index 8843dd1..4fd5786 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -777,7 +777,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev) } rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; hw->hw_addr= (void *)pci_dev->mem_resource[0].addr; @@ -986,7 +986,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev) pci_dev = E1000_DEV_TO_PCI(eth_dev); rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; hw->device_id = pci_dev->id.device_id; hw->vendor_id = pci_dev->id.vendor_id; diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c index dd021e4..c4fe746 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -2858,7 +2858,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev) return 0; rte_eth_copy_pci_info(dev, pdev); - dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private); memset(macvlan, 0, sizeof(*macvlan)); diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index a818998..f8ec91c 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -1045,7 +1045,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev) intr_handle = &pci_dev->intr_handle; rte_eth_copy_pci_info(dev, pci_dev); - dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; pf->adapter = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private); pf->adapter->eth_dev = dev; diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index 17a035c..d2c25e8 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -1478,7 +1478,7 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev) } rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; hw->vendor_id = pci_dev->id.vendor_id; hw->device_id = pci_dev->id.device_id; diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index ad63e5a..c2433c0 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -1170,7 +1170,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev) } rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; /* Vendor and Device ID need to be set before init of shared code */ hw->device_id = pci_dev->id.device_id; @@ -1621,7 +1621,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev) } rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; hw->device_id = pci_dev->id.device_id; hw->vendor_id = pci_dev->id.vendor_id; diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index ab2d331..a2c78a6 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -2465,7 +2465,7 @@ nfp_net_init(struct rte_eth_dev *eth_dev) pci_dev = RTE_DEV_TO_PCI(eth_dev->device); rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; hw->device_id = pci_dev->id.device_id; hw->vendor_id = pci_dev->id.vendor_id; diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 52e3644..2a52f04 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -250,7 +250,7 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev) return 0; rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE; + eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; /* Vendor and Device ID need to be set before init of shared code */ hw->device_id = pci_dev->id.device_id;
dev_flags is wrong overwritten by RTE_ETH_DEV_DETACHABLE after rte_eth_copy_pci_info. Fixes: 22dda618c00c ("pci: separate detaching ethernet ports from PCI devices") Signed-off-by: Qi Zhang <qi.z.zhang@intel.com> --- drivers/net/bnxt/bnxt_ethdev.c | 2 +- drivers/net/e1000/igb_ethdev.c | 4 ++-- drivers/net/fm10k/fm10k_ethdev.c | 2 +- drivers/net/i40e/i40e_ethdev.c | 2 +- drivers/net/i40e/i40e_ethdev_vf.c | 2 +- drivers/net/ixgbe/ixgbe_ethdev.c | 4 ++-- drivers/net/nfp/nfp_net.c | 2 +- drivers/net/vmxnet3/vmxnet3_ethdev.c | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-)