[dpdk-dev] [PATCH 2/2] net/hns3: refactor PF support LSC event report

["Min Hu (Connor)" <humin29@huawei.com>]

From: Chengwen Feng <fengchengwen@huawei.com>

Currently, PF driver will report lsc when it detect the link status
change, it's not a generic implementation.

We refactor PF lsc event report by following scheme:
1. PF driver marks RTE_PCI_DRV_INTR_LSC in rte_pci_driver default.
2. In the init stage, PF driver will detect firmware whether support
lsc interrupt, driver will clear RTE_ETH_DEV_INTR_LSC flag if firmware
don't support lsc interrupt.
3. PF driver will report lsc event only when dev_conf.intr_conf.lsc is
set.

Note: If the firmware support lsc interrupt, we also keep periodic
polling to deal with the interrupt loss.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 doc/guides/nics/features/hns3.ini |  1 +
 drivers/net/hns3/hns3_ethdev.c    | 85 ++++++++++++++++++++++++---------------
 drivers/net/hns3/hns3_ethdev.h    |  2 +-
 drivers/net/hns3/hns3_mbx.c       |  2 +-
 4 files changed, 56 insertions(+), 34 deletions(-)

diff --git a/doc/guides/nics/features/hns3.ini b/doc/guides/nics/features/hns3.ini
index 502bfe7..be4da07 100644
--- a/doc/guides/nics/features/hns3.ini
+++ b/doc/guides/nics/features/hns3.ini
@@ -5,6 +5,7 @@
 ;
 [Features]
 Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Runtime Rx queue setup = Y
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 24583af..56b063e 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2738,10 +2738,15 @@ static int
 hns3_update_port_link_info(struct rte_eth_dev *eth_dev)
 {
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
+	int ret;
 
 	(void)hns3_update_link_status(hw);
 
-	return hns3_update_link_info(eth_dev);
+	ret = hns3_update_link_info(eth_dev);
+	if (ret)
+		hw->mac.link_status = ETH_LINK_DOWN;
+
+	return ret;
 }
 
 static void
@@ -2788,7 +2793,6 @@ hns3_dev_link_update(struct rte_eth_dev *eth_dev,
 
 	ret = hns3_update_port_link_info(eth_dev);
 	if (ret) {
-		mac->link_status = ETH_LINK_DOWN;
 		hns3_err(hw, "failed to get port link info, ret = %d.", ret);
 	}
 
@@ -4753,30 +4757,22 @@ hns3_update_link_status(struct hns3_hw *hw)
 	return false;
 }
 
-/*
- * Current, the PF driver get link status by two ways:
- * 1) Periodic polling in the intr thread context, driver call
- *    hns3_update_link_status to update link status.
- * 2) Firmware report async interrupt, driver process the event in the intr
- *    thread context, and call hns3_update_link_status to update link status.
- *
- * If detect link status changed, driver need report LSE. One method is add the
- * report LSE logic in hns3_update_link_status.
- *
- * But the PF driver ops(link_update) also call hns3_update_link_status to
- * update link status.
- * If we report LSE in hns3_update_link_status, it may lead to deadlock in the
- * bonding application.
- *
- * So add the one new API which used only in intr thread context.
- */
 void
-hns3_update_link_status_and_event(struct hns3_hw *hw)
+hns3_update_linkstatus_and_event(struct hns3_hw *hw, bool query)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
-	bool changed = hns3_update_link_status(hw);
-	if (changed)
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+	struct rte_eth_link new_link;
+	int ret;
+
+	if (query)
+		hns3_update_port_link_info(dev);
+
+	memset(&new_link, 0, sizeof(new_link));
+	hns3_setup_linkstatus(dev, &new_link);
+
+	ret = rte_eth_linkstatus_set(dev, &new_link);
+	if (ret == 0 && dev->data->dev_conf.intr_conf.lsc != 0)
+		hns3_start_report_lse(dev);
 }
 
 static void
@@ -4786,16 +4782,36 @@ hns3_service_handler(void *param)
 	struct hns3_adapter *hns = eth_dev->data->dev_private;
 	struct hns3_hw *hw = &hns->hw;
 
-	if (!hns3_is_reset_pending(hns)) {
-		hns3_update_link_status_and_event(hw);
-		hns3_update_link_info(eth_dev);
-	} else {
+	if (!hns3_is_reset_pending(hns))
+		hns3_update_linkstatus_and_event(hw, true);
+	else
 		hns3_warn(hw, "Cancel the query when reset is pending");
-	}
 
 	rte_eal_alarm_set(HNS3_SERVICE_INTERVAL, hns3_service_handler, eth_dev);
 }
 
+static void
+hns3_update_dev_lsc_cap(struct hns3_hw *hw,
+			int fw_compact_cmd_result)
+{
+	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
+
+	if (hw->adapter_state != HNS3_NIC_UNINITIALIZED)
+		return;
+
+	if (fw_compact_cmd_result != 0) {
+		/*
+		 * If fw_compact_cmd_result is not zero, it means firmware don't
+		 * support link status change interrupt.
+		 * Framework already set RTE_ETH_DEV_INTR_LSC bit because driver
+		 * declared RTE_PCI_DRV_INTR_LSC in drv_flags. It need to clear
+		 * the RTE_ETH_DEV_INTR_LSC capability when detect firmware
+		 * don't support link status change interrupt.
+		 */
+		dev->data->dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
+	}
+}
+
 static int
 hns3_init_hardware(struct hns3_adapter *hns)
 {
@@ -4883,6 +4899,7 @@ hns3_init_hardware(struct hns3_adapter *hns)
 	if (ret)
 		PMD_INIT_LOG(WARNING, "firmware compatible features not "
 			     "supported, ret = %d.", ret);
+	hns3_update_dev_lsc_cap(hw, ret);
 
 	return 0;
 
@@ -5275,7 +5292,6 @@ hns3_dev_start(struct rte_eth_dev *dev)
 	hns3_rx_scattered_calc(dev);
 	hns3_set_rxtx_function(dev);
 	hns3_mp_req_start_rxtx(dev);
-	rte_eal_alarm_set(HNS3_SERVICE_INTERVAL, hns3_service_handler, dev);
 
 	hns3_restore_filter(dev);
 
@@ -5290,6 +5306,10 @@ hns3_dev_start(struct rte_eth_dev *dev)
 
 	hns3_tm_dev_start_proc(hw);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0)
+		hns3_dev_link_update(dev, 0);
+	rte_eal_alarm_set(HNS3_SERVICE_INTERVAL, hns3_service_handler, dev);
+
 	hns3_info(hw, "hns3 dev start successful!");
 
 	return 0;
@@ -5403,6 +5423,7 @@ hns3_dev_stop(struct rte_eth_dev *dev)
 	}
 	hns3_rx_scattered_reset(dev);
 	rte_eal_alarm_cancel(hns3_service_handler, dev);
+	hns3_stop_report_lse(dev);
 	rte_spinlock_unlock(&hw->lock);
 
 	return 0;
@@ -5901,11 +5922,11 @@ hns3_stop_service(struct hns3_adapter *hns)
 	struct rte_eth_dev *eth_dev;
 
 	eth_dev = &rte_eth_devices[hw->data->port_id];
+	hw->mac.link_status = ETH_LINK_DOWN;
 	if (hw->adapter_state == HNS3_NIC_STARTED) {
 		rte_eal_alarm_cancel(hns3_service_handler, eth_dev);
-		hns3_update_link_status_and_event(hw);
+		hns3_update_linkstatus_and_event(hw, false);
 	}
-	hw->mac.link_status = ETH_LINK_DOWN;
 
 	hns3_set_rxtx_function(eth_dev);
 	rte_wmb();
@@ -6907,7 +6928,7 @@ static const struct rte_pci_id pci_id_hns3_map[] = {
 
 static struct rte_pci_driver rte_hns3_pmd = {
 	.id_table = pci_id_hns3_map,
-	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	.probe = eth_hns3_pci_probe,
 	.remove = eth_hns3_pci_remove,
 };
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 09d89a2..d44947f 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -1022,7 +1022,7 @@ int hns3_dev_flow_ops_get(struct rte_eth_dev *dev,
 			  const struct rte_flow_ops **ops);
 bool hns3_is_reset_pending(struct hns3_adapter *hns);
 bool hns3vf_is_reset_pending(struct hns3_adapter *hns);
-void hns3_update_link_status_and_event(struct hns3_hw *hw);
+void hns3_update_linkstatus_and_event(struct hns3_hw *hw, bool query);
 void hns3_ether_format_addr(char *buf, uint16_t size,
 			const struct rte_ether_addr *ether_addr);
 int hns3_dev_infos_get(struct rte_eth_dev *eth_dev,
diff --git a/drivers/net/hns3/hns3_mbx.c b/drivers/net/hns3/hns3_mbx.c
index 2b97978..ad3238b 100644
--- a/drivers/net/hns3/hns3_mbx.c
+++ b/drivers/net/hns3/hns3_mbx.c
@@ -315,7 +315,7 @@ hns3_handle_link_change_event(struct hns3_hw *hw,
 	if (!req->msg[LINK_STATUS_OFFSET])
 		hns3_link_fail_parse(hw, req->msg[LINK_FAIL_CODE_OFFSET]);
 
-	hns3_update_link_status_and_event(hw);
+	hns3_update_linkstatus_and_event(hw, true);
 }
 
 static void
-- 
2.7.4