From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matan Azrad Subject: Re: [PATCH v4 03/24] ethdev: add function to release port in local process Date: Tue, 26 Jun 2018 11:50:16 +0000 Message-ID: References: <20180607123849.14439-1-qi.z.zhang@intel.com> <20180626070832.3055-1-qi.z.zhang@intel.com> <20180626070832.3055-4-qi.z.zhang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "konstantin.ananyev@intel.com" , "dev@dpdk.org" , "bruce.richardson@intel.com" , "ferruh.yigit@intel.com" , "benjamin.h.shelton@intel.com" , "narender.vangati@intel.com" To: Qi Zhang , Thomas Monjalon , "anatoly.burakov@intel.com" Return-path: Received: from EUR04-DB3-obe.outbound.protection.outlook.com (mail-eopbgr60080.outbound.protection.outlook.com [40.107.6.80]) by dpdk.org (Postfix) with ESMTP id EEA0F1B4EE for ; Tue, 26 Jun 2018 13:50:18 +0200 (CEST) In-Reply-To: <20180626070832.3055-4-qi.z.zhang@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Qi Please see comments\questions.. From: Qi Zhang > Add driver API rte_eth_release_port_private to support the requirement th= at > an ethdev only be released on secondary process, so only local state be s= et to > unused , share data will not be reset so primary process can still use it= . >=20 > Signed-off-by: Qi Zhang > --- >=20 > lib/librte_ethdev/rte_ethdev.c | 24 +++++++++++++++++++++--- > lib/librte_ethdev/rte_ethdev_driver.h | 13 +++++++++++++ > 2 files changed, 34 insertions(+), 3 deletions(-) >=20 > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethde= v.c > index a9977df97..205b2ee33 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -359,6 +359,23 @@ rte_eth_dev_attach_secondary(const char *name) } >=20 > int > +rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev) { > + if (eth_dev =3D=3D NULL) > + return -EINVAL; > + > + _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_DESTROY, > NULL); > + > + rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock); I don't think you need the lock here because there is not shared data to re= set. > + eth_dev->state =3D RTE_ETH_DEV_UNUSED; > + > + rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock); > + > + return 0; > +} > + > +int > rte_eth_dev_release_port(struct rte_eth_dev *eth_dev) { > if (eth_dev =3D=3D NULL) > @@ -370,9 +387,10 @@ rte_eth_dev_release_port(struct rte_eth_dev > *eth_dev) >=20 > rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock); >=20 > - eth_dev->state =3D RTE_ETH_DEV_UNUSED; > - > - memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); > + if (eth_dev->state !=3D RTE_ETH_DEV_UNUSED) { Can you explain why not to release the shared data in case of locally unuse= d port? > + eth_dev->state =3D RTE_ETH_DEV_UNUSED; > + memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); > + } >=20 > rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock); >=20 > diff --git a/lib/librte_ethdev/rte_ethdev_driver.h > b/lib/librte_ethdev/rte_ethdev_driver.h > index c9c825e3f..49c27223d 100644 > --- a/lib/librte_ethdev/rte_ethdev_driver.h > +++ b/lib/librte_ethdev/rte_ethdev_driver.h > @@ -70,6 +70,19 @@ int rte_eth_dev_release_port(struct rte_eth_dev > *eth_dev); >=20 > /** > * @internal > + * Release the specified ethdev port in local process, only set to > +ethdev > + * state to unused, but not reset share data since it assume other > +process > + * is still using it, typically it is called by secondary process. > + * > + * @param eth_dev > + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure. > + * @return > + * - 0 on success, negative on error > + */ > +int rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev); > + > +/** > + * @internal > * Release device queues and clear its configuration to force the user > * application to reconfigure it. It is for internal use only. > * > -- > 2.13.6