patch 'net/txgbe: fix blocking system events' has been queued to stable release 22.11.3

Xueming Li xuemingl at nvidia.com
Thu Aug 10 01:58:44 CEST 2023


Hi,

FYI, your patch has been queued to stable release 22.11.3

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 08/11/23. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://git.dpdk.org/dpdk-stable/log/?h=22.11-staging

This queued commit can be viewed at:
https://git.dpdk.org/dpdk-stable/commit/?h=22.11-staging&id=09d8d1fe4f7b75079d1aede67252b8c32f210033

Thanks.

Xueming Li <xuemingl at nvidia.com>

---
>From 09d8d1fe4f7b75079d1aede67252b8c32f210033 Mon Sep 17 00:00:00 2001
From: Jiawen Wu <jiawenwu at trustnetic.com>
Date: Fri, 30 Jun 2023 17:35:12 +0800
Subject: [PATCH] net/txgbe: fix blocking system events
Cc: Xueming Li <xuemingl at nvidia.com>

[ upstream commit 5e170dd8b690cd55a2931ddbfe3db0f9b1cad462 ]

Refer to commit 819d0d1d57f1 ("net/ixgbe: fix blocking system events").
Fix the same issue as ixgbe.

TXGBE link status task uses rte alarm thread in old implementation.
Sometime txgbe link status task takes up to 9 seconds. This will
severely affect the rte-alarm-thread dependent tasks in the
system, like interrupt or hotplug event. So replace with an
independent thread which has the same thread affinity settings
as rte interrupt.

Fixes: 0c061eadec59 ("net/txgbe: add link status change")

Signed-off-by: Jiawen Wu <jiawenwu at trustnetic.com>
---
 drivers/net/txgbe/txgbe_ethdev.c    | 69 ++++++++++++++++++++++++++---
 drivers/net/txgbe/txgbe_ethdev.h    |  6 +++
 drivers/net/txgbe/txgbe_ethdev_vf.c |  6 ++-
 3 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c
index 95aef27092..2c7d71c0db 100644
--- a/drivers/net/txgbe/txgbe_ethdev.c
+++ b/drivers/net/txgbe/txgbe_ethdev.c
@@ -545,6 +545,7 @@ null:
 static int
 eth_txgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 {
+	struct txgbe_adapter *ad = eth_dev->data->dev_private;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct txgbe_hw *hw = TXGBE_DEV_HW(eth_dev);
 	struct txgbe_vfta *shadow_vfta = TXGBE_DEV_VFTA(eth_dev);
@@ -593,6 +594,7 @@ eth_txgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 		return 0;
 	}
 
+	__atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 
 	hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
@@ -1668,7 +1670,7 @@ txgbe_dev_start(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 
 	/* Stop the link setup handler before resetting the HW. */
-	rte_eal_alarm_cancel(txgbe_dev_setup_link_alarm_handler, dev);
+	txgbe_dev_wait_setup_link_complete(dev, 0);
 
 	/* disable uio/vfio intr/eventfd mapping */
 	rte_intr_disable(intr_handle);
@@ -1906,7 +1908,7 @@ txgbe_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	rte_eal_alarm_cancel(txgbe_dev_setup_link_alarm_handler, dev);
+	txgbe_dev_wait_setup_link_complete(dev, 0);
 
 	/* disable interrupts */
 	txgbe_disable_intr(hw);
@@ -2722,11 +2724,52 @@ txgbe_dev_setup_link_alarm_handler(void *param)
 	intr->flags &= ~TXGBE_FLAG_NEED_LINK_CONFIG;
 }
 
+/*
+ * If @timeout_ms was 0, it means that it will not return until link complete.
+ * It returns 1 on complete, return 0 on timeout.
+ */
+int
+txgbe_dev_wait_setup_link_complete(struct rte_eth_dev *dev, uint32_t timeout_ms)
+{
+#define WARNING_TIMEOUT    9000 /* 9s in total */
+	struct txgbe_adapter *ad = TXGBE_DEV_ADAPTER(dev);
+	uint32_t timeout = timeout_ms ? timeout_ms : WARNING_TIMEOUT;
+
+	while (__atomic_load_n(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
+		msec_delay(1);
+		timeout--;
+
+		if (timeout_ms) {
+			if (!timeout)
+				return 0;
+		} else if (!timeout) {
+			/* It will not return until link complete */
+			timeout = WARNING_TIMEOUT;
+			PMD_DRV_LOG(ERR, "TXGBE link thread not complete too long time!");
+		}
+	}
+
+	return 1;
+}
+
+static uint32_t
+txgbe_dev_setup_link_thread_handler(void *param)
+{
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
+	struct txgbe_adapter *ad = TXGBE_DEV_ADAPTER(dev);
+
+	rte_thread_detach(rte_thread_self());
+	txgbe_dev_setup_link_alarm_handler(dev);
+	__atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
+	return 0;
+}
+
 /* return 0 means link status changed, -1 means not changed */
 int
 txgbe_dev_link_update_share(struct rte_eth_dev *dev,
 			    int wait_to_complete)
 {
+	struct txgbe_adapter *ad = TXGBE_DEV_ADAPTER(dev);
 	struct txgbe_hw *hw = TXGBE_DEV_HW(dev);
 	struct rte_eth_link link;
 	u32 link_speed = TXGBE_LINK_SPEED_UNKNOWN;
@@ -2763,10 +2806,24 @@ txgbe_dev_link_update_share(struct rte_eth_dev *dev,
 		if ((hw->subsystem_device_id & 0xFF) ==
 				TXGBE_DEV_ID_KR_KX_KX4) {
 			hw->mac.bp_down_event(hw);
-		} else if (hw->phy.media_type == txgbe_media_type_fiber) {
-			intr->flags |= TXGBE_FLAG_NEED_LINK_CONFIG;
-			rte_eal_alarm_set(10,
-				txgbe_dev_setup_link_alarm_handler, dev);
+		} else if (hw->phy.media_type == txgbe_media_type_fiber &&
+				dev->data->dev_conf.intr_conf.lsc != 0) {
+			txgbe_dev_wait_setup_link_complete(dev, 0);
+			if (!__atomic_test_and_set(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
+				/* To avoid race condition between threads, set
+				 * the TXGBE_FLAG_NEED_LINK_CONFIG flag only
+				 * when there is no link thread running.
+				 */
+				intr->flags |= TXGBE_FLAG_NEED_LINK_CONFIG;
+				if (rte_thread_create(&ad->link_thread_tid, NULL,
+					txgbe_dev_setup_link_thread_handler, dev) < 0) {
+					PMD_DRV_LOG(ERR, "Create link thread failed!");
+					__atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
+				}
+			} else {
+				PMD_DRV_LOG(ERR,
+					"Other link thread is running now!");
+			}
 		}
 		return rte_eth_linkstatus_set(dev, &link);
 	} else if (!hw->dev_start) {
diff --git a/drivers/net/txgbe/txgbe_ethdev.h b/drivers/net/txgbe/txgbe_ethdev.h
index 6a18865e23..b8a39204e2 100644
--- a/drivers/net/txgbe/txgbe_ethdev.h
+++ b/drivers/net/txgbe/txgbe_ethdev.h
@@ -369,6 +369,9 @@ struct txgbe_adapter {
 
 	/* For RSS reta table update */
 	uint8_t rss_reta_updated;
+
+	uint32_t link_thread_running;
+	rte_thread_t link_thread_tid;
 };
 
 #define TXGBE_DEV_ADAPTER(dev) \
@@ -560,6 +563,9 @@ void txgbe_configure_dcb(struct rte_eth_dev *dev);
 int
 txgbe_dev_link_update_share(struct rte_eth_dev *dev,
 		int wait_to_complete);
+int
+txgbe_dev_wait_setup_link_complete(struct rte_eth_dev *dev,
+		uint32_t timeout_ms);
 int txgbe_pf_host_init(struct rte_eth_dev *eth_dev);
 
 void txgbe_pf_host_uninit(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/txgbe/txgbe_ethdev_vf.c b/drivers/net/txgbe/txgbe_ethdev_vf.c
index 3b1f7c913b..f1341fbf7e 100644
--- a/drivers/net/txgbe/txgbe_ethdev_vf.c
+++ b/drivers/net/txgbe/txgbe_ethdev_vf.c
@@ -165,6 +165,7 @@ eth_txgbevf_dev_init(struct rte_eth_dev *eth_dev)
 {
 	int err;
 	uint32_t tc, tcs;
+	struct txgbe_adapter *ad = eth_dev->data->dev_private;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	struct txgbe_hw *hw = TXGBE_DEV_HW(eth_dev);
@@ -205,6 +206,7 @@ eth_txgbevf_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
+	__atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 
 	hw->device_id = pci_dev->id.device_id;
@@ -618,7 +620,7 @@ txgbevf_dev_start(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 
 	/* Stop the link setup handler before resetting the HW. */
-	rte_eal_alarm_cancel(txgbe_dev_setup_link_alarm_handler, dev);
+	txgbe_dev_wait_setup_link_complete(dev, 0);
 
 	err = hw->mac.reset_hw(hw);
 	if (err) {
@@ -720,7 +722,7 @@ txgbevf_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	rte_eal_alarm_cancel(txgbe_dev_setup_link_alarm_handler, dev);
+	txgbe_dev_wait_setup_link_complete(dev, 0);
 
 	txgbevf_intr_disable(dev);
 
-- 
2.25.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2023-08-09 21:51:19.846628300 +0800
+++ 0065-net-txgbe-fix-blocking-system-events.patch	2023-08-09 21:51:18.204352000 +0800
@@ -1 +1 @@
-From 5e170dd8b690cd55a2931ddbfe3db0f9b1cad462 Mon Sep 17 00:00:00 2001
+From 09d8d1fe4f7b75079d1aede67252b8c32f210033 Mon Sep 17 00:00:00 2001
@@ -4,0 +5,3 @@
+Cc: Xueming Li <xuemingl at nvidia.com>
+
+[ upstream commit 5e170dd8b690cd55a2931ddbfe3db0f9b1cad462 ]
@@ -17 +19,0 @@
-Cc: stable at dpdk.org
@@ -21 +23 @@
- drivers/net/txgbe/txgbe_ethdev.c    | 70 ++++++++++++++++++++++++++---
+ drivers/net/txgbe/txgbe_ethdev.c    | 69 ++++++++++++++++++++++++++---
@@ -24 +26 @@
- 3 files changed, 74 insertions(+), 8 deletions(-)
+ 3 files changed, 73 insertions(+), 8 deletions(-)
@@ -27 +29 @@
-index 74765a469d..d942b542ea 100644
+index 95aef27092..2c7d71c0db 100644
@@ -30 +32 @@
-@@ -546,6 +546,7 @@ null:
+@@ -545,6 +545,7 @@ null:
@@ -38 +40 @@
-@@ -594,6 +595,7 @@ eth_txgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
+@@ -593,6 +594,7 @@ eth_txgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
@@ -46 +48,2 @@
-@@ -1680,7 +1682,7 @@ txgbe_dev_start(struct rte_eth_dev *dev)
+@@ -1668,7 +1670,7 @@ txgbe_dev_start(struct rte_eth_dev *dev)
+ 	PMD_INIT_FUNC_TRACE();
@@ -49 +51,0 @@
- 	rte_eal_alarm_cancel(txgbe_dev_detect_sfp, dev);
@@ -55 +57,2 @@
-@@ -1919,7 +1921,7 @@ txgbe_dev_stop(struct rte_eth_dev *dev)
+@@ -1906,7 +1908,7 @@ txgbe_dev_stop(struct rte_eth_dev *dev)
+ 
@@ -58 +60,0 @@
- 	rte_eal_alarm_cancel(txgbe_dev_detect_sfp, dev);
@@ -64 +66 @@
-@@ -2803,11 +2805,52 @@ txgbe_dev_setup_link_alarm_handler(void *param)
+@@ -2722,11 +2724,52 @@ txgbe_dev_setup_link_alarm_handler(void *param)
@@ -117 +119 @@
-@@ -2844,10 +2887,25 @@ txgbe_dev_link_update_share(struct rte_eth_dev *dev,
+@@ -2763,10 +2806,24 @@ txgbe_dev_link_update_share(struct rte_eth_dev *dev,
@@ -134,2 +136 @@
-+				if (rte_thread_create_control(&ad->link_thread_tid,
-+					"txgbe-link-thread", NULL,
++				if (rte_thread_create(&ad->link_thread_tid, NULL,
@@ -148 +149 @@
-index c59b6370bb..6b296d6fd1 100644
+index 6a18865e23..b8a39204e2 100644
@@ -151 +152 @@
-@@ -370,6 +370,9 @@ struct txgbe_adapter {
+@@ -369,6 +369,9 @@ struct txgbe_adapter {
@@ -161 +162 @@
-@@ -561,6 +564,9 @@ void txgbe_configure_dcb(struct rte_eth_dev *dev);
+@@ -560,6 +563,9 @@ void txgbe_configure_dcb(struct rte_eth_dev *dev);


More information about the stable mailing list