[v6,06/19] ethdev: support attach private device as first

Message ID 20180628015247.42232-7-qi.z.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series enable hotplug on multi-process |

Checks

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

Commit Message

Qi Zhang June 28, 2018, 1:52 a.m. UTC
  When attach a private device from secondary as the first one, we need
to make sure rte_eth_dev_shared_data is initialized, the patch add
necessary IPC for secondary to inform primary to do initialization.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 lib/librte_ethdev/ethdev_mp.c      |  2 ++
 lib/librte_ethdev/ethdev_mp.h      |  1 +
 lib/librte_ethdev/ethdev_private.h |  3 +++
 lib/librte_ethdev/rte_ethdev.c     | 31 ++++++++++++++++++++-----------
 4 files changed, 26 insertions(+), 11 deletions(-)
  

Comments

Burakov, Anatoly June 28, 2018, 9:24 a.m. UTC | #1
On 28-Jun-18 2:52 AM, Qi Zhang wrote:
> When attach a private device from secondary as the first one, we need
> to make sure rte_eth_dev_shared_data is initialized, the patch add
> necessary IPC for secondary to inform primary to do initialization.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---

Does this mean hotplug is broken before this patch? Can it be moved earlier?
  
Qi Zhang June 28, 2018, 9:29 a.m. UTC | #2
> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Thursday, June 28, 2018 5:25 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Shelton, Benjamin H
> <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>
> Subject: Re: [PATCH v6 06/19] ethdev: support attach private device as first
> 
> On 28-Jun-18 2:52 AM, Qi Zhang wrote:
> > When attach a private device from secondary as the first one, we need
> > to make sure rte_eth_dev_shared_data is initialized, the patch add
> > necessary IPC for secondary to inform primary to do initialization.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> 
> Does this mean hotplug is broken before this patch? Can it be moved earlier?

Basically patch 4,5,6 depends on patch 3, they have no dependency each other.

> 
> --
> Thanks,
> Anatoly
  
Burakov, Anatoly June 28, 2018, 9:41 a.m. UTC | #3
On 28-Jun-18 10:29 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Burakov, Anatoly
>> Sent: Thursday, June 28, 2018 5:25 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
>> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
>> <ferruh.yigit@intel.com>; Shelton, Benjamin H
>> <benjamin.h.shelton@intel.com>; Vangati, Narender
>> <narender.vangati@intel.com>
>> Subject: Re: [PATCH v6 06/19] ethdev: support attach private device as first
>>
>> On 28-Jun-18 2:52 AM, Qi Zhang wrote:
>>> When attach a private device from secondary as the first one, we need
>>> to make sure rte_eth_dev_shared_data is initialized, the patch add
>>> necessary IPC for secondary to inform primary to do initialization.
>>>
>>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>>> ---
>>
>> Does this mean hotplug is broken before this patch? Can it be moved earlier?
> 
> Basically patch 4,5,6 depends on patch 3, they have no dependency each other.

So... is that a yes? :)

> 
>>
>> --
>> Thanks,
>> Anatoly
  
Qi Zhang June 28, 2018, 11:26 a.m. UTC | #4
> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Thursday, June 28, 2018 5:41 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Shelton, Benjamin H
> <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>
> Subject: Re: [PATCH v6 06/19] ethdev: support attach private device as first
> 
> On 28-Jun-18 10:29 AM, Zhang, Qi Z wrote:
> >
> >
> >> -----Original Message-----
> >> From: Burakov, Anatoly
> >> Sent: Thursday, June 28, 2018 5:25 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
> >> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> >> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> >> <ferruh.yigit@intel.com>; Shelton, Benjamin H
> >> <benjamin.h.shelton@intel.com>; Vangati, Narender
> >> <narender.vangati@intel.com>
> >> Subject: Re: [PATCH v6 06/19] ethdev: support attach private device
> >> as first
> >>
> >> On 28-Jun-18 2:52 AM, Qi Zhang wrote:
> >>> When attach a private device from secondary as the first one, we
> >>> need to make sure rte_eth_dev_shared_data is initialized, the patch
> >>> add necessary IPC for secondary to inform primary to do initialization.
> >>>
> >>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> >>> ---
> >>
> >> Does this mean hotplug is broken before this patch? Can it be moved
> earlier?
> >
> > Basically patch 4,5,6 depends on patch 3, they have no dependency each
> other.
> 
> So... is that a yes? :)

Yes, 3 ,6,4, 5 is better, will re-order
Thanks
Qi

> 
> >
> >>
> >> --
> >> Thanks,
> >> Anatoly
> 
> 
> --
> Thanks,
> Anatoly
  
Qi Zhang June 28, 2018, 11:45 a.m. UTC | #5
> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Thursday, June 28, 2018 7:27 PM
> To: Burakov, Anatoly <anatoly.burakov@intel.com>; thomas@monjalon.net
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Shelton, Benjamin H
> <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>
> Subject: RE: [PATCH v6 06/19] ethdev: support attach private device as first
> 
> 
> 
> > -----Original Message-----
> > From: Burakov, Anatoly
> > Sent: Thursday, June 28, 2018 5:41 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> > Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; Shelton, Benjamin H
> > <benjamin.h.shelton@intel.com>; Vangati, Narender
> > <narender.vangati@intel.com>
> > Subject: Re: [PATCH v6 06/19] ethdev: support attach private device as
> > first
> >
> > On 28-Jun-18 10:29 AM, Zhang, Qi Z wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Burakov, Anatoly
> > >> Sent: Thursday, June 28, 2018 5:25 PM
> > >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
> > >> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> > >> dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>;
> > >> Yigit, Ferruh <ferruh.yigit@intel.com>; Shelton, Benjamin H
> > >> <benjamin.h.shelton@intel.com>; Vangati, Narender
> > >> <narender.vangati@intel.com>
> > >> Subject: Re: [PATCH v6 06/19] ethdev: support attach private device
> > >> as first
> > >>
> > >> On 28-Jun-18 2:52 AM, Qi Zhang wrote:
> > >>> When attach a private device from secondary as the first one, we
> > >>> need to make sure rte_eth_dev_shared_data is initialized, the
> > >>> patch add necessary IPC for secondary to inform primary to do
> initialization.
> > >>>
> > >>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > >>> ---
> > >>
> > >> Does this mean hotplug is broken before this patch? Can it be moved
> > earlier?
> > >
> > > Basically patch 4,5,6 depends on patch 3, they have no dependency
> > > each
> > other.
> >
> > So... is that a yes? :)
> 
> Yes, 3 ,6,4, 5 is better, will re-order

Apologies, actually patch 6 depends on patch 5, which build the secondary to primary request channel, so current sequence is necessary.

> Thanks
> Qi
> 
> >
> > >
> > >>
> > >> --
> > >> Thanks,
> > >> Anatoly
> >
> >
> > --
> > Thanks,
> > Anatoly
  
Burakov, Anatoly June 28, 2018, 12:34 p.m. UTC | #6
On 28-Jun-18 12:45 PM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Zhang, Qi Z
>> Sent: Thursday, June 28, 2018 7:27 PM
>> To: Burakov, Anatoly <anatoly.burakov@intel.com>; thomas@monjalon.net
>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
>> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
>> <ferruh.yigit@intel.com>; Shelton, Benjamin H
>> <benjamin.h.shelton@intel.com>; Vangati, Narender
>> <narender.vangati@intel.com>
>> Subject: RE: [PATCH v6 06/19] ethdev: support attach private device as first
>>
>>
>>
>>> -----Original Message-----
>>> From: Burakov, Anatoly
>>> Sent: Thursday, June 28, 2018 5:41 PM
>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
>>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
>>> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
>>> <ferruh.yigit@intel.com>; Shelton, Benjamin H
>>> <benjamin.h.shelton@intel.com>; Vangati, Narender
>>> <narender.vangati@intel.com>
>>> Subject: Re: [PATCH v6 06/19] ethdev: support attach private device as
>>> first
>>>
>>> On 28-Jun-18 10:29 AM, Zhang, Qi Z wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Burakov, Anatoly
>>>>> Sent: Thursday, June 28, 2018 5:25 PM
>>>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
>>>>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>;
>>>>> dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>;
>>>>> Yigit, Ferruh <ferruh.yigit@intel.com>; Shelton, Benjamin H
>>>>> <benjamin.h.shelton@intel.com>; Vangati, Narender
>>>>> <narender.vangati@intel.com>
>>>>> Subject: Re: [PATCH v6 06/19] ethdev: support attach private device
>>>>> as first
>>>>>
>>>>> On 28-Jun-18 2:52 AM, Qi Zhang wrote:
>>>>>> When attach a private device from secondary as the first one, we
>>>>>> need to make sure rte_eth_dev_shared_data is initialized, the
>>>>>> patch add necessary IPC for secondary to inform primary to do
>> initialization.
>>>>>>
>>>>>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>>>>>> ---
>>>>>
>>>>> Does this mean hotplug is broken before this patch? Can it be moved
>>> earlier?
>>>>
>>>> Basically patch 4,5,6 depends on patch 3, they have no dependency
>>>> each
>>> other.
>>>
>>> So... is that a yes? :)
>>
>> Yes, 3 ,6,4, 5 is better, will re-order
> 
> Apologies, actually patch 6 depends on patch 5, which build the secondary to primary request channel, so current sequence is necessary.

OK, we can leave it as is then.
  

Patch

diff --git a/lib/librte_ethdev/ethdev_mp.c b/lib/librte_ethdev/ethdev_mp.c
index 0a93fa669..f2982c790 100644
--- a/lib/librte_ethdev/ethdev_mp.c
+++ b/lib/librte_ethdev/ethdev_mp.c
@@ -182,6 +182,8 @@  __handle_secondary_request(void *param)
 				ret = tmp_req.result;
 			}
 		}
+	} else if (req->t == REQ_TYPE_SHARE_DATA_PREPARE) {
+		eth_dev_shared_data_prepare();
 	} else {
 		ethdev_log(ERR, "unsupported secondary to primary request\n");
 		ret = -ENOTSUP;
diff --git a/lib/librte_ethdev/ethdev_mp.h b/lib/librte_ethdev/ethdev_mp.h
index 40be46c89..61fc381da 100644
--- a/lib/librte_ethdev/ethdev_mp.h
+++ b/lib/librte_ethdev/ethdev_mp.h
@@ -15,6 +15,7 @@  enum eth_dev_req_type {
 	REQ_TYPE_PRE_DETACH,
 	REQ_TYPE_DETACH,
 	REQ_TYPE_ATTACH_ROLLBACK,
+	REQ_TYPE_SHARE_DATA_PREPARE,
 };
 
 struct eth_dev_mp_req {
diff --git a/lib/librte_ethdev/ethdev_private.h b/lib/librte_ethdev/ethdev_private.h
index 981e7de8a..005d63afc 100644
--- a/lib/librte_ethdev/ethdev_private.h
+++ b/lib/librte_ethdev/ethdev_private.h
@@ -36,4 +36,7 @@  int do_eth_dev_attach(const char *devargs, uint16_t *port_id);
  */
 int do_eth_dev_detach(uint16_t port_id);
 
+/* Prepare shared data for multi-process */
+void eth_dev_shared_data_prepare(void);
+
 #endif
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 54d3d2369..c0f68fe09 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,11 +199,14 @@  rte_eth_find_next(uint16_t port_id)
 	return port_id;
 }
 
-static void
-rte_eth_dev_shared_data_prepare(void)
+void
+eth_dev_shared_data_prepare(void)
 {
 	const unsigned flags = 0;
 	const struct rte_memzone *mz;
+	struct eth_dev_mp_req req;
+
+	memset(&req, 0, sizeof(req));
 
 	rte_spinlock_lock(&rte_eth_shared_data_lock);
 
@@ -215,6 +218,12 @@  rte_eth_dev_shared_data_prepare(void)
 					rte_socket_id(), flags);
 		} else
 			mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
+		/* if secondary attach a private device first */
+		if (mz == NULL && rte_eal_process_type() != RTE_PROC_PRIMARY) {
+			req.t = REQ_TYPE_SHARE_DATA_PREPARE;
+			eth_dev_request_to_primary(&req);
+			mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
+		}
 		if (mz == NULL)
 			rte_panic("Cannot allocate ethdev shared data\n");
 
@@ -255,7 +264,7 @@  rte_eth_dev_allocated(const char *name)
 {
 	struct rte_eth_dev *ethdev;
 
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
 
@@ -300,7 +309,7 @@  rte_eth_dev_allocate(const char *name)
 	uint16_t port_id;
 	struct rte_eth_dev *eth_dev = NULL;
 
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	/* Synchronize port creation between primary and secondary threads. */
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -339,7 +348,7 @@  rte_eth_dev_attach_secondary(const char *name)
 	uint16_t i;
 	struct rte_eth_dev *eth_dev = NULL;
 
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	/* Synchronize port attachment to primary port creation and release. */
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -379,7 +388,7 @@  rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
 	if (eth_dev == NULL)
 		return -EINVAL;
 
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_DESTROY, NULL);
 
@@ -432,7 +441,7 @@  rte_eth_find_next_owned_by(uint16_t port_id, const uint64_t owner_id)
 int __rte_experimental
 rte_eth_dev_owner_new(uint64_t *owner_id)
 {
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
 
@@ -487,7 +496,7 @@  rte_eth_dev_owner_set(const uint16_t port_id,
 {
 	int ret;
 
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
 
@@ -504,7 +513,7 @@  rte_eth_dev_owner_unset(const uint16_t port_id, const uint64_t owner_id)
 			{.id = RTE_ETH_DEV_NO_OWNER, .name = ""};
 	int ret;
 
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
 
@@ -519,7 +528,7 @@  rte_eth_dev_owner_delete(const uint64_t owner_id)
 {
 	uint16_t port_id;
 
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
 
@@ -541,7 +550,7 @@  rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
 	int ret = 0;
 	struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
 
-	rte_eth_dev_shared_data_prepare();
+	eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);