DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] refactor support LSC event report
@ 2021-04-09  4:45 Min Hu (Connor)
  2021-04-09  4:45 ` [dpdk-dev] [PATCH 1/2] net/hns3: refactor VF " Min Hu (Connor)
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Min Hu (Connor) @ 2021-04-09  4:45 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

This set of patches refactor support LSC event report.

Chengwen Feng (2):
  net/hns3: refactor VF support LSC event report
  net/hns3: refactor PF support LSC event report

 doc/guides/nics/features/hns3.ini    |   1 +
 doc/guides/nics/features/hns3_vf.ini |   1 +
 drivers/net/hns3/hns3_ethdev.c       |  85 ++++++++++-------
 drivers/net/hns3/hns3_ethdev.h       |  29 +++++-
 drivers/net/hns3/hns3_ethdev_vf.c    | 171 ++++++++++++++++++++++++++++-------
 drivers/net/hns3/hns3_intr.c         |  30 ++++++
 drivers/net/hns3/hns3_intr.h         |   2 +
 drivers/net/hns3/hns3_mbx.c          |   5 +-
 8 files changed, 255 insertions(+), 69 deletions(-)

-- 
2.7.4


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

* [dpdk-dev] [PATCH 1/2] net/hns3: refactor VF support LSC event report
  2021-04-09  4:45 [dpdk-dev] [PATCH 0/2] refactor support LSC event report Min Hu (Connor)
@ 2021-04-09  4:45 ` Min Hu (Connor)
  2021-04-09  4:45 ` [dpdk-dev] [PATCH 2/2] net/hns3: refactor PF " Min Hu (Connor)
  2021-04-13  0:45 ` [dpdk-dev] [PATCH 0/2] refactor " Ferruh Yigit
  2 siblings, 0 replies; 4+ messages in thread
From: Min Hu (Connor) @ 2021-04-09  4:45 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

From: Chengwen Feng <fengchengwen@huawei.com>

Currently, VF driver periodic obtains link status from PF kernel
driver, and report lsc event when detect link status change. Because
the period is 1 second, it's probably too late to report especially
in sush as bonding scenario.

To solve this problem we use the following scheme:
1. PF kernel driver support immediate push link status to all VFs when
it detects the link status changes.
2. VF driver will detect PF kernel driver whether support push link
status in device init stage by sending request link info mailbox
message to PF, PF then tell VF the push capability by extend
HNS3_MBX_LINK_STAT_CHANGE mailbox message.
3. VF driver marks RTE_PCI_DRV_INTR_LSC in rte_pci_driver default,
when detect PF don't support push link status then it will clear
RTE_ETH_DEV_INTR_LSC flag.

So if PF kernel driver support push link status to VF, then VF driver
will have RTE_ETH_DEV_INTR_LSC capability.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 doc/guides/nics/features/hns3_vf.ini |   1 +
 drivers/net/hns3/hns3_ethdev.h       |  27 ++++++
 drivers/net/hns3/hns3_ethdev_vf.c    | 171 ++++++++++++++++++++++++++++-------
 drivers/net/hns3/hns3_intr.c         |  30 ++++++
 drivers/net/hns3/hns3_intr.h         |   2 +
 drivers/net/hns3/hns3_mbx.c          |   3 +
 6 files changed, 199 insertions(+), 35 deletions(-)

diff --git a/doc/guides/nics/features/hns3_vf.ini b/doc/guides/nics/features/hns3_vf.ini
index 1640669..9238350 100644
--- a/doc/guides/nics/features/hns3_vf.ini
+++ b/doc/guides/nics/features/hns3_vf.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.h b/drivers/net/hns3/hns3_ethdev.h
index d666b1a..09d89a2 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -764,8 +764,26 @@ struct hns3_pf {
 	struct hns3_tm_conf tm_conf;
 };
 
+enum {
+	HNS3_PF_PUSH_LSC_CAP_NOT_SUPPORTED,
+	HNS3_PF_PUSH_LSC_CAP_SUPPORTED,
+	HNS3_PF_PUSH_LSC_CAP_UNKNOWN
+};
+
 struct hns3_vf {
 	struct hns3_adapter *adapter;
+
+	/* Whether PF support push link status change to VF */
+	uint16_t pf_push_lsc_cap;
+
+	/*
+	 * If PF support push link status change, VF still need send request to
+	 * get link status in some cases (such as reset recover stage), so use
+	 * the req_link_info_cnt to control max request count.
+	 */
+	uint16_t req_link_info_cnt;
+
+	uint16_t poll_job_started; /* whether poll job is started */
 };
 
 struct hns3_adapter {
@@ -850,6 +868,8 @@ enum {
 	(&((struct hns3_adapter *)adapter)->hw)
 #define HNS3_DEV_PRIVATE_TO_PF(adapter) \
 	(&((struct hns3_adapter *)adapter)->pf)
+#define HNS3_DEV_PRIVATE_TO_VF(adapter) \
+	(&((struct hns3_adapter *)adapter)->vf)
 #define HNS3_DEV_HW_TO_ADAPTER(hw) \
 	container_of(hw, struct hns3_adapter, hw)
 
@@ -859,6 +879,12 @@ static inline struct hns3_pf *HNS3_DEV_HW_TO_PF(struct hns3_hw *hw)
 	return &adapter->pf;
 }
 
+static inline struct hns3_vf *HNS3_DEV_HW_TO_VF(struct hns3_hw *hw)
+{
+	struct hns3_adapter *adapter = HNS3_DEV_HW_TO_ADAPTER(hw);
+	return &adapter->vf;
+}
+
 #define hns3_set_field(origin, mask, shift, val) \
 	do { \
 		(origin) &= (~(mask)); \
@@ -1004,6 +1030,7 @@ int hns3_dev_infos_get(struct rte_eth_dev *eth_dev,
 void hns3vf_update_link_status(struct hns3_hw *hw, uint8_t link_status,
 			  uint32_t link_speed, uint8_t link_duplex);
 void hns3_parse_devargs(struct rte_eth_dev *dev);
+void hns3vf_update_push_lsc_cap(struct hns3_hw *hw, bool supported);
 int hns3_restore_ptp(struct hns3_adapter *hns);
 int hns3_mbuf_dyn_rx_timestamp_register(struct rte_eth_dev *dev,
 				    struct rte_eth_conf *conf);
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index a677cc8..fb697f8 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -44,6 +44,9 @@ static int hns3vf_add_mc_mac_addr(struct hns3_hw *hw,
 				  struct rte_ether_addr *mac_addr);
 static int hns3vf_remove_mc_mac_addr(struct hns3_hw *hw,
 				     struct rte_ether_addr *mac_addr);
+static int hns3vf_dev_link_update(struct rte_eth_dev *eth_dev,
+				   __rte_unused int wait_to_complete);
+
 /* set PCI bus mastering */
 static int
 hns3vf_set_bus_master(const struct rte_pci_device *device, bool op)
@@ -1195,6 +1198,65 @@ hns3vf_query_dev_specifications(struct hns3_hw *hw)
 	return hns3vf_check_dev_specifications(hw);
 }
 
+void
+hns3vf_update_push_lsc_cap(struct hns3_hw *hw, bool supported)
+{
+	uint16_t val = supported ? HNS3_PF_PUSH_LSC_CAP_SUPPORTED :
+				   HNS3_PF_PUSH_LSC_CAP_NOT_SUPPORTED;
+	uint16_t exp = HNS3_PF_PUSH_LSC_CAP_UNKNOWN;
+	struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw);
+
+	if (vf->pf_push_lsc_cap == HNS3_PF_PUSH_LSC_CAP_UNKNOWN)
+		__atomic_compare_exchange(&vf->pf_push_lsc_cap, &exp, &val, 0,
+					  __ATOMIC_ACQUIRE, __ATOMIC_ACQUIRE);
+}
+
+static void
+hns3vf_get_push_lsc_cap(struct hns3_hw *hw)
+{
+#define HNS3_CHECK_PUSH_LSC_CAP_TIMEOUT_MS	500
+
+	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
+	int32_t remain_ms = HNS3_CHECK_PUSH_LSC_CAP_TIMEOUT_MS;
+	uint16_t val = HNS3_PF_PUSH_LSC_CAP_NOT_SUPPORTED;
+	uint16_t exp = HNS3_PF_PUSH_LSC_CAP_UNKNOWN;
+	struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw);
+
+	__atomic_store_n(&vf->pf_push_lsc_cap, HNS3_PF_PUSH_LSC_CAP_UNKNOWN,
+			 __ATOMIC_RELEASE);
+
+	(void)hns3_send_mbx_msg(hw, HNS3_MBX_GET_LINK_STATUS, 0, NULL, 0, false,
+				NULL, 0);
+
+	while (remain_ms > 0) {
+		rte_delay_ms(HNS3_POLL_RESPONE_MS);
+		if (__atomic_load_n(&vf->pf_push_lsc_cap, __ATOMIC_ACQUIRE) !=
+			HNS3_PF_PUSH_LSC_CAP_UNKNOWN)
+			break;
+		remain_ms--;
+	}
+
+	/*
+	 * When exit above loop, the pf_push_lsc_cap could be one of the three
+	 * state: unknown (means pf not ack), not_supported, supported.
+	 * Here config it as 'not_supported' when it's 'unknown' state.
+	 */
+	__atomic_compare_exchange(&vf->pf_push_lsc_cap, &exp, &val, 0,
+				  __ATOMIC_ACQUIRE, __ATOMIC_ACQUIRE);
+
+	if (__atomic_load_n(&vf->pf_push_lsc_cap, __ATOMIC_ACQUIRE) ==
+		HNS3_PF_PUSH_LSC_CAP_SUPPORTED) {
+		hns3_info(hw, "detect PF support push link status change!");
+	} else {
+		/*
+		 * Framework already set RTE_ETH_DEV_INTR_LSC bit because driver
+		 * declared RTE_PCI_DRV_INTR_LSC in drv_flags. So here cleared
+		 * the RTE_ETH_DEV_INTR_LSC capability.
+		 */
+		dev->data->dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
+	}
+}
+
 static int
 hns3vf_get_capability(struct hns3_hw *hw)
 {
@@ -1408,6 +1470,8 @@ hns3vf_get_configuration(struct hns3_hw *hw)
 		return ret;
 	}
 
+	hns3vf_get_push_lsc_cap(hw);
+
 	/* Get queue configuration from PF */
 	ret = hns3vf_get_queue_info(hw);
 	if (ret)
@@ -1455,15 +1519,28 @@ hns3vf_set_tc_queue_mapping(struct hns3_adapter *hns, uint16_t nb_rx_q,
 static void
 hns3vf_request_link_info(struct hns3_hw *hw)
 {
+	struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw);
 	uint8_t resp_msg;
+	bool send_req;
 	int ret;
 
 	if (__atomic_load_n(&hw->reset.resetting, __ATOMIC_RELAXED))
 		return;
+
+	send_req = vf->pf_push_lsc_cap == HNS3_PF_PUSH_LSC_CAP_NOT_SUPPORTED ||
+		   vf->req_link_info_cnt > 0;
+	if (!send_req)
+		return;
+
 	ret = hns3_send_mbx_msg(hw, HNS3_MBX_GET_LINK_STATUS, 0, NULL, 0, false,
 				&resp_msg, sizeof(resp_msg));
-	if (ret)
-		hns3_err(hw, "Failed to fetch link status from PF: %d", ret);
+	if (ret) {
+		hns3_err(hw, "failed to fetch link status, ret = %d", ret);
+		return;
+	}
+
+	if (vf->req_link_info_cnt > 0)
+		vf->req_link_info_cnt--;
 }
 
 void
@@ -1471,34 +1548,30 @@ hns3vf_update_link_status(struct hns3_hw *hw, uint8_t link_status,
 			  uint32_t link_speed, uint8_t link_duplex)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
+	struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw);
 	struct hns3_mac *mac = &hw->mac;
-	bool report_lse;
-	bool changed;
-
-	changed = mac->link_status != link_status ||
-		  mac->link_speed != link_speed ||
-		  mac->link_duplex != link_duplex;
-	if (!changed)
-		return;
+	int ret;
 
 	/*
-	 * VF's link status/speed/duplex were updated by polling from PF driver,
-	 * because the link status/speed/duplex may be changed in the polling
-	 * interval, so driver will report lse (lsc event) once any of the above
-	 * thress variables changed.
-	 * But if the PF's link status is down and driver saved link status is
-	 * also down, there are no need to report lse.
+	 * PF kernel driver may push link status when VF driver is in resetting,
+	 * driver will stop polling job in this case, after resetting done
+	 * driver will start polling job again.
+	 * When polling job started, driver will get initial link status by
+	 * sending request to PF kernel driver, then could update link status by
+	 * process PF kernel driver's link status mailbox message.
 	 */
-	report_lse = true;
-	if (link_status == ETH_LINK_DOWN && link_status == mac->link_status)
-		report_lse = false;
+	if (!__atomic_load_n(&vf->poll_job_started, __ATOMIC_RELAXED))
+		return;
+
+	if (hw->adapter_state != HNS3_NIC_STARTED)
+		return;
 
 	mac->link_status = link_status;
 	mac->link_speed = link_speed;
 	mac->link_duplex = link_duplex;
-
-	if (report_lse)
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+	ret = hns3vf_dev_link_update(dev, 0);
+	if (ret == 0 && dev->data->dev_conf.intr_conf.lsc != 0)
+		hns3_start_report_lse(dev);
 }
 
 static int
@@ -1714,8 +1787,8 @@ hns3vf_service_handler(void *param)
 
 	/*
 	 * The query link status and reset processing are executed in the
-	 * interrupt thread.When the IMP reset occurs, IMP will not respond,
-	 * and the query operation will time out after 30ms. In the case of
+	 * interrupt thread. When the IMP reset occurs, IMP will not respond,
+	 * and the query operation will timeout after 30ms. In the case of
 	 * multiple PF/VFs, each query failure timeout causes the IMP reset
 	 * interrupt to fail to respond within 100ms.
 	 * Before querying the link status, check whether there is a reset
@@ -1730,6 +1803,31 @@ hns3vf_service_handler(void *param)
 			  eth_dev);
 }
 
+static void
+hns3vf_start_poll_job(struct rte_eth_dev *dev)
+{
+#define HNS3_REQUEST_LINK_INFO_REMAINS_CNT	3
+
+	struct hns3_vf *vf = HNS3_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+	if (vf->pf_push_lsc_cap == HNS3_PF_PUSH_LSC_CAP_SUPPORTED)
+		vf->req_link_info_cnt = HNS3_REQUEST_LINK_INFO_REMAINS_CNT;
+
+	__atomic_store_n(&vf->poll_job_started, 1, __ATOMIC_RELAXED);
+
+	hns3vf_service_handler(dev);
+}
+
+static void
+hns3vf_stop_poll_job(struct rte_eth_dev *dev)
+{
+	struct hns3_vf *vf = HNS3_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+	rte_eal_alarm_cancel(hns3vf_service_handler, dev);
+
+	__atomic_store_n(&vf->poll_job_started, 0, __ATOMIC_RELAXED);
+}
+
 static int
 hns3_query_vf_resource(struct hns3_hw *hw)
 {
@@ -2036,7 +2134,8 @@ hns3vf_dev_stop(struct rte_eth_dev *dev)
 		hw->adapter_state = HNS3_NIC_CONFIGURED;
 	}
 	hns3_rx_scattered_reset(dev);
-	rte_eal_alarm_cancel(hns3vf_service_handler, dev);
+	hns3vf_stop_poll_job(dev);
+	hns3_stop_report_lse(dev);
 	rte_spinlock_unlock(&hw->lock);
 
 	return 0;
@@ -2306,19 +2405,17 @@ hns3vf_dev_start(struct rte_eth_dev *dev)
 	hns3_rx_scattered_calc(dev);
 	hns3_set_rxtx_function(dev);
 	hns3_mp_req_start_rxtx(dev);
-	hns3vf_service_handler(dev);
 
 	hns3vf_restore_filter(dev);
 
 	/* Enable interrupt of all rx queues before enabling queues */
 	hns3_dev_all_rx_queue_intr_enable(hw, true);
-
-	/*
-	 * After finished the initialization, start all tqps to receive/transmit
-	 * packets and refresh all queue status.
-	 */
 	hns3_start_tqps(hw);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0)
+		hns3vf_dev_link_update(dev, 0);
+	hns3vf_start_poll_job(dev);
+
 	return ret;
 
 start_all_rxqs_fail:
@@ -2458,9 +2555,13 @@ hns3vf_stop_service(struct hns3_adapter *hns)
 
 	eth_dev = &rte_eth_devices[hw->data->port_id];
 	if (hw->adapter_state == HNS3_NIC_STARTED) {
-		rte_eal_alarm_cancel(hns3vf_service_handler, eth_dev);
+		/*
+		 * Make sure call update link status before hns3vf_stop_poll_job
+		 * because update link status depend on polling job exist.
+		 */
 		hns3vf_update_link_status(hw, ETH_LINK_DOWN, hw->mac.link_speed,
-			hw->mac.link_duplex);
+					  hw->mac.link_duplex);
+		hns3vf_stop_poll_job(eth_dev);
 	}
 	hw->mac.link_status = ETH_LINK_DOWN;
 
@@ -2501,7 +2602,7 @@ hns3vf_start_service(struct hns3_adapter *hns)
 	hns3_set_rxtx_function(eth_dev);
 	hns3_mp_req_start_rxtx(eth_dev);
 	if (hw->adapter_state == HNS3_NIC_STARTED) {
-		hns3vf_service_handler(eth_dev);
+		hns3vf_start_poll_job(eth_dev);
 
 		/* Enable interrupt of all rx queues before enabling queues */
 		hns3_dev_all_rx_queue_intr_enable(hw, true);
@@ -2972,7 +3073,7 @@ static const struct rte_pci_id pci_id_hns3vf_map[] = {
 
 static struct rte_pci_driver rte_hns3vf_pmd = {
 	.id_table = pci_id_hns3vf_map,
-	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	.probe = eth_hns3vf_pci_probe,
 	.remove = eth_hns3vf_pci_remove,
 };
diff --git a/drivers/net/hns3/hns3_intr.c b/drivers/net/hns3/hns3_intr.c
index c8d6137..a48c3f2 100644
--- a/drivers/net/hns3/hns3_intr.c
+++ b/drivers/net/hns3/hns3_intr.c
@@ -2614,3 +2614,33 @@ hns3_reset_abort(struct hns3_adapter *hns)
 			 reset_string[hw->reset.level], tv.tv_sec, tv.tv_usec);
 	}
 }
+
+static void
+hns3_report_lse(void *arg)
+{
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)arg;
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	if (hw->adapter_state == HNS3_NIC_STARTED)
+		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+}
+
+void
+hns3_start_report_lse(struct rte_eth_dev *dev)
+{
+#define DELAY_REPORT_LSE_US	1
+	/*
+	 * When this function called, the context may hold hns3_hw.lock, if
+	 * report lse right now, in some application such as bonding, it will
+	 * trigger call driver's ops which may acquire hns3_hw.lock again, so
+	 * lead to deadlock.
+	 * Here we use delay report to avoid the deadlock.
+	 */
+	rte_eal_alarm_set(DELAY_REPORT_LSE_US, hns3_report_lse, dev);
+}
+
+void
+hns3_stop_report_lse(struct rte_eth_dev *dev)
+{
+	rte_eal_alarm_cancel(hns3_report_lse, dev);
+}
diff --git a/drivers/net/hns3/hns3_intr.h b/drivers/net/hns3/hns3_intr.h
index e10e094..850518d 100644
--- a/drivers/net/hns3/hns3_intr.h
+++ b/drivers/net/hns3/hns3_intr.h
@@ -115,5 +115,7 @@ int hns3_reset_req_hw_reset(struct hns3_adapter *hns);
 int hns3_reset_process(struct hns3_adapter *hns,
 		       enum hns3_reset_level reset_level);
 void hns3_reset_abort(struct hns3_adapter *hns);
+void hns3_start_report_lse(struct rte_eth_dev *dev);
+void hns3_stop_report_lse(struct rte_eth_dev *dev);
 
 #endif /* _HNS3_INTR_H_ */
diff --git a/drivers/net/hns3/hns3_mbx.c b/drivers/net/hns3/hns3_mbx.c
index 597a2d1..2b97978 100644
--- a/drivers/net/hns3/hns3_mbx.c
+++ b/drivers/net/hns3/hns3_mbx.c
@@ -205,6 +205,7 @@ hns3_mbx_handler(struct hns3_hw *hw)
 {
 	enum hns3_reset_level reset_level;
 	uint8_t link_status, link_duplex;
+	uint8_t support_push_lsc;
 	uint32_t link_speed;
 	uint16_t *msg_q;
 	uint8_t opcode;
@@ -224,6 +225,8 @@ hns3_mbx_handler(struct hns3_hw *hw)
 			link_duplex = (uint8_t)rte_le_to_cpu_16(msg_q[4]);
 			hns3vf_update_link_status(hw, link_status, link_speed,
 						  link_duplex);
+			support_push_lsc = (*(uint8_t *)&msg_q[5]) & 1u;
+			hns3vf_update_push_lsc_cap(hw, support_push_lsc);
 			break;
 		case HNS3_MBX_ASSERTING_RESET:
 			/* PF has asserted reset hence VF should go in pending
-- 
2.7.4


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

* [dpdk-dev] [PATCH 2/2] net/hns3: refactor PF support LSC event report
  2021-04-09  4:45 [dpdk-dev] [PATCH 0/2] refactor support LSC event report Min Hu (Connor)
  2021-04-09  4:45 ` [dpdk-dev] [PATCH 1/2] net/hns3: refactor VF " Min Hu (Connor)
@ 2021-04-09  4:45 ` Min Hu (Connor)
  2021-04-13  0:45 ` [dpdk-dev] [PATCH 0/2] refactor " Ferruh Yigit
  2 siblings, 0 replies; 4+ messages in thread
From: Min Hu (Connor) @ 2021-04-09  4:45 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

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


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

* Re: [dpdk-dev] [PATCH 0/2] refactor support LSC event report
  2021-04-09  4:45 [dpdk-dev] [PATCH 0/2] refactor support LSC event report Min Hu (Connor)
  2021-04-09  4:45 ` [dpdk-dev] [PATCH 1/2] net/hns3: refactor VF " Min Hu (Connor)
  2021-04-09  4:45 ` [dpdk-dev] [PATCH 2/2] net/hns3: refactor PF " Min Hu (Connor)
@ 2021-04-13  0:45 ` Ferruh Yigit
  2 siblings, 0 replies; 4+ messages in thread
From: Ferruh Yigit @ 2021-04-13  0:45 UTC (permalink / raw)
  To: Min Hu (Connor), dev

On 4/9/2021 5:45 AM, Min Hu (Connor) wrote:
> This set of patches refactor support LSC event report.
> 
> Chengwen Feng (2):
>    net/hns3: refactor VF support LSC event report
>    net/hns3: refactor PF support LSC event report
> 

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

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

end of thread, other threads:[~2021-04-13  0:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09  4:45 [dpdk-dev] [PATCH 0/2] refactor support LSC event report Min Hu (Connor)
2021-04-09  4:45 ` [dpdk-dev] [PATCH 1/2] net/hns3: refactor VF " Min Hu (Connor)
2021-04-09  4:45 ` [dpdk-dev] [PATCH 2/2] net/hns3: refactor PF " Min Hu (Connor)
2021-04-13  0:45 ` [dpdk-dev] [PATCH 0/2] refactor " Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).