[PATCH 22.11 0/7] backport patch series from NFP PMD

[PATCH 22.11 0/7] backport patch series from NFP PMD

[Chaoyong He]

This patch series include 7 bugfix backport patch from NFP PMD.

Chaoyong He (7):
  net/nfp: fix resource leak for CoreNIC firmware
  net/nfp: fix resource leak for flower firmware
  net/nfp: fix resource leak for exit of CoreNIC firmware
  net/nfp: fix resource leak for exit of flower firmware
  net/nfp: fix device close
  net/nfp: fix device resource freeing
  net/nfp: free switch domain ID on close

 drivers/net/nfp/flower/nfp_flower.c           |  71 ++------
 drivers/net/nfp/flower/nfp_flower.h           |   1 +
 .../net/nfp/flower/nfp_flower_representor.c   | 166 +++++++++++++++++-
 .../net/nfp/flower/nfp_flower_representor.h   |   1 +
 drivers/net/nfp/nfp_common.h                  |   1 +
 drivers/net/nfp/nfp_ethdev.c                  |  80 +++++++--
 6 files changed, 241 insertions(+), 79 deletions(-)

-- 
2.39.1

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

[Chaoyong He]

[ upstream commit 8b8f116b199e31795ecd5cedb12302f10c0ae5a4 ]

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

Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 3b70f5c..77f573c 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -920,10 +920,9 @@ nfp_init_app_fw_nic(struct nfp_pf_dev *pf_dev)
 			struct rte_eth_dev *tmp_dev;
 			tmp_dev = app_fw_nic->ports[i]->eth_dev;
 			rte_eth_dev_release_port(tmp_dev);
-			app_fw_nic->ports[i] = 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 22.11 2/7] net/nfp: fix resource leak for flower firmware

[Chaoyong He]

[ upstream commit 7c596721ae5f41d1dbab8b936a4983928d6b5603 ]

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

Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")

Signed-off-by: Chaoyong He <chaoyong.he@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 32c4574..854d117 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -524,6 +524,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,
 
@@ -640,6 +677,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;
 }
@@ -731,6 +769,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:
@@ -741,6 +781,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)
 {
@@ -816,7 +885,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.
@@ -845,9 +914,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
@@ -866,7 +940,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");
 	}
 
@@ -908,8 +982,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 685cbe4..c043214 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.h
+++ b/drivers/net/nfp/flower/nfp_flower_representor.h
@@ -34,6 +34,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

Re: [PATCH 22.11 2/7] net/nfp: fix resource leak for flower firmware

[Luca Boccassi]

On Thu, 7 Mar 2024 at 06:05, Chaoyong He <chaoyong.he@corigine.com> wrote:
>
> [ upstream commit 7c596721ae5f41d1dbab8b936a4983928d6b5603 ]
>
> Fix the resource leak problem in the logic of flower firmware
> application.
>
> Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
>
> Signed-off-by: Chaoyong He <chaoyong.he@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 32c4574..854d117 100644
> --- a/drivers/net/nfp/flower/nfp_flower_representor.c
> +++ b/drivers/net/nfp/flower/nfp_flower_representor.c
> ...
> +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;
> +               }
> +       }

Thanks for sending the series, but this causes a compiler warning on
Debian stable with gcc 12.2.0:

[8/34] Compiling C object
drivers/libtmp_rte_net_nfp.a.p/net_nfp_flower_nfp_flower_representor.c.o
../drivers/net/nfp/flower/nfp_flower_representor.c: In function
‘nfp_flower_repr_free_all’:
../drivers/net/nfp/flower/nfp_flower_representor.c:876:22: warning:
iteration 8 invokes undefined behavior
[-Waggressive-loop-optimizations]
  876 |                 repr = app_fw_flower->phy_reprs[i];
      |                 ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/nfp/flower/nfp_flower_representor.c:875:23: note:
within this loop

RE: [PATCH 22.11 2/7] net/nfp: fix resource leak for flower firmware

[Chaoyong He]

> [Some people who received this message don't often get email from
> bluca@debian.org. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Thu, 7 Mar 2024 at 06:05, Chaoyong He <chaoyong.he@corigine.com>
> wrote:
> >
> > [ upstream commit 7c596721ae5f41d1dbab8b936a4983928d6b5603 ]
> >
> > Fix the resource leak problem in the logic of flower firmware
> > application.
> >
> > Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
> >
> > Signed-off-by: Chaoyong He <chaoyong.he@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 32c4574..854d117 100644
> > --- a/drivers/net/nfp/flower/nfp_flower_representor.c
> > +++ b/drivers/net/nfp/flower/nfp_flower_representor.c
> > ...
> > +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;
> > +               }
> > +       }
> 
> Thanks for sending the series, but this causes a compiler warning on Debian
> stable with gcc 12.2.0:
> 
> [8/34] Compiling C object
> drivers/libtmp_rte_net_nfp.a.p/net_nfp_flower_nfp_flower_representor.c.o
> ../drivers/net/nfp/flower/nfp_flower_representor.c: In function
> ‘nfp_flower_repr_free_all’:
> ../drivers/net/nfp/flower/nfp_flower_representor.c:876:22: warning:
> iteration 8 invokes undefined behavior
> [-Waggressive-loop-optimizations]
>   876 |                 repr = app_fw_flower->phy_reprs[i];
>       |                 ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/net/nfp/flower/nfp_flower_representor.c:875:23: note:
> within this loop

Oh, sorry for this.
In line 875, the 'NFP_MAX_PHYPORTS' should be 'MAX_FLOWER_PHYPORTS' actually.

RE: [PATCH 22.11 2/7] net/nfp: fix resource leak for flower firmware

[Chaoyong He]

> > On Thu, 7 Mar 2024 at 06:05, Chaoyong He <chaoyong.he@corigine.com>
> > wrote:
> > >
> > > [ upstream commit 7c596721ae5f41d1dbab8b936a4983928d6b5603 ]
> > >
> > > Fix the resource leak problem in the logic of flower firmware
> > > application.
> > >
> > > Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
> > >
> > > Signed-off-by: Chaoyong He <chaoyong.he@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 32c4574..854d117 100644
> > > --- a/drivers/net/nfp/flower/nfp_flower_representor.c
> > > +++ b/drivers/net/nfp/flower/nfp_flower_representor.c
> > > ...
> > > +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;
> > > +               }
> > > +       }
> >
> > Thanks for sending the series, but this causes a compiler warning on
> > Debian stable with gcc 12.2.0:
> >
> > [8/34] Compiling C object
> > drivers/libtmp_rte_net_nfp.a.p/net_nfp_flower_nfp_flower_representor.c
> > .o
> > ../drivers/net/nfp/flower/nfp_flower_representor.c: In function
> > ‘nfp_flower_repr_free_all’:
> > ../drivers/net/nfp/flower/nfp_flower_representor.c:876:22: warning:
> > iteration 8 invokes undefined behavior
> > [-Waggressive-loop-optimizations]
> >   876 |                 repr = app_fw_flower->phy_reprs[i];
> >       |                 ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ../drivers/net/nfp/flower/nfp_flower_representor.c:875:23: note:
> > within this loop
> 
> Oh, sorry for this.
> In line 875, the 'NFP_MAX_PHYPORTS' should be 'MAX_FLOWER_PHYPORTS'
> actually.

And I will send a v2 patch series to fix that, thanks.

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

[Chaoyong He]

[ upstream commit 66d5f53d3e1beebf31de3b3b2e15371ffe322866 ]

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

Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/nfp_common.h |  1 +
 drivers/net/nfp/nfp_ethdev.c | 77 ++++++++++++++++++++++++++++--------
 2 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h
index d1a07f5..5b5c0aa 100644
--- a/drivers/net/nfp/nfp_common.h
+++ b/drivers/net/nfp/nfp_common.h
@@ -450,6 +450,7 @@ void nfp_net_close_tx_queue(struct rte_eth_dev *dev);
 int nfp_net_set_vxlan_port(struct nfp_net_hw *hw, size_t idx, uint16_t port);
 int nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name);
 void nfp_net_irq_unmask(struct rte_eth_dev *dev);
+void nfp_pf_uninit(struct nfp_pf_dev *pf_dev);
 
 #define NFP_NET_DEV_PRIVATE_TO_HW(adapter)\
 	(&((struct nfp_net_adapter *)adapter)->hw)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 77f573c..68fd67a 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -264,6 +264,45 @@ nfp_net_set_link_down(struct rte_eth_dev *dev)
 					      hw->nfp_idx, 0);
 }
 
+static void
+nfp_cleanup_port_app_fw_nic(struct nfp_pf_dev *pf_dev,
+		uint8_t id)
+{
+	struct nfp_app_fw_nic *app_fw_nic;
+
+	app_fw_nic = pf_dev->app_fw_priv;
+	if (app_fw_nic->ports[id] != NULL)
+		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->hwqueues_area);
+	free(pf_dev->sym_tbl);
+	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)
@@ -274,8 +313,19 @@ nfp_net_close(struct rte_eth_dev *dev)
 	struct nfp_app_fw_nic *app_fw_nic;
 	int i;
 
-	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;
+	}
 
 	PMD_INIT_LOG(DEBUG, "Close");
 
@@ -303,7 +353,11 @@ nfp_net_close(struct rte_eth_dev *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, 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++) {
 		/* Check to see if ports are still in use */
@@ -311,26 +365,15 @@ 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");
-	nfp_cpp_area_free(pf_dev->ctrl_area);
-	nfp_cpp_area_free(pf_dev->hwqueues_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);
 
-	/*
-	 * The ixgbe PMD disables the pcie master on the
-	 * device. The i40e does not...
-	 */
+	nfp_uninit_app_fw_nic(pf_dev);
+	nfp_pf_uninit(pf_dev);
 
 	return 0;
 }
-- 
2.39.1

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

[Chaoyong He]

[ upstream commit a256a1227dbe0eac576a42b6c336ce55b7713109 ]

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

Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower.c           | 70 ++++---------------
 drivers/net/nfp/flower/nfp_flower.h           |  1 +
 .../net/nfp/flower/nfp_flower_representor.c   | 64 +++++++++++++++++
 3 files changed, 79 insertions(+), 56 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index 5896d20..28be1b9 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -182,61 +182,6 @@ nfp_flower_pf_stop(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 = (struct nfp_flower_representor *)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);
-
-	/*
-	 * We assume that the DPDK application is stopping all the
-	 * threads/queues before calling the device close function.
-	 */
-	nfp_pf_repr_disable_queues(dev);
-
-	/* Clear queues */
-	for (i = 0; i < dev->data->nb_tx_queues; i++) {
-		this_tx_q = (struct nfp_net_txq *)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 = (struct nfp_net_rxq *)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, 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->hwqueues_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,
@@ -244,7 +189,6 @@ static const struct eth_dev_ops nfp_flower_pf_vnic_ops = {
 
 	.dev_start              = nfp_flower_pf_start,
 	.dev_stop               = nfp_flower_pf_stop,
-	.dev_close              = nfp_flower_pf_close,
 };
 
 static inline void
@@ -1221,6 +1165,20 @@ 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_flow_priv_uninit(pf_dev);
+	rte_free(app_fw_flower);
+}
+
 int
 nfp_secondary_init_app_fw_flower(struct nfp_cpp *cpp)
 {
diff --git a/drivers/net/nfp/flower/nfp_flower.h b/drivers/net/nfp/flower/nfp_flower.h
index c05a761..d82e35f 100644
--- a/drivers/net/nfp/flower/nfp_flower.h
+++ b/drivers/net/nfp/flower/nfp_flower.h
@@ -85,6 +85,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);
+void nfp_uninit_app_fw_flower(struct nfp_pf_dev *pf_dev);
 int nfp_secondary_init_app_fw_flower(struct nfp_cpp *cpp);
 uint16_t nfp_flower_pf_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		uint16_t nb_pkts);
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index 854d117..f615f02 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -561,12 +561,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_flower_repr_dev_configure,
 	.dev_stop             = nfp_flower_pf_stop,
+	.dev_close            = nfp_flower_repr_dev_close,
 
 	.rx_queue_setup       = nfp_pf_repr_rx_queue_setup,
 	.tx_queue_setup       = nfp_pf_repr_tx_queue_setup,
@@ -588,6 +651,7 @@ static const struct eth_dev_ops nfp_flower_repr_dev_ops = {
 	.dev_start            = nfp_flower_repr_dev_start,
 	.dev_configure        = nfp_flower_repr_dev_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 22.11 5/7] net/nfp: fix device close

[Chaoyong He]

[ upstream commit 243bbfa0f92f57851ae9369133256a622c79a2a0 ]

Close interface use "rte_eth_dev_destroy()" to destroy representor.
The "rte_eth_dev_destroy()" will call "rte_eth_dev_release_port()" but
the "rte_eth_dev_close()" also calls "rte_eth_dev_release_port()".
This will cause Segmentation fault.

Remove the "rte_eth_dev_destroy()" in nfp representor close interface.

Fixes: 7c596721ae5f ("net/nfp: fix resource leak for flower firmware")

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower_representor.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index f615f02..61638c8 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -547,13 +547,13 @@ nfp_flower_repr_free(struct nfp_flower_representor *repr,
 {
 	switch (repr_type) {
 	case NFP_REPR_TYPE_PHYS_PORT:
-		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_repr_uninit);
+		nfp_flower_repr_uninit(repr->eth_dev);
 		break;
 	case NFP_REPR_TYPE_PF:
-		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_pf_repr_uninit);
+		nfp_flower_pf_repr_uninit(repr->eth_dev);
 		break;
 	case NFP_REPR_TYPE_VF:
-		rte_eth_dev_destroy(repr->eth_dev, nfp_flower_repr_uninit);
+		nfp_flower_repr_uninit(repr->eth_dev);
 		break;
 	default:
 		PMD_DRV_LOG(ERR, "Unsupported repr port type.");
-- 
2.39.1

[PATCH 22.11 6/7] net/nfp: fix device resource freeing

[Chaoyong He]

[ upstream commit 09309f87358b3a538af47e31340898a15bb34d85 ]

Set the representor array to NULL to avoid that close interface
does not free some resource.

Representor array points to dev_private which is already freed by
'rte_eth_dev_release_port()', and 'nfp_flower_repr_dev_close()' requires
pointers to be NULL to proceed freeing other resources.

Fixes: 7c596721ae5f ("net/nfp: fix resource leak for flower firmware")

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower_representor.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index 61638c8..ac35a50 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -527,17 +527,30 @@ nfp_flower_repr_tx_burst(void *tx_queue,
 static int
 nfp_flower_repr_uninit(struct rte_eth_dev *eth_dev)
 {
+	uint16_t index;
 	struct nfp_flower_representor *repr;
 
 	repr = eth_dev->data->dev_private;
 	rte_ring_free(repr->ring);
 
+	if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
+		index = NFP_FLOWER_CMSG_PORT_PHYS_PORT_NUM(repr->port_id);
+		repr->app_fw_flower->phy_reprs[index] = NULL;
+	} else {
+		index = repr->vf_id;
+		repr->app_fw_flower->vf_reprs[index] = NULL;
+	}
+
 	return 0;
 }
 
 static int
-nfp_flower_pf_repr_uninit(__rte_unused struct rte_eth_dev *eth_dev)
+nfp_flower_pf_repr_uninit(struct rte_eth_dev *eth_dev)
 {
+	struct nfp_flower_representor *repr = eth_dev->data->dev_private;
+
+	repr->app_fw_flower->pf_repr = NULL;
+
 	return 0;
 }
 
-- 
2.39.1

[PATCH 22.11 7/7] net/nfp: free switch domain ID on close

[Chaoyong He]

[ upstream commit 20eaa8e2ebae621b32d740f6c0529e11d33519ff ]

Free domain id in close interface.

Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index 28be1b9..8490d17 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -1176,6 +1176,7 @@ nfp_uninit_app_fw_flower(struct nfp_pf_dev *pf_dev)
 	nfp_cpp_area_free(pf_dev->ctrl_area);
 	rte_free(app_fw_flower->pf_hw);
 	nfp_flow_priv_uninit(pf_dev);
+	rte_eth_switch_domain_free(app_fw_flower->switch_domain_id);
 	rte_free(app_fw_flower);
 }
 
-- 
2.39.1