[dpdk-dev] [RFC PATCH] librte_pmd_fm10k: Add hotplug support for fm10k

[dpdk-dev] [RFC PATCH] librte_pmd_fm10k: Add hotplug support for fm10k

[Michael Qiu]

Add hotplug support for fm10k

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 lib/librte_pmd_fm10k/fm10k_ethdev.c | 134 ++++++++++++++++++++++++++++++++++--
 1 file changed, 129 insertions(+), 5 deletions(-)

diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c b/lib/librte_pmd_fm10k/fm10k_ethdev.c
index 7f5f513..3d72607 100644
--- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
+++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
@@ -55,6 +55,10 @@
 
 static void fm10k_close_mbx_service(struct fm10k_hw *hw);
 
+static void fm10k_tx_queue_release(void *queue);
+
+static void fm10k_rx_queue_release(void *queue);
+
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)
 {
@@ -688,11 +692,35 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	for (i = 0; i < dev->data->nb_tx_queues; i++)
-		fm10k_dev_tx_queue_stop(dev, i);
+	if (dev->data->tx_queues)
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_dev_tx_queue_stop(dev, i);
 
-	for (i = 0; i < dev->data->nb_rx_queues; i++)
-		fm10k_dev_rx_queue_stop(dev, i);
+	if (dev->data->rx_queues)
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_dev_rx_queue_stop(dev, i);
+}
+
+static void
+fm10k_dev_queue_release(__rte_unused struct rte_eth_dev *dev)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dev->data->tx_queues) {
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_tx_queue_release(dev->data->tx_queues[i]);
+		rte_free(dev->data->tx_queues);
+		dev->data->tx_queues = NULL;
+	}
+
+	if (dev->data->rx_queues) {
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_rx_queue_release(dev->data->rx_queues[i]);
+		rte_free(dev->data->rx_queues);
+		dev->data->rx_queues = NULL;
+	}
 }
 
 static void
@@ -705,6 +733,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
+	fm10k_dev_queue_release(dev);
 	fm10k_stop_hw(hw);
 }
 
@@ -1406,6 +1435,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)
 }
 
 static void
+fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
+
+	/* Disable misc causes */
+	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
+				FM10K_EIMR_DISABLE(THI_FAULT) |
+				FM10K_EIMR_DISABLE(FUM_FAULT) |
+				FM10K_EIMR_DISABLE(MAILBOX) |
+				FM10K_EIMR_DISABLE(SWITCHREADY) |
+				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
+				FM10K_EIMR_DISABLE(SRAMERROR) |
+				FM10K_EIMR_DISABLE(VFLR));
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
+static void
 fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 {
 	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -1423,6 +1482,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 	FM10K_WRITE_FLUSH(hw);
 }
 
+static void
+fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	/* Only INT 0 available, other 15 are reserved. */
+	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
 static int
 fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
 {
@@ -1868,6 +1943,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+eth_fm10k_dev_uninit(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	PMD_INIT_FUNC_TRACE();
+
+	/* only uninitialize in the primary process */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	/* safe to close dev here */
+	fm10k_dev_close(dev);
+
+	dev->dev_ops = NULL;
+	dev->rx_pkt_burst = NULL;
+	dev->tx_pkt_burst = NULL;
+
+	/* disable uio/vfio intr */
+	rte_intr_disable(&(dev->pci_dev->intr_handle));
+
+	/*PF/VF has different interrupt handling mechanism */
+	if (hw->mac.type == fm10k_mac_pf) {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_pf(dev);
+
+		/* unregister callback func to eal lib */
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_pf, (void *)dev);
+	} else {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_vf(dev);
+
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_vf, (void *)dev);
+	}
+
+	/* free mac memory */
+	if (dev->data->mac_addrs) {
+		rte_free(dev->data->mac_addrs);
+		dev->data->mac_addrs = NULL;
+	}
+
+	memset(hw, 0, sizeof(*hw));
+
+	return 0;
+}
+
 /*
  * The set of PCI devices this driver supports. This driver will enable both PF
  * and SRIOV-VF devices.
@@ -1883,9 +2006,10 @@ static struct eth_driver rte_pmd_fm10k = {
 	{
 		.name = "rte_pmd_fm10k",
 		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 	},
 	.eth_dev_init = eth_fm10k_dev_init,
+	.eth_dev_uninit = eth_fm10k_dev_uninit,
 	.dev_private_size = sizeof(struct fm10k_adapter),
 };
 
-- 
1.9.3

Re: [dpdk-dev] [RFC PATCH] librte_pmd_fm10k: Add hotplug support for fm10k

[Iremonger, Bernard]

-----Original Message-----
From: Qiu, Michael 
Sent: Sunday, May 31, 2015 3:37 PM
To: dev@dpdk.org
Cc: Chen, Jing D; Iremonger, Bernard; He, Shaopeng; Qiu, Michael
Subject: [RFC PATCH] librte_pmd_fm10k: Add hotplug support for fm10k

Add hotplug support for fm10k

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 lib/librte_pmd_fm10k/fm10k_ethdev.c | 134 ++++++++++++++++++++++++++++++++++--
 1 file changed, 129 insertions(+), 5 deletions(-)

Hi Michael,

The location of the fm10k  pmd has changed to  drivers/net/fm10k.
lib/librte_pmd_fm10k  no longer exists.
You will need to rebase to use the new directory structure.

Regards,

Bernard.
 

diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c b/lib/librte_pmd_fm10k/fm10k_ethdev.c
index 7f5f513..3d72607 100644
--- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
+++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
@@ -55,6 +55,10 @@
 
 static void fm10k_close_mbx_service(struct fm10k_hw *hw);
 
+static void fm10k_tx_queue_release(void *queue);
+
+static void fm10k_rx_queue_release(void *queue);
+
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)  { @@ -688,11 +692,35 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	for (i = 0; i < dev->data->nb_tx_queues; i++)
-		fm10k_dev_tx_queue_stop(dev, i);
+	if (dev->data->tx_queues)
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_dev_tx_queue_stop(dev, i);
 
-	for (i = 0; i < dev->data->nb_rx_queues; i++)
-		fm10k_dev_rx_queue_stop(dev, i);
+	if (dev->data->rx_queues)
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_dev_rx_queue_stop(dev, i);
+}
+
+static void
+fm10k_dev_queue_release(__rte_unused struct rte_eth_dev *dev) {
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dev->data->tx_queues) {
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_tx_queue_release(dev->data->tx_queues[i]);
+		rte_free(dev->data->tx_queues);
+		dev->data->tx_queues = NULL;
+	}
+
+	if (dev->data->rx_queues) {
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_rx_queue_release(dev->data->rx_queues[i]);
+		rte_free(dev->data->rx_queues);
+		dev->data->rx_queues = NULL;
+	}
 }
 
 static void
@@ -705,6 +733,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
+	fm10k_dev_queue_release(dev);
 	fm10k_stop_hw(hw);
 }
 
@@ -1406,6 +1435,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)  }
 
 static void
+fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev) {
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
+
+	/* Disable misc causes */
+	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
+				FM10K_EIMR_DISABLE(THI_FAULT) |
+				FM10K_EIMR_DISABLE(FUM_FAULT) |
+				FM10K_EIMR_DISABLE(MAILBOX) |
+				FM10K_EIMR_DISABLE(SWITCHREADY) |
+				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
+				FM10K_EIMR_DISABLE(SRAMERROR) |
+				FM10K_EIMR_DISABLE(VFLR));
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
+static void
 fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)  {
 	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -1423,6 +1482,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 	FM10K_WRITE_FLUSH(hw);
 }
 
+static void
+fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev) {
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	/* Only INT 0 available, other 15 are reserved. */
+	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
 static int
 fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)  { @@ -1868,6 +1943,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+eth_fm10k_dev_uninit(struct rte_eth_dev *dev) {
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	PMD_INIT_FUNC_TRACE();
+
+	/* only uninitialize in the primary process */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	/* safe to close dev here */
+	fm10k_dev_close(dev);
+
+	dev->dev_ops = NULL;
+	dev->rx_pkt_burst = NULL;
+	dev->tx_pkt_burst = NULL;
+
+	/* disable uio/vfio intr */
+	rte_intr_disable(&(dev->pci_dev->intr_handle));
+
+	/*PF/VF has different interrupt handling mechanism */
+	if (hw->mac.type == fm10k_mac_pf) {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_pf(dev);
+
+		/* unregister callback func to eal lib */
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_pf, (void *)dev);
+	} else {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_vf(dev);
+
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_vf, (void *)dev);
+	}
+
+	/* free mac memory */
+	if (dev->data->mac_addrs) {
+		rte_free(dev->data->mac_addrs);
+		dev->data->mac_addrs = NULL;
+	}
+
+	memset(hw, 0, sizeof(*hw));
+
+	return 0;
+}
+
 /*
  * The set of PCI devices this driver supports. This driver will enable both PF
  * and SRIOV-VF devices.
@@ -1883,9 +2006,10 @@ static struct eth_driver rte_pmd_fm10k = {
 	{
 		.name = "rte_pmd_fm10k",
 		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 	},
 	.eth_dev_init = eth_fm10k_dev_init,
+	.eth_dev_uninit = eth_fm10k_dev_uninit,
 	.dev_private_size = sizeof(struct fm10k_adapter),  };
 
--
1.9.3

Re: [dpdk-dev] [RFC PATCH] librte_pmd_fm10k: Add hotplug support for fm10k

[Qiu, Michael]

On 6/2/2015 4:26 PM, Iremonger, Bernard wrote:
>
> -----Original Message-----
> From: Qiu, Michael 
> Sent: Sunday, May 31, 2015 3:37 PM
> To: dev@dpdk.org
> Cc: Chen, Jing D; Iremonger, Bernard; He, Shaopeng; Qiu, Michael
> Subject: [RFC PATCH] librte_pmd_fm10k: Add hotplug support for fm10k
>
> Add hotplug support for fm10k
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  lib/librte_pmd_fm10k/fm10k_ethdev.c | 134 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 129 insertions(+), 5 deletions(-)
>
> Hi Michael,
>
> The location of the fm10k  pmd has changed to  drivers/net/fm10k.
> lib/librte_pmd_fm10k  no longer exists.
> You will need to rebase to use the new directory structure.

OK, got it, I will rebase it,

Thanks,
Michael
> Regards,
>
> Bernard.
>  
>
> diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> index 7f5f513..3d72607 100644
> --- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> +++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> @@ -55,6 +55,10 @@
>  
>  static void fm10k_close_mbx_service(struct fm10k_hw *hw);
>  
> +static void fm10k_tx_queue_release(void *queue);
> +
> +static void fm10k_rx_queue_release(void *queue);
> +
>  static void
>  fm10k_mbx_initlock(struct fm10k_hw *hw)  { @@ -688,11 +692,35 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
>  
>  	PMD_INIT_FUNC_TRACE();
>  
> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
> -		fm10k_dev_tx_queue_stop(dev, i);
> +	if (dev->data->tx_queues)
> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> +			fm10k_dev_tx_queue_stop(dev, i);
>  
> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
> -		fm10k_dev_rx_queue_stop(dev, i);
> +	if (dev->data->rx_queues)
> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> +			fm10k_dev_rx_queue_stop(dev, i);
> +}
> +
> +static void
> +fm10k_dev_queue_release(__rte_unused struct rte_eth_dev *dev) {
> +	int i;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (dev->data->tx_queues) {
> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> +			fm10k_tx_queue_release(dev->data->tx_queues[i]);
> +		rte_free(dev->data->tx_queues);
> +		dev->data->tx_queues = NULL;
> +	}
> +
> +	if (dev->data->rx_queues) {
> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> +			fm10k_rx_queue_release(dev->data->rx_queues[i]);
> +		rte_free(dev->data->rx_queues);
> +		dev->data->rx_queues = NULL;
> +	}
>  }
>  
>  static void
> @@ -705,6 +733,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
>  	/* Stop mailbox service first */
>  	fm10k_close_mbx_service(hw);
>  	fm10k_dev_stop(dev);
> +	fm10k_dev_queue_release(dev);
>  	fm10k_stop_hw(hw);
>  }
>  
> @@ -1406,6 +1435,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)  }
>  
>  static void
> +fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev) {
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t int_map = FM10K_INT_MAP_DISABLE;
> +
> +	int_map |= 0;
> +
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
> +
> +	/* Disable misc causes */
> +	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
> +				FM10K_EIMR_DISABLE(THI_FAULT) |
> +				FM10K_EIMR_DISABLE(FUM_FAULT) |
> +				FM10K_EIMR_DISABLE(MAILBOX) |
> +				FM10K_EIMR_DISABLE(SWITCHREADY) |
> +				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
> +				FM10K_EIMR_DISABLE(SRAMERROR) |
> +				FM10K_EIMR_DISABLE(VFLR));
> +
> +	/* Disable ITR 0 */
> +	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
> +	FM10K_WRITE_FLUSH(hw);
> +}
> +
> +static void
>  fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)  {
>  	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> @@ -1423,6 +1482,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
>  	FM10K_WRITE_FLUSH(hw);
>  }
>  
> +static void
> +fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev) {
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t int_map = FM10K_INT_MAP_DISABLE;
> +
> +	int_map |= 0;
> +
> +	/* Only INT 0 available, other 15 are reserved. */
> +	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
> +
> +	/* Disable ITR 0 */
> +	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
> +	FM10K_WRITE_FLUSH(hw);
> +}
> +
>  static int
>  fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)  { @@ -1868,6 +1943,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  	return 0;
>  }
>  
> +static int
> +eth_fm10k_dev_uninit(struct rte_eth_dev *dev) {
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	/* only uninitialize in the primary process */
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return 0;
> +
> +	/* safe to close dev here */
> +	fm10k_dev_close(dev);
> +
> +	dev->dev_ops = NULL;
> +	dev->rx_pkt_burst = NULL;
> +	dev->tx_pkt_burst = NULL;
> +
> +	/* disable uio/vfio intr */
> +	rte_intr_disable(&(dev->pci_dev->intr_handle));
> +
> +	/*PF/VF has different interrupt handling mechanism */
> +	if (hw->mac.type == fm10k_mac_pf) {
> +		/* disable interrupt */
> +		fm10k_dev_disable_intr_pf(dev);
> +
> +		/* unregister callback func to eal lib */
> +		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
> +			fm10k_dev_interrupt_handler_pf, (void *)dev);
> +	} else {
> +		/* disable interrupt */
> +		fm10k_dev_disable_intr_vf(dev);
> +
> +		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
> +			fm10k_dev_interrupt_handler_vf, (void *)dev);
> +	}
> +
> +	/* free mac memory */
> +	if (dev->data->mac_addrs) {
> +		rte_free(dev->data->mac_addrs);
> +		dev->data->mac_addrs = NULL;
> +	}
> +
> +	memset(hw, 0, sizeof(*hw));
> +
> +	return 0;
> +}
> +
>  /*
>   * The set of PCI devices this driver supports. This driver will enable both PF
>   * and SRIOV-VF devices.
> @@ -1883,9 +2006,10 @@ static struct eth_driver rte_pmd_fm10k = {
>  	{
>  		.name = "rte_pmd_fm10k",
>  		.id_table = pci_id_fm10k_map,
> -		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
>  	},
>  	.eth_dev_init = eth_fm10k_dev_init,
> +	.eth_dev_uninit = eth_fm10k_dev_uninit,
>  	.dev_private_size = sizeof(struct fm10k_adapter),  };
>  
> --
> 1.9.3
>
>

[dpdk-dev] [PATCH 0/2 v2] Enable hotplug support for fm10k

[Michael Qiu]

Hotplug feature is supported in EAL, this patch set is to enable
this feature in driver side.

Michael Qiu (2):
  fm10k: Free queues when close port
  fm10k: Add hotplug support for fm10k

 drivers/net/fm10k/fm10k_ethdev.c | 134 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 129 insertions(+), 5 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH 1/2] fm10k: Free queues when close port

[Michael Qiu]

When close a port, lots of memory should be released,
such as software rings, queues, etc.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 87852ed..e310698 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -52,6 +52,10 @@
 
 static void fm10k_close_mbx_service(struct fm10k_hw *hw);
 
+static void fm10k_tx_queue_release(void *queue);
+
+static void fm10k_rx_queue_release(void *queue);
+
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)
 {
@@ -665,11 +669,35 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	for (i = 0; i < dev->data->nb_tx_queues; i++)
-		fm10k_dev_tx_queue_stop(dev, i);
+	if (dev->data->tx_queues)
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_dev_tx_queue_stop(dev, i);
+
+	if (dev->data->rx_queues)
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_dev_rx_queue_stop(dev, i);
+}
 
-	for (i = 0; i < dev->data->nb_rx_queues; i++)
-		fm10k_dev_rx_queue_stop(dev, i);
+static void
+fm10k_dev_queue_release(__rte_unused struct rte_eth_dev *dev)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dev->data->tx_queues) {
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_tx_queue_release(dev->data->tx_queues[i]);
+		rte_free(dev->data->tx_queues);
+		dev->data->tx_queues = NULL;
+	}
+
+	if (dev->data->rx_queues) {
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_rx_queue_release(dev->data->rx_queues[i]);
+		rte_free(dev->data->rx_queues);
+		dev->data->rx_queues = NULL;
+	}
 }
 
 static void
@@ -682,6 +710,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
+	fm10k_dev_queue_release(dev);
 	fm10k_stop_hw(hw);
 }
 
-- 
1.9.3

Re: [dpdk-dev] [PATCH 1/2] fm10k: Free queues when close port

[Chen, Jing D]

Hi,

> -----Original Message-----
> From: Qiu, Michael
> Sent: Wednesday, June 10, 2015 8:22 PM
> To: dev@dpdk.org
> Cc: Chen, Jing D; Iremonger, Bernard; He, Shaopeng; Qiu, Michael
> Subject: [PATCH 1/2] fm10k: Free queues when close port
> 
> When close a port, lots of memory should be released,
> such as software rings, queues, etc.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c | 37
> +++++++++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 87852ed..e310698 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -52,6 +52,10 @@
> 
>  static void fm10k_close_mbx_service(struct fm10k_hw *hw);
> 
> +static void fm10k_tx_queue_release(void *queue);
> +
> +static void fm10k_rx_queue_release(void *queue);
> +
>  static void
>  fm10k_mbx_initlock(struct fm10k_hw *hw)
>  {
> @@ -665,11 +669,35 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
> -		fm10k_dev_tx_queue_stop(dev, i);
> +	if (dev->data->tx_queues)
> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> +			fm10k_dev_tx_queue_stop(dev, i);
> +
> +	if (dev->data->rx_queues)
> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> +			fm10k_dev_rx_queue_stop(dev, i);
> +}
> 
> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
> -		fm10k_dev_rx_queue_stop(dev, i);
> +static void
> +fm10k_dev_queue_release(__rte_unused struct rte_eth_dev *dev)

Remove __rte_unused ?

> +{
> +	int i;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (dev->data->tx_queues) {
> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> +			fm10k_tx_queue_release(dev->data->tx_queues[i]);
> +		rte_free(dev->data->tx_queues);
> +		dev->data->tx_queues = NULL;
> +	}
> +
> +	if (dev->data->rx_queues) {
> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> +			fm10k_rx_queue_release(dev->data->rx_queues[i]);
> +		rte_free(dev->data->rx_queues);
> +		dev->data->rx_queues = NULL;
> +	}
>  }
> 
>  static void
> @@ -682,6 +710,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
>  	/* Stop mailbox service first */
>  	fm10k_close_mbx_service(hw);
>  	fm10k_dev_stop(dev);
> +	fm10k_dev_queue_release(dev);
>  	fm10k_stop_hw(hw);
>  }
> 
> --
> 1.9.3

[dpdk-dev] [PATCH 1/2 v2] fm10k: Free queues when close port

[Michael Qiu]

When close a port, lots of memory should be released,
such as software rings, queues, etc.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
change log:
 v2 --> v1: remove __rte_unused flag 

 drivers/net/fm10k/fm10k_ethdev.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 87852ed..e310698 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -52,6 +52,10 @@
 
 static void fm10k_close_mbx_service(struct fm10k_hw *hw);
 
+static void fm10k_tx_queue_release(void *queue);
+
+static void fm10k_rx_queue_release(void *queue);
+
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)
 {
@@ -665,11 +669,35 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	for (i = 0; i < dev->data->nb_tx_queues; i++)
-		fm10k_dev_tx_queue_stop(dev, i);
+	if (dev->data->tx_queues)
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_dev_tx_queue_stop(dev, i);
+
+	if (dev->data->rx_queues)
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_dev_rx_queue_stop(dev, i);
+}
 
-	for (i = 0; i < dev->data->nb_rx_queues; i++)
-		fm10k_dev_rx_queue_stop(dev, i);
+static void
+fm10k_dev_queue_release(struct rte_eth_dev *dev)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dev->data->tx_queues) {
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_tx_queue_release(dev->data->tx_queues[i]);
+		rte_free(dev->data->tx_queues);
+		dev->data->tx_queues = NULL;
+	}
+
+	if (dev->data->rx_queues) {
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_rx_queue_release(dev->data->rx_queues[i]);
+		rte_free(dev->data->rx_queues);
+		dev->data->rx_queues = NULL;
+	}
 }
 
 static void
@@ -682,6 +710,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
+	fm10k_dev_queue_release(dev);
 	fm10k_stop_hw(hw);
 }
 
-- 
1.9.3

Re: [dpdk-dev] [PATCH 1/2 v2] fm10k: Free queues when close port

[Chen, Jing D]

> -----Original Message-----
> From: Qiu, Michael
> Sent: Wednesday, June 17, 2015 3:59 PM
> To: dev@dpdk.org
> Cc: Chen, Jing D; Iremonger, Bernard; He, Shaopeng; Qiu, Michael
> Subject: [PATCH 1/2 v2] fm10k: Free queues when close port
> 
> When close a port, lots of memory should be released,
> such as software rings, queues, etc.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>

Acked-by : Jing Chen <jing.d.chen@intel.com>

Re: [dpdk-dev] [PATCH 1/2 v2] fm10k: Free queues when close port

[Iremonger, Bernard]

> -----Original Message-----
> From: Qiu, Michael
> Sent: Wednesday, June 17, 2015 8:59 AM
> To: dev@dpdk.org
> Cc: Chen, Jing D; Iremonger, Bernard; He, Shaopeng; Qiu, Michael
> Subject: [PATCH 1/2 v2] fm10k: Free queues when close port
> 
> When close a port, lots of memory should be released, such as software
> rings, queues, etc.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>

Hi Michael,

There are two comments inline.


> ---
> change log:
>  v2 --> v1: remove __rte_unused flag
> 
>  drivers/net/fm10k/fm10k_ethdev.c | 37
> +++++++++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 87852ed..e310698 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -52,6 +52,10 @@
> 
>  static void fm10k_close_mbx_service(struct fm10k_hw *hw);
> 
> +static void fm10k_tx_queue_release(void *queue);
> +
> +static void fm10k_rx_queue_release(void *queue);
> +
>  static void
>  fm10k_mbx_initlock(struct fm10k_hw *hw)  { @@ -665,11 +669,35 @@
> fm10k_dev_stop(struct rte_eth_dev *dev)
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
> -		fm10k_dev_tx_queue_stop(dev, i);
> +	if (dev->data->tx_queues)
> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> +			fm10k_dev_tx_queue_stop(dev, i);
> +
> +	if (dev->data->rx_queues)
> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> +			fm10k_dev_rx_queue_stop(dev, i);
> +}
> 
> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
> -		fm10k_dev_rx_queue_stop(dev, i);
> +static void
> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
> +	int i;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (dev->data->tx_queues) {
> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> +			fm10k_tx_queue_release(dev->data-
> >tx_queues[i]);
> +		rte_free(dev->data->tx_queues);
> +		dev->data->tx_queues = NULL;

The following line should be added here:
dev->data->nb_tx_queues = 0;

> +	}
> +
> +	if (dev->data->rx_queues) {
> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> +			fm10k_rx_queue_release(dev->data-
> >rx_queues[i]);
> +		rte_free(dev->data->rx_queues);
> +		dev->data->rx_queues = NULL;

The following line should be added here:
dev->data->nb_rx_queues = 0;

> +	}
>  }
> 
>  static void
> @@ -682,6 +710,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
>  	/* Stop mailbox service first */
>  	fm10k_close_mbx_service(hw);
>  	fm10k_dev_stop(dev);
> +	fm10k_dev_queue_release(dev);
>  	fm10k_stop_hw(hw);
>  }
> 
> --
> 1.9.3

Re: [dpdk-dev] [PATCH 1/2 v2] fm10k: Free queues when close port

[Qiu, Michael]

On 6/19/2015 12:29 AM, Iremonger, Bernard wrote:
>> -----Original Message-----
>> From: Qiu, Michael
>> Sent: Wednesday, June 17, 2015 8:59 AM
>> To: dev@dpdk.org
>> Cc: Chen, Jing D; Iremonger, Bernard; He, Shaopeng; Qiu, Michael
>> Subject: [PATCH 1/2 v2] fm10k: Free queues when close port
>>
>> When close a port, lots of memory should be released, such as software
>> rings, queues, etc.
>>
>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> Hi Michael,
>
> There are two comments inline.
>
>
>> ---
>> change log:
>>  v2 --> v1: remove __rte_unused flag
>>
>>  drivers/net/fm10k/fm10k_ethdev.c | 37
>> +++++++++++++++++++++++++++++++++----
>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
>> b/drivers/net/fm10k/fm10k_ethdev.c
>> index 87852ed..e310698 100644
>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>> @@ -52,6 +52,10 @@
>>
>>  static void fm10k_close_mbx_service(struct fm10k_hw *hw);
>>
>> +static void fm10k_tx_queue_release(void *queue);
>> +
>> +static void fm10k_rx_queue_release(void *queue);
>> +
>>  static void
>>  fm10k_mbx_initlock(struct fm10k_hw *hw)  { @@ -665,11 +669,35 @@
>> fm10k_dev_stop(struct rte_eth_dev *dev)
>>
>>  	PMD_INIT_FUNC_TRACE();
>>
>> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
>> -		fm10k_dev_tx_queue_stop(dev, i);
>> +	if (dev->data->tx_queues)
>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>> +			fm10k_dev_tx_queue_stop(dev, i);
>> +
>> +	if (dev->data->rx_queues)
>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>> +			fm10k_dev_rx_queue_stop(dev, i);
>> +}
>>
>> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
>> -		fm10k_dev_rx_queue_stop(dev, i);
>> +static void
>> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
>> +	int i;
>> +
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	if (dev->data->tx_queues) {
>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>> +			fm10k_tx_queue_release(dev->data-
>>> tx_queues[i]);
>> +		rte_free(dev->data->tx_queues);
>> +		dev->data->tx_queues = NULL;
> The following line should be added here:
> dev->data->nb_tx_queues = 0;

Hi, Bernard

dev->data->nb_tx_queues will be init as 0 in rte_eth_dev_tx_queue_config()

And it has no issue here, I think. But I can add and send out new version

Thanks,
Michael

>
>> +	}
>> +
>> +	if (dev->data->rx_queues) {
>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>> +			fm10k_rx_queue_release(dev->data-
>>> rx_queues[i]);
>> +		rte_free(dev->data->rx_queues);
>> +		dev->data->rx_queues = NULL;
> The following line should be added here:
> dev->data->nb_rx_queues = 0;
>
>> +	}
>>  }
>>
>>  static void
>> @@ -682,6 +710,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
>>  	/* Stop mailbox service first */
>>  	fm10k_close_mbx_service(hw);
>>  	fm10k_dev_stop(dev);
>> +	fm10k_dev_queue_release(dev);
>>  	fm10k_stop_hw(hw);
>>  }
>>
>> --
>> 1.9.3
>

[dpdk-dev] [PATCH 2/2 v2] fm10k: Add hotplug support for fm10k

[Michael Qiu]

Add hotplug support for fm10k.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 97 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index e310698..0d3eaf1 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1412,6 +1412,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)
 }
 
 static void
+fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
+
+	/* Disable misc causes */
+	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
+				FM10K_EIMR_DISABLE(THI_FAULT) |
+				FM10K_EIMR_DISABLE(FUM_FAULT) |
+				FM10K_EIMR_DISABLE(MAILBOX) |
+				FM10K_EIMR_DISABLE(SWITCHREADY) |
+				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
+				FM10K_EIMR_DISABLE(SRAMERROR) |
+				FM10K_EIMR_DISABLE(VFLR));
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
+static void
 fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 {
 	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -1429,6 +1459,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 	FM10K_WRITE_FLUSH(hw);
 }
 
+static void
+fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	/* Only INT 0 available, other 15 are reserved. */
+	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
 static int
 fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
 {
@@ -1858,6 +1904,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+eth_fm10k_dev_uninit(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	PMD_INIT_FUNC_TRACE();
+
+	/* only uninitialize in the primary process */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	/* safe to close dev here */
+	fm10k_dev_close(dev);
+
+	dev->dev_ops = NULL;
+	dev->rx_pkt_burst = NULL;
+	dev->tx_pkt_burst = NULL;
+
+	/* disable uio/vfio intr */
+	rte_intr_disable(&(dev->pci_dev->intr_handle));
+
+	/*PF/VF has different interrupt handling mechanism */
+	if (hw->mac.type == fm10k_mac_pf) {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_pf(dev);
+
+		/* unregister callback func to eal lib */
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_pf, (void *)dev);
+	} else {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_vf(dev);
+
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_vf, (void *)dev);
+	}
+
+	/* free mac memory */
+	if (dev->data->mac_addrs) {
+		rte_free(dev->data->mac_addrs);
+		dev->data->mac_addrs = NULL;
+	}
+
+	memset(hw, 0, sizeof(*hw));
+
+	return 0;
+}
+
 /*
  * The set of PCI devices this driver supports. This driver will enable both PF
  * and SRIOV-VF devices.
@@ -1873,9 +1967,10 @@ static struct eth_driver rte_pmd_fm10k = {
 	{
 		.name = "rte_pmd_fm10k",
 		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 	},
 	.eth_dev_init = eth_fm10k_dev_init,
+	.eth_dev_uninit = eth_fm10k_dev_uninit,
 	.dev_private_size = sizeof(struct fm10k_adapter),
 };
 
-- 
1.9.3

Re: [dpdk-dev] [PATCH 2/2 v2] fm10k: Add hotplug support for fm10k

[Iremonger, Bernard]

> -----Original Message-----
> From: Qiu, Michael
> Sent: Wednesday, June 17, 2015 9:45 AM
> To: dev@dpdk.org
> Cc: Chen, Jing D; Iremonger, Bernard; He, Shaopeng; Qiu, Michael
> Subject: [PATCH 2/2 v2] fm10k: Add hotplug support for fm10k
> 
> Add hotplug support for fm10k.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>

Hi Michael,

There is one comment inline.

> ---
>  drivers/net/fm10k/fm10k_ethdev.c | 97
> +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index e310698..0d3eaf1 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -1412,6 +1412,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev
> *dev)  }
> 
>  static void
> +fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev) {
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +	uint32_t int_map = FM10K_INT_MAP_DISABLE;
> +
> +	int_map |= 0;
> +
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox),
> int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault),
> int_map);
> +	FM10K_WRITE_REG(hw,
> FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent),
> int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM),
> int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR),
> int_map);
> +
> +	/* Disable misc causes */
> +	FM10K_WRITE_REG(hw, FM10K_EIMR,
> FM10K_EIMR_DISABLE(PCA_FAULT) |
> +				FM10K_EIMR_DISABLE(THI_FAULT) |
> +				FM10K_EIMR_DISABLE(FUM_FAULT) |
> +				FM10K_EIMR_DISABLE(MAILBOX) |
> +				FM10K_EIMR_DISABLE(SWITCHREADY) |
> +				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
> +				FM10K_EIMR_DISABLE(SRAMERROR) |
> +				FM10K_EIMR_DISABLE(VFLR));
> +
> +	/* Disable ITR 0 */
> +	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
> +	FM10K_WRITE_FLUSH(hw);
> +}
> +
> +static void
>  fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)  {
>  	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> @@ -1429,6 +1459,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev
> *dev)
>  	FM10K_WRITE_FLUSH(hw);
>  }
> 
> +static void
> +fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev) {
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +	uint32_t int_map = FM10K_INT_MAP_DISABLE;
> +
> +	int_map |= 0;
> +
> +	/* Only INT 0 available, other 15 are reserved. */
> +	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
> +
> +	/* Disable ITR 0 */
> +	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
> +	FM10K_WRITE_FLUSH(hw);
> +}
> +
>  static int
>  fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)  { @@ -
> 1858,6 +1904,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  	return 0;
>  }
> 
> +static int
> +eth_fm10k_dev_uninit(struct rte_eth_dev *dev) {
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	/* only uninitialize in the primary process */
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return 0;
> +
> +	/* safe to close dev here */

Should a flag be added so as not to call fm10k_dev_close() if it has been called already ?

Regards,

Bernard.

> +	fm10k_dev_close(dev);
> +
> +	dev->dev_ops = NULL;
> +	dev->rx_pkt_burst = NULL;
> +	dev->tx_pkt_burst = NULL;
> +
> +	/* disable uio/vfio intr */
> +	rte_intr_disable(&(dev->pci_dev->intr_handle));
> +
> +	/*PF/VF has different interrupt handling mechanism */
> +	if (hw->mac.type == fm10k_mac_pf) {
> +		/* disable interrupt */
> +		fm10k_dev_disable_intr_pf(dev);
> +
> +		/* unregister callback func to eal lib */
> +		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
> +			fm10k_dev_interrupt_handler_pf, (void *)dev);
> +	} else {
> +		/* disable interrupt */
> +		fm10k_dev_disable_intr_vf(dev);
> +
> +		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
> +			fm10k_dev_interrupt_handler_vf, (void *)dev);
> +	}
> +
> +	/* free mac memory */
> +	if (dev->data->mac_addrs) {
> +		rte_free(dev->data->mac_addrs);
> +		dev->data->mac_addrs = NULL;
> +	}
> +
> +	memset(hw, 0, sizeof(*hw));
> +
> +	return 0;
> +}
> +
>  /*
>   * The set of PCI devices this driver supports. This driver will enable both PF
>   * and SRIOV-VF devices.
> @@ -1873,9 +1967,10 @@ static struct eth_driver rte_pmd_fm10k = {
>  	{
>  		.name = "rte_pmd_fm10k",
>  		.id_table = pci_id_fm10k_map,
> -		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> RTE_PCI_DRV_DETACHABLE,
>  	},
>  	.eth_dev_init = eth_fm10k_dev_init,
> +	.eth_dev_uninit = eth_fm10k_dev_uninit,
>  	.dev_private_size = sizeof(struct fm10k_adapter),  };
> 
> --
> 1.9.3

Re: [dpdk-dev] [PATCH 2/2 v2] fm10k: Add hotplug support for fm10k

[Qiu, Michael]

On 6/19/2015 12:42 AM, Iremonger, Bernard wrote:
>> -----Original Message-----
>> From: Qiu, Michael
>> Sent: Wednesday, June 17, 2015 9:45 AM
>> To: dev@dpdk.org
>> Cc: Chen, Jing D; Iremonger, Bernard; He, Shaopeng; Qiu, Michael
>> Subject: [PATCH 2/2 v2] fm10k: Add hotplug support for fm10k
>>
>> Add hotplug support for fm10k.
>>
>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> Hi Michael,
>
> There is one comment inline.
>
>> ---
>>  drivers/net/fm10k/fm10k_ethdev.c | 97
>> +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
>> b/drivers/net/fm10k/fm10k_ethdev.c
>> index e310698..0d3eaf1 100644
>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>> @@ -1412,6 +1412,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev
>> *dev)  }
>>
>>  static void
>> +fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev) {
>> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
>>> dev_private);
>> +	uint32_t int_map = FM10K_INT_MAP_DISABLE;
>> +
>> +	int_map |= 0;
>> +
>> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox),
>> int_map);
>> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault),
>> int_map);
>> +	FM10K_WRITE_REG(hw,
>> FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
>> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent),
>> int_map);
>> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM),
>> int_map);
>> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR),
>> int_map);
>> +
>> +	/* Disable misc causes */
>> +	FM10K_WRITE_REG(hw, FM10K_EIMR,
>> FM10K_EIMR_DISABLE(PCA_FAULT) |
>> +				FM10K_EIMR_DISABLE(THI_FAULT) |
>> +				FM10K_EIMR_DISABLE(FUM_FAULT) |
>> +				FM10K_EIMR_DISABLE(MAILBOX) |
>> +				FM10K_EIMR_DISABLE(SWITCHREADY) |
>> +				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
>> +				FM10K_EIMR_DISABLE(SRAMERROR) |
>> +				FM10K_EIMR_DISABLE(VFLR));
>> +
>> +	/* Disable ITR 0 */
>> +	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
>> +	FM10K_WRITE_FLUSH(hw);
>> +}
>> +
>> +static void
>>  fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)  {
>>  	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
>>> dev_private);
>> @@ -1429,6 +1459,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev
>> *dev)
>>  	FM10K_WRITE_FLUSH(hw);
>>  }
>>
>> +static void
>> +fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev) {
>> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
>>> dev_private);
>> +	uint32_t int_map = FM10K_INT_MAP_DISABLE;
>> +
>> +	int_map |= 0;
>> +
>> +	/* Only INT 0 available, other 15 are reserved. */
>> +	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
>> +
>> +	/* Disable ITR 0 */
>> +	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
>> +	FM10K_WRITE_FLUSH(hw);
>> +}
>> +
>>  static int
>>  fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)  { @@ -
>> 1858,6 +1904,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>>  	return 0;
>>  }
>>
>> +static int
>> +eth_fm10k_dev_uninit(struct rte_eth_dev *dev) {
>> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
>>> dev_private);
>> +
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	/* only uninitialize in the primary process */
>> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>> +		return 0;
>> +
>> +	/* safe to close dev here */
> Should a flag be added so as not to call fm10k_dev_close() if it has been called already ?

It's OK for fm10k, for i40e for 82599, there has a flag in share code to
show whether has been closed, but for fm10k there doesn't.

Also function fm10k_dev_close() would check all pointer and resource,
that's why need first patch in fm10k_dev_stop().

Thanks,
Michael
> Regards,
>
> Bernard.
>
>> +	fm10k_dev_close(dev);
>> +
>> +	dev->dev_ops = NULL;
>> +	dev->rx_pkt_burst = NULL;
>> +	dev->tx_pkt_burst = NULL;
>> +
>> +	/* disable uio/vfio intr */
>> +	rte_intr_disable(&(dev->pci_dev->intr_handle));
>> +
>> +	/*PF/VF has different interrupt handling mechanism */
>> +	if (hw->mac.type == fm10k_mac_pf) {
>> +		/* disable interrupt */
>> +		fm10k_dev_disable_intr_pf(dev);
>> +
>> +		/* unregister callback func to eal lib */
>> +		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
>> +			fm10k_dev_interrupt_handler_pf, (void *)dev);
>> +	} else {
>> +		/* disable interrupt */
>> +		fm10k_dev_disable_intr_vf(dev);
>> +
>> +		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
>> +			fm10k_dev_interrupt_handler_vf, (void *)dev);
>> +	}
>> +
>> +	/* free mac memory */
>> +	if (dev->data->mac_addrs) {
>> +		rte_free(dev->data->mac_addrs);
>> +		dev->data->mac_addrs = NULL;
>> +	}
>> +
>> +	memset(hw, 0, sizeof(*hw));
>> +
>> +	return 0;
>> +}
>> +
>>  /*
>>   * The set of PCI devices this driver supports. This driver will enable both PF
>>   * and SRIOV-VF devices.
>> @@ -1873,9 +1967,10 @@ static struct eth_driver rte_pmd_fm10k = {
>>  	{
>>  		.name = "rte_pmd_fm10k",
>>  		.id_table = pci_id_fm10k_map,
>> -		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
>> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
>> RTE_PCI_DRV_DETACHABLE,
>>  	},
>>  	.eth_dev_init = eth_fm10k_dev_init,
>> +	.eth_dev_uninit = eth_fm10k_dev_uninit,
>>  	.dev_private_size = sizeof(struct fm10k_adapter),  };
>>
>> --
>> 1.9.3
>

[dpdk-dev] [PATCH 2/2] fm10k: Add hotplug support for fm10k

[Michael Qiu]

Add hotplug support for fm10k.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 97 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index e310698..0d3eaf1 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1412,6 +1412,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)
 }
 
 static void
+fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
+
+	/* Disable misc causes */
+	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
+				FM10K_EIMR_DISABLE(THI_FAULT) |
+				FM10K_EIMR_DISABLE(FUM_FAULT) |
+				FM10K_EIMR_DISABLE(MAILBOX) |
+				FM10K_EIMR_DISABLE(SWITCHREADY) |
+				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
+				FM10K_EIMR_DISABLE(SRAMERROR) |
+				FM10K_EIMR_DISABLE(VFLR));
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
+static void
 fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 {
 	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -1429,6 +1459,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 	FM10K_WRITE_FLUSH(hw);
 }
 
+static void
+fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	/* Only INT 0 available, other 15 are reserved. */
+	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
 static int
 fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
 {
@@ -1858,6 +1904,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+eth_fm10k_dev_uninit(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	PMD_INIT_FUNC_TRACE();
+
+	/* only uninitialize in the primary process */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	/* safe to close dev here */
+	fm10k_dev_close(dev);
+
+	dev->dev_ops = NULL;
+	dev->rx_pkt_burst = NULL;
+	dev->tx_pkt_burst = NULL;
+
+	/* disable uio/vfio intr */
+	rte_intr_disable(&(dev->pci_dev->intr_handle));
+
+	/*PF/VF has different interrupt handling mechanism */
+	if (hw->mac.type == fm10k_mac_pf) {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_pf(dev);
+
+		/* unregister callback func to eal lib */
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_pf, (void *)dev);
+	} else {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_vf(dev);
+
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_vf, (void *)dev);
+	}
+
+	/* free mac memory */
+	if (dev->data->mac_addrs) {
+		rte_free(dev->data->mac_addrs);
+		dev->data->mac_addrs = NULL;
+	}
+
+	memset(hw, 0, sizeof(*hw));
+
+	return 0;
+}
+
 /*
  * The set of PCI devices this driver supports. This driver will enable both PF
  * and SRIOV-VF devices.
@@ -1873,9 +1967,10 @@ static struct eth_driver rte_pmd_fm10k = {
 	{
 		.name = "rte_pmd_fm10k",
 		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 	},
 	.eth_dev_init = eth_fm10k_dev_init,
+	.eth_dev_uninit = eth_fm10k_dev_uninit,
 	.dev_private_size = sizeof(struct fm10k_adapter),
 };
 
-- 
1.9.3

Re: [dpdk-dev] [PATCH 0/2 v2] Enable hotplug support for fm10k

[Qiu, Michael]

Compare with RFC patch:
1. split one patch to two, especially for free memory.
2. rework from lib/ to drivers/net

On 6/10/2015 8:21 PM, Qiu, Michael wrote:
> Hotplug feature is supported in EAL, this patch set is to enable
> this feature in driver side.
>
> Michael Qiu (2):
>   fm10k: Free queues when close port
>   fm10k: Add hotplug support for fm10k
>
>  drivers/net/fm10k/fm10k_ethdev.c | 134 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 129 insertions(+), 5 deletions(-)
>

[dpdk-dev] [PATCH 0/2 v3] Enable hotplug support for fm10k

[Michael Qiu]

Hotplug feature is supported in EAL, this patch set is to enable
this feature in driver side.

change log:
	v3-->v2:
	reset queue numbers to zero.
	v2 --> v1:
	remove __rte_unused flag

Michael Qiu (2):
  fm10k: Free queues when close port
  fm10k: Add hotplug support for fm10k

 drivers/net/fm10k/fm10k_ethdev.c | 134 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 129 insertions(+), 5 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH 1/2 v3] fm10k: Free queues when close port

[Michael Qiu]

When close a port, lots of memory should be released,
such as software rings, queues, etc.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 4afd5ab..6a14633 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -52,6 +52,10 @@
 
 static void fm10k_close_mbx_service(struct fm10k_hw *hw);
 
+static void fm10k_tx_queue_release(void *queue);
+
+static void fm10k_rx_queue_release(void *queue);
+
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)
 {
@@ -665,11 +669,37 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	for (i = 0; i < dev->data->nb_tx_queues; i++)
-		fm10k_dev_tx_queue_stop(dev, i);
+	if (dev->data->tx_queues)
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_dev_tx_queue_stop(dev, i);
+
+	if (dev->data->rx_queues)
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_dev_rx_queue_stop(dev, i);
+}
 
-	for (i = 0; i < dev->data->nb_rx_queues; i++)
-		fm10k_dev_rx_queue_stop(dev, i);
+static void
+fm10k_dev_queue_release(struct rte_eth_dev *dev)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dev->data->tx_queues) {
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_tx_queue_release(dev->data->tx_queues[i]);
+		rte_free(dev->data->tx_queues);
+		dev->data->tx_queues = NULL;
+		dev->data->nb_tx_queues = 0;
+	}
+
+	if (dev->data->rx_queues) {
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_rx_queue_release(dev->data->rx_queues[i]);
+		rte_free(dev->data->rx_queues);
+		dev->data->rx_queues = NULL;
+		dev->data->nb_rx_queues = 0;
+	}
 }
 
 static void
@@ -682,6 +712,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
+	fm10k_dev_queue_release(dev);
 	fm10k_stop_hw(hw);
 }
 
-- 
1.9.3

[dpdk-dev] [PATCH 2/2 v2] fm10k: Add hotplug support for fm10k

[Michael Qiu]

Add hotplug support for fm10k.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 97 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 6a14633..38d1eb7 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1414,6 +1414,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)
 }
 
 static void
+fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
+
+	/* Disable misc causes */
+	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
+				FM10K_EIMR_DISABLE(THI_FAULT) |
+				FM10K_EIMR_DISABLE(FUM_FAULT) |
+				FM10K_EIMR_DISABLE(MAILBOX) |
+				FM10K_EIMR_DISABLE(SWITCHREADY) |
+				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
+				FM10K_EIMR_DISABLE(SRAMERROR) |
+				FM10K_EIMR_DISABLE(VFLR));
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
+static void
 fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 {
 	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -1431,6 +1461,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 	FM10K_WRITE_FLUSH(hw);
 }
 
+static void
+fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	/* Only INT 0 available, other 15 are reserved. */
+	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
 static int
 fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
 {
@@ -1860,6 +1906,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+eth_fm10k_dev_uninit(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	PMD_INIT_FUNC_TRACE();
+
+	/* only uninitialize in the primary process */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	/* safe to close dev here */
+	fm10k_dev_close(dev);
+
+	dev->dev_ops = NULL;
+	dev->rx_pkt_burst = NULL;
+	dev->tx_pkt_burst = NULL;
+
+	/* disable uio/vfio intr */
+	rte_intr_disable(&(dev->pci_dev->intr_handle));
+
+	/*PF/VF has different interrupt handling mechanism */
+	if (hw->mac.type == fm10k_mac_pf) {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_pf(dev);
+
+		/* unregister callback func to eal lib */
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_pf, (void *)dev);
+	} else {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_vf(dev);
+
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_vf, (void *)dev);
+	}
+
+	/* free mac memory */
+	if (dev->data->mac_addrs) {
+		rte_free(dev->data->mac_addrs);
+		dev->data->mac_addrs = NULL;
+	}
+
+	memset(hw, 0, sizeof(*hw));
+
+	return 0;
+}
+
 /*
  * The set of PCI devices this driver supports. This driver will enable both PF
  * and SRIOV-VF devices.
@@ -1875,9 +1969,10 @@ static struct eth_driver rte_pmd_fm10k = {
 	.pci_drv = {
 		.name = "rte_pmd_fm10k",
 		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 	},
 	.eth_dev_init = eth_fm10k_dev_init,
+	.eth_dev_uninit = eth_fm10k_dev_uninit,
 	.dev_private_size = sizeof(struct fm10k_adapter),
 };
 
-- 
1.9.3

Re: [dpdk-dev] [PATCH 2/2 v2] fm10k: Add hotplug support for fm10k

[Qiu, Michael]

Sorry this is version 3,

Thanks
Michael
On 6/19/2015 4:32 PM, Qiu, Michael wrote:
> Add hotplug support for fm10k.
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c | 97 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index 6a14633..38d1eb7 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -1414,6 +1414,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)
>  }
>  
>  static void
> +fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev)
> +{
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t int_map = FM10K_INT_MAP_DISABLE;
> +
> +	int_map |= 0;
> +
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
> +	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
> +
> +	/* Disable misc causes */
> +	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
> +				FM10K_EIMR_DISABLE(THI_FAULT) |
> +				FM10K_EIMR_DISABLE(FUM_FAULT) |
> +				FM10K_EIMR_DISABLE(MAILBOX) |
> +				FM10K_EIMR_DISABLE(SWITCHREADY) |
> +				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
> +				FM10K_EIMR_DISABLE(SRAMERROR) |
> +				FM10K_EIMR_DISABLE(VFLR));
> +
> +	/* Disable ITR 0 */
> +	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
> +	FM10K_WRITE_FLUSH(hw);
> +}
> +
> +static void
>  fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
>  {
>  	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> @@ -1431,6 +1461,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
>  	FM10K_WRITE_FLUSH(hw);
>  }
>  
> +static void
> +fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev)
> +{
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t int_map = FM10K_INT_MAP_DISABLE;
> +
> +	int_map |= 0;
> +
> +	/* Only INT 0 available, other 15 are reserved. */
> +	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
> +
> +	/* Disable ITR 0 */
> +	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
> +	FM10K_WRITE_FLUSH(hw);
> +}
> +
>  static int
>  fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
>  {
> @@ -1860,6 +1906,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  	return 0;
>  }
>  
> +static int
> +eth_fm10k_dev_uninit(struct rte_eth_dev *dev)
> +{
> +	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	/* only uninitialize in the primary process */
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return 0;
> +
> +	/* safe to close dev here */
> +	fm10k_dev_close(dev);
> +
> +	dev->dev_ops = NULL;
> +	dev->rx_pkt_burst = NULL;
> +	dev->tx_pkt_burst = NULL;
> +
> +	/* disable uio/vfio intr */
> +	rte_intr_disable(&(dev->pci_dev->intr_handle));
> +
> +	/*PF/VF has different interrupt handling mechanism */
> +	if (hw->mac.type == fm10k_mac_pf) {
> +		/* disable interrupt */
> +		fm10k_dev_disable_intr_pf(dev);
> +
> +		/* unregister callback func to eal lib */
> +		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
> +			fm10k_dev_interrupt_handler_pf, (void *)dev);
> +	} else {
> +		/* disable interrupt */
> +		fm10k_dev_disable_intr_vf(dev);
> +
> +		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
> +			fm10k_dev_interrupt_handler_vf, (void *)dev);
> +	}
> +
> +	/* free mac memory */
> +	if (dev->data->mac_addrs) {
> +		rte_free(dev->data->mac_addrs);
> +		dev->data->mac_addrs = NULL;
> +	}
> +
> +	memset(hw, 0, sizeof(*hw));
> +
> +	return 0;
> +}
> +
>  /*
>   * The set of PCI devices this driver supports. This driver will enable both PF
>   * and SRIOV-VF devices.
> @@ -1875,9 +1969,10 @@ static struct eth_driver rte_pmd_fm10k = {
>  	.pci_drv = {
>  		.name = "rte_pmd_fm10k",
>  		.id_table = pci_id_fm10k_map,
> -		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
>  	},
>  	.eth_dev_init = eth_fm10k_dev_init,
> +	.eth_dev_uninit = eth_fm10k_dev_uninit,
>  	.dev_private_size = sizeof(struct fm10k_adapter),
>  };
>  

[dpdk-dev] [PATCH 0/2 v4] Enable hotplug support for fm10k

[Michael Qiu]

Hotplug feature is supported in EAL, this patch set is to enable
this feature in driver side.

change log:
	v4 --> v3:
	rebase code.
	v3 --> v2:
	reset queue numbers to zero.
	v2 --> v1:
	remove __rte_unused flag

Michael Qiu (2):
  fm10k: Free queues when close port
  fm10k: Add hotplug support for fm10k

 drivers/net/fm10k/fm10k_ethdev.c | 134 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 129 insertions(+), 5 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Michael Qiu]

When close a port, lots of memory should be released,
such as software rings, queues, etc.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 406c350..eba7228 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -65,6 +65,8 @@ static void
 fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add);
 static void
 fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev);
+static void fm10k_tx_queue_release(void *queue);
+static void fm10k_rx_queue_release(void *queue);
 
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)
@@ -809,11 +811,37 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	for (i = 0; i < dev->data->nb_tx_queues; i++)
-		fm10k_dev_tx_queue_stop(dev, i);
+	if (dev->data->tx_queues)
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_dev_tx_queue_stop(dev, i);
 
-	for (i = 0; i < dev->data->nb_rx_queues; i++)
-		fm10k_dev_rx_queue_stop(dev, i);
+	if (dev->data->rx_queues)
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_dev_rx_queue_stop(dev, i);
+}
+
+static void
+fm10k_dev_queue_release(struct rte_eth_dev *dev)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dev->data->tx_queues) {
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_tx_queue_release(dev->data->tx_queues[i]);
+		rte_free(dev->data->tx_queues);
+		dev->data->tx_queues = NULL;
+		dev->data->nb_tx_queues = 0;
+	}
+
+	if (dev->data->rx_queues) {
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_rx_queue_release(dev->data->rx_queues[i]);
+		rte_free(dev->data->rx_queues);
+		dev->data->rx_queues = NULL;
+		dev->data->nb_rx_queues = 0;
+	}
 }
 
 static void
@@ -828,6 +856,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
+	fm10k_dev_queue_release(dev);
 	fm10k_stop_hw(hw);
 }
 
-- 
1.9.3

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Iremonger, Bernard]

> -----Original Message-----
> From: Qiu, Michael
> Sent: Friday, June 26, 2015 9:30 AM
> To: dev@dpdk.org
> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
> 
> When close a port, lots of memory should be released, such as software
> rings, queues, etc.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
Hi Michael,

There are 2 comments inline 

>  drivers/net/fm10k/fm10k_ethdev.c | 37
> +++++++++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 406c350..eba7228 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -65,6 +65,8 @@ static void
>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add);
> static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev);
> +static void fm10k_tx_queue_release(void *queue); static void
> +fm10k_rx_queue_release(void *queue);
> 
>  static void
>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
> fm10k_dev_stop(struct rte_eth_dev *dev)
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
> -		fm10k_dev_tx_queue_stop(dev, i);
> +	if (dev->data->tx_queues)
> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> +			fm10k_dev_tx_queue_stop(dev, i);
> 
> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
> -		fm10k_dev_rx_queue_stop(dev, i);
> +	if (dev->data->rx_queues)
> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> +			fm10k_dev_rx_queue_stop(dev, i);
> +}
> +
> +static void
> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
> +	int i;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (dev->data->tx_queues) {
> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> +			fm10k_tx_queue_release(dev->data-
> >tx_queues[i]);
> +		rte_free(dev->data->tx_queues);
> +		dev->data->tx_queues = NULL;

The memory for dev->data->tx_queues  is not allocated in the fm10k PMD,
so it should probably not be released here.
I have submitted a patch today for rte_eth_dev.c  to do this.  
/dev/patchwork/patch/5829/

> +		dev->data->nb_tx_queues = 0;
> +	}
> +
> +	if (dev->data->rx_queues) {
> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> +			fm10k_rx_queue_release(dev->data-
> >rx_queues[i]);
> +		rte_free(dev->data->rx_queues);
> +		dev->data->rx_queues = NULL;

The memory for dev->data->rx_queues  is not allocated in the fm10k PMD,
so it should probably not be released here.
I have submitted a patch today for rte_eth_dev.c  to do this.  
/dev/patchwork/patch/5829/

Regards,

Bernard.


> +		dev->data->nb_rx_queues = 0;
> +	}
>  }
> 
>  static void
> @@ -828,6 +856,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
>  	/* Stop mailbox service first */
>  	fm10k_close_mbx_service(hw);
>  	fm10k_dev_stop(dev);
> +	fm10k_dev_queue_release(dev);
>  	fm10k_stop_hw(hw);
>  }
> 
> --
> 1.9.3

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Qiu, Michael]

On 6/26/2015 7:02 PM, Iremonger, Bernard wrote:
>> -----Original Message-----
>> From: Qiu, Michael
>> Sent: Friday, June 26, 2015 9:30 AM
>> To: dev@dpdk.org
>> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
>> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
>>
>> When close a port, lots of memory should be released, such as software
>> rings, queues, etc.
>>
>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>> ---
> Hi Michael,
>
> There are 2 comments inline 
>
>>  drivers/net/fm10k/fm10k_ethdev.c | 37
>> +++++++++++++++++++++++++++++++++----
>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
>> b/drivers/net/fm10k/fm10k_ethdev.c
>> index 406c350..eba7228 100644
>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>> @@ -65,6 +65,8 @@ static void
>>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add);
>> static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev);
>> +static void fm10k_tx_queue_release(void *queue); static void
>> +fm10k_rx_queue_release(void *queue);
>>
>>  static void
>>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
>> fm10k_dev_stop(struct rte_eth_dev *dev)
>>
>>  	PMD_INIT_FUNC_TRACE();
>>
>> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
>> -		fm10k_dev_tx_queue_stop(dev, i);
>> +	if (dev->data->tx_queues)
>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>> +			fm10k_dev_tx_queue_stop(dev, i);
>>
>> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
>> -		fm10k_dev_rx_queue_stop(dev, i);
>> +	if (dev->data->rx_queues)
>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>> +			fm10k_dev_rx_queue_stop(dev, i);
>> +}
>> +
>> +static void
>> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
>> +	int i;
>> +
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	if (dev->data->tx_queues) {
>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>> +			fm10k_tx_queue_release(dev->data-
>>> tx_queues[i]);
>> +		rte_free(dev->data->tx_queues);
>> +		dev->data->tx_queues = NULL;
> The memory for dev->data->tx_queues  is not allocated in the fm10k PMD,
> so it should probably not be released here.
> I have submitted a patch today for rte_eth_dev.c  to do this.  
> /dev/patchwork/patch/5829/
>
>> +		dev->data->nb_tx_queues = 0;
>> +	}
>> +
>> +	if (dev->data->rx_queues) {
>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>> +			fm10k_rx_queue_release(dev->data-
>>> rx_queues[i]);
>> +		rte_free(dev->data->rx_queues);
>> +		dev->data->rx_queues = NULL;
> The memory for dev->data->rx_queues  is not allocated in the fm10k PMD,
> so it should probably not be released here.
> I have submitted a patch today for rte_eth_dev.c  to do this.  
> /dev/patchwork/patch/5829/

Is it a good idea?  What about to close the port for twice at a time?
I think it is better to do it in rte_eth_dev_close(), I will give the
comments to you.

Thanks,
Michael

> Regards,
>
> Bernard.
>
>
>> +		dev->data->nb_rx_queues = 0;
>> +	}
>>  }
>>
>>  static void
>> @@ -828,6 +856,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
>>  	/* Stop mailbox service first */
>>  	fm10k_close_mbx_service(hw);
>>  	fm10k_dev_stop(dev);
>> +	fm10k_dev_queue_release(dev);
>>  	fm10k_stop_hw(hw);
>>  }
>>
>> --
>> 1.9.3
>

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Iremonger, Bernard]

> -----Original Message-----
> From: Qiu, Michael
> Sent: Monday, June 29, 2015 9:17 AM
> To: Iremonger, Bernard; dev@dpdk.org
> Cc: Chen, Jing D; He, Shaopeng
> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
> 
> On 6/26/2015 7:02 PM, Iremonger, Bernard wrote:
> >> -----Original Message-----
> >> From: Qiu, Michael
> >> Sent: Friday, June 26, 2015 9:30 AM
> >> To: dev@dpdk.org
> >> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
> >> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
> >>
> >> When close a port, lots of memory should be released, such as
> >> software rings, queues, etc.
> >>
> >> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> >> ---
> > Hi Michael,
> >
> > There are 2 comments inline
> >
> >>  drivers/net/fm10k/fm10k_ethdev.c | 37
> >> +++++++++++++++++++++++++++++++++----
> >>  1 file changed, 33 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> >> b/drivers/net/fm10k/fm10k_ethdev.c
> >> index 406c350..eba7228 100644
> >> --- a/drivers/net/fm10k/fm10k_ethdev.c
> >> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> >> @@ -65,6 +65,8 @@ static void
> >>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool
> >> add); static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev
> *dev);
> >> +static void fm10k_tx_queue_release(void *queue); static void
> >> +fm10k_rx_queue_release(void *queue);
> >>
> >>  static void
> >>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
> >> fm10k_dev_stop(struct rte_eth_dev *dev)
> >>
> >>  	PMD_INIT_FUNC_TRACE();
> >>
> >> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
> >> -		fm10k_dev_tx_queue_stop(dev, i);
> >> +	if (dev->data->tx_queues)
> >> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> >> +			fm10k_dev_tx_queue_stop(dev, i);
> >>
> >> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
> >> -		fm10k_dev_rx_queue_stop(dev, i);
> >> +	if (dev->data->rx_queues)
> >> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> >> +			fm10k_dev_rx_queue_stop(dev, i);
> >> +}
> >> +
> >> +static void
> >> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
> >> +	int i;
> >> +
> >> +	PMD_INIT_FUNC_TRACE();
> >> +
> >> +	if (dev->data->tx_queues) {
> >> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> >> +			fm10k_tx_queue_release(dev->data-
> >>> tx_queues[i]);
> >> +		rte_free(dev->data->tx_queues);
> >> +		dev->data->tx_queues = NULL;
> > The memory for dev->data->tx_queues  is not allocated in the fm10k
> > PMD, so it should probably not be released here.
> > I have submitted a patch today for rte_eth_dev.c  to do this.
> > /dev/patchwork/patch/5829/
> >
> >> +		dev->data->nb_tx_queues = 0;
> >> +	}
> >> +
> >> +	if (dev->data->rx_queues) {
> >> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> >> +			fm10k_rx_queue_release(dev->data-
> >>> rx_queues[i]);
> >> +		rte_free(dev->data->rx_queues);
> >> +		dev->data->rx_queues = NULL;
> > The memory for dev->data->rx_queues  is not allocated in the fm10k
> > PMD, so it should probably not be released here.
> > I have submitted a patch today for rte_eth_dev.c  to do this.
> > /dev/patchwork/patch/5829/
> 
> Is it a good idea?  What about to close the port for twice at a time?
> I think it is better to do it in rte_eth_dev_close(), I will give the comments to
> you.
> 
> Thanks,
> Michael

Hi Michael,
Could you take a look at the comments on http://dpdk.org/dev/patchwork/patch/5829/
The consensus is that memory should be freed in the component that allocated it.
In my pmd hotplug patches I have used a flag to ensure that dev_close is not called twice.
In the e1000 patch I have added a stopped flag to struct e1000_adapter.   
http://dpdk.org/dev/patchwork/patch/5655/

Regards,

Bernard.

<snip>

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Qiu, Michael]

On 6/29/2015 4:57 PM, Iremonger, Bernard wrote:
>> -----Original Message-----
>> From: Qiu, Michael
>> Sent: Monday, June 29, 2015 9:17 AM
>> To: Iremonger, Bernard; dev@dpdk.org
>> Cc: Chen, Jing D; He, Shaopeng
>> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
>>
>> On 6/26/2015 7:02 PM, Iremonger, Bernard wrote:
>>>> -----Original Message-----
>>>> From: Qiu, Michael
>>>> Sent: Friday, June 26, 2015 9:30 AM
>>>> To: dev@dpdk.org
>>>> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
>>>> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
>>>>
>>>> When close a port, lots of memory should be released, such as
>>>> software rings, queues, etc.
>>>>
>>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>>>> ---
>>> Hi Michael,
>>>
>>> There are 2 comments inline
>>>
>>>>  drivers/net/fm10k/fm10k_ethdev.c | 37
>>>> +++++++++++++++++++++++++++++++++----
>>>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
>>>> b/drivers/net/fm10k/fm10k_ethdev.c
>>>> index 406c350..eba7228 100644
>>>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>>>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>>>> @@ -65,6 +65,8 @@ static void
>>>>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool
>>>> add); static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev
>> *dev);
>>>> +static void fm10k_tx_queue_release(void *queue); static void
>>>> +fm10k_rx_queue_release(void *queue);
>>>>
>>>>  static void
>>>>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
>>>> fm10k_dev_stop(struct rte_eth_dev *dev)
>>>>
>>>>  	PMD_INIT_FUNC_TRACE();
>>>>
>>>> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>> -		fm10k_dev_tx_queue_stop(dev, i);
>>>> +	if (dev->data->tx_queues)
>>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>> +			fm10k_dev_tx_queue_stop(dev, i);
>>>>
>>>> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>> -		fm10k_dev_rx_queue_stop(dev, i);
>>>> +	if (dev->data->rx_queues)
>>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>> +			fm10k_dev_rx_queue_stop(dev, i);
>>>> +}
>>>> +
>>>> +static void
>>>> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
>>>> +	int i;
>>>> +
>>>> +	PMD_INIT_FUNC_TRACE();
>>>> +
>>>> +	if (dev->data->tx_queues) {
>>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>> +			fm10k_tx_queue_release(dev->data-
>>>>> tx_queues[i]);
>>>> +		rte_free(dev->data->tx_queues);
>>>> +		dev->data->tx_queues = NULL;
>>> The memory for dev->data->tx_queues  is not allocated in the fm10k
>>> PMD, so it should probably not be released here.
>>> I have submitted a patch today for rte_eth_dev.c  to do this.
>>> /dev/patchwork/patch/5829/
>>>
>>>> +		dev->data->nb_tx_queues = 0;
>>>> +	}
>>>> +
>>>> +	if (dev->data->rx_queues) {
>>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>> +			fm10k_rx_queue_release(dev->data-
>>>>> rx_queues[i]);
>>>> +		rte_free(dev->data->rx_queues);
>>>> +		dev->data->rx_queues = NULL;
>>> The memory for dev->data->rx_queues  is not allocated in the fm10k
>>> PMD, so it should probably not be released here.
>>> I have submitted a patch today for rte_eth_dev.c  to do this.
>>> /dev/patchwork/patch/5829/
>> Is it a good idea?  What about to close the port for twice at a time?
>> I think it is better to do it in rte_eth_dev_close(), I will give the comments to
>> you.
>>
>> Thanks,
>> Michael
> Hi Michael,
> Could you take a look at the comments on http://dpdk.org/dev/patchwork/patch/5829/

Hi, Bernard

I have give comments on it.

> The consensus is that memory should be freed in the component that allocated it.
> In my pmd hotplug patches I have used a flag to ensure that dev_close is not called twice.
> In the e1000 patch I have added a stopped flag to struct e1000_adapter.   
> http://dpdk.org/dev/patchwork/patch/5655/



I reviewed your patch about ixgbe and fvl before. But forget e1000.

In my logic, when dev->data->rx_queues is NULL, that means this device
has been closed before. What else, we even do not care about whether it
has been closed or not, when close() function be called, all memory
should be freed if exist am I right?

So, check  dev->data->rx_queues whether it is NULL will be recommend in
close function, only this could avoid unsafe situations for pointer.

Thanks,
Michael
> Regards,
>
> Bernard.
>
> <snip>
>
>
>

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Iremonger, Bernard]

> -----Original Message-----
> From: Qiu, Michael
> Sent: Monday, June 29, 2015 10:21 AM
> To: Iremonger, Bernard; dev@dpdk.org
> Cc: Chen, Jing D; He, Shaopeng
> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
> 
> On 6/29/2015 4:57 PM, Iremonger, Bernard wrote:
> >> -----Original Message-----
> >> From: Qiu, Michael
> >> Sent: Monday, June 29, 2015 9:17 AM
> >> To: Iremonger, Bernard; dev@dpdk.org
> >> Cc: Chen, Jing D; He, Shaopeng
> >> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
> >>
> >> On 6/26/2015 7:02 PM, Iremonger, Bernard wrote:
> >>>> -----Original Message-----
> >>>> From: Qiu, Michael
> >>>> Sent: Friday, June 26, 2015 9:30 AM
> >>>> To: dev@dpdk.org
> >>>> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
> >>>> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
> >>>>
> >>>> When close a port, lots of memory should be released, such as
> >>>> software rings, queues, etc.
> >>>>
> >>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> >>>> ---
> >>> Hi Michael,
> >>>
> >>> There are 2 comments inline
> >>>
> >>>>  drivers/net/fm10k/fm10k_ethdev.c | 37
> >>>> +++++++++++++++++++++++++++++++++----
> >>>>  1 file changed, 33 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> >>>> b/drivers/net/fm10k/fm10k_ethdev.c
> >>>> index 406c350..eba7228 100644
> >>>> --- a/drivers/net/fm10k/fm10k_ethdev.c
> >>>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> >>>> @@ -65,6 +65,8 @@ static void
> >>>>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool
> >>>> add); static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev
> >> *dev);
> >>>> +static void fm10k_tx_queue_release(void *queue); static void
> >>>> +fm10k_rx_queue_release(void *queue);
> >>>>
> >>>>  static void
> >>>>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
> >>>> fm10k_dev_stop(struct rte_eth_dev *dev)
> >>>>
> >>>>  	PMD_INIT_FUNC_TRACE();
> >>>>
> >>>> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
> >>>> -		fm10k_dev_tx_queue_stop(dev, i);
> >>>> +	if (dev->data->tx_queues)
> >>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> >>>> +			fm10k_dev_tx_queue_stop(dev, i);
> >>>>
> >>>> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
> >>>> -		fm10k_dev_rx_queue_stop(dev, i);
> >>>> +	if (dev->data->rx_queues)
> >>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> >>>> +			fm10k_dev_rx_queue_stop(dev, i); }
> >>>> +
> >>>> +static void
> >>>> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
> >>>> +	int i;
> >>>> +
> >>>> +	PMD_INIT_FUNC_TRACE();
> >>>> +
> >>>> +	if (dev->data->tx_queues) {
> >>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> >>>> +			fm10k_tx_queue_release(dev->data-
> >>>>> tx_queues[i]);
> >>>> +		rte_free(dev->data->tx_queues);
> >>>> +		dev->data->tx_queues = NULL;
> >>> The memory for dev->data->tx_queues  is not allocated in the fm10k
> >>> PMD, so it should probably not be released here.
> >>> I have submitted a patch today for rte_eth_dev.c  to do this.
> >>> /dev/patchwork/patch/5829/
> >>>
> >>>> +		dev->data->nb_tx_queues = 0;
> >>>> +	}
> >>>> +
> >>>> +	if (dev->data->rx_queues) {
> >>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> >>>> +			fm10k_rx_queue_release(dev->data-
> >>>>> rx_queues[i]);
> >>>> +		rte_free(dev->data->rx_queues);
> >>>> +		dev->data->rx_queues = NULL;
> >>> The memory for dev->data->rx_queues  is not allocated in the fm10k
> >>> PMD, so it should probably not be released here.
> >>> I have submitted a patch today for rte_eth_dev.c  to do this.
> >>> /dev/patchwork/patch/5829/
> >> Is it a good idea?  What about to close the port for twice at a time?
> >> I think it is better to do it in rte_eth_dev_close(), I will give the
> >> comments to you.
> >>
> >> Thanks,
> >> Michael
> > Hi Michael,
> > Could you take a look at the comments on
> > http://dpdk.org/dev/patchwork/patch/5829/
> 
> Hi, Bernard
> 
> I have give comments on it.
> 
> > The consensus is that memory should be freed in the component that
> allocated it.
> > In my pmd hotplug patches I have used a flag to ensure that dev_close is
> not called twice.
> > In the e1000 patch I have added a stopped flag to struct e1000_adapter.
> > http://dpdk.org/dev/patchwork/patch/5655/
> 
> 
> 
> I reviewed your patch about ixgbe and fvl before. But forget e1000.
> 
> In my logic, when dev->data->rx_queues is NULL, that means this device has
> been closed before. What else, we even do not care about whether it has
> been closed or not, when close() function be called, all memory should be
> freed if exist am I right?
> 
> So, check  dev->data->rx_queues whether it is NULL will be recommend in
> close function, only this could avoid unsafe situations for pointer.
> 
> Thanks,
> Michael

Hi Michael,

In most of the pmd's memory is allocated in the dev_init()functions and released in the dev_uninit() functions. The dev_uninit() functions call dev_close(), so either way the memory is released.
The point I am trying to make is that the rx_queue and tx_queue memory is not allocated by the pmd and so it should not be freed by the pmd (please see comments on 
http://dpdk.org/dev/patchwork/patch/5790/)
The memory is allocated in rte_eth_dev_rx_queue_config() and rte_eth_dev_tx_queue_config(),
which are both called from rte_eth_dev_configure() which is called by the application (for example test_pmd). So it seems to make sense to free this memory  in rte_eth_dev_uninit().

Regards,

Bernard.

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Qiu, Michael]

On 2015/6/29 17:54, Iremonger, Bernard wrote:
>> -----Original Message-----
>> From: Qiu, Michael
>> Sent: Monday, June 29, 2015 10:21 AM
>> To: Iremonger, Bernard; dev@dpdk.org
>> Cc: Chen, Jing D; He, Shaopeng
>> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
>>
>> On 6/29/2015 4:57 PM, Iremonger, Bernard wrote:
>>>> -----Original Message-----
>>>> From: Qiu, Michael
>>>> Sent: Monday, June 29, 2015 9:17 AM
>>>> To: Iremonger, Bernard; dev@dpdk.org
>>>> Cc: Chen, Jing D; He, Shaopeng
>>>> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
>>>>
>>>> On 6/26/2015 7:02 PM, Iremonger, Bernard wrote:
>>>>>> -----Original Message-----
>>>>>> From: Qiu, Michael
>>>>>> Sent: Friday, June 26, 2015 9:30 AM
>>>>>> To: dev@dpdk.org
>>>>>> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
>>>>>> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
>>>>>>
>>>>>> When close a port, lots of memory should be released, such as
>>>>>> software rings, queues, etc.
>>>>>>
>>>>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>>>>>> ---
>>>>> Hi Michael,
>>>>>
>>>>> There are 2 comments inline
>>>>>
>>>>>>  drivers/net/fm10k/fm10k_ethdev.c | 37
>>>>>> +++++++++++++++++++++++++++++++++----
>>>>>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
>>>>>> b/drivers/net/fm10k/fm10k_ethdev.c
>>>>>> index 406c350..eba7228 100644
>>>>>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>>>>>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>>>>>> @@ -65,6 +65,8 @@ static void
>>>>>>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool
>>>>>> add); static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev
>>>> *dev);
>>>>>> +static void fm10k_tx_queue_release(void *queue); static void
>>>>>> +fm10k_rx_queue_release(void *queue);
>>>>>>
>>>>>>  static void
>>>>>>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
>>>>>> fm10k_dev_stop(struct rte_eth_dev *dev)
>>>>>>
>>>>>>  	PMD_INIT_FUNC_TRACE();
>>>>>>
>>>>>> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>> -		fm10k_dev_tx_queue_stop(dev, i);
>>>>>> +	if (dev->data->tx_queues)
>>>>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>> +			fm10k_dev_tx_queue_stop(dev, i);
>>>>>>
>>>>>> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>> -		fm10k_dev_rx_queue_stop(dev, i);
>>>>>> +	if (dev->data->rx_queues)
>>>>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>> +			fm10k_dev_rx_queue_stop(dev, i); }
>>>>>> +
>>>>>> +static void
>>>>>> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
>>>>>> +	int i;
>>>>>> +
>>>>>> +	PMD_INIT_FUNC_TRACE();
>>>>>> +
>>>>>> +	if (dev->data->tx_queues) {
>>>>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>> +			fm10k_tx_queue_release(dev->data-
>>>>>>> tx_queues[i]);
>>>>>> +		rte_free(dev->data->tx_queues);
>>>>>> +		dev->data->tx_queues = NULL;
>>>>> The memory for dev->data->tx_queues  is not allocated in the fm10k
>>>>> PMD, so it should probably not be released here.
>>>>> I have submitted a patch today for rte_eth_dev.c  to do this.
>>>>> /dev/patchwork/patch/5829/
>>>>>
>>>>>> +		dev->data->nb_tx_queues = 0;
>>>>>> +	}
>>>>>> +
>>>>>> +	if (dev->data->rx_queues) {
>>>>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>> +			fm10k_rx_queue_release(dev->data-
>>>>>>> rx_queues[i]);
>>>>>> +		rte_free(dev->data->rx_queues);
>>>>>> +		dev->data->rx_queues = NULL;
>>>>> The memory for dev->data->rx_queues  is not allocated in the fm10k
>>>>> PMD, so it should probably not be released here.
>>>>> I have submitted a patch today for rte_eth_dev.c  to do this.
>>>>> /dev/patchwork/patch/5829/
>>>> Is it a good idea?  What about to close the port for twice at a time?
>>>> I think it is better to do it in rte_eth_dev_close(), I will give the
>>>> comments to you.
>>>>
>>>> Thanks,
>>>> Michael
>>> Hi Michael,
>>> Could you take a look at the comments on
>>> http://dpdk.org/dev/patchwork/patch/5829/
>> Hi, Bernard
>>
>> I have give comments on it.
>>
>>> The consensus is that memory should be freed in the component that
>> allocated it.
>>> In my pmd hotplug patches I have used a flag to ensure that dev_close is
>> not called twice.
>>> In the e1000 patch I have added a stopped flag to struct e1000_adapter.
>>> http://dpdk.org/dev/patchwork/patch/5655/
>>
>>
>> I reviewed your patch about ixgbe and fvl before. But forget e1000.
>>
>> In my logic, when dev->data->rx_queues is NULL, that means this device has
>> been closed before. What else, we even do not care about whether it has
>> been closed or not, when close() function be called, all memory should be
>> freed if exist am I right?
>>
>> So, check  dev->data->rx_queues whether it is NULL will be recommend in
>> close function, only this could avoid unsafe situations for pointer.
>>
>> Thanks,
>> Michael
> Hi Michael,
>
> In most of the pmd's memory is allocated in the dev_init()functions and released in the dev_uninit() functions. The dev_uninit() functions call dev_close(), so either way the memory is released.

Yes, but consider that, without hotplug, users just stop a port and
close it. then what happens? the memory does not released!
So that's why I recommend to release the memory on close() function, it
could be in EAL level like rte_eth_dev_close().

Maybe my understand is wrong, please point me out :)

Thanks,
Michael

> The point I am trying to make is that the rx_queue and tx_queue memory is not allocated by the pmd and so it should not be freed by the pmd (please see comments on 
> http://dpdk.org/dev/patchwork/patch/5790/)
> The memory is allocated in rte_eth_dev_rx_queue_config() and rte_eth_dev_tx_queue_config(),
> which are both called from rte_eth_dev_configure() which is called by the application (for example test_pmd). So it seems to make sense to free this memory  in rte_eth_dev_uninit().
>
> Regards,
>
> Bernard.
>
>
>

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Qiu, Michael]

On 2015/6/29 22:58, Qiu, Michael wrote:
> On 2015/6/29 17:54, Iremonger, Bernard wrote:
>>> -----Original Message-----
>>> From: Qiu, Michael
>>> Sent: Monday, June 29, 2015 10:21 AM
>>> To: Iremonger, Bernard; dev@dpdk.org
>>> Cc: Chen, Jing D; He, Shaopeng
>>> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
>>>
>>> On 6/29/2015 4:57 PM, Iremonger, Bernard wrote:
>>>>> -----Original Message-----
>>>>> From: Qiu, Michael
>>>>> Sent: Monday, June 29, 2015 9:17 AM
>>>>> To: Iremonger, Bernard; dev@dpdk.org
>>>>> Cc: Chen, Jing D; He, Shaopeng
>>>>> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
>>>>>
>>>>> On 6/26/2015 7:02 PM, Iremonger, Bernard wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Qiu, Michael
>>>>>>> Sent: Friday, June 26, 2015 9:30 AM
>>>>>>> To: dev@dpdk.org
>>>>>>> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
>>>>>>> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
>>>>>>>
>>>>>>> When close a port, lots of memory should be released, such as
>>>>>>> software rings, queues, etc.
>>>>>>>
>>>>>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>>>>>>> ---
>>>>>> Hi Michael,
>>>>>>
>>>>>> There are 2 comments inline
>>>>>>
>>>>>>>  drivers/net/fm10k/fm10k_ethdev.c | 37
>>>>>>> +++++++++++++++++++++++++++++++++----
>>>>>>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
>>>>>>> b/drivers/net/fm10k/fm10k_ethdev.c
>>>>>>> index 406c350..eba7228 100644
>>>>>>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>>>>>>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>>>>>>> @@ -65,6 +65,8 @@ static void
>>>>>>>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool
>>>>>>> add); static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev
>>>>> *dev);
>>>>>>> +static void fm10k_tx_queue_release(void *queue); static void
>>>>>>> +fm10k_rx_queue_release(void *queue);
>>>>>>>
>>>>>>>  static void
>>>>>>>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
>>>>>>> fm10k_dev_stop(struct rte_eth_dev *dev)
>>>>>>>
>>>>>>>  	PMD_INIT_FUNC_TRACE();
>>>>>>>
>>>>>>> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>>> -		fm10k_dev_tx_queue_stop(dev, i);
>>>>>>> +	if (dev->data->tx_queues)
>>>>>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>>> +			fm10k_dev_tx_queue_stop(dev, i);
>>>>>>>
>>>>>>> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>>> -		fm10k_dev_rx_queue_stop(dev, i);
>>>>>>> +	if (dev->data->rx_queues)
>>>>>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>>> +			fm10k_dev_rx_queue_stop(dev, i); }
>>>>>>> +
>>>>>>> +static void
>>>>>>> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
>>>>>>> +	int i;
>>>>>>> +
>>>>>>> +	PMD_INIT_FUNC_TRACE();
>>>>>>> +
>>>>>>> +	if (dev->data->tx_queues) {
>>>>>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>>> +			fm10k_tx_queue_release(dev->data-
>>>>>>>> tx_queues[i]);
>>>>>>> +		rte_free(dev->data->tx_queues);
>>>>>>> +		dev->data->tx_queues = NULL;
>>>>>> The memory for dev->data->tx_queues  is not allocated in the fm10k
>>>>>> PMD, so it should probably not be released here.
>>>>>> I have submitted a patch today for rte_eth_dev.c  to do this.
>>>>>> /dev/patchwork/patch/5829/
>>>>>>
>>>>>>> +		dev->data->nb_tx_queues = 0;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	if (dev->data->rx_queues) {
>>>>>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>>> +			fm10k_rx_queue_release(dev->data-
>>>>>>>> rx_queues[i]);
>>>>>>> +		rte_free(dev->data->rx_queues);
>>>>>>> +		dev->data->rx_queues = NULL;
>>>>>> The memory for dev->data->rx_queues  is not allocated in the fm10k
>>>>>> PMD, so it should probably not be released here.
>>>>>> I have submitted a patch today for rte_eth_dev.c  to do this.
>>>>>> /dev/patchwork/patch/5829/
>>>>> Is it a good idea?  What about to close the port for twice at a time?
>>>>> I think it is better to do it in rte_eth_dev_close(), I will give the
>>>>> comments to you.
>>>>>
>>>>> Thanks,
>>>>> Michael
>>>> Hi Michael,
>>>> Could you take a look at the comments on
>>>> http://dpdk.org/dev/patchwork/patch/5829/
>>> Hi, Bernard
>>>
>>> I have give comments on it.
>>>
>>>> The consensus is that memory should be freed in the component that
>>> allocated it.
>>>> In my pmd hotplug patches I have used a flag to ensure that dev_close is
>>> not called twice.
>>>> In the e1000 patch I have added a stopped flag to struct e1000_adapter.
>>>> http://dpdk.org/dev/patchwork/patch/5655/
>>>
>>> I reviewed your patch about ixgbe and fvl before. But forget e1000.
>>>
>>> In my logic, when dev->data->rx_queues is NULL, that means this device has
>>> been closed before. What else, we even do not care about whether it has
>>> been closed or not, when close() function be called, all memory should be
>>> freed if exist am I right?
>>>
>>> So, check  dev->data->rx_queues whether it is NULL will be recommend in
>>> close function, only this could avoid unsafe situations for pointer.
>>>
>>> Thanks,
>>> Michael
>> Hi Michael,
>>
>> In most of the pmd's memory is allocated in the dev_init()functions and released in the dev_uninit() functions. The dev_uninit() functions call dev_close(), so either way the memory is released.
> Yes, but consider that, without hotplug, users just stop a port and
> close it. then what happens? the memory does not released!
> So that's why I recommend to release the memory on close() function, it
> could be in EAL level like rte_eth_dev_close().

Sorry, here EAL should be rte_ether

>
> Maybe my understand is wrong, please point me out :)
>
> Thanks,
> Michael
>
>> The point I am trying to make is that the rx_queue and tx_queue memory is not allocated by the pmd and so it should not be freed by the pmd (please see comments on 
>> http://dpdk.org/dev/patchwork/patch/5790/)
>> The memory is allocated in rte_eth_dev_rx_queue_config() and rte_eth_dev_tx_queue_config(),
>> which are both called from rte_eth_dev_configure() which is called by the application (for example test_pmd). So it seems to make sense to free this memory  in rte_eth_dev_uninit().
>>
>> Regards,
>>
>> Bernard.
>>
>>
>>
>

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Ananyev, Konstantin]

Hi Michael,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiu, Michael
> Sent: Monday, June 29, 2015 10:21 AM
> To: Iremonger, Bernard; dev@dpdk.org
> Cc: He, Shaopeng
> Subject: Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port
> 
> On 6/29/2015 4:57 PM, Iremonger, Bernard wrote:
> >> -----Original Message-----
> >> From: Qiu, Michael
> >> Sent: Monday, June 29, 2015 9:17 AM
> >> To: Iremonger, Bernard; dev@dpdk.org
> >> Cc: Chen, Jing D; He, Shaopeng
> >> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
> >>
> >> On 6/26/2015 7:02 PM, Iremonger, Bernard wrote:
> >>>> -----Original Message-----
> >>>> From: Qiu, Michael
> >>>> Sent: Friday, June 26, 2015 9:30 AM
> >>>> To: dev@dpdk.org
> >>>> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
> >>>> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
> >>>>
> >>>> When close a port, lots of memory should be released, such as
> >>>> software rings, queues, etc.
> >>>>
> >>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> >>>> ---
> >>> Hi Michael,
> >>>
> >>> There are 2 comments inline
> >>>
> >>>>  drivers/net/fm10k/fm10k_ethdev.c | 37
> >>>> +++++++++++++++++++++++++++++++++----
> >>>>  1 file changed, 33 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> >>>> b/drivers/net/fm10k/fm10k_ethdev.c
> >>>> index 406c350..eba7228 100644
> >>>> --- a/drivers/net/fm10k/fm10k_ethdev.c
> >>>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> >>>> @@ -65,6 +65,8 @@ static void
> >>>>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool
> >>>> add); static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev
> >> *dev);
> >>>> +static void fm10k_tx_queue_release(void *queue); static void
> >>>> +fm10k_rx_queue_release(void *queue);
> >>>>
> >>>>  static void
> >>>>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
> >>>> fm10k_dev_stop(struct rte_eth_dev *dev)
> >>>>
> >>>>  	PMD_INIT_FUNC_TRACE();
> >>>>
> >>>> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
> >>>> -		fm10k_dev_tx_queue_stop(dev, i);
> >>>> +	if (dev->data->tx_queues)
> >>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> >>>> +			fm10k_dev_tx_queue_stop(dev, i);
> >>>>
> >>>> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
> >>>> -		fm10k_dev_rx_queue_stop(dev, i);
> >>>> +	if (dev->data->rx_queues)
> >>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> >>>> +			fm10k_dev_rx_queue_stop(dev, i);
> >>>> +}
> >>>> +
> >>>> +static void
> >>>> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
> >>>> +	int i;
> >>>> +
> >>>> +	PMD_INIT_FUNC_TRACE();
> >>>> +
> >>>> +	if (dev->data->tx_queues) {
> >>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
> >>>> +			fm10k_tx_queue_release(dev->data-
> >>>>> tx_queues[i]);
> >>>> +		rte_free(dev->data->tx_queues);
> >>>> +		dev->data->tx_queues = NULL;
> >>> The memory for dev->data->tx_queues  is not allocated in the fm10k
> >>> PMD, so it should probably not be released here.
> >>> I have submitted a patch today for rte_eth_dev.c  to do this.
> >>> /dev/patchwork/patch/5829/
> >>>
> >>>> +		dev->data->nb_tx_queues = 0;
> >>>> +	}
> >>>> +
> >>>> +	if (dev->data->rx_queues) {
> >>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
> >>>> +			fm10k_rx_queue_release(dev->data-
> >>>>> rx_queues[i]);
> >>>> +		rte_free(dev->data->rx_queues);
> >>>> +		dev->data->rx_queues = NULL;
> >>> The memory for dev->data->rx_queues  is not allocated in the fm10k
> >>> PMD, so it should probably not be released here.
> >>> I have submitted a patch today for rte_eth_dev.c  to do this.
> >>> /dev/patchwork/patch/5829/
> >> Is it a good idea?  What about to close the port for twice at a time?
> >> I think it is better to do it in rte_eth_dev_close(), I will give the comments to
> >> you.
> >>
> >> Thanks,
> >> Michael
> > Hi Michael,
> > Could you take a look at the comments on http://dpdk.org/dev/patchwork/patch/5829/
> 
> Hi, Bernard
> 
> I have give comments on it.
> 
> > The consensus is that memory should be freed in the component that allocated it.
> > In my pmd hotplug patches I have used a flag to ensure that dev_close is not called twice.
> > In the e1000 patch I have added a stopped flag to struct e1000_adapter.
> > http://dpdk.org/dev/patchwork/patch/5655/
> 
> 
> 
> I reviewed your patch about ixgbe and fvl before. But forget e1000.
> 
> In my logic, when dev->data->rx_queues is NULL, that means this device
> has been closed before. What else, we even do not care about whether it
> has been closed or not, when close() function be called, all memory
> should be freed if exist am I right?
> 
> So, check  dev->data->rx_queues whether it is NULL will be recommend in
> close function, only this could avoid unsafe situations for pointer.


It seems you are mixing 2 things there:
Contents of  dev->data->rx_queues[] is mamanged by each PMD, and yes should be alloced and freed by each PMD.
dev->data->rx_queues[] itself is allocated/reallocated and should be freed by rte_ethdev layer.
PMD, in theory, simly doesn't know how it was allocated.
Konstantin


> 
> Thanks,
> Michael
> > Regards,
> >
> > Bernard.
> >
> > <snip>
> >
> >
> >

Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port

[Qiu, Michael]

On 2015/6/29 19:08, Ananyev, Konstantin wrote:
> Hi Michael,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiu, Michael
>> Sent: Monday, June 29, 2015 10:21 AM
>> To: Iremonger, Bernard; dev@dpdk.org
>> Cc: He, Shaopeng
>> Subject: Re: [dpdk-dev] [PATCH 1/2 v4] fm10k: Free queues when close port
>>
>> On 6/29/2015 4:57 PM, Iremonger, Bernard wrote:
>>>> -----Original Message-----
>>>> From: Qiu, Michael
>>>> Sent: Monday, June 29, 2015 9:17 AM
>>>> To: Iremonger, Bernard; dev@dpdk.org
>>>> Cc: Chen, Jing D; He, Shaopeng
>>>> Subject: Re: [PATCH 1/2 v4] fm10k: Free queues when close port
>>>>
>>>> On 6/26/2015 7:02 PM, Iremonger, Bernard wrote:
>>>>>> -----Original Message-----
>>>>>> From: Qiu, Michael
>>>>>> Sent: Friday, June 26, 2015 9:30 AM
>>>>>> To: dev@dpdk.org
>>>>>> Cc: Chen, Jing D; He, Shaopeng; Iremonger, Bernard; Qiu, Michael
>>>>>> Subject: [PATCH 1/2 v4] fm10k: Free queues when close port
>>>>>>
>>>>>> When close a port, lots of memory should be released, such as
>>>>>> software rings, queues, etc.
>>>>>>
>>>>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>>>>>> ---
>>>>> Hi Michael,
>>>>>
>>>>> There are 2 comments inline
>>>>>
>>>>>>  drivers/net/fm10k/fm10k_ethdev.c | 37
>>>>>> +++++++++++++++++++++++++++++++++----
>>>>>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
>>>>>> b/drivers/net/fm10k/fm10k_ethdev.c
>>>>>> index 406c350..eba7228 100644
>>>>>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>>>>>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>>>>>> @@ -65,6 +65,8 @@ static void
>>>>>>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool
>>>>>> add); static void  fm10k_MACVLAN_remove_all(struct rte_eth_dev
>>>> *dev);
>>>>>> +static void fm10k_tx_queue_release(void *queue); static void
>>>>>> +fm10k_rx_queue_release(void *queue);
>>>>>>
>>>>>>  static void
>>>>>>  fm10k_mbx_initlock(struct fm10k_hw *hw) @@ -809,11 +811,37 @@
>>>>>> fm10k_dev_stop(struct rte_eth_dev *dev)
>>>>>>
>>>>>>  	PMD_INIT_FUNC_TRACE();
>>>>>>
>>>>>> -	for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>> -		fm10k_dev_tx_queue_stop(dev, i);
>>>>>> +	if (dev->data->tx_queues)
>>>>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>> +			fm10k_dev_tx_queue_stop(dev, i);
>>>>>>
>>>>>> -	for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>> -		fm10k_dev_rx_queue_stop(dev, i);
>>>>>> +	if (dev->data->rx_queues)
>>>>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>> +			fm10k_dev_rx_queue_stop(dev, i);
>>>>>> +}
>>>>>> +
>>>>>> +static void
>>>>>> +fm10k_dev_queue_release(struct rte_eth_dev *dev) {
>>>>>> +	int i;
>>>>>> +
>>>>>> +	PMD_INIT_FUNC_TRACE();
>>>>>> +
>>>>>> +	if (dev->data->tx_queues) {
>>>>>> +		for (i = 0; i < dev->data->nb_tx_queues; i++)
>>>>>> +			fm10k_tx_queue_release(dev->data-
>>>>>>> tx_queues[i]);
>>>>>> +		rte_free(dev->data->tx_queues);
>>>>>> +		dev->data->tx_queues = NULL;
>>>>> The memory for dev->data->tx_queues  is not allocated in the fm10k
>>>>> PMD, so it should probably not be released here.
>>>>> I have submitted a patch today for rte_eth_dev.c  to do this.
>>>>> /dev/patchwork/patch/5829/
>>>>>
>>>>>> +		dev->data->nb_tx_queues = 0;
>>>>>> +	}
>>>>>> +
>>>>>> +	if (dev->data->rx_queues) {
>>>>>> +		for (i = 0; i < dev->data->nb_rx_queues; i++)
>>>>>> +			fm10k_rx_queue_release(dev->data-
>>>>>>> rx_queues[i]);
>>>>>> +		rte_free(dev->data->rx_queues);
>>>>>> +		dev->data->rx_queues = NULL;
>>>>> The memory for dev->data->rx_queues  is not allocated in the fm10k
>>>>> PMD, so it should probably not be released here.
>>>>> I have submitted a patch today for rte_eth_dev.c  to do this.
>>>>> /dev/patchwork/patch/5829/
>>>> Is it a good idea?  What about to close the port for twice at a time?
>>>> I think it is better to do it in rte_eth_dev_close(), I will give the comments to
>>>> you.
>>>>
>>>> Thanks,
>>>> Michael
>>> Hi Michael,
>>> Could you take a look at the comments on http://dpdk.org/dev/patchwork/patch/5829/
>> Hi, Bernard
>>
>> I have give comments on it.
>>
>>> The consensus is that memory should be freed in the component that allocated it.
>>> In my pmd hotplug patches I have used a flag to ensure that dev_close is not called twice.
>>> In the e1000 patch I have added a stopped flag to struct e1000_adapter.
>>> http://dpdk.org/dev/patchwork/patch/5655/
>>
>>
>> I reviewed your patch about ixgbe and fvl before. But forget e1000.
>>
>> In my logic, when dev->data->rx_queues is NULL, that means this device
>> has been closed before. What else, we even do not care about whether it
>> has been closed or not, when close() function be called, all memory
>> should be freed if exist am I right?
>>
>> So, check  dev->data->rx_queues whether it is NULL will be recommend in
>> close function, only this could avoid unsafe situations for pointer.
>
> It seems you are mixing 2 things there:
> Contents of  dev->data->rx_queues[] is mamanged by each PMD, and yes should be alloced and freed by each PMD.
> dev->data->rx_queues[] itself is allocated/reallocated and should be freed by rte_ethdev layer.
> PMD, in theory, simly doesn't know how it was allocated.

I really do not mix these, what I mean is when you try to release the

dev->data->rx_queues you must confirm two things:
1.  dev->data->rx_queues is not NULL
2.  dev->data->rx_queues[i] is already freed

Also I agree dev->data->rx_queues is better freed in rte_ethdev layer.

And I will remove free dev->data->rx_queues memory in PMD, and based on bernard's new patch.
 
Also in PMD, to check dev->data->rx_queues to see if could release dev->data->rx_queues[i] when close()

Thanks,
Michael

> Konstantin
>
>
>> Thanks,
>> Michael
>>> Regards,
>>>
>>> Bernard.
>>>
>>> <snip>
>>>
>>>
>>>
>

[dpdk-dev] [PATCH 2/2 v4] fm10k: Add hotplug support for fm10k

[Michael Qiu]

Add hotplug support for fm10k.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 97 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index eba7228..0f55686 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1710,6 +1710,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)
 }
 
 static void
+fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
+
+	/* Disable misc causes */
+	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
+				FM10K_EIMR_DISABLE(THI_FAULT) |
+				FM10K_EIMR_DISABLE(FUM_FAULT) |
+				FM10K_EIMR_DISABLE(MAILBOX) |
+				FM10K_EIMR_DISABLE(SWITCHREADY) |
+				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
+				FM10K_EIMR_DISABLE(SRAMERROR) |
+				FM10K_EIMR_DISABLE(VFLR));
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
+static void
 fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 {
 	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -1727,6 +1757,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 	FM10K_WRITE_FLUSH(hw);
 }
 
+static void
+fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	/* Only INT 0 available, other 15 are reserved. */
+	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
 static int
 fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
 {
@@ -2177,6 +2223,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+eth_fm10k_dev_uninit(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	PMD_INIT_FUNC_TRACE();
+
+	/* only uninitialize in the primary process */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	/* safe to close dev here */
+	fm10k_dev_close(dev);
+
+	dev->dev_ops = NULL;
+	dev->rx_pkt_burst = NULL;
+	dev->tx_pkt_burst = NULL;
+
+	/* disable uio/vfio intr */
+	rte_intr_disable(&(dev->pci_dev->intr_handle));
+
+	/*PF/VF has different interrupt handling mechanism */
+	if (hw->mac.type == fm10k_mac_pf) {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_pf(dev);
+
+		/* unregister callback func to eal lib */
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_pf, (void *)dev);
+	} else {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_vf(dev);
+
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_vf, (void *)dev);
+	}
+
+	/* free mac memory */
+	if (dev->data->mac_addrs) {
+		rte_free(dev->data->mac_addrs);
+		dev->data->mac_addrs = NULL;
+	}
+
+	memset(hw, 0, sizeof(*hw));
+
+	return 0;
+}
+
 /*
  * The set of PCI devices this driver supports. This driver will enable both PF
  * and SRIOV-VF devices.
@@ -2192,9 +2286,10 @@ static struct eth_driver rte_pmd_fm10k = {
 	.pci_drv = {
 		.name = "rte_pmd_fm10k",
 		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 	},
 	.eth_dev_init = eth_fm10k_dev_init,
+	.eth_dev_uninit = eth_fm10k_dev_uninit,
 	.dev_private_size = sizeof(struct fm10k_adapter),
 };
 
-- 
1.9.3

[dpdk-dev] [PATCH 0/2 v5] Enable hotplug support for fm10k

[Michael Qiu]

Hotplug feature is supported in EAL, this patch set is to enable
this feature in driver side.

change log:
	v5 --> v4:
	remove rte queue memory release in PMD level
	v4 --> v3:
	rebase code.
	v3 --> v2:
	reset queue numbers to zero.
	v2 --> v1:
	remove __rte_unused flag

Michael Qiu (2):
  fm10k: Free queues when close port
  fm10k: Add hotplug support for fm10k

 drivers/net/fm10k/fm10k_ethdev.c | 134 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 129 insertions(+), 5 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH 1/2 v5] fm10k: Free queues when close port

[Michael Qiu]

When close a port, lots of memory should be released,
such as software rings, queues, etc.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index c1a2069..b10c546 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -65,6 +65,8 @@ static void
 fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add);
 static void
 fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev);
+static void fm10k_tx_queue_release(void *queue);
+static void fm10k_rx_queue_release(void *queue);
 
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)
@@ -809,11 +811,31 @@ fm10k_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	for (i = 0; i < dev->data->nb_tx_queues; i++)
-		fm10k_dev_tx_queue_stop(dev, i);
+	if (dev->data->tx_queues)
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_dev_tx_queue_stop(dev, i);
 
-	for (i = 0; i < dev->data->nb_rx_queues; i++)
-		fm10k_dev_rx_queue_stop(dev, i);
+	if (dev->data->rx_queues)
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_dev_rx_queue_stop(dev, i);
+}
+
+static void
+fm10k_dev_queue_release(struct rte_eth_dev *dev)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dev->data->tx_queues) {
+		for (i = 0; i < dev->data->nb_tx_queues; i++)
+			fm10k_tx_queue_release(dev->data->tx_queues[i]);
+	}
+
+	if (dev->data->rx_queues) {
+		for (i = 0; i < dev->data->nb_rx_queues; i++)
+			fm10k_rx_queue_release(dev->data->rx_queues[i]);
+	}
 }
 
 static void
@@ -828,6 +850,7 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
+	fm10k_dev_queue_release(dev);
 	fm10k_stop_hw(hw);
 }
 
-- 
1.9.3

[dpdk-dev] [PATCH 2/2 v5] fm10k: Add hotplug support for fm10k

[Michael Qiu]

Add hotplug support for fm10k.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 97 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index b10c546..1d056f3 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1712,6 +1712,36 @@ fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)
 }
 
 static void
+fm10k_dev_disable_intr_pf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM), int_map);
+	FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR), int_map);
+
+	/* Disable misc causes */
+	FM10K_WRITE_REG(hw, FM10K_EIMR, FM10K_EIMR_DISABLE(PCA_FAULT) |
+				FM10K_EIMR_DISABLE(THI_FAULT) |
+				FM10K_EIMR_DISABLE(FUM_FAULT) |
+				FM10K_EIMR_DISABLE(MAILBOX) |
+				FM10K_EIMR_DISABLE(SWITCHREADY) |
+				FM10K_EIMR_DISABLE(SWITCHNOTREADY) |
+				FM10K_EIMR_DISABLE(SRAMERROR) |
+				FM10K_EIMR_DISABLE(VFLR));
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
+static void
 fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 {
 	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -1729,6 +1759,22 @@ fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
 	FM10K_WRITE_FLUSH(hw);
 }
 
+static void
+fm10k_dev_disable_intr_vf(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t int_map = FM10K_INT_MAP_DISABLE;
+
+	int_map |= 0;
+
+	/* Only INT 0 available, other 15 are reserved. */
+	FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
+
+	/* Disable ITR 0 */
+	FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_MASK_SET);
+	FM10K_WRITE_FLUSH(hw);
+}
+
 static int
 fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
 {
@@ -2179,6 +2225,54 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+eth_fm10k_dev_uninit(struct rte_eth_dev *dev)
+{
+	struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	PMD_INIT_FUNC_TRACE();
+
+	/* only uninitialize in the primary process */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	/* safe to close dev here */
+	fm10k_dev_close(dev);
+
+	dev->dev_ops = NULL;
+	dev->rx_pkt_burst = NULL;
+	dev->tx_pkt_burst = NULL;
+
+	/* disable uio/vfio intr */
+	rte_intr_disable(&(dev->pci_dev->intr_handle));
+
+	/*PF/VF has different interrupt handling mechanism */
+	if (hw->mac.type == fm10k_mac_pf) {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_pf(dev);
+
+		/* unregister callback func to eal lib */
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_pf, (void *)dev);
+	} else {
+		/* disable interrupt */
+		fm10k_dev_disable_intr_vf(dev);
+
+		rte_intr_callback_unregister(&(dev->pci_dev->intr_handle),
+			fm10k_dev_interrupt_handler_vf, (void *)dev);
+	}
+
+	/* free mac memory */
+	if (dev->data->mac_addrs) {
+		rte_free(dev->data->mac_addrs);
+		dev->data->mac_addrs = NULL;
+	}
+
+	memset(hw, 0, sizeof(*hw));
+
+	return 0;
+}
+
 /*
  * The set of PCI devices this driver supports. This driver will enable both PF
  * and SRIOV-VF devices.
@@ -2194,9 +2288,10 @@ static struct eth_driver rte_pmd_fm10k = {
 	.pci_drv = {
 		.name = "rte_pmd_fm10k",
 		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 	},
 	.eth_dev_init = eth_fm10k_dev_init,
+	.eth_dev_uninit = eth_fm10k_dev_uninit,
 	.dev_private_size = sizeof(struct fm10k_adapter),
 };
 
-- 
1.9.3

Re: [dpdk-dev] [PATCH 0/2 v5] Enable hotplug support for fm10k

[Thomas Monjalon]

> Michael Qiu (2):
>   fm10k: Free queues when close port
>   fm10k: Add hotplug support for fm10k

Applied, thanks