On 09/12/2018 05:57 PM, Thomas Monjalon wrote:
10/09/2018 10:54, Andrew Rybchenko:
On 09/10/2018 11:42 AM, Thomas Monjalon wrote:
10/09/2018 10:03, Andrew Rybchenko:
On 09/08/2018 02:39 AM, Thomas Monjalon wrote:
After closing a port, it cannot be restarted.
So there is no reason to not free all associated resources.
The last step was done with rte_eth_dev_detach() which is deprecated.
Instead of removing the associated rte_device, the driver should check
if no more port (ethdev, cryptodev, etc) is still open for the device.
Then the device resources can be freed by the driver inside the
dev_close() driver callback operation.
The last ethdev freeing (dev_private and final release), which were done
by rte_eth_dev_detach(), are now done at the end of rte_eth_dev_close().
For me, it sounds more logical to kill dev_close and keep detach.
IMHO, dev_close is artificial and hardly useful. detach is a local pair
to attach.
I don't get your point.
In order to free a port, we need close + detach.
We can keep only one.
I choose close because:
1) attach/detach are deprecated
2) probe/close is a more obvious pair
3) we need the driver to free the lower level resources
Yes, I'm sorry I used bad terminology.
We have probe/remove pair for both PCI and vdev drivers and I mean
that remove is a better candidate to be kept (as a pair for probe which
allocates all resources).
OK, yes probe/remove is the pair at EAL level.
But if we want to request removal at ethdev level, rte_eth_dev_close
is the function.
Note that there is no function to request creation of an ethdev port.
Adding a new port is done only by the PMD during probing (rte_bus level).
The overall picture is too vague. May be it is simply incomplete looking
at the patch only. I understand that restructuring is required looking at
rte_eth_dev_destroy() (which is used by i40e and ixgbe drivers only) and
rte_eth_dev_pci_release() used by other PCI drivers.
Right now it looks symmetric at least on drivers level which provides
init callback to probe to do driver-specific job and uninit callback to
remove to free driver-specific resources.
I can't say if the patch is right or wrong direction since final design
is unclear.
