Re: [dpdk-dev] [PATCH v5 2/4] net/ixgbe: install ethdev hotplug handler in ixgbe

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 7/11/2018 12:51 PM, Jeff Guo wrote:
> This patch aim to enable hotplug detect in ixgbe PMD. Firstly it
> set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
> ability, and then use rte_eth_dev_event_handler_install to install
> the hotplug event handler for ethdev. When eal detect the hotplug event,
> it will call the ethdev callback to process it. If the event is hotplug
> removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
> callback to let app process the hotplug for this ethdev.
> 
> This is an example for other driver, that if any driver support hotplug
> feature could be use this way to install hotplug handler.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> v5->v4:
> no change.
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 87d2ad0..e7ae9bf 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1678,6 +1678,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
>  	rte_intr_enable(intr_handle);
>  	ixgbevf_intr_enable(eth_dev);
>  
> +	/* install the dev event handler for ethdev. */
> +	rte_eth_dev_event_handler_install(eth_dev);
> +
>  	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
>  		     eth_dev->data->port_id, pci_dev->id.vendor_id,
>  		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
> @@ -1718,6 +1721,9 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>  	rte_intr_callback_unregister(intr_handle,
>  				     ixgbevf_dev_interrupt_handler, eth_dev);
>  
> +	/* uninstall the dev event handler for ethdev. */
> +	rte_eth_dev_event_handler_uninstall(eth_dev);
> +
>  	return 0;
>  }
>  
> @@ -1801,7 +1807,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
>  static struct rte_pci_driver rte_ixgbe_pmd = {
>  	.id_table = pci_id_ixgbe_map,
>  	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> -		     RTE_PCI_DRV_IOVA_AS_VA,
> +		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,

Instead of each driver explicitly install/uninstall handler, can it be possible
to do this in a common code for drivers that report RTE_PCI_DRV_INTR_RMV support?
With this you may not need helper functions but implement them as static
functions in common code.

Also should registered_callback remove eth_dev? (after calling user registered
callbacks)
And what is the relation of RTE_ETH_DEV_REMOVED state, which is to say device
removed and remove callback?

Lastly, do you think can there be cases driver specific actions needs to be
taken, so should driver provide a callback for removal?