[PATCH 0/8] net/ice: fix DCF memory leaks

[PATCH 0/8] net/ice: fix DCF memory leaks

[Anurag Mandal]

This patch series fixes memory leaks, and related error path issues, 
in the ice DCF driver.

1. The VF VSI map leaked on DCF hardware init failure, and a failed
   VLAN capability query left the interrupt callback registered.

2. A failed DCF parent adapter init leaked the dummy main VSI, and a
   failed flow init also leaked the DDP parser and flow engine state.

3. The queue config message leaked when the required RXDID was not
   supported.

4. The temporary QoS bandwidth buffers still leaked on every
   successful hierarchy commit.

5. The QoS bandwidth config saved for replay leaked on every commit,
   close and reset.

6. TM nodes and shaper profiles leaked on close when the hierarchy was
   never committed.

7. The representor info leaked when the DCF port was closed.

8. A failed probe left the DCF port and its representors behind. A
   representor type other than VF is now rejected before the DCF port
   is created.

Anurag Mandal (8):
  net/ice: fix DCF hardware init error path
  net/ice: fix DCF parent adapter init error path
  net/ice: fix DCF queue config buffer leak
  net/ice: fix DCF QoS buffer leak on commit
  net/ice: fix DCF QoS bandwidth replay config leak
  net/ice: fix DCF TM config leak on close
  net/ice: fix DCF representor info leak on close
  net/ice: fix DCF port leak on probe failure

 drivers/net/intel/ice/ice_dcf.c        | 27 ++++++++++++---
 drivers/net/intel/ice/ice_dcf_ethdev.c | 48 +++++++++++++++++++-------
 drivers/net/intel/ice/ice_dcf_parent.c |  9 +++--
 drivers/net/intel/ice/ice_dcf_sched.c  |  4 +++
 4 files changed, 68 insertions(+), 20 deletions(-)

-- 
2.25.1

[PATCH 1/8] net/ice: fix DCF hardware init error path

[Anurag Mandal]

ice_dcf_init_hw() never frees the VF VSI map on its error paths,
so it leaks when any step after getting the map fails.

Similarly, when getting the VLAN offload V2 capabilities fails,
the error path leaves the interrupt callback registered and
enabled, and leaks the QoS bandwidth config array.
As the DCF hardware structure passed to the callback is freed
with the port when probe fails, a later interrupt would use
freed memory.

Free the VSI map, and add an error label that disables the
interrupt, unregisters the callback and frees the QoS array.

Fixes: 9a7980cfdf67 ("net/ice: complete VLAN offload capability for DCF")
Fixes: 8e1cf1823191 ("net/ice: init RSS and supported RXDID in DCF")
Fixes: 7564d5509611 ("net/ice: add DCF hardware initialization")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
 drivers/net/intel/ice/ice_dcf.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/intel/ice/ice_dcf.c b/drivers/net/intel/ice/ice_dcf.c
index 976f2ed260..d87655c11d 100644
--- a/drivers/net/intel/ice/ice_dcf.c
+++ b/drivers/net/intel/ice/ice_dcf.c
@@ -853,14 +853,23 @@ ice_dcf_init_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw)
 
 	if ((hw->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN_V2) &&
 	    dcf_get_vlan_offload_caps_v2(hw))
-		goto err_rss;
+		goto err_intr;
 
 	return 0;
 
+err_intr:
+	ice_dcf_disable_irq0(hw);
+	rte_intr_disable(pci_dev->intr_handle);
+	rte_intr_callback_unregister(pci_dev->intr_handle,
+				     ice_dcf_dev_interrupt_handler, hw);
+	rte_free(hw->qos_bw_cfg);
+	hw->qos_bw_cfg = NULL;
 err_rss:
 	rte_free(hw->rss_key);
 	rte_free(hw->rss_lut);
 err_alloc:
+	rte_free(hw->vf_vsi_map);
+	hw->vf_vsi_map = NULL;
 	rte_free(hw->vf_res);
 err_api:
 	iavf_shutdown_adminq(&hw->avf);
-- 
2.25.1

[PATCH 2/8] net/ice: fix DCF parent adapter init error path

[Anurag Mandal]

ice_dcf_init_parent_adapter() allocates a dummy main VSI
first, but none of its error paths free it.

When ice_flow_init() fails, the error path also skips
ice_flow_uninit(), so the DDP parser and any flow engine
state set up before the failure are not released.

Free the main VSI on all error paths, and call
ice_flow_uninit() when flow init fails.

Fixes: 0b02c9519432 ("net/ice: handle PF initialization by DCF")
Fixes: 829c3106812d ("net/ice: enable switch flow on DCF")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
 drivers/net/intel/ice/ice_dcf_parent.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/intel/ice/ice_dcf_parent.c b/drivers/net/intel/ice/ice_dcf_parent.c
index e6398205ce..d5d37f8810 100644
--- a/drivers/net/intel/ice/ice_dcf_parent.c
+++ b/drivers/net/intel/ice/ice_dcf_parent.c
@@ -457,7 +457,7 @@ ice_dcf_init_parent_adapter(struct rte_eth_dev *eth_dev)
 	if (err) {
 		PMD_INIT_LOG(ERR, "failed to init the DCF parent hardware with error %d",
 			     err);
-		return err;
+		goto free_vsi;
 	}
 
 	if (hw->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS) {
@@ -491,7 +491,7 @@ ice_dcf_init_parent_adapter(struct rte_eth_dev *eth_dev)
 	err = ice_flow_init(parent_adapter);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Failed to initialize flow");
-		goto uninit_hw;
+		goto uninit_flow;
 	}
 
 	mac = (const struct rte_ether_addr *)hw->avf.mac.addr;
@@ -504,8 +504,13 @@ ice_dcf_init_parent_adapter(struct rte_eth_dev *eth_dev)
 
 	return 0;
 
+uninit_flow:
+	ice_flow_uninit(parent_adapter);
 uninit_hw:
 	ice_dcf_uninit_parent_hw(parent_hw);
+free_vsi:
+	rte_free(parent_adapter->pf.main_vsi);
+	parent_adapter->pf.main_vsi = NULL;
 	return err;
 }
 
-- 
2.25.1

[PATCH 3/8] net/ice: fix DCF queue config buffer leak

[Anurag Mandal]

ice_dcf_configure_queues() returns -EINVAL without
freeing the virtchnl message it allocated when the
required Rx descriptor ID (RXDID) is not supported,
so the message leaks on each failed port start.

Free it before return.

Fixes: 4b0d391f0eab ("net/ice: add queue config in DCF")
Fixes: f3fddc93de16 ("net/ice: fix build with 16-byte Rx descriptor in DCF")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
 drivers/net/intel/ice/ice_dcf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/intel/ice/ice_dcf.c b/drivers/net/intel/ice/ice_dcf.c
index d87655c11d..09fcf6454f 100644
--- a/drivers/net/intel/ice/ice_dcf.c
+++ b/drivers/net/intel/ice/ice_dcf.c
@@ -1232,6 +1232,7 @@ ice_dcf_configure_queues(struct ice_dcf_hw *hw)
 				    "Queue[%d]", vc_qp->rxq.rxdid, i);
 		} else {
 			PMD_DRV_LOG(ERR, "RXDID 16 is not supported");
+			rte_free(vc_config);
 			return -EINVAL;
 		}
 #else
@@ -1244,6 +1245,7 @@ ice_dcf_configure_queues(struct ice_dcf_hw *hw)
 					"Queue[%d]", vc_qp->rxq.rxdid, i);
 		} else {
 			PMD_DRV_LOG(ERR, "RXDID == 0 is not supported");
+			rte_free(vc_config);
 			return -EINVAL;
 		}
 #endif
-- 
2.25.1

[PATCH 4/8] net/ice: fix DCF QoS buffer leak on commit

[Anurag Mandal]

ice_dcf_hierarchy_commit() frees its temporary VF
and TC bandwidth buffers only on the error path,
so every successful commit still leaks them.

Free them on success as well.

Fixes: efd5d15a886b ("net/ice: fix memory leak in DCF QoS bandwidth config")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
 drivers/net/intel/ice/ice_dcf_sched.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/intel/ice/ice_dcf_sched.c b/drivers/net/intel/ice/ice_dcf_sched.c
index 84342241aa..de28019c5b 100644
--- a/drivers/net/intel/ice/ice_dcf_sched.c
+++ b/drivers/net/intel/ice/ice_dcf_sched.c
@@ -849,6 +849,8 @@ static int ice_dcf_hierarchy_commit(struct rte_eth_dev *dev,
 		   ICE_NONDMA_TO_NONDMA);
 
 	hw->tm_conf.committed = true;
+	free(vf_bw);
+	free(tc_bw);
 	return ret_val;
 
 fail_clear:
-- 
2.25.1

[PATCH 5/8] net/ice: fix DCF QoS bandwidth replay config leak

[Anurag Mandal]

ice_dcf_hierarchy_commit() keeps a copy of the bandwidth config
of each VF, and of the TC nodes, in qos_bw_cfg[] to replay it
after a VF reset.
Each commit allocates new copies without freeing the old ones,
and ice_dcf_uninit_hw() frees the array but not the copies, so
they also leak on close and reset.

Free the old copy before allocating a new one, and free all copies
in ice_dcf_uninit_hw().

Fixes: 931ee54072b1 ("net/ice: support QoS bandwidth config after VF reset in DCF")
Fixes: 775a25a372f5 ("net/ice: clear QoS bandwidth on DCF close")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
 drivers/net/intel/ice/ice_dcf.c       | 6 ++++++
 drivers/net/intel/ice/ice_dcf_sched.c | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/intel/ice/ice_dcf.c b/drivers/net/intel/ice/ice_dcf.c
index 09fcf6454f..c2b557d0da 100644
--- a/drivers/net/intel/ice/ice_dcf.c
+++ b/drivers/net/intel/ice/ice_dcf.c
@@ -884,6 +884,7 @@ ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw)
 {
 	struct rte_pci_device *pci_dev = RTE_CLASS_TO_BUS_DEVICE(eth_dev, *pci_dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
+	uint16_t i;
 
 	if (hw->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS)
 		if (hw->tm_conf.committed) {
@@ -919,6 +920,11 @@ ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw)
 	rte_free(hw->rss_key);
 	hw->rss_key = NULL;
 
+	if (hw->qos_bw_cfg != NULL) {
+		for (i = 0; i < hw->num_vfs; i++)
+			rte_free(hw->qos_bw_cfg[i]);
+	}
+
 	rte_free(hw->qos_bw_cfg);
 	hw->qos_bw_cfg = NULL;
 
diff --git a/drivers/net/intel/ice/ice_dcf_sched.c b/drivers/net/intel/ice/ice_dcf_sched.c
index de28019c5b..f8755c7dcb 100644
--- a/drivers/net/intel/ice/ice_dcf_sched.c
+++ b/drivers/net/intel/ice/ice_dcf_sched.c
@@ -813,6 +813,7 @@ static int ice_dcf_hierarchy_commit(struct rte_eth_dev *dev,
 		if (ret_val)
 			goto fail_clear;
 
+		rte_free(hw->qos_bw_cfg[vf_id]);
 		hw->qos_bw_cfg[vf_id] = rte_zmalloc("vf_bw_cfg", size, 0);
 		if (!hw->qos_bw_cfg[vf_id]) {
 			ret_val = ICE_ERR_NO_MEMORY;
@@ -840,6 +841,7 @@ static int ice_dcf_hierarchy_commit(struct rte_eth_dev *dev,
 		goto fail_clear;
 
 	/* store TC node bw configuration */
+	rte_free(hw->qos_bw_cfg[ICE_DCF_VFID]);
 	hw->qos_bw_cfg[ICE_DCF_VFID] = rte_zmalloc("tc_bw_cfg", size, 0);
 	if (!hw->qos_bw_cfg[ICE_DCF_VFID]) {
 		ret_val = ICE_ERR_NO_MEMORY;
-- 
2.25.1

[PATCH 6/8] net/ice: fix DCF TM config leak on close

[Anurag Mandal]

ice_dcf_uninit_hw() releases the TM nodes and shaper profiles only
when the hierarchy gets committed.
If the application adds nodes or profiles but never commits them,
or the commit fails without clearing them, they leak on close and
reset.

Only clearing the bandwidth needs a committed hierarchy,
so releasing the TM config whenever QoS is supported.

Fixes: 775a25a372f5 ("net/ice: clear QoS bandwidth on DCF close")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
 drivers/net/intel/ice/ice_dcf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/intel/ice/ice_dcf.c b/drivers/net/intel/ice/ice_dcf.c
index c2b557d0da..df4212267d 100644
--- a/drivers/net/intel/ice/ice_dcf.c
+++ b/drivers/net/intel/ice/ice_dcf.c
@@ -886,11 +886,11 @@ ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw)
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	uint16_t i;
 
-	if (hw->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS)
-		if (hw->tm_conf.committed) {
+	if (hw->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS) {
+		if (hw->tm_conf.committed)
 			ice_dcf_clear_bw(hw);
-			ice_dcf_tm_conf_uninit(eth_dev);
-		}
+		ice_dcf_tm_conf_uninit(eth_dev);
+	}
 
 	ice_dcf_disable_irq0(hw);
 	rte_intr_disable(intr_handle);
-- 
2.25.1

[PATCH 7/8] net/ice: fix DCF representor info leak on close

[Anurag Mandal]

The representor info array is only freed by ice_dcf_dev_uninit(),
which runs on device removal.
Closing the DCF port with rte_eth_dev_close() releases the port
without freeing it.

Free it in ice_dcf_dev_close().
ice_dcf_dev_reset() needs the info to survive, as init notifies
the representors again, so move the rest of close into
ice_dcf_dev_teardown() and call that from reset instead.

Fixes: c7e1a1a3bfeb ("net/ice: refactor DCF VLAN handling")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
 drivers/net/intel/ice/ice_dcf_ethdev.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/net/intel/ice/ice_dcf_ethdev.c b/drivers/net/intel/ice/ice_dcf_ethdev.c
index c78b290b0d..26c22265eb 100644
--- a/drivers/net/intel/ice/ice_dcf_ethdev.c
+++ b/drivers/net/intel/ice/ice_dcf_ethdev.c
@@ -1660,20 +1660,28 @@ ice_dcf_init_repr_info(struct ice_dcf_adapter *dcf_adapter)
 	return 0;
 }
 
-static int
-ice_dcf_dev_close(struct rte_eth_dev *dev)
+static void
+ice_dcf_dev_teardown(struct rte_eth_dev *dev)
 {
 	struct ice_dcf_adapter *adapter = dev->data->dev_private;
 
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return 0;
-
 	ice_dcf_vf_repr_notify_all(adapter, false);
 	(void)ice_dcf_dev_stop(dev);
 
 	ice_free_queues(dev);
 	ice_dcf_uninit_parent_adapter(dev);
 	ice_dcf_uninit_hw(dev, &adapter->real_hw);
+}
+
+static int
+ice_dcf_dev_close(struct rte_eth_dev *dev)
+{
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	ice_dcf_dev_teardown(dev);
+	/* representor info survives ice_dcf_dev_reset, so only release it here */
+	ice_dcf_free_repr_info(dev->data->dev_private);
 
 	return 0;
 }
@@ -1861,9 +1869,8 @@ ice_dcf_dev_reset(struct rte_eth_dev *dev)
 		ice_dcf_reset_hw(dev, hw);
 	}
 
-	ret = ice_dcf_dev_close(dev);
-	if (ret)
-		return ret;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		ice_dcf_dev_teardown(dev);
 
 	ret = ice_dcf_dev_init(dev);
 
-- 
2.25.1

[PATCH 8/8] net/ice: fix DCF port leak on probe failure

[Anurag Mandal]

When representors are requested, eth_ice_dcf_pci_probe() creates
the DCF port before handling them.
If a later step fails, such as an unsupported representor type,
an invalid VF ID or a failed representor creation, the probe
returns an error but leaves the DCF port and any representors
already created in place.
As the probe failed, the PCI bus also unmaps the device, so
these ports are leaked and cannot be closed safely.

Check the representor type before creating the DCF port, and
on later failures destroy the representors created so far and
remove the DCF port.

Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
Fixes: d654167641bf ("ethdev: introduce representor type")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
 drivers/net/intel/ice/ice_dcf_ethdev.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/net/intel/ice/ice_dcf_ethdev.c b/drivers/net/intel/ice/ice_dcf_ethdev.c
index 26c22265eb..b4e81433ba 100644
--- a/drivers/net/intel/ice/ice_dcf_ethdev.c
+++ b/drivers/net/intel/ice/ice_dcf_ethdev.c
@@ -2060,13 +2060,14 @@ eth_ice_dcf_pci_probe(__rte_unused struct rte_pci_driver *pci_drv,
 	if (ret < 0)
 		return ret;
 
+	if (eth_da.nb_representor_ports && eth_da.type != RTE_ETH_REPRESENTOR_VF)
+		return -ENOTSUP;
+
 	ret = rte_eth_dev_pci_generic_probe(pci_dev,
 					    sizeof(struct ice_dcf_adapter),
 					    ice_dcf_dev_init);
 	if (ret || !eth_da.nb_representor_ports)
 		return ret;
-	if (eth_da.type != RTE_ETH_REPRESENTOR_VF)
-		return -ENOTSUP;
 
 	dcf_ethdev = rte_eth_dev_allocated(pci_dev->device.name);
 	if (dcf_ethdev == NULL)
@@ -2075,14 +2076,14 @@ eth_ice_dcf_pci_probe(__rte_unused struct rte_pci_driver *pci_drv,
 	dcf_adapter = dcf_ethdev->data->dev_private;
 	ret = ice_dcf_init_repr_info(dcf_adapter);
 	if (ret)
-		return ret;
+		goto err_remove_dcf;
 
 	if (eth_da.nb_representor_ports > dcf_adapter->real_hw.num_vfs ||
 	    eth_da.nb_representor_ports >= RTE_MAX_ETHPORTS) {
 		PMD_DRV_LOG(ERR, "the number of port representors is too large: %u",
 			    eth_da.nb_representor_ports);
-		ice_dcf_free_repr_info(dcf_adapter);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_remove_dcf;
 	}
 
 	dcf_vsi_id = dcf_adapter->real_hw.vsi_id | VIRTCHNL_DCF_VF_VSI_VALID;
@@ -2133,6 +2134,20 @@ eth_ice_dcf_pci_probe(__rte_unused struct rte_pci_driver *pci_drv,
 		dcf_adapter->num_reprs++;
 	}
 
+	if (ret == 0)
+		return 0;
+
+	for (i = 0; i < dcf_adapter->real_hw.num_vfs; i++) {
+		struct rte_eth_dev *vf_rep_eth_dev =
+			dcf_adapter->repr_infos[i].vf_rep_eth_dev;
+
+		if (vf_rep_eth_dev != NULL)
+			rte_eth_dev_destroy(vf_rep_eth_dev, ice_dcf_vf_repr_uninit);
+	}
+
+err_remove_dcf:
+	rte_eth_dev_pci_generic_remove(pci_dev, ice_dcf_dev_uninit);
+
 	return ret;
 }
 
-- 
2.25.1