30/04/2019 09:33, Ferruh Yigit: > On 4/29/2019 11:34 PM, Thomas Monjalon wrote: > > 30/04/2019 00:28, Ferruh Yigit: > >> On 4/29/2019 9:14 PM, Thomas Monjalon wrote: > >>> 29/04/2019 19:00, Ferruh Yigit: > >>>> On 4/26/2019 6:09 AM, Xiaolong Ye wrote: > >>>>> Since 18.11, it is suggested that driver should release all its private > >>>>> resources at the dev_close routine. So all resources previously released > >>>>> in remove routine are now released at the dev_close routine, and the > >>>>> dev_close routine will be called in driver remove routine in order to > >>>>> support removing a device without closing its ports. > >>>>> > >>>>> Above behavior changes are supported by setting RTE_ETH_DEV_CLOSE_REMOVE > >>>>> flag during probe stage. > >>>>> > >>>>> Signed-off-by: Xiaolong Ye <xiaolong...@intel.com> > >>>> > >>>> <...> > >>>> > >>>>> @@ -936,14 +940,7 @@ rte_pmd_af_xdp_remove(struct rte_vdev_device *dev) > >>>>> if (eth_dev == NULL) > >>>>> return -1; > >>>>> > >>>>> - internals = eth_dev->data->dev_private; > >>>>> - > >>>>> - rte_ring_free(internals->umem->buf_ring); > >>>>> - rte_memzone_free(internals->umem->mz); > >>>>> - rte_free(internals->umem); > >>>>> - > >>>>> - rte_eth_dev_release_port(eth_dev); > >>>> > >>>> I thinks we should keep 'rte_eth_dev_release_port()' in '.remove()' path, > >>>> the 'RTE_ETH_DEV_CLOSE_REMOVE' flag will take care of this in > >>>> 'rte_eth_dev_close()' but still needed in '.remove()' path. > >>> > >>> I don't understand your comment. > >>> Calling the close function looks the right thing to do in "remove". > >> > >> No concern on calling the 'close'. > >> My comment was to keep 'rte_eth_dev_release_port()' which this patch > >> removes. > > > > rte_eth_dev_release_port() is called in eth_dev_close(), isn't it? > > No, 'eth_dev_close()' is local 'dev_close()' ops, the one to clear driver > private resources. > I assume it is confused with 'rte_eth_dev_close()' ... > > >>>>> - > >>>>> + eth_dev_close(eth_dev);
Ah yes, I overlooked it. Why not calling rte_eth_dev_close()?