On Mon, Apr 29, 2019 at 10:30:00PM +0200, Thomas Monjalon wrote:
> 29/04/2019 18:51, Ferruh Yigit:
> > On 4/18/2019 11:59 AM, Thomas Monjalon wrote:
> > > Hi all,
> > >
> > > Since DPDK 18.11, the behaviour of the close operation is changed
> > > if RTE_ETH_DEV_CLOSE_REMOVE is enabled in the driver:
> > > port is released (i.e. totally freed and data erased) on close.
> > > This new behaviour is enabled per driver for a migration period.
> > >
> > > Looking at the code, you can see these comments:
> > > /* old behaviour: only free queue arrays */
> > > RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n"
> > > "The driver %s should migrate to the new behaviour.\n",
> > > /* new behaviour: send event + reset state + free all data */
> > >
> > > You can find an advice in the commit:
> > > http://git.dpdk.org/dpdk/commit/?id=23ea57a2a
> > > "
> > > When enabling RTE_ETH_DEV_CLOSE_REMOVE,
> > > the PMD must free all its private resources for the port,
> > > in its dev_close function.
> > > It is advised to call the dev_close function in the remove function
> > > in order to support removing a device without closing its ports.
> > > "
> > >
> > > It would be great to complete this migration for the next LTS
> > > version, which will be 19.11.
> > > It means the work should be ideally finished during the summer.
> >
> > I would like to detail a little more what needs to be done, mainly for the
> > sake
> > of the discussion, please comment if something missing/wrong.
> >
> > There are two exit paths for driver:
> > 1) Hotplug remove the device
> > rte_dev_remove() OR rte_eal_hotplug_remove()
> >
> > 2) Application exit:
> > rte_eth_dev_close()
> >
> >
> > (1) can be called without ethdev stop and close functions called, so the
> > path
> > should cover those functions.
> > And in the PMD entry point in this path is .remove() function. In this
> > .remove()
> > function PMD should:
> > - stop forwarding
> > - clean PMD private resources (dev_close() ? )
>
> yes
>
> > - clean ethdev generic resources (rte_eth_dev_release_port() ? )
>
> already done in dev_close thanks to RTE_ETH_DEV_CLOSE_REMOVE
>
> > - remove device resources, which already done by remove APIs
>
> There can be some specific PMD private resources,
> not cleaned when closing a port, to clean in "remove".
>
> > (2) ethdev won't be usable after "rte_eth_dev_close()" and it should clear
> > PMD
> > private and ethdev generic resources.
> > With RTE_ETH_DEV_CLOSE_REMOVE flag 'rte_eth_dev_close()' will free generic
> > ethdev resources (rte_eth_dev_release_port()) so PMD specific
> > '.dev_close()' should:
> > - stop forwarding
> > - clean PMD private resources
>
> yes, but only port-related resources,
> not those common with other ports or features of the device.
I have a related query. Even after rte_eth_dev_close(), we have to call
rte_eal_hotplug_remove() to free up common resource if any
and remove the device from application ?
And rte_eal_hotplug_remove() would still call drivers .remove function for this.
>
> > If agreed on above, the task is:
> > - '.dev_close()'
> > - stop forwarding
> > - clean PMD private resources
> >
> > - '.remove()'
> > - call '.dev_close()'
>
> Yes, looks right.
>
> > A question is if application should call 'rte_eth_dev_stop()' explicitly
> > before
> > close or should it be part of close? For (2) it makes sense app call the
> > stop
> > explicitly, but for device remove it is not clear who to stop the
> > forwarding,
> > for this case 'dev_close()' stopping forwarding makes things easier.
>
> "stop" is a prerequisite for "close" anyway.
> In most cases, the app should manage "stop" itself to properly free
> related resources.
> The question is to know whether we return an error on "close"
> if the port is not stopped, or we stop it implicitly.
> The API says:
> * Close a stopped Ethernet device. The device cannot be restarted!
> We may explicit the behaviour if the port is not stopped.
>
>
