[PATCH 1/7] net/nfp: fix resource leak for device initialization

[PATCH 1/7] net/nfp: fix resource leak for device initialization

[Chaoyong He]

Fix the resource leak problem in the abnormal logic of device
initialize function.

Fixes: f26e82397f6d ("net/nfp: implement xstats")
Fixes: 547137405be7 ("net/nfp: initialize IPsec related content")
Cc: james.hershaw@corigine.com
Cc: chang.miao@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index f02caf8056..25feb8e394 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -590,9 +590,6 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 
 		net_hw->mac_stats = net_hw->mac_stats_bar;
 	} else {
-		if (pf_dev->ctrl_bar == NULL)
-			return -ENODEV;
-
 		/* Use port offset in pf ctrl_bar for this ports control bar */
 		hw->ctrl_bar = pf_dev->ctrl_bar + (port * NFP_NET_CFG_BAR_SZ);
 		net_hw->mac_stats = app_fw_nic->ports[0]->mac_stats_bar +
@@ -604,18 +601,19 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 
 	err = nfp_net_common_init(pci_dev, net_hw);
 	if (err != 0)
-		return err;
+		goto free_area;
 
 	err = nfp_net_tlv_caps_parse(eth_dev);
 	if (err != 0) {
 		PMD_INIT_LOG(ERR, "Failed to parser TLV caps");
 		return err;
+		goto free_area;
 	}
 
 	err = nfp_ipsec_init(eth_dev);
 	if (err != 0) {
 		PMD_INIT_LOG(ERR, "Failed to init IPsec module");
-		return err;
+		goto free_area;
 	}
 
 	nfp_net_ethdev_ops_mount(net_hw, eth_dev);
@@ -625,7 +623,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	if (net_hw->eth_xstats_base == NULL) {
 		PMD_INIT_LOG(ERR, "no memory for xstats base values on device %s!",
 				pci_dev->device.name);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto ipsec_exit;
 	}
 
 	/* Work out where in the BAR the queues start. */
@@ -655,7 +654,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	eth_dev->data->mac_addrs = rte_zmalloc("mac_addr", RTE_ETHER_ADDR_LEN, 0);
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR, "Failed to space for MAC address");
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto xstats_free;
 	}
 
 	nfp_net_pf_read_mac(app_fw_nic, port);
@@ -693,6 +693,16 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	nfp_net_stats_reset(eth_dev);
 
 	return 0;
+
+xstats_free:
+	rte_free(net_hw->eth_xstats_base);
+ipsec_exit:
+	nfp_ipsec_uninit(eth_dev);
+free_area:
+	if (net_hw->mac_stats_area != NULL)
+		nfp_cpp_area_release_free(net_hw->mac_stats_area);
+
+	return err;
 }
 
 #define DEFAULT_FW_PATH       "/lib/firmware/netronome"
-- 
2.39.1

[PATCH 2/7] net/nfp: fix resource leak for CoreNIC firmware

[Chaoyong He]

Fix the resource leak problem in the logic of CoreNIC firmware
application.

Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 25feb8e394..38ee1c399a 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -310,6 +310,18 @@ nfp_net_keepalive_stop(struct nfp_multi_pf *multi_pf)
 	rte_eal_alarm_cancel(nfp_net_beat_timer, (void *)multi_pf);
 }
 
+static void
+nfp_net_uninit(struct rte_eth_dev *eth_dev)
+{
+	struct nfp_net_hw *net_hw;
+
+	net_hw = eth_dev->data->dev_private;
+	rte_free(net_hw->eth_xstats_base);
+	nfp_ipsec_uninit(eth_dev);
+	if (net_hw->mac_stats_area != NULL)
+		nfp_cpp_area_release_free(net_hw->mac_stats_area);
+}
+
 /* Reset and stop device. The device can not be restarted. */
 static int
 nfp_net_close(struct rte_eth_dev *dev)
@@ -1130,12 +1142,11 @@ nfp_init_app_fw_nic(struct nfp_pf_dev *pf_dev,
 				app_fw_nic->ports[id]->eth_dev != NULL) {
 			struct rte_eth_dev *tmp_dev;
 			tmp_dev = app_fw_nic->ports[id]->eth_dev;
-			nfp_ipsec_uninit(tmp_dev);
+			nfp_net_uninit(tmp_dev);
 			rte_eth_dev_release_port(tmp_dev);
-			app_fw_nic->ports[id] = NULL;
 		}
 	}
-	nfp_cpp_area_free(pf_dev->ctrl_area);
+	nfp_cpp_area_release_free(pf_dev->ctrl_area);
 app_cleanup:
 	rte_free(app_fw_nic);
 
-- 
2.39.1

[PATCH 3/7] net/nfp: fix resource leak for PF initialization

[Chaoyong He]

Fix the resource leak problem in the abnormal logic of PF initialize
function.

Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Fixes: 8ba461d1eecc ("net/nfp: introduce keepalive mechanism for multiple PF")
Cc: peng.zhang@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 38ee1c399a..bb0ddf3d54 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1326,12 +1326,13 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
 	return 0;
 
 hwqueues_cleanup:
-	nfp_cpp_area_free(pf_dev->qc_area);
+	nfp_cpp_area_release_free(pf_dev->qc_area);
 sym_tbl_cleanup:
 	free(sym_tbl);
 fw_cleanup:
 	nfp_fw_unload(cpp);
 	nfp_net_keepalive_stop(&pf_dev->multi_pf);
+	nfp_net_keepalive_uninit(&pf_dev->multi_pf);
 eth_table_cleanup:
 	free(nfp_eth_table);
 hwinfo_cleanup:
-- 
2.39.1

[PATCH 4/7] net/nfp: fix resource leak for flower firmware

[Chaoyong He]

Fix the resource leak problem in the logic of flower firmware
application.

Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 .../net/nfp/flower/nfp_flower_representor.c   | 89 ++++++++++++++++++-
 .../net/nfp/flower/nfp_flower_representor.h   |  1 +
 2 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index 0f0e63aae0..7212d9e024 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -291,6 +291,43 @@ nfp_flower_repr_tx_burst(void *tx_queue,
 	return sent;
 }
 
+static int
+nfp_flower_repr_uninit(struct rte_eth_dev *eth_dev)
+{
+	struct nfp_flower_representor *repr;
+
+	repr = eth_dev->data->dev_private;
+	rte_ring_free(repr->ring);
+
+	return 0;
+}
+
+static int
+nfp_flower_pf_repr_uninit(__rte_unused struct rte_eth_dev *eth_dev)
+{
+	return 0;
+}
+
+static void
+nfp_flower_repr_free(struct nfp_flower_representor *repr,
+		enum nfp_repr_type repr_type)
+{
+	switch (repr_type) {
+	case NFP_REPR_TYPE_PHYS_PORT:
+		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_repr_uninit);
+		break;
+	case NFP_REPR_TYPE_PF:
+		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_pf_repr_uninit);
+		break;
+	case NFP_REPR_TYPE_VF:
+		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_repr_uninit);
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "Unsupported repr port type.");
+		break;
+	}
+}
+
 static const struct eth_dev_ops nfp_flower_pf_repr_dev_ops = {
 	.dev_infos_get        = nfp_flower_repr_dev_infos_get,
 
@@ -410,6 +447,7 @@ nfp_flower_pf_repr_init(struct rte_eth_dev *eth_dev,
 
 	repr->app_fw_flower->pf_repr = repr;
 	repr->app_fw_flower->pf_hw->eth_dev = eth_dev;
+	repr->eth_dev = eth_dev;
 
 	return 0;
 }
@@ -501,6 +539,8 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
 		app_fw_flower->vf_reprs[index] = repr;
 	}
 
+	repr->eth_dev = eth_dev;
+
 	return 0;
 
 mac_cleanup:
@@ -511,6 +551,35 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
 	return ret;
 }
 
+static void
+nfp_flower_repr_free_all(struct nfp_app_fw_flower *app_fw_flower)
+{
+	uint32_t i;
+	struct nfp_flower_representor *repr;
+
+	for (i = 0; i < MAX_FLOWER_VFS; i++) {
+		repr = app_fw_flower->vf_reprs[i];
+		if (repr != NULL) {
+			nfp_flower_repr_free(repr, NFP_REPR_TYPE_VF);
+			app_fw_flower->vf_reprs[i] = NULL;
+		}
+	}
+
+	for (i = 0; i < NFP_MAX_PHYPORTS; i++) {
+		repr = app_fw_flower->phy_reprs[i];
+		if (repr != NULL) {
+			nfp_flower_repr_free(repr, NFP_REPR_TYPE_PHYS_PORT);
+			app_fw_flower->phy_reprs[i] = NULL;
+		}
+	}
+
+	repr = app_fw_flower->pf_repr;
+	if (repr != NULL) {
+		nfp_flower_repr_free(repr, NFP_REPR_TYPE_PF);
+		app_fw_flower->pf_repr = NULL;
+	}
+}
+
 static int
 nfp_flower_repr_alloc(struct nfp_app_fw_flower *app_fw_flower)
 {
@@ -585,7 +654,7 @@ nfp_flower_repr_alloc(struct nfp_app_fw_flower *app_fw_flower)
 	}
 
 	if (i < app_fw_flower->num_phyport_reprs)
-		return ret;
+		goto repr_free;
 
 	/*
 	 * Now allocate eth_dev's for VF representors.
@@ -614,9 +683,14 @@ nfp_flower_repr_alloc(struct nfp_app_fw_flower *app_fw_flower)
 	}
 
 	if (i < app_fw_flower->num_vf_reprs)
-		return ret;
+		goto repr_free;
 
 	return 0;
+
+repr_free:
+	nfp_flower_repr_free_all(app_fw_flower);
+
+	return ret;
 }
 
 int
@@ -635,7 +709,7 @@ nfp_flower_repr_create(struct nfp_app_fw_flower *app_fw_flower)
 
 	/* Allocate a switch domain for the flower app */
 	if (app_fw_flower->switch_domain_id == RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID &&
-			rte_eth_switch_domain_alloc(&app_fw_flower->switch_domain_id)) {
+			rte_eth_switch_domain_alloc(&app_fw_flower->switch_domain_id) != 0) {
 		PMD_INIT_LOG(WARNING, "failed to allocate switch domain for device");
 	}
 
@@ -677,8 +751,15 @@ nfp_flower_repr_create(struct nfp_app_fw_flower *app_fw_flower)
 	ret = nfp_flower_repr_alloc(app_fw_flower);
 	if (ret != 0) {
 		PMD_INIT_LOG(ERR, "representors allocation failed");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto domain_free;
 	}
 
 	return 0;
+
+domain_free:
+	if (rte_eth_switch_domain_free(app_fw_flower->switch_domain_id) != 0)
+		PMD_INIT_LOG(WARNING, "failed to free switch domain for device");
+
+	return ret;
 }
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.h b/drivers/net/nfp/flower/nfp_flower_representor.h
index bcb4c3cdb5..8053617562 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.h
+++ b/drivers/net/nfp/flower/nfp_flower_representor.h
@@ -20,6 +20,7 @@ struct nfp_flower_representor {
 	struct rte_ring *ring;
 	struct rte_eth_link link;
 	struct rte_eth_stats repr_stats;
+	struct rte_eth_dev *eth_dev;
 };
 
 int nfp_flower_repr_create(struct nfp_app_fw_flower *app_fw_flower);
-- 
2.39.1

[PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware

[Chaoyong He]

Fix the resource leak problem in the exit logic of CoreNIC firmware.

Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c     | 94 ++++++++++++++++++++++++--------
 drivers/net/nfp/nfp_net_common.h |  1 +
 2 files changed, 73 insertions(+), 22 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index bb0ddf3d54..5c5e658671 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -322,6 +322,54 @@ nfp_net_uninit(struct rte_eth_dev *eth_dev)
 		nfp_cpp_area_release_free(net_hw->mac_stats_area);
 }
 
+static void
+nfp_cleanup_port_app_fw_nic(struct nfp_pf_dev *pf_dev,
+		uint8_t id)
+{
+	struct rte_eth_dev *eth_dev;
+	struct nfp_app_fw_nic *app_fw_nic;
+
+	app_fw_nic = pf_dev->app_fw_priv;
+	if (app_fw_nic->ports[id] != NULL) {
+		eth_dev = app_fw_nic->ports[id]->eth_dev;
+		if (eth_dev != NULL)
+			nfp_net_uninit(eth_dev);
+
+		app_fw_nic->ports[id] = NULL;
+	}
+}
+
+static void
+nfp_uninit_app_fw_nic(struct nfp_pf_dev *pf_dev)
+{
+	nfp_cpp_area_release_free(pf_dev->ctrl_area);
+	rte_free(pf_dev->app_fw_priv);
+}
+
+void
+nfp_pf_uninit(struct nfp_pf_dev *pf_dev)
+{
+	nfp_cpp_area_release_free(pf_dev->qc_area);
+	free(pf_dev->sym_tbl);
+	if (pf_dev->multi_pf.enabled) {
+		nfp_net_keepalive_stop(&pf_dev->multi_pf);
+		nfp_net_keepalive_uninit(&pf_dev->multi_pf);
+	}
+	free(pf_dev->nfp_eth_table);
+	free(pf_dev->hwinfo);
+	nfp_cpp_free(pf_dev->cpp);
+	rte_free(pf_dev);
+}
+
+static int
+nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev)
+{
+	free(pf_dev->sym_tbl);
+	rte_free(pf_dev);
+
+	return 0;
+}
+
 /* Reset and stop device. The device can not be restarted. */
 static int
 nfp_net_close(struct rte_eth_dev *dev)
@@ -333,14 +381,25 @@ nfp_net_close(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev;
 	struct nfp_app_fw_nic *app_fw_nic;
 
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return 0;
-
 	hw = dev->data->dev_private;
 	pf_dev = hw->pf_dev;
 	pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
 
+	/*
+	 * In secondary process, a released eth device can be found by its name
+	 * in shared memory.
+	 * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
+	 * eth device has been released.
+	 */
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		if (dev->state == RTE_ETH_DEV_UNUSED)
+			return 0;
+
+		nfp_pf_secondary_uninit(pf_dev);
+		return 0;
+	}
+
 	/*
 	 * We assume that the DPDK application is stopping all the
 	 * threads/queues before calling the device close function.
@@ -351,16 +410,17 @@ nfp_net_close(struct rte_eth_dev *dev)
 	nfp_net_close_tx_queue(dev);
 	nfp_net_close_rx_queue(dev);
 
-	/* Clear ipsec */
-	nfp_ipsec_uninit(dev);
-
 	/* Cancel possible impending LSC work here before releasing the port */
 	rte_eal_alarm_cancel(nfp_net_dev_interrupt_delayed_handler, (void *)dev);
 
 	/* Only free PF resources after all physical ports have been closed */
 	/* Mark this port as unused and free device priv resources */
 	nn_cfg_writeb(&hw->super, NFP_NET_CFG_LSC, 0xff);
-	app_fw_nic->ports[hw->idx] = NULL;
+
+	if (pf_dev->app_fw_id != NFP_APP_FW_CORE_NIC)
+		return -EINVAL;
+
+	nfp_cleanup_port_app_fw_nic(pf_dev, hw->idx);
 
 	for (i = 0; i < app_fw_nic->total_phyports; i++) {
 		id = nfp_function_id_get(pf_dev, i);
@@ -370,26 +430,16 @@ nfp_net_close(struct rte_eth_dev *dev)
 			return 0;
 	}
 
-	/* Now it is safe to free all PF resources */
-	PMD_INIT_LOG(INFO, "Freeing PF resources");
-	if (pf_dev->multi_pf.enabled) {
-		nfp_net_keepalive_stop(&pf_dev->multi_pf);
-		nfp_net_keepalive_uninit(&pf_dev->multi_pf);
-	}
-	nfp_cpp_area_free(pf_dev->ctrl_area);
-	nfp_cpp_area_free(pf_dev->qc_area);
-	free(pf_dev->hwinfo);
-	free(pf_dev->sym_tbl);
-	nfp_cpp_free(pf_dev->cpp);
-	rte_free(app_fw_nic);
-	rte_free(pf_dev);
-
+	/* Enable in nfp_net_start() */
 	rte_intr_disable(pci_dev->intr_handle);
 
-	/* Unregister callback func from eal lib */
+	/* Register in nfp_net_init() */
 	rte_intr_callback_unregister(pci_dev->intr_handle,
 			nfp_net_dev_interrupt_handler, (void *)dev);
 
+	nfp_uninit_app_fw_nic(pf_dev);
+	nfp_pf_uninit(pf_dev);
+
 	return 0;
 }
 
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index 30fea7ae02..ded491cbdc 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -272,6 +272,7 @@ int nfp_net_flow_ctrl_get(struct rte_eth_dev *dev,
 		struct rte_eth_fc_conf *fc_conf);
 int nfp_net_flow_ctrl_set(struct rte_eth_dev *dev,
 		struct rte_eth_fc_conf *fc_conf);
+void nfp_pf_uninit(struct nfp_pf_dev *pf_dev);
 
 #define NFP_PRIV_TO_APP_FW_NIC(app_fw_priv)\
 	((struct nfp_app_fw_nic *)app_fw_priv)
-- 
2.39.1

Re: [PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware

[Ferruh Yigit]

On 11/30/2023 8:52 AM, Chaoyong He wrote:
> Fix the resource leak problem in the exit logic of CoreNIC firmware.
> 
> Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>

<...>

> +static int
> +nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev)
> +{
> +	free(pf_dev->sym_tbl);
> +	rte_free(pf_dev);
> +
> +	return 0;
> +}
> +
>  /* Reset and stop device. The device can not be restarted. */
>  static int
>  nfp_net_close(struct rte_eth_dev *dev)
> @@ -333,14 +381,25 @@ nfp_net_close(struct rte_eth_dev *dev)
>  	struct rte_pci_device *pci_dev;
>  	struct nfp_app_fw_nic *app_fw_nic;
>  
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> -		return 0;
> -
>  	hw = dev->data->dev_private;
>  	pf_dev = hw->pf_dev;
>  	pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
>  
> +	/*
> +	 * In secondary process, a released eth device can be found by its name
> +	 * in shared memory.
> +	 * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
> +	 * eth device has been released.
> +	 */
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> +		if (dev->state == RTE_ETH_DEV_UNUSED)
> +			return 0;
> +
> +		nfp_pf_secondary_uninit(pf_dev);
> +		return 0;
> +	}
> +
>

Mostly expectation is secondary process doesn't free shared resources,
but init and free done by primary process.

When there are multiple secondaries active, and if one of them closes
the port, will system behave properly? Can you please double check above
logic?

RE: [PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware

[Chaoyong He]

> On 11/30/2023 8:52 AM, Chaoyong He wrote:
> > Fix the resource leak problem in the exit logic of CoreNIC firmware.
> >
> > Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Long Wu <long.wu@corigine.com>
> > Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> 
> <...>
> 
> > +static int
> > +nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev) {
> > +	free(pf_dev->sym_tbl);
> > +	rte_free(pf_dev);
> > +
> > +	return 0;
> > +}
> > +
> >  /* Reset and stop device. The device can not be restarted. */  static
> > int  nfp_net_close(struct rte_eth_dev *dev) @@ -333,14 +381,25 @@
> > nfp_net_close(struct rte_eth_dev *dev)
> >  	struct rte_pci_device *pci_dev;
> >  	struct nfp_app_fw_nic *app_fw_nic;
> >
> > -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > -		return 0;
> > -
> >  	hw = dev->data->dev_private;
> >  	pf_dev = hw->pf_dev;
> >  	pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> >  	app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
> >
> > +	/*
> > +	 * In secondary process, a released eth device can be found by its
> name
> > +	 * in shared memory.
> > +	 * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
> > +	 * eth device has been released.
> > +	 */
> > +	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> > +		if (dev->state == RTE_ETH_DEV_UNUSED)
> > +			return 0;
> > +
> > +		nfp_pf_secondary_uninit(pf_dev);
> > +		return 0;
> > +	}
> > +
> >
> 
> Mostly expectation is secondary process doesn't free shared resources, but
> init and free done by primary process.

I agree.
Maybe the comment here make reader a little confused.
But the `nfp_pf_secondary_uninit()` does not free any shared resources, it only free two memory which private to each secondary process.

> When there are multiple secondaries active, and if one of them closes the port,
> will system behave properly? Can you please double check above logic?

Yes, the system behave properly.

Re: [PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware

[Ferruh Yigit]

On 12/1/2023 3:00 AM, Chaoyong He wrote:
>> On 11/30/2023 8:52 AM, Chaoyong He wrote:
>>> Fix the resource leak problem in the exit logic of CoreNIC firmware.
>>>
>>> Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
>>> Reviewed-by: Long Wu <long.wu@corigine.com>
>>> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
>>
>> <...>
>>
>>> +static int
>>> +nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev) {
>>> +	free(pf_dev->sym_tbl);
>>> +	rte_free(pf_dev);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  /* Reset and stop device. The device can not be restarted. */  static
>>> int  nfp_net_close(struct rte_eth_dev *dev) @@ -333,14 +381,25 @@
>>> nfp_net_close(struct rte_eth_dev *dev)
>>>  	struct rte_pci_device *pci_dev;
>>>  	struct nfp_app_fw_nic *app_fw_nic;
>>>
>>> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>>> -		return 0;
>>> -
>>>  	hw = dev->data->dev_private;
>>>  	pf_dev = hw->pf_dev;
>>>  	pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>>>  	app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
>>>
>>> +	/*
>>> +	 * In secondary process, a released eth device can be found by its
>> name
>>> +	 * in shared memory.
>>> +	 * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
>>> +	 * eth device has been released.
>>> +	 */
>>> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
>>> +		if (dev->state == RTE_ETH_DEV_UNUSED)
>>> +			return 0;
>>> +
>>> +		nfp_pf_secondary_uninit(pf_dev);
>>> +		return 0;
>>> +	}
>>> +
>>>
>>
>> Mostly expectation is secondary process doesn't free shared resources, but
>> init and free done by primary process.
> 
> I agree.
> Maybe the comment here make reader a little confused.
> But the `nfp_pf_secondary_uninit()` does not free any shared resources, it only free two memory which private to each secondary process.
> 

What freed is not process private, it is in the shared memory:

 	hw = dev->data->dev_private;
 	pf_dev = hw->pf_dev;
	rte_free(pf_dev);


And when there are multiple secondaries, one of them frees `pf_dev`, how
this is not effecting others that may use `pf_dev`?


>> When there are multiple secondaries active, and if one of them closes the port,
>> will system behave properly? Can you please double check above logic?
> 
> Yes, the system behave properly.
> 

RE: [PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware

[Chaoyong He]

> On 12/1/2023 3:00 AM, Chaoyong He wrote:
> >> On 11/30/2023 8:52 AM, Chaoyong He wrote:
> >>> Fix the resource leak problem in the exit logic of CoreNIC firmware.
> >>>
> >>> Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> >>> Reviewed-by: Long Wu <long.wu@corigine.com>
> >>> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> >>
> >> <...>
> >>
> >>> +static int
> >>> +nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev) {
> >>> +	free(pf_dev->sym_tbl);
> >>> +	rte_free(pf_dev);
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>>  /* Reset and stop device. The device can not be restarted. */
> >>> static int  nfp_net_close(struct rte_eth_dev *dev) @@ -333,14
> >>> +381,25 @@ nfp_net_close(struct rte_eth_dev *dev)
> >>>  	struct rte_pci_device *pci_dev;
> >>>  	struct nfp_app_fw_nic *app_fw_nic;
> >>>
> >>> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >>> -		return 0;
> >>> -
> >>>  	hw = dev->data->dev_private;
> >>>  	pf_dev = hw->pf_dev;
> >>>  	pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> >>>  	app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
> >>>
> >>> +	/*
> >>> +	 * In secondary process, a released eth device can be found by its
> >> name
> >>> +	 * in shared memory.
> >>> +	 * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
> >>> +	 * eth device has been released.
> >>> +	 */
> >>> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> >>> +		if (dev->state == RTE_ETH_DEV_UNUSED)
> >>> +			return 0;
> >>> +
> >>> +		nfp_pf_secondary_uninit(pf_dev);
> >>> +		return 0;
> >>> +	}
> >>> +
> >>>
> >>
> >> Mostly expectation is secondary process doesn't free shared
> >> resources, but init and free done by primary process.
> >
> > I agree.
> > Maybe the comment here make reader a little confused.
> > But the `nfp_pf_secondary_uninit()` does not free any shared resources, it
> only free two memory which private to each secondary process.
> >
> 
> What freed is not process private, it is in the shared memory:
> 
>  	hw = dev->data->dev_private;
>  	pf_dev = hw->pf_dev;
> 	rte_free(pf_dev);
> 
> 
> And when there are multiple secondaries, one of them frees `pf_dev`, how
> this is not effecting others that may use `pf_dev`?

Oh, I see what you mean now.
I will a v2 patch to fix this.
Thanks.

> 
> >> When there are multiple secondaries active, and if one of them closes
> >> the port, will system behave properly? Can you please double check above
> logic?
> >
> > Yes, the system behave properly.
> >

[PATCH 6/7] net/nfp: fix resource leak for exit of flower firmware

[Chaoyong He]

Fix the resource leak problem in the exit logic of flower firmware.

Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower.c           | 73 ++++---------------
 drivers/net/nfp/flower/nfp_flower.h           |  1 +
 .../net/nfp/flower/nfp_flower_representor.c   | 64 ++++++++++++++++
 3 files changed, 80 insertions(+), 58 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index 6b523d98b0..3698a3d4aa 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -82,63 +82,6 @@ nfp_flower_pf_start(struct rte_eth_dev *dev)
 	return 0;
 }
 
-/* Reset and stop device. The device can not be restarted. */
-static int
-nfp_flower_pf_close(struct rte_eth_dev *dev)
-{
-	uint16_t i;
-	struct nfp_net_hw *hw;
-	struct nfp_pf_dev *pf_dev;
-	struct nfp_net_txq *this_tx_q;
-	struct nfp_net_rxq *this_rx_q;
-	struct nfp_flower_representor *repr;
-	struct nfp_app_fw_flower *app_fw_flower;
-
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return 0;
-
-	repr = dev->data->dev_private;
-	hw = repr->app_fw_flower->pf_hw;
-	pf_dev = hw->pf_dev;
-	app_fw_flower = NFP_PRIV_TO_APP_FW_FLOWER(pf_dev->app_fw_priv);
-
-	nfp_mtr_priv_uninit(pf_dev);
-
-	/*
-	 * We assume that the DPDK application is stopping all the
-	 * threads/queues before calling the device close function.
-	 */
-	nfp_net_disable_queues(dev);
-
-	/* Clear queues */
-	for (i = 0; i < dev->data->nb_tx_queues; i++) {
-		this_tx_q = dev->data->tx_queues[i];
-		nfp_net_reset_tx_queue(this_tx_q);
-	}
-
-	for (i = 0; i < dev->data->nb_rx_queues; i++) {
-		this_rx_q = dev->data->rx_queues[i];
-		nfp_net_reset_rx_queue(this_rx_q);
-	}
-
-	/* Cancel possible impending LSC work here before releasing the port */
-	rte_eal_alarm_cancel(nfp_net_dev_interrupt_delayed_handler, (void *)dev);
-
-	nn_cfg_writeb(&hw->super, NFP_NET_CFG_LSC, 0xff);
-
-	/* Now it is safe to free all PF resources */
-	PMD_DRV_LOG(INFO, "Freeing PF resources");
-	nfp_cpp_area_free(pf_dev->ctrl_area);
-	nfp_cpp_area_free(pf_dev->qc_area);
-	free(pf_dev->hwinfo);
-	free(pf_dev->sym_tbl);
-	nfp_cpp_free(pf_dev->cpp);
-	rte_free(app_fw_flower);
-	rte_free(pf_dev);
-
-	return 0;
-}
-
 static const struct eth_dev_ops nfp_flower_pf_vnic_ops = {
 	.dev_infos_get          = nfp_net_infos_get,
 	.link_update            = nfp_net_link_update,
@@ -146,7 +89,6 @@ static const struct eth_dev_ops nfp_flower_pf_vnic_ops = {
 
 	.dev_start              = nfp_flower_pf_start,
 	.dev_stop               = nfp_net_stop,
-	.dev_close              = nfp_flower_pf_close,
 };
 
 static inline struct nfp_flower_representor *
@@ -858,6 +800,21 @@ nfp_init_app_fw_flower(struct nfp_pf_dev *pf_dev,
 	return ret;
 }
 
+void
+nfp_uninit_app_fw_flower(struct nfp_pf_dev *pf_dev)
+{
+	struct nfp_app_fw_flower *app_fw_flower;
+
+	app_fw_flower = pf_dev->app_fw_priv;
+	nfp_flower_cleanup_ctrl_vnic(app_fw_flower->ctrl_hw);
+	nfp_cpp_area_free(app_fw_flower->ctrl_hw->ctrl_area);
+	nfp_cpp_area_free(pf_dev->ctrl_area);
+	rte_free(app_fw_flower->pf_hw);
+	nfp_mtr_priv_uninit(pf_dev);
+	nfp_flow_priv_uninit(pf_dev);
+	rte_free(app_fw_flower);
+}
+
 int
 nfp_secondary_init_app_fw_flower(struct nfp_pf_dev *pf_dev)
 {
diff --git a/drivers/net/nfp/flower/nfp_flower.h b/drivers/net/nfp/flower/nfp_flower.h
index 6f27c06acc..8393de66c5 100644
--- a/drivers/net/nfp/flower/nfp_flower.h
+++ b/drivers/net/nfp/flower/nfp_flower.h
@@ -106,6 +106,7 @@ nfp_flower_support_decap_v2(const struct nfp_app_fw_flower *app_fw_flower)
 
 int nfp_init_app_fw_flower(struct nfp_pf_dev *pf_dev,
 		const struct nfp_dev_info *dev_info);
+void nfp_uninit_app_fw_flower(struct nfp_pf_dev *pf_dev);
 int nfp_secondary_init_app_fw_flower(struct nfp_pf_dev *pf_dev);
 bool nfp_flower_pf_dispatch_pkts(struct nfp_net_hw *hw,
 		struct rte_mbuf *mbuf,
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index 7212d9e024..02089d390e 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -328,12 +328,75 @@ nfp_flower_repr_free(struct nfp_flower_representor *repr,
 	}
 }
 
+/* Reset and stop device. The device can not be restarted. */
+static int
+nfp_flower_repr_dev_close(struct rte_eth_dev *dev)
+{
+	uint16_t i;
+	struct nfp_net_hw *hw;
+	struct nfp_pf_dev *pf_dev;
+	struct nfp_net_txq *this_tx_q;
+	struct nfp_net_rxq *this_rx_q;
+	struct nfp_flower_representor *repr;
+	struct nfp_app_fw_flower *app_fw_flower;
+
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	repr = dev->data->dev_private;
+	app_fw_flower = repr->app_fw_flower;
+	hw = app_fw_flower->pf_hw;
+	pf_dev = hw->pf_dev;
+
+	/*
+	 * We assume that the DPDK application is stopping all the
+	 * threads/queues before calling the device close function.
+	 */
+	nfp_net_disable_queues(dev);
+
+	/* Clear queues */
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		this_tx_q = dev->data->tx_queues[i];
+		nfp_net_reset_tx_queue(this_tx_q);
+	}
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		this_rx_q = dev->data->rx_queues[i];
+		nfp_net_reset_rx_queue(this_rx_q);
+	}
+
+	if (pf_dev->app_fw_id != NFP_APP_FW_FLOWER_NIC)
+		return -EINVAL;
+
+	nfp_flower_repr_free(repr, repr->repr_type);
+
+	for (i = 0; i < MAX_FLOWER_VFS; i++) {
+		if (app_fw_flower->vf_reprs[i] != NULL)
+			return 0;
+	}
+
+	for (i = 0; i < NFP_MAX_PHYPORTS; i++) {
+		if (app_fw_flower->phy_reprs[i] != NULL)
+			return 0;
+	}
+
+	if (app_fw_flower->pf_repr != NULL)
+		return 0;
+
+	/* Now it is safe to free all PF resources */
+	nfp_uninit_app_fw_flower(pf_dev);
+	nfp_pf_uninit(pf_dev);
+
+	return 0;
+}
+
 static const struct eth_dev_ops nfp_flower_pf_repr_dev_ops = {
 	.dev_infos_get        = nfp_flower_repr_dev_infos_get,
 
 	.dev_start            = nfp_flower_pf_start,
 	.dev_configure        = nfp_net_configure,
 	.dev_stop             = nfp_net_stop,
+	.dev_close            = nfp_flower_repr_dev_close,
 
 	.rx_queue_setup       = nfp_net_rx_queue_setup,
 	.tx_queue_setup       = nfp_net_tx_queue_setup,
@@ -356,6 +419,7 @@ static const struct eth_dev_ops nfp_flower_repr_dev_ops = {
 	.dev_start            = nfp_flower_repr_dev_start,
 	.dev_configure        = nfp_net_configure,
 	.dev_stop             = nfp_flower_repr_dev_stop,
+	.dev_close            = nfp_flower_repr_dev_close,
 
 	.rx_queue_setup       = nfp_flower_repr_rx_queue_setup,
 	.tx_queue_setup       = nfp_flower_repr_tx_queue_setup,
-- 
2.39.1

[PATCH 7/7] net/nfp: fix resource leak for VF

[Chaoyong He]

Fix the resource leak problem in the logic of VF.

Fixes: f26e82397f6d ("net/nfp: implement xstats")
Cc: james.hershaw@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev_vf.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index 7927f53403..88da593190 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -160,13 +160,17 @@ nfp_netvf_set_link_down(struct rte_eth_dev *dev __rte_unused)
 static int
 nfp_netvf_close(struct rte_eth_dev *dev)
 {
+	struct nfp_net_hw *net_hw;
 	struct rte_pci_device *pci_dev;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	net_hw = dev->data->dev_private;
 	pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 
+	rte_free(net_hw->eth_xstats_base);
+
 	/*
 	 * We assume that the DPDK application is stopping all the
 	 * threads/queues before calling the device close function.
@@ -323,7 +327,7 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR, "Failed to space for MAC address");
 		err = -ENOMEM;
-		goto dev_err_ctrl_map;
+		goto free_xstats;
 	}
 
 	nfp_read_mac(hw);
@@ -360,8 +364,8 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 
 	return 0;
 
-dev_err_ctrl_map:
-		nfp_cpp_area_free(net_hw->ctrl_area);
+free_xstats:
+	rte_free(net_hw->eth_xstats_base);
 
 	return err;
 }
-- 
2.39.1

[PATCH v2 2/8] net/nfp: fix resource leak for device initialization

[Chaoyong He]

Fix the resource leak problem in the abnormal logic of device
initialize function.

Fixes: f26e82397f6d ("net/nfp: implement xstats")
Fixes: 547137405be7 ("net/nfp: initialize IPsec related content")
Cc: james.hershaw@corigine.com
Cc: chang.miao@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 9e40bce4dd..2a80a592f2 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -597,9 +597,6 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 
 		net_hw->mac_stats = net_hw->mac_stats_bar;
 	} else {
-		if (pf_dev->ctrl_bar == NULL)
-			return -ENODEV;
-
 		/* Use port offset in pf ctrl_bar for this ports control bar */
 		hw->ctrl_bar = pf_dev->ctrl_bar + (port * NFP_NET_CFG_BAR_SZ);
 		net_hw->mac_stats = app_fw_nic->ports[0]->mac_stats_bar +
@@ -611,18 +608,19 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 
 	err = nfp_net_common_init(pci_dev, net_hw);
 	if (err != 0)
-		return err;
+		goto free_area;
 
 	err = nfp_net_tlv_caps_parse(eth_dev);
 	if (err != 0) {
 		PMD_INIT_LOG(ERR, "Failed to parser TLV caps");
 		return err;
+		goto free_area;
 	}
 
 	err = nfp_ipsec_init(eth_dev);
 	if (err != 0) {
 		PMD_INIT_LOG(ERR, "Failed to init IPsec module");
-		return err;
+		goto free_area;
 	}
 
 	nfp_net_ethdev_ops_mount(net_hw, eth_dev);
@@ -632,7 +630,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	if (net_hw->eth_xstats_base == NULL) {
 		PMD_INIT_LOG(ERR, "no memory for xstats base values on device %s!",
 				pci_dev->device.name);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto ipsec_exit;
 	}
 
 	/* Work out where in the BAR the queues start. */
@@ -662,7 +661,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	eth_dev->data->mac_addrs = rte_zmalloc("mac_addr", RTE_ETHER_ADDR_LEN, 0);
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR, "Failed to space for MAC address");
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto xstats_free;
 	}
 
 	nfp_net_pf_read_mac(app_fw_nic, port);
@@ -700,6 +700,16 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	nfp_net_stats_reset(eth_dev);
 
 	return 0;
+
+xstats_free:
+	rte_free(net_hw->eth_xstats_base);
+ipsec_exit:
+	nfp_ipsec_uninit(eth_dev);
+free_area:
+	if (net_hw->mac_stats_area != NULL)
+		nfp_cpp_area_release_free(net_hw->mac_stats_area);
+
+	return err;
 }
 
 #define DEFAULT_FW_PATH       "/lib/firmware/netronome"
-- 
2.39.1

[PATCH v2 3/8] net/nfp: fix resource leak for CoreNIC firmware

[Chaoyong He]

Fix the resource leak problem in the logic of CoreNIC firmware
application.

Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 2a80a592f2..c132e97d1a 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -317,6 +317,18 @@ nfp_net_keepalive_stop(struct nfp_multi_pf *multi_pf)
 	rte_eal_alarm_cancel(nfp_net_beat_timer, (void *)multi_pf);
 }
 
+static void
+nfp_net_uninit(struct rte_eth_dev *eth_dev)
+{
+	struct nfp_net_hw *net_hw;
+
+	net_hw = eth_dev->data->dev_private;
+	rte_free(net_hw->eth_xstats_base);
+	nfp_ipsec_uninit(eth_dev);
+	if (net_hw->mac_stats_area != NULL)
+		nfp_cpp_area_release_free(net_hw->mac_stats_area);
+}
+
 /* Reset and stop device. The device can not be restarted. */
 static int
 nfp_net_close(struct rte_eth_dev *dev)
@@ -1137,12 +1149,11 @@ nfp_init_app_fw_nic(struct nfp_pf_dev *pf_dev,
 				app_fw_nic->ports[id]->eth_dev != NULL) {
 			struct rte_eth_dev *tmp_dev;
 			tmp_dev = app_fw_nic->ports[id]->eth_dev;
-			nfp_ipsec_uninit(tmp_dev);
+			nfp_net_uninit(tmp_dev);
 			rte_eth_dev_release_port(tmp_dev);
-			app_fw_nic->ports[id] = NULL;
 		}
 	}
-	nfp_cpp_area_free(pf_dev->ctrl_area);
+	nfp_cpp_area_release_free(pf_dev->ctrl_area);
 app_cleanup:
 	rte_free(app_fw_nic);
 
-- 
2.39.1

[PATCH v2 4/8] net/nfp: fix resource leak for PF initialization

[Chaoyong He]

Fix the resource leak problem in the abnormal logic of PF initialize
function.

Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Fixes: 8ba461d1eecc ("net/nfp: introduce keepalive mechanism for multiple PF")
Cc: peng.zhang@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index c132e97d1a..6fdde105ba 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1333,12 +1333,13 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
 	return 0;
 
 hwqueues_cleanup:
-	nfp_cpp_area_free(pf_dev->qc_area);
+	nfp_cpp_area_release_free(pf_dev->qc_area);
 sym_tbl_cleanup:
 	free(sym_tbl);
 fw_cleanup:
 	nfp_fw_unload(cpp);
 	nfp_net_keepalive_stop(&pf_dev->multi_pf);
+	nfp_net_keepalive_uninit(&pf_dev->multi_pf);
 eth_table_cleanup:
 	free(nfp_eth_table);
 hwinfo_cleanup:
-- 
2.39.1

[PATCH v2 5/8] net/nfp: fix resource leak for flower firmware

[Chaoyong He]

Fix the resource leak problem in the logic of flower firmware
application.

Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 .../net/nfp/flower/nfp_flower_representor.c   | 89 ++++++++++++++++++-
 .../net/nfp/flower/nfp_flower_representor.h   |  1 +
 2 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index 0f0e63aae0..7212d9e024 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -291,6 +291,43 @@ nfp_flower_repr_tx_burst(void *tx_queue,
 	return sent;
 }
 
+static int
+nfp_flower_repr_uninit(struct rte_eth_dev *eth_dev)
+{
+	struct nfp_flower_representor *repr;
+
+	repr = eth_dev->data->dev_private;
+	rte_ring_free(repr->ring);
+
+	return 0;
+}
+
+static int
+nfp_flower_pf_repr_uninit(__rte_unused struct rte_eth_dev *eth_dev)
+{
+	return 0;
+}
+
+static void
+nfp_flower_repr_free(struct nfp_flower_representor *repr,
+		enum nfp_repr_type repr_type)
+{
+	switch (repr_type) {
+	case NFP_REPR_TYPE_PHYS_PORT:
+		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_repr_uninit);
+		break;
+	case NFP_REPR_TYPE_PF:
+		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_pf_repr_uninit);
+		break;
+	case NFP_REPR_TYPE_VF:
+		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_repr_uninit);
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "Unsupported repr port type.");
+		break;
+	}
+}
+
 static const struct eth_dev_ops nfp_flower_pf_repr_dev_ops = {
 	.dev_infos_get        = nfp_flower_repr_dev_infos_get,
 
@@ -410,6 +447,7 @@ nfp_flower_pf_repr_init(struct rte_eth_dev *eth_dev,
 
 	repr->app_fw_flower->pf_repr = repr;
 	repr->app_fw_flower->pf_hw->eth_dev = eth_dev;
+	repr->eth_dev = eth_dev;
 
 	return 0;
 }
@@ -501,6 +539,8 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
 		app_fw_flower->vf_reprs[index] = repr;
 	}
 
+	repr->eth_dev = eth_dev;
+
 	return 0;
 
 mac_cleanup:
@@ -511,6 +551,35 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
 	return ret;
 }
 
+static void
+nfp_flower_repr_free_all(struct nfp_app_fw_flower *app_fw_flower)
+{
+	uint32_t i;
+	struct nfp_flower_representor *repr;
+
+	for (i = 0; i < MAX_FLOWER_VFS; i++) {
+		repr = app_fw_flower->vf_reprs[i];
+		if (repr != NULL) {
+			nfp_flower_repr_free(repr, NFP_REPR_TYPE_VF);
+			app_fw_flower->vf_reprs[i] = NULL;
+		}
+	}
+
+	for (i = 0; i < NFP_MAX_PHYPORTS; i++) {
+		repr = app_fw_flower->phy_reprs[i];
+		if (repr != NULL) {
+			nfp_flower_repr_free(repr, NFP_REPR_TYPE_PHYS_PORT);
+			app_fw_flower->phy_reprs[i] = NULL;
+		}
+	}
+
+	repr = app_fw_flower->pf_repr;
+	if (repr != NULL) {
+		nfp_flower_repr_free(repr, NFP_REPR_TYPE_PF);
+		app_fw_flower->pf_repr = NULL;
+	}
+}
+
 static int
 nfp_flower_repr_alloc(struct nfp_app_fw_flower *app_fw_flower)
 {
@@ -585,7 +654,7 @@ nfp_flower_repr_alloc(struct nfp_app_fw_flower *app_fw_flower)
 	}
 
 	if (i < app_fw_flower->num_phyport_reprs)
-		return ret;
+		goto repr_free;
 
 	/*
 	 * Now allocate eth_dev's for VF representors.
@@ -614,9 +683,14 @@ nfp_flower_repr_alloc(struct nfp_app_fw_flower *app_fw_flower)
 	}
 
 	if (i < app_fw_flower->num_vf_reprs)
-		return ret;
+		goto repr_free;
 
 	return 0;
+
+repr_free:
+	nfp_flower_repr_free_all(app_fw_flower);
+
+	return ret;
 }
 
 int
@@ -635,7 +709,7 @@ nfp_flower_repr_create(struct nfp_app_fw_flower *app_fw_flower)
 
 	/* Allocate a switch domain for the flower app */
 	if (app_fw_flower->switch_domain_id == RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID &&
-			rte_eth_switch_domain_alloc(&app_fw_flower->switch_domain_id)) {
+			rte_eth_switch_domain_alloc(&app_fw_flower->switch_domain_id) != 0) {
 		PMD_INIT_LOG(WARNING, "failed to allocate switch domain for device");
 	}
 
@@ -677,8 +751,15 @@ nfp_flower_repr_create(struct nfp_app_fw_flower *app_fw_flower)
 	ret = nfp_flower_repr_alloc(app_fw_flower);
 	if (ret != 0) {
 		PMD_INIT_LOG(ERR, "representors allocation failed");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto domain_free;
 	}
 
 	return 0;
+
+domain_free:
+	if (rte_eth_switch_domain_free(app_fw_flower->switch_domain_id) != 0)
+		PMD_INIT_LOG(WARNING, "failed to free switch domain for device");
+
+	return ret;
 }
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.h b/drivers/net/nfp/flower/nfp_flower_representor.h
index bcb4c3cdb5..8053617562 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.h
+++ b/drivers/net/nfp/flower/nfp_flower_representor.h
@@ -20,6 +20,7 @@ struct nfp_flower_representor {
 	struct rte_ring *ring;
 	struct rte_eth_link link;
 	struct rte_eth_stats repr_stats;
+	struct rte_eth_dev *eth_dev;
 };
 
 int nfp_flower_repr_create(struct nfp_app_fw_flower *app_fw_flower);
-- 
2.39.1

[PATCH v2 6/8] net/nfp: fix resource leak for exit of CoreNIC firmware

[Chaoyong He]

Fix the resource leak problem in the exit logic of CoreNIC firmware.

Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c     | 91 +++++++++++++++++++++++++-------
 drivers/net/nfp/nfp_net_common.h |  1 +
 2 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 6fdde105ba..537b4fe792 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -329,6 +329,55 @@ nfp_net_uninit(struct rte_eth_dev *eth_dev)
 		nfp_cpp_area_release_free(net_hw->mac_stats_area);
 }
 
+static void
+nfp_cleanup_port_app_fw_nic(struct nfp_pf_dev *pf_dev,
+		uint8_t id)
+{
+	struct rte_eth_dev *eth_dev;
+	struct nfp_app_fw_nic *app_fw_nic;
+
+	app_fw_nic = pf_dev->app_fw_priv;
+	if (app_fw_nic->ports[id] != NULL) {
+		eth_dev = app_fw_nic->ports[id]->eth_dev;
+		if (eth_dev != NULL)
+			nfp_net_uninit(eth_dev);
+
+		app_fw_nic->ports[id] = NULL;
+	}
+}
+
+static void
+nfp_uninit_app_fw_nic(struct nfp_pf_dev *pf_dev)
+{
+	nfp_cpp_area_release_free(pf_dev->ctrl_area);
+	rte_free(pf_dev->app_fw_priv);
+}
+
+void
+nfp_pf_uninit(struct nfp_pf_dev *pf_dev)
+{
+	nfp_cpp_area_release_free(pf_dev->qc_area);
+	free(pf_dev->sym_tbl);
+	if (pf_dev->multi_pf.enabled) {
+		nfp_net_keepalive_stop(&pf_dev->multi_pf);
+		nfp_net_keepalive_uninit(&pf_dev->multi_pf);
+	}
+	free(pf_dev->nfp_eth_table);
+	free(pf_dev->hwinfo);
+	nfp_cpp_free(pf_dev->cpp);
+	rte_free(pf_dev);
+}
+
+static int
+nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev)
+{
+	free(pf_dev->sym_tbl);
+	nfp_cpp_free(pf_dev->cpp);
+	rte_free(pf_dev);
+
+	return 0;
+}
+
 /* Reset and stop device. The device can not be restarted. */
 static int
 nfp_net_close(struct rte_eth_dev *dev)
@@ -340,8 +389,19 @@ nfp_net_close(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev;
 	struct nfp_app_fw_nic *app_fw_nic;
 
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+	/*
+	 * In secondary process, a released eth device can be found by its name
+	 * in shared memory.
+	 * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
+	 * eth device has been released.
+	 */
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		if (dev->state == RTE_ETH_DEV_UNUSED)
+			return 0;
+
+		nfp_pf_secondary_uninit(dev->process_private);
 		return 0;
+	}
 
 	hw = dev->data->dev_private;
 	pf_dev = hw->pf_dev;
@@ -358,16 +418,17 @@ nfp_net_close(struct rte_eth_dev *dev)
 	nfp_net_close_tx_queue(dev);
 	nfp_net_close_rx_queue(dev);
 
-	/* Clear ipsec */
-	nfp_ipsec_uninit(dev);
-
 	/* Cancel possible impending LSC work here before releasing the port */
 	rte_eal_alarm_cancel(nfp_net_dev_interrupt_delayed_handler, (void *)dev);
 
 	/* Only free PF resources after all physical ports have been closed */
 	/* Mark this port as unused and free device priv resources */
 	nn_cfg_writeb(&hw->super, NFP_NET_CFG_LSC, 0xff);
-	app_fw_nic->ports[hw->idx] = NULL;
+
+	if (pf_dev->app_fw_id != NFP_APP_FW_CORE_NIC)
+		return -EINVAL;
+
+	nfp_cleanup_port_app_fw_nic(pf_dev, hw->idx);
 
 	for (i = 0; i < app_fw_nic->total_phyports; i++) {
 		id = nfp_function_id_get(pf_dev, i);
@@ -377,26 +438,16 @@ nfp_net_close(struct rte_eth_dev *dev)
 			return 0;
 	}
 
-	/* Now it is safe to free all PF resources */
-	PMD_INIT_LOG(INFO, "Freeing PF resources");
-	if (pf_dev->multi_pf.enabled) {
-		nfp_net_keepalive_stop(&pf_dev->multi_pf);
-		nfp_net_keepalive_uninit(&pf_dev->multi_pf);
-	}
-	nfp_cpp_area_free(pf_dev->ctrl_area);
-	nfp_cpp_area_free(pf_dev->qc_area);
-	free(pf_dev->hwinfo);
-	free(pf_dev->sym_tbl);
-	nfp_cpp_free(pf_dev->cpp);
-	rte_free(app_fw_nic);
-	rte_free(pf_dev);
-
+	/* Enable in nfp_net_start() */
 	rte_intr_disable(pci_dev->intr_handle);
 
-	/* Unregister callback func from eal lib */
+	/* Register in nfp_net_init() */
 	rte_intr_callback_unregister(pci_dev->intr_handle,
 			nfp_net_dev_interrupt_handler, (void *)dev);
 
+	nfp_uninit_app_fw_nic(pf_dev);
+	nfp_pf_uninit(pf_dev);
+
 	return 0;
 }
 
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index 30fea7ae02..ded491cbdc 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -272,6 +272,7 @@ int nfp_net_flow_ctrl_get(struct rte_eth_dev *dev,
 		struct rte_eth_fc_conf *fc_conf);
 int nfp_net_flow_ctrl_set(struct rte_eth_dev *dev,
 		struct rte_eth_fc_conf *fc_conf);
+void nfp_pf_uninit(struct nfp_pf_dev *pf_dev);
 
 #define NFP_PRIV_TO_APP_FW_NIC(app_fw_priv)\
 	((struct nfp_app_fw_nic *)app_fw_priv)
-- 
2.39.1

[PATCH v2 7/8] net/nfp: fix resource leak for exit of flower firmware

[Chaoyong He]

Fix the resource leak problem in the exit logic of flower firmware.

Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower.c           | 73 ++++---------------
 drivers/net/nfp/flower/nfp_flower.h           |  1 +
 .../net/nfp/flower/nfp_flower_representor.c   | 64 ++++++++++++++++
 3 files changed, 80 insertions(+), 58 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index f172c8350d..f950ae233b 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -82,63 +82,6 @@ nfp_flower_pf_start(struct rte_eth_dev *dev)
 	return 0;
 }
 
-/* Reset and stop device. The device can not be restarted. */
-static int
-nfp_flower_pf_close(struct rte_eth_dev *dev)
-{
-	uint16_t i;
-	struct nfp_net_hw *hw;
-	struct nfp_pf_dev *pf_dev;
-	struct nfp_net_txq *this_tx_q;
-	struct nfp_net_rxq *this_rx_q;
-	struct nfp_flower_representor *repr;
-	struct nfp_app_fw_flower *app_fw_flower;
-
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return 0;
-
-	repr = dev->data->dev_private;
-	hw = repr->app_fw_flower->pf_hw;
-	pf_dev = hw->pf_dev;
-	app_fw_flower = NFP_PRIV_TO_APP_FW_FLOWER(pf_dev->app_fw_priv);
-
-	nfp_mtr_priv_uninit(pf_dev);
-
-	/*
-	 * We assume that the DPDK application is stopping all the
-	 * threads/queues before calling the device close function.
-	 */
-	nfp_net_disable_queues(dev);
-
-	/* Clear queues */
-	for (i = 0; i < dev->data->nb_tx_queues; i++) {
-		this_tx_q = dev->data->tx_queues[i];
-		nfp_net_reset_tx_queue(this_tx_q);
-	}
-
-	for (i = 0; i < dev->data->nb_rx_queues; i++) {
-		this_rx_q = dev->data->rx_queues[i];
-		nfp_net_reset_rx_queue(this_rx_q);
-	}
-
-	/* Cancel possible impending LSC work here before releasing the port */
-	rte_eal_alarm_cancel(nfp_net_dev_interrupt_delayed_handler, (void *)dev);
-
-	nn_cfg_writeb(&hw->super, NFP_NET_CFG_LSC, 0xff);
-
-	/* Now it is safe to free all PF resources */
-	PMD_DRV_LOG(INFO, "Freeing PF resources");
-	nfp_cpp_area_free(pf_dev->ctrl_area);
-	nfp_cpp_area_free(pf_dev->qc_area);
-	free(pf_dev->hwinfo);
-	free(pf_dev->sym_tbl);
-	nfp_cpp_free(pf_dev->cpp);
-	rte_free(app_fw_flower);
-	rte_free(pf_dev);
-
-	return 0;
-}
-
 static const struct eth_dev_ops nfp_flower_pf_vnic_ops = {
 	.dev_infos_get          = nfp_net_infos_get,
 	.link_update            = nfp_net_link_update,
@@ -146,7 +89,6 @@ static const struct eth_dev_ops nfp_flower_pf_vnic_ops = {
 
 	.dev_start              = nfp_flower_pf_start,
 	.dev_stop               = nfp_net_stop,
-	.dev_close              = nfp_flower_pf_close,
 };
 
 static inline struct nfp_flower_representor *
@@ -858,6 +800,21 @@ nfp_init_app_fw_flower(struct nfp_pf_dev *pf_dev,
 	return ret;
 }
 
+void
+nfp_uninit_app_fw_flower(struct nfp_pf_dev *pf_dev)
+{
+	struct nfp_app_fw_flower *app_fw_flower;
+
+	app_fw_flower = pf_dev->app_fw_priv;
+	nfp_flower_cleanup_ctrl_vnic(app_fw_flower->ctrl_hw);
+	nfp_cpp_area_free(app_fw_flower->ctrl_hw->ctrl_area);
+	nfp_cpp_area_free(pf_dev->ctrl_area);
+	rte_free(app_fw_flower->pf_hw);
+	nfp_mtr_priv_uninit(pf_dev);
+	nfp_flow_priv_uninit(pf_dev);
+	rte_free(app_fw_flower);
+}
+
 int
 nfp_secondary_init_app_fw_flower(struct nfp_pf_dev *pf_dev)
 {
diff --git a/drivers/net/nfp/flower/nfp_flower.h b/drivers/net/nfp/flower/nfp_flower.h
index 6f27c06acc..8393de66c5 100644
--- a/drivers/net/nfp/flower/nfp_flower.h
+++ b/drivers/net/nfp/flower/nfp_flower.h
@@ -106,6 +106,7 @@ nfp_flower_support_decap_v2(const struct nfp_app_fw_flower *app_fw_flower)
 
 int nfp_init_app_fw_flower(struct nfp_pf_dev *pf_dev,
 		const struct nfp_dev_info *dev_info);
+void nfp_uninit_app_fw_flower(struct nfp_pf_dev *pf_dev);
 int nfp_secondary_init_app_fw_flower(struct nfp_pf_dev *pf_dev);
 bool nfp_flower_pf_dispatch_pkts(struct nfp_net_hw *hw,
 		struct rte_mbuf *mbuf,
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index 7212d9e024..02089d390e 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -328,12 +328,75 @@ nfp_flower_repr_free(struct nfp_flower_representor *repr,
 	}
 }
 
+/* Reset and stop device. The device can not be restarted. */
+static int
+nfp_flower_repr_dev_close(struct rte_eth_dev *dev)
+{
+	uint16_t i;
+	struct nfp_net_hw *hw;
+	struct nfp_pf_dev *pf_dev;
+	struct nfp_net_txq *this_tx_q;
+	struct nfp_net_rxq *this_rx_q;
+	struct nfp_flower_representor *repr;
+	struct nfp_app_fw_flower *app_fw_flower;
+
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	repr = dev->data->dev_private;
+	app_fw_flower = repr->app_fw_flower;
+	hw = app_fw_flower->pf_hw;
+	pf_dev = hw->pf_dev;
+
+	/*
+	 * We assume that the DPDK application is stopping all the
+	 * threads/queues before calling the device close function.
+	 */
+	nfp_net_disable_queues(dev);
+
+	/* Clear queues */
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		this_tx_q = dev->data->tx_queues[i];
+		nfp_net_reset_tx_queue(this_tx_q);
+	}
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		this_rx_q = dev->data->rx_queues[i];
+		nfp_net_reset_rx_queue(this_rx_q);
+	}
+
+	if (pf_dev->app_fw_id != NFP_APP_FW_FLOWER_NIC)
+		return -EINVAL;
+
+	nfp_flower_repr_free(repr, repr->repr_type);
+
+	for (i = 0; i < MAX_FLOWER_VFS; i++) {
+		if (app_fw_flower->vf_reprs[i] != NULL)
+			return 0;
+	}
+
+	for (i = 0; i < NFP_MAX_PHYPORTS; i++) {
+		if (app_fw_flower->phy_reprs[i] != NULL)
+			return 0;
+	}
+
+	if (app_fw_flower->pf_repr != NULL)
+		return 0;
+
+	/* Now it is safe to free all PF resources */
+	nfp_uninit_app_fw_flower(pf_dev);
+	nfp_pf_uninit(pf_dev);
+
+	return 0;
+}
+
 static const struct eth_dev_ops nfp_flower_pf_repr_dev_ops = {
 	.dev_infos_get        = nfp_flower_repr_dev_infos_get,
 
 	.dev_start            = nfp_flower_pf_start,
 	.dev_configure        = nfp_net_configure,
 	.dev_stop             = nfp_net_stop,
+	.dev_close            = nfp_flower_repr_dev_close,
 
 	.rx_queue_setup       = nfp_net_rx_queue_setup,
 	.tx_queue_setup       = nfp_net_tx_queue_setup,
@@ -356,6 +419,7 @@ static const struct eth_dev_ops nfp_flower_repr_dev_ops = {
 	.dev_start            = nfp_flower_repr_dev_start,
 	.dev_configure        = nfp_net_configure,
 	.dev_stop             = nfp_flower_repr_dev_stop,
+	.dev_close            = nfp_flower_repr_dev_close,
 
 	.rx_queue_setup       = nfp_flower_repr_rx_queue_setup,
 	.tx_queue_setup       = nfp_flower_repr_tx_queue_setup,
-- 
2.39.1

[PATCH v2 8/8] net/nfp: fix resource leak for VF

[Chaoyong He]

Fix the resource leak problem in the logic of VF.

Fixes: f26e82397f6d ("net/nfp: implement xstats")
Cc: james.hershaw@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev_vf.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index 7927f53403..88da593190 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -160,13 +160,17 @@ nfp_netvf_set_link_down(struct rte_eth_dev *dev __rte_unused)
 static int
 nfp_netvf_close(struct rte_eth_dev *dev)
 {
+	struct nfp_net_hw *net_hw;
 	struct rte_pci_device *pci_dev;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	net_hw = dev->data->dev_private;
 	pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 
+	rte_free(net_hw->eth_xstats_base);
+
 	/*
 	 * We assume that the DPDK application is stopping all the
 	 * threads/queues before calling the device close function.
@@ -323,7 +327,7 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR, "Failed to space for MAC address");
 		err = -ENOMEM;
-		goto dev_err_ctrl_map;
+		goto free_xstats;
 	}
 
 	nfp_read_mac(hw);
@@ -360,8 +364,8 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 
 	return 0;
 
-dev_err_ctrl_map:
-		nfp_cpp_area_free(net_hw->ctrl_area);
+free_xstats:
+	rte_free(net_hw->eth_xstats_base);
 
 	return err;
 }
-- 
2.39.1