From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 740BB42D94; Fri, 30 Jun 2023 11:21:02 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4439E406B5; Fri, 30 Jun 2023 11:21:02 +0200 (CEST) Received: from smtpbgau2.qq.com (smtpbgau2.qq.com [54.206.34.216]) by mails.dpdk.org (Postfix) with ESMTP id 331254021F; Fri, 30 Jun 2023 11:20:58 +0200 (CEST) X-QQ-mid: bizesmtp82t1688116854tq996xdg Received: from wxdbg.localdomain.com ( [125.120.155.89]) by bizesmtp.qq.com (ESMTP) with id ; Fri, 30 Jun 2023 17:20:46 +0800 (CST) X-QQ-SSF: 01400000000000K0Z000000A0000000 X-QQ-FEAT: Fc2LLDWeHZ96f7Wpj7f8eKcQvt6oEP5XQqqZmkNRm3q2CPE5y5xWtOJkVhltw VFybYKr1VbMnbIaQGskFsxlw3KQbtIaiLYDpQqtHHjPZAuEgd8eSd+wyog+1iINduHne5d6 TrR3IxauuiEQPaJo+SxUBuL1fgw3l6jC5/llSbH1Pq1rIj7/2YzKkkvWOdsDlkBUtgEBTqL S28MFz1AdMvdTabE1A+Hc3jm3rbjMrZQKLlvpOTw0uHfS/qfhOZV1NboSRIsONxITvn8CM5 +Rhqc9px3LrCCuCpY8GBM0iGT2iErk0XzkNPrIoSZcK6ZCbA+KTHzaRGau1ZZhDI5gOhdJO +/A79GcmIge+8box1Wo0F9tMAeoUs6VxsTvrOtwbKpFmTXFNs8b2n32+t+lqj2nNELVRqmW X-QQ-GoodBg: 2 X-BIZMAIL-ID: 2196687223104991358 From: Jiawen Wu To: dev@dpdk.org Cc: Jiawen Wu , stable@dpdk.org Subject: [PATCH v2] net/txgbe: fix blocking system events Date: Fri, 30 Jun 2023 17:35:12 +0800 Message-Id: <20230630093512.314184-1-jiawenwu@trustnetic.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20230629100735.297306-1-jiawenwu@trustnetic.com> References: <20230629100735.297306-1-jiawenwu@trustnetic.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:trustnetic.com:qybglogicsvrgz:qybglogicsvrgz5a-1 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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") Cc: stable@dpdk.org Signed-off-by: Jiawen Wu --- drivers/net/txgbe/txgbe_ethdev.c | 70 ++++++++++++++++++++++++++--- drivers/net/txgbe/txgbe_ethdev.h | 6 +++ drivers/net/txgbe/txgbe_ethdev_vf.c | 6 ++- 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c index 74765a469d..d942b542ea 100644 --- a/drivers/net/txgbe/txgbe_ethdev.c +++ b/drivers/net/txgbe/txgbe_ethdev.c @@ -546,6 +546,7 @@ txgbe_parse_devargs(struct txgbe_hw *hw, struct rte_devargs *devargs) 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); @@ -594,6 +595,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; @@ -1680,7 +1682,7 @@ txgbe_dev_start(struct rte_eth_dev *dev) /* Stop the link setup handler before resetting the HW. */ rte_eal_alarm_cancel(txgbe_dev_detect_sfp, dev); - 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); @@ -1919,7 +1921,7 @@ txgbe_dev_stop(struct rte_eth_dev *dev) PMD_INIT_FUNC_TRACE(); rte_eal_alarm_cancel(txgbe_dev_detect_sfp, dev); - 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); @@ -2803,11 +2805,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; @@ -2844,10 +2887,25 @@ 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_control(&ad->link_thread_tid, + "txgbe-link-thread", 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 c59b6370bb..6b296d6fd1 100644 --- a/drivers/net/txgbe/txgbe_ethdev.h +++ b/drivers/net/txgbe/txgbe_ethdev.h @@ -370,6 +370,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) \ @@ -561,6 +564,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.27.0