patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH 19.11 0/4] fix some bugs for hns3
@ 2021-12-25 10:53 Huisong Li
  2021-12-25 10:53 ` [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC Huisong Li
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Huisong Li @ 2021-12-25 10:53 UTC (permalink / raw)
  To: stable, christian.ehrhardt; +Cc: lihuisong, humin29

Backport a MAC related bugfix and multi-process bugfixes.

Huisong Li (4):
  net/hns3: fix residual MAC after setting default MAC
  net/hns3: fix secondary process reference count
  net/hns3: fix multi-process action register and unregister
  net/hns3: unregister MP action on close for secondary

 drivers/net/hns3/hns3_ethdev.c    | 71 +++++++++++--------------------
 drivers/net/hns3/hns3_ethdev.h    |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c | 14 ++++--
 drivers/net/hns3/hns3_mp.c        | 45 +++++++++++---------
 drivers/net/hns3/hns3_mp.h        |  9 +++-
 5 files changed, 68 insertions(+), 72 deletions(-)

-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC
  2021-12-25 10:53 [PATCH 19.11 0/4] fix some bugs for hns3 Huisong Li
@ 2021-12-25 10:53 ` Huisong Li
  2022-01-10  7:45   ` Christian Ehrhardt
  2021-12-25 10:53 ` [PATCH 19.11 2/4] net/hns3: fix secondary process reference count Huisong Li
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Huisong Li @ 2021-12-25 10:53 UTC (permalink / raw)
  To: stable, christian.ehrhardt; +Cc: lihuisong, humin29

[ upstream commit 19e67d8ebced5cb12829f75c70e6c497b5925e82 ]

This problem occurs in the following scenarios:
1) reset is encountered when the adapter is running.
2) set a new default MAC address.

After the above two steps, the old default MAC address should be not
take effect. But the current behavior is contrary to that. This is due
to the change of the "default_addr_setted" in hw->mac from 'true' to
'false' after the reset. As a result, the old MAC address is not removed
when the new default MAC address is set. This variable controls whether
to delete the old default MAC address when setting the default MAC
address. It is only used when the mac_addr_set API is called for the
first time. In fact, when a unicast MAC address is deleted, if the
address isn't in the MAC address table, the driver doesn't return
failure. So this patch remove the redundant and troublesome variables to
resolve this problem.

Fixes: 7d7f9f80bbfb ("net/hns3: support MAC address related operations")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 58 ++++++++++------------------------
 drivers/net/hns3/hns3_ethdev.h |  1 -
 2 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 157dd34d4f..a2676f84b0 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -1545,7 +1545,7 @@ hns3_remove_mc_addr_common(struct hns3_hw *hw, struct rte_ether_addr *mac_addr)
 
 static int
 hns3_add_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr,
-		  uint32_t idx, __attribute__ ((unused)) uint32_t pool)
+		  __rte_unused uint32_t idx, __rte_unused uint32_t pool)
 {
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	char mac_str[RTE_ETHER_ADDR_FMT_SIZE];
@@ -1576,8 +1576,6 @@ hns3_add_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr,
 		return ret;
 	}
 
-	if (idx == 0)
-		hw->mac.default_addr_setted = true;
 	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
@@ -1642,35 +1640,18 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_ether_addr *oaddr;
 	char mac_str[RTE_ETHER_ADDR_FMT_SIZE];
-	bool default_addr_setted;
-	bool rm_succes = false;
 	int ret, ret_val;
 
-	/* check if mac addr is valid */
-	if (!rte_is_valid_assigned_ether_addr(mac_addr)) {
-		rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
-				      mac_addr);
-		hns3_err(hw, "Failed to set mac addr, addr(%s) invalid",
-			 mac_str);
-		return -EINVAL;
-	}
-
-	oaddr = (struct rte_ether_addr *)hw->mac.mac_addr;
-	default_addr_setted = hw->mac.default_addr_setted;
-	if (default_addr_setted && !!rte_is_same_ether_addr(mac_addr, oaddr))
-		return 0;
-
 	rte_spinlock_lock(&hw->lock);
-	if (default_addr_setted) {
-		ret = hns3_remove_uc_addr_common(hw, oaddr);
-		if (ret) {
-			rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
-					      oaddr);
-			hns3_warn(hw, "Remove old uc mac address(%s) fail: %d",
-				  mac_str, ret);
-			rm_succes = false;
-		} else
-			rm_succes = true;
+	oaddr = (struct rte_ether_addr *)hw->mac.mac_addr;
+	ret = hns3_remove_uc_addr_common(hw, oaddr);
+	if (ret) {
+		rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
+					oaddr);
+		hns3_warn(hw, "Remove old uc mac address(%s) fail: %d",
+				mac_str, ret);
+		rte_spinlock_unlock(&hw->lock);
+		return ret;
 	}
 
 	ret = hns3_add_uc_addr_common(hw, mac_addr);
@@ -1689,7 +1670,6 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	rte_ether_addr_copy(mac_addr,
 			    (struct rte_ether_addr *)hw->mac.mac_addr);
-	hw->mac.default_addr_setted = true;
 	rte_spinlock_unlock(&hw->lock);
 
 	return 0;
@@ -1705,16 +1685,12 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
 	}
 
 err_add_uc_addr:
-	if (rm_succes) {
-		ret_val = hns3_add_uc_addr_common(hw, oaddr);
-		if (ret_val) {
-			rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
-					      oaddr);
-			hns3_warn(hw,
-				  "Failed to restore old uc mac addr(%s): %d",
-				  mac_str, ret_val);
-			hw->mac.default_addr_setted = false;
-		}
+	ret_val = hns3_add_uc_addr_common(hw, oaddr);
+	if (ret_val) {
+		rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
+					oaddr);
+		hns3_warn(hw, "Failed to restore old uc mac addr(%s): %d",
+				mac_str, ret_val);
 	}
 	rte_spinlock_unlock(&hw->lock);
 
@@ -2880,7 +2856,6 @@ hns3_get_board_configuration(struct hns3_hw *hw)
 	hw->rss_dis_flag = false;
 	memcpy(hw->mac.mac_addr, cfg.mac_addr, RTE_ETHER_ADDR_LEN);
 	hw->mac.phy_addr = cfg.phy_addr;
-	hw->mac.default_addr_setted = false;
 	hw->num_tx_desc = cfg.tqp_desc_num;
 	hw->num_rx_desc = cfg.tqp_desc_num;
 	hw->dcb_info.num_pg = 1;
@@ -4699,7 +4674,6 @@ hns3_do_stop(struct hns3_adapter *hns)
 		reset_queue = true;
 	} else
 		reset_queue = false;
-	hw->mac.default_addr_setted = false;
 	return hns3_stop_queues(hns, reset_queue);
 }
 
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 6c3ac6f8a9..be0fac4fd2 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -144,7 +144,6 @@ enum hns3_media_type {
 
 struct hns3_mac {
 	uint8_t mac_addr[RTE_ETHER_ADDR_LEN];
-	bool default_addr_setted; /* whether default addr(mac_addr) is set */
 	uint8_t media_type;
 	uint8_t phy_addr;
 	uint8_t link_duplex  : 1; /* ETH_LINK_[HALF/FULL]_DUPLEX */
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 19.11 2/4] net/hns3: fix secondary process reference count
  2021-12-25 10:53 [PATCH 19.11 0/4] fix some bugs for hns3 Huisong Li
  2021-12-25 10:53 ` [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC Huisong Li
@ 2021-12-25 10:53 ` Huisong Li
  2022-01-10  7:45   ` Christian Ehrhardt
  2021-12-25 10:53 ` [PATCH 19.11 3/4] net/hns3: fix multi-process action register and unregister Huisong Li
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Huisong Li @ 2021-12-25 10:53 UTC (permalink / raw)
  To: stable, christian.ehrhardt; +Cc: lihuisong, humin29

[ upstream commit 323263717774df318d8a6e64ac8bfe546e03b8f6 ]

The "secondary_cnt" will be increased when a secondary process
initialized. But the value of this variable is not decreased when the
secondary process exits, which causes the primary process senses that
the secondary process still exists. As a result, the primary process
fails to send messages to the secondary process after the secondary
process exits.

Fixes: 23d4b61fee5d ("net/hns3: support multiple process")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 5 +++--
 drivers/net/hns3/hns3_ethdev_vf.c | 6 ++++--
 drivers/net/hns3/hns3_mp.c        | 3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index a2676f84b0..d245c5db8b 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -4749,6 +4749,7 @@ hns3_dev_close(struct rte_eth_dev *eth_dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 		rte_free(eth_dev->process_private);
 		eth_dev->process_private = NULL;
+		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
 		return;
 	}
 
@@ -5437,8 +5438,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 				     "process, ret = %d", ret);
 			goto err_mp_init_secondary;
 		}
-
-		hw->secondary_cnt++;
+		__atomic_fetch_add(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
 		return 0;
 	}
 
@@ -5551,6 +5551,7 @@ hns3_dev_uninit(struct rte_eth_dev *eth_dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 		rte_free(eth_dev->process_private);
 		eth_dev->process_private = NULL;
+		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
 		return 0;
 	}
 
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 790e76a0db..972a6f00e4 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -1653,6 +1653,8 @@ hns3vf_dev_close(struct rte_eth_dev *eth_dev)
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 		rte_free(eth_dev->process_private);
+		eth_dev->process_private = NULL;
+		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
 		return;
     }
 
@@ -2291,8 +2293,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 					  "process, ret = %d", ret);
 			goto err_mp_init_secondary;
 		}
-
-		hw->secondary_cnt++;
+		__atomic_fetch_add(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
 		return 0;
 	}
 
@@ -2386,6 +2387,7 @@ hns3vf_dev_uninit(struct rte_eth_dev *eth_dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
 		rte_free(eth_dev->process_private);
 		eth_dev->process_private = NULL;
+		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
 		return 0;
 	}
 
diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
index a03f2cf13c..9b5ff475a9 100644
--- a/drivers/net/hns3/hns3_mp.c
+++ b/drivers/net/hns3/hns3_mp.c
@@ -132,7 +132,8 @@ mp_req_on_rxtx(struct rte_eth_dev *dev, enum hns3_mp_req_type type)
 	int ret;
 	int i;
 
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY || !hw->secondary_cnt)
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY ||
+		__atomic_load_n(&hw->secondary_cnt, __ATOMIC_RELAXED) == 0)
 		return;
 	if (type != HNS3_MP_REQ_START_RXTX && type != HNS3_MP_REQ_STOP_RXTX) {
 		hns3_err(hw, "port %u unknown request (req_type %d)",
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 19.11 3/4] net/hns3: fix multi-process action register and unregister
  2021-12-25 10:53 [PATCH 19.11 0/4] fix some bugs for hns3 Huisong Li
  2021-12-25 10:53 ` [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC Huisong Li
  2021-12-25 10:53 ` [PATCH 19.11 2/4] net/hns3: fix secondary process reference count Huisong Li
@ 2021-12-25 10:53 ` Huisong Li
  2022-01-10  7:46   ` Christian Ehrhardt
  2021-12-25 10:53 ` [PATCH 19.11 4/4] net/hns3: unregister MP action on close for secondary Huisong Li
  2022-01-03 11:36 ` [PATCH 19.11 0/4] fix some bugs for hns3 Christian Ehrhardt
  4 siblings, 1 reply; 11+ messages in thread
From: Huisong Li @ 2021-12-25 10:53 UTC (permalink / raw)
  To: stable, christian.ehrhardt; +Cc: lihuisong, humin29

[ upstream commit 841f8693536f9410fd51d385e1090d35cfe59914 ]

The multi-process has the following problems:
1) After a port in primary process is closed, the mp action of the
   process is unregistered. Which will cause that other device in the
   primary process cannot respond to requests from secondary processes.
2) Because variable "hns3_inited" is set to true without returning an
   initial value, the mp action cannot be registered again after it is
   unregistered.
3) The mp action of primary and secondary process need to be registered
   only once regardless of port numbers in the process. That's what
   variable "hns3_inited" does. But the variable is difficult to
   understand.

This patch adds a hns3_process_local_data structure to resolve above
problems.

Fixes: 9570b1fdbdad ("net/hns3: check multi-process action register result")
Fixes: 23d4b61fee5d ("net/hns3: support multiple process")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    |  2 ++
 drivers/net/hns3/hns3_ethdev_vf.c |  2 ++
 drivers/net/hns3/hns3_mp.c        | 37 ++++++++++++++++++-------------
 drivers/net/hns3/hns3_mp.h        |  7 ++++++
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index d245c5db8b..122a2bc66c 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5439,6 +5439,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 			goto err_mp_init_secondary;
 		}
 		__atomic_fetch_add(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
+		process_data.eth_dev_cnt++;
 		return 0;
 	}
 
@@ -5449,6 +5450,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 			     ret);
 		goto err_mp_init_primary;
 	}
+	process_data.eth_dev_cnt++;
 
 	hw->adapter_state = HNS3_NIC_UNINITIALIZED;
 
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 972a6f00e4..562f6f7662 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2294,6 +2294,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 			goto err_mp_init_secondary;
 		}
 		__atomic_fetch_add(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
+		process_data.eth_dev_cnt++;
 		return 0;
 	}
 
@@ -2304,6 +2305,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 			     ret);
 		goto err_mp_init_primary;
 	}
+	process_data.eth_dev_cnt++;
 
 	hw->adapter_state = HNS3_NIC_UNINITIALIZED;
 	hns->is_vf = true;
diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
index 9b5ff475a9..c7e49a798a 100644
--- a/drivers/net/hns3/hns3_mp.c
+++ b/drivers/net/hns3/hns3_mp.c
@@ -14,7 +14,8 @@
 #include "hns3_rxtx.h"
 #include "hns3_mp.h"
 
-static bool hns3_inited;
+/* local data for primary or secondary process. */
+struct hns3_process_local_data process_data;
 
 /*
  * Initialize IPC message.
@@ -199,14 +200,15 @@ int hns3_mp_init_primary(void)
 {
 	int ret;
 
-	if (!hns3_inited) {
-		/* primary is allowed to not support IPC */
-		ret = rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
-		if (ret && rte_errno != ENOTSUP)
-			return ret;
+	if (process_data.init_done)
+		return 0;
 
-		hns3_inited = true;
-	}
+	/* primary is allowed to not support IPC */
+	ret = rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
+	if (ret && rte_errno != ENOTSUP)
+		return ret;
+
+	process_data.init_done = true;
 
 	return 0;
 }
@@ -216,8 +218,12 @@ int hns3_mp_init_primary(void)
  */
 void hns3_mp_uninit_primary(void)
 {
-	if (hns3_inited)
+	process_data.eth_dev_cnt--;
+
+	if (process_data.eth_dev_cnt == 0) {
 		rte_mp_action_unregister(HNS3_MP_NAME);
+		process_data.init_done = false;
+	}
 }
 
 /*
@@ -227,13 +233,14 @@ int hns3_mp_init_secondary(void)
 {
 	int ret;
 
-	if (!hns3_inited) {
-		ret = rte_mp_action_register(HNS3_MP_NAME, mp_secondary_handle);
-		if (ret)
-			return ret;
+	if (process_data.init_done)
+		return 0;
 
-		hns3_inited = true;
-	}
+	ret = rte_mp_action_register(HNS3_MP_NAME, mp_secondary_handle);
+	if (ret)
+		return ret;
+
+	process_data.init_done = true;
 
 	return 0;
 }
diff --git a/drivers/net/hns3/hns3_mp.h b/drivers/net/hns3/hns3_mp.h
index 60ef2315db..8ef432e763 100644
--- a/drivers/net/hns3/hns3_mp.h
+++ b/drivers/net/hns3/hns3_mp.h
@@ -5,6 +5,13 @@
 #ifndef _HNS3_MP_H_
 #define _HNS3_MP_H_
 
+/* Local data for primary or secondary process. */
+struct hns3_process_local_data {
+	bool init_done; /* Process action register completed flag. */
+	int eth_dev_cnt; /* Ethdev count under the current process. */
+};
+extern struct hns3_process_local_data process_data;
+
 void hns3_mp_req_start_rxtx(struct rte_eth_dev *dev);
 void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev);
 int hns3_mp_init_primary(void);
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 19.11 4/4] net/hns3: unregister MP action on close for secondary
  2021-12-25 10:53 [PATCH 19.11 0/4] fix some bugs for hns3 Huisong Li
                   ` (2 preceding siblings ...)
  2021-12-25 10:53 ` [PATCH 19.11 3/4] net/hns3: fix multi-process action register and unregister Huisong Li
@ 2021-12-25 10:53 ` Huisong Li
  2022-01-10  7:45   ` Christian Ehrhardt
  2022-01-03 11:36 ` [PATCH 19.11 0/4] fix some bugs for hns3 Christian Ehrhardt
  4 siblings, 1 reply; 11+ messages in thread
From: Huisong Li @ 2021-12-25 10:53 UTC (permalink / raw)
  To: stable, christian.ehrhardt; +Cc: lihuisong, humin29

[ upstream commit 443242212baeb67d298c54cc927553c92aa29bec ]

This patch fixes lack of unregistering MP action for secondary process
when PMD is closed.

Fixes: 9570b1fdbdad ("net/hns3: check multi-process action register result")
Fixes: 23d4b61fee5d ("net/hns3: support multiple process")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 6 ++++--
 drivers/net/hns3/hns3_ethdev_vf.c | 6 ++++--
 drivers/net/hns3/hns3_mp.c        | 5 +----
 drivers/net/hns3/hns3_mp.h        | 2 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 122a2bc66c..df76dfce7f 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -4750,6 +4750,7 @@ hns3_dev_close(struct rte_eth_dev *eth_dev)
 		rte_free(eth_dev->process_private);
 		eth_dev->process_private = NULL;
 		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
+		hns3_mp_uninit();
 		return;
 	}
 
@@ -4768,7 +4769,7 @@ hns3_dev_close(struct rte_eth_dev *eth_dev)
 	rte_free(hw->reset.wait_data);
 	rte_free(eth_dev->process_private);
 	eth_dev->process_private = NULL;
-	hns3_mp_uninit_primary();
+	hns3_mp_uninit();
 	hns3_warn(hw, "Close port %d finished", hw->data->port_id);
 }
 
@@ -5529,7 +5530,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 	rte_free(hw->reset.wait_data);
 
 err_init_reset:
-	hns3_mp_uninit_primary();
+	hns3_mp_uninit();
 
 err_mp_init_primary:
 err_mp_init_secondary:
@@ -5554,6 +5555,7 @@ hns3_dev_uninit(struct rte_eth_dev *eth_dev)
 		rte_free(eth_dev->process_private);
 		eth_dev->process_private = NULL;
 		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
+		hns3_mp_uninit();
 		return 0;
 	}
 
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 562f6f7662..dbd46cd278 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -1655,6 +1655,7 @@ hns3vf_dev_close(struct rte_eth_dev *eth_dev)
 		rte_free(eth_dev->process_private);
 		eth_dev->process_private = NULL;
 		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
+		hns3_mp_uninit();
 		return;
     }
 
@@ -1672,7 +1673,7 @@ hns3vf_dev_close(struct rte_eth_dev *eth_dev)
 	rte_free(hw->reset.wait_data);
 	rte_free(eth_dev->process_private);
 	eth_dev->process_private = NULL;
-	hns3_mp_uninit_primary();
+	hns3_mp_uninit();
 	hns3_warn(hw, "Close port %d finished", hw->data->port_id);
 }
 
@@ -2364,7 +2365,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_free(hw->reset.wait_data);
 
 err_init_reset:
-	hns3_mp_uninit_primary();
+	hns3_mp_uninit();
 
 err_mp_init_primary:
 err_mp_init_secondary:
@@ -2390,6 +2391,7 @@ hns3vf_dev_uninit(struct rte_eth_dev *eth_dev)
 		rte_free(eth_dev->process_private);
 		eth_dev->process_private = NULL;
 		__atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
+		hns3_mp_uninit();
 		return 0;
 	}
 
diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
index c7e49a798a..b017387efd 100644
--- a/drivers/net/hns3/hns3_mp.c
+++ b/drivers/net/hns3/hns3_mp.c
@@ -213,10 +213,7 @@ int hns3_mp_init_primary(void)
 	return 0;
 }
 
-/*
- * Un-initialize by primary process.
- */
-void hns3_mp_uninit_primary(void)
+void hns3_mp_uninit(void)
 {
 	process_data.eth_dev_cnt--;
 
diff --git a/drivers/net/hns3/hns3_mp.h b/drivers/net/hns3/hns3_mp.h
index 8ef432e763..ef334648ff 100644
--- a/drivers/net/hns3/hns3_mp.h
+++ b/drivers/net/hns3/hns3_mp.h
@@ -15,7 +15,7 @@ extern struct hns3_process_local_data process_data;
 void hns3_mp_req_start_rxtx(struct rte_eth_dev *dev);
 void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev);
 int hns3_mp_init_primary(void);
-void hns3_mp_uninit_primary(void);
+void hns3_mp_uninit(void);
 int hns3_mp_init_secondary(void);
 
 #endif /* _HNS3_MP_H_ */
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 19.11 0/4] fix some bugs for hns3
  2021-12-25 10:53 [PATCH 19.11 0/4] fix some bugs for hns3 Huisong Li
                   ` (3 preceding siblings ...)
  2021-12-25 10:53 ` [PATCH 19.11 4/4] net/hns3: unregister MP action on close for secondary Huisong Li
@ 2022-01-03 11:36 ` Christian Ehrhardt
  2022-01-04  2:50   ` lihuisong (C)
  4 siblings, 1 reply; 11+ messages in thread
From: Christian Ehrhardt @ 2022-01-03 11:36 UTC (permalink / raw)
  To: Huisong Li; +Cc: stable, humin29

On Sat, Dec 25, 2021 at 11:58 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> Backport a MAC related bugfix and multi-process bugfixes.

Hi,
thanks for these backports. Since those are functional changes/fixes
and given how late in the release cycle for 19.11.11 we are I'll
accept them, but not into 19.11.11.
These changes would force us to start another RC and test/validation
round which is not gonna happen.
Instead I'll queue them up for 19.11.12 right after 19.11.11 is
released (end of this week).

I'll reply on your individual patches once they are queued

> Huisong Li (4):
>   net/hns3: fix residual MAC after setting default MAC
>   net/hns3: fix secondary process reference count
>   net/hns3: fix multi-process action register and unregister
>   net/hns3: unregister MP action on close for secondary
>
>  drivers/net/hns3/hns3_ethdev.c    | 71 +++++++++++--------------------
>  drivers/net/hns3/hns3_ethdev.h    |  1 -
>  drivers/net/hns3/hns3_ethdev_vf.c | 14 ++++--
>  drivers/net/hns3/hns3_mp.c        | 45 +++++++++++---------
>  drivers/net/hns3/hns3_mp.h        |  9 +++-
>  5 files changed, 68 insertions(+), 72 deletions(-)
>
> --
> 2.33.0
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 19.11 0/4] fix some bugs for hns3
  2022-01-03 11:36 ` [PATCH 19.11 0/4] fix some bugs for hns3 Christian Ehrhardt
@ 2022-01-04  2:50   ` lihuisong (C)
  0 siblings, 0 replies; 11+ messages in thread
From: lihuisong (C) @ 2022-01-04  2:50 UTC (permalink / raw)
  To: Christian Ehrhardt; +Cc: stable, humin29


在 2022/1/3 19:36, Christian Ehrhardt 写道:
> On Sat, Dec 25, 2021 at 11:58 AM Huisong Li <lihuisong@huawei.com> wrote:
>> Backport a MAC related bugfix and multi-process bugfixes.
> Hi,
> thanks for these backports. Since those are functional changes/fixes
> and given how late in the release cycle for 19.11.11 we are I'll
> accept them, but not into 19.11.11.
> These changes would force us to start another RC and test/validation
> round which is not gonna happen.
> Instead I'll queue them up for 19.11.12 right after 19.11.11 is
> released (end of this week).
>
> I'll reply on your individual patches once they are queued

ok, thanks.

If these patches are merged to 19.11.12 without any conflict, do I need

to send them again?

>
>> Huisong Li (4):
>>    net/hns3: fix residual MAC after setting default MAC
>>    net/hns3: fix secondary process reference count
>>    net/hns3: fix multi-process action register and unregister
>>    net/hns3: unregister MP action on close for secondary
>>
>>   drivers/net/hns3/hns3_ethdev.c    | 71 +++++++++++--------------------
>>   drivers/net/hns3/hns3_ethdev.h    |  1 -
>>   drivers/net/hns3/hns3_ethdev_vf.c | 14 ++++--
>>   drivers/net/hns3/hns3_mp.c        | 45 +++++++++++---------
>>   drivers/net/hns3/hns3_mp.h        |  9 +++-
>>   5 files changed, 68 insertions(+), 72 deletions(-)
>>
>> --
>> 2.33.0
>>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC
  2021-12-25 10:53 ` [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC Huisong Li
@ 2022-01-10  7:45   ` Christian Ehrhardt
  0 siblings, 0 replies; 11+ messages in thread
From: Christian Ehrhardt @ 2022-01-10  7:45 UTC (permalink / raw)
  To: Huisong Li; +Cc: stable, humin29

On Sat, Dec 25, 2021 at 11:58 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> [ upstream commit 19e67d8ebced5cb12829f75c70e6c497b5925e82 ]
>

Thank you, queued for 19.11.12 (later this year)

> This problem occurs in the following scenarios:
> 1) reset is encountered when the adapter is running.
> 2) set a new default MAC address.
>
> After the above two steps, the old default MAC address should be not
> take effect. But the current behavior is contrary to that. This is due
> to the change of the "default_addr_setted" in hw->mac from 'true' to
> 'false' after the reset. As a result, the old MAC address is not removed
> when the new default MAC address is set. This variable controls whether
> to delete the old default MAC address when setting the default MAC
> address. It is only used when the mac_addr_set API is called for the
> first time. In fact, when a unicast MAC address is deleted, if the
> address isn't in the MAC address table, the driver doesn't return
> failure. So this patch remove the redundant and troublesome variables to
> resolve this problem.
>
> Fixes: 7d7f9f80bbfb ("net/hns3: support MAC address related operations")
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/hns3/hns3_ethdev.c | 58 ++++++++++------------------------
>  drivers/net/hns3/hns3_ethdev.h |  1 -
>  2 files changed, 16 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index 157dd34d4f..a2676f84b0 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -1545,7 +1545,7 @@ hns3_remove_mc_addr_common(struct hns3_hw *hw, struct rte_ether_addr *mac_addr)
>
>  static int
>  hns3_add_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr,
> -                 uint32_t idx, __attribute__ ((unused)) uint32_t pool)
> +                 __rte_unused uint32_t idx, __rte_unused uint32_t pool)
>  {
>         struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>         char mac_str[RTE_ETHER_ADDR_FMT_SIZE];
> @@ -1576,8 +1576,6 @@ hns3_add_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr,
>                 return ret;
>         }
>
> -       if (idx == 0)
> -               hw->mac.default_addr_setted = true;
>         rte_spinlock_unlock(&hw->lock);
>
>         return ret;
> @@ -1642,35 +1640,18 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
>         struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>         struct rte_ether_addr *oaddr;
>         char mac_str[RTE_ETHER_ADDR_FMT_SIZE];
> -       bool default_addr_setted;
> -       bool rm_succes = false;
>         int ret, ret_val;
>
> -       /* check if mac addr is valid */
> -       if (!rte_is_valid_assigned_ether_addr(mac_addr)) {
> -               rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> -                                     mac_addr);
> -               hns3_err(hw, "Failed to set mac addr, addr(%s) invalid",
> -                        mac_str);
> -               return -EINVAL;
> -       }
> -
> -       oaddr = (struct rte_ether_addr *)hw->mac.mac_addr;
> -       default_addr_setted = hw->mac.default_addr_setted;
> -       if (default_addr_setted && !!rte_is_same_ether_addr(mac_addr, oaddr))
> -               return 0;
> -
>         rte_spinlock_lock(&hw->lock);
> -       if (default_addr_setted) {
> -               ret = hns3_remove_uc_addr_common(hw, oaddr);
> -               if (ret) {
> -                       rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> -                                             oaddr);
> -                       hns3_warn(hw, "Remove old uc mac address(%s) fail: %d",
> -                                 mac_str, ret);
> -                       rm_succes = false;
> -               } else
> -                       rm_succes = true;
> +       oaddr = (struct rte_ether_addr *)hw->mac.mac_addr;
> +       ret = hns3_remove_uc_addr_common(hw, oaddr);
> +       if (ret) {
> +               rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> +                                       oaddr);
> +               hns3_warn(hw, "Remove old uc mac address(%s) fail: %d",
> +                               mac_str, ret);
> +               rte_spinlock_unlock(&hw->lock);
> +               return ret;
>         }
>
>         ret = hns3_add_uc_addr_common(hw, mac_addr);
> @@ -1689,7 +1670,6 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
>
>         rte_ether_addr_copy(mac_addr,
>                             (struct rte_ether_addr *)hw->mac.mac_addr);
> -       hw->mac.default_addr_setted = true;
>         rte_spinlock_unlock(&hw->lock);
>
>         return 0;
> @@ -1705,16 +1685,12 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
>         }
>
>  err_add_uc_addr:
> -       if (rm_succes) {
> -               ret_val = hns3_add_uc_addr_common(hw, oaddr);
> -               if (ret_val) {
> -                       rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> -                                             oaddr);
> -                       hns3_warn(hw,
> -                                 "Failed to restore old uc mac addr(%s): %d",
> -                                 mac_str, ret_val);
> -                       hw->mac.default_addr_setted = false;
> -               }
> +       ret_val = hns3_add_uc_addr_common(hw, oaddr);
> +       if (ret_val) {
> +               rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> +                                       oaddr);
> +               hns3_warn(hw, "Failed to restore old uc mac addr(%s): %d",
> +                               mac_str, ret_val);
>         }
>         rte_spinlock_unlock(&hw->lock);
>
> @@ -2880,7 +2856,6 @@ hns3_get_board_configuration(struct hns3_hw *hw)
>         hw->rss_dis_flag = false;
>         memcpy(hw->mac.mac_addr, cfg.mac_addr, RTE_ETHER_ADDR_LEN);
>         hw->mac.phy_addr = cfg.phy_addr;
> -       hw->mac.default_addr_setted = false;
>         hw->num_tx_desc = cfg.tqp_desc_num;
>         hw->num_rx_desc = cfg.tqp_desc_num;
>         hw->dcb_info.num_pg = 1;
> @@ -4699,7 +4674,6 @@ hns3_do_stop(struct hns3_adapter *hns)
>                 reset_queue = true;
>         } else
>                 reset_queue = false;
> -       hw->mac.default_addr_setted = false;
>         return hns3_stop_queues(hns, reset_queue);
>  }
>
> diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
> index 6c3ac6f8a9..be0fac4fd2 100644
> --- a/drivers/net/hns3/hns3_ethdev.h
> +++ b/drivers/net/hns3/hns3_ethdev.h
> @@ -144,7 +144,6 @@ enum hns3_media_type {
>
>  struct hns3_mac {
>         uint8_t mac_addr[RTE_ETHER_ADDR_LEN];
> -       bool default_addr_setted; /* whether default addr(mac_addr) is set */
>         uint8_t media_type;
>         uint8_t phy_addr;
>         uint8_t link_duplex  : 1; /* ETH_LINK_[HALF/FULL]_DUPLEX */
> --
> 2.33.0
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 19.11 4/4] net/hns3: unregister MP action on close for secondary
  2021-12-25 10:53 ` [PATCH 19.11 4/4] net/hns3: unregister MP action on close for secondary Huisong Li
@ 2022-01-10  7:45   ` Christian Ehrhardt
  0 siblings, 0 replies; 11+ messages in thread
From: Christian Ehrhardt @ 2022-01-10  7:45 UTC (permalink / raw)
  To: Huisong Li; +Cc: stable, humin29

On Sat, Dec 25, 2021 at 11:58 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> [ upstream commit 443242212baeb67d298c54cc927553c92aa29bec ]
>

Thank you, queued for 19.11.12 (later this year)

> This patch fixes lack of unregistering MP action for secondary process
> when PMD is closed.
>
> Fixes: 9570b1fdbdad ("net/hns3: check multi-process action register result")
> Fixes: 23d4b61fee5d ("net/hns3: support multiple process")
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/hns3/hns3_ethdev.c    | 6 ++++--
>  drivers/net/hns3/hns3_ethdev_vf.c | 6 ++++--
>  drivers/net/hns3/hns3_mp.c        | 5 +----
>  drivers/net/hns3/hns3_mp.h        | 2 +-
>  4 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index 122a2bc66c..df76dfce7f 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -4750,6 +4750,7 @@ hns3_dev_close(struct rte_eth_dev *eth_dev)
>                 rte_free(eth_dev->process_private);
>                 eth_dev->process_private = NULL;
>                 __atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
> +               hns3_mp_uninit();
>                 return;
>         }
>
> @@ -4768,7 +4769,7 @@ hns3_dev_close(struct rte_eth_dev *eth_dev)
>         rte_free(hw->reset.wait_data);
>         rte_free(eth_dev->process_private);
>         eth_dev->process_private = NULL;
> -       hns3_mp_uninit_primary();
> +       hns3_mp_uninit();
>         hns3_warn(hw, "Close port %d finished", hw->data->port_id);
>  }
>
> @@ -5529,7 +5530,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
>         rte_free(hw->reset.wait_data);
>
>  err_init_reset:
> -       hns3_mp_uninit_primary();
> +       hns3_mp_uninit();
>
>  err_mp_init_primary:
>  err_mp_init_secondary:
> @@ -5554,6 +5555,7 @@ hns3_dev_uninit(struct rte_eth_dev *eth_dev)
>                 rte_free(eth_dev->process_private);
>                 eth_dev->process_private = NULL;
>                 __atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
> +               hns3_mp_uninit();
>                 return 0;
>         }
>
> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
> index 562f6f7662..dbd46cd278 100644
> --- a/drivers/net/hns3/hns3_ethdev_vf.c
> +++ b/drivers/net/hns3/hns3_ethdev_vf.c
> @@ -1655,6 +1655,7 @@ hns3vf_dev_close(struct rte_eth_dev *eth_dev)
>                 rte_free(eth_dev->process_private);
>                 eth_dev->process_private = NULL;
>                 __atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
> +               hns3_mp_uninit();
>                 return;
>      }
>
> @@ -1672,7 +1673,7 @@ hns3vf_dev_close(struct rte_eth_dev *eth_dev)
>         rte_free(hw->reset.wait_data);
>         rte_free(eth_dev->process_private);
>         eth_dev->process_private = NULL;
> -       hns3_mp_uninit_primary();
> +       hns3_mp_uninit();
>         hns3_warn(hw, "Close port %d finished", hw->data->port_id);
>  }
>
> @@ -2364,7 +2365,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
>         rte_free(hw->reset.wait_data);
>
>  err_init_reset:
> -       hns3_mp_uninit_primary();
> +       hns3_mp_uninit();
>
>  err_mp_init_primary:
>  err_mp_init_secondary:
> @@ -2390,6 +2391,7 @@ hns3vf_dev_uninit(struct rte_eth_dev *eth_dev)
>                 rte_free(eth_dev->process_private);
>                 eth_dev->process_private = NULL;
>                 __atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
> +               hns3_mp_uninit();
>                 return 0;
>         }
>
> diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
> index c7e49a798a..b017387efd 100644
> --- a/drivers/net/hns3/hns3_mp.c
> +++ b/drivers/net/hns3/hns3_mp.c
> @@ -213,10 +213,7 @@ int hns3_mp_init_primary(void)
>         return 0;
>  }
>
> -/*
> - * Un-initialize by primary process.
> - */
> -void hns3_mp_uninit_primary(void)
> +void hns3_mp_uninit(void)
>  {
>         process_data.eth_dev_cnt--;
>
> diff --git a/drivers/net/hns3/hns3_mp.h b/drivers/net/hns3/hns3_mp.h
> index 8ef432e763..ef334648ff 100644
> --- a/drivers/net/hns3/hns3_mp.h
> +++ b/drivers/net/hns3/hns3_mp.h
> @@ -15,7 +15,7 @@ extern struct hns3_process_local_data process_data;
>  void hns3_mp_req_start_rxtx(struct rte_eth_dev *dev);
>  void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev);
>  int hns3_mp_init_primary(void);
> -void hns3_mp_uninit_primary(void);
> +void hns3_mp_uninit(void);
>  int hns3_mp_init_secondary(void);
>
>  #endif /* _HNS3_MP_H_ */
> --
> 2.33.0
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 19.11 2/4] net/hns3: fix secondary process reference count
  2021-12-25 10:53 ` [PATCH 19.11 2/4] net/hns3: fix secondary process reference count Huisong Li
@ 2022-01-10  7:45   ` Christian Ehrhardt
  0 siblings, 0 replies; 11+ messages in thread
From: Christian Ehrhardt @ 2022-01-10  7:45 UTC (permalink / raw)
  To: Huisong Li; +Cc: stable, humin29

On Sat, Dec 25, 2021 at 11:58 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> [ upstream commit 323263717774df318d8a6e64ac8bfe546e03b8f6 ]
>

Thank you, queued for 19.11.12 (later this year)

> The "secondary_cnt" will be increased when a secondary process
> initialized. But the value of this variable is not decreased when the
> secondary process exits, which causes the primary process senses that
> the secondary process still exists. As a result, the primary process
> fails to send messages to the secondary process after the secondary
> process exits.
>
> Fixes: 23d4b61fee5d ("net/hns3: support multiple process")
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/hns3/hns3_ethdev.c    | 5 +++--
>  drivers/net/hns3/hns3_ethdev_vf.c | 6 ++++--
>  drivers/net/hns3/hns3_mp.c        | 3 ++-
>  3 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index a2676f84b0..d245c5db8b 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -4749,6 +4749,7 @@ hns3_dev_close(struct rte_eth_dev *eth_dev)
>         if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>                 rte_free(eth_dev->process_private);
>                 eth_dev->process_private = NULL;
> +               __atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
>                 return;
>         }
>
> @@ -5437,8 +5438,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
>                                      "process, ret = %d", ret);
>                         goto err_mp_init_secondary;
>                 }
> -
> -               hw->secondary_cnt++;
> +               __atomic_fetch_add(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
>                 return 0;
>         }
>
> @@ -5551,6 +5551,7 @@ hns3_dev_uninit(struct rte_eth_dev *eth_dev)
>         if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>                 rte_free(eth_dev->process_private);
>                 eth_dev->process_private = NULL;
> +               __atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
>                 return 0;
>         }
>
> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
> index 790e76a0db..972a6f00e4 100644
> --- a/drivers/net/hns3/hns3_ethdev_vf.c
> +++ b/drivers/net/hns3/hns3_ethdev_vf.c
> @@ -1653,6 +1653,8 @@ hns3vf_dev_close(struct rte_eth_dev *eth_dev)
>
>         if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>                 rte_free(eth_dev->process_private);
> +               eth_dev->process_private = NULL;
> +               __atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
>                 return;
>      }
>
> @@ -2291,8 +2293,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
>                                           "process, ret = %d", ret);
>                         goto err_mp_init_secondary;
>                 }
> -
> -               hw->secondary_cnt++;
> +               __atomic_fetch_add(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
>                 return 0;
>         }
>
> @@ -2386,6 +2387,7 @@ hns3vf_dev_uninit(struct rte_eth_dev *eth_dev)
>         if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>                 rte_free(eth_dev->process_private);
>                 eth_dev->process_private = NULL;
> +               __atomic_fetch_sub(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
>                 return 0;
>         }
>
> diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
> index a03f2cf13c..9b5ff475a9 100644
> --- a/drivers/net/hns3/hns3_mp.c
> +++ b/drivers/net/hns3/hns3_mp.c
> @@ -132,7 +132,8 @@ mp_req_on_rxtx(struct rte_eth_dev *dev, enum hns3_mp_req_type type)
>         int ret;
>         int i;
>
> -       if (rte_eal_process_type() == RTE_PROC_SECONDARY || !hw->secondary_cnt)
> +       if (rte_eal_process_type() == RTE_PROC_SECONDARY ||
> +               __atomic_load_n(&hw->secondary_cnt, __ATOMIC_RELAXED) == 0)
>                 return;
>         if (type != HNS3_MP_REQ_START_RXTX && type != HNS3_MP_REQ_STOP_RXTX) {
>                 hns3_err(hw, "port %u unknown request (req_type %d)",
> --
> 2.33.0
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 19.11 3/4] net/hns3: fix multi-process action register and unregister
  2021-12-25 10:53 ` [PATCH 19.11 3/4] net/hns3: fix multi-process action register and unregister Huisong Li
@ 2022-01-10  7:46   ` Christian Ehrhardt
  0 siblings, 0 replies; 11+ messages in thread
From: Christian Ehrhardt @ 2022-01-10  7:46 UTC (permalink / raw)
  To: Huisong Li; +Cc: stable, humin29

On Sat, Dec 25, 2021 at 11:58 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> [ upstream commit 841f8693536f9410fd51d385e1090d35cfe59914 ]
>

Thank you, queued for 19.11.12 (later this year)

> The multi-process has the following problems:
> 1) After a port in primary process is closed, the mp action of the
>    process is unregistered. Which will cause that other device in the
>    primary process cannot respond to requests from secondary processes.
> 2) Because variable "hns3_inited" is set to true without returning an
>    initial value, the mp action cannot be registered again after it is
>    unregistered.
> 3) The mp action of primary and secondary process need to be registered
>    only once regardless of port numbers in the process. That's what
>    variable "hns3_inited" does. But the variable is difficult to
>    understand.
>
> This patch adds a hns3_process_local_data structure to resolve above
> problems.
>
> Fixes: 9570b1fdbdad ("net/hns3: check multi-process action register result")
> Fixes: 23d4b61fee5d ("net/hns3: support multiple process")
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/hns3/hns3_ethdev.c    |  2 ++
>  drivers/net/hns3/hns3_ethdev_vf.c |  2 ++
>  drivers/net/hns3/hns3_mp.c        | 37 ++++++++++++++++++-------------
>  drivers/net/hns3/hns3_mp.h        |  7 ++++++
>  4 files changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index d245c5db8b..122a2bc66c 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -5439,6 +5439,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
>                         goto err_mp_init_secondary;
>                 }
>                 __atomic_fetch_add(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
> +               process_data.eth_dev_cnt++;
>                 return 0;
>         }
>
> @@ -5449,6 +5450,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
>                              ret);
>                 goto err_mp_init_primary;
>         }
> +       process_data.eth_dev_cnt++;
>
>         hw->adapter_state = HNS3_NIC_UNINITIALIZED;
>
> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
> index 972a6f00e4..562f6f7662 100644
> --- a/drivers/net/hns3/hns3_ethdev_vf.c
> +++ b/drivers/net/hns3/hns3_ethdev_vf.c
> @@ -2294,6 +2294,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
>                         goto err_mp_init_secondary;
>                 }
>                 __atomic_fetch_add(&hw->secondary_cnt, 1, __ATOMIC_RELAXED);
> +               process_data.eth_dev_cnt++;
>                 return 0;
>         }
>
> @@ -2304,6 +2305,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
>                              ret);
>                 goto err_mp_init_primary;
>         }
> +       process_data.eth_dev_cnt++;
>
>         hw->adapter_state = HNS3_NIC_UNINITIALIZED;
>         hns->is_vf = true;
> diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
> index 9b5ff475a9..c7e49a798a 100644
> --- a/drivers/net/hns3/hns3_mp.c
> +++ b/drivers/net/hns3/hns3_mp.c
> @@ -14,7 +14,8 @@
>  #include "hns3_rxtx.h"
>  #include "hns3_mp.h"
>
> -static bool hns3_inited;
> +/* local data for primary or secondary process. */
> +struct hns3_process_local_data process_data;
>
>  /*
>   * Initialize IPC message.
> @@ -199,14 +200,15 @@ int hns3_mp_init_primary(void)
>  {
>         int ret;
>
> -       if (!hns3_inited) {
> -               /* primary is allowed to not support IPC */
> -               ret = rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
> -               if (ret && rte_errno != ENOTSUP)
> -                       return ret;
> +       if (process_data.init_done)
> +               return 0;
>
> -               hns3_inited = true;
> -       }
> +       /* primary is allowed to not support IPC */
> +       ret = rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
> +       if (ret && rte_errno != ENOTSUP)
> +               return ret;
> +
> +       process_data.init_done = true;
>
>         return 0;
>  }
> @@ -216,8 +218,12 @@ int hns3_mp_init_primary(void)
>   */
>  void hns3_mp_uninit_primary(void)
>  {
> -       if (hns3_inited)
> +       process_data.eth_dev_cnt--;
> +
> +       if (process_data.eth_dev_cnt == 0) {
>                 rte_mp_action_unregister(HNS3_MP_NAME);
> +               process_data.init_done = false;
> +       }
>  }
>
>  /*
> @@ -227,13 +233,14 @@ int hns3_mp_init_secondary(void)
>  {
>         int ret;
>
> -       if (!hns3_inited) {
> -               ret = rte_mp_action_register(HNS3_MP_NAME, mp_secondary_handle);
> -               if (ret)
> -                       return ret;
> +       if (process_data.init_done)
> +               return 0;
>
> -               hns3_inited = true;
> -       }
> +       ret = rte_mp_action_register(HNS3_MP_NAME, mp_secondary_handle);
> +       if (ret)
> +               return ret;
> +
> +       process_data.init_done = true;
>
>         return 0;
>  }
> diff --git a/drivers/net/hns3/hns3_mp.h b/drivers/net/hns3/hns3_mp.h
> index 60ef2315db..8ef432e763 100644
> --- a/drivers/net/hns3/hns3_mp.h
> +++ b/drivers/net/hns3/hns3_mp.h
> @@ -5,6 +5,13 @@
>  #ifndef _HNS3_MP_H_
>  #define _HNS3_MP_H_
>
> +/* Local data for primary or secondary process. */
> +struct hns3_process_local_data {
> +       bool init_done; /* Process action register completed flag. */
> +       int eth_dev_cnt; /* Ethdev count under the current process. */
> +};
> +extern struct hns3_process_local_data process_data;
> +
>  void hns3_mp_req_start_rxtx(struct rte_eth_dev *dev);
>  void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev);
>  int hns3_mp_init_primary(void);
> --
> 2.33.0
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-01-10  7:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-25 10:53 [PATCH 19.11 0/4] fix some bugs for hns3 Huisong Li
2021-12-25 10:53 ` [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC Huisong Li
2022-01-10  7:45   ` Christian Ehrhardt
2021-12-25 10:53 ` [PATCH 19.11 2/4] net/hns3: fix secondary process reference count Huisong Li
2022-01-10  7:45   ` Christian Ehrhardt
2021-12-25 10:53 ` [PATCH 19.11 3/4] net/hns3: fix multi-process action register and unregister Huisong Li
2022-01-10  7:46   ` Christian Ehrhardt
2021-12-25 10:53 ` [PATCH 19.11 4/4] net/hns3: unregister MP action on close for secondary Huisong Li
2022-01-10  7:45   ` Christian Ehrhardt
2022-01-03 11:36 ` [PATCH 19.11 0/4] fix some bugs for hns3 Christian Ehrhardt
2022-01-04  2:50   ` lihuisong (C)

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git