[dpdk-dev] [PATCH v2 2/2] net/i40e: add hot plug monitor in i40e

[Guo, Jia]

Yes, if got remove uevent might be directly return to avoid invalid i/o. but if got other uevent such as add and change, must be go done to keep the interrupt process in device. I will refine this part, thanks. 

Best regards,
Jeff Guo


-----Original Message-----
From: Wu, Jingjing 
Sent: Thursday, June 29, 2017 9:42 AM
To: Guo, Jia <jia.guo at intel.com>; Zhang, Helin <helin.zhang at intel.com>
Cc: dev at dpdk.org
Subject: RE: [PATCH v2 2/2] net/i40e: add hot plug monitor in i40e



> -----Original Message-----
> From: Guo, Jia
> Sent: Wednesday, June 28, 2017 7:07 PM
> To: Zhang, Helin <helin.zhang at intel.com>; Wu, Jingjing 
> <jingjing.wu at intel.com>
> Cc: dev at dpdk.org; Guo, Jia <jia.guo at intel.com>
> Subject: [PATCH v2 2/2] net/i40e: add hot plug monitor in i40e
> 
> From: "Guo, Jia" <jia.guo at intel.com>
> 
> This patch enable the hot plug feature in i40e, by monitoring the hot 
> plug uevent of the device. When remove event got, call the app 
> callback function to handle the detach process.
> 
> Signed-off-by: Guo, Jia <jia.guo at intel.com>
> ---
> v2->v1: remove unused part for current stage.
> ---
>  drivers/net/i40e/i40e_ethdev.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c 
> b/drivers/net/i40e/i40e_ethdev.c index 4ee1113..122187e 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1283,6 +1283,7 @@ static inline void i40e_GLQF_reg_init(struct 
> i40e_hw
> *hw)
> 
>  	/* enable uio intr after callback register */
>  	rte_intr_enable(intr_handle);
> +
>  	/*
>  	 * Add an ethertype filter to drop all flow control frames transmitted
>  	 * from VSIs. By doing so, we stop VF from sending out PAUSE or PFC 
> @@ -5832,11 +5833,28 @@ struct i40e_vsi *  {
>  	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
>  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +	struct rte_uevent event;
>  	uint32_t icr0;
> +	struct rte_pci_device *pci_dev;
> +	struct rte_intr_handle *intr_handle;
> +
> +	pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +	intr_handle = &pci_dev->intr_handle;
> 
>  	/* Disable interrupt */
>  	i40e_pf_disable_irq0(hw);
> 
> +	/* check device uevent */
> +	if (rte_uevent_get(intr_handle->uevent_fd, &event) > 0) {

You declare the rte_uevnet_get like

+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+int
+rte_uevent_get(int fd, struct rte_uevent *uevent);


But here you check if it > 0?

> +		if (event.subsystem == RTE_UEVENT_SUBSYSTEM_UIO) {
> +			if (event.action == RTE_UEVENT_REMOVE) {
> +				_rte_eth_dev_callback_process(dev,
> +					RTE_ETH_EVENT_INTR_RMV, NULL);
> +			}
> +		}
> +		goto done;

I think when the remove happen, no need to goto done, you can just return.
> +	}
> +
>  	/* read out interrupt causes */
>  	icr0 = I40E_READ_REG(hw, I40E_PFINT_ICR0);
> 
> --
> 1.8.3.1