[PATCH 0/7] net/hns3: bugfix for hns3

[PATCH 0/7] net/hns3: bugfix for hns3

[Dengdui Huang]

This patchset fixes some bugs.

Dengdui Huang (7):
  net/hns3: fix variable overflow
  net/hns3: fix memory leakage in failure path
  net/hns3: fix extra waiting for link up
  net/hns3: fix memory leakage for indirect action
  net/hns3: fix incorrect failed rollback
  net/hns3: fix divide by zero
  net/hns3: fix unrelease some resources on reset case

 drivers/net/hns3/hns3_common.c |  4 +--
 drivers/net/hns3/hns3_ethdev.c | 45 ++++++++++++++++++++++------------
 drivers/net/hns3/hns3_flow.c   | 41 ++++++++++++++-----------------
 drivers/net/hns3/hns3_flow.h   |  9 +++++--
 drivers/net/hns3/hns3_regs.c   |  5 ++--
 5 files changed, 60 insertions(+), 44 deletions(-)

-- 
2.33.0

[PATCH 1/7] net/hns3: fix variable overflow

[Dengdui Huang]

The number of interrupt vector may exceed the range of uint8_t.
So hns3_unmap_rx_interrupt() should use uint16_t for 'vec' variable.

Fixes: 02a7b55657b2 ("net/hns3: support Rx interrupt")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_common.c b/drivers/net/hns3/hns3_common.c
index 25a45212be..dc70bf3cff 100644
--- a/drivers/net/hns3/hns3_common.c
+++ b/drivers/net/hns3/hns3_common.c
@@ -882,8 +882,8 @@ hns3_unmap_rx_interrupt(struct rte_eth_dev *dev)
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	struct hns3_adapter *hns = dev->data->dev_private;
 	struct hns3_hw *hw = &hns->hw;
-	uint8_t base = RTE_INTR_VEC_ZERO_OFFSET;
-	uint8_t vec = RTE_INTR_VEC_ZERO_OFFSET;
+	uint16_t base = RTE_INTR_VEC_ZERO_OFFSET;
+	uint16_t vec = RTE_INTR_VEC_ZERO_OFFSET;
 	uint16_t q_id;
 
 	if (dev->data->dev_conf.intr_conf.rxq == 0)
-- 
2.33.0

[PATCH 2/7] net/hns3: fix memory leakage in failure path

[Dengdui Huang]

When the hns3_dfx_reg_fetch_data() function returns from processing
failure, cmd_descs is not freed, which leads to leakage.
This patch fit it.

By the way, this patch uses calloc to apply for heap memory instead
of hugepage memory.

Fixes: ef1fbd355451 ("net/hns3: add more registers to dump")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_regs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c
index 8a6ddbfe8c..5c74f9ae2e 100644
--- a/drivers/net/hns3/hns3_regs.c
+++ b/drivers/net/hns3/hns3_regs.c
@@ -1270,7 +1270,7 @@ hns3_get_dfx_regs(struct hns3_hw *hw, struct rte_dev_reg_info *regs, uint32_t mo
 	for (i = 0; i < opcode_num; i++)
 		max_bd_num = RTE_MAX(bd_num_list[i], max_bd_num);
 
-	cmd_descs = rte_zmalloc(NULL, sizeof(*cmd_descs) * max_bd_num, 0);
+	cmd_descs = calloc(max_bd_num, sizeof(*cmd_descs));
 	if (cmd_descs == NULL)
 		return -ENOMEM;
 
@@ -1290,13 +1290,14 @@ hns3_get_dfx_regs(struct hns3_hw *hw, struct rte_dev_reg_info *regs, uint32_t mo
 		if (regs_num !=  hns3_reg_lists[i].entry_num) {
 			hns3_err(hw, "Query register number differ from the list for module %s!",
 				 hns3_get_name_by_module(i));
+			free(cmd_descs);
 			return -EINVAL;
 		}
 		hns3_fill_dfx_regs_name(hw, regs, hns3_reg_lists[i].reg_list, regs_num);
 		regs->length += regs_num;
 		data += regs_num;
 	}
-	rte_free(cmd_descs);
+	free(cmd_descs);
 
 	return ret;
 }
-- 
2.33.0

[PATCH 3/7] net/hns3: fix extra waiting for link up

[Dengdui Huang]

If the link auto-negotiation of the NIC is disabled,
or the flow control auto-negotiation is not supported,
it's unnecessary to wait for link establishment.

Fixes: 1f411e31a826 ("net/hns3: support flow control autoneg for copper port")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 9f7119b734..20ad249b8b 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5308,12 +5308,6 @@ hns3_get_current_fc_mode(struct rte_eth_dev *dev)
 	struct hns3_pf *pf = HNS3_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct hns3_mac *mac = &hw->mac;
 
-	/*
-	 * When the flow control mode is obtained, the device may not complete
-	 * auto-negotiation. It is necessary to wait for link establishment.
-	 */
-	(void)hns3_dev_link_update(dev, 1);
-
 	/*
 	 * If the link auto-negotiation of the nic is disabled, or the flow
 	 * control auto-negotiation is not supported, the forced flow control
@@ -5322,6 +5316,12 @@ hns3_get_current_fc_mode(struct rte_eth_dev *dev)
 	if (mac->link_autoneg == 0 || !pf->support_fc_autoneg)
 		return hw->requested_fc_mode;
 
+	/*
+	 * When the flow control mode is obtained, the device may not complete
+	 * auto-negotiation. It is necessary to wait for link establishment.
+	 */
+	(void)hns3_dev_link_update(dev, 1);
+
 	return hns3_get_autoneg_fc_mode(hw);
 }
 
-- 
2.33.0

[PATCH 4/7] net/hns3: fix memory leakage for indirect action

[Dengdui Huang]

Currently, when the application creates an indirect action,
the hns3 driver allocates a memory for the structure
rte_flow_action_handle and returns this structure pointer to
application. When the application invokes the destroy function
to destroy the indirect action, the driver releases the memory.

However, when the application destroys all flows by using the
flush function before destroying the indirect action, the memory
is not released. This patch fix it by using uint64_t instead of
rte_flow_action_handle* to store indirect action avoids memory
leakage.

Fixes: fdfcb94d8fb3 ("net/hns3: support indirect counter flow action")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_flow.c | 41 ++++++++++++++++--------------------
 drivers/net/hns3/hns3_flow.h |  9 ++++++--
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index 266934b45b..c0238d2bfa 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -473,19 +473,20 @@ hns3_handle_action_indirect(struct rte_eth_dev *dev,
 			    struct hns3_fdir_rule *rule,
 			    struct rte_flow_error *error)
 {
-	const struct rte_flow_action_handle *indir = action->conf;
+	struct rte_flow_action_handle indir;
 
-	if (indir->indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT)
+	indir.val64 = (uint64_t)action->conf;
+	if (indir.indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT)
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 				action, "Invalid indirect type");
 
-	if (hns3_counter_lookup(dev, indir->counter_id) == NULL)
+	if (hns3_counter_lookup(dev, indir.counter_id) == NULL)
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 				action, "Counter id not exist");
 
-	rule->act_cnt.id = indir->counter_id;
+	rule->act_cnt.id = indir.counter_id;
 	rule->flags |= (HNS3_RULE_FLAG_COUNTER | HNS3_RULE_FLAG_COUNTER_INDIR);
 
 	return 0;
@@ -2726,20 +2727,12 @@ hns3_flow_action_create(struct rte_eth_dev *dev,
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct hns3_pf *pf = HNS3_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	const struct rte_flow_action_count *act_count;
-	struct rte_flow_action_handle *handle = NULL;
+	struct rte_flow_action_handle handle;
 	struct hns3_flow_counter *counter;
 
 	if (hns3_check_indir_action(conf, action, error))
 		return NULL;
 
-	handle = rte_zmalloc("hns3 action handle",
-			     sizeof(struct rte_flow_action_handle), 0);
-	if (handle == NULL) {
-		rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,
-				   NULL, "Failed to allocate action memory");
-		return NULL;
-	}
-
 	pthread_mutex_lock(&hw->flows_lock);
 
 	act_count = (const struct rte_flow_action_count *)action->conf;
@@ -2762,15 +2755,14 @@ hns3_flow_action_create(struct rte_eth_dev *dev,
 	}
 
 	counter->indirect = true;
-	handle->indirect_type = HNS3_INDIRECT_ACTION_TYPE_COUNT;
-	handle->counter_id = counter->id;
+	handle.indirect_type = HNS3_INDIRECT_ACTION_TYPE_COUNT;
+	handle.counter_id = counter->id;
 
 	pthread_mutex_unlock(&hw->flows_lock);
-	return handle;
+	return (struct rte_flow_action_handle *)handle.val64;
 
 err_exit:
 	pthread_mutex_unlock(&hw->flows_lock);
-	rte_free(handle);
 	return NULL;
 }
 
@@ -2780,18 +2772,20 @@ hns3_flow_action_destroy(struct rte_eth_dev *dev,
 			 struct rte_flow_error *error)
 {
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_flow_action_handle indir;
 	struct hns3_flow_counter *counter;
 
 	pthread_mutex_lock(&hw->flows_lock);
 
-	if (handle->indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT) {
+	indir.val64 = (uint64_t)handle;
+	if (indir.indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT) {
 		pthread_mutex_unlock(&hw->flows_lock);
 		return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					handle, "Invalid indirect type");
 	}
 
-	counter = hns3_counter_lookup(dev, handle->counter_id);
+	counter = hns3_counter_lookup(dev, indir.counter_id);
 	if (counter == NULL) {
 		pthread_mutex_unlock(&hw->flows_lock);
 		return rte_flow_error_set(error, EINVAL,
@@ -2806,8 +2800,7 @@ hns3_flow_action_destroy(struct rte_eth_dev *dev,
 				handle, "Counter id in use");
 	}
 
-	(void)hns3_counter_release(dev, handle->counter_id);
-	rte_free(handle);
+	(void)hns3_counter_release(dev, indir.counter_id);
 
 	pthread_mutex_unlock(&hw->flows_lock);
 	return 0;
@@ -2820,12 +2813,14 @@ hns3_flow_action_query(struct rte_eth_dev *dev,
 		 struct rte_flow_error *error)
 {
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_flow_action_handle indir;
 	struct rte_flow flow;
 	int ret;
 
 	pthread_mutex_lock(&hw->flows_lock);
 
-	if (handle->indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT) {
+	indir.val64 = (uint64_t)handle;
+	if (indir.indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT) {
 		pthread_mutex_unlock(&hw->flows_lock);
 		return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION_CONF,
@@ -2833,7 +2828,7 @@ hns3_flow_action_query(struct rte_eth_dev *dev,
 	}
 
 	memset(&flow, 0, sizeof(flow));
-	flow.counter_id = handle->counter_id;
+	flow.counter_id = indir.counter_id;
 	ret = hns3_counter_query(dev, &flow,
 				 (struct rte_flow_query_count *)data, error);
 	pthread_mutex_unlock(&hw->flows_lock);
diff --git a/drivers/net/hns3/hns3_flow.h b/drivers/net/hns3/hns3_flow.h
index 1b49673f11..612890391d 100644
--- a/drivers/net/hns3/hns3_flow.h
+++ b/drivers/net/hns3/hns3_flow.h
@@ -50,8 +50,13 @@ enum {
 };
 
 struct rte_flow_action_handle {
-	int indirect_type;
-	uint32_t counter_id;
+	union {
+		uint64_t val64;
+		struct {
+			int indirect_type;
+			uint32_t counter_id;
+		};
+	};
 };
 
 union hns3_filter_conf {
-- 
2.33.0

[PATCH 5/7] net/hns3: fix incorrect failed rollback

[Dengdui Huang]

When the port is started, if the Tx queue fails to be started,
the map interrupt should be rolled back.

Fixes: fdfde7a4a0f8 ("net/hns3: fix mbuf leakage")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 20ad249b8b..2b56b6e44e 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5122,7 +5122,7 @@ hns3_dev_start(struct rte_eth_dev *dev)
 	 */
 	ret = hns3_start_all_txqs(dev);
 	if (ret)
-		goto map_rx_inter_err;
+		goto start_all_txqs_fail;
 
 	ret = hns3_start_all_rxqs(dev);
 	if (ret)
@@ -5155,6 +5155,8 @@ hns3_dev_start(struct rte_eth_dev *dev)
 
 start_all_rxqs_fail:
 	hns3_stop_all_txqs(dev);
+start_all_txqs_fail:
+	hns3_unmap_rx_interrupt(dev);
 map_rx_inter_err:
 	(void)hns3_do_stop(hns);
 do_start_fail:
-- 
2.33.0

[PATCH 6/7] net/hns3: fix divide by zero

[Dengdui Huang]

Driver may encounter divide-by-zero if the total_tqps_num
and rss_size_max in hw structure from firmware are zero.
So add some verification to them.

Fixes: d51867db65c1 ("net/hns3: add initialization")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 2b56b6e44e..2d4e348442 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2544,6 +2544,10 @@ hns3_query_pf_resource(struct hns3_hw *hw)
 	req = (struct hns3_pf_res_cmd *)desc.data;
 	hw->total_tqps_num = rte_le_to_cpu_16(req->tqp_num) +
 			     rte_le_to_cpu_16(req->ext_tqp_num);
+	if (hw->total_tqps_num == 0) {
+		PMD_INIT_LOG(ERR, "the total tqp number of the port is 0.");
+		return -EINVAL;
+	}
 	ret = hns3_get_pf_max_tqp_num(hw);
 	if (ret)
 		return ret;
@@ -2795,6 +2799,7 @@ hns3_check_media_type(struct hns3_hw *hw, uint8_t media_type)
 static int
 hns3_get_board_configuration(struct hns3_hw *hw)
 {
+#define HNS3_RSS_SIZE_MAX_DEFAULT	64
 	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
 	struct hns3_pf *pf = &hns->pf;
 	struct hns3_cfg cfg;
@@ -2813,6 +2818,11 @@ hns3_get_board_configuration(struct hns3_hw *hw)
 
 	hw->mac.media_type = cfg.media_type;
 	hw->rss_size_max = cfg.rss_size_max;
+	if (hw->rss_size_max == 0) {
+		PMD_INIT_LOG(WARNING, "rss_size_max is 0, already adjust to %u.",
+			     HNS3_RSS_SIZE_MAX_DEFAULT);
+		hw->rss_size_max = HNS3_RSS_SIZE_MAX_DEFAULT;
+	}
 	memcpy(hw->mac.mac_addr, cfg.mac_addr, RTE_ETHER_ADDR_LEN);
 	hw->mac.phy_addr = cfg.phy_addr;
 	hw->dcb_info.num_pg = 1;
-- 
2.33.0

[PATCH 7/7] net/hns3: fix unrelease some resources on reset case

[Dengdui Huang]

Some resources, like, unmapping Rx interrupt, doesn't perform
when execute dev_stop on reset. This will lead to other issues.

Fixes: 2790c6464725 ("net/hns3: support device reset")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 2d4e348442..9f99525e70 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5219,20 +5219,23 @@ hns3_dev_stop(struct rte_eth_dev *dev)
 	struct hns3_hw *hw = &hns->hw;
 
 	PMD_INIT_FUNC_TRACE();
+	if (rte_atomic_load_explicit(&hw->reset.resetting, rte_memory_order_relaxed) != 0) {
+		hns3_warn(hw, "device is resetting, stop operation is not allowed.");
+		return -EBUSY;
+	}
+
 	dev->data->dev_started = 0;
 
 	hw->adapter_state = HNS3_NIC_STOPPING;
 	hns3_stop_rxtx_datapath(dev);
 
 	rte_spinlock_lock(&hw->lock);
-	if (rte_atomic_load_explicit(&hw->reset.resetting, rte_memory_order_relaxed) == 0) {
-		hns3_tm_dev_stop_proc(hw);
-		hns3_config_mac_tnl_int(hw, false);
-		hns3_stop_tqps(hw);
-		hns3_do_stop(hns);
-		hns3_unmap_rx_interrupt(dev);
-		hw->adapter_state = HNS3_NIC_CONFIGURED;
-	}
+	hns3_tm_dev_stop_proc(hw);
+	hns3_config_mac_tnl_int(hw, false);
+	hns3_stop_tqps(hw);
+	hns3_do_stop(hns);
+	hns3_unmap_rx_interrupt(dev);
+	hw->adapter_state = HNS3_NIC_CONFIGURED;
 	hns3_rx_scattered_reset(dev);
 	rte_eal_alarm_cancel(hns3_service_handler, dev);
 	hns3_stop_report_lse(dev);
-- 
2.33.0