[1/4] net/hns3: check if registering mp action successfully

Message ID 1593857391-14940-2-git-send-email-xavier.huwei@huawei.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series updates for hns3 PMD driver |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues
ci/Intel-compilation fail apply issues

Commit Message

Wei Hu (Xavier) July 4, 2020, 10:09 a.m. UTC
  Currently, there is a coverity defect warning about hns3 PMD driver, the
detail information as blow:
CID 289969 (#1 of 1): Unchecked return value (CHECKED_RETURN)
1. check_return: Calling rte_mp_action_register without checking return
   value (as is done elsewhere 11 out of 13 times).

The problem is that missing checking the return value of calling the API
rte_mp_action_register during initialization. If regitering an action
function for primary and secondary communication failed, the secondary
process can't work properly.

This patch fixes it by adding check return value of the API function
named rte_mp_action_register in the '.dev_init' implementation function
of hns3 PMD driver.

Coverity issue: 289969
Fixes: 23d4b61fee5d ("net/hns3: support multiple process")
Cc: stable@dpdk.org

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 22 ++++++++++++++++++++--
 drivers/net/hns3/hns3_ethdev_vf.c | 20 ++++++++++++++++++--
 drivers/net/hns3/hns3_mp.c        | 34 +++++++++++++++++++++++++++++-----
 drivers/net/hns3/hns3_mp.h        |  4 ++--
 4 files changed, 69 insertions(+), 11 deletions(-)
  

Patch

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 00ed3e2..265d620 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5451,12 +5451,25 @@  hns3_dev_init(struct rte_eth_dev *eth_dev)
 	hns3_set_rxtx_function(eth_dev);
 	eth_dev->dev_ops = &hns3_eth_dev_ops;
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-		hns3_mp_init_secondary();
+		ret = hns3_mp_init_secondary();
+		if (ret) {
+			PMD_INIT_LOG(ERR, "Failed to init for secondary "
+				     "process, ret = %d", ret);
+			goto err_mp_init_secondary;
+		}
+
 		hw->secondary_cnt++;
 		return 0;
 	}
 
-	hns3_mp_init_primary();
+	ret = hns3_mp_init_primary();
+	if (ret) {
+		PMD_INIT_LOG(ERR,
+			     "Failed to init for primary process, ret = %d",
+			     ret);
+		goto err_mp_init_primary;
+	}
+
 	hw->adapter_state = HNS3_NIC_UNINITIALIZED;
 	hns->is_vf = false;
 	hw->data = eth_dev->data;
@@ -5517,7 +5530,12 @@  hns3_dev_init(struct rte_eth_dev *eth_dev)
 
 err_init_pf:
 	rte_free(hw->reset.wait_data);
+
 err_init_reset:
+	hns3_mp_uninit_primary();
+
+err_mp_init_primary:
+err_mp_init_secondary:
 	eth_dev->dev_ops = NULL;
 	eth_dev->rx_pkt_burst = NULL;
 	eth_dev->tx_pkt_burst = NULL;
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 3c5998a..54e5dac 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2524,12 +2524,24 @@  hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 	hns3_set_rxtx_function(eth_dev);
 	eth_dev->dev_ops = &hns3vf_eth_dev_ops;
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-		hns3_mp_init_secondary();
+		ret = hns3_mp_init_secondary();
+		if (ret) {
+			PMD_INIT_LOG(ERR, "Failed to init for secondary "
+					  "process, ret = %d", ret);
+			goto err_mp_init_secondary;
+		}
+
 		hw->secondary_cnt++;
 		return 0;
 	}
 
-	hns3_mp_init_primary();
+	ret = hns3_mp_init_primary();
+	if (ret) {
+		PMD_INIT_LOG(ERR,
+			     "Failed to init for primary process, ret = %d",
+			     ret);
+		goto err_mp_init_primary;
+	}
 
 	hw->adapter_state = HNS3_NIC_UNINITIALIZED;
 	hns->is_vf = true;
@@ -2586,6 +2598,10 @@  hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_free(hw->reset.wait_data);
 
 err_init_reset:
+	hns3_mp_uninit_primary();
+
+err_mp_init_primary:
+err_mp_init_secondary:
 	eth_dev->dev_ops = NULL;
 	eth_dev->rx_pkt_burst = NULL;
 	eth_dev->tx_pkt_burst = NULL;
diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
index 596c310..639f46c 100644
--- a/drivers/net/hns3/hns3_mp.c
+++ b/drivers/net/hns3/hns3_mp.c
@@ -14,6 +14,8 @@ 
 #include "hns3_rxtx.h"
 #include "hns3_mp.h"
 
+static bool hns3_inited;
+
 /*
  * Initialize IPC message.
  *
@@ -192,9 +194,20 @@  void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev)
 /*
  * Initialize by primary process.
  */
-void hns3_mp_init_primary(void)
+int hns3_mp_init_primary(void)
 {
-	rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
+	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;
+
+		hns3_inited = true;
+	}
+
+	return 0;
 }
 
 /*
@@ -202,13 +215,24 @@  void hns3_mp_init_primary(void)
  */
 void hns3_mp_uninit_primary(void)
 {
-	rte_mp_action_unregister(HNS3_MP_NAME);
+	if (hns3_inited)
+		rte_mp_action_unregister(HNS3_MP_NAME);
 }
 
 /*
  * Initialize by secondary process.
  */
-void hns3_mp_init_secondary(void)
+int hns3_mp_init_secondary(void)
 {
-	rte_mp_action_register(HNS3_MP_NAME, mp_secondary_handle);
+	int ret;
+
+	if (!hns3_inited) {
+		ret = rte_mp_action_register(HNS3_MP_NAME, mp_secondary_handle);
+		if (ret)
+			return ret;
+
+		hns3_inited = true;
+	}
+
+	return 0;
 }
diff --git a/drivers/net/hns3/hns3_mp.h b/drivers/net/hns3/hns3_mp.h
index aefbeb1..036546a 100644
--- a/drivers/net/hns3/hns3_mp.h
+++ b/drivers/net/hns3/hns3_mp.h
@@ -7,8 +7,8 @@ 
 
 void hns3_mp_req_start_rxtx(struct rte_eth_dev *dev);
 void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev);
-void hns3_mp_init_primary(void);
+int hns3_mp_init_primary(void);
 void hns3_mp_uninit_primary(void);
-void hns3_mp_init_secondary(void);
+int hns3_mp_init_secondary(void);
 
 #endif /* _HNS3_MP_H_ */