From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH] ethdev: fix device state on close Date: Wed, 16 Aug 2017 14:41:30 +0200 Message-ID: <20170816124130.GL8124@bidouze.vm.6wind.com> References: <20170816114308.165850-1-shahafs@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: thomas@monjalon.net, dev@dpdk.org, stable@dpdk.org To: Shahaf Shuler Return-path: Received: from mail-wr0-f174.google.com (mail-wr0-f174.google.com [209.85.128.174]) by dpdk.org (Postfix) with ESMTP id 6F4DD9985 for ; Wed, 16 Aug 2017 14:41:41 +0200 (CEST) Received: by mail-wr0-f174.google.com with SMTP id x43so14794935wrb.3 for ; Wed, 16 Aug 2017 05:41:41 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20170816114308.165850-1-shahafs@mellanox.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hello Shahaf, On Wed, Aug 16, 2017 at 02:43:08PM +0300, Shahaf Shuler wrote: > Currently device state moves between ATTACHED when device was > successfully probed to UNUSED when device is detached or released. > > The device state following rte_eth_dev_close() operation is inconsist, > The device is still in ATTACHED state, however it cannot be used > in any way till it will be probed again. > > Fixing it by changing the state to UNUSED. > You are right that simply closing the device leaves it in a unusable state. However it seems to be by design. Most drivers call `rte_eth_dev_release_port` when being removed, which sets the state to RTE_ETH_DEV_UNUSED. If I'm not mistaken, the API of rte_eth_dev_close is that the only available action should then be to detach the driver. At least PCI and vdev buses expects a `remove` callback from their driver, which can be called by the user (previously using specific API like `rte_eal_vdev_uninit` for example, now using `rte_eal_hotplug_remove` or `rte_eth_dev_detach` from the ether layer). So, it seems that this burden lies with the driver which should call the proper API when removing their device. Maybe Thomas will have a better insight about the scope of the `rte_eth_dev_close` function. But IMO the API is respected. After all, until the proper `dev_detach` function is called, the device is still attached, even if closed. If you disagree, there might possibly be an argument to make about either adding finer-grained device states or streamlining the API. This is however a discussion about API design and not about its implementation anymore. > Fixes: d52268a8b24b ("ethdev: expose device states") > Cc: gaetan.rivet@6wind.com > Cc: stable@dpdk.org > > Signed-off-by: Shahaf Shuler > --- > lib/librte_ether/rte_ethdev.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 0597641ee..98d9e929c 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -992,6 +992,8 @@ rte_eth_dev_close(uint8_t port_id) > dev->data->nb_tx_queues = 0; > rte_free(dev->data->tx_queues); > dev->data->tx_queues = NULL; > + > + dev->state = RTE_ETH_DEV_UNUSED; > } > > int > -- > 2.12.0 > -- Gaëtan Rivet 6WIND