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() ? )
- clean ethdev generic resources (rte_eth_dev_release_port() ? )
- remove device resources, which already done by remove APIs


(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


If agreed on above, the task is:
- '.dev_close()'
  - stop forwarding
  - clean PMD private resources

- '.remove()'
  - call '.dev_close()'


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.

Reply via email to