[PATCH 0/6] bugfixes for hns3 PMD

[PATCH 0/6] bugfixes for hns3 PMD

[Min Hu (Connor)]

This patch set contains 6 bugfixes for hns3 PMD.

Chengwen Feng (1):
  net/hns3: delete duplicate code

Huisong Li (4):
  net/hns3: fix fail to rollback the max packet size in PF
  net/hns3: fix insecure way to query MAC statistics
  net/hns3: fix double decrement of secondary count
  net/hns3: fix operating queue when TCAM table is invalid

Lijun Ou (1):
  net/hns3: fix RSS key with null

 drivers/net/hns3/hns3_ethdev.c    |  27 ++++---
 drivers/net/hns3/hns3_ethdev.h    |   1 +
 drivers/net/hns3/hns3_ethdev_vf.c |   1 -
 drivers/net/hns3/hns3_flow.c      |  12 +++-
 drivers/net/hns3/hns3_rss.c       |   4 --
 drivers/net/hns3/hns3_stats.c     | 116 +++++++++++++++---------------
 drivers/net/hns3/hns3_stats.h     |  11 ++-
 7 files changed, 86 insertions(+), 86 deletions(-)

-- 
2.33.0

[PATCH 1/6] net/hns3: fix fail to rollback the max packet size in PF

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

HNS3 PF driver use the hns->pf.mps to restore the MTU when a reset occurs.
If user fails to configure the MTU, the MPS of PF may not be restored to
the original value,

Fixes: 25fb790f7868 ("net/hns3: fix HW buffer size on MTU update")
Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
Fixes: d51867db65c1 ("net/hns3: add initialization")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index a5114662d2..73bf209717 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2075,7 +2075,6 @@ static int
 hns3_config_mtu(struct hns3_hw *hw, uint16_t mps)
 {
 	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
-	uint16_t original_mps = hns->pf.mps;
 	int err;
 	int ret;
 
@@ -2085,22 +2084,20 @@ hns3_config_mtu(struct hns3_hw *hw, uint16_t mps)
 		return ret;
 	}
 
-	hns->pf.mps = mps;
 	ret = hns3_buffer_alloc(hw);
 	if (ret) {
 		hns3_err(hw, "failed to allocate buffer, ret = %d", ret);
 		goto rollback;
 	}
 
+	hns->pf.mps = mps;
+
 	return 0;
 
 rollback:
-	err = hns3_set_mac_mtu(hw, original_mps);
-	if (err) {
+	err = hns3_set_mac_mtu(hw, hns->pf.mps);
+	if (err)
 		hns3_err(hw, "fail to rollback MTU, err = %d", err);
-		return ret;
-	}
-	hns->pf.mps = original_mps;
 
 	return ret;
 }
-- 
2.33.0

Re: [PATCH 1/6] net/hns3: fix fail to rollback the max packet size in PF

[Ferruh Yigit]

On 1/28/2022 2:07 AM, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> HNS3 PF driver use the hns->pf.mps to restore the MTU when a reset occurs.
> If user fails to configure the MTU, the MPS of PF may not be restored to
> the original value,
> 
> Fixes: 25fb790f7868 ("net/hns3: fix HW buffer size on MTU update")
> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
> Fixes: d51867db65c1 ("net/hns3: add initialization")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Series applied to dpdk-next-net/main, thanks.

[PATCH 2/6] net/hns3: fix RSS key with null

[Min Hu (Connor)]

From: Lijun Ou <oulijun@huawei.com>

Since the patch '1848b117' has initialized the variable 'key' in
'struct rte_flow_action_rss' with 'NULL', the PMD will use the
default RSS key when create the first RSS rule with NULL RSS key.
Then, if create a repeated RSS rule with the above, it will not
identify duplicate rules and return an error message.

To solve the preceding problem, determine whether the current RSS keys
are the same based on whether the length of key_len of rss is 0.

Fixes: 1848b117cca1 ("app/testpmd: fix RSS key for flow API RSS rule")
Cc: stable@dpdk.org

Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_flow.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index 56ef6f57b2..aba07aaa6f 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -1286,6 +1286,7 @@ static bool
 hns3_action_rss_same(const struct rte_flow_action_rss *comp,
 		     const struct rte_flow_action_rss *with)
 {
+	bool rss_key_is_same;
 	bool func_is_same;
 
 	/*
@@ -1302,11 +1303,16 @@ hns3_action_rss_same(const struct rte_flow_action_rss *comp,
 		func_is_same = (with->func != RTE_ETH_HASH_FUNCTION_DEFAULT) ?
 				(comp->func == with->func) : true;
 
-	return (func_is_same &&
+	if (with->key_len == 0 || with->key == NULL)
+		rss_key_is_same = 1;
+	else
+		rss_key_is_same = comp->key_len == with->key_len &&
+		!memcmp(comp->key, with->key, with->key_len);
+
+	return (func_is_same && rss_key_is_same &&
 		comp->types == (with->types & HNS3_ETH_RSS_SUPPORT) &&
-		comp->level == with->level && comp->key_len == with->key_len &&
+		comp->level == with->level &&
 		comp->queue_num == with->queue_num &&
-		!memcmp(comp->key, with->key, with->key_len) &&
 		!memcmp(comp->queue, with->queue,
 			sizeof(*with->queue) * with->queue_num));
 }
-- 
2.33.0

[PATCH 3/6] net/hns3: fix insecure way to query MAC statistics

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

The query way of MAC statistics in HNS3 PF driver is as following:
1) get MAC statistics register number and calculate descriptor number.
2) use above descriptor number to send command to firmware to query all MAC
   statistics and copy to hns3_mac_stats struct in driver.

The preceding way does not verify the validity of the number of obtained
register, which may cause memory out-of-bounds.

Fixes: 8839c5e202f3 ("net/hns3: support device stats")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c |   4 ++
 drivers/net/hns3/hns3_ethdev.h |   1 +
 drivers/net/hns3/hns3_stats.c  | 116 ++++++++++++++++-----------------
 drivers/net/hns3/hns3_stats.h  |  11 ++--
 4 files changed, 65 insertions(+), 67 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 73bf209717..57f1572340 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2733,6 +2733,10 @@ hns3_get_capability(struct hns3_hw *hw)
 	if (ret)
 		return ret;
 
+	ret = hns3_query_mac_stats_reg_num(hw);
+	if (ret)
+		return ret;
+
 	if (hw->revision < PCI_REVISION_ID_HIP09_A) {
 		hns3_set_default_dev_specifications(hw);
 		hw->intr.mapping_mode = HNS3_INTR_MAPPING_VEC_RSV_ONE;
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index cf6380ebb2..ef028a7b2c 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -500,6 +500,7 @@ struct hns3_hw {
 	struct hns3_tqp_stats tqp_stats;
 	/* Include Mac stats | Rx stats | Tx stats */
 	struct hns3_mac_stats mac_stats;
+	uint32_t mac_stats_reg_num;
 	struct hns3_rx_missed_stats imissed_stats;
 	uint64_t oerror_stats;
 	uint32_t fw_version;
diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index 606b72509a..806720faff 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -307,24 +307,21 @@ static const struct hns3_xstats_name_offset hns3_imissed_stats_strings[] = {
 
 static void hns3_tqp_stats_clear(struct hns3_hw *hw);
 
-/*
- * Query all the MAC statistics data of Network ICL command ,opcode id: 0x0034.
- * This command is used before send 'query_mac_stat command', the descriptor
- * number of 'query_mac_stat command' must match with reg_num in this command.
- * @praram hw
- *   Pointer to structure hns3_hw.
- * @return
- *   0 on success.
- */
 static int
-hns3_update_mac_stats(struct hns3_hw *hw, const uint32_t desc_num)
+hns3_update_mac_stats(struct hns3_hw *hw)
 {
+#define HNS3_MAC_STATS_REG_NUM_PER_DESC	4
+
 	uint64_t *data = (uint64_t *)(&hw->mac_stats);
 	struct hns3_cmd_desc *desc;
+	uint32_t stats_iterms;
 	uint64_t *desc_data;
-	uint16_t i, k, n;
+	uint32_t desc_num;
+	uint16_t i;
 	int ret;
 
+	/* The first desc has a 64-bit header, so need to consider it. */
+	desc_num = hw->mac_stats_reg_num / HNS3_MAC_STATS_REG_NUM_PER_DESC + 1;
 	desc = rte_malloc("hns3_mac_desc",
 			  desc_num * sizeof(struct hns3_cmd_desc), 0);
 	if (desc == NULL) {
@@ -340,65 +337,71 @@ hns3_update_mac_stats(struct hns3_hw *hw, const uint32_t desc_num)
 		return ret;
 	}
 
-	for (i = 0; i < desc_num; i++) {
-		/* For special opcode 0034, only the first desc has the head */
-		if (i == 0) {
-			desc_data = (uint64_t *)(&desc[i].data[0]);
-			n = HNS3_RD_FIRST_STATS_NUM;
-		} else {
-			desc_data = (uint64_t *)(&desc[i]);
-			n = HNS3_RD_OTHER_STATS_NUM;
-		}
-
-		for (k = 0; k < n; k++) {
-			*data += rte_le_to_cpu_64(*desc_data);
-			data++;
-			desc_data++;
-		}
+	stats_iterms = RTE_MIN(sizeof(hw->mac_stats) / sizeof(uint64_t),
+			       hw->mac_stats_reg_num);
+	desc_data = (uint64_t *)(&desc[0].data[0]);
+	for (i = 0; i < stats_iterms; i++) {
+		/*
+		 * Data memory is continuous and only the first descriptor has a
+		 * header in this command.
+		 */
+		*data += rte_le_to_cpu_64(*desc_data);
+		data++;
+		desc_data++;
 	}
 	rte_free(desc);
 
 	return 0;
 }
 
-/*
- * Query Mac stat reg num command ,opcode id: 0x0033.
- * This command is used before send 'query_mac_stat command', the descriptor
- * number of 'query_mac_stat command' must match with reg_num in this command.
- * @praram rte_stats
- *   Pointer to structure rte_eth_stats.
- * @return
- *   0 on success.
- */
 static int
-hns3_mac_query_reg_num(struct rte_eth_dev *dev, uint32_t *desc_num)
+hns3_mac_query_reg_num(struct hns3_hw *hw, uint32_t *reg_num)
 {
-	struct hns3_adapter *hns = dev->data->dev_private;
-	struct hns3_hw *hw = &hns->hw;
+#define HNS3_MAC_STATS_RSV_REG_NUM_ON_HIP08_B	3
 	struct hns3_cmd_desc desc;
-	uint32_t *desc_data;
-	uint32_t reg_num;
 	int ret;
 
 	hns3_cmd_setup_basic_desc(&desc, HNS3_OPC_QUERY_MAC_REG_NUM, true);
 	ret = hns3_cmd_send(hw, &desc, 1);
-	if (ret)
+	if (ret) {
+		hns3_err(hw, "failed to query MAC statistic reg number, ret = %d",
+			 ret);
 		return ret;
+	}
 
-	/*
-	 * The num of MAC statistics registers that are provided by IMP in this
-	 * version.
-	 */
-	desc_data = (uint32_t *)(&desc.data[0]);
-	reg_num = rte_le_to_cpu_32(*desc_data);
+	/* The number of MAC statistics registers are provided by firmware. */
+	*reg_num = rte_le_to_cpu_32(desc.data[0]);
+	if (*reg_num == 0) {
+		hns3_err(hw, "MAC statistic reg number is invalid!");
+		return -ENODATA;
+	}
 
 	/*
-	 * The descriptor number of 'query_additional_mac_stat command' is
-	 * '1 + (reg_num-3)/4 + ((reg_num-3)%4 !=0)';
-	 * This value is 83 in this version
+	 * If driver doesn't request the firmware to report more MAC statistics
+	 * iterms and the total number of MAC statistics registers by using new
+	 * method, firmware will only reports the number of valid statistics
+	 * registers. However, structure hns3_mac_stats in driver contains valid
+	 * and reserved statistics iterms. In this case, the total register
+	 * number must be added to three reserved statistics registers.
 	 */
-	*desc_num = 1 + ((reg_num - 3) >> 2) +
-		    (uint32_t)(((reg_num - 3) & 0x3) ? 1 : 0);
+	*reg_num += HNS3_MAC_STATS_RSV_REG_NUM_ON_HIP08_B;
+
+	return 0;
+}
+
+int
+hns3_query_mac_stats_reg_num(struct hns3_hw *hw)
+{
+	uint32_t mac_stats_reg_num = 0;
+	int ret;
+
+	ret = hns3_mac_query_reg_num(hw, &mac_stats_reg_num);
+	if (ret)
+		return ret;
+
+	hw->mac_stats_reg_num = mac_stats_reg_num;
+	if (hw->mac_stats_reg_num > sizeof(hw->mac_stats) / sizeof(uint64_t))
+		hns3_warn(hw, "MAC stats reg number from firmware is greater than stats iterms in driver.");
 
 	return 0;
 }
@@ -408,15 +411,8 @@ hns3_query_update_mac_stats(struct rte_eth_dev *dev)
 {
 	struct hns3_adapter *hns = dev->data->dev_private;
 	struct hns3_hw *hw = &hns->hw;
-	uint32_t desc_num;
-	int ret;
 
-	ret = hns3_mac_query_reg_num(dev, &desc_num);
-	if (ret == 0)
-		ret = hns3_update_mac_stats(hw, desc_num);
-	else
-		hns3_err(hw, "Query mac reg num fail : %d", ret);
-	return ret;
+	return hns3_update_mac_stats(hw);
 }
 
 static int
diff --git a/drivers/net/hns3/hns3_stats.h b/drivers/net/hns3/hns3_stats.h
index d1230f94cb..c81d351082 100644
--- a/drivers/net/hns3/hns3_stats.h
+++ b/drivers/net/hns3/hns3_stats.h
@@ -5,11 +5,6 @@
 #ifndef _HNS3_STATS_H_
 #define _HNS3_STATS_H_
 
-/* stats macro */
-#define HNS3_MAC_CMD_NUM		21
-#define HNS3_RD_FIRST_STATS_NUM		2
-#define HNS3_RD_OTHER_STATS_NUM		4
-
 /* TQP stats */
 struct hns3_tqp_stats {
 	uint64_t rcb_tx_ring_pktnum_rcd; /* Total num of transmitted packets */
@@ -22,6 +17,7 @@ struct hns3_tqp_stats {
 struct hns3_mac_stats {
 	uint64_t mac_tx_mac_pause_num;
 	uint64_t mac_rx_mac_pause_num;
+	uint64_t rsv0;
 	uint64_t mac_tx_pfc_pri0_pkt_num;
 	uint64_t mac_tx_pfc_pri1_pkt_num;
 	uint64_t mac_tx_pfc_pri2_pkt_num;
@@ -58,7 +54,7 @@ struct hns3_mac_stats {
 	uint64_t mac_tx_1519_2047_oct_pkt_num;
 	uint64_t mac_tx_2048_4095_oct_pkt_num;
 	uint64_t mac_tx_4096_8191_oct_pkt_num;
-	uint64_t rsv0;
+	uint64_t rsv1;
 	uint64_t mac_tx_8192_9216_oct_pkt_num;
 	uint64_t mac_tx_9217_12287_oct_pkt_num;
 	uint64_t mac_tx_12288_16383_oct_pkt_num;
@@ -85,7 +81,7 @@ struct hns3_mac_stats {
 	uint64_t mac_rx_1519_2047_oct_pkt_num;
 	uint64_t mac_rx_2048_4095_oct_pkt_num;
 	uint64_t mac_rx_4096_8191_oct_pkt_num;
-	uint64_t rsv1;
+	uint64_t rsv2;
 	uint64_t mac_rx_8192_9216_oct_pkt_num;
 	uint64_t mac_rx_9217_12287_oct_pkt_num;
 	uint64_t mac_rx_12288_16383_oct_pkt_num;
@@ -168,5 +164,6 @@ int hns3_stats_reset(struct rte_eth_dev *dev);
 int hns3_tqp_stats_init(struct hns3_hw *hw);
 void hns3_tqp_stats_uninit(struct hns3_hw *hw);
 int hns3_update_imissed_stats(struct hns3_hw *hw, bool is_clear);
+int hns3_query_mac_stats_reg_num(struct hns3_hw *hw);
 
 #endif /* _HNS3_STATS_H_ */
-- 
2.33.0

[PATCH 4/6] net/hns3: fix double decrement of secondary count

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

The "secondary_cnt" indicates the number of secondary processes on an
Ethernet device. But the variable is double subtracted when detach the
device in secondary processes.

Fixes: ff6dc76e40b8 ("net/hns3: refactor multi-process initialization")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev_vf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 1af2e07e81..dab1130dad 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2399,7 +2399,6 @@ hns3vf_dev_uninit(struct rte_eth_dev *eth_dev)
 	PMD_INIT_FUNC_TRACE();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
 		hns3_mp_uninit(eth_dev);
 		return 0;
 	}
-- 
2.33.0

[PATCH 5/6] net/hns3: fix operating queue when TCAM table is invalid

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

Reset queues will query the TCAM table. The table is cleared after global
or imp reset. Currently, PF driver first resets Rx/Tx queues and then
restore the table during the reset recovery process, which will fail to
query the table and trigger a RAS error.

Fixes: fa29fe45a7b4 ("net/hns3: support queue start and stop")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 57f1572340..2641b6f79b 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -4378,6 +4378,10 @@ hns3_init_hardware(struct hns3_adapter *hns)
 	struct hns3_hw *hw = &hns->hw;
 	int ret;
 
+	/*
+	 * All queue-related HW operations must be performed after the TCAM
+	 * table is configured.
+	 */
 	ret = hns3_map_tqp(hw);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to map tqp: %d", ret);
@@ -5547,15 +5551,15 @@ hns3_reinit_dev(struct hns3_adapter *hns)
 		return ret;
 	}
 
-	ret = hns3_reset_all_tqps(hns);
+	ret = hns3_init_hardware(hns);
 	if (ret) {
-		hns3_err(hw, "Failed to reset all queues: %d", ret);
+		hns3_err(hw, "Failed to init hardware: %d", ret);
 		return ret;
 	}
 
-	ret = hns3_init_hardware(hns);
+	ret = hns3_reset_all_tqps(hns);
 	if (ret) {
-		hns3_err(hw, "Failed to init hardware: %d", ret);
+		hns3_err(hw, "Failed to reset all queues: %d", ret);
 		return ret;
 	}
 
-- 
2.33.0

[PATCH 6/6] net/hns3: delete duplicate code

[Min Hu (Connor)]

From: Chengwen Feng <fengchengwen@huawei.com>

The hns3_set_rss_types hold two IPV4_TCP items, this patch deletes
duplicate item.

Fixes: 806f1d5ab0e3 ("net/hns3: set RSS hash type input configuration")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_rss.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index 3a4b699ae2..1782d63883 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -152,10 +152,6 @@ static const struct {
 	  BIT_ULL(HNS3_RSS_FIELD_IPV4_TCP_EN_IP_D) |
 	  BIT_ULL(HNS3_RSS_FIELD_IPV4_TCP_EN_TCP_S) |
 	  BIT_ULL(HNS3_RSS_FIELD_IPV4_TCP_EN_TCP_D) },
-	{ RTE_ETH_RSS_NONFRAG_IPV4_TCP, BIT_ULL(HNS3_RSS_FIELD_IPV4_TCP_EN_IP_S) |
-	  BIT_ULL(HNS3_RSS_FIELD_IPV4_TCP_EN_IP_D) |
-	  BIT_ULL(HNS3_RSS_FIELD_IPV4_TCP_EN_TCP_S) |
-	  BIT_ULL(HNS3_RSS_FIELD_IPV4_TCP_EN_TCP_D) },
 	{ RTE_ETH_RSS_NONFRAG_IPV4_UDP, BIT_ULL(HNS3_RSS_FIELD_IPV4_UDP_EN_IP_S) |
 	  BIT_ULL(HNS3_RSS_FIELD_IPV4_UDP_EN_IP_D) |
 	  BIT_ULL(HNS3_RSS_FIELD_IPV4_UDP_EN_UDP_S) |
-- 
2.33.0