drivers/net: fix segfault in secondary process

Message ID 20180719164556.93162-1-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series drivers/net: fix segfault in secondary process |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit July 19, 2018, 4:45 p.m. UTC
  Calling rte_eth_dev_info_get() on secondary process cause a crash
because eth_dev->device is not set properly.

Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Vipin Varghese <vipin.varghese@intel.com>
---
 drivers/net/af_packet/rte_eth_af_packet.c | 1 +
 drivers/net/bonding/rte_eth_bond_pmd.c    | 1 +
 drivers/net/failsafe/failsafe.c           | 1 +
 drivers/net/kni/rte_eth_kni.c             | 1 +
 drivers/net/null/rte_eth_null.c           | 1 +
 drivers/net/octeontx/octeontx_ethdev.c    | 1 +
 drivers/net/pcap/rte_eth_pcap.c           | 1 +
 drivers/net/tap/rte_eth_tap.c             | 2 ++
 drivers/net/vhost/rte_eth_vhost.c         | 1 +
 9 files changed, 10 insertions(+)
  

Comments

Stephen Hemminger July 19, 2018, 4:32 p.m. UTC | #1
On Thu, 19 Jul 2018 17:45:56 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> Calling rte_eth_dev_info_get() on secondary process cause a crash
> because eth_dev->device is not set properly.
> 
> Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Good catch.

Maybe eth_dev should do this for the device drivers?
Better to make device drivers as simple and safe as possible.

There seem to be a lot of bugs related to secondary process model.
Do we have a test suite for that.
  
Qi Zhang July 19, 2018, 10:19 p.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Friday, July 20, 2018 12:46 AM
> To: John W. Linville <linville@tuxdriver.com>; Doherty, Declan
> <declan.doherty@intel.com>; Chas Williams <chas3@att.com>; Gaetan Rivet
> <gaetan.rivet@6wind.com>; Tetsuya Mukawa <mtetsuyah@gmail.com>;
> Santosh Shukla <santosh.shukla@caviumnetworks.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>; Wiles, Keith <keith.wiles@intel.com>;
> Maxime Coquelin <maxime.coquelin@redhat.com>; Bie, Tiwei
> <tiwei.bie@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org;
> Varghese, Vipin <vipin.varghese@intel.com>
> Subject: [dpdk-dev] [PATCH] drivers/net: fix segfault in secondary process
> 
> Calling rte_eth_dev_info_get() on secondary process cause a crash because
> eth_dev->device is not set properly.
> 
> Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Yes, this is the same issue I met when detaching the device from a secondary process.
The backend device should be assigned on secondary.

Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
  
Ferruh Yigit July 20, 2018, 10:13 a.m. UTC | #3
On 7/19/2018 5:32 PM, Stephen Hemminger wrote:
> On Thu, 19 Jul 2018 17:45:56 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> Calling rte_eth_dev_info_get() on secondary process cause a crash
>> because eth_dev->device is not set properly.
>>
>> Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Good catch.
> 
> Maybe eth_dev should do this for the device drivers?
> Better to make device drivers as simple and safe as possible.

Agreed, let me try to create a helper function for this.

> 
> There seem to be a lot of bugs related to secondary process model.
> Do we have a test suite for that.
>
  

Patch

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index ce1e31aa4..eb3cce3a6 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -936,6 +936,7 @@  rte_pmd_af_packet_probe(struct rte_vdev_device *dev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &ops;
+		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index fc4d4fd97..ca491a820 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3184,6 +3184,7 @@  bond_probe(struct rte_vdev_device *dev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &default_dev_ops;
+		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 7f89486d4..657919f93 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -328,6 +328,7 @@  rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &failsafe_ops;
+		eth_dev->device = &vdev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index b038fbf1a..085bb8452 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -420,6 +420,7 @@  eth_kni_probe(struct rte_vdev_device *vdev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &eth_kni_ops;
+		eth_dev->device = &vdev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index d85d25f7e..244f86545 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -624,6 +624,7 @@  rte_pmd_null_probe(struct rte_vdev_device *dev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &ops;
+		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c
index 21e5e4fca..f264bc64e 100644
--- a/drivers/net/octeontx/octeontx_ethdev.c
+++ b/drivers/net/octeontx/octeontx_ethdev.c
@@ -1183,6 +1183,7 @@  octeontx_probe(struct rte_vdev_device *dev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &octeontx_dev_ops;
+		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 6779f97c1..e8810a171 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -1017,6 +1017,7 @@  pmd_pcap_probe(struct rte_vdev_device *dev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &ops;
+		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 22ba872ed..0331eb9f8 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1931,6 +1931,7 @@  rte_pmd_tun_probe(struct rte_vdev_device *dev)
 			return -1;
 		}
 		eth_dev->dev_ops = &ops;
+		eth_dev->device = &dev->device;
 		return 0;
 	}
 
@@ -2000,6 +2001,7 @@  rte_pmd_tap_probe(struct rte_vdev_device *dev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &ops;
+		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 47b33456c..e58f32211 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1354,6 +1354,7 @@  rte_pmd_vhost_probe(struct rte_vdev_device *dev)
 		}
 		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &ops;
+		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}